Libguestfs - Mar 2023 - [libnbd PATCH v4 0/2] lib/utils: introduce async-signal-safe execvpe()

On 3/20/23 20:41, Eric Blake wrote:
> On Sun, Mar 19, 2023 at 10:41:37AM +0100, Laszlo Ersek wrote:
>> This is version 4 of the following sub-series:
>>
>> [libnbd PATCH v3 09/29] lib/utils: introduce async-signal-safe
execvpe()
>> [libnbd PATCH v3 10/29] lib/utils: add unit tests for async-signal-safe
execvpe()
>>
>> http://mid.mail-archive.com/20230215141158.2426855-10-lersek at
redhat.com
>> http://mid.mail-archive.com/20230215141158.2426855-11-lersek at
redhat.com
>>
>> The Notes section on each patch records the updates for that patch.
>>
>> For assisting with incremental review, here's a range-diff:
> 
> Thank you; the changes compared to v3 look sane, and my R-b still stands on
both patches.
> 
>>>       ## lib/test-fork-safe-execvpe.sh (new) ##
>>>      @@
>>>     @@ lib/test-fork-safe-execvpe.sh (new)
>>>      +# License along with this library; if not, write to the Free
Software
>>>      +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
MA 02110-1301 USA
>>>      +
>>>     ++. ../tests/functions.sh
>>>     ++
>>>      +set -e
>>>      +
>>>      +# Determine the absolute pathname of the execvpe helper
binary. The "realpath"
>>>     @@ lib/test-fork-safe-execvpe.sh (new)
>>>      +
>>>      +# Create a temporary directory and change the working
directory to it.
>>>      +tmpd=$(mktemp -d)
>>>     -+trap 'rm -r -- "$tmpd"' EXIT
>>>     ++cleanup_fn rm -r -- "$tmpd"
> 
> "$tmpd" probably starts with /, such that the -- is not strictly
> necessary; but keep it (good practice, and saves us in case someone
> sets $TMPDIR to an unusual relative name and mktemp does not give us a
> leading / after all).
> 
Series merged as commit range 742cbd8c7adc..0b7172b3cffa.

Thank you!
Laszlo

On Tue, Mar 21, 2023 at 07:04:59AM +0100, Laszlo Ersek
wrote:
> On 3/20/23 20:41, Eric Blake wrote:
> > On Sun, Mar 19, 2023 at 10:41:37AM +0100, Laszlo Ersek wrote:
> >> This is version 4 of the following sub-series:
> >>
> >> [libnbd PATCH v3 09/29] lib/utils: introduce async-signal-safe
execvpe()
> >> [libnbd PATCH v3 10/29] lib/utils: add unit tests for
async-signal-safe execvpe()
> >>
...
> > 
> 
> Series merged as commit range 742cbd8c7adc..0b7172b3cffa.
I see you already fixed one pipeline failure due to some gcc versions
being more picky about function __attribute__ placement than others.
The remaining failures are with alpine Linux, where /bin/expr comes
from busybox instead of coreutils, and has the unfortunate quality of
having its behavior dependent on argv[0].

Starting from a clean clone, I reproduced it locally with:

$ git diff
diff --git i/lib/test-fork-safe-execvpe.sh w/lib/test-fork-safe-execvpe.sh
index 838bac9..4b3700c 100755
--- i/lib/test-fork-safe-execvpe.sh
+++ w/lib/test-fork-safe-execvpe.sh
@@ -18,7 +18,7 @@

 . ../tests/functions.sh

-set -e
+set -ex

 # Determine the absolute pathname of the execvpe helper binary. The
"realpath"
 # utility is not in POSIX, but Linux, FreeBSD and OpenBSD all have it.
@@ -155,7 +155,7 @@ success()

 # Create a temporary directory and change the working directory to it.
 tmpd=$(mktemp -d)
-cleanup_fn rm -r -- "$tmpd"
+#cleanup_fn rm -r -- "$tmpd"
 cd "$tmpd"

 # If the "file" parameter of execvpe() is an empty string, then we
must fail --

$ podman build -f ci/containers/alpine-edge.Dockerfile -t libnbd-alpine-edge
$ podman run -it --rm --userns=keep-id -v .:/repo:z -w /repo libnbd-alpine-edge
bash
$ ./configure
$ make check
$ grep tmpd= lib/test-suite.log 
+ tmpd=/tmp/tmp.EMgKeF
$ /tmp/tmp.EMgKeF/bin/f 1 + 1
f: applet not found
0b748c9fe495:~$ 

So it looks like we need some way to work around busybox' insistance
that argv[0] determines which applet to run.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org