Re: git 2.34.0: Behavior of `**` in gitignore is different from previous versions.
On 11/18/2021 11:01 PM, Danial Alihosseini wrote:
Show 16 quoted lines
> Thanks for your follow-up.
>
> I wanted to ignore all files in the "data" folder except ".txt" ones.
>
> As mentioned in the gitignore doc, there should be a difference
> between "**" and "**/".
>
>> A trailing "/**" matches everything inside. For example, "abc/**" matches all files inside directory "abc", relative to the location of the .gitignore file, with infinite depth.
>
> and,
>>
>> A slash followed by two consecutive asterisks then a slash matches zero or more directories. For example, "a/**/b" matches "a/b", "a/x/b", "a/x/y/b" and so on.
>
> and also,
>>
>> It is not possible to re-include a file if a parent directory of that file is excluded.
So this is the key point of why you can't just omit the !data/**/ line.
What is unclear to me is what exactly "match a directory" means. If we ignore a directory, then we ignore everything inside it (until another pattern says we should care about it), but the converse should also hold: if we have a pattern like "!data/**/", then that should mean "include everything inside data/<A>/ where <A> is any directory name".
My inability to form a mental model where the existing behavior matches the documented specification is an indicator that this was changed erroneously. A revert patch is included at the end of this message.
If anyone could help clarify my understanding here, then maybe there is room for improving the documentation.
Show 26 quoted lines
> So, I excluded all files by "data/**", re-included just directories
> (at any depth) by "!data/**/" and, re-included ".txt" files (at any
> depth) by "!data/**/*.txt".
> However, if I wanted to re-include a directory and all of its
> contents, I could use something like "!data/data1/**", without
> trailing slash.
> As I see, there is no separation between "**" and "**/" in the current version.
>
> I think there is another point that the previous behavior test case
> should be like this:
> test_expect_success 'directories and ** matches' '
> cat >.gitignore <<-\EOF &&
> data/**
> !data/**/
> !data/**/*.txt
> EOF
> git check-ignore file \
> data/file data/data1/file1 data/data1/file1.txt \
> data/data2/file2 data/data2/file2.txt >actual &&
> cat >expect <<-\EOF &&
> data/file
> data/data1/file1
> data/data2/file2
> EOF
> test_cmp expect actual
> '
Thank you for seeing my poorly-edited test and understanding what it _should_ have been.
Here is a revert including this new test. I further tested with the Scalar functional tests [1] to see if I could find what I was thinking when making this original change. Since this is so similar to another revert we caught before the 2.34.0 release, I regret not checking adjacent commits for a similar "reverting does not affect the test suite" case.
---- >8 ----
From f833967da83ecd51090ebb75a4b8173a775d16f1 Mon Sep 17 00:00:00 2001
From: Derrick Stolee <dstolee@microsoft.com>
Date: Fri, 19 Nov 2021 09:13:49 -0500
Subject: [PATCH] dir: revert "dir: select directories correctly"
This reverts commit f6526728f950cacfd5b5e42bcc65f2c47f3da654.
The change in f652672 (dir: select directories correctly, 2021-09-24) caused a regression in directory-based matches with non-cone-mode patterns, especially for .gitignore patterns. A test is included to prevent this regression in the future.
The commit ed495847 (dir: fix pattern matching on dirs, 2021-09-24) was reverted in 5ceb663 (dir: fix directory-matching bug, 2021-11-02) for similar reasons. Neither commit changed tests, and tests added later in the series continue to pass when these commits are reverted.
Reported-by: Danial Alihosseini <danial.alihosseini@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
dir.c | 54 +++++-----------------------------------------
t/t0008-ignores.sh | 17 +++++++++++++++
2 files changed, 22 insertions(+), 49 deletions(-)
diff --git a/dir.c b/dir.c
index 9ea6cfe61c..86afa2eae0 100644
--- a/dir.c
+++ b/dir.c
@@ -1303,44 +1303,6 @@ int match_pathname(const char *pathname, int pathlen,
WM_PATHNAME) == 0;
}
-static int path_matches_dir_pattern(const char *pathname,
- int pathlen,
- struct strbuf **path_parent,
- int *dtype,
- struct path_pattern *pattern,
- struct index_state *istate)
-{
- if (!*path_parent) {
- char *slash;
- CALLOC_ARRAY(*path_parent, 1);
- strbuf_add(*path_parent, pathname, pathlen);
- slash = find_last_dir_sep((*path_parent)->buf);
-
- if (slash)
- strbuf_setlen(*path_parent, slash - (*path_parent)->buf);
- else
- strbuf_setlen(*path_parent, 0);
- }
-
- /*
- * If the parent directory matches the pattern, then we do not
- * need to check for dtype.
- */
- if ((*path_parent)->len &&
- match_pathname((*path_parent)->buf, (*path_parent)->len,
- pattern->base,
- pattern->baselen ? pattern->baselen - 1 : 0,
- pattern->pattern, pattern->nowildcardlen,
- pattern->patternlen, pattern->flags))
- return 1;
-
- *dtype = resolve_dtype(*dtype, istate, pathname, pathlen);
- if (*dtype != DT_DIR)
- return 0;
-
- return 1;
-}
-
/*
* Scan the given exclude list in reverse to see whether pathname
* should be ignored. The first match (i.e. the last on the list), if
@@ -1356,7 +1318,6 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
{
struct path_pattern *res = NULL; /* undecided */
int i;
- struct strbuf *path_parent = NULL;
if (!pl->nr)
return NULL; /* undefined */
@@ -1366,10 +1327,11 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
const char *exclude = pattern->pattern;
int prefix = pattern->nowildcardlen;
- if (pattern->flags & PATTERN_FLAG_MUSTBEDIR &&
- !path_matches_dir_pattern(pathname, pathlen, &path_parent,
- dtype, pattern, istate))
- continue;
+ if (pattern->flags & PATTERN_FLAG_MUSTBEDIR) {
+ *dtype = resolve_dtype(*dtype, istate, pathname, pathlen);
+ if (*dtype != DT_DIR)
+ continue;
+ }
if (pattern->flags & PATTERN_FLAG_NODIR) {
if (match_basename(basename,
@@ -1393,12 +1355,6 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
break;
}
}
-
- if (path_parent) {
- strbuf_release(path_parent);
- free(path_parent);
- }
-
return res;
}
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 46a5038fed..fce7c7b408 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -828,6 +828,23 @@ test_expect_success 'exact prefix matching (without root)' '
test_cmp expect actual
'
+test_expect_success 'directories and ** matches' '
+ cat >.gitignore <<-\EOF &&
+ data/**
+ !data/**/
+ !data/**/*.txt
+ EOF
+ git check-ignore file \
+ data/file data/data1/file1 data/data1/file1.txt \
+ data/data2/file2 data/data2/file2.txt >actual &&
+ cat >expect <<-\EOF &&
+ data/file
+ data/data1/file1
+ data/data2/file2
+ EOF
+ test_cmp expect actual
+'
+
############################################################################
#
# test whitespace handling
--
2.34.0.dirty
