Git Mailing List Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content
@ 2020-07-01  9:36 Ben Wijen
  2020-07-01  9:36 ` [PATCH 1/2] git clone: check for non-empty directory Ben Wijen
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Ben Wijen @ 2020-07-01  9:36 UTC (permalink / raw)
  To: git; +Cc: Ben Wijen

Hi,

I found an issue with git clone, when using --separate-git-dir and that directory already exists,
it's content is destroyed.

I have created a patch set on origin/next, but (as I have learned this the hard way) please let me know
if this needs to be backported...


Ben...


Ben Wijen (2):
  git clone: check for non-empty directory
  git clone: don't clone into non-empty directory

 builtin/clone.c  | 12 ++++++++++--
 t/t5601-clone.sh |  3 ++-
 2 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] git clone: check for non-empty directory
  2020-07-01  9:36 [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Ben Wijen
@ 2020-07-01  9:36 ` Ben Wijen
  2020-07-01 16:00   ` Eric Sunshine
  2020-07-01  9:36 ` [PATCH 2/2] git clone: don't clone into " Ben Wijen
  2020-07-01 15:39 ` [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Eric Sunshine
  2 siblings, 1 reply; 14+ messages in thread
From: Ben Wijen @ 2020-07-01  9:36 UTC (permalink / raw)
  To: git; +Cc: Ben Wijen

When using git clone with --separate-git-dir realgitdir and
realgitdir already exists, it's content is destroyed.

Extend test to make sure content is left intact

Signed-off-by: Ben Wijen <ben@wijen.net>
---
 t/t5601-clone.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 84ea2a3eb7..03901c55f2 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -271,7 +271,8 @@ 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
+	test_must_fail git clone --separate-git-dir realgitdir src dst &&
+	test -f realgitdir/config
 '
 
 test_expect_success 'clone --reference from original' '
-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] git clone: don't clone into non-empty directory
  2020-07-01  9:36 [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Ben Wijen
  2020-07-01  9:36 ` [PATCH 1/2] git clone: check for non-empty directory Ben Wijen
@ 2020-07-01  9:36 ` Ben Wijen
  2020-07-01 16:10   ` Eric Sunshine
  2020-07-01 15:39 ` [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Eric Sunshine
  2 siblings, 1 reply; 14+ messages in thread
From: Ben Wijen @ 2020-07-01  9:36 UTC (permalink / raw)
  To: git; +Cc: Ben Wijen

When using git clone with --separate-git-dir realgitdir and
realgitdir already exists, it's content is destroyed.

Make sure we don't clone into an existing non-empty directory.

Signed-off-by: Ben Wijen <ben@wijen.net>
---
 builtin/clone.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index bef70745c0..54a91acf06 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(_("destination 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 {
-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content
  2020-07-01  9:36 [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Ben Wijen
  2020-07-01  9:36 ` [PATCH 1/2] git clone: check for non-empty directory Ben Wijen
  2020-07-01  9:36 ` [PATCH 2/2] git clone: don't clone into " Ben Wijen
@ 2020-07-01 15:39 ` Eric Sunshine
  2020-07-02  8:13   ` [PATCH v2 0/1] git clone: don't clone into non-empty directory Ben Wijen
  2020-07-02  8:13   ` [PATCH v2 1/1] " Ben Wijen
  2 siblings, 2 replies; 14+ messages in thread
From: Eric Sunshine @ 2020-07-01 15:39 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Git List

On Wed, Jul 1, 2020 at 5:46 AM Ben Wijen <ben@wijen.net> wrote:
> I found an issue with git clone, when using --separate-git-dir and that directory already exists,
> it's content is destroyed.

Thanks for fixing this.

> I have created a patch set on origin/next, but (as I have learned this the hard way) please let me know
> if this needs to be backported...

Does this patch series depend on certain topics in 'next'? If not,
please base it on 'master' instead. This will allow it to graduate
from 'seen' to 'next' and eventually 'master' without being held back
by any other topics in flight. On the other hand, if it does depend
upon some topic in 'next', then it is a good idea to specify which
specific topics it needs. That way, your patches will be able to
graduate after those specific topics graduate rather than having to
wait until _all_ of 'next' graduates (which could take forever).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] git clone: check for non-empty directory
  2020-07-01  9:36 ` [PATCH 1/2] git clone: check for non-empty directory Ben Wijen
@ 2020-07-01 16:00   ` Eric Sunshine
  2020-07-01 17:40     ` Eric Sunshine
  2020-07-02  5:32     ` Eric Sunshine
  0 siblings, 2 replies; 14+ messages in thread
From: Eric Sunshine @ 2020-07-01 16:00 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Git List

On Wed, Jul 1, 2020 at 5:46 AM Ben Wijen <ben@wijen.net> wrote:
> When using git clone with --separate-git-dir realgitdir and
> realgitdir already exists, it's content is destroyed.
>
> Extend test to make sure content is left intact
>
> Signed-off-by: Ben Wijen <ben@wijen.net>
> ---
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> @@ -271,7 +271,8 @@ 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
> +       test_must_fail git clone --separate-git-dir realgitdir src dst &&
> +       test -f realgitdir/config
>  '

