[v3,02/13] unpack-trees: heed requests to overwrite ignored files

Commit Message

Elijah Newren Feb. 27, 2023, 3:28 p.m. UTC

From: Elijah Newren <[e-mail]>

When a directory exists but has only ignored files within it and we are
trying to switch to a branch that has a file where that directory is,
the behavior depends upon --[no]-overwrite-ignore.  If the user wants to
--overwrite-ignore (the default), then we should delete the ignored file
and directory and switch to the new branch.

The code to handle this in verify_clean_subdirectory() in unpack-trees
tried to handle this via paying attention to the exclude_per_dir setting
of the internal dir field.  This came from commit c81935348b ("Fix
switching to a branch with D/F when current branch has file D.",
2007-03-15), which pre-dated 039bc64e88 ("core.excludesfile clean-up",
2007-11-14), and thus did not pay attention to ignore patterns from
other relevant files.  Change it to use setup_standard_excludes() so
that it is also aware of excludes specified in other locations.

Signed-off-by: Elijah Newren <[e-mail]>
---
 t/t2021-checkout-overwrite.sh | 11 +++++++++++
 unpack-trees.c                |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

Comments

Jonathan Tan Feb. 27, 2023, 11:20 p.m. UTC | #1

"Elijah Newren via GitGitGadget" <[e-mail]> writes:
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3d05e45a279..4518d33ed99 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2337,7 +2337,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
>  
>  	memset(&d, 0, sizeof(d));
>  	if (o->dir)
> -		d.exclude_per_dir = o->dir->exclude_per_dir;
> +		setup_standard_excludes(&d);
>  	i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
>  	dir_clear(&d);
>  	free(pathbuf);

Thanks to the later patches in this patch set, I only needed to look at
unpack-trees.c to see how o->dir (later, o->internal.dir) is set. The
only place it is set is in unpack_trees(), in which a flag is set and
setup_standard_excludes() is called. So the RHS of the diff here does
effectively the same thing as the LHS. (As for the flag, it is not set
in the RHS, but it was not set in the LHS in the first place, so that's
fine.)

Thanks - all 13 patches in this patch set look good.

Patch

diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index baca66e1a31..034f62c13c5 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -69,4 +69,15 @@  test_expect_success SYMLINKS 'checkout -f must not follow symlinks when removing
 	test_path_is_file untracked/f
 '
 
+test_expect_success 'checkout --overwrite-ignore should succeed if only ignored files in the way' '
+	git checkout -b df_conflict &&
+	test_commit contents some_dir &&
+	git checkout start &&
+	mkdir some_dir &&
+	echo autogenerated information >some_dir/ignore &&
+	echo ignore >.git/info/exclude &&
+	git checkout --overwrite-ignore df_conflict &&
+	! test_path_is_dir some_dir
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 3d05e45a279..4518d33ed99 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2337,7 +2337,7 @@  static int verify_clean_subdirectory(const struct cache_entry *ce,
 
 	memset(&d, 0, sizeof(d));
 	if (o->dir)
-		d.exclude_per_dir = o->dir->exclude_per_dir;
+		setup_standard_excludes(&d);
 	i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
 	dir_clear(&d);
 	free(pathbuf);