Pass location of built zlib to LLVM build #42622
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks! Does everything still work after a |
|
Yeah everything seemed to work after the nix build finished (which includes make install and removal of build dir) However I have not tested on other systems, like what you use normally for building the binaries, but I assume the GH checks would cover that? |
|
The tester_{linux32,linux64,linuxaarch64} failures are all of the same form and unrelated (breakage on master? still catching up this morning): The tester_win64 failure is also unrelated: tester_freebsd64 exhibits the Download failures of testers mentioned above and also fails in a couple other seemingly unrelated ways, probably known: and particularly which I think is to say that CI is happy modulo unrelated breakage. |
|
Confirmed, the Download failures occurred on other jobs launched at roughly the same time. Cycling those jobs. |
vchuravy
approved these changes
Oct 13, 2021
This was referenced Aug 2, 2024
giordano
added a commit
that referenced
this pull request
Aug 6, 2024
### Elaboration of the issue After #55180 we implicitly require an LLVM built with Zlib support, but compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM without Zlib support, despite the fact we attempt to request it at https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97 This was first identified in #55337. ### Explanation of how configuration of LLVM failed `ZLIB_LIBRARY` must be the path to the zlib library, but we currently set it to the libdir where the library is installed (introduced in #42622): https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97 which is wrong. However, CMake is actually able to find Zlib correctly, but then the check at https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141 uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test, but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib and thus tries to run the test without linking any Zlib, and the test silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`), resulting in no usable Zlib available, even if found. ### Proposed solution `ZLIB_ROOT` is the only [hint recommended by the CMake module `FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints). This PR replaces a broken `ZLIB_LIBRARY` with an appropriate `ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only way to make CMake fail loudly if no usable Zlib is available, and avoid going on with a non-usable build. ### Other comments I confirm this fixes #55337 for me, it should likely address JuliaCI/julia-buildkite#373 as well. Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`, `COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore, and they are removed.
lazarusA
pushed a commit
to lazarusA/julia
that referenced
this pull request
Aug 17, 2024
### Elaboration of the issue After JuliaLang#55180 we implicitly require an LLVM built with Zlib support, but compiling Julia with `make USE_BINARYBUILDER_LLVM=0` builds an LLVM without Zlib support, despite the fact we attempt to request it at https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97 This was first identified in JuliaLang#55337. ### Explanation of how configuration of LLVM failed `ZLIB_LIBRARY` must be the path to the zlib library, but we currently set it to the libdir where the library is installed (introduced in JuliaLang#42622): https://github.com/JuliaLang/julia/blob/996351f5f6651d1508aef3c35c7d37eb22a0fb1e/deps/llvm.mk#L97 which is wrong. However, CMake is actually able to find Zlib correctly, but then the check at https://github.com/llvm/llvm-project/blob/46425b8d0fac3c529aa4a716d19abd7032e452f3/llvm/cmake/config-ix.cmake#L139-L141 uses the value of `ZLIB_LIBRARY` to list the Zlib to link for the test, but being `ZLIB_LIBRARY` a directory, CMake doesn't see any valid Zlib and thus tries to run the test without linking any Zlib, and the test silently fails (they're silent only when `LLVM_ENABLE_ZLIB=ON`), resulting in no usable Zlib available, even if found. ### Proposed solution `ZLIB_ROOT` is the only [hint recommended by the CMake module `FindZLIB`](https://cmake.org/cmake/help/latest/module/FindZLIB.html#hints). This PR replaces a broken `ZLIB_LIBRARY` with an appropriate `ZLIB_ROOT`. Also, we set `LLVM_ENABLE_ZLIB=FORCE_ON` which is the only way to make CMake fail loudly if no usable Zlib is available, and avoid going on with a non-usable build. ### Other comments I confirm this fixes JuliaLang#55337 for me, it should likely address JuliaCI/julia-buildkite#373 as well. Also, options `COMPILER_RT_ENABLE_IOS`, `COMPILER_RT_ENABLE_WATCHOS`, `COMPILER_RT_ENABLE_TVOS`, and `HAVE_HISTEDIT_H` don't exist anymore, and they are removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes the source build when run in Nix.
Related: #42524