On 18 Oct 2007, at 07:08, Johannes Sixt wrote:
Jonathan del Strother schrieb:
On 17 Oct 2007, at 12:32, Johannes Sixt wrote:
Jonathan del Strother schrieb:
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -25,7 +25,7 @@ perl -w -e "
use SVN::Core;
use SVN::Repos;
\$SVN::Core::VERSION gt '1.1.0' or exit(42);
-system(qw/svnadmin create --fs-type fsfs/, '$svnrepo') == 0 or
exit(41);
+system(qw/svnadmin create --fs-type fsfs/, \"$svnrepo\") == 0 or
exit(41);
Here you have to work harder: The reason is that this is part of a
perl expression (as opposed to an eval'd string), which does not
have access to $svnrepo of the shell by which it is invoked. The
original version failed if there were single-quotes in $svnrepo,
the new version fails if it contains double-quotes.
You can rewrite this expression as
perl -w -e '$svnrepo = shift;
...
$SVN::Core::Version gt "1.1.0" ...
system(qw/svnadmin create --fs-type fsfs/, $svnrepo) == 0 ...
...
' >&3 2>&4 "$svnrepo"
i.e. you pass the repository name as an argument to the scriptlet.
May I recommend that you run the test suite in a directory named
like this:
$ mkdir \"\ \$GIT_DIR\ \'
$ ls
" $GIT_DIR '
Eww. I'm struggling a bit with paths this perverse, actually.
For instance, git_editor in git-sh-setup expects the editor path to
be pre-quoted. So in t3404, you need to produce escaped double
quotes & dollar signs, resulting in unpleasantness like this :
VISUAL="`pwd`/fake-editor.sh"
VISUAL=${VISUAL//\"/\\\"}
VISUAL=${VISUAL//$/\\\$}
This is a bashism - that's a big no-no.
VISUAL=\"$VISUAL\"
export VISUAL
And I'm struggling to come up with neat ways of rewriting things
like, eg, this bit from t5500 -
test_expect_success "clone shallow" "git-clone --depth 2 \"file://
`pwd`/.\" shallow"
- to handle paths like that properly.
These examples expand `pwd` too early. Can't you just put everything
inside single-quotes? Although I'm not sure about VISUAL: Is it
invoked with $PWD that is different from $PWD when VISUAL is
defined? If so, then you can hardly delay `pwd`...
I know I'm a bit anal with my criticism. I reviewed your patch
because I think fixing for paths with whitespace is worthwhile.
However, I also think any fix should go the full way and not only
shift the problems into a different corner. Maybe a word from
$maintainer would be in order ;)
In theory, I agree that the tests should properly handle perverse
paths, but it's beginning to stretch my shell scripting skills.
So now our esteemed leader is back in business, any thoughts on how
hard we want to work to quote things?
Also, from "What's cooking in git.git" :
I have a feeling that this should have forked off of 'maint'.
The change looks obvious and trivial, so perhaps after getting a
testcase (hint, hint) merge to 'master' and then cherry-pick to
'maint' as well.
Noted
Jon del Strother
-
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
