fixLibtool(): replace /usr/bin/file in ./configure, add file to common-path.nix · Pull Request #168413 · NixOS/nixpkgs

ghost · 2022-04-12T21:54:36Z

This is a much cleaner replacement for #166879, but it will cause everything to rebuild.

Description of changes

libtool's libtool.m4 script assumes that file is available, and can be found at /usr/bin/file (this path is hardwired). Furthermore, this script is vendored into the ./configure scripts of an enormous number of packages. Without this PR, you will frequently see errors like this during the configurePhase when the sandbox is enabled:

  ./configure: line 9595: /usr/bin/file: command not found

Due mostly to luck, this error does not affect native compiles on nixpkgs' two most popular platforms, x86_64-linux and aarch64-linux. However it will cause incorrect linker flag detection and a failure to generate shared libraries for sandboxed cross-builds to a x86_64-linux host as well as any sandboxed build (cross or native) for the following hosts: x86_64-freebsd, *-hpux, *-irix, mips64*-linux, powerpc*-linux, s390x-linux, s390x-tpf, sparc-linux, and *-solaris.

This PR fixes the problem by adding an extra line to the if-block which calls fixLibtool() in pkgs/stdenv/generic/setup.sh. This extra line will scan the unpacked source code for executable files named configure which contain the following text:

GNU Libtool is free software; you can redistribute it and/or modify

This text is taken to be an indicator of a vendored libtool.m4. When it is found, the configure script containing it is subjected to sed -i s_/usr/bin/file_file_ which replaces all occurrences of /usr/bin/file with file, and the mtime of the file is preserved across this replacement.

Additionally, the file package is now considered to be part of stdenv. It has been added to common-path.nix so that the file binary will be found in the $PATH of every build, except for the bootstrap-tools and the first few stages of stdenv boostrapping.

Verified no regressions under:

  nix-build --arg pkgs 'import ./. {}' ./lib/tests/release.nix

This commit allows the following commands to complete, which should enable Hydra to produce bootstrap-files for mips64el:

  nix-build \
    --option sandbox true \
    --option sandbox-fallback false \
    pkgs/top-level/release-cross.nix \
    -A bootstrapTools.mips64el-linux-gnuabi64.build

  nix-build \
    --option sandbox true \
    --option sandbox-fallback false \
    . \
    -A pkgsCross.mips64el-linux-gnuabi64.nix_2_4

CC: @sternenseemann, @SuperSandro2000

Things done

Built on platform(s)
- x86_64-linux
- aarch64-linux
- x86_64-darwin
- aarch64-darwin
- powerpc64le-linux
- mips64el-linux
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

