Re: [PATCH 8/8] Fix tests breaking when checkout path contains shell metacharacters

[Johannes Sixt <j.sixt@viscovery.net>]

Bryan Donlan schrieb:
> (did you mean to send the original reply to the list as well? not ccing for
>  now, feel free to re-send/forward to the list if you like)

Sorry, yes, I hit the wrong button. I'm lazy again and simply resend this
reply with the complete Cc list, with everything quoted.

> On Wed, Apr 09, 2008 at 09:01:12AM +0200, Johannes Sixt wrote:
>> Bryan Donlan schrieb:
>>> -		GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init
>>> +		GIT_WORK_TREE="$(pwd)" GIT_DIR=git-dir-wt-1.git git init
>> This ...
>>
>>> -HERE=`pwd`
>>> +HERE="$(pwd)"
>> ... and this and a lot of similar cases shouldn't be required: The RHS of
>> an assignment does not undergo word-splitting.
> 
> Will fix.
> 
>>> -export GIT_DIR=$(pwd)/repo.git
>>> -export GIT_CONFIG=$GIT_DIR/config
>>> +export GIT_DIR="$(pwd)/repo.git"
>>> +export GIT_CONFIG="$GIT_DIR/config"
>> This, OTOH, is not an assignment, and the change is to the better. But not
>> all shells support export with assignment. Hence this should be changed to
>> the form
>>
>> 	GIT_DIR=$(pwd)/repo.git
>> 	GIT_CONFIG=$GIT_DIR/config
>> 	export GIT_DIR GIT_CONFIG
>>
>> (and similar cases at other places).
> 
> I'll break these bits out into another patch in the sequence and fix.
> 
>>>  test_expect_success  ".rev_db auto-converted to .rev_map.UUID" "
>>>  	git-svn fetch -i trunk &&
>>> -	test -z \"\$(ls $GIT_DIR/svn/trunk/.rev_db.* 2>/dev/null)\" &&
>>> -	expect=\"\$(ls $GIT_DIR/svn/trunk/.rev_map.*)\" &&
>>> +	test -z \"\$(ls \"\$GIT_DIR\"/svn/trunk/.rev_db.* 2>/dev/null)\" &&
>>> +	expect=\"\$(ls \"\$GIT_DIR\"/svn/trunk/.rev_map.*)\" &&
>>>  	test -n \"\$expect\" &&
>>> -	rev_db=\$(echo \$expect | sed -e 's,_map,_db,') &&
>>> -	convert_to_rev_db \$expect \$rev_db &&
>>> -	rm -f \$expect &&
>>> -	test -f \$rev_db &&
>>> +	rev_db=\"\$(echo \$expect | sed -e 's,_map,_db,')\" &&
>>> +	convert_to_rev_db \"\$expect\" \"\$rev_db\" &&
>>> +	rm -f \"\$expect\" &&
>>> +	test -f \"\$rev_db\" &&
>>>  	git-svn fetch -i trunk &&
>>> -	test -z \"\$(ls $GIT_DIR/svn/trunk/.rev_db.* 2>/dev/null)\" &&
>>> -	test ! -e $GIT_DIR/svn/trunk/.rev_db &&
>>> -	test -f \$expect
>>> +	test -z \"\$(ls \"\$GIT_DIR\"/svn/trunk/.rev_db.* 2>/dev/null)\" &&
>>> +	test ! -e \"\$GIT_DIR\"/svn/trunk/.rev_db &&
>>> +	test -f \"\$expect\"
>>>  	"
>> While looking at this test: Wouldn't it be easier to just place the whole
>> thing (and probably similar cases, too) in single-quotes?
> 
> To be honest, I fixed all the git-svn tests with a suitably clever vim s///
> expression, so at the time this way was actually easier :)

Fair enough. If you still have the statement in you vim history, it's
worth quoting in the commit message. It's another piece of information
that helps reviewing.

> (Essentially all of the git-svn tests were broken, and it was a little
>  annoying to fix them all by hand... though of course I verified the
>  results)

-- Hannes