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
stdenv, haskell: bonafied GHCJS cross compilation without stdenv.cc #74090
stdenv, haskell: bonafied GHCJS cross compilation without stdenv.cc #74090
Conversation
Before, we'd always use `cc = null`, and check for that. The problem is this breaks for cross compilation to platforms that don't support a C compiler. It's a very subtle issue. One might think there is no problem because we have `stdenvNoCC`, and presumably one would only build derivations that use that. The problem is that one still wants to use tools at build-time that are themselves built with a C compiler, and those are gotten via "splicing". The runtime version of those deps will explode, but the build time / `buildPackages` versions of those deps will be fine, and splicing attempts to work this by using `builtins.tryEval` to filter out any broken "higher priority" packages (runtime is the default and highest priority) so that both `foo` and `foo.nativeDrv` works. However, `tryEval` only catches certain evaluation failures (e.g. exceptions), and not arbitrary failures (such as `cc.attr` when `cc` is null). This means `tryEval` fails to let us use our build time deps, and everything comes apart. The right solution is, as usually, to get rid of splicing. Or, baring that, to make it so `foo` never works and one has to explicitly do `foo.*`. But that is a much larger change, and certaily one unsuitable to be backported to stable. Given that, we instead make an exception-throwing `cc` attribute, and create a `hasCC` attribute for those derivations which wish to condtionally use a C compiler: instead of doing `stdenv.cc or null == null` or something similar, one does `stdenv.hasCC`. This allows quering without "tripping" the exception, while also allowing `tryEval` to work. No platform without a C compiler is yet wired up by default. That will be done in a following commit.
This platform doesn't have a C compiler, and so relies and the changes in the previous commit to work.
This is GHCJS, and perhaps other obscure targets.
@@ -144,7 +144,7 @@ let | |||
substituteInPlace "$out"/lib/perl5/*/*/Config_heavy.pl \ | |||
--replace "${libcInc}" /no-such-path \ | |||
--replace "${ | |||
if stdenv.cc.cc or null != null then stdenv.cc.cc else "/no-such-path" | |||
if stdenv.hasCC then stdenv.cc.cc else "/no-such-path" |
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.
Why not just optionalString
the whole --replace
argument?
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 agree eventually, but trying to avoid a mass rebuild for now :)
js-ghcjs didn't fit in an existing categor.
js-ghcjs didn't fit in an existing categor.
…into ghcjs-cross-without-cc
Can you add at least one test for release-cross.nix as well? |
Great idea |
Use `buildPackages.stdenv.mkDerivation` because we are making a shell script to start hoogle on the build platform.
I added it, but it won't work yet (at least on 19.09) because ghcjs itself doesn't build. I think that's fine; I rather upstream whatever reflex-platform does to fix it after this change. |
This is a bit dubvious, but the alternative of making nodejs a nativeBuildInput for node packages is worse. In general the cross story for interpreted languages is murky, and this fits that pattern.
This makes us a bit more robust to various splicing nastiness. May splicing someday go so we don't have to resort to such hacks.
(cherry picked from commit 59dbb00)
Otherwise it passes `--with-ghc=ghc`, and we do the wrong thing.
@@ -156,7 +160,9 @@ let | |||
"--libsubdir=\\$abi/\\$libname" | |||
(optionalString enableSeparateDataOutput "--datadir=$data/share/${ghc.name}") | |||
(optionalString enableSeparateDocOutput "--docdir=${docdir "$doc"}") | |||
] ++ optionals stdenv.hasCC [ |
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 little weird considering haskell/cabal#6466. Isn't --with-gcc always needed for GHC?
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 suppose but those "cross flags" do that. We should really just combine the list of flags and make cross and native more similar.
Motivation for this change
This commit allows treating GHCJS builds as cross compilation. This is useful for a number of correctness reasons, chiefly getting the right build-time
build-tool-depends
so things like literate READMEs work.There is some trickiness to get this to work however. Read the first commit for details. In short, as usual, dependency "splicing" is a terrible idea but one with no better alternative that isn't a huge refactor.
I was very careful to break any interfaces with this e.g. existing native and cross platforms will have the same hashes, and whether
stdenv.cc == null
before, that is still the case. This will allow me to backport this to19.09
for e.g. reflex-platform and haskell.nix.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @ElvishJerricco @ElvishJerricco @ali-abrar @cdepillabout