-
Notifications
You must be signed in to change notification settings - Fork 154
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
Fixes for building bindings on cygwin #276
base: master
Are you sure you want to change the base?
Conversation
Building the tcl binding as a SHARED type library means that an INSTALL of that target is treated as a RUNTIME target on DLL platforms, which currently has no DESTINATION provided. Change the type to MODULE (the same as other bindings), as an INSTALL of a target of that type uses the LIBRARY target on all platforms.
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.
I'm not familiar with cygwin stuff so I can't test it. However from first glance looks good.
I wonder if the change won't lead to having library references on non-cygwin systems, so maybe we want to do this conditionally. I'll do some testing. I'm also curious why cygwin needs this and other systems don't. Thanks for the pull request! |
@mlschroe well, technically it works by accident...
|
But that's not an accident. It works, because it is loaded from the library it extends (i.e. as a "plugin"). I'm just wondering why it doesn't work that way in cygwin. |
This is what I call "by accident". In any case, I think solution is correct because whatever is built using setuptools with native extension are linked against libpython..
|
And since I'm planning to use |
Hmm, but all of the perl modules and some of the python modules on my system have those undefined references. So why should libsolv be different? |
Because dynamic linkage works differently for PE, specifically the library which provides a symbol is resolved at link time, not by the loader. |
The discussion didn't seems to reach a conclusion on this. Should I update this PR to make full linkage conditional on Win32? |
Follow up to #211