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

[Eric Blake]

On Wed, Aug 31, 2022 at 08:59:15AM +0200, Laszlo Ersek wrote:
> > 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
> > 

> 
> 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.

Actually, this is exactly the POSIX language that describes the
behavior above.  As soon as you invoke 'f && ...', f is now on the
left-hand side of an AND-OR list, and the entire body of f is invoked
in a scenario where you cannot re-enable 'set -e' no matter how hard
you try.

> 
> 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.

Yes, this is the counter-argument for why some people use 'set -e' in
a shell script with functions, despite the potential for pitfall.  The
rule of thumb for such a script becomes: never invoke a function in a
conditional if the function was not careful about handling errors
without reliance on 'set -e'; or more generally, write all functions
to assume that 'set -e' is a no-op. At which point, 'set -e' is only
useful for the top-level code outside of functions; the more your
script relies on functions instead of top-level code, the less likely
'set -e' is something you want to use.

> 
> 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).

Whether it is "intuitive" or "correct" may be a matter of
interpretation; but at the end of the day, POSIX standardized what
existing practice does ('set -e' being disabled on the left side of &&
was historical practice even before shell functions were introduced),
rather than what would be sane if the shell language were being
developed from scratch.

> 
> 
> 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

You can trim it down to something more legible:

foo=$(somecommand ...) || fatal
some_other_command -- "$foo" || fatal

where you write a helper function that encapulates the repetitive
nature of invoking the command and checking for expected exit status.
That particular style is how GNU coreutils writes much of its
testsuite; picking a random example:

https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=tests/ls/a-option.sh

But yeah, converting a script that relied on 'set -e' to one that is
equally safe without requires a framework of helper functions and a
whole-script audit, which is not as scalable as designing that way
from the outset.

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