Don't duplicate the configureFlags when not on Windows

[mrBliss]

When not on Windows, the configureFlags attr of libmpc is set to
(drv.configureFlags or []). Since drv.configureFlags seems to be equal to
["--enable-static" "--disable-shared"] (at least on my machine),
configureFlags appears to be merged with itself and in the end, it is equal
to:
["--enable-static" "--disable-shared" "--enable-static" "--disable-shared"].

You can verify this by comparing the derivations of libmpc with and without
this patch.

This changes the derivation of libmpc as well the derivation of all dependent
packages, like gcc, causing cache misses from https://cache.nixos.org/.

Because these packages are built and stored in the IOHK cache, users don't
notice this, until they use a different version of nixpkgs than the one pinned
by haskell.nix, e.g., one with a different version of glibc so that the IOHK
cache cannot be used. This results in gcc and other packages being built from
scratch instead of being downloaded from https://cache.nixos.org/.

Fix this by hoisting the conditional one level higher so that overrideAttrs
isn't even called on non-Windows OSes. Do the same for mpfr for consistency.

I haven't tested this on Windows. I'm not sure whether I have hoisted the
conditional too high, see the first comment in the file.

[michaelpj]

Since drv.configureFlags seems to be equal to
["--enable-static" "--disable-shared"] (at least on my machine),
configureFlags appears to be merged with itself and in the end, it is equal
to:

I don't really understand this. We are overriding configureFlags to itself or [], I don't see how it could get merged with itself?

[mrBliss]

Since drv.configureFlags seems to be equal to
["--enable-static" "--disable-shared"] (at least on my machine),
configureFlags appears to be merged with itself and in the end, it is equal
to:

I don't really understand this. We are overriding configureFlags to itself or [], I don't see how it could get merged with itself?

When I wrote the explanation above, I thought it was because mkMerge is used for configureFlags so that doing configureFlags = [X] twice will result in configureFlags = [X X]. Maybe mkMerge doesn't even work like this or is cleverer than that (and filters out duplicates), but I have a hard time finding its documentation to check this.

Actually, after opening the PR, I discovered that mkMerge isn't even used for configureFlags. After experimenting a bit more with it, I admit that I don't really understand what is going.

While experimenting with it, I noticed that the cmakeFlags and mesonFlags are related to this. This led me to makeStaticLibraries, which is used for mpfr and libmpc. My guess is that this code is responsible for it, but, as a Nix newbie, I'm having a hard time figuring out what is going on. Thought: should makeStaticLibraries be used when on Windows instead of the manual overriding of the configureFlags?

