threads / patch / 3376

patch, 3 partsTrap exit to clean up created directory if clone fails.

Subject: [PATCH 1/3] Trap exit to clean up created directory if clone fails.

## tl;dr

4 messages between Feb 17, 2006 and Feb 17, 2006. Diffs are folded; open one to read it.

replies: 3people: 1as markdown or json

Carl Worth· Feb 17, 2006, 21:33 UTC · lore

Making git-clone clean up when it fails

So instead of just making noise, I'll start actually contributing to git. The first patch here is designed to fix the annoying bug that when git-clone fails it still leaves the created directory around, (so that a subsequent git-clone will be guaranteed to fail since the directory already exists).

The second and third patches provide a new test case for the new behavior. I had to guess at the number for the new test, since the distinction of the various numbers in t/README was too vague for me to know where a git-clone test should go. Feel free to adjust that as you might see fit.

-Carl

-- 
cworth@redhat.com

Carl Worth· Feb 17, 2006, 21:33 UTC · re: Carl Worth · lore

Signed-off-by: Carl Worth <cworth@cworth.org>

---

 git-clone.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

589b68168e3dad89238b879b06680c662a99ad8d

Show changes to git-clone.sh +4 −0

diff --git a/git-clone.sh b/git-clone.sh
index e192b08..d184ceb 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -118,6 +118,7 @@ dir="$2"
 [ -e "$dir" ] && echo "$dir already exists." && usage
 mkdir -p "$dir" &&
 D=$(cd "$dir" && pwd) &&
+trap 'err=$?; rm -r $D; exit $err' exit
 case "$bare" in
 yes) GIT_DIR="$D" ;;
 *) GIT_DIR="$D/.git" ;;
@@ -255,3 +256,6 @@ Pull: $head_points_at:$origin" &&
 		git checkout
 	esac
 fi
+
+trap - exit
+

-- 
1.2.0.gf6e8

Carl Worth· Feb 17, 2006, 21:33 UTC · re: Carl Worth · lore

[PATCH 2/3] Abstract test_create_repo out for use in tests.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

 t/test-lib.sh |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

6110a439c2b1962c6f128cdaac3a2ee8450ac22c

Show changes to t/test-lib.sh +17 −6

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7a58a86..66f62b9 100755
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -149,6 +149,21 @@ test_expect_code () {
 	fi
 }
 
+# Most tests can use the created repository, but some amy need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	owd=`pwd`
+	repo="$1"
+	mkdir "$repo"
+	cd "$repo" || error "Cannot setup test environment"
+	"$GIT_EXEC_PATH/git" init-db --template=$GIT_EXEC_PATH/templates/blt/ 2>/dev/null ||
+	error "cannot run git init-db -- have you built things yet?"
+	mv .git/hooks .git/hooks-disabled
+	cd "$owd"
+}
+	
 test_done () {
 	trap - exit
 	case "$test_failure" in
@@ -196,9 +211,5 @@ test -d ../templates/blt || {
 # Test repository
 test=trash
 rm -fr "$test"
-mkdir "$test"
-cd "$test" || error "Cannot setup test environment"
-"$GIT_EXEC_PATH/git" init-db --template=../../templates/blt/ 2>/dev/null ||
-error "cannot run git init-db -- have you built things yet?"
-
-mv .git/hooks .git/hooks-disabled
+test_create_repo $test
+cd "$test"

-- 
1.2.0.gf6e8

Carl Worth· Feb 17, 2006, 21:33 UTC · re: Carl Worth · lore

[PATCH 3/3] New test to verify that when git-clone fails it cleans up the new directory.

Signed-off-by: Carl Worth <cworth@cworth.org>

---

 t/t5600-clone-fail-cleanup.sh |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100755 t/t5600-clone-fail-cleanup.sh

0d8852c105e5f910ba53fc974a5d929a2136aecd

Show changes to t/t5600-clone-fail-cleanup.sh +36 −0

diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh
new file mode 100755
index 0000000..0c6a363
--- /dev/null
+++ b/t/t5600-clone-fail-cleanup.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+#
+# Copyright (C) 2006 Carl D. Worth <cworth@cworth.org>
+#
+
+test_description='test git-clone to cleanup after failure
+
+This test covers the fact that if git-clone fails, it should remove
+the directory it created, to avoid the user having to manually
+remove the directory before attempting a clone again.'
+
+. ./test-lib.sh
+
+test_expect_failure \
+    'clone of non-existent source should fail' \
+    'git-clone foo bar'
+
+test_expect_failure \
+    'failed clone should not leave a directory' \
+    'cd bar'
+
+# Need a repo to clone
+test_create_repo foo
+
+# clone doesn't like it if there is no HEAD. Is that a bug?
+(cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1)
+
+test_expect_success \
+    'clone should work now that source exists' \
+    'git-clone foo bar'
+
+test_expect_success \
+    'successfull clone must leave the directory' \
+    'cd bar'
+
+test_done

-- 
1.2.0.gf6e8