Fix some GHC dependencies on iOS #40393

ElvishJerricco · 2018-05-12T06:22:06Z

Motivation for this change

This fixes a few iOS related packages, particularly with the goal of fixing GHC dependencies. GHC itself does not quite build yet, but its dependencies do.

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.

/cc @ryantrinkle @Ericson2314

ElvishJerricco · 2018-05-12T06:23:27Z

@ryantrinkle I left out 47f2e72. @angerman seemed to think it should have worked as it was before. Could you clarify the purpose of that commit?

ElvishJerricco · 2018-05-12T06:24:53Z

I also left out 1bb6287. I think it's better to just require people to use haskell.packages.integer-simple; that way they don't have to do overrides to work with the gmp version (which I believe we expect to have working eventually?)

peti · 2018-05-12T08:05:37Z

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

@@ -402,6 +406,21 @@ stdenv.mkDerivation ({
        export NIX_${ghcCommandCaps}PKG="${ghcEnv}/bin/${ghcCommand}-pkg"
        # TODO: is this still valid?
        export NIX_${ghcCommandCaps}_DOCDIR="${ghcEnv}/share/doc/ghc/html"
+        export LD_LIBRARY_PATH="''${LD_LIBRARY_PATH:+''${LD_LIBRARY_PATH}:}${


Setting LD_LIBRARY_PATH in the generic builder is not a good idea. It's important that all relevant search paths are registered with Cabal because otherwise the generated library config file does not contain them and then libraries will fail to link when they're used by other components, etc.

What is the right way to register this with Cabal?

In cc-wrapper. I'll do this once I get home. It will be mass rebuild however.

Err sorry that would be for the cflags link part. Maybe Cabal has an --extra-framework-dirs?

Ah but this is the shell env. @peti the env is really screwed up, you know, because the system-haskell division of deps does not take into account propagated dependencies. We drop propagated systems deps (including frameworks) on the floor since ghcWithPackages doesn't reexport them. I like the idea of ghcWithPackages, but it is very incompatible with the bash-heavy cc-wrapper way of doing things. More thought is needed.

I'll make a separate PR for --extra-framework-dirs. (edit done in #40442 and #40443.)

@ElvishJerricco let's try to land #39735 by @kirelagin for this reflex-platform release too.

@Ericson2314 So you're going to tackle these env issues in a separate PR? Should I remove the env changes from this PR entirely?

peti · 2018-05-12T08:13:15Z

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

+# On macOS, statically linking against system frameworks is not supported;
+# see https://developer.apple.com/library/content/qa/qa1118/_index.html
+# They must be propagated to the environment of any executable linking with the library
+, libraryDarwinFrameworkDepends ? [], executableDarwinFrameworkDepends ? []


Cabal refers to these dependencies simply as frameworks in its BuildInfo type, therefore I think we should adopt that nomenclature and simply call them framework here, too, rather than darwinFramework.

peti · 2018-05-12T08:14:45Z

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

-                     optionals (allPkgconfigDepends != []) allPkgconfigDepends ++
+  propagatedBuildInputs = buildDepends ++ libraryHaskellDepends ++ executableHaskellDepends ++ libraryDarwinFrameworkDepends;
+  otherBuildInputs = setupHaskellDepends ++ extraLibraries ++ librarySystemDepends ++ executableSystemDepends ++ executableDarwinFrameworkDepends ++
+                     optionals (allPkgconfigDepends != []) ([pkgconfig] ++ allPkgconfigDepends) ++
                     optionals doCheck (testDepends ++ testHaskellDepends ++ testSystemDepends ++ testToolDepends) ++


We require a testFrameworkDepends argument, too.

peti · 2018-05-12T08:15:22Z

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

-                     optionals (allPkgconfigDepends != []) allPkgconfigDepends ++
+  propagatedBuildInputs = buildDepends ++ libraryHaskellDepends ++ executableHaskellDepends ++ libraryDarwinFrameworkDepends;
+  otherBuildInputs = setupHaskellDepends ++ extraLibraries ++ librarySystemDepends ++ executableSystemDepends ++ executableDarwinFrameworkDepends ++
+                     optionals (allPkgconfigDepends != []) ([pkgconfig] ++ allPkgconfigDepends) ++
                     optionals doCheck (testDepends ++ testHaskellDepends ++ testSystemDepends ++ testToolDepends) ++
                     optionals doBenchmark (benchmarkDepends ++ benchmarkHaskellDepends ++ benchmarkSystemDepends ++ benchmarkToolDepends);


Ditto with benckmarkFrameworkDepends.

Ericson2314 · 2018-05-12T22:48:52Z

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

-                     optionals (allPkgconfigDepends != []) allPkgconfigDepends ++
+  propagatedBuildInputs = buildDepends ++ libraryHaskellDepends ++ executableHaskellDepends ++ libraryDarwinFrameworkDepends;
+  otherBuildInputs = setupHaskellDepends ++ extraLibraries ++ librarySystemDepends ++ executableSystemDepends ++ executableDarwinFrameworkDepends ++
+                     optionals (allPkgconfigDepends != []) ([pkgconfig] ++ allPkgconfigDepends) ++


Pkg-config is up above as a native build input.

Ericson2314 · 2018-05-12T22:55:48Z

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

@@ -402,6 +406,21 @@ stdenv.mkDerivation ({
        export NIX_${ghcCommandCaps}PKG="${ghcEnv}/bin/${ghcCommand}-pkg"
        # TODO: is this still valid?
        export NIX_${ghcCommandCaps}_DOCDIR="${ghcEnv}/share/doc/ghc/html"
+        export LD_LIBRARY_PATH="''${LD_LIBRARY_PATH:+''${LD_LIBRARY_PATH}:}${


In cc-wrapper. I'll do this once I get home. It will be mass rebuild however.

Ericson2314 · 2018-05-12T22:58:15Z

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

@@ -402,6 +406,21 @@ stdenv.mkDerivation ({
        export NIX_${ghcCommandCaps}PKG="${ghcEnv}/bin/${ghcCommand}-pkg"
        # TODO: is this still valid?
        export NIX_${ghcCommandCaps}_DOCDIR="${ghcEnv}/share/doc/ghc/html"
+        export LD_LIBRARY_PATH="''${LD_LIBRARY_PATH:+''${LD_LIBRARY_PATH}:}${


Err sorry that would be for the cflags link part. Maybe Cabal has an --extra-framework-dirs?

Ericson2314 · 2018-05-12T22:58:50Z

pkgs/os-specific/darwin/apple-source-releases/libiconv/default.nix

@@ -1,14 +1,18 @@
 { stdenv, appleDerivation }:

 appleDerivation {
-  preConfigure = "cd libiconv";
+  preConfigure = "cd libiconv" #TODO: This `cd` should be in postUnpack, not preConfigure; otherwise, autoreconfHook breaks


This TODO is done on staging.

@Ericson2314 So I should remove this preConfigure entirely?

Sadly not, because I can't figure out how to get autoreconf hook to work.

So then what do you want me to do with this?

Keep the below part but get rid of the comment so that specific line is unchanged.

@Ericson2314 I should switch this from stdenv.hostPlatform != stdenv.buildPlatform to something like hostPlatform.isDarwin or something, right?

You could do hostPlatofrm.isiOS. This is already a Darwin-specific version of libiconv.

ElvishJerricco · 2018-05-13T06:03:02Z

@Ericson2314 Also, should this PR be pointing at master or staging?

Ericson2314 · 2018-05-13T15:00:33Z

@ElvishJerricco by the rebuild count I guess staging? We can still base them off a master commit so we can use them without pulling in as much of a rebuild I suppose.

ElvishJerricco · 2018-05-13T22:54:18Z

@Ericson2314 what about the env changes? Should I remove those from this PR?

ElvishJerricco · 2018-05-16T07:34:05Z

@Ericson2314 ping

peti · 2018-05-16T16:17:30Z

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

-                     optionals doBenchmark (benchmarkDepends ++ benchmarkHaskellDepends ++ benchmarkSystemDepends ++ benchmarkToolDepends);
+  propagatedBuildInputs = buildDepends ++ libraryHaskellDepends ++ executableHaskellDepends ++ libraryFrameworkDepends;
+  otherBuildInputs = setupHaskellDepends ++ extraLibraries ++ librarySystemDepends ++ executableSystemDepends ++ executableFrameworkDepends ++
+                     optionals (allPkgconfigDepends != []) ([pkgconfig] ++ allPkgconfigDepends) ++


Why do you add [pkgconfig] here? It's already part of nativeBuildInputs.

Agreed. I had a comment on that too but GitHub evidently ate it.

Ericson2314 · 2018-05-16T17:34:51Z

@ElvishJerricco lets just land the framework changes (maybe on 18.03, too) and then return to the rest of this.

ElvishJerricco · 2018-05-16T17:58:43Z

@Ericson2314 I've removed all the changes to generic-builder and the comment in libiconv, and I've switched the condition in libiconv to isiOS. Does that make this merge-able?

Ericson2314 · 2018-05-16T18:14:24Z

@ElvishJerricco see 62fd669 on staging, it should be targetPrefix now. That's the last thing

Overall, I wanted to start with the framework depends (outside env) because that is unambiguously a good thing, whereas these most of these are our grosser hacks I'm fine not upstreaming until the last second.

Ericson2314 · 2018-05-16T18:16:06Z

pkgs/development/node-packages/default-v6.nix

@@ -59,6 +59,7 @@ nodePackages // {

  ios-deploy = nodePackages.ios-deploy.override (oldAttrs: {
    preRebuild = ''
+      LD=$CC


there is another default-v*.nix that probably needs this too.

pushed wrong radio button

ElvishJerricco · 2018-05-16T18:25:34Z

@Ericson2314 Yes, the targetPrefix thing was fixed in the second commit. I'll squash the first one away.

With the generic-builder stuff gone, this PR is pretty small. I don't really see anything particularly gross.

Needs to link with a C compiler, not linker directly

Ericson2314 · 2018-05-16T18:31:12Z

OK will merge. I don't like not being able to run autoreconfHook and having to sed instead but whatever; stuff is small. :)

ElvishJerricco requested a review from peti as a code owner May 12, 2018 06:22

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

peti requested changes May 12, 2018

View reviewed changes

ElvishJerricco force-pushed the ios-prebuilt-dependency-fixes branch from 7bbacff to 0466a57 Compare May 12, 2018 23:06

Ericson2314 reviewed May 13, 2018

View reviewed changes

peti requested changes May 16, 2018

View reviewed changes

ElvishJerricco force-pushed the ios-prebuilt-dependency-fixes branch 2 times, most recently from febf5aa to 5120c8c Compare May 16, 2018 17:56

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 0 and removed 6.topic: haskell 10.rebuild-darwin: 501+ 10.rebuild-linux: 501+ labels May 16, 2018

Ericson2314 previously approved these changes May 16, 2018

View reviewed changes

Make libiconv build for iOS with prefixed tools

4956767

ElvishJerricco force-pushed the ios-prebuilt-dependency-fixes branch from 5120c8c to ccdc91b Compare May 16, 2018 18:26

ios-rebuild: Unbreak build

ceb8f6e

Needs to link with a C compiler, not linker directly

ncurses: when cross-compiling for iOS, use built-in ncurses

4f25cf5

ElvishJerricco force-pushed the ios-prebuilt-dependency-fixes branch from ccdc91b to 4f25cf5 Compare May 16, 2018 18:28

peti approved these changes May 16, 2018

View reviewed changes

Ericson2314 merged commit f3fcf1b into NixOS:master May 16, 2018

Fix some GHC dependencies on iOS #40393

Fix some GHC dependencies on iOS #40393

Conversation

ElvishJerricco commented May 12, 2018

Motivation for this change

Things done

ElvishJerricco commented May 12, 2018

ElvishJerricco commented May 12, 2018

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

Choose a reason for hiding this comment

Ericson2314 May 13, 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

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

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

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

ElvishJerricco commented May 13, 2018

Ericson2314 commented May 13, 2018

ElvishJerricco commented May 13, 2018

ElvishJerricco commented May 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ericson2314 commented May 16, 2018 • edited

ElvishJerricco commented May 16, 2018

Ericson2314 commented May 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElvishJerricco commented May 16, 2018

Ericson2314 commented May 16, 2018

Ericson2314 May 13, 2018 •

edited

Ericson2314 commented May 16, 2018 •

edited