However, this PR does fix the problem of the duplicate flags on my machine (which is a bug IMHO), but probably not in the proper way (I also don't know whether it does the right thing on Windows).

[hamishmack]

I wonder why this used "--enable-static --disable-shared" instead of "--enable-static" "--disable-shared". That is only on windows though and I don't think it could have made a difference on non windows systems.

Is there a way to reproduce the issue you are seeing? It seems like something somewhere else must have make a distinction between configureFlags not existing and being [] and I think that is probably a bug somewhere else.

In particular I would love to be able to run a nix diff with or without this change to see what is going on.

[mrBliss]

I wonder why this used "--enable-static --disable-shared" instead of "--enable-static" "--disable-shared". That is only on windows though and I don't think it could have made a difference on non windows systems.

Yeah, we thought the same 🙂.

Is there a way to reproduce the issue you are seeing? It seems like something somewhere else must have make a distinction between configureFlags not existing and being [] and I think that is probably a bug somewhere else.

In particular I would love to be able to run a nix diff with or without this change to see what is going on.

Something to reproduce:

1. Check out this branch and cd to the root of the repo.

2. Instantiate the derivation of gcc, which depends on libmpc, saving it as with-patch:

   $ nix-instantiate -E 'with import ./default.nix { }; import sources.nixpkgs-2111 nixpkgsArgs' -A gcc --add-root with-patch

3. Instantiate the derivation of gcc again, but without any of haskell.nix's overlays:

   $ nix-instantiate -E 'with import ./default.nix { }; import sources.nixpkgs-2111 (nixpkgsArgs // { overlays = []; })' -A gcc --add-root without-overlays

4. Use nix-diff to compare with-patch with without-overlays:

   $ nix-diff $(readlink with-patch) $(readlink without-overlays)
   # No output, no diff

   This is what I expect, at least on non-Windows OSses (I reproduced this on Linux): haskell.nix just uses the stock version of gcc from nixpkgs.

5. Now undo the patch:

   $ git revert HEAD --no-edit

6. Instantiate the derivation of gcc again, saving it as without-patch:

   $ nix-instantiate -E 'with import ./default.nix { }; import sources.nixpkgs-2111 nixpkgsArgs' -A gcc --add-root without-patch

7. Use nix-diff to compare with-patch with without-patch:

   $ nix-diff $(readlink with-patch) $(readlink without-patch)
   - /nix/store/2vb30d6i3lvx6x5wqrdn5sh6dw03sxpl-gcc-wrapper-10.3.0.drv:{out}
   + /nix/store/44dad9q35ycjxb47b8c8bpgyz965r6i1-gcc-wrapper-10.3.0.drv:{out}
   • The input derivation named `bash-5.1-p8` differs
     - /nix/store/5lv335av7515vrbvs0l3f5pdzh01ly4c-bash-5.1-p8.drv:{out}
     + /nix/store/v8wcppd3nykz1q5hvjkzag0qwmqxdxi3-bash-5.1-p8.drv:{out}
     • The input derivation named `bootstrap-stage4-gcc-wrapper-10.3.0` differs
       - /nix/store/7x5j7048q13arnz2164vwcdxdgr7xlic-bootstrap-stage4-gcc-wrapper-10.3.0.drv:{out}
       + /nix/store/q1l31bxbhavgjniqgly59i3nq6n36bf0-bootstrap-stage4-gcc-wrapper-10.3.0.drv:{out}
       • The input derivation named `gcc-10.3.0` differs
         - /nix/store/mi1r3hp3giiph33n757f76si4cb7wfzs-gcc-10.3.0.drv:{info,lib,man,out}
         + /nix/store/l1zpvxx939dzxw1in2w5s3pvc6r8y482-gcc-10.3.0.drv:{info,lib,man,out}
         • The input derivation named `libmpc-1.2.1` differs
           - /nix/store/lc3mqqrm0vkl23brclrpmpacccylf8ax-libmpc-1.2.1.drv:{out}
           + /nix/store/ms9fjhk5ywd9a3ypi2skh9yvjv5dky49-libmpc-1.2.1.drv:{out}
           • The environments do not match:
               cmakeFlags=-DBUILD_SHARED_LIBS:BOOL=OFF→ →→-DBUILD_SHARED_LIBS:BOOL=OFF→
               configureFlags=''
               --enable-static --disable-shared→ →→--enable-static→→ →→--disable-shared→
           ''
               mesonFlags=-Ddefault_library=static→ →→-Ddefault_library=static→
         • Skipping environment comparison
       • Skipping environment comparison
     • The input derivation named `bootstrap-stage4-stdenv-linux` differs
       - /nix/store/cs6vb9gffh7qixayh4lwsb8a2jg6bvkf-bootstrap-stage4-stdenv-linux.drv:{out}
       + /nix/store/4p8lran3jll7wapf5czyvr1c436v8mcf-bootstrap-stage4-stdenv-linux.drv:{out}
   ...

   I recommend reproducing this diff locally, with colors, that's much more readable, IMHO, than what I pasted above.

   In the diff you can see the configureFlags, cmakeFlags, and mesonFlags differ. The diff says they are duplicated. This causes gcc to be compiled from source instead of using the prebuilt one from nixpkgs.

[hamishmack]

It seems that even just .overrideAttrs (drv: {}) on libmpc is enough to change the derivation.

The issue seems to be caused by NixOS/nixpkgs@f110a18

Found using the following script with git bisect run:

#!/bin/sh

if [ "$(nix-instantiate -E 'import ./. { overlays = [(final: prev: { libmpc = prev.libmpc.overrideAttrs (drv: {}); })]; }' -A gcc)" == "$(nix-instantiate -E 'import ./. {}' -A gcc)" ]; then
	echo GOOD
	exit 0
else
	echo BAD
	exit 1
fi