Skip to contentSkip to conversation

Fix handling of matching nonbare repo vs. matching something under nonbare repo

PATCH New Elijah Newren via GitGitGadget 1 participant 3 messages Aug 12, 2020

lore
Share & tools
lore
#0 lore raw

cover Fix handling of matching nonbare repo vs. matching something under nonbare repo

This fixes the issue reported at
https://lore.kernel.org/git/CADjceoQxoL932W4mkfhG6VOgrQBhs9k6tXkWSkraKVPmUP+uCw@mail.gmail.com/
, a regression in git-2.27.0 (that wasn't reported until after 2.28.0). It involves running ls-files --others with globs when nonbare repositories are present.

Thanks to christian w for reporting, and Kyle Meyer for finding a way to simplify the testcase and bisecting. Very helpful.

Elijah Newren (2):
t3000: fix some test description typos
dir: avoid prematurely marking nonbare repositories as matches

 dir.c                      |  9 ++++++---
 t/t3000-ls-files-others.sh | 24 ++++++++++++++++++++----
 2 files changed, 26 insertions(+), 7 deletions(-)

base-commit: 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-829%2Fnewren%2Fls-files-wildmatch-nonbare-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-829/newren/ls-files-wildmatch-nonbare-v1
Pull-Request: https://github.com/git/git/pull/829

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/2 t3000: fix some test description typos

From: Elijah Newren <[email protected]>

There is no such flag as --o; it is either --others or -o.

---
 t/t3000-ls-files-others.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
t/t3000-ls-files-others.sh +4 -4
index ffdfb16f58..1b9327b780 100755
@@ -152,7 +152,7 @@ test_expect_success 'ls-files -o --directory with mix dir/file pathspecs' '
 	)
 '
 
-test_expect_success 'ls-files --o --directory with glob filetype match' '
+test_expect_success 'ls-files -o --directory with glob filetype match' '
 	(
 		cd nested &&
 
@@ -168,7 +168,7 @@ test_expect_success 'ls-files --o --directory with glob filetype match' '
 	)
 '
 
-test_expect_success 'ls-files --o --directory with mix of tracked states' '
+test_expect_success 'ls-files -o --directory with mix of tracked states' '
 	(
 		cd nested &&
 
@@ -184,7 +184,7 @@ test_expect_success 'ls-files --o --directory with mix of tracked states' '
 	)
 '
 
-test_expect_success 'ls-files --o --directory with glob filetype match only' '
+test_expect_success 'ls-files -o --directory with glob filetype match only' '
 	(
 		cd nested &&
 
@@ -198,7 +198,7 @@ test_expect_success 'ls-files --o --directory with glob filetype match only' '
 	)
 '
 
-test_expect_success 'ls-files --o --directory to get immediate paths under one dir only' '
+test_expect_success 'ls-files -o --directory to get immediate paths under one dir only' '
 	(
 		cd nested &&
 
Signature · 1 line
gitgitgadget
#2 lore raw

PATCH 2/2 dir: avoid prematurely marking nonbare repositories as matches

In reply to Elijah Newren via GitGitGadget

From: Elijah Newren <[email protected]>

Nonbare repositories are special directories. Unlike normal directories that we might recurse into to list the files they contain, nonbare repositories must themselves match and then we always report only on the nonbare repository directory itself and not on any of its contents.

Separately, when traversing directories to try to find untracked or excluded files, we often think in terms of paths either matching the specified pathspec, or not matching them. However, there is a special value that do_match_pathspec() uses named MATCHED_RECURSIVELY_LEADING_PATHSPEC which means "this directory does not match any pathspec BUT it is possible a file or directory underneath it does." That special value prevents us from prematurely thinking that some directory and everything under it is irrelevant, but also allows us to differentiate from "this is a match".

The combination of these two special cases was previously uncovered. Add a test to the testsuite to cover it, and make sure that we return a nonbare repository as a non-match if the best match it got was MATCHED_RECURSIVELY_LEADING_PATHSPEC.

---
 dir.c                      |  9 ++++++---
 t/t3000-ls-files-others.sh | 16 ++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)
dir.c +6 -3
index 1045cc9c6f..b5082ca05f 100644
@@ -1792,9 +1792,12 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
 		nested_repo = is_nonbare_repository_dir(&sb);
 		strbuf_release(&sb);
 	}
-	if (nested_repo)
-		return ((dir->flags & DIR_SKIP_NESTED_GIT) ? path_none :
-			(excluded ? path_excluded : path_untracked));
+	if (nested_repo) {
+		if ((dir->flags & DIR_SKIP_NESTED_GIT) ||
+		    (matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC))
+			return path_none;
+		return excluded ? path_excluded : path_untracked;
+	}
 
 	if (!(dir->flags & DIR_SHOW_OTHER_DIRECTORIES)) {
 		if (excluded &&
t/t3000-ls-files-others.sh +16 -0
index 1b9327b780..740ce56eab 100755
@@ -212,4 +212,20 @@ test_expect_success 'ls-files -o --directory to get immediate paths under one di
 	)
 '
 
+test_expect_success 'ls-files -o avoids listing untracked non-matching gitdir' '
+	test_when_finished "rm -rf nested/untracked/deep/empty" &&
+	(
+		cd nested &&
+
+		git init untracked/deep/empty &&
+		git ls-files --others "untracked/*.c" >actual &&
+
+		cat <<-EOF >expect &&
+		untracked/deep/foo.c
+		EOF
+
+		test_cmp expect actual
+	)
+'
+
 test_done
Signature · 1 line
gitgitgadget