libtool: fix /usr/bin/file impurity

[ghost]

Problem

In libtool<=2.4.6 the libtool.m4 script assumes that file is available, and can be found at /usr/bin/file (this path is hardwired). This fails for sandboxed builds. This script is vendored into the ./configure scripts of an enormous number of packages.

The use of /usr/bin/file in ./configure scripts affects sandboxed cross-builds to a x86_64-linux hosts as well as all sandboxed builds (cross and native) to x86_64-freebsd, *-hpux, *-irix, mips64*-linux, powerpc*-linux, s390x-linux, s390x-tpf, sparc-linux, and *-solaris.

(Immediate) Motivation

This is currently preventing the generation of Hydra-built bootstrap-files for mips64el-linux-gnuabi64, and will affect many of the other new platforms we try to bootstrap on.

The build of the pcre package (and a few others) emits these lines when its configure script is run:

  ./configure: line 9843: /usr/bin/file: No such file or directory
  ./configure: line 9851: /usr/bin/file: No such file or directory
  ./configure: line 9859: /usr/bin/file: No such file or directory
  ...
  checking whether to build shared libraries... no

The build does not fail until much later, when make-bootstrap-files.nix tries to make a copy libpcre.so, which was never built.

Solution

An earlier revision of this PR tried to use fancy stdenv adapters to fix this problem without causing mass rebuilds. SuperSandro2000's suggestion to simply apply the fix everywhere seems like it will be more maintainable.

Currently this PR adds a new top-level expression replaceUsrBinFile which will check to see if the package being built has a vendored libtool.m4 older than 2.4.7. If it does, then it will substituteInPlace ./configure --replace /usr/bin/file ${buildPackages.file}/bin/file. If it does not find this, it will abort the build with an error message. This ensures that maintainers will remove the replaceUsrBinFile as soon as upstream regenerates their autoconf with an unaffected libtool >= 2.4.7.

This commit also adds replaceUsrBinFile to nativeBuildInputs of a large number (but not all) of the packages that need it.

Things done

• Built on platform(s)
  • x86_64-linux
  • mips64el-linux cross from x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• 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/)
