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
ghc:8.10.2Binary bootstrap for 8.8 on aarch64 (NixOS#97407) #101214
Conversation
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.
This is a collection of workarounds for aarch64 to bootstrap ghc8.8.4(default) and ghc8.10.2 using a ghc8.10.2Binary. With these changes these versions of ghc should work as they do on other architectures to the extent that they work on aarch64 (these versions of ghc are considered experimental on Aarch64, and in particular can have race conditions. etc, with parallel builds, discussion of these problems is in issue #97407.)
I didn't read through all of #97407, but could you be more explicit in what the purpose of this PR is?
For instance, without this PR, is it not possible to build ghc8.8.4 and ghc8.10.2 on aarch64? (If so, then we should probably prioritize getting this PR in.)
these versions of ghc are considered experimental on Aarch64, and in particular can have race conditions. etc, with parallel builds
Does "these versions of ghc" refer to ghc-8.8.4 and ghc-8.10.2? Or are you specifically referring to ghc8_10_2Binary?
If they are experimental (and can have race conditions with parallel builds), then maybe we will have more problems than it is worth having Hydra build and distribute them?
'' + stdenv.lib.optionalString stdenv.hostPlatform.isAarch64 '' | ||
find . -name package.conf.in \ | ||
-exec sed -i "s@FFI_LIB_DIR@FFI_LIB_DIR ${numactl.out}/lib@g" {} \; |
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.
Could you add a short comment as to why this is needed?
# Keep rpath as small as possible on aarch64 for patchelf#244 | ||
'' | ||
(cd $out/lib; ln -s ${ncurses6.out}/lib/libtinfo.so.6) | ||
(cd $out/lib; ln -s ${gmp.out}/lib/libgmp.so.10) | ||
(cd $out/lib; ln -s ${numactl.out}/lib/libnuma.so.1) | ||
for p in $(find "$out/lib" -type f -name "*\.so*"); do | ||
(cd $out/lib; ln -s $p) | ||
done | ||
|
||
for p in $(find "$out/lib" -type f -executable); do | ||
if isELF "$p"; then | ||
echo "Patchelfing $p" | ||
patchelf --set-rpath "\$ORIGIN:\$ORIGIN/../.." $p | ||
fi | ||
done | ||
'' |
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.
Could you add a little more explanation about what is going on here, and why this is needed?
I imagine you want to modify the rpaths as little as possible, so you link the various libraries into a single directory and set the rpaths to that directory.
It would be nice to have a little bit more of an explanation so that someone doesn't come along and remove this section (not understanding that it was necessary).
description = "The Glasgow Haskell Compiler"; | ||
license = stdenv.lib.licenses.bsd3; | ||
platforms = ["x86_64-linux" "armv7l-linux" "aarch64-linux" "i686-linux" "x86_64-darwin"]; | ||
maintainers = with stdenv.lib.maintainers; [ lostnet ]; |
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.
👍
bootPkgs = if stdenv.isAarch64 then | ||
packages.ghc8102Binary | ||
else | ||
packages.ghc865Binary; |
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.
You might want to add a comment here as well explaining why this is necessary (so that someone doesn't accidentally try to change it in the future).
@cdepillabout: yes, ghc on aarch64 is completely broken now, at least on Hydra.nixos.org (and thus no binaries from cache.nixos.org). That holds for 20.09 and master (20.03 seems OK). Example: https://hydra.nixos.org/eval/1620539?filter=ghc |
This refers to ghc-aarch of any current form. I think a bug fixed in 8.10.1 is the cause of the parallelbuilds segv being demonstrated with build time SEGVs in 8.6.5Binary (and non-Binary for me) which is why I trust that binary to back bootstrap over trying to bring a 8.8Binary in.
I think it is worth having some official ghc release on aarch64, there are plenty of compilers that have bugs but are better than nothing. For 8.8, I hope it has less problems than 8.6.5, but I don't think it will be easy to quantify without some usage. The objection to moving the default to 8.10.2 is stackage LTS. I will try to address the comments with code comments. I think the libnuma fix is an example of bit rot caused by no bootstrap, when libnuma was added to fix the 8.10.2Binary, resulting binaries were not tested due to the segv and rts now needs to link libnuma on aarch64 binaries. |
Thanks for clarifying things! It sounds like this is a pretty important change, so let's merge it in after @lostnet adds a little more documentation. |
Thanks @cdepillabout! I've added the comments that hopefully clarify/warn about the several different aarch64 quirks and verified it still doesn't change the x86_64 hashes and has the same aarch64 hashes from the initial push. |
LGTM @lostnet Thanks for taking the time to figure this out and documenting everything. I'll merge this in, but please try to keep an eye on Hydra the next couple days and see if it ends up being able to build Haskell packages on aarch64 again! |
Not really merged yet, for some reason? I'd suggest merging into the |
@vcunat Thanks for the message. Looks like I had just forgotten to actually merge this in! I changed the base branch to |
This should also go to 20.09-staging |
I think it can go directly in 20.09, but I'd wait for |
@vcunat it looks to me like a cherry pick with the commits that create/modify 8.10.2-binary.nix should be fine on 20.09, so you probably wont need an external PR? I've been watching staging-next on hydra, but it looks like it could be another day or two at its current pace.. |
Motivation for this change
This is a collection of workarounds for aarch64 to bootstrap ghc8.8.4(default) and ghc8.10.2 using a ghc8.10.2Binary. With these changes these versions of ghc should work as they do on other architectures to the extent that they work on aarch64 (these versions of ghc are considered experimental on Aarch64, and in particular can have race conditions. etc, with parallel builds, discussion of these problems is in issue #97407.)
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
(tested some sample packages, this is the bootstrap for all of the standard ghc collection on aarch64. I tested on a small machine(rock64) and @vcunat tested on aarch64.nixos.community which is faster and more likely to surface parallel build problems as they may occur on hydra.)
./result/bin/
)nix path-info -S
before and after)(ghc-8.10.2-binary is very large ~2.9gb on aarch64; large libraries sensative to stripping.)
(avoided any changes to other platforms and workaround specific bugs without changing default versions on aarch64, etc)