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.

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.x: fix cross compilation to windows #132199

Merged
merged 2 commits into from
Sep 9, 2021

Conversation

alexfmpe
Copy link
Member

@alexfmpe alexfmpe commented Jul 31, 2021

Tried to cross-compile trivial haskell packages, and the error messages led me to #36200. This PR is pretty much derived from following the advice there.
This fixes the ghc build per-se, but unless enableIntegerSimple is disabled, #132188 is also needed

cc @angerman @nomeata @Ericson2314 @hamishmack

Motivation for this change
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/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@alyssais
Copy link
Member

This needs to target staging. :) Adding the patch causes a rebuild on every platform.

@maralorn
Copy link
Member

This needs to target staging. :) Adding the patch causes a rebuild on every platform.

We have hydra builds for all plattforms on haskell-updates. So you can just target the haskell-updates branch.

@alexfmpe alexfmpe changed the base branch from master to haskell-updates July 31, 2021 10:35
@alexfmpe
Copy link
Member Author

I see there are also derivations for 8.6 / 8.8 / 9.0 / head. Should I try to replicate these patches in those files? Or maybe put these changes in some overlay and import it from multiple ghc versions?

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Jul 31, 2021
@maralorn
Copy link
Member

Yeah, sounds like a good idea. If it’s pain, imo we don‘t need it for 8.6 and 8.8, but we definitely wont to keep the change for the future.

The overlay sounds like a smart solution. However there are plans to refactor the ghc derivation anyways, to have more code reuse between multiple ghc version (and also we need to update it to the new ghc build system, hadrian, at some point in the future.) But I am not sure what exactly that means for your change.

Anyways I’ll leave this one to @sternenseemann because he’s our cross-compiling guy.