[nix-shell:~/.cache/nixpkgs-review/rev-5952698e53b489a2e92e5b87b765a1577678ed90]$ cat report.json
{
    "blacklisted": [],
    "broken": [],
    "built": [],
    "failed": [],
    "non-existant": [],
    "pr": null,
    "system": "x86_64-linux",
    "tests": []
}
[nix-shell:~/.cache/nixpkgs-review/rev-5952698e53b489a2e92e5b87b765a1577678ed90]$ cat report.md
Result of `nixpkgs-review` run on x86_64-linux [1](https://github.com/Mic92/nixpkgs-review)

Tested basic functionality of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

pkgs/stdenv/common-path.nix

ghost · 2022-05-29T19:32:32Z

Mindavi merged commit f174277 into NixOS:staging yesterday

Thank you thank you thank you!!!!

We should have a bootstrap for mips64el very soon now. Thank you again!

As requested here: #168413 (review)

alex-ameen · 2022-06-01T04:16:28Z

Shit is it writing `FILECMD=/definitely/evil/bin/file` as a hard coded path into people's source tarballs? If it is let me know so I can patch. Or is the issue that `libtoolize` now requires the runtime `coreutils` store path to match the one used to build `libtool`?

…

On Sun, May 29, 2022, 2:34 PM Adam Joseph ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkgs/stdenv/common-path.nix <#168413 (comment)>: > @@ -12,4 +12,12 @@ pkgs.bash pkgs.patch pkgs.xz.bin + + # The `file` command is added here because an enormous number of + # packages have a vendored dependency upon `file` in their + # `./configure` script, due to libtool<=2.4.6, or due to + # libtool>=2.4.7 in which the package author decided to set FILECMD + # when running libtoolize. In fact, file-5.4.6 *depends on itself* + # and tries to invoke `file` from its own ./configure script. + pkgs.file a comment like https://github.com/NixOS/nixpkgs/blob/89a3d054790bbcf6c9d73394cd1c541466c5a7c8/pkgs/tools/compression/gzip/default.nix#L7-L10 should be added to file. #175343 <#175343> — Reply to this email directly, view it on GitHub <#168413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3ORYTVY5MMOEL35YENJOLVMPBFPANCNFSM5TIUO2GQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

alex-ameen · 2022-06-01T04:20:31Z

Oh jesus, the bootstrap paradox of `file` needing `file` is fun. We'll resolve this, but honestly I might rewrite `magiccmd` ( the reason `file` is needed ) to use `read` to check file magic to avoid this whole mess: ``` # Test if a file is an AR archive. function isAR() { local fn="$1" local fd local magic exec {fd}<"$fn" # In Zsh `read -k NUM' behaves like Bash's `read -n NUM'. if test "${ZSH_VERSION+set}" = set; then read -r -k 7 -u "$fd" magic else read -r -n 7 -u "$fd" magic fi read -r -n 7 -u "$fd" magic exec {fd}<&- # First seven characters should be "!<arch>" if test "$magic" = $'\041\074arch\076'; then return 0; else return 1; fi } ```

sternenseemann · 2022-06-01T08:04:33Z

If it is let me know so I can patch.

AIUI it is already patched, but we need a workaround for tarballs generated with older libtool versions.

ghost · 2022-06-02T02:36:57Z

@aakropotkin, thank you again so much for fixing the upstream issue in libtool. Let's be honest, this PR (#168413) is basically a hack. Thanks to your work on libtool we will be able to revert this hack ~3-4 years from now instead of infinitely-many years from now.

Shit is it writing FILECMD=/definitely/evil/bin/file as a hard coded path into people's source tarballs?

I believe you are referring to my comment upthread about other failure modes, where I wrote:

If upstream authors set FILECMD when they run the autotools, whatever they set it to will get hardcoded into the vendored source tarball.

I swear that I experienced FILECMD=/foobarbaz libtoolize; aclocal; autoconf leaking "/foobarbaz"es into ./configure, but I am unable to reproduce this, so I must have been imagining it.

However there are still some hardcoded /usr/bin/files in libtool itself:

nix@moore:/nix/nixpkgs$ nix-build . -A libtool
/nix/store/cykvcas3pyipzmwvnskc0mb1x43nmdfs-libtool-2.4.7
nix@moore:/nix/nixpkgs$ rg /usr/bin/file /nix/store/cykvcas3pyipzmwvnskc0mb1x43nmdfs-libtool-2.4.7
/nix/store/cykvcas3pyipzmwvnskc0mb1x43nmdfs-libtool-2.4.7/share/libtool/build-aux/ltmain.sh
8543:               if /usr/bin/file -L $add 2> /dev/null |

/nix/store/cykvcas3pyipzmwvnskc0mb1x43nmdfs-libtool-2.4.7/bin/libtool
9064:               if /usr/bin/file -L $add 2> /dev/null |

Perhaps those don't matter.

Oh jesus, the bootstrap paradox of file needing file is fun.

:)

We'll resolve this, but honestly I might rewrite magiccmd ( the reason file is needed ) to use read to check file magic to avoid this whole mess:

That would be wonderful. You'll need a bit more than the four "magic" bytes, but not much. Reading the first nine bytes will give you the CPU, endianness, and ABI, which should be enough:

https://en.wikipedia.org/wiki/Executable_and_Linkable_Format#File_header

mehmooda · 2022-06-06T11:59:48Z

If file is now in stdenv does this mean that it can be removed from mingw stdenv/cross?

SuperSandro2000 · 2022-06-06T12:24:13Z

Worth a try

ghost · 2022-06-06T18:29:47Z

If file is now in stdenv does this mean that it can be removed from mingw stdenv/cross?

Yes. Thanks for noticing this.

I will take the blame if removing it breaks the world: #176597

