minimal/do: fix a really scary bugs in "set -e" behaviour.
If you run something like blah_function || return 1 then everything even *inside* blah_function is *not* subject to the "set -e" that would otherwise be in effect. That's true even for ". subfile" inside blah_function - which is exactly how minimal/do runs .do files. Instead, rewrite it as blah_function [ "$?" = "0" ] || return 1 And add a bit to the unit tests to ensure that "set -e" behaviour is enabled in .do files as we expect, and crash loudly otherwise. (This weird behaviour may only happen in some shells and not others.) Also, we had a "helpful" alias of redo() defined at the bottom of the file. Combined with the way we use '.' to source the .do files, this would make it not start a new shell just to run a recursive 'redo' command. It almost works, but this stupid "set -e" bug could cause a nested .do file to not honour "set -e" if someone ran "redo foo || exit 1" from inside a .do script. The performance optimization is clearly not worth it here, so rename it to _redo(); that causes it to actually re-exec the redo program (which is a symlink to minimal/do).
This commit is contained in:
parent
ede182cb84
commit
c28181e26f
6 changed files with 24 additions and 4 deletions
10
minimal/do
10
minimal/do
|
|
@ -153,19 +153,21 @@ _dir_shovel()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
redo()
|
_redo()
|
||||||
{
|
{
|
||||||
|
set +e
|
||||||
for i in "$@"; do
|
for i in "$@"; do
|
||||||
_dirsplit "$i"
|
_dirsplit "$i"
|
||||||
_dir_shovel "$dir" "$base"
|
_dir_shovel "$dir" "$base"
|
||||||
dir=$xdir base=$xbase basetmp=$xbasetmp
|
dir=$xdir base=$xbase basetmp=$xbasetmp
|
||||||
( cd "$dir" && _do "$dir" "$base" "$basetmp" ) || return 1
|
( cd "$dir" && _do "$dir" "$base" "$basetmp" )
|
||||||
|
[ "$?" = 0 ] || return 1
|
||||||
done
|
done
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
set -e
|
_redo "$@"
|
||||||
redo "$@"
|
[ "$?" = 0 ] || exit 1
|
||||||
|
|
||||||
if [ -n "$DO_TOP" ]; then
|
if [ -n "$DO_TOP" ]; then
|
||||||
echo "Removing stamp files..." >&2
|
echo "Removing stamp files..." >&2
|
||||||
|
|
|
||||||
1
t/000-set-minus-e/.gitignore
vendored
Normal file
1
t/000-set-minus-e/.gitignore
vendored
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
log
|
||||||
4
t/000-set-minus-e/all.do
Normal file
4
t/000-set-minus-e/all.do
Normal file
|
|
@ -0,0 +1,4 @@
|
||||||
|
rm -f log
|
||||||
|
redo fatal >&/dev/null || true
|
||||||
|
|
||||||
|
[ "$(cat log)" = "ok" ] || exit 5
|
||||||
1
t/000-set-minus-e/clean.do
Normal file
1
t/000-set-minus-e/clean.do
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
rm -f log *~ .*~
|
||||||
4
t/000-set-minus-e/fatal.do
Normal file
4
t/000-set-minus-e/fatal.do
Normal file
|
|
@ -0,0 +1,4 @@
|
||||||
|
rm -f log
|
||||||
|
echo ok >>log
|
||||||
|
this-should-cause-a-fatal-error
|
||||||
|
echo fail >>log # this line should never run
|
||||||
8
t/all.do
8
t/all.do
|
|
@ -1,3 +1,11 @@
|
||||||
|
# tests that "set -e" works (.do scripts always run with -e set by default)
|
||||||
|
rm -f 000-set-minus-e/log
|
||||||
|
redo 000-set-minus-e/all
|
||||||
|
if [ "$(cat 000-set-minus-e/log)" != "ok" ]; then
|
||||||
|
echo "FATAL! .do file not run with 'set -e' in effect!" >&2
|
||||||
|
exit 5
|
||||||
|
fi
|
||||||
|
|
||||||
# builds 1xx*/all
|
# builds 1xx*/all
|
||||||
/bin/ls 1[0-9][0-9]*/all.do |
|
/bin/ls 1[0-9][0-9]*/all.do |
|
||||||
sed 's/\.do$//' |
|
sed 's/\.do$//' |
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue