From dda702fd16bafac1ecb081590f6b00eea8a1136b Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Mon, 10 Feb 2020 16:13:46 -0500 Subject: [PATCH] bash scripts: use /usr/bin/env for bash shebangs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not all systems / distros have a `/bin/bash`, and these scripts are more difficult to run at development time. For example, my system is NixOS which doesn't have a /bin/bash. This is not a problem for NixOS building ZFS as a package: the build environment automatically replaces these shebangs with corrected paths. The problem is much more annoying at development time: either the scripts don't run, or I correct them for my local machine and deal with a perpetually dirty work tree. Before committing this patch I confirmed there are existing scripts which use `/usr/bin/env` to locate bash, so I am thinking this is a safe transformation. There are a handful of other shebangs in this repository which don't work on my system. This patch is useful on its own specifically for `commitcheck.sh`, otherwise I can't validate my commits before submission. Here are the remaining shebangs which NixOS systems won't have: 1274 #!/bin/ksh -p 91 #!/bin/ksh 89 #! /bin/ksh -p 2 #!/bin/sed -f 1 #!/usr/bin/perl -w 1 #!/usr/bin/ksh 1 #!/bin/nawk -f plus this which will create an invalid shebang in `tests/zfs-tests/tests/functional/mv_files/mv_files_common.kshlib`: echo "#!/bin/ksh" > $TEST_BASE_DIR/exitsZero.ksh I chose to leave those alone for now, and gauge the interest in this much smaller patch first. The fixes for these are easy enough by simply using `/usr/bin/env ksh`: 91 #!/bin/ksh 1 #!/usr/bin/ksh The fix for the other set is much trickier. Quoting the GNU coreutils manual: Most operating systems (e.g. GNU/Linux, BSDs) treat all text after the first space as a single argument. When using env in a script it is thus not possible to specify multiple arguments. and not all `env`'s support arguments. Mine (GNU Coreutils 8.31) does, though this feature is new since April 2018, GNU Coreutils 8.30: https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=668306ed86c8c79b0af0db8b9c882654ebb66db2 and worse, requires the -S argument: -S, --split-string=S process and split S into separate arguments; used to pass multiple arguments on shebang lines Example: $ seq 1 2 | $(nix-build '' -A coreutils)/bin/env "sort -nr" /nix/[...]-coreutils-8.31/bin/env: ‘sort -nr’: No such file or directory /nix/[...]-coreutils-8.31/bin/env: use -[v]S to pass options in shebang lines $ seq 1 2 | $(nix-build '' -A coreutils)/bin/env "-S sort -nr" 2 1 GNU Coreutils says FreeBSD's `env` does, though I wonder if FreeBSD's would be unhappy with the `-S`: https://www.gnu.org/software/coreutils/manual/html_node/env-invocation.html#env-invocation BusyBox v1.30.1 does not, and does not have a `-S`-like option: $ seq 1 2 | $(nix-build '' -A busybox)/bin/env "sort -nr" env: can't execute 'sort -nr': No such file or directory Toybox 0.8.1 also does not, and also does not have a `-S` option: $ seq 1 2 | $(nix-build '' -A toybox)/bin/env "sort -nr" env: exec sort -nr: No such file or directory --- At any rate, if this patch merges and the remaining ~1,500 are updated, the much larger patch should probably include a checkstyle-like test asserting all new shebangs use `/usr/bin/env`. I also don't mind dealing with NixOS weirdness if the project would prefer that. Reviewed-by: Brian Behlendorf Reviewed-by: Ryan Moeller Signed-off-by: Graham Christensen Closes #9893 --- cmd/zgenhostid/zgenhostid | 2 +- contrib/dracut/02zfsexpandknowledge/module-setup.sh.in | 2 +- contrib/dracut/90zfs/module-setup.sh.in | 2 +- contrib/dracut/90zfs/zfs-generator.sh.in | 2 +- copy-builtin | 2 +- scripts/commitcheck.sh | 2 +- scripts/kmodtool | 2 +- scripts/zimport.sh | 2 +- tests/zfs-tests/include/commands.cfg | 1 + tests/zfs-tests/tests/perf/scripts/prefetch_io.sh | 2 +- 10 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/zgenhostid/zgenhostid b/cmd/zgenhostid/zgenhostid index db690eca32..8b468740c7 100755 --- a/cmd/zgenhostid/zgenhostid +++ b/cmd/zgenhostid/zgenhostid @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Emulate genhostid(1) available on RHEL/CENTOS, for use on distros # which do not provide that utility. diff --git a/contrib/dracut/02zfsexpandknowledge/module-setup.sh.in b/contrib/dracut/02zfsexpandknowledge/module-setup.sh.in index c22141f00f..a2a3ef0370 100755 --- a/contrib/dracut/02zfsexpandknowledge/module-setup.sh.in +++ b/contrib/dracut/02zfsexpandknowledge/module-setup.sh.in @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash get_devtype() { local typ diff --git a/contrib/dracut/90zfs/module-setup.sh.in b/contrib/dracut/90zfs/module-setup.sh.in index 4efc4b0186..e94f9f2dc9 100755 --- a/contrib/dracut/90zfs/module-setup.sh.in +++ b/contrib/dracut/90zfs/module-setup.sh.in @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash check() { # We depend on udev-rules being loaded diff --git a/contrib/dracut/90zfs/zfs-generator.sh.in b/contrib/dracut/90zfs/zfs-generator.sh.in index 0b8a8aaca7..120b9ecf95 100755 --- a/contrib/dracut/90zfs/zfs-generator.sh.in +++ b/contrib/dracut/90zfs/zfs-generator.sh.in @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash echo "zfs-generator: starting" >> /dev/kmsg diff --git a/copy-builtin b/copy-builtin index 700e6f4f07..81fd175656 100755 --- a/copy-builtin +++ b/copy-builtin @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash set -e diff --git a/scripts/commitcheck.sh b/scripts/commitcheck.sh index 2954b0fd72..c7515c23e1 100755 --- a/scripts/commitcheck.sh +++ b/scripts/commitcheck.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash REF="HEAD" diff --git a/scripts/kmodtool b/scripts/kmodtool index b928c92862..240cde3106 100755 --- a/scripts/kmodtool +++ b/scripts/kmodtool @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # kmodtool - Helper script for building kernel module RPMs # Copyright (c) 2003-2012 Ville Skyttä , diff --git a/scripts/zimport.sh b/scripts/zimport.sh index d7e82fe9f0..304ab7623d 100755 --- a/scripts/zimport.sh +++ b/scripts/zimport.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # # Verify that an assortment of known good reference pools can be imported # using different versions of the ZoL code. diff --git a/tests/zfs-tests/include/commands.cfg b/tests/zfs-tests/include/commands.cfg index 79f05e54a5..77562700c8 100644 --- a/tests/zfs-tests/include/commands.cfg +++ b/tests/zfs-tests/include/commands.cfg @@ -131,6 +131,7 @@ export SYSTEM_FILES_FREEBSD='chflags uncompress' export SYSTEM_FILES_LINUX='attr + bash blkid blockdev chattr diff --git a/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh b/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh index 75bf08f4b6..b8d8ae8859 100755 --- a/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh +++ b/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # # This file and its contents are supplied under the terms of the