gzip: Don't depend on gzip being in $PATH #81227
Conversation
| mv "$f.new" "$f" | ||
| chmod +x "$f" | ||
| fi | ||
| done |
There was a problem hiding this comment.
Any reason not to use wrapProgram?
There was a problem hiding this comment.
I was trying to minimise dependencies, since gzip seems like the sort of thing that other low-level stuff would want to depend upon. Bash is already in use here, and this can be done in pure bash, so I did it in pure bash.
At your suggestion I tried it with wrapProgram here, and it didn't immediately blow up, so maybe that's fine?
I don't know which is more valuable here: minimal dependencies or the simpler build script.
There was a problem hiding this comment.
What dependencies does wrapProgram bring in? From what I can see it's just a shell?
There was a problem hiding this comment.
$ nix-store -q --references "$(nix-instantiate --expr 'with import <nixpkgs> {}; makeWrapper')"
/nix/store/0s9...-make-wrapper.sh
/nix/store/9kr...-default-builder.sh
/nix/store/hla...-bash-4.4-p23.drv
/nix/store/jxg...-stdenv-linux.drv
/nix/store/l64...-hook.drv
But then stdenv-linux depends upon gzip:
$ nix-store -q --requisites "$(nix-instantiate --expr 'with import <nixpkgs> {}; makeWrapper')" | grep gzip
/nix/store/310...-gzip-1.10.tar.xz.drv
/nix/store/lqm...-gzip-1.10.drv
There was a problem hiding this comment.
Argh, well, I say keep this as-is then
There was a problem hiding this comment.
I think the set of dependencies that is always OK to use even in build scripts for low-level tools is the ones in bootstrap-tools. After all, everything requires tools to build, and Bash is no exception. 🙂
Those tools include, notably, GNU sed and the GNU coreutils:
$ ls $(nix-build -A stdenv.bootstrapTools)/bin
'[' cksum env install nohup rmdir test
ar cmp expand join nproc sdiff timeout
as comm expr kill numfmt sed touch
awk coreutils false ld objdump seq tr
b2sum cp fgrep link od sh true
base32 cpp find ln paste sleep truncate
base64 csplit fmt logname patch sort tsort
basename cut fold ls patchelf split tty
basenc date g++ make pr stat uname
bash dd gawk md5sum printenv stdbuf unexpand
bunzip2 df gcc mkdir printf strip uniq
bzip2 diff grep mkfifo ptx stty unlink
cat diff3 groups mknod pwd sum uptime
chcon dircolors gunzip mktemp ranlib sync wc
chgrp dirname gzip mv readelf tac xargs
chmod du head nice readlink tail xz
chown echo hostid nl realpath tar yes
chroot egrep id nm rm tee
So I think that opens up some ways to keep the script simpler. For example something like (untested):
preFixup = ''
for f in $out/bin/*; do
if head -1 "$f" | grep -qxF '#!/bin/sh'; then
sed -i '2iPATH='$out'/bin:"$PATH"' "$f"
fi
done
'';
That should do exactly the same thing as the all-Bash implementation in the current version of the PR.
(But if I'm mistaken about the meaning of bootstrap-tools, I'd be glad to learn! I tried looking in the docs and didn't succeed in finding much there about this.)
There was a problem hiding this comment.
Cool, thanks! I shortened it to a single sed -i.
Motivation for this change
Direct references to
gunzip&zcatought to work in minimal environments like systemd scripts without having to ensure thangzipis added to PATH.(This only addresses gzip's executables referencing each other -- it does not fix
zgrepexpectinggrepto be in PATH orzdiffexpectingdiffto be in PATH. I think fixing those would need to be done with care to avoid introducing circular dependencies, since gzip is used to extract sources when grep's source is fetched.)Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)