haskell: work around linker limits on Mac OS X Sierra. by judah · Pull Request #25537 · NixOS/nixpkgs

judah · 2017-05-05T16:27:24Z

The Sierra linker added a limit on the number of paths that any one
dynamic library (*.dylib) can reference. This causes problems when
a Haskell library has many immediate dependencies (#22810).

We follow a similar fix as GHC/Cabal/Stack: for each derivation,
create a new directory with symlinks to all the dylibs of its immediate
dependencies, and patch its package DB to reference that directory
using the new dynamic-library-dirs field.

Note that this change is a no-op for older versions of GHC, i.e., they will
continue to fail on some packages as before.

Also note that this change causes the bootstrapped versions of GHC to be
recompiled, since they depend on hscolour which is built by
generic-builder.nix.

Tested by building the stack binary as described in #22810.

Built on platform(s)
- NixOS
- macOS
- Linux
Fits CONTRIBUTING.md.

The Sierra linker added a limit on the number of paths that any one dynamic library (`*.dylib`) can reference. This causes problems when a Haskell library has many immediate dependencies (NixOS#22810). We follow a similar fix as GHC/Cabal/Stack: for each derivation, create a new directory with symlinks to all the dylibs of its immediate dependencies, and patch its package DB to reference that directory using the new `dynamic-library-dirs` field. Note that this change is a no-op for older versions of GHC, i.e., they will continue to fail on some packages as before. Also note that this change causes the bootstrapped versions of GHC to be recompiled, since they depend on `hscolour` which is built by `generic-builder.nix`. Tested by building the `stack` binary as described in NixOS#22810.

mention-bot · 2017-05-05T16:27:26Z

@judah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @peti, @cstrahan and @dmjio to be potential reviewers.

judah · 2017-05-05T16:29:23Z

cc @shlevy

peti

I wonder about the strange way the Darwin-specific code in integrated.

peti · 2017-05-05T16:33:19Z

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

      fi
    done
+
+    if "${if stdenv.isDarwin then "true" else "false"}"; then


Considering the fact you have conditionals in the Nix language, why is that feature used here to create input for yet another level of conditional that's evaluated by the shell? This looks awkward. Is there a deeper reason for this implementation choice?

It was mostly due to my lack of familiarity with Nix. I've switched to @shlevy 's suggestion.

peti · 2017-05-05T16:33:32Z

pkgs/development/haskell-modules/with-packages-wrapper.nix

    done

+    # Work around a linker limit in Mac OS X Sierra (see generic-builder.nix):
+    if "${if stdenv.isDarwin then "true" else "false"}"; then


shlevy

Untested, but overall looks great! Can you just change the stylistic nit (in both files)?

shlevy · 2017-05-05T16:34:25Z

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

      fi
    done
+
+    if "${if stdenv.isDarwin then "true" else "false"}"; then


Nit: prefer

'' + (stdenv.lib.optionalString stdenv.isDarwin '' darwin-only-command '') + '' the-rest ''

Done, thanks for the suggestion.

shlevy · 2017-05-05T16:55:49Z

@judah You've tested the latest?

judah · 2017-05-05T16:57:34Z

Running the test again now. It will probably take a couple hours since it needs to re-bootstrap GHC. I'll ping back on this thread once I've seen the test pass.

LnL7 · 2017-05-05T18:52:52Z

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

        configureFlags+=" --extra-lib-dirs=$p/lib"
      fi
    done
+  '' + (optionalString stdenv.isDarwin ''


I think we also want to skip this when building a static library/executable.

It's a no-op in that case since static libraries have the different extension *.a and aren't affected by the dynamic-library-dirs field.

judah · 2017-05-05T21:44:09Z

Confirmed that the build still passed after the fix.

dmjio · 2017-05-05T22:36:45Z

LGTM 👍

cstrahan · 2017-05-05T22:53:10Z

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

+    # libraries) from all the dependencies.
+    local dynamicLinksDir="$out/lib/links"
+    mkdir -p $dynamicLinksDir
+    local foundDylib=false


I don't see where foundDylib is used. Perhaps you wanted to set foundDylib to "true" in the for loop below, and then if no dylib is found, delete the (empty) $out/lib/links dir?

Sorry about that; sent out #25602 to remove the line.

Originally I was going to add more complicated logic to handle the different Linux/Mac cases (since the former uses a different extension for shared libraries). But the call to stdenv.isDarwin has an equivalent effect.

peti · 2017-05-07T07:06:42Z

@judah, can you please address @cstrahan's comments from #25537 (review)?

Previously the package conf files were handled without paying attention to the fact that it's pretty-printed output. One problem was discovered with GHC 8.8.1 on Darwin, where the dynamic-library-dirs first field seems to have increased in length, meaning while before it was dynamic-library-dirs: some-small-directory-name some-more-directories Now it is dynamic-library-dirs: some-larger-directory-name some-more-directories Which breaks the code installed for NixOS#25537, because that assumed the former format, resulting in the reoccurence of the bug in NixOS#22810, see infinisil/all-hies#43 This commit fixes this by "unprettyfying" the package conf files before processing them.

Previously the package conf files were handled without paying attention to the fact that it's pretty-printed output. One problem was discovered with GHC 8.8.1 on Darwin, where the dynamic-library-dirs first field seems to have increased in length, meaning while before it was dynamic-library-dirs: some-small-directory-name some-more-directories Now it is dynamic-library-dirs: some-larger-directory-name some-more-directories Which breaks the code installed for #25537, because that assumed the former format, resulting in the reoccurence of the bug in #22810, see infinisil/all-hies#43 This commit fixes this by "unprettyfying" the package conf files before processing them. Closes #78738.

Previously the package conf files were handled without paying attention to the fact that it's pretty-printed output. One problem was discovered with GHC 8.8.1 on Darwin, where the dynamic-library-dirs first field seems to have increased in length, meaning while before it was dynamic-library-dirs: some-small-directory-name some-more-directories Now it is dynamic-library-dirs: some-larger-directory-name some-more-directories Which breaks the code installed for NixOS#25537, because that assumed the former format, resulting in the reoccurence of the bug in NixOS#22810, see infinisil/all-hies#43 This commit fixes this by "unprettyfying" the package conf files before processing them. Closes NixOS#78738.

The workaround that this change deletes was initially contributed in NixOS#25537 to mitigate NixOS#22810 until a more permanent solution could be devised. That more permanent solution was eventually contributed in NixOS#27536, which now obviates the initial workaround, so this change removes it.

LnL7 added 6.topic: darwin Running or building packages on Darwin 6.topic: haskell General-purpose, statically typed, purely functional programming language labels May 5, 2017

peti requested changes May 5, 2017

View reviewed changes

shlevy requested changes May 5, 2017

View reviewed changes

Fix use of isDarwin conditionals.

2caa7b8

peti approved these changes May 5, 2017

View reviewed changes

shlevy approved these changes May 5, 2017

View reviewed changes

LnL7 reviewed May 5, 2017

View reviewed changes

cstrahan reviewed May 5, 2017

View reviewed changes

shlevy merged commit 2caa7b8 into NixOS:master May 5, 2017

judah mentioned this pull request May 7, 2017

Remove unused local command. #25602

Merged

Dridus mentioned this pull request May 8, 2017

macOS Sierra and Haskell "malformed mach-o: load commands size" #22810

Closed

shlevy mentioned this pull request Jul 21, 2017

Linking hack for macOS Sierra #27536

Merged

8 tasks

LnL7 mentioned this pull request Dec 5, 2017

Revive multiple outputs for Haskell packages. #32112

Merged

avieth mentioned this pull request May 1, 2018

[CBR-51] use stack lts-11.2 (#2735) input-output-hk/cardano-sl#2745

Merged

infinisil mentioned this pull request Jan 28, 2020

hie-8.8.1 from unstable errors out at launch infinisil/all-hies#43

Closed

infinisil mentioned this pull request Jan 29, 2020

haskell generic-builder: Fix package conf handling #78738

Closed

1 task

gnprice mentioned this pull request Apr 2, 2020

Ignore error of symlink #83605

Closed

10 tasks

Gabriella439 mentioned this pull request Dec 1, 2022

haskellPackages.mkDerivation: Remove Darwin dynamic libraries workaround #203963

Closed

1 task

Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023

Uh oh!

Comments

Conversation

judah commented May 5, 2017

Uh oh!

mention-bot commented May 5, 2017

Uh oh!

judah commented May 5, 2017

Uh oh!

peti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shlevy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shlevy commented May 5, 2017

Uh oh!

judah commented May 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

judah commented May 5, 2017

Uh oh!

dmjio commented May 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peti commented May 7, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants