From: Johannes Sixt <j.sixt@viscovery.net>
Date: Wed, 17 Oct 2007 11:32:54 GMT
Subject: Re: [PATCH 2/2] Quoting paths in tests
Message-ID: <4715F2E6.1000708@viscovery.net>
In-Reply-To: <11926134961275-git-send-email-maillist@steelskies.com>

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.

>  " >&3 2>&4


> -	svn import -m 'import for git-svn' . $svnrepo >/dev/null &&
> +	svn import -m 'import for git-svn' . \"$svnrepo\" >/dev/null &&

This must be

	svn import -m 'import for git-svn' . \"\$svnrepo\" >/dev/null &&

to be safe. Your version would break with names with double-quotes, because 
$svnrepo would be expanded and then eval'd inside test_expect_*. This error 
recurs numerous times until the end of the patch.

May I recommend that you run the test suite in a directory named like this:

	$ mkdir \"\ \$GIT_DIR\ \'
	$ ls
	" $GIT_DIR '

> -		( mkdir -p $GIT_DIR/svn/\$ref/info/ &&
> -		echo $svnrepo\$path > $GIT_DIR/svn/\$ref/info/url ) || exit 1;
> +		( mkdir -p \"$GIT_DIR\"/svn/\$ref/info/ &&
> +		echo \"$svnrepo\"\$path > \"$GIT_DIR\"/svn/\$ref/info/url ) || exit 1;

I assume $path is under control of the test script, otherwise it must be 
inside the double-quotes, too.

>  test_expect_success  ".rev_db auto-converted to .rev_db.UUID" "
>  	git-svn fetch -i trunk &&
> -	expect=$GIT_DIR/svn/trunk/.rev_db.* &&
> +	expect=\"\`find \"\$GIT_DIR\"/svn/trunk/ -name '.rev_db.*'\`\" &&

Why is this trickery with find needed? Isn't it easier to put the whole test 
case in single-quotes instead?

-- Hannes