On Fri, Sep 04, 2015 at 11:43:15AM -0700, Junio C Hamano wrote:
> > t7800 (in its final test) calls test_config in a subshell which has cd'd
> > into a submodule.
> >
> > Is this something worth worrying about, or is it sufficiently rare that
> > we can live with the current behaviour?
>
> Fixing the instances you found is good, obviously ;-). Thanks for
> working on this.
>
> Even though the proposed detection is BASH-ism, I think it would not
> hurt other shells (they obviously do not help you catch bugs, but
> they would not misbehave as long as you make sure BASH_SUBSHELL is
> either unset or set to 0 at the beginning of the test), and the only
> impact to them would be a invocation of (often built-in) 'test'
> utility, whose performance impact should be miniscule.
>
> I'll wait for opinion from others, of course.
I like it. In general I'm in favor of any lint-like fixes (whether for
the tests or the C code itself) as long as:
1. they don't create false positive noise
2. they don't require extra effort at each call-site
3. they don't have a performance impact
And I think this passes all three. Of course it would be nice if the new
check ran on all shells, but even this seems like a strict improvement.
And I couldn't come up with anything better. I thought we might be able
to use a canary trap, like this:
trap canary QUIT
echo outside
trap
echo inside
(trap)
Because traps are reset inside a subshell, the first "trap" without
arguments should print a string with "canary" in it, and the second
should print nothing.
Except that it isn't remotely how it works in practice. :) Bash leaves
the trap in place inside the subshell. And while dash does the right
thing, any attempt to actually _look_ at the output will put us in a
subshell. So:
case "$(trap)" in
*canary*) ... not in a subshell ...
*) ... in a subshell ...
esac
doesn't work; the trap report inside backticks is always empty (despite
there being an explicit example in the POSIX manpage for trap of using
$(trap) to get the value for a later eval).
It looks like if we use the EXIT trap rather than a signal, bash _does_
do the right thing (which kind of makes sense, I guess). So rather than
a custom canary, we could use our regular EXIT handler as the canary.
But I couldn't find a way to work around dash's issue.
Looks like there was even some discussion in 2010:
http://www.spinics.net/lists/dash/msg00331.html
but my dash 0.5.7 remains broken. Oh, well.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
