Re: [PATCH V3] git clone: is an URL local or ssh

From: Jeff King <peff@peff.net>
Date: Oct 30, 2013, 06:42 UTC
Message-ID: <20131030064231.GA11317@sigill.intra.peff.net>
In-Reply-To: <201310292207.50869.tboegi@web.de>

On Tue, Oct 29, 2013 at 10:07:50PM +0100, Torsten Bögershausen wrote:

>  Changes since V2:
>  clear_ssh and expect_ssh did come back
>  And I couldn't get it working without the
>  >"$TRASH_DIRECTORY/ssh-output"

Thanks.

>  test_when_finished:
>  I could not get that to work. Probably because of the
>  battle between the quotings: '"' "'" '"'

The quoting should be straight-forward, since you can do it in expect_ssh, outside of the regular test eval. But what is tricky is that you do not actually want "ssh-output" to disappear, but rather you want it to be an empty file, so that tests which do not trigger ssh can compare against it using test_cmp.

The patch below makes it work, but I'm thinking that it does not actually improve readability. While the "clear_ssh" call is something each test needs to remember, at least it is obvious there that the test is clearing the state before running the clone.

Show 6 quoted lines

>  Other note about test_might_fail:
>  The first version did not need it, git clone did
>  always succeed.
>  After a while it always failed.
>  According to my understanding, git clone ssh://xxx.yy
>  should fail (as we can not clone) ??

Cloning over ssh via our fake wrapper should work, as the wrapper finds the shell command and execs it. So ssh://host/path should end up running:

  ssh host 'git-upload-pack path'

and the ssh wrapper converts that to:

  git-upload-pack path

If we want to make it more robust, we could cd into a "hosts/$host" directory that simulates the remote host, but I don't know if that is necessary.

--- This is the test_when_finished patch.

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 7db7f48..05afe5a 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -291,14 +291,14 @@ test_expect_success 'setup ssh wrapper' '
 
 	GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
 	export GIT_SSH &&
-	export TRASH_DIRECTORY
+	export TRASH_DIRECTORY &&
+	>"$TRASH_DIRECTORY"/ssh-output
 '
 
-clear_ssh () {
-	>"$TRASH_DIRECTORY/ssh-output"
-}
-
 expect_ssh () {
+	test_when_finished '
+	  (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
+	' &&
 	{
 		case "$1" in
 		none)
@@ -311,7 +311,6 @@ expect_ssh () {
 }
 
 test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
-	clear_ssh &&
 	cp -R src "foo:bar" &&
 	git clone "foo:bar" foobar &&
 	expect_ssh none
@@ -323,7 +322,6 @@ counter=0
 # $3 path
 test_clone_url () {
 	counter=$(($counter + 1))
-	clear_ssh &&
 	test_might_fail git clone "$1" tmp$counter &&
 	expect_ssh "$2" "$3"
 }