[Libguestfs] [p2v PATCH 1/4] Makefile.am: factor out "make-physical-machine.sh"

[Laszlo Ersek]

On 08/30/22 21:34, Eric Blake wrote:
> On Tue, Aug 30, 2022 at 07:28:43AM +0200, Laszlo Ersek wrote:
>> I had grepped the virt-p2v and virt-v2v shell scripts for shebangs:
>>
>>       1 #!/bin/sh
>>       2 #!/bin/bash
>>       4 #!/usr/bin/env perl
>>      17 #!/bin/bash -
>>
>>       1 #!/bin/bash
>>       1 #!/usr/bin/env python3
>>       1 #!/usr/bin/sh
>>       5 #!/usr/bin/env perl
>>      67 #!/bin/bash -
>>
>> The thinking seems to have been that
>> - python3 and perl may "move",
>> - bash is considered always available.
> 
> For p2v, yeah, we are pretty much guaranteed to be running on Linux,
> and therefore have bash (even if /bin/sh is not bash).
> 
>>>
>>>> +
>>>> +set -e -u -C
>>>
>>> My personal opinion is that 'set -e' is a crutch that should be
>>> avoided because of its unintended interaction with functions;
>>
>> Can you please elaborate?
>>
>> POSIX writes, "When a function is executed, it shall have the
>> syntax-error and variable-assignment properties described for special
>> built-in utilities in the enumerated list at the beginning of Special
>> Built-In Utilities", and I've checked that list -- I don't know what you
>> mean.
> 
> Here's a good writeup:
> http://mywiki.wooledge.org/BashFAQ/105

(Some of the points this page makes fall in the category "utilities may
have unexpected exit statuses" -- but that isn't a problem only for such
scripts that use "set -e", but also for such scripts that meticulously
check the explicit exit status of each command.)

> 
> One of the simplest demonstrations on that page:
> 
> $ bash -c 'set -e; f() { set -e; false; echo huh; }; f; echo survived'
> $ bash -c 'set -e; f() { set -e; false; echo huh; }; f && echo survived'
> huh
> survived
> 
> That is, using 'set -e' to end function f early "works", but ONLY in
> situations where f itself is not invoked in a context where 'set -e'
> is suppressed because of a conditional in an outer scope.  And once
> 'if' or '&&' suppresses 'set -e', you cannot re-enable it within f
> itself.  Context-sensitive behavior of your function body based on the
> context of the caller is NOT intuitive.
> 
> That said, if you KNOW that -e behaves non-intuitively, and plan to
> write your script with that in mind, it can be helpful.  The complaint
> is that because -e is disabled in so many situations, it is harder to
> prove that -e catches all the scenarios that you WANTED to be caught
> than it is to just write the error handling yourself.

I agree this behavior is not intuitive. It does not seem to follow from
the POSIX language

    The -e setting shall be ignored when executing the compound list
    following the while, until, if, or elif reserved word, a pipeline
    beginning with the ! reserved word, or any command of an AND-OR list
    other than the last.

(which is the language I found "least distantly related" to this
behavior). Such "context rules" are frequent in other languages too, but
they only ever apply directly, and not recursively -- sub-contexts tend
to have their own separate environments.

I've checked the subject script now and it does not seem to suffer from
this "set -e" pitfall; thus, I'm going to merge it.

Now, whether this kills "set -e" for me for good... I'm not so sure. I'm
trying to think up a shell function that I would want to (a) call from
an outer conditional context, and at the same time (b) cause the whole
script to abort due to an internal error.

I'm coming up empty here: those goals look mutually exclusive. Here's
why I think so: whether the exit status ("return value") of a function
matters or not is part of the function's specification; i.e., design. If
I design a function such that it return a meaningful value, I *already*
cannot allow any errors to go uncaught in the function body, and I
*also* cannot allow the function to kill the outer context due to any
internal problems. Conversely, if I only need a simple code extraction
from the outer, larger context, I will certainly rely on internal errors
in the function to abort the whole script -- but then I will have *zero
reason* to invoke the function from within an outer conditional.

This is why I think that, although I've been using "set -e" for years
(decades?), I may not have written a single script plagued by this
particular misbehavior. Not because I'm that clever, but because (I
suspect) the situation demonstrated above "almost never" occurs in practice.

So while I'm very surprised by the above demonstration, I'm quite
tempted to believe that the "set -e" masking behavior, albeit not
intuitive, is correct (and that at least I personally can continue using
"set -e", while keeping this non-intuitive behavior in mind).


Either way: what would be an alternative to "set -e" that:

(1) scaled (in the sense that it does not mangle the whole script to
unreadability),

(2) did not introduce the Arrow anti-pattern due to deeply nested "if"s
<https://blog.codinghorror.com/flattening-arrow-code/>,
<http://wiki.c2.com/?ArrowAntiPattern>?

Would we have to write code like

  foo=$(somecommand ...)
  ret=$?
  if [ $ret -ne 0 ]; then
    exit $ret
  fi
  some_other_command -- "$foo"
  ret=$?
  if [ $ret -ne 0 ]; then
    exit $ret
  fi

?

(That's what I mean by "scaling".)


> 
>>
>>> but I'm
>>> not adamant enough about it to tell people to rip it out of scripts.
>>> For short scripts, like this one, it's easy enough to check that we
>>> aren't tripping over any of -e's surprises.
>>>
>>>> +
>>>> +disk=
>>>> +
>>>> +cleanup()
>>>> +{
>>>> +  set +e
>>>> +  if test -n "$disk"; then
>>>> +    rm -f -- "$disk"
>>>> +    disk=
>>>> +  fi
>>>> +}
>>>> +
>>>> +trap cleanup EXIT
>>>
>>> Is it intentional that you are not also cleaning up on signals like
>>> INT and HUP?
>>
>> Yes, as EXIT covers those.
> 
> Okay; currently true enough for bash, but not portable to other
> shells.  In fact, the Austin Group visited that topic just this week:
> 
> https://austingroupbugs.net/view.php?id=621
>  After 2018 edition page 2420 line 77499 section 2.14 trap, add a new
> paragraph:
>     The EXIT condition shall occur when the shell terminates normally
>     (exits), and may occur when the shell terminates abnormally as a
>     result of delivery of a signal (other than SIGKILL) whose trap
>     action is the default.
> 

Thanks!
Laszlo