Stefano Lattarini <stefano.lattar...@gmail.com> wrote:
> Hi Ralf.
>
> A new version of the patch is attached. It's still not definitive,
> but I hope most things are settled by now.
And, of course, they're not. Sigh.
With FreeBSD-7.2 /bin/sh, the construct:
if eval "$run_evald_cmd"; then
run_exitcode_got=0
else
run_exitcode_got=$?
fi
causes the shell to exit if `set -e' is on.
Minimal testcase exposing the bug:
$ /bin/sh -ec 'if false; then :; else :; fi; echo OK'; echo $?
OK
0
$ /bin/sh -ec 'if eval false; then :; else :; fi; echo OK'; echo $?
OK
1
l thought to revert to the uglier form:
run_exitcode_got=0
eval "$run_evald_cmd" || run_exitcode_got=$?
but this does not work either. Minimal testcase:
/bin/sh -ec 'false || :; echo OK'; echo $?
OK
0
/bin/sh -ec 'eval false || :; echo OK'; echo $?
1
Finally I settled to the following code:
if (eval "exec $run_evald_cmd"); then
run_exitcode_got=0
else
run_exitcode_got=$?
fi
which works, and has also the advantage of ensuring to run only
external programs, not builtins or shell functions.
After testing on FreeBSD and re-testing on Debian, I will send an
amended patch (and let's hope it will work eventually).
In the meantime...
> > Your run_command already has an -e option. If we're going to
> > return the exit status anyway from the function, then we wouldn't
> > need that -e *at all*, we could just return whatever status the
> > command caused.
> >
> > So if it pleases you better, then we can change it to either of
> > the following:
> >
> > - '-e ignore' ignores the status of the command; run_command
> > returns it;
> > - no '-e' just lets run_command return the status of the command,
> > rather than checking the command against zero.
> > I prefer the first, but only on the grounds that more typing is
> > done only in the rare case.
>
> I thought about it, and now I strongly agree with you: adding
> another subroutine is going to make things unnecessarly
> complicated. But I'd prefer that `run_command -e IGNORE' continues
> to do what it states, e.g. ignore the failures instead of making
> the test case abort if `set -e' is on. Instead, what about adding
> support for a new special argument to `-e', say `RETURN' (which is
> what I did in the attached patch)? It's easy, far less obtrusive
> than adding another function, and it also keeps the "extra-typing"
> low (no need to ever add `-e 0'). Objections?
I went on and implemented `-e RETURN'. Hope you're OK with it.
Regards,
Stefano