Side question: are there any tools to make it easier to trace the provenance of a line of code using git blame when it has been moved around by refactoring commits? Sort of like git rebase --continue where you're alternating between the tool doing as much as it can before stopping for human assistance. Just using git grep doesn't work too well because the search string might change a few times during the search as refactoring alter the identifier names change (due to refactoring).

Artturin · 2022-06-06T19:35:46Z

i have this alias betterblame='git log -p -M --follow --stat --'

trofi · 2022-06-14T20:44:47Z

Bisect claims that 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix broke abuild in staging and staging-next:

$ nix build -f. abuild -L
abuild> unpacking sources
abuild> unpacking source archive /nix/store/yipn1z720ag27brbnxak7nhv56qkhrl5-source
abuild> source root is source
abuild> find: unrecognized: -printf
abuild> BusyBox v1.35.0 () multi-call binary.
abuild> Usage: find [-HL] [PATH]... [OPTIONS] [ACTIONS]
abuild> Search for files and perform actions on them.
abuild> First failed action stops processing of current file.
abuild> Defaults: PATH is current directory, action is '-print'
abuild>         -L,-follow      Follow symlinks
abuild>         -H              ...on command line only
abuild>         -xdev           Don't descend directories on other filesystems
abuild>         -maxdepth N     Descend at most N levels. -maxdepth 0 applies
abuild>                         actions to command line arguments only
abuild>         -mindepth N     Don't act on first N levels
abuild>         -depth          Act on directory *after* traversing it
...

ghost · 2022-06-14T20:46:53Z

Bisect claims that 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix broke abuild in staging and staging-next:

I am investigating.

Don't hesitate to revert this if it's blocking other people's progress.

You can expect a more detailed response from me within the next 12 hours (likely much sooner than that).

ghost · 2022-06-14T21:21:38Z

Bisect claims that 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix broke abuild in staging and staging-next:

Fixed: #177682

trofi · 2022-06-22T11:12:23Z

Bisect claims epson-escpr2 was broken by 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix:

$ nix build -f. -L epson-escpr2
...
epson-inkjet-printer-escpr2> patching sources
epson-inkjet-printer-escpr2> applying patch /nix/store/ljqnzvnhv6v0c4pvwry3617fp8j7q3bv-cups-filter-ppd-dirs.patch
epson-inkjet-printer-escpr2> patching file configure
epson-inkjet-printer-escpr2> configuring
epson-inkjet-printer-escpr2> fixing libtool script ./ltmain.sh
epson-inkjet-printer-escpr2> mktemp: (null): Invalid argument

ghost · 2022-06-22T19:41:12Z

Bisect claims epson-escpr2 was broken by 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix:

Ugh, this is another case of a package fooling setup.sh into using the busybox version of a command. In this case, mktemp:

$ busybox mktemp --help
BusyBox v1.35.0 () multi-call binary.

Usage: mktemp [-dt] [-p DIR] [TEMPLATE]

Create a temporary file with name based on TEMPLATE and print its name.
TEMPLATE must end with XXXXXX (e.g. [/dir/]nameXXXXXX).

Notice the special requirement on the number of Xes.

ghost · 2022-06-22T19:45:16Z

Bisect claims epson-escpr2 was broken by 97c4382 fixLibtool(): patch ./configure, add file to common-path.nix:

Fix: #178626

alex-ameen · 2022-06-22T19:49:59Z

This thread has grown quite a bit, and I'll admit I've fallen behind on reading it.

If there's any issues that could or should be resolved upstream in libtool please tag me so I will see them.

I'm sure there's a mix of "old builds broke because of issues in their usage" vs "the libtool maintainer used a GNU specific flag in a script and blew up ". I did what I could in terms of testing before the release, but I'll be transparent and admit that I was only able to run NixOS, Debian, Ubuntu, OSX ( aarch64 and x86_64 ), Windows ( pretty unsuccessfully ) and RHEL. And for all of those I just ran with the default tools on a box. Other than that I relied on shellcheck and the GNU shell portability manual, but I would be unsurprised to hear that I used a flag/utility that isn't available for certain systems.

ghost · 2022-06-23T03:52:08Z

This thread has grown quite a bit, and I'll admit I've fallen behind on reading it.

If there's any issues that could or should be resolved upstream in libtool please tag me so I will see them.

