On Fri, Nov 03, 2023 at 05:57:08PM +0100, ASSI via Cygwin-apps wrote:
> Adam Dinwoodie via Cygwin-apps writes:
> > When running as part of a `&&` chain, Bash's `set -e` behaviour is
> > suppressed entirely, which means calls that produce non-zero error codes
> > will be ignored if they're called inside functions that are part of such
> > a chain.
> >
> > To avoid silent failures from commands in a src_install function being
> > ignored, don't chain the function calls with &&, and instead just let
> > Bash's `set -e` behaviour handle failures.
> 
> That patch circumvents the shortcutting of the execution chain at the
> first fail and might produce undesirable results elsewhere.  Unless
> there's another interesting behaviour in Bash, it should suffice to
> follow the last command in the "&&" chain by "|| false" to transfer the
> status of the chain to PIPESTATUS (provided that the return code of the
> functions involved is set correctly).

I think you've misunderstood how `set -e` works.  When `set -e` is
active, any non-zero return code from the final command in a pipeline
will cause the script to exit.  So both with and without my patch, a
non-zero return code from __prepinstalldirs, src_install or
__src_postinst will cause execution to stop.

The issue is that the && chain disables `set -e` for anything other than
the final command in the chain, *including within functions*.  In most
functions in cygport, a non-zero return code will cause cygport to
error, but because the && chain disables `set -e`, failures within
src_install are silently ignored.  Counterintuitively, having the &&
chain present means that execution will _continue_ after a failure!

Compare the two scripts below:

```
#!/usr/bin/bash

set -e

src_install () {
        false
        echo "This step never runs"
}

src_install
echo "This also doesn't run"
```

```
#!/usr/bin/bash

src_install () {
        false
        echo "This step *does* run"
}

src_install && echo "As does this step"
```

I believe the intent of using `set -e` in cygport is that when there's a
failure -- as with the false command in the src_install functions above
-- execution will stop.  This works as expected in the first example,
but in the second example, the `false` call has no effect whatsoever:
Bash will run both the echo within the function and the one that follows
the function.

See also:
- https://www.shellcheck.net/wiki/SC2310
- https://mywiki.wooledge.org/BashFAQ/105

Alternatively, just have a look at the test case I attached to the cover
email; I'd expect that cygport file to fail the install stage, because
the first command in the src_install function is an unhandled failure.
Currently, the src_install succeeds, with no hint of any problems, but
with my patch, it produces an error as expected.

Reply via email to