Skip to content

Commit 1363914

Browse files
gitstergitster
committed
Merge branch 'jk/abort-clone-with-existing-dest' into maint
"git clone $there $here" is allowed even when here directory exists as long as it is an empty directory, but the command incorrectly removed it upon a failure of the operation. * jk/abort-clone-with-existing-dest: clone: do not clean up directories we didn't create clone: factor out dir_exists() helper t5600: modernize style t5600: fix outdated comment about unborn HEAD
2 parents ff19620 + d45420c commit 1363914

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
@@ -473,7 +473,9 @@ static void clone_local(const char *src_repo, const char *dest_repo)
473473
}
474474

475475
static const char *junk_work_tree;
476+
static int junk_work_tree_flags;
476477
static const char *junk_git_dir;
478+
static int junk_git_dir_flags;
477479
static enum {
478480
JUNK_LEAVE_NONE,
479481
JUNK_LEAVE_REPO,
@@ -502,12 +504,12 @@ static void remove_junk(void)
502504

503505
if (junk_git_dir) {
504506
strbuf_addstr(&sb, junk_git_dir);
505-
remove_dir_recursively(&sb, 0);
507+
remove_dir_recursively(&sb, junk_git_dir_flags);
506508
strbuf_reset(&sb);
507509
}
508510
if (junk_work_tree) {
509511
strbuf_addstr(&sb, junk_work_tree);
510-
remove_dir_recursively(&sb, 0);
512+
remove_dir_recursively(&sb, junk_work_tree_flags);
511513
}
512514
strbuf_release(&sb);
513515
}
@@ -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;
@@ -938,7 +945,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
938945
dir = guess_dir_name(repo_name, is_bundle, option_bare);
939946
strip_trailing_slashes(dir);
940947

941-
dest_exists = !stat(dir, &buf);
948+
dest_exists = dir_exists(dir);
942949
if (dest_exists && !is_empty_dir(dir))
943950
die(_("destination path '%s' already exists and is not "
944951
"an empty directory."), dir);
@@ -949,7 +956,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
949956
work_tree = NULL;
950957
else {
951958
work_tree = getenv("GIT_WORK_TREE");
952-
if (work_tree && !stat(work_tree, &buf))
959+
if (work_tree && dir_exists(work_tree))
953960
die(_("working tree '%s' already exists."), work_tree);
954961
}
955962

@@ -967,14 +974,24 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
967974
if (safe_create_leading_directories_const(work_tree) < 0)
968975
die_errno(_("could not create leading directories of '%s'"),
969976
work_tree);
970-
if (!dest_exists && mkdir(work_tree, 0777))
977+
if (dest_exists)
978+
junk_work_tree_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
979+
else if (mkdir(work_tree, 0777))
971980
die_errno(_("could not create work tree dir '%s'"),
972981
work_tree);
973982
junk_work_tree = work_tree;
974983
set_git_work_tree(work_tree);
975984
}
976985

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

‎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)