* Re: [PATCH] dir: skip excluded directory with nested repo on prefix match
2026-09-18 11:04 ` [PATCH] dir: skip excluded directory with nested repo on prefix match René Scharfe
@ 2026-09-22 4:57 ` Junio C Hamano
2026-09-26 10:51 ` René Scharfe
2026-09-26 10:48 ` [PATCH v2] " René Scharfe
1 sibling, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2026-09-22 4:57 UTC (permalink / raw)
To: René Scharfe
Cc: git, 81625, Lester Longley, Sean Whitton, Elijah Newren,
Martin Ågren
René Scharfe <l.s.r@web.de> writes:
> +static int dir_match(struct index_state *istate,
> + const struct pathspec *pathspec,
> + const char *dirname, int len)
> +{
> + return match_pathspec_with_flags(istate, pathspec, dirname, len,
> + 0 /* prefix */,
> + NULL /* seen */,
> + DO_MATCH_LEADING_PATHSPEC);
> +}
OK, this is a good helper to extract and reuse.
> @@ -1996,11 +2006,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
> * for matching patterns.
> */
> if (pathspec && !excluded) {
> - matches_how = match_pathspec_with_flags(istate, pathspec,
> - dirname, len,
> - 0 /* prefix */,
> - NULL /* seen */,
> - DO_MATCH_LEADING_PATHSPEC);
> + matches_how = dir_match(istate, pathspec, dirname, len);
> if (!matches_how)
> return path_none;
> }
So, this hunk shows that the treat_directory() function is prepared
to be passed a NULL in pathspec. We make sure we do not trigger
match_pathspec_with_flags() when pathspec is NULL, and this should
extends to dir_match() now. This is a very sensible conditional, as
match_pathspec_with_flags() calls do_match_pathspec() as the first
thing, which begins with GUARD_PATHSPEC() macro that unconditionally
dereferences the pathspec.
> @@ -2034,8 +2040,13 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
> strbuf_release(&sb);
>
> if (nested_repo) {
> - if ((dir->flags & DIR_SKIP_NESTED_GIT) ||
> - (matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC))
> + if (dir->flags & DIR_SKIP_NESTED_GIT)
> + return path_none;
> + if (!matches_how)
> + matches_how = dir_match(istate, pathspec,
> + dirname, len);
> + if (!matches_how ||
> + matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
> return path_none;
Here, we do not know if we have pathspec==NULL. Shouldn't "make
sure we have a result from dir_match() and return path_none as
appropriate" be done only when pathspec != NULL or something like
that, since dir_match() will crash if pathspec is NULL?
Taking all together, something along the following line squashed
into your patch, perhaps? The newly added test is not essential; it
merely is to demonstrate why an extra conditional I added below
would help avoid segfaulting.
dir.c | 15 +++++++++------
t/t7061-wtstatus-ignore.sh | 25 +++++++++++++++++++++++++
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git c/dir.c w/dir.c
index 6bda650891..8e858c26d1 100644
--- c/dir.c
+++ w/dir.c
@@ -2042,12 +2042,15 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
if (nested_repo) {
if (dir->flags & DIR_SKIP_NESTED_GIT)
return path_none;
- if (!matches_how)
- matches_how = dir_match(istate, pathspec,
- dirname, len);
- if (!matches_how ||
- matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
- return path_none;
+
+ if (pathspec) {
+ if (!matches_how)
+ matches_how = dir_match(istate, pathspec,
+ dirname, len);
+ if (!matches_how ||
+ matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
+ return path_none;
+ }
return excluded ? path_excluded : path_untracked;
}
}
diff --git c/t/t7061-wtstatus-ignore.sh w/t/t7061-wtstatus-ignore.sh
index 182933471f..6bc680312e 100755
--- c/t/t7061-wtstatus-ignore.sh
+++ w/t/t7061-wtstatus-ignore.sh
@@ -340,6 +340,31 @@ test_expect_success 'status ignores submodule in excluded directory' '
test_cmp expected actual
'
+test_expect_success 'replace directory with untracked nested repo' '
+ test_create_repo checkout-nested &&
+ (
+ cd checkout-nested &&
+ test_commit base &&
+
+ # Branch with tracked file "dir"
+ git checkout -b branch-file &&
+ echo "tracked file" >dir &&
+ git add dir &&
+ git commit -m "add file dir" &&
+
+ # Switch back to base
+ git checkout -b branch-nested base &&
+
+ # Create an untracked directory containing a nested git repo
+ mkdir -p dir/nested &&
+ git init dir/nested &&
+
+ # Attempting checkout invokes verify_clean_subdirectory()
+ # with pathspec=NULL
+ test_must_fail git checkout branch-file
+ )
+'
+
test_expect_success 'status omits excluded directory with submodule on prefix match' '
git status --porcelain --ignored -u tracke >actual &&
test_must_be_empty actual
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2] dir: skip excluded directory with nested repo on prefix match
2026-09-18 11:04 ` [PATCH] dir: skip excluded directory with nested repo on prefix match René Scharfe
2026-09-22 4:57 ` Junio C Hamano
@ 2026-09-26 10:48 ` René Scharfe
1 sibling, 0 replies; 6+ messages in thread
From: René Scharfe @ 2026-09-26 10:48 UTC (permalink / raw)
To: git
Cc: 81625, Lester Longley, Sean Whitton, Elijah Newren,
Martin Ågren, Junio C Hamano
95c11ecc73 (Fix error-prone fill_directory() API; make it only return
matches, 2020-04-01) optimized away pathspec matching too eagerly and
cada7308ad (dir: check pathspecs before returning `path_excluded`,
2020-07-20) fixed that for files and symlinks.
A corner case remained unaddressed: Ignored directories that contain a
nested repository. Make sure to run match_pathspec_with_flags() for it
if necessary.
Add a status test to check whether the pathspec is applied correctly as
well as a checkout test to exercise the changed code without a pathspec.
Reported-by: Sean Whitton <spwhitton@spwhitton.name>
Reported-by: Lester Longley <lester@ieee.org>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
dir.c | 27 ++++++++++++++++++++-------
t/t2021-checkout-overwrite.sh | 7 +++++++
t/t7061-wtstatus-ignore.sh | 5 +++++
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/dir.c b/dir.c
index d896e7be4b..ac246ced1d 100644
--- a/dir.c
+++ b/dir.c
@@ -1935,6 +1935,16 @@ static enum exist_status directory_exists_in_index(struct index_state *istate,
return index_nonexistent;
}
+static int dir_match(struct index_state *istate,
+ const struct pathspec *pathspec,
+ const char *dirname, int len)
+{
+ return match_pathspec_with_flags(istate, pathspec, dirname, len,
+ 0 /* prefix */,
+ NULL /* seen */,
+ DO_MATCH_LEADING_PATHSPEC);
+}
+
/*
* When we find a directory when traversing the filesystem, we
* have three distinct cases:
@@ -2001,11 +2011,7 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
* for matching patterns.
*/
if (pathspec && !excluded) {
- matches_how = match_pathspec_with_flags(istate, pathspec,
- dirname, len,
- 0 /* prefix */,
- NULL /* seen */,
- DO_MATCH_LEADING_PATHSPEC);
+ matches_how = dir_match(istate, pathspec, dirname, len);
if (!matches_how)
return path_none;
}
@@ -2039,8 +2045,15 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
strbuf_release(&sb);
if (nested_repo) {
- if ((dir->flags & DIR_SKIP_NESTED_GIT) ||
- (matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC))
+ if (dir->flags & DIR_SKIP_NESTED_GIT)
+ return path_none;
+ if (pathspec && !matches_how) {
+ matches_how = dir_match(istate, pathspec,
+ dirname, len);
+ if (!matches_how)
+ return path_none;
+ }
+ if (matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
return path_none;
return excluded ? path_excluded : path_untracked;
}
diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index 38c41ae373..2b7fff5159 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -79,4 +79,11 @@ test_expect_success 'checkout --overwrite-ignore should succeed if only ignored
test_path_is_file some_dir
'
+test_expect_success 'checkout must not overwrite untracked nested repo' '
+ git checkout -f start &&
+ rm -rf some_dir &&
+ git init some_dir &&
+ test_must_fail git checkout df_conflict
+'
+
test_done
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 14ddaba2f3..182933471f 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -340,4 +340,9 @@ test_expect_success 'status ignores submodule in excluded directory' '
test_cmp expected actual
'
+test_expect_success 'status omits excluded directory with submodule on prefix match' '
+ git status --porcelain --ignored -u tracke >actual &&
+ test_must_be_empty actual
+'
+
test_done
Interdiff against v1:
diff --git a/dir.c b/dir.c
index aa457f0f4e..ac246ced1d 100644
--- a/dir.c
+++ b/dir.c
@@ -2047,11 +2047,13 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
if (nested_repo) {
if (dir->flags & DIR_SKIP_NESTED_GIT)
return path_none;
- if (!matches_how)
+ if (pathspec && !matches_how) {
matches_how = dir_match(istate, pathspec,
dirname, len);
- if (!matches_how ||
- matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
+ if (!matches_how)
+ return path_none;
+ }
+ if (matches_how == MATCHED_RECURSIVELY_LEADING_PATHSPEC)
return path_none;
return excluded ? path_excluded : path_untracked;
}
diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index 38c41ae373..2b7fff5159 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -79,4 +79,11 @@ test_expect_success 'checkout --overwrite-ignore should succeed if only ignored
test_path_is_file some_dir
'
+test_expect_success 'checkout must not overwrite untracked nested repo' '
+ git checkout -f start &&
+ rm -rf some_dir &&
+ git init some_dir &&
+ test_must_fail git checkout df_conflict
+'
+
test_done
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread