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

Remove indirect dependency on system ZLIB #26888

Merged
merged 2 commits into from
Apr 30, 2018
Merged

Remove indirect dependency on system ZLIB #26888

merged 2 commits into from
Apr 30, 2018

Conversation

vchuravy
Copy link
Sponsor Member

IIUC we have been trying to minimise our dependency tree and one of those dependencies we previously excised was ZLIB.
I reviewed a build of mine that I had laying around and besides system libraries the only indirect dependency (e.g. that we don't provide by default and that is pulled in by one of our main dependency)
was ZLIB, which was pulled in by LLVM and libgit. This builds LLVM without ZLIB support (should be a NFC since we don't use compression) and uses the bundled ZLIB for libgit.

Indirect dependencies can cause issues for downstream users like Conda.jl so IMHO it is best to minimise them as much as possible.

- build LLVM without ZLIB support
- use the bundled ZLIB for libgit
@vchuravy vchuravy added domain:building Build system, or building Julia or its dependencies backport pending 0.6 external dependencies Involves LLVM, OpenBLAS, or other linked libraries labels Apr 23, 2018
@vchuravy
Copy link
Sponsor Member Author

Note: this should be a squash merge.

@vchuravy vchuravy requested a review from vtjnash April 23, 2018 21:21
@vtjnash
Copy link
Sponsor Member

vtjnash commented Apr 23, 2018

should be a NFC since we don't use compression

In some cases, we may try to use zlib to read debug info (dwarf dwz files). LLVM should fail gracefully here (e.g. just return "debug info not present") if it is turned off, so we'll just lose line number information for those libraries, but it's not entirely NFC.

@stevengj
Copy link
Member

stevengj commented Apr 24, 2018

Hopefully this will resolve JuliaPy/PyPlot.jl#151, which is a longstanding issue arising from Julia linking an older system version of ZLIB than the one required by Anaconda's libraries?

Is the bundled zlib in libgit linked in such a way that it won't conflict with dlopening libraries that depend on a different zlib?

@ararslan ararslan mentioned this pull request Apr 26, 2018
7 tasks
@ararslan
Copy link
Member

Good to go?

@ararslan
Copy link
Member

Let's give this guy another run through CI just in case. I believe Elliot has since fixed the issue on macOS.

@ararslan
Copy link
Member

Oh, I think the tests failed because of that httpbin thing. Let's try this again. Sorry for the close/reopen spam, Valentin.

@ararslan ararslan closed this Apr 30, 2018
@ararslan ararslan reopened this Apr 30, 2018
@ararslan
Copy link
Member

All green except 64-bit Windows, which timed out.

@ararslan ararslan merged commit 8770821 into master Apr 30, 2018
@ararslan ararslan deleted the vc/indirect branch April 30, 2018 21:25
ararslan pushed a commit that referenced this pull request Apr 30, 2018
* Remove indirect dependency on system ZLIB

- build LLVM without ZLIB support
- use the bundled ZLIB for libgit

* backport bundled zlib patch

Ref #26888
(cherry picked from commit 8770821)
ararslan pushed a commit that referenced this pull request May 8, 2018
* Remove indirect dependency on system ZLIB

- build LLVM without ZLIB support
- use the bundled ZLIB for libgit

* backport bundled zlib patch

Ref #26888
(cherry picked from commit 8770821)
vchuravy added a commit to staticfloat/LLVMBuilder that referenced this pull request May 19, 2018
ararslan pushed a commit that referenced this pull request May 27, 2018
* Remove indirect dependency on system ZLIB

- build LLVM without ZLIB support
- use the bundled ZLIB for libgit

* backport bundled zlib patch

Ref #26888
(cherry picked from commit 8770821)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:building Build system, or building Julia or its dependencies external dependencies Involves LLVM, OpenBLAS, or other linked libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants