On 27.01.13 10:31, Jonathan Nieder wrote:
> Hi,
>
> Torsten Bögershausen wrote:
>> On 15.01.13 21:38, Junio C Hamano wrote:
>>> Torsten Bögershausen <tboegi@xxxxxx> writes:
>
>>>> What do we think about something like this for fishing for which:
> [...]
>>>> +which () {
>>>> + echo >&2 "which is not portable (please use type)"
>>>> + exit 1
>>>> +}
> [...]
>>> if (
>>> which frotz &&
>>> test $(frobonitz --version" -le 2.0
>>> )
>
> With the above definition of "which", the only sign of a mistake would
> be some extra output to stderr (which is quelled when running tests in
> the normal way). The "exit" is caught by the subshell and just makes
> the "if" condition false.
>
> That's not so terrible --- it could still dissuade new test authors
> from using "which". The downside I'd worry about is that it provides
> a false sense of security despite not catching problems like
>
> write_script x <<-EOF &&
> # Use "foo" if possible. Otherwise use "bar".
> if which foo && test $(foo --version) -le 2.0
> then
> ...
> ...
> EOF
> ./x
>
> That's not a great tradeoff relative to the impact of the problem
> being solved.
>
> Don't get me wrong. I really do want to see more static or dynamic
> analysis of git's shell scripts in the future. I fear that for the
> tradeoffs to make sense, though, the analysis needs to be more
> sophisticated:
>
> * A very common error in test scripts is leaving out the "&&"
> connecting adjacent statements, which causes early errors
> in a test assertion to be missed and tests to pass by mistake.
> Unfortunately the grammar of the dialect of shell used in tests is
> not regular enough to make this easily detectable using regexps.
>
> * Another common mistake is using "cd" without entering a subshell.
> Detecting this requires counting nested parentheses and noticing
> when a parenthesis is quoted.
>
> * Another common mistake is relying on the semantics of variable
> assignments in front of function calls. Detecting this requires
> recognizing which commands are function calls.
>
> In the end the analysis that works best would probably involve a
> full-fledged shell script parser. Something like "sparse", except for
> shell command language.
>
> Sorry I don't have more practical advice in the short term.
>
> My two cents,
> Jonathan
Jonathan,
thanks for the review.
My ambition is to get the check-non-portable-shell.pl into a shape
that we can enable it by default.
This may be with or without checking for "which".
As a first step we will hopefully see less breakage for e.g. Mac OS
caused by "echo -n" or "sed -i" usage.
On the longer run, we may be able to have something more advanced.
Back to the which:
Writing a t0009-no-which.sh like this:
--------------------
#!/bin/sh
test_description='Test the which'
. ./test-lib.sh
which () {
echo >&2 "which is not portable (please use type)"
exit 1
}
test_expect_success 'which is not portable' '
if which frotz
then
say "frotz does not exist"
else
say "frotz does exist"
fi
'
test_done
--------------
and running "make test" gives the following, at least in my system:
[snip]
*** t0009-no-which.sh ***
FATAL: Unexpected exit with code 1
make[2]: *** [t0009-no-which.sh] Error 1
make[1]: *** [test] Error 2
make: *** [test] Error 2
-----------------------
running inside t/
./t0009-no-which.sh --verbose
Initialized empty Git repository in /Users/tb/projects/git/tb/t/trash directory.t0009-no-which/.git/
expecting success:
if which frotz
then
say "frotz does not exist"
else
say "frotz does exist"
fi
which is not portable (please use type)
FATAL: Unexpected exit with code 1
/Torsten
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html