wrapCC, wrapBintools: use absolute paths

[tie]

Description of changes

This change sets {CC,CXX,LD,etc…}{,_FOR_BUILD,_FOR_TARGET} variables to absolute paths in cc-wrapper and bintools-wrapper setup hooks.

It’s common to set

{
  depsBuildBuild = [ buildPackages.stdenv.cc ];
}

when cross-compiling (see doc/stdenv/cross-compilation.chapter.md), however, cc/bintools wrapper setup hook currently sets these variables to program name instead of absolute path. That obviously doesn’t work e.g. when build and host platform have the same C compiler name because only the first program from PATH is used (for example, this is part of the reason why static glibc doesn’t work in Nixpkgs and we have pkgsStatic use musl that has different target prefix).

Prerequisites:

• gawk: move gawkbug to gawkInteractive #314720
• gnumake: do not use MAKE_CXX #314716
• tests-stdenv-gcc-stageCompare: cleanup #316011
• puppet-bolt: do not reset default gem config #316023
• postgresql: remove unnecessary store references #322025
  postgresql: split -lib and -dev outputs cleanly #294504

Note: I’ll be adding new PRs split from this one to the list above.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[tie]

Rebuild from nixpkgs-review is going well so far, most of the failures seems to be unrelated to this change (i.e. broken packages, outdated source hashes).

nixpkgs-review pr \
  --no-shell \
  --eval local \
  --checkout commit \
  --extra-nixpkgs-config '{ allowUnfree = false; allowBroken = false; }' \
  --build-args '--option substitute false' \
  --print-result \
  314920

[alyssais]

Looks like some unrelated commits have found their way in.

[tie]

@alyssais, I don’t think so? The main change is in 3b6195a82ee60f528852b34e097c0582734deb1e with other commits mostly fixing occasional $CC references introduced by package build systems. I’ll try to merge these separately (hence the draft status for now), but they are still required for this PR, especially stdenv stuff like gnumake and gawk.

[alyssais]

ah right, I missed the prerequisites mention in the PR body

[tie]

Uh, I think we have a problem, looks like pkgs/test/stdenv/gcc-stageCompare.nix is broken (as in, it doesn’t check anything because compiler override has no effect) and this PR elevates the issue to a build failure.

At b1a4e83 (master branch) on aarch64-linux:

nix develop --impure --expr 'with (import ./. { });
let ccWrapper = wrapCCWith { cc = stdenv.cc; };
in (gcc-unwrapped.override {
  stdenv = overrideCC stdenv ccWrapper;
}).overrideAttrs (_: { expectedCC = ccWrapper; })' \
-c bash -c 'echo "expectedCC: $expectedCC"
echo "NIX_CC: $NIX_CC"
echo "PATH lookup: $CC -> $(type -P "$CC")"
echo "CC_FOR_BUILD: $CC_FOR_BUILD"'

Note that we use stripped down version of pkgs/test/stdenv/gcc-stageCompare.nix in the --expr argument above.

Output:

expectedCC: /nix/store/ahdbqcc10l1m9l543msjjiglyphabqz3-gcc-wrapper-wrapper-13.2.0
NIX_CC: /nix/store/ahdbqcc10l1m9l543msjjiglyphabqz3-gcc-wrapper-wrapper-13.2.0
PATH lookup: gcc -> /nix/store/4jwr53s7jxhcqcxqyhc83jb0ksdjj1r0-bootstrap-stage3-gcc-wrapper-13.2.0/bin/gcc
CC_FOR_BUILD: gcc

So, stdenv overrides work fine, we get the correct $NIX_CC, but $CC from PATH ends up with bootstrap C compiler. No wonder this test fails with this PR—it was rebuilding gcc with the same stage3 $CC since its introduction in #209870, but now gets a proper absolute path (and then there are some bugs in the code that this test covers and rebuilt gcc does not match stdenv’s gcc).

What would be the right path to move forward with this PR? I’d really like to avoid diving even deeper down this rabbit hole and focus on a single issue at hand 😬

cc @Ericson2314 @trofi @vcunat @SuperSandro2000

[trofi]

That obviously doesn’t work e.g. when build and host platform have the same C compiler name because only the first program from PATH is used

Yeah, I'm not a big fan of absolute paths as they tend to leak everywhere as you have discovered and sometimes are not even supported by the build systems. But if there is no better solution then it might be a reasonable workaround.

Ideally I would say that targetPrefix should differ for different enough targets. Otherwise you have to use other means to convince autoconf that ./configure --build=foo --host=foo is a cross-compilation. It breaks many assumptions in both upstream packages and in nixpkgs downstream. At least in nixpkgs isStatic looks very different from !isStatic as they don't share much of /nix/store (even if both targets are musl). But changing targetPrefix for those is a big (and somewhat controversial) change as well.

[tie]

Ideally I would say that targetPrefix should differ for different enough targets.

Nixpkgs platforms can be configured in so many different ways (microarchitectures, compiler flags, etc) that IMO the only reasonable encoding is a hash of all parameters, which we already have in a form of absolute paths in Nix store 😅
In addition to that, most of the configure scripts out there works fine-ish with absolute paths but I can’t say that for some “non-standard” prefix that is not a target triplet in the basename.

[tie]

[absolute paths] tend to leak everywhere […] and sometimes are not even supported by the build systems