This addresses the immediate problem of the directory content being
destroyed, which is good. But, we can future-proof it even more by
also verifying (to some degree) that the existing content has not been
disturbed. Doing so would give us greater confidence against some
future change breaking "realgitdir" in some fashion other than merely
emptying it. For instance, we might do this:

    rm -rf dst &&
    echo foo=bar >realgitdir/config &&
    test_must_fail git clone --separate-git-dir realgitdir src dst &&
    grep foo=bar realgitdir/config

One other observation: To preserve bisectability (git-bisect) of
git.git itself, we want to ensure that the entire test suite still
passes at each point in a patch series. Making this change to the test
in its own patch introduces a failure into the test suite, which is
undesirable. One way to address this shortcoming would be to
temporarily change this test from 'test_must_succeed' to
'test_must_fail', and then flip it back to 'test_must_succeed' in
patch 2/2. A cleaner approach is simply to combine the two patches so
that the fix and updated test are bundled together (which makes sense
anyhow in this case since this patch is so short and need not stand on
its own).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] git clone: don't clone into non-empty directory
  2020-07-01  9:36 ` [PATCH 2/2] git clone: don't clone into " Ben Wijen
@ 2020-07-01 16:10   ` Eric Sunshine
  2020-07-02  7:50     ` Ben
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Sunshine @ 2020-07-01 16:10 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Git List

On Wed, Jul 1, 2020 at 5:46 AM Ben Wijen <ben@wijen.net> wrote:
> When using git clone with --separate-git-dir realgitdir and
> realgitdir already exists, it's content is destroyed.
>
> Make sure we don't clone into an existing non-empty directory.
>
> Signed-off-by: Ben Wijen <ben@wijen.net>
> ---
> diff --git a/builtin/clone.c b/builtin/clone.c
> @@ -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))

"git init --separate-git-dir=" checks only whether the path exists,
and errors out if it does; it doesn't care whether the directory is
empty or not. I'm wondering, therefore, if this check should be
tightened to more closely align the behavior of the two commands.
(Using the tighter semantic now doesn't prohibit loosening it in the
future, whereas it's harder to tighten behavior which started out
loose.)

> +                       die(_("destination path '%s' already exists and is not "
> +                               "an empty directory."), real_git_dir);

This message might be a bit clearer if "destination" is changed to
"repository". (And drop the bit about not being empty -- if we go that
route.)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] git clone: check for non-empty directory
  2020-07-01 16:00   ` Eric Sunshine
@ 2020-07-01 17:40     ` Eric Sunshine
  2020-07-02  5:32     ` Eric Sunshine
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Sunshine @ 2020-07-01 17:40 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Git List

On Wed, Jul 1, 2020 at 12:00 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> [...] For instance, we might do this:
>     rm -rf dst &&
>     echo foo=bar >realgitdir/config &&

I meant to use '>>' here, not '>'.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] git clone: check for non-empty directory
  2020-07-01 16:00   ` Eric Sunshine
  2020-07-01 17:40     ` Eric Sunshine
@ 2020-07-02  5:32     ` Eric Sunshine
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Sunshine @ 2020-07-02  5:32 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Git List

On Wed, Jul 1, 2020 at 12:00 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> One other observation: To preserve bisectability (git-bisect) of
> git.git itself, we want to ensure that the entire test suite still
> passes at each point in a patch series. Making this change to the test
> in its own patch introduces a failure into the test suite, which is
> undesirable. One way to address this shortcoming would be to
> temporarily change this test from 'test_must_succeed' to
> 'test_must_fail', and then flip it back to 'test_must_succeed' in
> patch 2/2. [...]

And another correction: I meant 'test_expect_success' and
'test_expect_failure', respectively.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] git clone: don't clone into non-empty directory
  2020-07-01 16:10   ` Eric Sunshine
