On Tue, Feb 26, 2019 at 08:39:12PM +0100, SZEDER Gábor wrote:
> > > I didn't find this to be an issue, but because of functions like
> > > 'test_seq' and 'test_must_fail' I've thought about suppressing '-x'
> > > output for test helpers (haven't actually done anything about it,
> > > though).
> >
> > I'd be curious how you'd do that.
>
> Well, I started replying with "Dunno" and explaining why I don't think
> that it can be done with 'test_must_fail'... but then got a bit of a
> lightbulb moment. Now look at this:
> [...]
> + { set +x ; } 2>/dev/null 4>/dev/null
Ah, this is the magic. Doing:
set +x 2>/dev/null
will still show it, but doing the redirection in a wrapping block means
that it is applied before the command inside the block is run. Clever.
I think this braces trick could be used in general to fix all of the
remaining "you can't run this under -x" cases, though it might be ugly.
It might also be possible to make test_eval_ a bit less subtle with it,
though I think it is relying on the braces already (which makes me
wonder if I just totally forgot about its existence today, or if I
earlier somehow stumbled onto a working recipe because I wanted to run
multiple redirected commands).
> There are a couple of tricky cases:
>
> - Some test helper functions call other test helper functions, and
> in those cases tracing would be enabled upon returning from the
> inner helper function. This is not an issue with e.g.
> 'test_might_fail' or 'test_cmp_config', because the inner helper
> function is the last command anyway. However, there is
> 'test_must_be_empty', 'test_dir_is_empty', 'test_config',
> 'test_commit', etc. which call the other test helper functions
> right at the start or in the middle.
Yeah, this is inherently a global flag that we're playing games with. It
does seem like it would be easy to get it wrong. I guess the right model
is considering it like a stack, like:
-- >8 --
#!/bin/sh
x_counter=0
pop_x() {
ret=$?
case "$x_counter" in
0)
echo >&2 "BUG: too many pops"
exit 1
;;
1)
x_counter=0
set -x
;;
*)
x_counter=$((x_counter - 1))
;;
esac
{ return $ret; } 2>/dev/null
}
# you _must_ call this as "{ push_x; } 2>/dev/null" to avoid polluting
# trace output with the push call
push_x() {
set +x 2>/dev/null
x_counter=$((x_counter + 1))
}
bar() {
{ push_x; } 2>/dev/null
echo in bar
pop_x
}
foo() {
{ push_x; } 2>/dev/null
echo in foo, before bar
bar
echo in foo, after bar
false
pop_x
}
set -x
foo
echo \$? is $?
-- 8< --
I wish there was a way to avoid having to do the block-and-redirect in
the push_x calls in each function, though.
I dunno. I do like the output, but this is rapidly getting complex.
> - && chains in test helper functions; we must make sure that the
> tracing is restored even in case of a failure.
Yeah, there is no "goto out" to help give a common exit point from the
function. You could probably do it with a wrapper, like:
foo() {
{ push_x; } 2>/dev/null
real_foo "$@"
pop_x
}
and then real_foo() is free to return however it likes. I wonder if you
could even wrap that up in a helper:
disable_function_tracing () {
# rename foo() to orig_foo(); this works in bash, but I'm not
# sure if there's a portable way to do it (and ideally one that
# wouldn't involve an extra process).
eval "real_$1 () $(declare -f $1 | tail -n +2)"
# and then install a wrapper which pushes/pops tracing
eval "$1 () { { push_x; } 2>/dev/null; real_$1 \"\$@\"; pop_x; }"
}
foo () { .... }
disable_function_tracing foo
It would be easier if you could just declare the function body as an
argument (and then it would be "declare_untraceable_function", where you
do it all in one step). But then the function body has to be in single
quotes, which is a pain. I think this is definitely pushing the limits
of portable shell (and quite possibly the limits of good taste).
-Peff
