Feature/mingw fixes #43559

angerman · 2018-07-15T08:29:43Z

Motivation for this change

Allow cross compiling haskell programs to windows using mingw

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

This commit is not really correct. The `hasActiveLibrary` check is wrong. We can have an active library even if we do not ask for a static lirbary or dynamic one; we can still have just a set of objet files and archives.

angerman · 2018-07-15T08:46:23Z

/cc @matthewbauer, @domenkozar, @alpmestan, @bgamari

domenkozar · 2018-07-15T09:01:58Z

Is removing lib subfolder required for this work? I saw you missed a couple of instances.

angerman · 2018-07-15T13:15:14Z

@domenkozar to compile cardano-sl, yes. Otherwise I run into:

x86_64-pc-mingw32-gcc: error trying to exec '/nix/store/g79d8qxa4j8y50sjcinb5j6c54zhq4wg-x86_64-pc-mingw32-stage-final-gcc-debug-7.3.0/libexec/gcc/x86_64-pc-mingw32/7.3.0/collect2': execv: Argument list too long

In general I think putting the haskell libs in lib is bad, because the underlying logic in nix, expects $out/lib folder to contain libraries (such that dependencies are automatically discovered). For haskell, the libs aren't even directly in the lib folder.

If you could outline which libs I missed, I'd be happy to fix them up :-)

However I know there was some alternative to this fix somewhere(?) using response files instead? I'm slightly tending against such a solution, as it would still unnecessarily polute the library serach paths in nix.

Maybe we need a way to flag some lib folders not to be automatically placed into the library search paths. In that case I don't see why we can't just not put them in lib in the first place.

domenkozar · 2018-07-15T13:23:50Z

I think the proper solution is to not duplicate so many flags: #41340 (comment)

angerman · 2018-07-15T13:29:42Z

@domenkozar sure. But why put them in there in the first place? Any haskell tooling will go through the package-db, and pull the proper LD flags anyway. Why have nix provide LD flags what are useless?

E.g. why inject <pkg>/lib, when you actually want <pkg>/lib/$ghc anyway?

Don't get me wrong. Automatically injecting $out/lib is often the right thing to do, when libraries are put into $out/lib. But Haskell does for better or worse not really follow that approach, and expects the package-db to be the canonical source for path information.

angerman · 2018-07-15T13:30:25Z

#41340 should of course be fixed as well.

domenkozar · 2018-07-15T13:40:48Z

@angerman I agree it may be changed, but note that it should rather be done in #32629 and fix the GCC flag pollution properly

@angerman I couldn't compile cachix on darwin with your lib fixes, sorry but I forgot to record the error (even after fixing reminaing lib references in generic-builder)

angerman · 2018-07-15T13:55:52Z

@domenkozar I can't comment on cachix, as I haven't had a chance to compile that on macOS yet.

I believe this stuff

nixpkgs/pkgs/build-support/bintools-wrapper/ld-wrapper.sh

Lines 207 to 210 in 2dc8831

    
           exec @prog@ \ 
        
               ${extraBefore+"${extraBefore[@]}"} \ 
        
               ${params+"${params[@]}"} \ 
        
               ${extraAfter+"${extraAfter[@]}"}

should be really passed via response files.

And while we do

nixpkgs/pkgs/build-support/bintools-wrapper/setup-hook.sh

Lines 26 to 28 in 2dc8831

    
           if [[ -d "$1/lib" ]]; then 
        
               export NIX_${role}LDFLAGS+=" -L$1/lib" 
        
           fi

I think we should just not have $out/lib unless it contains proper libraries.

domenkozar · 2018-07-16T08:49:38Z

@angerman fair points. I'm testing on darwin, meanwhile you also need:

