python-hyper / brotlicffi

Python bindings to the Brotli compression library
MIT License
147 stars 27 forks source link

Allow to build with shared brotli #94

Closed felixonmars closed 7 years ago

felixonmars commented 7 years ago

It would be nice to allow building with shared brotli since we have one in the repositories. This commit would not break the default installation.

sigmavirus24 commented 7 years ago

It might look like I have commit on here, but I don't.

I'll say this: I just witnessed an interaction between someone on Debian sid who apparently forcibly installed a different version of libssl (ABI incompatible with 1.1.0) to satisfy a commercial package but was trying to install python's cryptography library and wanted to use libssl-1.1.0 but couldn't unbreak their system. In other words, they now have to compromise their python application's security by using an unmainted series of OpenSSL. Shared libraries here are a nice ideal, but a scary reality.

felixonmars commented 7 years ago

I understand, that's why I didn't make it just switch to shared libraries when available. With an option like this PR, downstream distributions don't have to do the same thing themselves, and normal users would not experience the mentioned problems :)

Lukasa commented 7 years ago

I don't think I have any objection to this: however, it'd be really nice to have a builder that actually demonstrates that this works. Can we arrange to do that somehow? Maybe by adding a Travis job that builds brotli as shared libraries first and then installs this way?

sigmavirus24 commented 7 years ago

Ah, @felixonmars I do like what you've done. I hadn't looked at the diff. I was mostly reacting to the surprising timing :)

Cheers! :tada:

felixonmars commented 7 years ago

@Lukasa Updated Travis conf to test in both cases :)

Lukasa commented 7 years ago

I'm glad I asked for a Travis build, because this seems not to be working. ;)

felixonmars commented 7 years ago

@Lukasa Failed due to a broken Travis conf :P Now all passing.