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
This commit is contained in:
commit
1363914a6a
@ -473,7 +473,9 @@ static void clone_local(const char *src_repo, const char *dest_repo)
|
||||
}
|
||||
|
||||
static const char *junk_work_tree;
|
||||
static int junk_work_tree_flags;
|
||||
static const char *junk_git_dir;
|
||||
static int junk_git_dir_flags;
|
||||
static enum {
|
||||
JUNK_LEAVE_NONE,
|
||||
JUNK_LEAVE_REPO,
|
||||
@ -502,12 +504,12 @@ static void remove_junk(void)
|
||||
|
||||
if (junk_git_dir) {
|
||||
strbuf_addstr(&sb, junk_git_dir);
|
||||
remove_dir_recursively(&sb, 0);
|
||||
remove_dir_recursively(&sb, junk_git_dir_flags);
|
||||
strbuf_reset(&sb);
|
||||
}
|
||||
if (junk_work_tree) {
|
||||
strbuf_addstr(&sb, junk_work_tree);
|
||||
remove_dir_recursively(&sb, 0);
|
||||
remove_dir_recursively(&sb, junk_work_tree_flags);
|
||||
}
|
||||
strbuf_release(&sb);
|
||||
}
|
||||
@ -863,10 +865,15 @@ static void dissociate_from_references(void)
|
||||
free(alternates);
|
||||
}
|
||||
|
||||
static int dir_exists(const char *path)
|
||||
{
|
||||
struct stat sb;
|
||||
return !stat(path, &sb);
|
||||
}
|
||||
|
||||
int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||
{
|
||||
int is_bundle = 0, is_local;
|
||||
struct stat buf;
|
||||
const char *repo_name, *repo, *work_tree, *git_dir;
|
||||
char *path, *dir;
|
||||
int dest_exists;
|
||||
@ -938,7 +945,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||
dir = guess_dir_name(repo_name, is_bundle, option_bare);
|
||||
strip_trailing_slashes(dir);
|
||||
|
||||
dest_exists = !stat(dir, &buf);
|
||||
dest_exists = dir_exists(dir);
|
||||
if (dest_exists && !is_empty_dir(dir))
|
||||
die(_("destination path '%s' already exists and is not "
|
||||
"an empty directory."), dir);
|
||||
@ -949,7 +956,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||
work_tree = NULL;
|
||||
else {
|
||||
work_tree = getenv("GIT_WORK_TREE");
|
||||
if (work_tree && !stat(work_tree, &buf))
|
||||
if (work_tree && dir_exists(work_tree))
|
||||
die(_("working tree '%s' already exists."), work_tree);
|
||||
}
|
||||
|
||||
@ -967,14 +974,24 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
|
||||
if (safe_create_leading_directories_const(work_tree) < 0)
|
||||
die_errno(_("could not create leading directories of '%s'"),
|
||||
work_tree);
|
||||
if (!dest_exists && mkdir(work_tree, 0777))
|
||||
if (dest_exists)
|
||||
junk_work_tree_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
|
||||
else if (mkdir(work_tree, 0777))
|
||||
die_errno(_("could not create work tree dir '%s'"),
|
||||
work_tree);
|
||||
junk_work_tree = work_tree;
|
||||
set_git_work_tree(work_tree);
|
||||
}
|
||||
|
||||
junk_git_dir = real_git_dir ? real_git_dir : git_dir;
|
||||
if (real_git_dir) {
|
||||
if (dir_exists(real_git_dir))
|
||||
junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
|
||||
junk_git_dir = real_git_dir;
|
||||
} else {
|
||||
if (dest_exists)
|
||||
junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;
|
||||
junk_git_dir = git_dir;
|
||||
}
|
||||
if (safe_create_leading_directories_const(git_dir) < 0)
|
||||
die(_("could not create leading directories of '%s'"), git_dir);
|
||||
|
||||
|
@ -7,46 +7,94 @@ 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
|
||||
|
||||
test_expect_success \
|
||||
'clone of non-existent source should fail' \
|
||||
'test_must_fail git clone foo bar'
|
||||
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 \
|
||||
'failed clone should not leave a directory' \
|
||||
'! test -d bar'
|
||||
test_expect_success 'clone of non-existent source should fail' '
|
||||
test_must_fail git clone foo bar
|
||||
'
|
||||
|
||||
# Need a repo to clone
|
||||
test_create_repo foo
|
||||
test_expect_success 'failed clone should not leave a directory' '
|
||||
test_path_is_missing bar
|
||||
'
|
||||
|
||||
# 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 'create a repo to clone' '
|
||||
test_create_repo foo
|
||||
'
|
||||
|
||||
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' '
|
||||
mkdir foo/.git/objects.bak/ &&
|
||||
mv foo/.git/objects/* foo/.git/objects.bak/ &&
|
||||
corrupt_repo &&
|
||||
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_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
|
||||
|
Loading…
Reference in New Issue
Block a user