diff --git a/pkgs/development/haskell-modules/lib.nix b/pkgs/development/haskell-modules/lib.nix
index c9dc7254800..d5e36981ba3 100644
--- a/pkgs/development/haskell-modules/lib.nix
+++ b/pkgs/development/haskell-modules/lib.nix
@@ -234,7 +234,7 @@ rec {
     enableSharedExecutables = false;
     isLibrary = false;
     doHaddock = false;
-    postFixup = "rm -rf $out/lib $out/nix-support $out/share/doc";
+    postFixup = "rm -rf $out/ghc* $out/nix-support $out/share/doc";
   });
 
   /* Build a source distribution tarball instead of using the source files

angerman · 2018-07-16T12:12:44Z

@domenkozar right. I've also changed $out/lib/links to $out/lib-links. That would otherwise inject yet another $out/lib.

domenkozar · 2018-07-16T15:15:20Z

@peti I think this can go to testing.

matthewbauer · 2018-07-16T16:54:35Z

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

@@ -138,12 +138,12 @@ let
  buildFlagsString = optionalString (buildFlags != []) (" " + concatStringsSep " " buildFlags);

  defaultConfigureFlags = [
-    "--verbose" "--prefix=$out" "--libdir=\\$prefix/lib/\\$compiler" "--libsubdir=\\$pkgid"
+    "--verbose" "--prefix=$out" "--libdir=\\$prefix/\\$compiler" "--libsubdir=\\$pkgid"


This seems weird. Is this a windows things?

This and the following is @angerman's change to avoid making cc-wrapper and ld-wrapper inject flags. But please don't do this as part of this PR; even if we end up going this route it should be done separately.

matthewbauer · 2018-07-16T16:55:20Z

pkgs/development/compilers/ghc/8.4.3.nix

@@ -46,8 +48,7 @@ let
    include mk/flavours/\$(BuildFlavour).mk
    endif
    DYNAMIC_GHC_PROGRAMS = ${if enableShared then "YES" else "NO"}
-  '' + stdenv.lib.optionalString enableIntegerSimple ''
-    INTEGER_LIBRARY = integer-simple
+    INTEGER_LIBRARY = ${if enableIntegerSimple then "integer-simple" else "integer-gmp"}


This is definitely a good change! This really confused me with cross-built always pulling in GMP but not ever using it (on android at least).

(might be worth backporting too)

Yes agreed heartily with both. Great change and please back-port!

peti

This PR seems to remove the lib directory hierarchy from the generated output, but I don't understand why it does.

peti · 2018-07-16T18:53:50Z

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

    (optionalString enableSeparateDataOutput "--datadir=$data/share/${ghc.name}")
    (optionalString enableSeparateDocOutput "--docdir=${docdir "$doc"}")
    "--with-gcc=$CC" # Clang won't work without that extra information.
    "--package-db=$packageConfDir"
-    (optionalString (enableSharedExecutables && stdenv.isLinux) "--ghc-option=-optl=-Wl,-rpath=$out/lib/${ghc.name}/${pname}-${version}")
+    (optionalString (enableSharedExecutables && stdenv.isLinux) "--ghc-option=-optl=-Wl,-rpath=$out/${ghc.name}/${pname}-${version}")


What is the purpose of this change?

peti · 2018-07-16T18:54:17Z

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

-    if [ -d "$p/lib/${ghcName}/package.conf.d" ]; then
-      cp -f "$p/lib/${ghcName}/package.conf.d/"*.conf ${packageConfDir}/
+    if [ -d "$p/${ghcName}/package.conf.d" ]; then
+      cp -f "$p/${ghcName}/package.conf.d/"*.conf ${packageConfDir}/


Dito. Why are you changing this?

peti · 2018-07-16T18:54:43Z

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

  + (optionalString (stdenv.isDarwin && (enableSharedLibraries || enableSharedExecutables)) ''
    # Work around a limit in the macOS Sierra linker on the number of paths
    # referenced by any one dynamic library:
    #
    # Create a local directory with symlinks of the *.dylib (macOS shared
    # libraries) from all the dependencies.
-    local dynamicLinksDir="$out/lib/links"
+    local dynamicLinksDir="$out/lib-links"


angerman · 2018-07-16T19:57:54Z

@matthewbauer, @peti: yes it dropps the $out/lib. Having $out/lib means that for any haskell dependency it’s $out/lib folder will be added to NIX_LDFLAGS. This nix feature makes sense when there are libXXX.ext or similar libs in that folder and as such linking with -lXXX does automatically the right thing.

For Haskell libraries however that makes little sense. First of all there aren’t even any libraries on that directory to beging with. They are in a ghc... subdirecotry. Also when linking Haskell libraries you usually use tools that inspect and extract the proper library paths from the package db. Those tools will inject the correct -L and -l flags. Thus when linking Haskell libs you end up with an almost duplicate set of library search paths, half of which are useless.

Please also refer to the previous discussion here between @domenkozar and me.

domenkozar · 2018-07-16T21:18:18Z

TLDR; $out/lib removal fixes #41340 (for haskell) by not polluting LDFLAGS which is unneeded for haskell libs.

matthewbauer · 2018-07-16T21:52:06Z

Ok seems reasonable if this fixes the ARG_MAX issue (I wonder if you have tested it with something like stack on macOS). We should definitely be careful because a lot stuff probably depends on things like the package conf dir being /lib/ghc-8.4.3/package.conf.d.

Ericson2314 · 2018-07-16T22:17:16Z

pkgs/build-support/cc-wrapper/default.nix

@@ -40,7 +40,7 @@ let
  libc_lib = if libc == null then null else getLib libc;
  cc_solib = getLib cc;
  # The wrapper scripts use 'cat' and 'grep', so we may need coreutils.
-  coreutils_bin = if nativeTools then "" else getBin coreutils;
+  coreutils_bin = if (nativeTools || targetPlatform.isWindows) then "" else getBin coreutils;


This coreutils for the build platform, not host platform, so why disable it?

This ends up trying to build coreutils for mingw32 when cross compiling gcc.
And coreutils can't be built on mingw32.

Ericson2314 · 2018-07-16T22:20:37Z

pkgs/development/compilers/ghc/8.4.3.nix

    "--with-gmp-includes=${gmp.dev}/include" "--with-gmp-libraries=${gmp.out}/lib"
-  ] ++ stdenv.lib.optional (targetPlatform == hostPlatform && hostPlatform.libc != "glibc" && !targetPlatform.isWindows) [
+  ] else [
+    "--with-gmp-includes=${targetPackages.gmp.dev}/include" "--with-gmp-libraries=${targetPackages.gmp.out}/lib"


This can just be

stdenv.lib.optional (!enableIntegerSimple) [ "--with-gmp-includes=${targetPackages.gmp.dev}/include" "--with-gmp-libraries=${targetPackages.gmp.out}/lib" ]

targetPackages = pkgs on native.

peti

IMHO, this goes into the wrong direction. The right solution is not to list Haskell packages in buildInputs at all. They don't belong there because the generic builder has no clue how to use them anyway. Similarly, one could argue that C libraries required by the Haskell build mustn't be included in buildInputs either because it's imperative that Cabal finds them through explicitly configured search paths -- not through some magic $NIX_XYZ variables that don't exist outside of the Nix builder.

Arguably, buildInputs should be empty except for build-tools that can be discovered through $PATH.

domenkozar · 2018-07-17T22:10:51Z

@peti this PR fixes several macos haskell packages in pragmatic way, without sacrificing much. It does not touch buildInputs, just renames paths.

While I agree there should be a discussion about how different haskell dependencies are handled and propagated, I'd prefer we either merge these or revert cross compilation fixes that got us here, to be able to build macos again.

angerman · 2018-07-18T01:15:03Z

I'm in the process of breaking this up into a few smaller PRs.

@peti, I agree that haskell lirbaries should not be in buildInputs, I do not agree about C libraries though. There is some reasonable expectancy that libraries can be found in the default search paths.

@domenkozar the primary motivation for this change was to make cross compiling to windows work. That it resolves the issue on macOS is just a side effect :-)

@matthewbauer I would really hope that not much depends on $out/lib/package.conf.d except for in-flight stuff from the builder. After all we carefully construct custom package dbs for setup and build already.

peti · 2018-07-18T07:58:45Z

@angerman,

I agree that haskell lirbaries should not be in buildInputs, I do not agree about C libraries though. There is some reasonable expectancy that libraries can be found in the default search paths.

the problem is that Cabal must record the -I and -L flags required to link the C library into the package's config file, because without that information other packages that try to use the code will fail to compile and/or link. To accomplish that, all those paths must be registered during the configure stage with --extra-{include,lib}-dirs. If you do that, then the ability to find those files without giving any explicit search paths -- which buildInputs provides -- is unnecessary and arguably even counter-productive.

angerman · 2018-07-18T08:07:17Z

If we are going to pass --extra-{include,lib}-dirs for every c library, I agree.

peti · 2018-07-18T08:12:48Z

@angerman,

If we are going to pass --extra-{include,lib}-dirs for every c library, I agree.

we have to, really, because otherwise the generated packages won't work when used outside of the Nix builder.

@domenkozar,

I'd prefer we either merge these or revert cross compilation fixes that got us here, to be able to build macos again.

I would prefer to revert the changes that broke Darwin support. We can move those patches into a topic branch and have it compiled by Hydra, IMHO, so that @angerman and @Ericson2314 and others can continue their work there and get it in shape for merging it into master with reasonable support by the NixOS infrastructure. We do want those improvements after all! But having Darwin support broken for several weeks on end is not a good idea and we have to do something about it.

nh2 · 2018-07-19T00:16:55Z

I came here to report that the removal of /lib patches in this PR fix the Argument list too long issue I am running into (note: on Linux!): #41340 (comment)

I'm happy to have a workaround now, so thanks @angerman, this came just in time.

I agree with what @peti says. In the long run, we should make sure that all native libs are provided using --extra-{include,lib}-dirs so that Cabal knows about them, and putting Haskell libs there doesn't make much sense. Magic discovery at the last step through the linker wrapper is weird and hard to debug.

angerman added 6 commits July 15, 2018 16:24

[cc-wrapper] no coreutils on with mingw

092369c

[ghc] assert gmp

36f4832

[ghc] proper handling of integer libraries

e699a26

[haskell/generic-builder] stop the lib madness

3192a9e

[haskell/generic-builder] windows always has an active library

a8a7c55

This commit is not really correct. The `hasActiveLibrary` check is wrong. We can have an active library even if we do not ask for a static lirbary or dynamic one; we can still have just a set of objet files and archives.

[coreutils] assert not msvcrt.

3e4cd7f

angerman requested review from Ericson2314 and peti as code owners July 15, 2018 08:29

GrahamcOfBorg added 6.topic: haskell 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ labels Jul 15, 2018

angerman added 2 commits July 16, 2018 20:10

Update generic-builder.nix

a7f208f

Update lib.nix

8ffb78a

matthewbauer reviewed Jul 16, 2018

View reviewed changes

peti requested changes Jul 16, 2018

View reviewed changes

Ericson2314 reviewed Jul 16, 2018

View reviewed changes

peti requested changes Jul 17, 2018

View reviewed changes

nh2 mentioned this pull request Jul 19, 2018

Fully static Haskell executables - overview issue #43795

Open

60 tasks

angerman closed this Jul 27, 2018

matthewbauer mentioned this pull request Aug 10, 2018

Add Windows XamlWebView backend ghcjs/jsaddle#76

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/mingw fixes #43559

Feature/mingw fixes #43559

angerman commented Jul 15, 2018

angerman commented Jul 15, 2018

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018 •

edited

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018

angerman commented Jul 15, 2018

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018

domenkozar commented Jul 16, 2018

angerman commented Jul 16, 2018

domenkozar commented Jul 16, 2018

matthewbauer Jul 16, 2018

Ericson2314 Jul 16, 2018 •

edited

matthewbauer Jul 16, 2018

matthewbauer Jul 16, 2018

Ericson2314 Jul 16, 2018

peti left a comment

peti Jul 16, 2018

peti Jul 16, 2018

peti Jul 16, 2018

angerman commented Jul 16, 2018

domenkozar commented Jul 16, 2018 •

edited

matthewbauer commented Jul 16, 2018

Ericson2314 Jul 16, 2018

angerman Jul 17, 2018

Ericson2314 Jul 16, 2018

peti left a comment

domenkozar commented Jul 17, 2018

angerman commented Jul 18, 2018

peti commented Jul 18, 2018

angerman commented Jul 18, 2018

peti commented Jul 18, 2018

nh2 commented Jul 19, 2018

Feature/mingw fixes #43559

Feature/mingw fixes #43559

Conversation

angerman commented Jul 15, 2018

Motivation for this change

Things done

angerman commented Jul 15, 2018

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018 • edited

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018

angerman commented Jul 15, 2018

domenkozar commented Jul 15, 2018

angerman commented Jul 15, 2018

domenkozar commented Jul 16, 2018

angerman commented Jul 16, 2018

domenkozar commented Jul 16, 2018

Choose a reason for hiding this comment

Ericson2314 Jul 16, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angerman commented Jul 16, 2018

domenkozar commented Jul 16, 2018 • edited

matthewbauer commented Jul 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peti left a comment

Choose a reason for hiding this comment

domenkozar commented Jul 17, 2018

angerman commented Jul 18, 2018

peti commented Jul 18, 2018

angerman commented Jul 18, 2018

peti commented Jul 18, 2018

nh2 commented Jul 19, 2018

angerman commented Jul 15, 2018 •

edited

Ericson2314 Jul 16, 2018 •

edited

domenkozar commented Jul 16, 2018 •

edited