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
