-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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 prebuilt: Add 7.8.4, 7.10.3 and 8.2.1, and make consistent style #29688
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peti, @shlevy and @edolstra to be potential reviewers. |
52cb654
to
17d2fb8
Compare
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.
I am all for adding the new builds, and I am completely against modifying the existing builds.
{ stdenv | ||
, fetchurl, perl | ||
, libedit, ncurses5, gmp | ||
}: |
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.
I don't believe that a consistent style is important for those files and I would prefer to avoid any changes whatsoever in the old compiler builds unless there is a technical reason to make the change. Reformatting whitespace in the argument lists feels pointless to me.
echo compilation ok | ||
[ $(./main) == "yes" ] | ||
''; | ||
dostallCheck = true; |
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.
Typo.
fe6dd1d
to
086be94
Compare
OK, I now realize why consistent-looking build files are useful in the light of the upcoming cross-compilation changes. I completely revert my previous position and am now in favor of it. :-) |
1e9d6db
to
ae243cc
Compare
ae243cc
to
429c7e2
Compare
I merged the reformats to master and rebase this (to staging) on top, making it much easier to read. |
|
429c7e2
to
bb903e5
Compare
These changes do break hashes, but are good to have everywhere for consistency. Plus, changing the bootstrap causes a Haskell mass rebuild anyways.
The newer to-be-added binaries have relative RPATHs that we'll break if we try to `patchelf` before installation. We instead us LD_LOAD_LIBRARY during the installation, which avoids needing to change the RPATH temporarily.
- Patch all executables and libraries, while skipping scripts. Base at least uses libiconv, so should need this too---I suspect it wasn't a problem before as we got away with the immpurity. - Rather than hardcoding the symlinks to add, do one per mach-o as needed TEMP no -L no script
bb903e5
to
960e00d
Compare
This is great, and thank you for including the ARM variants! This is better than the hack I devised to do the same for my ARMv7 and aarch64 systems. I'm looking forward to Hydra building aarch64 Haskell packages so I don't have to build them all myself each time I update! |
@Ericson2314 happy to test this once it's ready. Good work! |
@Ericson2314 Why not take this PR a step further, and use the binary snapshots you've added to bootstrap the non-binary GHC derivations? (See #19926) You've already done the hard part! |
@dhess Well, I haven't completely the hard part because these don't all build! Not sure when I will have time to finish this, so others feel free to push to this. |
@nixborg build |
@domenkozar is not a committer |
@Ericson2314 can you explain a bit more what's going on linux? If I understand correctly it's trying to parse /tmp/nix-build-ghc-7.10.3-binary.drv-7/ghc-7.10.3/libraries/ghc-prim/dist-install/setup-config and fails to do so. |
@Ericson2314 following fixes your current error, but resulting $out/bin/ghc still needs patchelfed libs:
|
I have all of these building on darwin/linux in #33054 |
Are there plans to add |
@dhess if you want to do the changes, why not :) |
@domenkozar Is there a reason that Unfortunately, because |
No good reason, we can bump it. |
OK, great. I should have time to work on ARM GHC stuff over the next few days, so I'll try it out locally and submit a PR if someone doesn't beat me to it. |
I just fixed a couple of issues that were preventing GHC from bootstrapping on both ARM platforms. I'm now kicking off |
Sounds good! But make sure you work from Domen's recent work. |
I propose we track GHC on ARM issues in #33353. |
Motivation for this change
I wanted to keep these maintainable, so I slightly tweaked the style to be consistent everywhere. The new ones have relative RPATHs in
ghc-cabal
and other build-time utilities.@peti I do have some of the formatting fixes that you were unsure about before, as part of that normalizing style and methodology, so definitely waiting on your approval.
Now that they are pretty close, we could make a common helper, too.
Closes #19926
Things done
I've currently tested
x86_64-linux
andx86_64-darwin
. Darwin succeeds everywhere. The new Linux ones however don't succeed with:build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)CC @cleverca22