@ 2020-07-02  7:50     ` Ben
  0 siblings, 0 replies; 14+ messages in thread
From: Ben @ 2020-07-02  7:50 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List

On 01-07-2020 18:10, Eric Sunshine wrote:
> On Wed, Jul 1, 2020 at 5:46 AM Ben Wijen <ben@wijen.net> wrote:
> 
> "git init --separate-git-dir=" checks only whether the path exists,
> and errors out if it does; it doesn't care whether the directory is
> empty or not. I'm wondering, therefore, if this check should be
> tightened to more closely align the behavior of the two commands.
> (Using the tighter semantic now doesn't prohibit loosening it in the
> future, whereas it's harder to tighten behavior which started out
> loose.)
> 

About `git init`, I did take a look at it, but saw 'exist_ok' is always
true when called from 'cmd_init_db' (see builtin/init_db.c:654)
This means `git init` always allows when git_dir/real_git_dir already exists,
which I understand for the worktree, but I doubt if one wants this for the
repository. (when --separate-git-dir is used)

Also, because 'exist_ok' is always true for `git init` and - with this patch -
`git clone` checks the git_dir/real_git_dir before the 'init_db' call I'm wondering if
the code in 'init_db' (builtin/init-db.c:394-400) can be removed. 

Then, even if we do take that route (no 'is_empty_dir', only 'path_exists') we must
also address junk_git_dir_flags, as REMOVE_DIR_KEEP_TOPLEVEL would invalidate
a second git clone.

So, as this patch only fixes the problem at hand, IMHO separate patch-sets should
be created for mentioned issues.

Ben...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 0/1] git clone: don't clone into non-empty directory
  2020-07-01 15:39 ` [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Eric Sunshine
@ 2020-07-02  8:13   ` Ben Wijen
  2020-07-02  8:13   ` [PATCH v2 1/1] " Ben Wijen
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Wijen @ 2020-07-02  8:13 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Ben Wijen

Eric, thank you for your review.

With this update I have:
* rebased (and squashed) on master
* included your proposals for the test
* changed "destination path..." to "repository path..."


Ben...

Ben Wijen (1):
  git clone: don't clone into non-empty directory

 builtin/clone.c  | 12 ++++++++++--
 t/t5601-clone.sh |  4 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/1] git clone: don't clone into non-empty directory
  2020-07-01 15:39 ` [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Eric Sunshine
  2020-07-02  8:13   ` [PATCH v2 0/1] git clone: don't clone into non-empty directory Ben Wijen
@ 2020-07-02  8:13   ` Ben Wijen
  2020-07-06 22:40     ` Junio C Hamano
  1 sibling, 1 reply; 14+ messages in thread
From: Ben Wijen @ 2020-07-02  8:13 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Ben Wijen

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

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' '
-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/1] git clone: don't clone into non-empty directory
  2020-07-02  8:13   ` [PATCH v2 1/1] " Ben Wijen
@ 2020-07-06 22:40     ` Junio C Hamano
  2020-07-10  8:47       ` [PATCH v3 0/1] " Ben Wijen
  2020-07-10  8:47       ` [PATCH v3 1/1] " Ben Wijen
  0 siblings, 2 replies; 14+ messages in thread
From: Junio C Hamano @ 2020-07-06 22:40 UTC (permalink / raw)
  To: Ben Wijen; +Cc: Eric Sunshine, Git List

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.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 0/1] git clone: don't clone into non-empty directory
  2020-07-06 22:40     ` Junio C Hamano
@ 2020-07-10  8:47       ` Ben Wijen
  2020-07-10  8:47       ` [PATCH v3 1/1] " Ben Wijen
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Wijen @ 2020-07-10  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Git List, Ben Wijen

Hi Junio,

I added a small note to the commit message.
Please let me know if you require more.

Thank you,


Ben Wijen (1):
  git clone: don't clone into non-empty directory

 builtin/clone.c  | 12 ++++++++++--
 t/t5601-clone.sh |  4 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/1] git clone: don't clone into non-empty directory
  2020-07-06 22:40     ` Junio C Hamano
  2020-07-10  8:47       ` [PATCH v3 0/1] " Ben Wijen
@ 2020-07-10  8:47       ` Ben Wijen
  1 sibling, 0 replies; 14+ messages in thread
From: Ben Wijen @ 2020-07-10  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Sunshine, Git List, Ben Wijen

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.

Note: #d45420c1: "clone: do not clean up directories we didn't create"
assumed we always write into an empty directory, but missed the check
for real_git_dir, this commit fixed it by adding the check.

Signed-off-by: Ben Wijen <ben@wijen.net>
---
 builtin/clone.c  | 12 ++++++++++--
 t/t5601-clone.sh |  4 +++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index bef70745c0..a9f3312a54 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' '
-- 
2.27.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01  9:36 [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Ben Wijen
2020-07-01  9:36 ` [PATCH 1/2] git clone: check for non-empty directory Ben Wijen
2020-07-01 16:00   ` Eric Sunshine
2020-07-01 17:40     ` Eric Sunshine
2020-07-02  5:32     ` Eric Sunshine
2020-07-01  9:36 ` [PATCH 2/2] git clone: don't clone into " Ben Wijen
2020-07-01 16:10   ` Eric Sunshine
2020-07-02  7:50     ` Ben
2020-07-01 15:39 ` [PATCH 0/2] git clone with --separate-git-dir destroys existing directory content Eric Sunshine
2020-07-02  8:13   ` [PATCH v2 0/1] git clone: don't clone into non-empty directory Ben Wijen
2020-07-02  8:13   ` [PATCH v2 1/1] " Ben Wijen
2020-07-06 22:40     ` Junio C Hamano
2020-07-10  8:47       ` [PATCH v3 0/1] " Ben Wijen
2020-07-10  8:47       ` [PATCH v3 1/1] " Ben Wijen

Git Mailing List Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/git/0 git/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 git git/ https://lore.kernel.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.git


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git