From: Junio C Hamano <hidden> Date: 2016-06-15 22:55:46
Torsten Bögershausen [off-list ref] writes:
quoted hunk
What do we think about something like this for fishing for which:
--- a/t/test-lib.sh+++ b/t/test-lib.sh
@@ -644,6 +644,10 @@ yes () {:done}+which(){+echo>&2"which is not portable (please use type)"+exit1+}
This will happen in runtime, which might be good enough ?
if (
which frotz &&
test $(frobonitz --version" -le 2.0
)
then
test_set_prereq FROTZ_FROBONITZ
else
echo >&2 "suitable Frotz/Frobonitz combo not available;"
echo >&2 "some tests may be skipped"
fi
I somehow think this is a lost cause.
What do we think about something like this for fishing for which:
--- a/t/test-lib.sh+++ b/t/test-lib.sh
@@ -644,6 +644,10 @@ yes () {:done}+which(){+echo>&2"which is not portable (please use type)"+exit1+}
This will happen in runtime, which might be good enough ?
if (
which frotz &&
test $(frobonitz --version" -le 2.0
)
then
test_set_prereq FROTZ_FROBONITZ
else
echo >&2 "suitable Frotz/Frobonitz combo not available;"
echo >&2 "some tests may be skipped"
fi
I somehow think this is a lost cause.
I found different ways to detect if frotz is installed in the test suite:
a) use "type" (Should be the fastest ?)
b) call the command directly, check the exit code
c) "! test_have_prereq" (easy to understand, propably most expensive ?)
Do we really need "which" to detect if frotz is installed?
/Torsten
=============
if ! type cvs >/dev/null 2>&1
then
skip_all='skipping cvsimport tests, cvs not found'
test_done
fi
===========
if test -n "$BASH" && test -z "$POSIXLY_CORRECT"; then
# we are in full-on bash mode
true
elif type bash >/dev/null 2>&1; then
# execute in full-on bash mode
unset POSIXLY_CORRECT
exec bash "$0" "$@"
else
echo '1..0 #SKIP skipping bash completion tests; bash not available'
exit 0
fi
===============
svn >/dev/null 2>&1
if test $? -ne 1
then
skip_all='skipping git svn tests, svn not found'
test_done
fi
===============
( p4 -h && p4d -h ) >/dev/null 2>&1 || {
skip_all='skipping git p4 tests; no p4 or p4d'
test_done
}
===============
if ! test_have_prereq PERL; then
skip_all='skipping gitweb tests, perl not available'
test_done
fi
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:55:55
Hi,
Torsten Bögershausen wrote:
On 15.01.13 21:38, Junio C Hamano wrote:
quoted
Torsten Bögershausen [off-list ref] writes:
quoted
quoted
What do we think about something like this for fishing for which:
[...]
quoted
quoted
+which () {
+ echo >&2 "which is not portable (please use type)"
+ exit 1
+}
[...]
quoted
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
What do we think about something like this for fishing for which:
[...]
quoted
quoted
quoted
+which () {
+ echo >&2 "which is not portable (please use type)"
+ exit 1
+}
[...]
quoted
quoted
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