Re: [PATCH 2/2] Quoting paths in tests

> 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 ;)