ghc, haskell-infra: Improve cross-compilation#40642

Merged

Ericson2314 merged 12 commits intoNixOS:stagingfrom

angerman:feature/clean-ghc

May 21, 2018

Contributor

angerman commented May 17, 2018

Motivation for this change

This adds some minor changes to the GHC expressions, and tries to align the head and 8.4.2 expression. Ideally we'd have one common one among all ghc expressions I think.

This should allow building quick flavors, and building cross compilers to windows.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

angerman requested a review from peti as a code owner

May 17, 2018 07:26

GrahamcOfBorg added 6.topic: haskell 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ labels

peti previously requested changes

View reviewed changes

pkgs/development/compilers/ghc/head.nix Outdated

Member

peti May 17, 2018

Is it really necessary to parameterize all aspects of the src attribute as function arguments? I would assume that few people actually mess with these settings, and those who want to can easily override the src attribute as a whole.

Contributor Author

angerman May 17, 2018

Right! that should better be handed like this once we have the generic expression. Will drop.

pkgs/development/haskell-modules/generic-builder.nix Outdated

Member

peti May 17, 2018

In the presence of buildFlags this attribute seems redundant?

Contributor Author

angerman May 17, 2018

I agree that we can drop this in favor or passing -v3 to the buildFlags.

pkgs/development/haskell-modules/generic-builder.nix Outdated

Member

peti May 17, 2018

If someone requests static builds for a given platform, then it feels like poor style to just ignore the setting and to silently build shared libraries instead. I'd prefer to see this particular limitation expressed as an assertion instead.

Contributor Author

angerman May 17, 2018

I'll amend the PR, to

enableStaticLibraries ? !hostPlatform.isWindows

and a

assert (enableStaticLibraries && hostPlatform.isWindows) "can't use `--enable-static` on windows."

The reason why this does not work is not that we can't built static objects, we can, but we can't use
-staticlib on ghc on windows. -staticlib rolls all dependencies into a combined archive. While this
would work on windows if we used gnu ar and MRI script, GHC can't rely on gnu ar, and as such has
a quck archive concatenation module for gnu and bsd archives only.

Member

Ericson2314 May 18, 2018

On Nix we do indeed provide GNU ar always, so it would be nice to optionally configure GHC to expect GNU binutils just as one can configure GCC for the same reasons.

Contributor Author

angerman May 19, 2018

I'd rather prefer some runtime flag over a build time flag, or even some runtime support int he form of trying to use an MRI script and falling back to the backup solution.

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

peti May 17, 2018

Can you please add a small comment here explaining that "" means "use ghc's default choice"?

Contributor Author

angerman May 17, 2018

will do.

peti changed the title ~~Feature/clean ghc~~ Improved cross-compilation abilities for ghc and generic haskell builder

peti added the 0.kind: enhancement label

Contributor Author

angerman commented May 17, 2018

@peti, thanks for the quick review, I'll make some amendments later today.

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

Ericson2314 May 17, 2018

, ghcFlavour = stdenv.lib.optionalString (targetPlatform != hostPlatform) "perf-cross"

Member

Ericson2314 May 17, 2018

Can we also backport this change to the earlier GHCs for consistency?

Contributor Author

angerman May 18, 2018

We can. But I'd argue against it. I'd rather see us align the 8.{4,6}.y and head into a common form, and then graft previous version onto the common.nix. Otherwise we are going to end up backporting this stuff every time again across all expressions.

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

Ericson2314 May 17, 2018

Simple ${ghcFlavour} per the above

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

Ericson2314 May 17, 2018

