-
Notifications
You must be signed in to change notification settings - Fork 90
DO NOT MERGE: makefile to make windows great again #19605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
29f1f77 to
8b869e7
Compare
Jenkins BuildsClick to see older builds (30)
|
8b869e7 to
b894d0f
Compare
Ivansete-status
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpick comments so far that I hope you find useful
| NIMSDS_IMPORT_LIB := $(NIMSDS_LIBDIR)/libsds.dll.a | ||
|
|
||
| $(NIMSDS_DEF): $(NIMSDS_DLL) | ||
| @echo -e "\033[92mCreating:\033[39m libsds.def" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can create the libsds.def file in nim-sds repo.
Why is it needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows with MinGW, when you link against a DLL using -lsds, the linker needs an import library (.dll.a file) that contains the list of exported symbols.
libsds.def is a module definition file that tells dlltool which symbols to include in the import library. Without it, dlltool may fail to extract all exports from the DLL automatically.
The .def file simply lists:
EXPORTS
SdsCleanupReliabilityManager
SdsSetEvent
Then dlltool uses it to create libsds.dll.a:
dlltool --dllname libsds.dll --def libsds.def --output-lib libsds.dll.a
Some build failures (undefined reference to 'SdsSetEventCallback') happened because dlltool --output-def didn't reliably extract all symbols from the DLL. Manually specifying them in the .def file ensures they're all included.
| $(NIMSDS_IMPORT_LIB): $(NIMSDS_DLL) $(NIMSDS_DEF) | ||
| @echo -e "\033[92mCreating:\033[39m libsds.dll.a" | ||
| @rm -f $(NIMSDS_IMPORT_LIB) | ||
| @cd $(NIMSDS_LIBDIR) && dlltool --dllname libsds.dll --def libsds.def --output-lib libsds.dll.a || \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, I think we can do that in nim-sds repo, instead :)
|
@Ivansete-status thats AI freestyle 😂 I am not planning to merge it, just sharing what helped me to bypass building issue i have on my machine |
b894d0f to
c4aa408
Compare
|
@alexjba @Ivansete-status @igor-sirotin @siddarthkay asking for your wisdom guys! added a task to collect details #19608 |
What does the PR do
experimental, to resolve issues with building app on my windows machine
Fixes #19608
my laptop spec is this
i am able to build the app now with this branch, but i cant build master
i wonder what is needed (with evaluation of these changes ofc) to be done to our makefile / build process to allow me to build windows again