I wouldn’t say it’s that bad. I feel like so far most of the time on this PR I’ve spent debugging issues with things like tests-stdenv-gcc-stageCompare that were silently broken because of the PATH lookup.

Paths leaks are relatively easy to fix by patching source code, removing references from output or setting fixed values at build time. Most build systems are fine with absolute paths too. There are some commits for bespoke configure scripts in this PR, but these are also easy to fix. Again, if possible, I’ll try to merge each change set separately since fixes for absolute paths can be merged before actually using them (see Prerequisites in the description for a list of split PRs).

[Ericson2314]

I think in the past both this and putting the target prefix on native binaries have both been half-attempted.

[AndersonTorres]

I remember some time ago that was cited Exherbo employed something like always prefixing the target on the name of CC.
Would this be useful?

Edit: links
#28374
#28374 (comment)

[tie]

We do prefix target when target ≠ host, but the PATH lookup is still ambiguous in many cases. For example, I recently had to add a workaround for this issue because a package required libtool from darwin.cctools and the latter shadowed $AR = ar from stdenv. Another example is #314920 (comment).

Aside from occasional reference leaks, I think that using absolute paths is better than prefixing toolchain (i.e. using Nix store paths instead of target prefixes). That is because nativeBuildInputs and other flags from mkDerivation arguments take precedence over defaults and we also embed some platform-specific options in the wrapper itself.

Fixing unwanted references is easy in most cases and is a common task for Nixpkgs contributors, but I start questioning my sanity every time the build uses wrong toolchain because of PATH lookup.

FWIW, some packages already set these variables to absolute paths for non-trivial builds, e.g.

• gcc:

  nixpkgs/pkgs/development/libraries/gcc/libgcc/default.nix

  Lines 59 to 75 in aa28fa2

  	# We "shift" the tools over to fake platforms perspective from the previous stage.
  	export AS_FOR_BUILD=${buildPackages.stdenv.cc}/bin/$AS_FOR_BUILD
  	export CC_FOR_BUILD=${buildPackages.stdenv.cc}/bin/$CC_FOR_BUILD
  	export CPP_FOR_BUILD=${buildPackages.stdenv.cc}/bin/$CPP_FOR_BUILD
  	export CXX_FOR_BUILD=${buildPackages.stdenv.cc}/bin/$CXX_FOR_BUILD
  	export LD_FOR_BUILD=${buildPackages.stdenv.cc.bintools}/bin/$LD_FOR_BUILD
  	export AS=$AS_FOR_BUILD
  	export CC=$CC_FOR_BUILD
  	export CPP=$CPP_FOR_BUILD
  	export CXX=$CXX_FOR_BUILD
  	export LD=$LD_FOR_BUILD
  	export AS_FOR_TARGET=${stdenv.cc}/bin/$AS
  	export CC_FOR_TARGET=${stdenv.cc}/bin/$CC
  	export CPP_FOR_TARGET=${stdenv.cc}/bin/$CPP
  	export LD_FOR_TARGET=${stdenv.cc.bintools}/bin/$LD

• fast-downward:

  nixpkgs/pkgs/applications/science/logic/fast-downward/default.nix

  Lines 31 to 33 in aa28fa2

  	# Needed because the package tries to be too smart.
  	export CC="$(which $CC)"
  	export CXX="$(which $CXX)"

[AndersonTorres]

We do prefix target when target ≠ host

Why not to generalize it? Regardless the need of absolute paths, I think we should begin to treat cross-compilation as first class and native compilation as a particular case of it.
And, given that GCC is not smart enough to run like gcc --host=<> --target=<>, the solution is to use the corresponding prefix.
Further, I believe it would eliminate the necessity of always writing code like CC=${stdenv.cc.targetPrefix}cc. (In my latest RG count, it returned 370 files.)

(I hope this is not too off-topic. If so, feel free to ignore it.)

[tie]

Why not to generalize it? Regardless the need of absolute paths, I think we should begin to treat cross-compilation as first class and native compilation as a particular case of it.

Yes, I completely agree with this! Unfortunately, GCC and the existing C ecosystem makes it much harder than it should be, and potentially having multiple matching binaries on the search path doesn’t help that…

On that note, we’ve had several PRs recently that added useful information to the stdenv (e.g. #290081), so perhaps we can also warn about binaries under the same name present in multiple PATH entries 🤔

[tie]

Just had another weird issue while working on #329721 where configure script does not detect $AR for pkgsStatic.xar unless it is set to an absolute path. This appears to be an issue with upstream configure.ac that uses AC_PATH_PROG, but still wanted to document this case in support for absolute paths.

$ echo $AR
aarch64-unknown-linux-musl-ar
$ AR=$AR ./configure $confugureFlags | fgrep 'checking for ar'
checking for ar... no
$ AR=$NIX_CC/bin/$AR ./configure $confugureFlags | fgrep 'checking for ar'
checking for ar... /nix/store/zzaivbdjmarqdcsfia9g5zfs8mgdhj9q-aarch64-unknown-linux-musl-gcc-wrapper-13.3.0/bin/aarch64-unknown-linux-musl-ar

[emilazy]

Relevant to the prefixing discussion: #44583.

[tie]

Note: the PR is not stale and I still think there aren’t any better solutions compared to using absolute paths. I’ll try to squeeze in some time for a rebase in a week or so.