kexec-tools: Only force ELFv2 on ppc64 when the platform settings aggree#422007
kexec-tools: Only force ELFv2 on ppc64 when the platform settings aggree#422007flokli merged 1 commit intoNixOS:stagingfrom
Conversation
9797f07 to
dd19138
Compare
Fixes building for ELFv1.
dd19138 to
196eb53
Compare
| hash = "sha256-Nq5HIcLY6KSvvrs2sbfE/vovMbleJYElHW9AVRU5rSA="; | ||
| }) | ||
| ] | ||
| ++ lib.optionals (stdenv.hostPlatform.isPower64 && stdenv.hostPlatform.isAbiElfv2) [ |
There was a problem hiding this comment.
Do we need to make the patch optional? The next person updating the package might easily miss this to the disadvantage of powerpc users.
There was a problem hiding this comment.
With this patch file? Yes, because it breaks building for ELFv1 (which, eventually, I'm planning to make our default ABI on powerpc64, because it's less effort to get it to work properly). Doing this in a way that supports both ABI would require a custom patch, because upstream currently doesn't support different flags for different ABI targets.
There was a problem hiding this comment.
We usually don't do conditional patches, because they end up breaking easily without being noticed during regular package bumps.
We're less strict for more "exotic" targets/platforms, see this example:
I think it's fine to pull it in conditionally, cause power64 is also somewhat niche, but I strongly recommend to start upstreaming this in a more generic fashion to avoid this breaking in the future.
There was a problem hiding this comment.
Yes, because it breaks building for ELFv1 (which, eventually, I'm planning to make our default ABI on powerpc64, because it's less effort to get it to work properly).
Isn't elfv1 marked as legacy? (see rust-lang/rust#60617)
There was a problem hiding this comment.
ELFv1 is considered legacy, but big-endian 64-bit POWER itself is kind of legacy too. Modern 64-bit POWER processors are usually run in little-endian mode, where ELFv2 is (IIRC) the only supported ABI. But 64-bit POWER started out as big-endian in Linux, ELFv1 was the current version for most of its lifetime, and glibc still defaults to ELFv1 unless explicitly told otherwise, so it's still the "default".
The ABI situation of powerpc64 in the libc's is as follows:
- glibc supports either ABI version, but defaults to ELFv1 unless specifically told otherwise
- musl only supports ELFv2
Certain (bad) assumptions have developed like ~ "on 64-bit POWER, the ABI is ELFv2 on LE and ELFv1 on BE". Patches like the one that gets shuffled around in this PR come mostly from musl-running distros - Void PPC when it was still around, Adelie Linux, Chimera Linux are all musl distros that I used to look towards for ELFv2 patches. All glibc-using distros supported powerpc64 that I could find (Debian, Gentoo) use the ELFv1 ABI that glibc defaults to. Some (Gentoo IIRC) attempted to switch their glibc to ELFv2 in the past, but it looks like they're still on ELFv1 as of today.
I gave up on ELFv2 when I got to Valgrind. I was unable to get it to accept & properly handle ELFv2 binaries produced by Nix, unless I built them against musl and used patchelf to replace our musl in the final binary with i.e. Adelie's musl. So either something is missing in the ELFv2 patch, or our toolchain just uniquely breaks Valgrind. The ELFv2 MR has been dormant for awhile upstream, so I'm not too optimistic about this support becoming official soon. In contrast, ELFv1 Valgrind has worked just fine.
There are more issues with ELFv2 on powerpc64. Most prebuilt binaries target ELFv1, so i.e. bootstrapping Rust, GHC etc gets more annoying, and would require cross-compilation between ABIs or something weird like that. I know Adelie has their own prebuilt, prepatched Rust toolchain artefacts on their GitLab instance that they use for Rust things. I imagine that those might not just magically work for us though. With GHC, I'm even less sure about the state of ELFv2 support. All of these issues naturally go away when we don't force ELFv2.
So in short, yes it's legacy, but it Just Works ™️, and is the path of least resistance on this platform.
|
Forgot to hit comment, successfully built |
Fixes building for ELFv1.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.