[Build] Fix llvm package to use correct glibc version checks#8391
[Build] Fix llvm package to use correct glibc version checks#8391HollowMan6 wants to merge 2 commits into
Conversation
|
Updated the |
|
Let me run the CI and see if anything wrong |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
|
can't we know the glibc version based on the linux distribution version? We could add that to the package name. |
|
I've given some thought to this problem, so I'll share my thoughts here. (but feel free to disregard them) To summarise the builds:
Both libc and libstdc++ on the system need to be newer in order to build. But it's slightly awkward because the Ubuntu build has a new libc and an old libstdc++, while the AlmaLinux build has the opposite. My thinking on a solution would be to downgrade the libstdc++ used for the AlmaLinux build (which is a simple change #8405), so it is compatible with a superset of the systems that the Ubuntu build is compatible with. And then make the AlmaLinux LLVM binaries the only version Triton distributes. Those binaries should work on almost every system, eliminating the need for checks. |
I don't think we already have such of list available somewhere, and maintaining it can also be time-consuming, as the list won't be exhaustive and can be outdated after a while at any time.
I think the original code already has support for setting
That could be another solution, but it will introduce breaking (building environments) changes for Linux distributions that use newer glibc versions, so I guess we will need to be careful if we decide to go this way, so that nothing is broken. I'm using SUSE Linux Enterprise Server 15 SP5, and AlmaLinux LLVM binaries work for me. |
Both glibc and libstdc++ maintain backward ABI compatibility. We already rely on that since we don't enforce exact matches, so it makes sense to me to target the oldest possible version of both. Fortunately LLVM coding standards require maintaining compatibility with even older compilers than we are considering here. Are you referring to something else? |
|
I just addressed the previously identified issue. @codex review |
|
To use Codex here, create a Codex account and connect to github. |
I'm not sure about that for 100%, also if similar things, just like you mentioned in #8191, happen for AlmaLinux in the future, we will still need to revert back to using Ubuntu builds. (The CentOS-based LLVM build doesn't work on my side with SUSE Linux Enterprise Server 15 SP5). So it might still be a good idea to also have the option to base the builds on something new so that we have some kind of backup. But anyway, this is up to the maintainer's decision, and I'm OK with any possible solution. |
Currently, we will get the following error when we try to build on a system with glic version 2.31: ```log : /lib64/libc.so.6: version `GLIBC_2.34' not found (required by ./lld) : /lib64/libc.so.6: version `GLIBC_2.32' not found (required by ./lld) : /lib64/libc.so.6: version `GLIBC_2.33' not found (required by ./lld) ``` The reason is that llvm-f6ded0be-ubuntu-x64 requires at least glibc 2.34 now, but the glibc version checks still think the minimum glibc version it requires is 2.29. So this PR updates by using a flexible way to set version numbers and fixes glibc version checks according to binary inspection: - llvm-f6ded0be-ubuntu-x64 requires at least glibc 2.34 - llvm-f6ded0be-almalinux-x64 requires at least glibc 2.27 - llvm-f6ded0be-centos-x64 requires at least glibc 2.16 Also, we should fall back to user-configured LLVM from source build if all the glib versions don't satisfy the needs here. Command used to check the glibc required version: `objdump -T ./bin/lld | grep GLIBC_ | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu` ```logs llvm-f6ded0be-ubuntu-x64/bin> objdump -T ./lld | grep GLIBC_ | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu 2.2.5 2.3 2.3.4 2.6 2.14 2.15 2.16 2.27 2.29 2.32 2.33 2.34 llvm-f6ded0be-almalinux-x64/bin> objdump -T ./lld | grep GLIBC_ | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu 2.2.5 2.3 2.3.4 2.6 2.12 2.14 2.15 2.16 2.27 llvm-f6ded0be-centos-x64/bin> objdump -T ./lld | grep GLIBC_ | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu 2.2.5 2.3 2.3.4 2.4 2.6 2.12 2.14 2.15 2.16 ``` Signed-off-by: Hollow Man <hollowman@opensuse.org>
|
ping @Jokeren |
There was a problem hiding this comment.
Pull request overview
This PR fixes glibc version checking logic for LLVM package selection to resolve build failures on systems with glibc < 2.34. The fix ensures that the correct pre-compiled LLVM package variant is selected based on the system's actual glibc version.
Key changes:
- Updated glibc version requirements for LLVM packages based on binary inspection (ubuntu-x64 requires 2.34, almalinux-x64 requires 2.27)
- Introduced a dynamic configuration system that reads glibc requirements from text files instead of hardcoded values
- Added a bash script to automatically update glibc version requirements when new LLVM builds are available
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| setup.py | Replaces hardcoded glibc version checks with dynamic config file reading; updates x64 Linux package selection logic to iterate through available configs and select the most appropriate LLVM build |
| cmake/llvm-vglibc/update.sh | Adds automation script to download LLVM pre-compiled builds, inspect binaries for glibc dependencies, and generate version requirement files |
| cmake/llvm-vglibc/ubuntu-x64.txt | Specifies minimum glibc version 2.34 for ubuntu-x64 LLVM builds |
| cmake/llvm-vglibc/almalinux-x64.txt | Specifies minimum glibc version 2.27 for almalinux-x64 LLVM builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolves #7088, #6747
Currently, we will get the following error when we try to build on a system with glic version 2.31:
The reason is that llvm-f6ded0be-ubuntu-x64 requires at least glibc 2.34 now, but the glibc version checks still thinks the minimum glibc version it requires is 2.29. So this PR fixes glibc version checks according to binary inspection:
Also, we should fall back to user-configured LLVM from source build if all the glib version doesn't satisfy the needs here.
Command used to check glibc required version:
objdump -T ./bin/lld | grep GLIBC_ | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -VuNew contributor declaration
I am not making a trivial change, such as fixing a typo in a comment.
I have written a PR description following these
rules.
I have run
pre-commit run --from-ref origin/main --to-ref HEAD.Select one of the following.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end testsThis is a fix related to build.Select one of the following.
littests.littests I have added follow these best practices,including the "tests should be minimal" section. (Usually running Python code
and using the instructions it generates is not minimal.)