Can we just do $out/lib/${name}/bin/* then?

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

Ericson2314 May 17, 2018

Nix doesn't require " to be escaped here, to be clear. Does make?

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/8.4.2.nix Outdated

Member

Ericson2314 May 17, 2018

You wish to allow dynamic linking here but not 8.2?

Contributor Author

angerman May 18, 2018

I don't follow? That line wasn't even changed?

Contributor Author

angerman May 18, 2018

Ahh... ok. fixed.

Ericson2314 reviewed

View reviewed changes

pkgs/development/haskell-modules/generic-builder.nix Outdated

Member

Ericson2314 May 17, 2018

two equal signs

Ericson2314 reviewed

View reviewed changes

pkgs/development/compilers/ghc/head.nix Outdated

Member

Ericson2314 May 17, 2018

blank line wasn't added in 8.4.1. In general it's nice to diff the different GHCs so they don't unnecessarily drift apart. We really should deduplicate soon...

Ericson2314 reviewed

View reviewed changes

pkgs/development/haskell-modules/generic-builder.nix Outdated

Member

Ericson2314 May 17, 2018

Can you abstract this part out like https://github.com/NixOS/nixpkgs/pull/39735/files#diff-d65852670e35e11b4bd12ed1c634abce ? Fine to wait on separate derivations until #39735 to avoid controversy though :)

Contributor Author

angerman May 18, 2018

I thought about it, but I kept the Setup.hs changes out of this diff on purpose.

Member

Ericson2314 commented May 18, 2018

I'm going to rebase onto #40442 after I merge it to staging.

Ericson2314 force-pushed the feature/clean-ghc branch from 008d6f0 to 934d825 Compare

May 21, 2018 03:29

Ericson2314 and others added 11 commits

May 21, 2018 15:09


          haskell generic builder: setupHaskellDepends should be `nativeBuild…

900862e

…Inputs

This is because they are just for Setup.hs, so they are just used at build time
and completely isolated from the normal components' dependencies.

This was previous implemented in 8a8f040, but
reverted in e69c7f5 because it broken
setup-depends non-cross in haskell shell environments (custom Setup.hs in cross
shell environments has never worked). This version adds a special native
exception to avoid that breakage.


          haskell generic builder: Use setup package database for setup-depends

15759df

Adapted from obsidiansystems@b69f420 by @Ericson2314


          haskell generic builder: Disable static libs on Windows because no -s…

4ea33de

…taticlib

The reason why this does not work is not that we can't built static
objects, we can, but we can't use `-staticlib` on GHC on windows.
`-staticlib` rolls all dependencies into a combined archive. While this
would work on windows if we used gnu ar and MRI script, GHC can't rely
on GNU ar, and as such has a quick archive concatenation module for GNU
and BSD archives only.


          haskell generic builder: Do the links dance only if shared is enabled.

5e589a4


          haskell infra: Adds buildFlags logic

82a847a


          used stdenv.targetPlatform.isDarwin and not stdenv.isDarwin.

f914fdd


          ghc: Normalize derivations

6f2f081


          ghc: Handle flavors better

8d2ce11


          ghc: paxmark all unwraped executables across the board

6af0c19

Shell glob works even as the exact set of executable (filenames) varries
beween configuations.


          ghc 8.4.2, head: Adjust enableShared enableTerminfo for windows

03602f8


          ghc 8.4.2, head: Drop libiconv on windows.

4b48094

nixpkgs#37012 and  nixpkgs#37707 introduces the setup-hooks for libiconv, which inject `-liconv` into the `NIX_LDFLAGS`. This breaks horribly on windows where the linker end up having no idea how to linke `-liconv`. The configure.ac file specifically ignores libiconv on windows.

Ericson2314 force-pushed the feature/clean-ghc branch from 934d825 to 4b48094 Compare

May 21, 2018 19:18

Ericson2314 force-pushed the feature/clean-ghc branch from 4b48094 to 934d825 Compare

May 21, 2018 19:41

Ericson2314 changed the title ~~Improved cross-compilation abilities for ghc and generic haskell builder~~ ghc, haskell-infra: Improved cross-compilation abilities

Ericson2314 changed the title ~~ghc, haskell-infra: Improved cross-compilation abilities~~ ghc, haskell-infra: Improve cross-compilation

Ericson2314 force-pushed the feature/clean-ghc branch from 934d825 to 4b48094 Compare

May 21, 2018 20:07

Ericson2314 dismissed peti’s stale review

May 21, 2018 20:09

All requested changes were made

Ericson2314 changed the base branch from master to staging

May 21, 2018 21:40


          Merge branch 'staging' into feature/clean-ghc

d6ecbbe

GrahamcOfBorg added 2.status: merge conflict and removed 2.status: merge conflict labels

Ericson2314 merged commit ae62436 into NixOS:staging

Member

Ericson2314 commented May 21, 2018

@peti I wanted to land this somewhere pronto so as to enable more work, but if you could also merge this into master via your normal haskell-updates workflow, that would be much appreciated.

Member

peti commented May 22, 2018

It's not that easy, unfortunately, because the patch does not apply to current versions of master.

Member

Ericson2314 commented May 22, 2018 •

edited

Loading

I'll prepare another PR then. Thanks for checking.

Ericson2314 mentioned this pull request

ghc, haskell infra: #40642 direct to master #40929

Merged

8 tasks

peti added a commit that referenced this pull request


          Merge pull request #40929 from obsidiansystems/cross-ghc-for-master

0e35858

ghc, haskell infra: #40642 direct to master

peti added a commit that referenced this pull request


          Revert "ghc, haskell infra: #40642 direct to master"

5db3a9d

teto pushed a commit to teto/nixpkgs that referenced this pull request


          Merge pull request NixOS#40973 from NixOS/revert-40929-cross-ghc-for-…

83baf1e

…master

Revert "ghc, haskell infra: NixOS#40642 direct to master"

Ericson2314 mentioned this pull request

haskell: build Setup.hs with a separate pkgdb #39735

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement 6.topic: haskell 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+