@@ -158,6 +160,11 @@ stdenv.mkDerivation (rec {
# Fix documentation configuration which causes a syntax error with sphinx 4.*
# See https://gitlab.haskell.org/ghc/ghc/-/issues/19962, remove at 8.10.6.
./sphinx-4-configuration.patch

(fetchpatch {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add an annotation in a comment above this on what it does / why it is needed and whether it has been upstreamed.

Is this change included in Cabal 3.4? If yes, I guess GHC 8.8.4 will also need this patch, if no practically all GHCs we have in nixpkgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, didn't mean to add this patch unconditionally, as non-cross has been working fine.

Is this change included in Cabal 3.4

Issue's still open, doesn't look like it's been fixed. Patch might be upstreamable though?

Pushed new version including it for cross only, and with a link to upstream issue.

@@ -105,6 +105,8 @@ let
GhcRtsHcOpts += -fPIC
'' + lib.optionalString targetPlatform.useAndroidPrebuilt ''
EXTRA_CC_OPTS += -std=gnu99
'' + lib.optionalString targetPlatform.isWindows ''
SplitSections = NO
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment why this was done? It seems to me that -split-sections should work for windows, but is extremely slow when using ld.bfd.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was done because there was a "too many sections" error otherwise. Took the change from https://github.com/input-output-hk/haskell.nix/blob/531c9f4cac0e335db245849df3b5036973826d36/compiler/ghc/default.nix#L126-L128

Though after looking it into it a bit more, it seems this could be fixed with "-Wa,-mbig-obj", but I can't figure out where to pass those flags.

Copy link
Member

Choose a reason for hiding this comment

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

-Wa,? Or -Wl,?

Either way, EXTRA_CC_OPTS should work.

Copy link
Member Author

Choose a reason for hiding this comment

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

-    SplitSections = NO
+    EXTRA_CC_OPTS += -Wa,-mbig-obj

yields same error

Copy link
Member Author

Choose a reason for hiding this comment

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

Given the actual error message

"inplace/bin/ghc-stage1" -hisuf p_hi -osuf  p_o -hcsuf p_hc -static -prof -eventlog  -O -H64m -Wall     -this-unit-id array-0.5.4.0 -hide-all-packages -i -ilibraries/array/. -ilibraries/array/dist-install/build -Ilibraries/array/dist-install/build -ilibraries/array/dist-install/build/./autogen -Ilibraries/array/dist-install/build/./autogen -Ilibraries/array/.    -optP-include -optPlibraries/array/dist-install/build/./autogen/cabal_macros.h -package-id base-4.14.1.0 -Wall -XHaskell2010 -O2 -fPIC  -no-user-package-db -rtsopts  -Wno-deprecated-flags    -Wnoncanonical-monad-instances  -outputdir libraries/array/dist-install/build -split-sections  -c libraries/array/./Data/Array/MArray/Safe.hs -o libraries/array/dist-install/build/Data/Array/MArray/Safe.p_o 
/nix/store/aq9brwnas2npzk92jy4bdv9mqyc6dqzc-x86_64-w64-mingw32-binutils-2.35.1/bin/x86_64-w64-mingw32-ld: libraries/base/dist-install/build/HSbase-4.14.1.0.p_o: too many sections (55449)
/nix/store/aq9brwnas2npzk92jy4bdv9mqyc6dqzc-x86_64-w64-mingw32-binutils-2.35.1/bin/x86_64-w64-mingw32-ld: final link failed: file too big
make[1]: *** [libraries/base/ghc.mk:4: libraries/base/dist-install/build/HSbase-4.14.1.0.p_o] Error 1
make[1]: *** Waiting for unfinished jobs....

comes from ld, I tried

-    SplitSections = NO
+    EXTRA_LD_OPTS += -Wl,--oformat,pe-bigobj-x86-64

following https://stackoverflow.com/a/64367746/6276652. This fails fairly early on with

/nix/store/hy3lz2vfv9qq2v5jz9nzlx6mmiaq79rj-binutils-2.35.1/bin/ld: target pe-bigobj-x86-64 not found

I tried forcing "--enable-targets=all in binutils/default.nix. This also fails early on with

"/nix/store/amw7sxk9qk4bsc8m1x71ifp3qcmw77j3-ghc-8.6.5-binary/bin/ghc" -o utils/deriveConstants/dist/build/tmp/deriveConstants -hisuf hi -osuf  o -hcsuf hc -static  -O -H64m -Wall   -package-db libraries/bootstrapping.conf  -hide-all-packages -i -iutils/deriveConstants/. -iutils/deriveConstants/dist/build -Iutils/deriveConstants/dist/build -iutils/deriveConstants/dist/build/deriveConstants/autogen -Iutils/deriveConstants/dist/build/deriveConstants/autogen     -optP-include -optPutils/deriveConstants/dist/build/deriveConstants/autogen/cabal_macros.h -package-id base-4.12.0.0 -package-id containers-0.6.0.1 -package-id filepath-1.4.2.1 -package-id process-1.6.5.0 -XHaskell2010  -no-user-package-db -rtsopts        -outputdir utils/deriveConstants/dist/build   -optl-Wl,--oformat,pe-bigobj-x86-64 -static  -O -H64m -Wall   -package-db libraries/bootstrapping.conf  -hide-all-packages -i -iutils/deriveConstants/. -iutils/deriveConstants/dist/build -Iutils/deriveConstants/dist/build -iutils/deriveConstants/dist/build/deriveConstants/autogen -Iutils/deriveConstants/dist/build/deriveConstants/autogen     -optP-include -optPutils/deriveConstants/dist/build/deriveConstants/autogen/cabal_macros.h -package-id base-4.12.0.0 -package-id containers-0.6.0.1 -package-id filepath-1.4.2.1 -package-id process-1.6.5.0 -XHaskell2010  -no-user-package-db -rtsopts        utils/deriveConstants/dist/build/Main.o    
"/nix/store/amw7sxk9qk4bsc8m1x71ifp3qcmw77j3-ghc-8.6.5-binary/bin/ghc" -hisuf hi -osuf  o -hcsuf hc -static  -O -H64m -Wall   -package-db libraries/bootstrapping.conf  -hide-all-packages -i -iutils/hsc2hs/. -iutils/hsc2hs/dist/build -Iutils/hsc2hs/dist/build -iutils/hsc2hs/dist/build/hsc2hs/autogen -Iutils/hsc2hs/dist/build/hsc2hs/autogen     -optP-include -optPutils/hsc2hs/dist/build/hsc2hs/autogen/cabal_macros.h -package-id base-4.12.0.0 -package-id containers-0.6.0.1 -package-id directory-1.3.3.0 -package-id filepath-1.4.2.1 -package-id process-1.6.5.0 -Wall -XHaskell2010  -no-user-package-db -rtsopts        -outputdir utils/hsc2hs/dist/build   -c utils/hsc2hs/./DirectCodegen.hs -o utils/hsc2hs/dist/build/DirectCodegen.o 
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: skipping incompatible /nix/store/khj4v46ckd20q4gal00336ia6qhasbnb-glibc-2.33-47/lib/libm.so when searching for -lm
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: skipping incompatible /nix/store/khj4v46ckd20q4gal00336ia6qhasbnb-glibc-2.33-47/lib/libm.so when searching for -lm
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: cannot find -lm
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: skipping incompatible /nix/store/khj4v46ckd20q4gal00336ia6qhasbnb-glibc-2.33-47/lib/libm.so when searching for -lm
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: skipping incompatible /nix/store/khj4v46ckd20q4gal00336ia6qhasbnb-glibc-2.33-47/lib/libm.so when searching for -lm
/nix/store/rmv3kghdvf3kzxdm85zywd8ibannlyly-binutils-2.35.1/bin/ld: /nix/store/khj4v46ckd20q4gal00336ia6qhasbnb-glibc-2.33-47/lib/librt.so: error adding symbols: file in wrong format
collect2: error: ld returned 1 exit status
`gcc' failed in phase `Linker'. (Exit code: 1)

Not sure how to proceed (or even if I'm going in the right direction).
I'm tempted to leave SplitSections disabled with a comment.

@@ -122,7 +124,7 @@ let
# Use gold either following the default, or to avoid the BFD linker due to some bugs / perf issues.
# But we cannot avoid BFD when using musl libc due to https://sourceware.org/bugzilla/show_bug.cgi?id=23856
# see #84670 and #49071 for more background.
useLdGold = targetPlatform.linker == "gold" || (targetPlatform.linker == "bfd" && !targetPlatform.isMusl);
useLdGold = targetPlatform.linker == "gold" || (targetPlatform.linker == "bfd" && !targetPlatform.isMusl && !targetPlatform.isWindows);
Copy link
Member

Choose a reason for hiding this comment

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

Does ld.gold not support windows? Noting something on this in a comment would be great.

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'm not sure.

Configuring ghc-prim-0.6.1...
ghc-cabal: Cannot find the program 'ld'. User-specified path
'/nix/store/yacysa24iax63f6m3wcm1y3jbw181v81-x86_64-w64-mingw32-binutils-wrapper-2.35.1/bin/x86_64-w64-mingw32-ld.gold'
does not refer to an executable and the program is not on the system path.

It doesn't appear to be currently built for windows

$ ls /nix/store/yacysa24iax63f6m3wcm1y3jbw181v81-x86_64-w64-mingw32-binutils-wrapper-2.35.1/bin/
x86_64-w64-mingw32-as  x86_64-w64-mingw32-ld  x86_64-w64-mingw32-ld.bfd

I'm a bit confused on where bfd is even coming from if all of binutils is disabled for windows?
https://github.com/NixOS/nixpkgs/blob/3e593430e07f36e0edce525c256b20f27934ef3f/pkgs/development/tools/misc/binutils/default.nix#L180

Maybe it's somehow enabled elsewhere but with gold = false ?
https://github.com/NixOS/nixpkgs/blob/3e593430e07f36e0edce525c256b20f27934ef3f/pkgs/development/tools/misc/binutils/default.nix#L7

Copy link
Member

Choose a reason for hiding this comment

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

That is the platform binutils runs on, not the one it targets.

Copy link
Member

Choose a reason for hiding this comment

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

gold is ELF only so this is likely the issue. I think we maybe should make this fact discoverable via a passthru attribute or something in the binutils derivation? I think executable format is part of the platform set already, so shouldn't be too hard.

Copy link
Member

Choose a reason for hiding this comment

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

Tried this in # #132538.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried this in # #132538.

Much better. I'll rebase this once that one's merged

@sternenseemann
Copy link
Member

I see there are also derivations for 8.6 / 8.8 / 9.0 / head. Should I try to replicate these patches in those files? Or maybe put these changes in some overlay and import it from multiple ghc versions?

Yes, that'd be great. Just replicate the changes in the different derivations, reducing code duplication is a concern for another day (see #123599).

@alexfmpe
Copy link
Member Author

alexfmpe commented Aug 2, 2021

Yes, that'd be great. Just replicate the changes in the different derivations, reducing code duplication is a concern for another day (see #123599).

Sure, I'll port changes over to the other ghcs once I'm fairly convinced they're settled - still double-guessing some stuff at the moment

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Aug 2, 2021
@veprbl veprbl added 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: windows Running, or buiding, packages on Windows labels Aug 2, 2021
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 9, 2021
@ofborg ofborg bot requested review from sternenseemann and guibou September 9, 2021 02:49
@ofborg ofborg bot added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Sep 9, 2021
@alexfmpe alexfmpe changed the title ghc: fix cross compilation to windows ghc 8.x: fix cross compilation to windows Sep 9, 2021
@alexfmpe
Copy link
Member Author

alexfmpe commented Sep 9, 2021

The ghc 9.x derivations hit https://gitlab.haskell.org/ghc/ghc/-/issues/18143 early in the build, so I'm not sure whether any of the changes here are needed there, or if they just make things worse, so I'm changing the scope of this PR to be 8.x only

@alexfmpe alexfmpe marked this pull request as ready for review September 9, 2021 15:07
@Ericson2314 Ericson2314 merged commit 29a06ad into NixOS:haskell-updates Sep 9, 2021
@alexfmpe alexfmpe mentioned this pull request Nov 22, 2021
13 tasks
@alexfmpe alexfmpe deleted the cross-mingw-ghc branch June 28, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: haskell 6.topic: windows Running, or buiding, packages on Windows 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants