Libguestfs - Mar 2023 - [libnbd PATCH 1/3] lib/test-fork-safe-execvpe.sh: generalize "run" to "run0"

These patches have been pushed (f5a065aa3a9c..9075f68ffc8b); I'm posting
them for visibility.

Laszlo

Laszlo Ersek (3):
  lib/test-fork-safe-execvpe.sh: generalize "run" to "run0"
  lib/test-fork-safe-execvpe.sh: cope with Alpine Linux / BusyBox
    limitations
  Revert "ci: skip "lib/test-fork-safe-execvpe.sh" on Alpine
Linux"

 ci/skipped_tests              | 10 ----
 lib/test-fork-safe-execvpe.sh | 63 +++++++++++++-------
 2 files changed, 42 insertions(+), 31 deletions(-)

It turns out that we'll need to put the generality of our
"test-fork-safe-execvpe" binary executable to use, in that it takes
separate "program-to-exec" and "argv0" (for the
program-to-exec)
arguments.

Currently, the "run" function duplicates $2 to both
"program-to-exec" and
"argv0", for "test-fork-safe-execvpe". Remove the
duplication (expect the
caller to provide separate $2 and $3 arguments, respectively) and rename
"run" to "run0". At the same time, reimplement
"run" as a simple wrapper
(i.e., with just the duplication) around "run0".

This patch is worth viewing with "git show --color-words" as well.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/test-fork-safe-execvpe.sh | 28 +++++++++++++-------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/lib/test-fork-safe-execvpe.sh b/lib/test-fork-safe-execvpe.sh
index 5d2671946c66..f055f7b05814 100755
--- a/lib/test-fork-safe-execvpe.sh
+++ b/lib/test-fork-safe-execvpe.sh
@@ -32,42 +32,52 @@ execvpe=$(realpath -- "$dname/$bname")
 # If $1 is "_", then the $execvpe helper binary is invoked with PATH
unset.
 # Otherwise, the binary is invoked with PATH set to $1.
 #
-# $2 and onward are passed to $execvpe; note that $2 becomes *both*
-# "program-to-exec" for the helper *and* argv[0] for the program
executed by the
-# helper.
+# $2 and onward are passed to $execvpe; $2 becomes "program-to-exec"
for the
+# helper and $3 becomes argv[0] for the program executed by the helper.
 #
 # The command itself (including the PATH setting) is written to "cmd"
(for error
 # reporting purposes only); the standard output and error are saved in
"out" and
 # "err" respectively; the exit status is written to
"status". This function
 # should never fail; if it does, then that's a bug in this unit test
script, or
 # the disk is full etc.
-run()
+run0()
 {
     local pathctl=$1
-    local program=$2
     local exit_status
 
     shift 1
 
     if test _ = "$pathctl"; then
-        printf 'unset PATH; %s %s %s\n' "$execvpe"
"$program" "$*" >cmd
+        printf 'unset PATH; %s %s\n' "$execvpe"
"$*" >cmd
         set +e
         (
             unset PATH
-            "$execvpe" "$program" "$@" >out
2>err
+            "$execvpe" "$@" >out 2>err
         )
         exit_status=$?
         set -e
     else
-        printf 'PATH=%s %s %s %s\n' "$pathctl"
"$execvpe" "$program" "$*" >cmd
+        printf 'PATH=%s %s %s\n' "$pathctl"
"$execvpe" "$*" >cmd
         set +e
-        PATH=$pathctl "$execvpe" "$program" "$@"
>out 2>err
+        PATH=$pathctl "$execvpe" "$@" >out 2>err
         exit_status=$?
         set -e
     fi
     printf '%d\n' $exit_status >status
 }
 
+# Does the same as "run0", but $2 becomes *both*
"program-to-exec" for the the
+# $execvpe helper binary *and* argv[0] for the program executed by the helper.
+run()
+{
+    local pathctl=$1
+    local program=$2
+
+    shift 1
+
+    run0 "$pathctl" "$program" "$@"
+}
+
 # After "run" returns, the following three functions can verify the
result.
 #
 # Check if the helper binary failed in nbd_internal_execvpe_init().

Whenever we expect our "bin/f" and "sh/f" files to execute
(and to
succeed), provide "expr" and "sh" as argv[0], respectively.
This is to
work around BusyBox, where the central busybox binary selects its behavior
based on argv[0]. See e.g.
<https://bugs.busybox.net/show_bug.cgi?id=15481>.

Also cope with "realpath" (from BusyBox) not accepting the
"--"
end-of-options argument separator. See
<https://bugs.busybox.net/show_bug.cgi?id=15466>.

This patch is worth viewing with "git show --color-words" as well.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 lib/test-fork-safe-execvpe.sh | 35 +++++++++++++-------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/lib/test-fork-safe-execvpe.sh b/lib/test-fork-safe-execvpe.sh
index f055f7b05814..0e2dd942c9b7 100755
--- a/lib/test-fork-safe-execvpe.sh
+++ b/lib/test-fork-safe-execvpe.sh
@@ -24,7 +24,11 @@ set -e
 # utility is not in POSIX, but Linux, FreeBSD and OpenBSD all have it.
 bname=$(basename -- "$0" .sh)
 dname=$(dirname -- "$0")
-execvpe=$(realpath -- "$dname/$bname")
+if ! execvpe=$(realpath -- "$dname/$bname"); then
+    # Work around <https://bugs.busybox.net/show_bug.cgi?id=15466>. For
example,
+    # Alpine Linux in the libnbd CI uses BusyBox.
+    execvpe=$(realpath "$dname/$bname")
+fi
 
 # This is an elaborate way to control the PATH variable around the $execvpe
 # helper binary as narrowly as possible.
@@ -35,6 +39,13 @@ execvpe=$(realpath -- "$dname/$bname")
 # $2 and onward are passed to $execvpe; $2 becomes "program-to-exec"
for the
 # helper and $3 becomes argv[0] for the program executed by the helper.
 #
+# (Outside of the "run" function below, "run0" should only
ever be called for
+# working around BusyBox. (Alpine Linux in the libnbd CI uses BusyBox.)
+# BusyBox's utilities "expr", "sh" etc. are just
symlinks to the central
+# "busybox" binary executable, and that executable keys its behavior
off the
+# name under which it is invoked -- that is, $3 here. See
+# <https://bugs.busybox.net/show_bug.cgi?id=15481>.)
+#
 # The command itself (including the PATH setting) is written to "cmd"
(for error
 # reporting purposes only); the standard output and error are saved in
"out" and
 # "err" respectively; the exit status is written to
"status". This function
@@ -218,7 +229,7 @@ run "" nxregf/f/; execve_fail nxregf/f/ ENOTDIR
 run "" symlink/f; execve_fail symlink/f ELOOP
 
 # This runs "expr 1 + 1".
-run "" bin/f 1 + 1; success bin/f,2
+run0 "" bin/f expr 1 + 1; success bin/f,2
 
 # This triggers the ENOEXEC branch in nbd_internal_fork_safe_execvpe().
 # nbd_internal_fork_safe_execvpe() will first try
@@ -231,7 +242,7 @@ run "" bin/f 1 + 1; success bin/f,2
 #
 # The shell script will get "sh/f" for $0 and "arg1" for
$1, and print those
 # out.
-run "" sh/f arg1; success sh/f,"sh/f arg1"
+run0 "" sh/f sh arg1; success sh/f,"sh/f arg1"
 
 # In the tests below, the "file" parameter of execvpe() will not
contain a
 # <slash> character.
@@ -267,21 +278,21 @@ execve_fail empty/f,fifo/f,subdir/f,nxregf/f,symlink/f
ELOOP
 # seen), and (b) that nbd_internal_fork_safe_execvpe() succeeds for the *last*
 # candidate. Repeat the test with "expr" (called "f" under
"bin") and the shell
 # script (called "f" under "sh", triggering the ENOEXEC
branch).
-run bin f 1 + 1; success bin/f,2
-run sh  f arg1;  success sh/f,"sh/f arg1"
+run0 bin f expr 1 + 1; success bin/f,2
+run0 sh  f sh   arg1;  success sh/f,"sh/f arg1"
 
 # Demonstrate that the order of candidates matters. The first invocation finds
 # "expr" (called "f" under "bin"), the second
invocation finds the shell script
 # under "sh" (triggering the ENOEXEC branch).
-run empty:bin:sh f 1 + 1; success empty/f,bin/f,sh/f,2
-run empty:sh:bin f arg1;  success empty/f,sh/f,bin/f,"sh/f arg1"
+run0 empty:bin:sh f expr 1 + 1; success empty/f,bin/f,sh/f,2
+run0 empty:sh:bin f sh   arg1;  success empty/f,sh/f,bin/f,"sh/f
arg1"
 
 # Check the expansion of zero-length prefixes in PATH to ".", plus
the
 # (non-)insertion of the "/" separator.
-run a/:       f;       execve_fail a/f,./f                 ENOENT
-run :a/       f;       execve_fail ./f,a/f                 ENOENT
-run :         f;       execve_fail ./f,./f                 ENOENT
+run  a/:       f;            execve_fail a/f,./f                 ENOENT
+run  :a/       f;            execve_fail ./f,a/f                 ENOENT
+run  :         f;            execve_fail ./f,./f                 ENOENT
 pushd bin
-run :         f 1 + 1; success     ./f,./f,2
+run0 :         f expr 1 + 1; success     ./f,./f,2
 popd
-run :a/:::b/: f;       execve_fail ./f,a/f,./f,./f,b/f,./f ENOENT
+run  :a/:::b/: f;            execve_fail ./f,a/f,./f,./f,b/f,./f ENOENT

This reverts commit f5a065aa3a9cc71854d5f6143a6c4e0c6717cf28.

"lib/test-fork-safe-execvpe.sh" now works around Alpine Linux /
BusyBox
limitations; reenable the test in the Alpine Linux containers.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 ci/skipped_tests | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ci/skipped_tests b/ci/skipped_tests
index 97158c0a17c2..0614a5c12201 100644
--- a/ci/skipped_tests
+++ b/ci/skipped_tests
@@ -6,13 +6,3 @@
 
 # CentOS (stream) 8 probably has an older, buggy version of nbdkit and is not
updated
 ^CentOS .*8$;info/info-uri.sh info/info-list-uris.sh
interop/list-exports-nbdkit
-
-# Alpine Linux provides the "expr" utility (under pathname
"/usr/bin/expr") in
-# one of two forms: (1) as a symlink to the "/bin/busybox" binary
executable,
-# when the "coreutils" package is not installed, (2) as a symlink to
the
-# "coreutils" binary executable, otherwise. Both linked-to binary
executables
-# key their behavior off the name under which they are invoked (that is,
argv0).
-# Unfortunately, this breaks "lib/test-fork-safe-execvpe.sh": that
test case
-# copies the "expr" utility to a file called "bin/f", and
actually expects
-# "bin/f" to work. Both "busybox" and "coreutils"
croak when invoked as "f".
-^Alpine Linux-.*$;lib/test-fork-safe-execvpe.sh