Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support #83048

Merged
merged 2 commits into from
Mar 21, 2020
Merged

ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support #83048

merged 2 commits into from
Mar 21, 2020

Conversation

thefloweringash
Copy link
Member

Motivation for this change

GHC on Aarch64.

I'm not in the loop with how the haskell infrastructure works in nixpkgs, I just require ghc to build my system for things like pandoc and nix-diff. I assume aarch64 was omitted from ghc863Binary due to the upstream 8.6.3 release not to having an appropriate tarball. The upstream 8.6.5 release includes ghc-8.6.5-aarch64-ubuntu18.04-linux.tar.xz.

With these changes, I can build ghc 8.8.3 on aarch64. I split out the commit for the version change and the aarch64 support to make it easier to read.

Related: #80355 -- contains the same llvm logic as here for propagatedBuildInputs
Related: #80176 -- issue for this and #80355

cc @MarcWeber @kosmikus @peti (maintainers of ghc)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@thefloweringash
Copy link
Member Author

I meant to open this as a draft.

@thefloweringash thefloweringash changed the title ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support [DRAFT] ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support Mar 21, 2020
@cdepillabout
Copy link
Member

@thefloweringash This seems reasonable. Is there any reason you've marked this as a WIP? Is there anything that is not currently working?

@GrahamcOfBorg build ghc

@thefloweringash
Copy link
Member Author

I'm not aware of anything not working. I built ghc, I'll see if I can build pandoc. I'll remove the WIP and DRAFT markers.

There was some contention about the llvm input, see #80355 (review) and further comments.

Now that I've seen the build count, this probably shouldn't go to master. Should this go to staging, or haskell-updates?

@thefloweringash thefloweringash changed the title [DRAFT] ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support ghc: update bootstrap 8.6.3 -> 8.6.5, add aarch64 support Mar 21, 2020
@cdepillabout
Copy link
Member

It looks like ghc was able to be built with the new bootstrap GHC, so I'd say this looks good.

Let's see if lens can be built as well:

@GrahamcOfBorg build haskellPackages.lens

@thefloweringash Okay, that sounds good.

I think you're right, this should probably go to the haskell-updates branch? Can you do that rebase?

After that, how about giving this 1 week for other people to review it, and if nobody has anything bad to say about it, I'll merge it in. Please ping me in 1 week if there has been no activity.

Also, cc @matthewbauer and @Ericson2314, who occasionally contribute to the GHC derivation.

cdepillabout
cdepillabout reviewed Mar 21, 2020
View reviewed changes
pkgs/development/compilers/ghc/8.6.5-binary.nix Outdated
};
}.${stdenv.hostPlatform.system}
or (throw "cannot bootstrap GHC on this platform"));

nativeBuildInputs = [ perl ];
propagatedBuildInputs = lib.optional stdenv.isAarch64 [ llvm ];
Copy link
Member

@cdepillabout cdepillabout Mar 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the big question from #80355 (review) is, "Why is this needed?"

It seems strange that aarch64 would need this, but nothing else would. Additionally, why is this not needed for the non-bootstrap GHCs?

I don't really have any knowledge of how GHC is built, or how aarch64 works, but this just seems like the wrong solution.

If this is actually needed, then ideally there would be a comment on this line explaining it (and possibly linking to upstream GHC documentation justifying it).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I have found the answers.

GHC on non-x86 uses the LLVM codegen by default [citation needed]. When using the LLVM codegen, ghc needs llvm binaries on the path.

You must install and have LLVM available on your PATH for the LLVM code generator to work. Specifically GHC needs to be able to call the opt and llc tools.

-- (from 10.10.2. LLVM Code Generator (-fllvm))

It seems strange that aarch64 would need this, but nothing else would. Additionally, why is this not needed for the non-bootstrap GHCs?

It turns out this is required for the non-bootstrap GHCs, and is already present in propagatedBuildInputs of, for example, ghc 8.8.3:

nixpkgs/pkgs/development/compilers/ghc/8.8.3.nix

Lines 193 to 194 in bfb747a

propagatedBuildInputs = [ targetPackages.stdenv.cc ]
++ stdenv.lib.optional useLLVM llvmPackages.llvm;

I've updated the branch to more closely resemble the non bootstrap GHC structure.

@thefloweringash thefloweringash changed the base branch from master to haskell-updates March 21, 2020 11:45
@thefloweringash thefloweringash force-pushed the ghc-aarch64 branch 2 times, most recently from 1049812 to 902e436 Compare March 21, 2020 12:08
@domenkozar
Copy link
Member

@GrahamcOfBorg build ghc

@domenkozar domenkozar merged commit 8c06685 into NixOS:haskell-updates Mar 21, 2020
@thefloweringash thefloweringash deleted the ghc-aarch64 branch March 22, 2020 02:43
@thefloweringash
Copy link
Member Author

The effects of this change can be tracked in hydra evaluation 1576936.

@cdepillabout cdepillabout mentioned this pull request Mar 23, 2020
Merged
10 tasks
@vcunat
Copy link
Member

vcunat commented Mar 23, 2020

Well, down-side is that this jobset only uses x86_64-linux :-)

@vcunat vcunat mentioned this pull request Mar 24, 2020
Closed
@cdepillabout cdepillabout mentioned this pull request Apr 1, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout approved these changes

@Ericson2314 Ericson2314 Ericson2314 approved these changes

Assignees
No one assigned
Labels
6.topic: haskell 8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thefloweringash @cdepillabout @domenkozar @vcunat @Ericson2314