Skip to content

Commit 01cd253

Browse files
dschodscho
authored
Merge pull request #1421 from isaac-s/fix-clone-removing-dest
When dest dir is specified and clone breaks for any reason, don't remove the dest dir
2 parents 5bb0817 + b50a218 commit 01cd253

File tree

2 files changed

+98
-33
lines changed

2 files changed

+98
-33
lines changed

‎builtin/clone.c‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,9 @@ static void clone_local(const char *src_repo, const char *dest_repo)
472472
}
473473

474474
static const char *junk_work_tree;
475+
static int junk_work_tree_flags;
475476
static const char *junk_git_dir;
477+
static int junk_git_dir_flags;
476478
static enum {
477479
JUNK_LEAVE_NONE,
478480
JUNK_LEAVE_REPO,
@@ -501,12 +503,12 @@ static void remove_junk(void)
501503

502504
if (junk_git_dir) {
503505
strbuf_addstr(&sb, junk_git_dir);
504-
remove_dir_recursively(&sb, 0);
506+
remove_dir_recursively(&sb, junk_git_dir_flags);
505507
strbuf_reset(&sb);
506508
}
507509
if (junk_work_tree) {
508510
strbuf_addstr(&sb, junk_work_tree);
509-
remove_dir_recursively(&sb, 0);
511+
remove_dir_recursively(&sb, junk_work_tree_flags);
510512
}
511513
strbuf_release(&sb);
512514
}
@@ -863,10 +865,15 @@ static void dissociate_from_references(void)
863865
free(alternates);
864866
}
865867

868+
static int dir_exists(const char *path)
869+
{
870+
struct stat sb;
871+
return !stat(path, &sb);
872+
}
873+
866874
int cmd_clone(int argc, const char **argv, const char *prefix)
867875
{
868876
int is_bundle = 0, is_local;
869-
struct stat buf;
870877
const char *repo_name, *repo, *work_tree, *git_dir;
871878
char *path, *dir;
872879
int dest_exists;
@@ -940,7 +947,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
940947
dir = guess_dir_name(repo_name, is_bundle, option_bare);
941948
strip_trailing_slashes(dir);
942949

943-
dest_exists = !stat(dir, &buf);
950+
dest_exists = dir_exists(dir);
944951
if (dest_exists && !is_empty_dir(dir))
945952
die(_("destination path '%s' already exists and is not "
946953
"an empty directory."), dir);
@@ -951,7 +958,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
951958
work_tree = NULL;
952959
else {
953960
work_tree = getenv("GIT_WORK_TREE");
954-
if (work_tree && !stat(work_tree, &buf))
961+
if (work_tree && dir_exists(work_tree))
955962
die(_("working tree '%s' already exists."), work_tree);
956963
}
957964

@@ -969,14 +976,24 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
969976
if (safe_create_leading_directories_const(work_tree) < 0)
970977
die_errno(_("could not create leading directories of '%s'"),
971978
work_tree);
972-
if (!dest_exists && mkdir(work_tree, 0777))
979+
if (dest_exists)
980+
junk_work_tree_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
981+
else if (mkdir(work_tree, 0777))
973982
die_errno(_("could not create work tree dir '%s'"),
974983
work_tree);
975984
junk_work_tree = work_tree;
976985
set_git_work_tree(work_tree);
977986
}
978987

979-
junk_git_dir = real_git_dir ? real_git_dir : git_dir;
988+
if (real_git_dir) {
989+
if (dir_exists(real_git_dir))
990+
junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
991+
junk_git_dir = real_git_dir;
992+
} else {
993+
if (dest_exists)
994+
junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
995+
junk_git_dir = git_dir;
996+
}
980997
if (safe_create_leading_directories_const(git_dir) < 0)
981998
die(_("could not create leading directories of '%s'"), git_dir);
982999

‎t/t5600-clone-fail-cleanup.sh‎

Lines changed: 74 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,46 +7,94 @@ test_description='test git clone to cleanup after failure
77
88
This test covers the fact that if git clone fails, it should remove
99
the directory it created, to avoid the user having to manually
10-
remove the directory before attempting a clone again.'
10+
remove the directory before attempting a clone again.
11+
12+
Unless the directory already exists, in which case we clean up only what we
13+
wrote.
14+
'
1115

1216
. ./test-lib.sh
1317

14-
test_expect_success \
15-
'clone of non-existent source should fail' \
16-
'test_must_fail git clone foo bar'
18+
corrupt_repo () {
19+
test_when_finished "rmdir foo/.git/objects.bak" &&
20+
mkdir foo/.git/objects.bak/ &&
21+
test_when_finished "mv foo/.git/objects.bak/* foo/.git/objects/" &&
22+
mv foo/.git/objects/* foo/.git/objects.bak/
23+
}
1724

18-
test_expect_success \
19-
'failed clone should not leave a directory' \
20-
'! test -d bar'
25+
test_expect_success 'clone of non-existent source should fail' '
26+
test_must_fail git clone foo bar
27+
'
2128

22-
# Need a repo to clone
23-
test_create_repo foo
29+
test_expect_success 'failed clone should not leave a directory' '
30+
test_path_is_missing bar
31+
'
2432

25-
# clone doesn't like it if there is no HEAD. Is that a bug?
26-
(cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1)
33+
test_expect_success 'create a repo to clone' '
34+
test_create_repo foo
35+
'
36+
37+
test_expect_success 'create objects in repo for later corruption' '
38+
test_commit -C foo file
39+
'
2740

2841
# source repository given to git clone should be relative to the
2942
# current path not to the target dir
30-
test_expect_success \
31-
'clone of non-existent (relative to $PWD) source should fail' \
32-
'test_must_fail git clone ../foo baz'
43+
test_expect_success 'clone of non-existent (relative to $PWD) source should fail' '
44+
test_must_fail git clone ../foo baz
45+
'
3346

34-
test_expect_success \
35-
'clone should work now that source exists' \
36-
'git clone foo bar'
47+
test_expect_success 'clone should work now that source exists' '
48+
git clone foo bar
49+
'
3750

38-
test_expect_success \
39-
'successful clone must leave the directory' \
40-
'test -d bar'
51+
test_expect_success 'successful clone must leave the directory' '
52+
test_path_is_dir bar
53+
'
4154

4255
test_expect_success 'failed clone --separate-git-dir should not leave any directories' '
43-
mkdir foo/.git/objects.bak/ &&
44-
mv foo/.git/objects/* foo/.git/objects.bak/ &&
56+
corrupt_repo &&
4557
test_must_fail git clone --separate-git-dir gitdir foo worktree &&
46-
test_must_fail test -e gitdir &&
47-
test_must_fail test -e worktree &&
48-
mv foo/.git/objects.bak/* foo/.git/objects/ &&
49-
rmdir foo/.git/objects.bak
58+
test_path_is_missing gitdir &&
59+
test_path_is_missing worktree
60+
'
61+
62+
test_expect_success 'failed clone into empty leaves directory (vanilla)' '
63+
mkdir -p empty &&
64+
corrupt_repo &&
65+
test_must_fail git clone foo empty &&
66+
test_dir_is_empty empty
67+
'
68+
69+
test_expect_success 'failed clone into empty leaves directory (bare)' '
70+
mkdir -p empty &&
71+
corrupt_repo &&
72+
test_must_fail git clone --bare foo empty &&
73+
test_dir_is_empty empty
74+
'
75+
76+
test_expect_success 'failed clone into empty leaves directory (separate)' '
77+
mkdir -p empty-git empty-wt &&
78+
corrupt_repo &&
79+
test_must_fail git clone --separate-git-dir empty-git foo empty-wt &&
80+
test_dir_is_empty empty-git &&
81+
test_dir_is_empty empty-wt
82+
'
83+
84+
test_expect_success 'failed clone into empty leaves directory (separate, git)' '
85+
mkdir -p empty-git &&
86+
corrupt_repo &&
87+
test_must_fail git clone --separate-git-dir empty-git foo no-wt &&
88+
test_dir_is_empty empty-git &&
89+
test_path_is_missing no-wt
90+
'
91+
92+
test_expect_success 'failed clone into empty leaves directory (separate, wt)' '
93+
mkdir -p empty-wt &&
94+
corrupt_repo &&
95+
test_must_fail git clone --separate-git-dir no-git foo empty-wt &&
96+
test_path_is_missing no-git &&
97+
test_dir_is_empty empty-wt
5098
'
5199

52100
test_done

0 commit comments

Comments
 (0)