I've discovered what I _think_ is a cygport bug: most functions within
cygport run with `set -e` enabled in Bash, so if a command has a
non-zero return code that isn't explicitly handled, that'll cause the
cygport command overall to fail.  This doesn't work within src_install,
and I _suspect_ that's a bug following from some decidedly unintuitive
Bash behaviour.

The attached test case demonstrates the problem: running `cygport
test.cygport prep compile test` will fail, because of the `false` call
at the start of `src_test`.  Running `cygport test.cygport prep compile
install`, however, won't fail, even though there's a `false` call at the
start of the `src_install` function.

Specifically, when src_install is called in the cygport executable, it's
called as below:

    (__prepinstalldirs && src_install && __src_postinst) 2>&1 | tee -a 
${installlog}

The fact that Bash normally only considers the final command in a
pipeline is handled by a subsequent check of ${PIPESTATUS[0]}, but
quoting the Bash man page description for `set -e`:

> The shell does not exit if the command that fails is ... part of any
> command executed in a && or || list except the command following the
> final && or || ....  If a compound command other than a subshell
> returns a non-zero status because a command failed while -e was being
> ignored, the shell does not exit.
>
> If a compound command or shell function executes in a context where -e
> is being ignored, none of the commands executed within the compound
> command or function body will be affected by the -e setting, even if
> -e is set and a command returns a failure status.

So because the function call is part of a && chain, the `set -e`
behaviour is completely disabled within that function; the only way a
non-zero return code would be propagated from within the function is if
(a) it were explicitly `return`ed or (b) it came from the final command
in the function.

I've provided a single patch to fix this specific issue, partly because
I'd already written it and thought I might as well share it, but I think
this probably wants a bit more thought.  In particular, there are two
things that might mean we want to handle this differently:

- I've only fixed a single problem, but I think bugs of this style are
  much more common, e.g. in the similar code for the `cygport compile`
  command.  It might make sense to fix this in far more locations.

- I wouldn't be at all surprised if this bug, and others of the same
  ilk, have become load-bearing.  If this gets fixed, it might cause a
  lot of build scripts that have otherwise been happily ignoring benign
  non-zero return codes suddenly start failing unexpectedly.

Adam Dinwoodie (1):
  Run install functions separately

 bin/cygport.in | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

-- 
2.40.1

NAME=test
VERSION=1
RELEASE=1
CATEGORY=Test
SUMMARY='Test package'
HOMEPAGE=
LICENSE=

src_compile () {
        cd "$B"
        {
                echo '#!/usr/bin/env sh'
                echo "echo 'Hello, world!'"
        } >helloworld
}

src_install () {
        false
        cd "$B"
        dobin helloworld
}

src_test () {
        false
        PATH="${D}/usr/bin:$PATH"
        helloworld | grep 'Hello'
}

Reply via email to