Skip to content

libtool: fix /usr/bin/file impurity #166879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ghost wants to merge 2 commits into from
Closed

libtool: fix /usr/bin/file impurity #166879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4db867f
libtool: 2.4.6 -> 2.4.7
Apr 3, 2022
2068b4c
libtool: fix /usr/bin/file impurity
Apr 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkgs/development/libraries/expat/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
, haskellPackages
, luaPackages
, ocamlPackages
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand All @@ -26,6 +27,8 @@ stdenv.mkDerivation rec {
outputs = [ "out" "dev" ]; # TODO: fix referrers
outputBin = "dev";

nativeBuildInputs = [ replaceUsrBinFile ];

configureFlags = lib.optional stdenv.isFreeBSD "--with-pic";

outputMan = "dev"; # tiny page for a dev tool
Expand Down
3 changes: 2 additions & 1 deletion pkgs/development/libraries/gmp/6.x.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
, cxx ? !stdenv.hostPlatform.useAndroidPrebuilt && !stdenv.hostPlatform.isWasm
, buildPackages
, withStatic ? stdenv.hostPlatform.isStatic
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand Down Expand Up @@ -29,7 +30,7 @@ let self = stdenv.mkDerivation rec {
passthru.static = self.out;

depsBuildBuild = [ buildPackages.stdenv.cc ];
nativeBuildInputs = [ m4 ];
nativeBuildInputs = [ m4 replaceUsrBinFile ];

configureFlags = [
"--with-pic"
Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/isl/generic.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
}:

{ lib, stdenv, fetchurl, gmp
, replaceUsrBinFile
}:

stdenv.mkDerivation {
Expand All @@ -18,6 +19,8 @@ stdenv.mkDerivation {

inherit patches;

nativeBuildInputs = [ replaceUsrBinFile ];

buildInputs = [ gmp ];

inherit configureFlags;
Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/libffi/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

, doCheck ? true # test suite depends on dejagnu which cannot be used during bootstrapping
, dejagnu
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand All @@ -23,6 +24,8 @@ stdenv.mkDerivation rec {

outputs = [ "out" "dev" "man" "info" ];

nativeBuildInputs = [ replaceUsrBinFile ];

configureFlags = [
"--with-gcc-arch=generic" # no detection of -march= or -mtune=
"--enable-pax_emutramp"
Expand Down
15 changes: 13 additions & 2 deletions pkgs/development/libraries/libidn2/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ fetchurl, lib, stdenv, libiconv, libunistring, help2man, texinfo, buildPackages }:
{ fetchurl, lib, stdenv, libiconv, libunistring, help2man, texinfo, buildPackages
, autoreconfHook
}:

# Note: this package is used for bootstrapping fetchurl, and thus
# cannot use fetchpatch! All mutable patches (generated by GitHub or
Expand All @@ -20,9 +22,18 @@ stdenv.mkDerivation rec {

patches = optional stdenv.isDarwin ./fix-error-darwin.patch;

# hack found here: https://github.com/sethrj/spack/commit/93393fe2f510a8ed9ace65534ee80e5433e19943
# works around the fact that autoreconf'ing libidn2 causes it to insist on gtk-doc being present
GTKDOCIZE = "echo";

# The above patch causes the documentation to be regenerated, so the
# documentation tools are required.
nativeBuildInputs = optionals stdenv.isDarwin [ help2man texinfo ];
nativeBuildInputs = [
autoreconfHook buildPackages.pkg-config
] ++ optionals stdenv.isDarwin [
help2man texinfo
];

buildInputs = [ libunistring ] ++ optional stdenv.isDarwin libiconv;
depsBuildBuild = [ buildPackages.stdenv.cc ];

Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/libmpc/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{ lib, stdenv, fetchurl
, gmp, mpfr
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand All @@ -16,6 +17,8 @@ stdenv.mkDerivation rec {
sha256 = "0n846hqfqvmsmim7qdlms0qr86f1hck19p12nq3g3z2x74n3sl0p";
};

nativeBuildInputs = [ replaceUsrBinFile ];

buildInputs = [ gmp mpfr ];

doCheck = true; # not cross;
Expand Down
6 changes: 5 additions & 1 deletion pkgs/development/libraries/libunistring/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ fetchurl, lib, stdenv, libiconv }:
{ fetchurl, lib, stdenv, libiconv
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
# cannot use fetchpatch! All mutable patches (generated by GitHub or
Expand All @@ -14,6 +16,8 @@ stdenv.mkDerivation rec {
sha256 = "sha256-PAGEwOSS18IIzjHSXdHSxY8MPtbLvgMsWySM3a0xhUQ=";
};

nativeBuildInputs = [ replaceUsrBinFile ];

outputs = [ "out" "dev" "info" "doc" ];

propagatedBuildInputs = lib.optional (!stdenv.isLinux) libiconv;
Expand Down
6 changes: 5 additions & 1 deletion pkgs/development/libraries/mpfr/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ lib, stdenv, fetchurl, gmp }:
{ lib, stdenv, fetchurl, gmp
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
# cannot use fetchpatch! All mutable patches (generated by GitHub or
Expand All @@ -19,6 +21,8 @@ stdenv.mkDerivation rec {

outputs = [ "out" "dev" "doc" "info" ];

nativeBuildInputs = [ replaceUsrBinFile ];

# mpfr.h requires gmp.h
propagatedBuildInputs = [ gmp ];

Expand Down
3 changes: 3 additions & 0 deletions pkgs/development/libraries/pcre/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{ lib, stdenv, fetchurl
, pcre, windows ? null
, variant ? null
, replaceUsrBinFile
}:

with lib;
Expand All @@ -18,6 +19,8 @@ stdenv.mkDerivation rec {
sha256 = "sha256-Ta5v3NK7C7bDe1+Xwzwr6VTadDmFNpzdrDVG4yGL/7g=";
};

nativeBuildInputs = [ replaceUsrBinFile ];

outputs = [ "bin" "dev" "out" "doc" "man" ];

# Disable jit on Apple Silicon, https://github.com/zherczeg/sljit/issues/51
Expand Down
32 changes: 0 additions & 32 deletions pkgs/development/tools/misc/libtool/libtool2-macos11.patch
View file
Open in desktop

This file was deleted.

26 changes: 17 additions & 9 deletions pkgs/development/tools/misc/libtool/libtool2.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ lib, stdenv, fetchurl, fetchpatch, autoconf, automake, m4, perl, help2man
, runtimeShell
, file
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand All @@ -8,20 +10,18 @@

stdenv.mkDerivation rec {
pname = "libtool";
version = "2.4.6";
version = "2.4.7";

src = fetchurl {
url = "mirror://gnu/libtool/${pname}-${version}.tar.gz";
sha256 = "1qq61k6lp1fp75xs398yzi6wvbx232l7xbyn3p13cnh27mflvgg3";
sha256 = "sha256-BOlsJATqcMWQxUbrpCAqThJyLGQAFsErmy8c49SB6ag=";
};

outputs = [ "out" "lib" ];

patches = [
# Suport macOS version 11.0
# https://lists.gnu.org/archive/html/libtool-patches/2020-06/msg00001.html
./libtool2-macos11.patch
];
# FILECMD was added in libtool 2.4.7; previous versions hardwired `/usr/bin/file`
# https://lists.gnu.org/archive/html/autotools-announce/2022-03/msg00000.html
FILECMD = "${file}/bin/file";

# Normally we'd use autoreconfHook, but that includes libtoolize.
postPatch = ''
Expand All @@ -34,10 +34,18 @@ stdenv.mkDerivation rec {
automake
autoconf
popd
'' +
# libtool commit da2e352735722917bf0786284411262195a6a3f6 changed
# the shebang from `/bin/sh` (which is a special sandbox exception)
# to `/usr/bin/env sh`, meaning that we now need to patch shebangs
# in libtoolize and ltmain.sh since `dontPatchShebangs` is set:
''
substituteInPlace libtoolize.in --replace '#! /usr/bin/env sh' '#!${runtimeShell}'
substituteInPlace build-aux/ltmain.in --replace '#! /usr/bin/env sh' '#!${runtimeShell}'
'';

nativeBuildInputs = [ perl help2man m4 ] ++ [ autoconf automake ];
propagatedBuildInputs = [ m4 ];
nativeBuildInputs = [ autoconf automake help2man m4 perl ];
propagatedBuildInputs = [ m4 file ];

# Don't fixup "#! /bin/sh" in Libtool, otherwise it will use the
# "fixed" path in generated files!
Expand Down
6 changes: 5 additions & 1 deletion pkgs/development/tools/misc/pkg-config/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{ lib, stdenv, fetchurl, libiconv, vanilla ? false }:
{ lib, stdenv, fetchurl, libiconv, vanilla ? false
, replaceUsrBinFile
}:

with lib;

Expand All @@ -24,6 +26,8 @@ stdenv.mkDerivation rec {
rm -f check/check-requires-private check/check-gtk check/missing
'';

nativeBuildInputs = [ replaceUsrBinFile ];

buildInputs = optional (stdenv.isCygwin || stdenv.isDarwin || stdenv.isSunOS) libiconv;

configureFlags = [ "--with-internal-glib" ]
Expand Down
3 changes: 3 additions & 0 deletions pkgs/tools/compression/xz/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{ lib, stdenv, fetchurl
, enableStatic ? stdenv.hostPlatform.isStatic
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand All @@ -16,6 +17,8 @@ stdenv.mkDerivation rec {
sha256 = "1ps2i8i212n0f4xpq6clp7h13q7m1y8slqvxha9i8d0bj0qgj5si";
};

nativeBuildInputs = [ replaceUsrBinFile ];

outputs = [ "bin" "dev" "out" "man" "doc" ];

configureFlags = lib.optional enableStatic "--disable-shared";
Expand Down
5 changes: 5 additions & 0 deletions pkgs/tools/misc/file/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{ lib, stdenv, fetchurl, file, zlib, libgnurx }:

# Note: this package is used for bootstrapping fetchurl, and thus
# cannot use fetchpatch! All mutable patches (generated by GitHub or
# cgit) that are needed here should be included directly in Nixpkgs as
# files.

stdenv.mkDerivation rec {
pname = "file";
version = "5.41";
Expand Down
3 changes: 2 additions & 1 deletion pkgs/tools/networking/curl/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
, wolfsslSupport ? false, wolfssl ? null
, zlibSupport ? true, zlib ? null
, zstdSupport ? false, zstd ? null
, replaceUsrBinFile
}:

# Note: this package is used for bootstrapping fetchurl, and thus
Expand Down Expand Up @@ -76,7 +77,7 @@ stdenv.mkDerivation rec {

strictDeps = true;

nativeBuildInputs = [ pkg-config perl ];
nativeBuildInputs = [ pkg-config perl replaceUsrBinFile ];

# Zlib and OpenSSL must be propagated because `libcurl.la' contains
# "-lz -lssl", which aren't necessary direct build inputs of
Expand Down
21 changes: 21 additions & 0 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,26 @@ with pkgs;
}
'');

# libtool.m4 prior to version 2.4.7 invokes "/usr/bin/file", and is vendored into many ./configure scripts
replaceUsrBinFile =
Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@sternenseemann sternenseemann Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

@ghost ghost Apr 12, 2022 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Copy link
Author

@ghost ghost Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could convert this to a setupHook

Thanks I will try that.

Copy link
Author

@ghost ghost Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@ghost ghost Apr 12, 2022 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

makeSetupHook {}
(writeScript "fix-path-to-user-bin-file.sh" ''
postUnpackHooks+=(_fixPathToUserBinFile)
_fixPathToUserBinFile() {
if [ ! -f $sourceRoot/configure ] || \
! ( grep -q 'Which release of libtool.m4 was used' $sourceRoot/configure ) || \
! ( egrep -q "^macro_version='([01].|2.[0-3]|2.4.[0-6])" $sourceRoot/configure ) || \
! ( grep -q /usr/bin/file $sourceRoot/configure )
then
echo "This package does not have a ./configure script which vendors a libtool.m4<=2.4.6."
echo "Therefore, it does not need the replaceUsrBinFile workaround. Please remove replaceUsrBinFile"
echo "from the nativeBuildInputs."
exit -1
fi
substituteInPlace $sourceRoot/configure --replace /usr/bin/file ${buildPackages.file}/bin/file
}
'');

addOpenGLRunpath = callPackage ../build-support/add-opengl-runpath { };

quickemu = callPackage ../development/quickemu { };
Expand Down Expand Up @@ -5489,6 +5509,7 @@ with pkgs;

file = callPackage ../tools/misc/file {
inherit (windows) libgnurx;
fetchurl = stdenv.fetchurlBoot;
};

filegive = callPackage ../tools/networking/filegive { };
Expand Down