-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Guix: Fix building for i686-linux-gnu #24448
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
Conversation
|
I'm ok with doing the minimal here, but a reminder that supporting i686 is outside the goals of this project. So NACK on reintroducing Edit: also as said on IRC, with gitian it was the best we could do, as we were bound to ubuntu's toolchains, with guix we control the entire build environment including toolchains so could patch those if necessary. There's no excuse for having something like --enable-glibc-back-compat in the leaf bitcoin-core build. But a more trivial solution is still preferred. Would bumping the minimum libc version for i686 help? |
| return False | ||
| return True | ||
|
|
||
| def control_flow_instruction_for(binary): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK on the security check changes
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
x86-32 uses 2 instead of 3
GCC 7 introduced the __divmoddi4 function required for x86-32 builds. This shifts the minimum required Debian version to 10, and Ubuntu to 18.04. No change to CentOS support (8).
d6b1692 to
e58867d
Compare
|
re foreign code: this version is very simple (likely too simple for copyright), but in the worst case is licensed under MIT the same as our code. see LLVM code here. (But the other reasons not to reintroduce glibc-back-compat still hold) re patching the toolchain: I'm not aware of any easy patch to make GCC 7+ not require this. Maybe we could include __divmoddi4 in crti.o or something, but that feels ugly too. In theory -static-libgcc should do the job - but I don't know what other ramifications that might have. re bumping the minimum libc version for i686: this doesn't involve libc, but we could presumably bump the minimum libgcc version. This would lose support for Debian stretch (supported under June), requiring Debian 10 (buster, oldstable, released 2019 July). It would lose Ubuntu xenial (supported until 2026), requiring at least bionic (released 2018). CentOS gained a new enough version with CentOS 8 (released 2019 September); we already don't support older versions of CentOS. FWIW, I also have a device that requires Linux 2.6.32, which holds it back to glibc 2.25, but has fully updated to GCC 11 with no problems. Dropped the glibc-back-compat commits and replaced them with one bumping the libgcc requirement enforced by symbol-check. |
e58867d to
c76ac9d
Compare
|
Concept ~0.
I agree. While the changes here are simpler than when first proposed, reintroducing |
Concept ~0. Bitcoin Core does not need these changes. Otherwise, code changes (c76ac9d) look correct. |
|
Going to close this then. If you would like to provide Guix built |
…6-linux-gnu` host 97af652 guix: Drop code for the unsupported `i686-linux-gnu` host (Hennadii Stepanov) Pull request description: Now GUIX build for the `i686-linux-gnu` host is broken, and [there are no plans to re-add it](bitcoin/bitcoin#24448). ACKs for top commit: fanquake: ACK 97af652 Tree-SHA512: 968181aff65e607a7c1a1b06ac7dfd79f6e2ce49b3c4c3828def020e925769fdbab1859d37ea924ded7632405b30539ac3ec81ac714cb9a01a2f7d5c93301dd9
…gnu` host 97af652 guix: Drop code for the unsupported `i686-linux-gnu` host (Hennadii Stepanov) Pull request description: Now GUIX build for the `i686-linux-gnu` host is broken, and [there are no plans to re-add it](bitcoin#24448). ACKs for top commit: fanquake: ACK 97af652 Tree-SHA512: 968181aff65e607a7c1a1b06ac7dfd79f6e2ce49b3c4c3828def020e925769fdbab1859d37ea924ded7632405b30539ac3ec81ac714cb9a01a2f7d5c93301dd9
This seems to be what is needed to get i686-linux-gnu Guix builds working.
Since we don't officially provide i686-linux-gnu release binaries, I'm not sure if this is wanted for Core. Considering the need to reintroduce
--enable-glibc-back-compat, I would guess we don't want it. But at the same time, our Guix stuff presently has incomplete/broken i686 support, so maybe others might disagree and think it's worth keeping. (I'm keeping it in Knots for now, so might as well offer it for Core here too.)Note that for
__divmoddi4, I have reintroduced it with a very simple (copyright-immune?) replacement similar to the one used by LLVM. Prior versions (0.17-0.21) using it had copied GPL 3 code from GCC.Previous versions had also used this
__divmoddi4substitution concept for ARM. As far as I can tell, it was never needed for ARM. While ARM uses a similar method, the equivalent libgcc functions it calls appear to be older than on x86-32.Also note we cannot use the "build with an older library" trick for
__divmoddi4because it is part of GCC, not glibc, and we require a fairly new GCC to build at all, at this point.