• 22.05 Release Notes (or backporting 21.11 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[NickCao]

Can't this be a part of autoreconfHook?

[ghost]

Can't this be a part of autoreconfHook?

Unfortunately autoreconfHook isn't called by default. So a stdenv adapter would still be needed (to add autoreconfHook as an extra dependency).

Calling autoreconfHook instead of fixPathToUsrBinFileInConfigureScript could be done in theory, but in practice autoreconfHook breaks a lot of packages -- it uses the latest autotools instead of whatever version was used when the package was created. The ./configure script is basically a vendored dependency. The long-run, decade-scale fix is to upgrade to libtool 2.4.7 and wait for all the packages in nixpkgs to run autoreconf upstream (and fix the breakage that occurs).

The breakage caused by blindly autoreconfHook'ing things is why I prefer this PR over #166722. I tried getting pkgsCross.mips64el-linux-gnuabi64.nix_2_4 to build with that one and it just turned into a huge mess of having to bodge one package after another. With this PR I just (minutes ago) finished cross-compiling nix_2_4 and everything else that runs on my routers, quite a lot of software, and the only additional patch needed is for libidn2 (since it decides to autoreconf itself when it detects changes to configure).

[Mindavi]

So this is a temporary solution until libtool is upgraded? At least it's nice to have this solved in the short term.

[ghost]

So this is a temporary solution until libtool is upgraded?

No.

Read the section titled "implementation". There are two changes, only one of them can be removed when libtool is upgraded.

The one that can be removed is clearly marked with a comment mentioning this, and is found in the same file where the version will be upgraded, so it isn't likely to be overlooked.

The rest needs to stay due to vendoring in ./configure scripts. We can't remove it until every single upstream package re-runs autoreconf, which will basically take decades. This is needed for the exact same reason that the standard builder has to have fixLibtool() instead of nixpkgs simply patching the libtool source code in libtool2.nix.

[ghost]

Pushed three additional patches. One just expands a comment with more detail, another factors out needsLibtoolUsrBinFileFix so it can be referenced by package expressions, and the third uses it to prevent the curl expression from breaking the ./configure script on platforms that need file to detect library flags.

As a side effect of the third patch, file is now part of the "stuff needed to bootstrap fetchurl" clique. I've added the boilerplate comment to declare this fact.

Perhaps we should consider including file in bootstrap-tools later on.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixlang-how-do-you-find-all-uses-of-a-declaration/18369/14

[Ericson2314]

I was talking to @alyssais about this. She pointed out https://discourse.nixos.org/t/rfc-libtool-updates-and-improved-nix-nixos-support/15660 this to me too, which is very exciting.

I would like to move closer to autoreconfPhase by default, for all platforms.

[ghost]

(Edit: I think the person who started this discourse thread is the same person who committed the bugfix that was released in libtool-2.4.7)

I was talking to @alyssais about this. She pointed out https://discourse.nixos.org/t/rfc-libtool-updates-and-improved-nix-nixos-support/15660 this to me too, which is very exciting.

Yes, that's certainly good news! But just like the bugfix in libtool-2.4.7, it won't affect the hundreds (or thousands?) of packages that have an old libtool.m4 vendored into their ./configure. This brings us to your other point:

I would like to move closer to autoreconfPhase by default, for all platforms.

I would like that as well, but the amount of breakage it will cause is mind-boggling. So many packages fail to build when you add autoreconfHook, it's depressing -- unless there is some secret magic trick I'm unaware of.

Making autoreconfPhase a requirement for newly-added packages is probably a good idea.

I hope this PR does not end up blocked on some hoped-for future autoreconfPhase-by-default. This PR is the one and only thing preventing Hydra from building a mips64el bootstrap-files whose hash can be committed to pkgs/stdenv/linux/bootstrap-files/ (proof from ofborg). Making that wait for all the autoreconfHook bugs in all of nixpkgs to get fixed would be pretty soul-crushing. I don't mind waiting until after the 22.05 release though; that is clearly more urgent right now.

[ghost]

(I'm currently tweaking this to reduce the amount of rebuilds it causes; mainly limiting fixPathToUsrBinFileInConfigureScript on hostPlatform.isx86_64 to cross-compiles-only)

done

[ghost]

2. fix libtool.m4 in the libtool package (this is fixed upstream in libtool 2.4.7)

#167071 is a PR to upgrade to libtool 2.4.7. Unfortunately it causes downstream build failures with --option sandbox true. I have a mass-rebuild running with --option sandbox false and it's going very well, so the sandboxing issue is probably the only obstacle. Fixed.

If #167071 merges first then the changes to pkgs/development/tools/misc/libtool/libtool2.nix in this PR can be dropped. However the rest of this PR remains necessary regardless of the libtool upgrade.

[ghost]

So many packages fail to build when you add autoreconfHook, it's depressing -- unless there is some secret magic trick I'm unaware of.

Here's a great example: #163401 (comment)

[SuperSandro2000]

Not a big fan that we add another package to bootstrapping.

The names you have chosen are horrible long. I don't think anyone can remember them. Maybe we can passthru them for autoconf and autoreconfHook so that you can easily access them.

[SuperSandro2000]

Suggestion: How about we always fix this to not needing to keep track of this and in projects? Because I don't think that anytime soon this is fixed everywhere.

[ghost]

@SuperSandro2000, thank you so much for taking the time to review this PR (and my others too, but this one especially). I really appreciate it; this is the PR I was afraid would get stuck in limbo forever.

Suggestion: How about we always fix this to not needing to keep track of this and in projects?

I would prefer this too. I was trying to minimize rebuilds, but in hindsight maintainability matters more.

If the fix is applied on all platforms then this definitely needs to go into staging. The good news: if this goes into staging it can build off of #167071, which will make this PR much shorter.

I will incorporate your other feedback, switch branches, and remove the parts that are no longer necessary after #167071.

Again, thank you so much for wading into this complicated issue and taking the time to understand what's going on here!

[ghost]

Converted to draft while I rework this.

[ghost]

Useful for future reference:

git fetch https://github.com/a-m-joseph/nixpkgs fail-if-unpatched-configure-with-buggy-vendored-libtool
git cherry-pick 20733ebed57

This will add code to pkgs/stdenv/generic/setup.sh which will abort a build if it discovers an unpatched ./configure which vendors the buggy (pre-2.4.7) libtool.m4.

[ghost]

Okay, I think this should work. Running some rebuilds overnight to be sure.

The new approach applies the substituteInPlace regardless of platform.

This let me simplify the top-level addition, which is now called just replaceUsrBinFile. It now checks to see if it is necessary, and fails the build if it is applied unnecessarily. This will ensure that this new extra dependency gets removed when we upgrade packages to versions that no longer need the hack.

[SuperSandro2000]

Maybe we could convert this to a setupHook in which case we would never need to call it manually? We would need to remove the exit part but it would make it a lot less work to maintain.

https://github.com/NixOS/nixpkgs/pull/166486/files#diff-7510b47653eab58ead900c36ffc4881682b881b87c0e7b7016d1f3947dee495cR50

[sternenseemann]

I think it would be logical to make this part of fixLibtool in pkgs/stdenv/generic/setup.sh.

[ghost]

I would love to do that but I can't figure out how to thread the reference to ${buildPackages.file} down into setup.sh. Also even if I could do that, it would create a circular dependency in many cases, wouldn't it? If you have ideas on how to do this please give me a hint and I will investigate this. It really belongs in fixLibtool() if it is possible to put it there.

By the way, speaking of crazy circular dependencies, the ./configure script that ships with file-5.41.tar.gz tries to call /usr/bin/file! The package ships with a circular dependency! Talk about crazy...

[ghost]

Maybe we could convert this to a setupHook

Thanks I will try that.

[ghost]

the ./configure script that ships with file-5.41.tar.gz tries to call /usr/bin/file! The package ships with a circular dependency!

I would love to do that but I can't figure out how to thread the reference to ${buildPackages.file} down into setup.sh. Also even if I could do that, it would create a circular dependency in many cases, wouldn't it?

I should add that these are two arguments for considering putting file in the bootstrap-tools. I know that adding more stuff to the bootstrap-tools is never an easy decision, but when you have packages that depend on themselves (like a C compiler) and which 99% of the ecosystem has a vendored dependency upon, well... that's sort of what the bootstrap-files is for.

Then we could have fixLibtool() replace /usr/bin/file with file, since the bootstrap-tools bin directory is always in the $PATH. So there wouldn't be any need to thread the package reference down into there through the nixlang code.

It's ugly, but so is the problem that it fixes.

[ghost]

See #168413 for a much, much simpler solution that simply (a) patches configure scripts unconditionally and (b) includes file in stdenv's common-path.nix.

I'm running a rebuild-the-world on that PR, it'll take all day.

I would appreciate insight on whether or not that approach is considered mergeable if it works. If so, I would much prefer it. It's basically a two-line change plus a workaround for libidn2 trying to autoreconf itself.

[ghost]

Closed in favor of #168413, which is much much simpler and more maintainable -- at the cost of having to rebuild everything.