From a4c4efd25159a8b308f9e204bc87e37b963bc2ae Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 2 Jan 2018 16:08:33 -0500 Subject: t5600: fix outdated comment about unborn HEAD Back when this test was written, git-clone could not handle a repository without any commits. These days it works fine, and this comment is out of date. At first glance it seems like we could just drop this code entirely now, but it's necessary for the final test, which was added later. That test corrupts the repository by temporarily removing its objects, which means we need to have some objects to move. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5600-clone-fail-cleanup.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't') diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh index 4435693bb2..f23f92e5a7 100755 --- a/t/t5600-clone-fail-cleanup.sh +++ b/t/t5600-clone-fail-cleanup.sh @@ -22,7 +22,7 @@ test_expect_success \ # Need a repo to clone test_create_repo foo -# clone doesn't like it if there is no HEAD. Is that a bug? +# create some objects so that we can corrupt the repo later (cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1) # source repository given to git clone should be relative to the -- cgit v1.2.3 From 8486b84f0ea4d7f943fdbe81050553d69d198742 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 2 Jan 2018 16:09:00 -0500 Subject: t5600: modernize style This is an old script which could use some updating before we add to it: - use the standard line-breaking: test_expect_success 'title' ' body ' - run all code inside test_expect blocks to catch unexpected failures in setup steps - use "test_commit -C" instead of manually entering sub-repo - use test_when_finished for cleanup steps - test_path_is_* as appropriate Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5600-clone-fail-cleanup.sh | 48 ++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 23 deletions(-) (limited to 't') diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh index f23f92e5a7..7b2a8052f8 100755 --- a/t/t5600-clone-fail-cleanup.sh +++ b/t/t5600-clone-fail-cleanup.sh @@ -11,42 +11,44 @@ remove the directory before attempting a clone again.' . ./test-lib.sh -test_expect_success \ - 'clone of non-existent source should fail' \ - 'test_must_fail git clone foo bar' +test_expect_success 'clone of non-existent source should fail' ' + test_must_fail git clone foo bar +' -test_expect_success \ - 'failed clone should not leave a directory' \ - '! test -d bar' +test_expect_success 'failed clone should not leave a directory' ' + test_path_is_missing bar +' -# Need a repo to clone -test_create_repo foo +test_expect_success 'create a repo to clone' ' + test_create_repo foo +' -# create some objects so that we can corrupt the repo later -(cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1) +test_expect_success 'create objects in repo for later corruption' ' + test_commit -C foo file +' # source repository given to git clone should be relative to the # current path not to the target dir -test_expect_success \ - 'clone of non-existent (relative to $PWD) source should fail' \ - 'test_must_fail git clone ../foo baz' +test_expect_success 'clone of non-existent (relative to $PWD) source should fail' ' + test_must_fail git clone ../foo baz +' -test_expect_success \ - 'clone should work now that source exists' \ - 'git clone foo bar' +test_expect_success 'clone should work now that source exists' ' + git clone foo bar +' -test_expect_success \ - 'successful clone must leave the directory' \ - 'test -d bar' +test_expect_success 'successful clone must leave the directory' ' + test_path_is_dir bar +' test_expect_success 'failed clone --separate-git-dir should not leave any directories' ' + test_when_finished "rmdir foo/.git/objects.bak" && mkdir foo/.git/objects.bak/ && + test_when_finished "mv foo/.git/objects.bak/* foo/.git/objects/" && mv foo/.git/objects/* foo/.git/objects.bak/ && test_must_fail git clone --separate-git-dir gitdir foo worktree && - test_must_fail test -e gitdir && - test_must_fail test -e worktree && - mv foo/.git/objects.bak/* foo/.git/objects/ && - rmdir foo/.git/objects.bak + test_path_is_missing gitdir && + test_path_is_missing worktree ' test_done -- cgit v1.2.3 From d45420c1c8612f085f1901c33ff6f0ccfbb72d3b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 2 Jan 2018 16:11:39 -0500 Subject: clone: do not clean up directories we didn't create Once upon a time, git-clone would refuse to write into a directory that it did not itself create. The cleanup routines for a failed clone could therefore just remove the git and worktree dirs completely. In 55892d2398 (Allow cloning to an existing empty directory, 2009-01-11), we learned to write into an existing directory. Which means that doing: mkdir foo git clone will-fail foo ends up deleting foo. This isn't a huge catastrophe, since by definition foo must be empty. But it's somewhat confusing; we should leave the filesystem as we found it. Because we know that the only directory we'll write into is an empty one, we can handle this case by just passing the KEEP_TOPLEVEL flag to our recursive delete (if we could write into populated directories, we'd have to keep track of what we wrote and what we did not, which would be much harder). Note that we need to handle the work-tree and git-dir separately, though, as only one might exist (and the new tests in t5600 cover all cases). Reported-by: Stephan Janssen Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t5600-clone-fail-cleanup.sh | 56 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 5 deletions(-) (limited to 't') diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh index 7b2a8052f8..4a1a912e03 100755 --- a/t/t5600-clone-fail-cleanup.sh +++ b/t/t5600-clone-fail-cleanup.sh @@ -7,10 +7,21 @@ 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.' +remove the directory before attempting a clone again. + +Unless the directory already exists, in which case we clean up only what we +wrote. +' . ./test-lib.sh +corrupt_repo () { + test_when_finished "rmdir foo/.git/objects.bak" && + mkdir foo/.git/objects.bak/ && + test_when_finished "mv foo/.git/objects.bak/* foo/.git/objects/" && + mv foo/.git/objects/* foo/.git/objects.bak/ +} + test_expect_success 'clone of non-existent source should fail' ' test_must_fail git clone foo bar ' @@ -42,13 +53,48 @@ test_expect_success 'successful clone must leave the directory' ' ' test_expect_success 'failed clone --separate-git-dir should not leave any directories' ' - test_when_finished "rmdir foo/.git/objects.bak" && - mkdir foo/.git/objects.bak/ && - test_when_finished "mv foo/.git/objects.bak/* foo/.git/objects/" && - mv foo/.git/objects/* foo/.git/objects.bak/ && + corrupt_repo && test_must_fail git clone --separate-git-dir gitdir foo worktree && test_path_is_missing gitdir && test_path_is_missing worktree ' +test_expect_success 'failed clone into empty leaves directory (vanilla)' ' + mkdir -p empty && + corrupt_repo && + test_must_fail git clone foo empty && + test_dir_is_empty empty +' + +test_expect_success 'failed clone into empty leaves directory (bare)' ' + mkdir -p empty && + corrupt_repo && + test_must_fail git clone --bare foo empty && + test_dir_is_empty empty +' + +test_expect_success 'failed clone into empty leaves directory (separate)' ' + mkdir -p empty-git empty-wt && + corrupt_repo && + test_must_fail git clone --separate-git-dir empty-git foo empty-wt && + test_dir_is_empty empty-git && + test_dir_is_empty empty-wt +' + +test_expect_success 'failed clone into empty leaves directory (separate, git)' ' + mkdir -p empty-git && + corrupt_repo && + test_must_fail git clone --separate-git-dir empty-git foo no-wt && + test_dir_is_empty empty-git && + test_path_is_missing no-wt +' + +test_expect_success 'failed clone into empty leaves directory (separate, wt)' ' + mkdir -p empty-wt && + corrupt_repo && + test_must_fail git clone --separate-git-dir no-git foo empty-wt && + test_path_is_missing no-git && + test_dir_is_empty empty-wt +' + test_done -- cgit v1.2.3