Skip to content
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

bpo-4032: Cygwin: Add .dll.a to UnixCCompiler for searching libraries #4153

Closed
wants to merge 3 commits into from

Conversation

ma8ma
Copy link
Contributor

@ma8ma ma8ma commented Oct 27, 2017

Allow the UnixCCompiler.find_library_file to correctly find DLL import libs (.dll.a).

This is an alternative PR for #4136:

This simple patch allows UnixCCompiler.find_library_file, which searching library directories (e.g. /usr/lib), to recognize .dll.a as matches for the library. This is necessary on Cygwin where actual DLLs are not typically found under /lib, but rather /bin (because Windows searches PATH for DLLs). However one does .dll.a import libs under /lib (these are simple static archives that contain stubs for symbols actually found in the DLL, which can be used in lieu of the DLL itself for linking). This satisfies that the library being searched for is typically found.

This is especially necessary now that the _ctypes module can only be built with --with-system-ffi; without this patch the system libffi cannot be found.

The main difference to the original is the use of the implib_lib_extension variable. The original patch will overwrite the dylib_lib_extension variable which previously only had meaning on OSX, but is a bit of a misnomer for searching for DLL import libs on Cygwin.

https://bugs.python.org/issue4032

@embray
Copy link
Contributor

embray commented Dec 6, 2017

Hi @ma8ma got pulled away to other things again, but I want to get back to this now. I have everything I need in place to run a CPython buildbot worker, we just need this PR and a few others merged so that it can work somewhat.

@ma8ma
Copy link
Contributor Author

ma8ma commented Dec 8, 2017

@embray Thank you for preparing the buildbot worker! Note that deadline of Python 3.7 features comes soon... so if you get ready for adding the worker, would you post on python-dev to request reviewing this PR?

@embray
Copy link
Contributor

embray commented Dec 8, 2017

I will, though I've run into a problem getting the buildbot worker up and running, specifically: https://bugs.python.org/issue32243

Besides that there are a couple other issues (for which I have workarounds) that cause the test suite to hang. For the time being I could ask to add a Cygwin-specific build that is configured to skip those tests for the time being, but it's not ideal.

@ma8ma
Copy link
Contributor Author

ma8ma commented Dec 9, 2017

I see. Hmm, I'm not sure core developers will accept this change without a buildbot right now, therefore I won't get in present master (i.e. 3.7) branch while they aren't resolved...

@embray
Copy link
Contributor

embray commented Jun 12, 2019

It would be great if we could get this merged. This is still a blocker to solving various other issues, because without this fix we can't build ctypes on Cygwin anymore.

@embray
Copy link
Contributor

embray commented Jun 12, 2019

@ma8ma Could you rebase this PR? Or if not I'll open a new one based on it.

@ma8ma
Copy link
Contributor Author

ma8ma commented Jun 12, 2019

Hi @embray .
I'll do that!

@embray
Copy link
Contributor

embray commented Jun 12, 2019

@ma8ma Awesome, thank you!

@embray
Copy link
Contributor

embray commented Sep 25, 2019

PR #14013 is still dependent on this PR which has created some confusion. I think this should be an uncontroversial patch and just needs review. This is a patch that has always been needed for extension modules to compile properly on Cygwin.

@iritkatriel
Copy link
Member

Closing the PR following the closure of its b.p.o issue (https://bugs.python.org/issue4032)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants