haskell generic-builder: fix Darwin regression for lmdb

[simonchatts]

Recent updates to the generic builder have caused haskellPackages.lmdb-simple to
fail to build on Darwin, since it cannot see the lmdb C dynamic library included
by its dependent haskellPackages.lmdb.

The C dynamic library has suffix .so not .dylib, so this fix allows for
that.

Closes #80190, but that issue may identify a preferable solution.

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/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[cdepillabout]

cc @infinisil

[cdepillabout]

@GrahamcOfBorg build haskellPackages.lmdb-simple

[FRidh]

${stdenv.hostPlatform.extensions.sharedLibrary}

[simonchatts]

Thanks for the review @FRidh! Just to clarify, you're suggesting it should be this:

- for lib in "$d/"*.dylib; do
+ for lib in "$d/"*{${stdenv.hostPlatform.extensions.sharedLibrary},.so}; do

right? Or maybe a bit clearer like this:

- for lib in "$d/"*.dylib; do
+ for lib in "$d/"*${stdenv.hostPlatform.extensions.sharedLibrary} "$d/"*.so; do

I'm happy to do either of these, but to my eyes this makes the code more obfuscated. This whole section of code is intensely Darwin-specific, and the code has to know about both .so and .dylib, so there's no generality gained.

So can I just check that this is what you think would be an improvement? If so I'll happily re-test and push.

[FRidh]

stdenv.hostPlatform.extensions.sharedLibrary evaluates to .so on Linux and .dylib on Darwin. I doubt you need to perform an action on both .dylib and .so in the same build.

[simonchatts]

Unfortunately, that is exactly the point of this patch. See #80190 for more details.

I share your intuition that there should be a better way, and would love someone to propose a superior alternative. Right now though, this is the only fix I have for the broken packages.

[simonchatts]

BTW the Darwin nix-store in front of me (no cross-compilation) has 44421 .dylibs and 778 .sos. Of those .sos, there are two distinct ones that otool -hv reports as DYLIB rather than BUNDLE:

• the libstdbuf.so from coreutils
• the liblmdb.so from lmdb at issue here

I have no idea whether the real root cause for this issue is something to do with them, but as it stands, there definitely are dylibs with suffix .so on Darwin.

[simonchatts]

Turns out the liblmdb .so suffix was specified manually in 84bd2f4 over two years ago.
I still do not understand why 8d4509e from a couple of weeks ago should trigger a breakage though.

[veprbl]

The install_name needs to point to the full path to the final library location. Perhaps, the library got moved as a result of 8d4509e so the line

nixpkgs/pkgs/development/libraries/lmdb/default.nix

Line 26 in 82945f3

++ stdenv.lib.optional stdenv.isDarwin "LDFLAGS=-Wl,-install_name,$(out)/lib/liblmdb.so";

needs to be updated to reflect the move.

[simonchatts]

Good thought @veprbl - I've looked, and to the best of my understanding, this isn't the diagnosis. Both working and broken builds end up depending on the exact same LMDB C package (same SHA) where otool -L does indeed show the full path to the final library location.

[veprbl]

I think the issue might be with this line

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

Line 393 in 8d4509e

sed -i "s,dynamic-library-dirs: .*,dynamic-library-dirs: $dynamicLinksDir," "$f"

It used to add "links" directory and replace other directory or multiple directories. After the 8d4509e it only replaces all directories in the "dynamic-library-dirs:".

This can be demonstrated using an override:

diff --git a/pkgs/development/haskell-modules/configuration-common.nix b/pkgs/development/haskell-modules/configuration-common.nix
index 8f127e9490d..21481682143 100644
--- a/pkgs/development/haskell-modules/configuration-common.nix
+++ b/pkgs/development/haskell-modules/configuration-common.nix
@@ -15,6 +15,15 @@ with haskellLib;
 
 self: super: {
 
+lmdb-simple = super.lmdb-simple.overrideAttrs (old: {
+  setupCompilerEnvironmentPhase =
+    builtins.replaceStrings [
+      ''sed -i "s,dynamic-library-dirs: .*,dynamic-library-dirs: $dynamicLinksDir," "$f"''
+    ] [
+      ''sed -i "s,dynamic-library-dirs: ,dynamic-library-dirs: $dynamicLinksDir ," "$f"''
+    ] old.setupCompilerEnvironmentPhase;
+});
+
   # Arion's test suite needs a Nixpkgs, which is cumbersome to do from Nixpkgs
   # itself. For instance, pkgs.path has dirty sources and puts a huge .git in the
   # store. Testing is done upstream.

[veprbl]

Some background on the code in question: 7131e06 #22810

We might be able to remove this logic altogether if the there is no size limit on the recent macOS versions. If that's not the case, this PR is doing the right fix, as far as I understand now.

[infinisil]

@veprbl Oh yeah I think you got it. I think the underlying problem is that the dynamic-library-dirs key is essentially processed like this (wherever it's used)

for d in $dynamic-library-dirs; do
  for f in "$d"/*; do
    if is_valid_dynamic_library "$f"; do
      use_dynamic_library "$f";
    fi
  done
done

This means it will catch all files, no matter the extension, even though it should always be .so on Linux and .dylib on macOS. Before 8d4509e, the generic builder would transform

# From dependency A
dynamic-library-dirs: alibs1
                      alibs2
# From dependency B
dynamic-library-dirs: blibs1
                      blibs2

into

# From dependency A
dynamic-library-dirs: link-dir
                      alibs2
# From dependency B
dynamic-library-dirs: link-dir
                      blibs2

Where link-dir contains all .dylibs from alibs1 and blibs1. But after 8d4509e it transforms it into

# From dependency A
dynamic-library-dirs: link-dir
# From dependency B
dynamic-library-dirs: link-dir

Where now link-dir contains all .dylibs from alibs1, alibs2, blibs1 and blibs2.

The problem with this is that all .so files (and others) in alibs2 and blibs2 are now ignored! (in addition to the ones in alibs1 and blibs1 that were ignored before already).

With this said, I think the "correct" way to fix this is to determine what files to link with a is_valid_dynamic_library-like function, instead of relying on extensions. E.g. with otool -hv as mentioned in #80190 to determine whether something is a mach-o library.

However I'd be fine with the fix in this PR too

[cdepillabout]

@infinisil What do you want to do with this PR?

I think the "correct" way to fix this is to determine what files to link with a is_valid_dynamic_library-like function

Do you want to merge in this PR? Or fix it as described above in a future PR?

[infinisil]

Eh this should be fine. I don't think we'll encounter any other extensions being marketed as dylib's, and if so it's not really our fault anyways.

[veprbl]

Mass rebuild shouldn't be going to master...

[infinisil]

Reverted in cfc21ad

[infinisil]

Opened #80337 for having this in staging

[kirelagin]

Unfortunately, it looks like you need to link .as as well, because this new directory overrides everything else. So linking will fail if your haskell package will try to link with a library that only exists in the static version.