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

update wheel builder script for static linking on linux #3811

Merged
merged 1 commit into from
Jul 25, 2017

Conversation

reaperhulk
Copy link
Member

We don't need to do an LD_LIBRARY_PATH when calling auditwheel because we're now statically linking OpenSSL.

This is blocked on:

We should consider doing a 2.0.1 for this probably.

We don't need to do an LD_LIBRARY_PATH when calling auditwheel because
we're now statically linking OpenSSL.
@mention-bot
Copy link

@reaperhulk, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alex to be a potential reviewer.

@alex
Copy link
Member

alex commented Jul 25, 2017

Can you please explain how this PR works? Shouldn't we have to use SUPRESS_LINK_FLAGS?

@reaperhulk
Copy link
Member Author

I honestly don't understand how static linking works on Linux. I actually used the documentation @Lukasa wrote up in our docs to do this and then confirmed that _openssl.so wasn't linked against libssl or libcrypto via ldd. It makes sense that we would want to suppress linker flags like we do on macOS, but maybe that's unnecessary (or maybe this is only working via coincidence). Sorry to do this again, but cc @njsmith (sorry Nathaniel!)

@njsmith
Copy link
Contributor

njsmith commented Jul 25, 2017

What's SUPPRESS_LIBRARY_FLAGS?

@alex
Copy link
Member

alex commented Jul 25, 2017

@njsmith
Copy link
Contributor

njsmith commented Jul 25, 2017

I think you want to not set SUPPRESS_LIBRARY_FLAGS on Linux. Right now you're eventually passing -lcrypto -lssl to the linker, and so it's looking for libcrypto.so.X or libcrypto.a, finding the latter, and linking it on. Likewise for libssl. You could stop passing those, but then you'd need to have some other way to get the information to the linker that you want to link to openssl. Which is possible and I guess must be what you're doing on Darwin for some sensible reason that I don't know about, but why jump through those hoops?

... It does randomly occur to me that you might want to double check that your static openssl is being built with -fPIC, but that's unrelated.

@reaperhulk
Copy link
Member Author

reaperhulk commented Jul 25, 2017 via email

@alex
Copy link
Member

alex commented Jul 25, 2017

I did not realize that -l would hapilly look for either a .so OR a .a this makes sense to me now.

@alex alex merged commit 7ad62db into pyca:master Jul 25, 2017
@alex alex added this to the Twenty first release milestone Jul 25, 2017
@njsmith
Copy link
Contributor

njsmith commented Jul 25, 2017

pass absolute paths to the .a files themselves.

You can do that too. I believe that the only difference is that -l triggers the library search logic, so you might prefer to pass absolute paths if you don't trust the search logic.

reaperhulk added a commit to reaperhulk/cryptography that referenced this pull request Jul 26, 2017
We don't need to do an LD_LIBRARY_PATH when calling auditwheel because
we're now statically linking OpenSSL.
alex pushed a commit that referenced this pull request Jul 26, 2017
* update wheel builder script for static linking on linux (#3811)

We don't need to do an LD_LIBRARY_PATH when calling auditwheel because
we're now statically linking OpenSSL.

* Fixed #3801 -- don't create py33 wheels (#3802)
openstack-gerrit pushed a commit to openstack/openstack-ansible that referenced this pull request Jul 2, 2018
Cryptography is shipping with wheels [1], and recent bugs for
dynamic linking in wheel building [2] should be fixed now.

We should therefore use the wheels.

[1]: https://pypi.org/project/cryptography/#files
[2]: pyca/cryptography#3811

Change-Id: I1b7e1ee1e127569b488d7202ae601ee17701d3f0
@reaperhulk reaperhulk deleted the static-link branch November 3, 2018 01:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants