| Message ID | 20200702081335.16786-2-ben@wijen.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git clone: don't clone into non-empty directory | expand |
Ben Wijen <ben@wijen.net> writes: > When using git clone with --separate-git-dir realgitdir and > realgitdir already exists, it's content is destroyed. > > So, make sure we don't clone into an existing non-empty directory. This came from d45420c1 (clone: do not clean up directories we didn't create, 2018-01-02), where we claimed 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). The assumed use case is that somebody created an empty directory for our use in a grandparent directory where we have no write permission and gave it to us, and we want to do a "git clone" into it while keeping that directory alive. But we didn't make sure the directory was not empty ourselves---we just assumed it to be empty. And the originally envisioned use case does not have anything to do with the use of --separate-git-dir at all. So from that point of view, this change does not break the originally envisioned use case, so this probably is not regressing any existing and valid use case that may not involve "--separate-git-dir", but I'd rather see that the commit message explain these things to its readers to justify itself. Thanks.
diff --git a/builtin/clone.c b/builtin/clone.c index 2a8e3aaaed..e470193bb8 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -946,7 +946,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) int is_bundle = 0, is_local; const char *repo_name, *repo, *work_tree, *git_dir; char *path, *dir, *display_repo = NULL; - int dest_exists; + int dest_exists, real_dest_exists = 0; const struct ref *refs, *remote_head; const struct ref *remote_head_points_at; const struct ref *our_head_points_at; @@ -1021,6 +1021,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix) die(_("destination path '%s' already exists and is not " "an empty directory."), dir); + if (real_git_dir) { + real_dest_exists = path_exists(real_git_dir); + if (real_dest_exists && !is_empty_dir(real_git_dir)) + die(_("repository path '%s' already exists and is not " + "an empty directory."), real_git_dir); + } + + strbuf_addf(&reflog_msg, "clone: from %s", display_repo ? display_repo : repo); free(display_repo); @@ -1057,7 +1065,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) } if (real_git_dir) { - if (path_exists(real_git_dir)) + if (real_dest_exists) junk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL; junk_git_dir = real_git_dir; } else { diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 84ea2a3eb7..eb9a093e25 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -271,7 +271,9 @@ test_expect_success 'fetch from gitfile parent' ' test_expect_success 'clone separate gitdir where target already exists' ' rm -rf dst && - test_must_fail git clone --separate-git-dir realgitdir src dst + echo foo=bar >>realgitdir/config && + test_must_fail git clone --separate-git-dir realgitdir src dst && + grep foo=bar realgitdir/config ' test_expect_success 'clone --reference from original' '
When using git clone with --separate-git-dir realgitdir and realgitdir already exists, it's content is destroyed. So, make sure we don't clone into an existing non-empty directory. Signed-off-by: Ben Wijen <ben@wijen.net> --- builtin/clone.c | 12 ++++++++++-- t/t5601-clone.sh | 4 +++- 2 files changed, 13 insertions(+), 3 deletions(-)