You can unsubscribe if you want. I will ping @aakropotkin if anything comes up that needs upstream action, but I doubt that it will.

Thank you again very much for adding FILECMD in libtool 2.4.7!

ghost · 2022-06-23T03:55:33Z

Fix: #178626

Actually epson-escpr2 needs both that and #177789.

alex-ameen · 2022-10-11T08:02:22Z

I wrote it so that you can either autoreconf or `export FILECMD="$( command command -v file:)"` IIRC. Let me know if that's not the case.

…

On Thu, May 26, 2022, 1:37 AM Rick van Schijndel ***@***.***> wrote: I'm okay with the workaround for now, autoreconf-ing is probably a pain to do for all packages. In general it's good to have this to improve exotic platform support and to have better cross-compilation support from more rare platforms. — Reply to this email directly, view it on GitHub <#168413 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3ORYQ3UMM2UNKVBL3QS5DVL4L2VANCNFSM5TIUO2GQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

ghost mentioned this pull request Apr 12, 2022

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

Closed

14 tasks

ghost changed the base branch from master to staging April 12, 2022 21:56

ghost changed the title ~~fixLibTool: replace /usr/bin/file in ./configure, add file to common-path.nix~~ fixLibTool(): replace /usr/bin/file in ./configure, add file to common-path.nix Apr 12, 2022

ghost changed the title ~~fixLibTool(): replace /usr/bin/file in ./configure, add file to common-path.nix~~ fixLibtool(): replace /usr/bin/file in ./configure, add file to common-path.nix Apr 12, 2022

ofborg bot added the 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. label Apr 13, 2022

Mindavi merged commit f174277 into NixOS:staging May 28, 2022

Artturin reviewed May 28, 2022

View reviewed changes

pkgs/stdenv/common-path.nix Show resolved Hide resolved

ghost mentioned this pull request May 29, 2022

pkgs/tools/misc/file: add cannot-use-fetchpatch warning #175343

Merged

13 tasks

ghost deleted the libtool-purity-fix-put-file-in-bootstrap branch May 29, 2022 19:34

sternenseemann pushed a commit that referenced this pull request May 29, 2022

pkgs/tools/misc/file: add cannot-use-fetchpatch warning

860c2a1

As requested here: #168413 (review)

ghost mentioned this pull request Jun 6, 2022

stdenv/cross: remove now-redundant file nativeBuildInput on mingw #176597

Merged

5 tasks

ghost mentioned this pull request Jun 14, 2022

abuild: put findutils in nativeBuildInputs explicitly #177682

Closed

13 tasks

ghost mentioned this pull request Jun 14, 2022

setup.sh: verify that find is from findutils #177693

Closed

13 tasks

ghost mentioned this pull request Jun 22, 2022

setup.sh does not work with ${busybox}/bin/mktemp instead of ${stdenv}/bin/mktemp #178625

Closed

Artturin mentioned this pull request Jan 29, 2024

List of @amjoseph-nixpkgs Pull requests before account removal #284796

Open

Uh oh!

Conversation

ghost commented Apr 12, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

Uh oh!

ghost commented May 29, 2022

Uh oh!

alex-ameen commented Jun 1, 2022 via email

Uh oh!

alex-ameen commented Jun 1, 2022 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sternenseemann commented Jun 1, 2022

Uh oh!

ghost commented Jun 2, 2022

Uh oh!

mehmooda commented Jun 6, 2022

Uh oh!

SuperSandro2000 commented Jun 6, 2022

Uh oh!

ghost commented Jun 6, 2022

Uh oh!

Artturin commented Jun 6, 2022

Uh oh!

trofi commented Jun 14, 2022

Uh oh!

ghost commented Jun 14, 2022

Uh oh!

ghost commented Jun 14, 2022

Uh oh!

trofi commented Jun 22, 2022

Uh oh!

ghost commented Jun 22, 2022

Uh oh!

ghost commented Jun 22, 2022

Uh oh!

alex-ameen commented Jun 22, 2022

Uh oh!

ghost commented Jun 23, 2022

Uh oh!

ghost commented Jun 23, 2022

Uh oh!

alex-ameen commented Oct 11, 2022 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ghost commented Apr 12, 2022 •

edited by ghost

Loading

alex-ameen commented Jun 1, 2022 via email •

edited

Loading