[PATCH v2] Dir: Fix and test wildcard pathspec handling

[PATCH v2] Dir: Fix and test wildcard pathspec handling

[K Jayatheerth]

Ensure wildcards expand, even with literal file match.
Fixes 'git add f*' skipping files like 'foo' if 'f*' exists.
Use 'f\*' to add the literal.
Tests added for add and commit where dir.c logic applies.
Skips windows specific test.

reported-by: piotrsiupa <piotrsiupa@gmail.com>
Mentored-by: Jeff King <peff@peff.net>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 dir.c                                 |   3 +-
 t/meson.build                         |   1 +
 t/t6137-pathspec-wildcards-literal.sh | 282 ++++++++++++++++++++++++++
 3 files changed, 285 insertions(+), 1 deletion(-)
 create mode 100755 t/t6137-pathspec-wildcards-literal.sh

diff --git a/dir.c b/dir.c
index 28b0e03feb..9405fee83a 100644
--- a/dir.c
+++ b/dir.c
@@ -519,7 +519,8 @@ static int do_match_pathspec(struct index_state *istate,
 		    ( exclude && !(ps->items[i].magic & PATHSPEC_EXCLUDE)))
 			continue;
 
-		if (seen && seen[i] == MATCHED_EXACTLY)
+		if (seen && seen[i] == MATCHED_EXACTLY &&
+			ps->items[i].nowildcard_len == ps->items[i].len)
 			continue;
 		/*
 		 * Make exclude patterns optional and never report
diff --git a/t/meson.build b/t/meson.build
index bfb744e886..61285852e9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -788,6 +788,7 @@ integration_tests = [
   't6134-pathspec-in-submodule.sh',
   't6135-pathspec-with-attrs.sh',
   't6136-pathspec-in-bare.sh',
+  't6137-pathspec-wildcards-literal.sh',
   't6200-fmt-merge-msg.sh',
   't6300-for-each-ref.sh',
   't6301-for-each-ref-errors.sh',
diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
new file mode 100755
index 0000000000..abf837bf6c
--- /dev/null
+++ b/t/t6137-pathspec-wildcards-literal.sh
@@ -0,0 +1,282 @@
+#!/bin/sh
+
+test_description='test wildcards and literals with various git commands'
+
+. ./test-lib.sh
+
+test_have_prereq FUNNYNAMES || {
+    skip_all='skipping: needs FUNNYNAMES (non-Windows only)'
+    test_done
+}
+
+reset_git_repo () {
+    rm -rf .git &&
+    git init &&
+    rm -rf "actual_files" "expected_files"
+}
+
+end_test_properly() {
+    cd .. &&
+    rm -rf "testdir"
+}
+
+
+test_expect_success 'setup' '
+    mkdir testdir &&
+    cd testdir &&
+    touch "*" "?" "[abc]" "f*" "f?z" "a" &&
+    touch "**" "foo*bar" "hello?world" "f**" "hello_world" &&
+    git init
+'
+
+test_expect_success 'check * wildcard in git add' '
+    git init &&
+    git add "*" &&
+    cat >expected_files <<EOF &&
+*
+**
+?
+[abc]
+a
+f*
+f**
+f?z
+foo*bar
+hello?world
+hello_world
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check \* literal in git add' '
+    reset_git_repo &&
+    git add "\*" &&
+    cat >expected_files <<EOF &&
+*
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check f* wildcard in git add' '
+    reset_git_repo &&
+    git add "f*" &&
+    cat >expected_files <<EOF &&
+f*
+f**
+f?z
+foo*bar
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check f\* literal in git add' '
+    reset_git_repo &&
+    git add "f\*" &&
+    cat >expected_files <<EOF &&
+f*
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check f** wildcard in git add' '
+    reset_git_repo &&
+    git add "f**" &&
+    cat >expected_files <<EOF &&
+f*
+f**
+f?z
+foo*bar
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check f\*\* literal in git add' '
+    reset_git_repo &&
+    git add "f\*\*" &&
+    cat >expected_files <<EOF &&
+f**
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check ? wildcard in git add' '
+    reset_git_repo &&
+    git add "?" &&
+    cat >expected_files <<EOF &&
+*
+?
+a
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check \? literal in git add' '
+    reset_git_repo &&
+    git add "\?" &&
+    cat >expected_files <<EOF &&
+?
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check hello?world wildcard in git add' '
+    reset_git_repo &&
+    git add "hello?world" &&
+    cat >expected_files <<EOF &&
+hello?world
+hello_world
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'check hello\?world literal in git add' '
+    reset_git_repo &&
+    git add "hello\?world" &&
+    cat >expected_files <<EOF &&
+hello?world
+EOF
+    git ls-files >actual_files &&
+    test_cmp expected_files actual_files
+'
+
+test_expect_success 'commit: wildcard *' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "*" &&
+    cat >expected_files <<-\EOF &&
+*
+**
+?
+[abc]
+a
+f*
+f**
+f?z
+foo*bar
+hello?world
+hello_world
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: literal *' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "\*" &&
+    cat >expected_files <<-\EOF &&
+*
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: wildcard f*' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "f*" &&
+    cat >expected_files <<-\EOF &&
+f*
+f**
+f?z
+foo*bar
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: literal f\*' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "f\*" &&
+    cat >expected_files <<-\EOF &&
+f*
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: wildcard pathspec limits commit' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "f**" &&
+    cat >expected_files <<-\EOF &&
+f*
+f**
+f?z
+foo*bar
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: literal f\*\*' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "f\*\*" &&
+    cat >expected_files <<-\EOF &&
+f**
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: wildcard ?' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "?" &&
+    cat >expected_files <<-\EOF &&
+*
+?
+a
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'commit: literal \?' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "\?" &&
+    cat >expected_files <<-\EOF &&
+?
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'check hello?world wildcard in git commit' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "hello?world" &&
+    cat >expected_files <<-\EOF &&
+hello?world
+hello_world
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+test_expect_success 'check hello\?world literal in git commit' '
+    reset_git_repo &&
+    git add . &&
+    git commit --allow-empty -m "Test" -- "hello\?world" &&
+    cat >expected_files <<-\EOF &&
+hello?world
+EOF
+    git ls-tree -r --name-only HEAD > actual_files &&
+    test_cmp expected_files actual_files 
+'
+
+end_test_properly
+
+test_done
-- 
2.49.0.223.ga3111b2db4.dirty

Re: [PATCH v2] Dir: Fix and test wildcard pathspec handling

[Junio C Hamano]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> Subject: Re: [PATCH v2] Dir: Fix and test wildcard pathspec handling

Subject: dir.c: literal match with wildcard in pathspec should still glob

or something?  "Fix" implies something may have been broken and the
change was an attempt to correct it, but otherwise it does not say
anything about what was wrong and how it was improved.




> Ensure wildcards expand, even with literal file match.
> Fixes 'git add f*' skipping files like 'foo' if 'f*' exists.

The usual way to compose a log message of this project is to

 - Give an observation on how the current system work in the present
   tense (so no need to say "Currently X is Y", just "X is Y"), and
   discuss what you perceive as a problem in it.

 - Propose a solution (optional---often, problem description
   trivially leads to an obvious solution in reader's minds).

 - Give commands to the codebase to "become like so".

in this order.  In the above, a clear problem description, an
observation on how the current behaviour is not what you want to
see, is missing.

    With a path with wildcard characters, e.g. 'f*o', exists in the
    working tree, "git add -- 'f*o'" stops after happily finding
    that there is 'f*o' and adding it to the index, without
    realizing there may be other paths, e.g. 'foooo', that may match
    the given pathspec.

    This is because dir.c:do_match_pathspec() disables further
    matches with pathspec when it finds an exact match.

or something.  The first paragraph gives end-user visible behaviour
(so that readers can try it at home if they wanted to), and then you
add a bit of explanation of the reason why it happens in the current
code.

Then it would be rather obvious how to correct it, so you probably
do not have to repeat what you did in the code in the log message in
this case.

> Use 'f\*' to add the literal.

The proposed log message is not a place to give an introductory
shell syntax lesson to your readers.

Remember, what you _did_ in the patch can be read by readers.  What
they may need help reading your patch is _why_ you did them, which
may not be obvious at times.  For example,

> Tests added for add and commit where dir.c logic applies.

your readers can see you only covered "add" and "commit" in the new
tests, but they cannot read your mind to find out why you did not
add tests for, say, "git rm f\*".  If you want to say this line, it
should explain how add and commit are affected by the code (and
possibly, how other commands are not affected, but that is
optional).

> Skips windows specific test.

Again, from the code we can read the test runs only on platforms
that can do FUNNYNAMES (not necessarily Windows-specific).  If you
want to say this line, it should explain why.

    As some file systems are incapable of holding files with
    wildcard letters in their names, guard the whole test script
    with FUNNYNAMES prerequisite.

or something.

To those who have been intimately following the discussion, it often
is understandable without some of the above, but we are not writing
for those who review the patches.  We are primarily writing for future
readers of "git log" who are not aware of the review discussion we
have on list, so we should give something to prepare them by setting
the stage and stating the objective first, before going into how the
patch solved it.

> reported-by: piotrsiupa <piotrsiupa@gmail.com>
> Mentored-by: Jeff King <peff@peff.net>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>

Unless this is part of a project done as some mentorship program
like GSoC and Outreachy, Helped-by: would be more appropriate.  It
is not like Peff is assigned as your mentor for working on this
particular "fix wildcard" project, is it?

> -		if (seen && seen[i] == MATCHED_EXACTLY)
> +		if (seen && seen[i] == MATCHED_EXACTLY &&
> +			ps->items[i].nowildcard_len == ps->items[i].len)
>  			continue;

We usually align the first letter of this second line with the first
letter of the same expression (i.e. "seen"), so this looks a bit
unconventionally indented, but the logic is correct, it seems.  If
the entire pathspec is without wildcard, then there is no reason to
spend more cycles to look for more matches, but otherwise, we may
find other paths that may match, so we do not continue and perform
the rest of the loop.

OK.

> diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
> new file mode 100755
> index 0000000000..abf837bf6c
> --- /dev/null
> +++ b/t/t6137-pathspec-wildcards-literal.sh
> @@ -0,0 +1,282 @@
> +#!/bin/sh
> +
> +test_description='test wildcards and literals with various git commands'
> +
> +. ./test-lib.sh
> +
> +test_have_prereq FUNNYNAMES || {
> +    skip_all='skipping: needs FUNNYNAMES (non-Windows only)'
> +    test_done
> +}

Do not do 4-space indentation.  We indent with HT (tab).

cf. Documentation/CodingGuidelines

> +reset_git_repo () {
> +    rm -rf .git &&
> +    git init &&
> +    rm -rf "actual_files" "expected_files"

If you have the habit of using "-r" unnecessarily, lose it.

	rm -f actual_files expected_files

Also it is easier to readers if you leave these "obviously literal"
names without quoted.

> +}
> +
> +end_test_properly() {

Style.


> +    cd .. &&
> +    rm -rf "testdir"
> +}
> +
> +
> +test_expect_success 'setup' '
> +    mkdir testdir &&
> +    cd testdir &&
> +    touch "*" "?" "[abc]" "f*" "f?z" "a" &&
> +    touch "**" "foo*bar" "hello?world" "f**" "hello_world" &&
> +    git init
> +'

This is a bad pattern.  What happens if any of the statements failed
before or after you did "cd testdir"?  The next and subsequent test
may or may not run inside "testdir".  The call to end_test at the end
would then go one level up (usually leading you to the t/ directory)
and try to clean things up there, which is not what you want.

> +test_expect_success 'check * wildcard in git add' '
> +    git init &&

In the previous you did "init".  What is the reason why you do
another in the same directory?

Rather, do things perhaps like this?

	git init test-add-asterisk &&
	(
		cd test-add-asterisk &&
                prepare_test_files &&
		git add \* &&
		cat >expect <<-\EOF &&
		*
		**
		?
		...
		EOF
		git ls-files >actual &&
		test_cmp expect actual
	)

That way, each test will be more independent from other tests.

Also unless you have to test with large quantity of data and want to
remove them from the disk as soon as you are done, do not sprinkle
your test with unnecessary "remove the repository and test data"
code.  It is easier when tests do break if you leave them be.

Also notice the use of "-" prefix to cause the leading tabs stripped
from the HERE-DOC lines, and quoting of EOF (here I used \EOF for
brevity and because it is customary to do so but you could use "EOF"
or 'EOF') that causes the HERE-DOC text to taken literally.  They
are both conventions used in this project to make the here-doc text
easier to read.

The prepare_test_files helper may do

	prepare_test_files () {
		for f in "*" "?" "[abc]" "f*" ...
		do
			>"$f" || return
		done
	}	

I'll stop here.

Thanks.

[PATCH] dir.c: literal match with wildcard in pathspec should still glob

[K Jayatheerth]

With a path with wildcard characters, e.g. 'f*o', exists in the
working tree, "git add -- 'f*o'" stops after happily finding
that there is 'f*o' and adding it to the index, without
realizing there may be other paths, e.g. 'foooo', that may match
the given pathspec.

This is because dir.c:do_match_pathspec() disables further
matches with pathspec when it finds an exact match.

Reported-by: piotrsiupa <piotrsiupa@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 dir.c                                 |   3 +-
 t/meson.build                         |   1 +
 t/t6137-pathspec-wildcards-literal.sh | 429 ++++++++++++++++++++++++++
 3 files changed, 432 insertions(+), 1 deletion(-)
 create mode 100755 t/t6137-pathspec-wildcards-literal.sh

diff --git a/dir.c b/dir.c
index 28b0e03feb..9405fee83a 100644
--- a/dir.c
+++ b/dir.c
@@ -519,7 +519,8 @@ static int do_match_pathspec(struct index_state *istate,
 		    ( exclude && !(ps->items[i].magic & PATHSPEC_EXCLUDE)))
 			continue;
 
-		if (seen && seen[i] == MATCHED_EXACTLY)
+		if (seen && seen[i] == MATCHED_EXACTLY &&
+			ps->items[i].nowildcard_len == ps->items[i].len)
 			continue;
 		/*
 		 * Make exclude patterns optional and never report
diff --git a/t/meson.build b/t/meson.build
index bfb744e886..61285852e9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -788,6 +788,7 @@ integration_tests = [
   't6134-pathspec-in-submodule.sh',
   't6135-pathspec-with-attrs.sh',
   't6136-pathspec-in-bare.sh',
+  't6137-pathspec-wildcards-literal.sh',
   't6200-fmt-merge-msg.sh',
   't6300-for-each-ref.sh',
   't6301-for-each-ref-errors.sh',
diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
new file mode 100755
index 0000000000..20abad5667
--- /dev/null
+++ b/t/t6137-pathspec-wildcards-literal.sh
@@ -0,0 +1,429 @@
+#!/bin/sh
+test_description='test wildcards and literals with git add/commit (subshell style)'
+
+. ./test-lib.sh
+
+test_have_prereq FUNNYNAMES || {
+	skip_all='skipping: needs FUNNYNAMES (non-Windows only)'
+	test_done
+}
+
+prepare_test_files () {
+	for f in "*" "**" "?" "[abc]" "a" "f*" "f**" "f?z" "foo*bar" "hello?world" "hello_world"
+	do
+		>"$f" || return
+	done
+}
+
+test_expect_success 'add wildcard *' '
+	git init test-asterisk &&
+	(
+		cd test-asterisk &&
+		prepare_test_files &&
+		git add "*" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal \*' '
+	git init test-asterisk-literal &&
+	(
+		cd test-asterisk-literal &&
+		prepare_test_files &&
+		git add "\*" &&
+		cat >expect <<-EOF &&
+		*
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard **' '
+	git init test-dstar &&
+	(
+		cd test-dstar &&
+		prepare_test_files &&
+		git add "**" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard ?' '
+	git init test-qmark &&
+	(
+		cd test-qmark &&
+		prepare_test_files &&
+		git add "?" &&
+		cat >expect <<-\EOF | sort &&
+		*
+		?
+		a
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard [abc]' '
+	git init test-brackets &&
+	(
+		cd test-brackets &&
+		prepare_test_files &&
+		git add "[abc]" &&
+		cat >expect <<-\EOF | sort &&
+		[abc]
+		a
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f*' '
+	git init test-f-wild &&
+	(
+		cd test-f-wild &&
+		prepare_test_files &&
+		git add "f*" &&
+		cat >expect <<-\EOF | sort &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal f\*' '
+	git init test-f-lit &&
+	(
+		cd test-f-lit &&
+		prepare_test_files &&
+		git add "f\*" &&
+		cat >expect <<-\EOF &&
+		f*
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f**' '
+	git init test-fdstar &&
+	(
+		cd test-fdstar &&
+		prepare_test_files &&
+		git add "f**" &&
+		cat >expect <<-\EOF | sort &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal f\*\*' '
+	git init test-fdstar-lit &&
+	(
+		cd test-fdstar-lit &&
+		prepare_test_files &&
+		git add "f\*\*" &&
+		cat >expect <<-\EOF &&
+		f**
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f?z' '
+	git init test-fqz &&
+	(
+		cd test-fqz &&
+		prepare_test_files &&
+		git add "f?z" &&
+		cat >expect <<-\EOF &&
+		f?z
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal \? literal' '
+	git init test-q-lit &&
+	(
+		cd test-q-lit &&
+		prepare_test_files &&
+		git add "\?" &&
+		cat >expect <<-\EOF &&
+		?
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard foo*bar' '
+	git init test-foobar &&
+	(
+		cd test-foobar &&
+		prepare_test_files &&
+		git add "foo*bar" &&
+		cat >expect <<-\EOF &&
+		foo*bar
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard hello?world' '
+	git init test-hellowild &&
+	(
+		cd test-hellowild &&
+		prepare_test_files &&
+		git add "hello?world" &&
+		cat >expect <<-\EOF &&
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal hello\?world' '
+	git init test-hellolit &&
+	(
+		cd test-hellolit &&
+		prepare_test_files &&
+		git add "hello\?world" &&
+		cat >expect <<-\EOF &&
+		hello?world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal [abc]' '
+	git init test-brackets-lit &&
+	(
+		cd test-brackets-lit &&
+		prepare_test_files &&
+		git add "\[abc\]" &&
+		cat >expect <<-\EOF &&
+		[abc]
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard *' '
+	git init test-c-asterisk &&
+	(
+		cd test-c-asterisk &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c1" -- "*" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal *' '
+	git init test-c-asterisk-lit &&
+	(
+		cd test-c-asterisk-lit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c2" -- "\*" &&
+		cat >expect <<-EOF &&
+		*
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard f*' '
+	git init test-c-fwild &&
+	(
+		cd test-c-fwild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c3" -- "f*" &&
+		cat >expect <<-EOF &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal f\*' '
+	git init test-c-flit &&
+	(
+		cd test-c-flit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c4" -- "f\*" &&
+		cat >expect <<-EOF &&
+		f*
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard pathspec limits commit' '
+	git init test-c-pathlimit &&
+	(
+		cd test-c-pathlimit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c5" -- "f**" &&
+		cat >expect <<-EOF &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal f\*\*' '
+	git init test-c-fdstar-lit &&
+	(
+		cd test-c-fdstar-lit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c6" -- "f\*\*" &&
+		cat >expect <<-EOF &&
+		f**
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard ?' '
+	git init test-c-qwild &&
+	(
+		cd test-c-qwild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c7" -- "?" &&
+		cat >expect <<-EOF &&
+		*
+		?
+		a
+		EOF
+		git ls-tree -r --name-only HEAD | sort >actual &&
+		sort expect >expect.sorted &&
+		test_cmp expect.sorted actual
+	)
+'
+
+test_expect_success 'commit: literal \?' '
+	git init test-c-qlit &&
+	(
+		cd test-c-qlit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c8" -- "\?" &&
+		cat >expect <<-EOF &&
+		?
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard hello?world' '
+	git init test-c-hellowild &&
+	(
+		cd test-c-hellowild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c9" -- "hello?world"  &&
+		cat >expect <<-EOF &&
+		hello?world
+		hello_world
+		EOF
+		git ls-tree -r --name-only HEAD | sort >actual &&
+		sort expect >expect.sorted &&
+		test_cmp expect.sorted actual
+	)
+'
+
+test_expect_success 'commit: literal hello\?world' '
+	git init test-c-hellolit &&
+	(
+		cd test-c-hellolit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c10" -- "hello\?world" &&
+		cat >expect <<-EOF &&
+		hello?world
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.49.GIT

Re: [PATCH] dir.c: literal match with wildcard in pathspec should still glob

[JAYATHEERTH K]

On Sat, May 3, 2025 at 11:37 AM K Jayatheerth
<jayatheerthkulkarni2005@gmail.com> wrote:
>
> With a path with wildcard characters, e.g. 'f*o', exists in the
> working tree, "git add -- 'f*o'" stops after happily finding
> that there is 'f*o' and adding it to the index, without
> realizing there may be other paths, e.g. 'foooo', that may match
> the given pathspec.
>
> This is because dir.c:do_match_pathspec() disables further
> matches with pathspec when it finds an exact match.
>
> Reported-by: piotrsiupa <piotrsiupa@gmail.com>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  dir.c                                 |   3 +-
>  t/meson.build                         |   1 +
>  t/t6137-pathspec-wildcards-literal.sh | 429 ++++++++++++++++++++++++++
>  3 files changed, 432 insertions(+), 1 deletion(-)
>  create mode 100755 t/t6137-pathspec-wildcards-literal.sh
>
> diff --git a/dir.c b/dir.c
> index 28b0e03feb..9405fee83a 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -519,7 +519,8 @@ static int do_match_pathspec(struct index_state *istate,
>                     ( exclude && !(ps->items[i].magic & PATHSPEC_EXCLUDE)))
>                         continue;
>
> -               if (seen && seen[i] == MATCHED_EXACTLY)
> +               if (seen && seen[i] == MATCHED_EXACTLY &&
> +                       ps->items[i].nowildcard_len == ps->items[i].len)
>                         continue;

For some reason on my computer when I check the formatted patch the
ps-> line
align with the (seen && ...) line perfectly
Again when I send the mail it is shifted forward,
I ensured to use tab space instead of 4 spaces as previously asked.

-Jayatheerth

Re: [PATCH] dir.c: literal match with wildcard in pathspec should still glob

[Junio C Hamano]

JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:

>> +               if (seen && seen[i] == MATCHED_EXACTLY &&
>> +                       ps->items[i].nowildcard_len == ps->items[i].len)
>>                         continue;
>
> For some reason on my computer when I check the formatted patch the
> ps-> line
> align with the (seen && ...) line perfectly

Because the quoted patch in the message I am responding to has tabs
expanded already, probably by your mailer, I looked at what you
originally posted, and it has these lines:

-		if (seen && seen[i] == MATCHED_EXACTLY)
+		if (seen && seen[i] == MATCHED_EXACTLY &&
+			ps->items[i].nowildcard_len == ps->items[i].len)
 			continue;

Removing the prefix '+'/'-'/' ', and replacing HT with ^I for
visibility, the above looks like this:

^I^Iif (seen && seen[i] == MATCHED_EXACTLY)
^I^Iif (seen && seen[i] == MATCHED_EXACTLY &&
^I^I^Ips->items[i].nowildcard_len == ps->items[i].len)
^I^I^Icontinue;

As the display width in monospace for "if (" is 4 spaces, "seen" and
"ps->" would align only if the third HT expands to 4 spaces on your
system.

Perhaps because you are telling your editor or terminal that your
tab, unlike everybody else's, is 4-space wide or something?

[PATCH] dir.c: literal match with wildcard in pathspec should still glob

[K Jayatheerth]

With a path with wildcard characters, e.g. 'f*o', exists in the
working tree, "git add -- 'f*o'" stops after happily finding
that there is 'f*o' and adding it to the index, without
realizing there may be other paths, e.g. 'foooo', that may match
the given pathspec.

This is because dir.c:do_match_pathspec() disables further
matches with pathspec when it finds an exact match.

Reported-by: piotrsiupa <piotrsiupa@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 dir.c                                 |   3 +-
 t/meson.build                         |   1 +
 t/t6137-pathspec-wildcards-literal.sh | 429 ++++++++++++++++++++++++++
 3 files changed, 432 insertions(+), 1 deletion(-)
 create mode 100755 t/t6137-pathspec-wildcards-literal.sh

diff --git a/dir.c b/dir.c
index 28b0e03feb..e162947d0a 100644
--- a/dir.c
+++ b/dir.c
@@ -519,7 +519,8 @@ static int do_match_pathspec(struct index_state *istate,
 		    ( exclude && !(ps->items[i].magic & PATHSPEC_EXCLUDE)))
 			continue;
 
-		if (seen && seen[i] == MATCHED_EXACTLY)
+		if (seen && seen[i] == MATCHED_EXACTLY && 
+		   ps->items[i].nowildcard_len == ps->items[i].len)
 			continue;
 		/*
 		 * Make exclude patterns optional and never report
diff --git a/t/meson.build b/t/meson.build
index bfb744e886..61285852e9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -788,6 +788,7 @@ integration_tests = [
   't6134-pathspec-in-submodule.sh',
   't6135-pathspec-with-attrs.sh',
   't6136-pathspec-in-bare.sh',
+  't6137-pathspec-wildcards-literal.sh',
   't6200-fmt-merge-msg.sh',
   't6300-for-each-ref.sh',
   't6301-for-each-ref-errors.sh',
diff --git a/t/t6137-pathspec-wildcards-literal.sh b/t/t6137-pathspec-wildcards-literal.sh
new file mode 100755
index 0000000000..20abad5667
--- /dev/null
+++ b/t/t6137-pathspec-wildcards-literal.sh
@@ -0,0 +1,429 @@
+#!/bin/sh
+test_description='test wildcards and literals with git add/commit (subshell style)'
+
+. ./test-lib.sh
+
+test_have_prereq FUNNYNAMES || {
+	skip_all='skipping: needs FUNNYNAMES (non-Windows only)'
+	test_done
+}
+
+prepare_test_files () {
+	for f in "*" "**" "?" "[abc]" "a" "f*" "f**" "f?z" "foo*bar" "hello?world" "hello_world"
+	do
+		>"$f" || return
+	done
+}
+
+test_expect_success 'add wildcard *' '
+	git init test-asterisk &&
+	(
+		cd test-asterisk &&
+		prepare_test_files &&
+		git add "*" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal \*' '
+	git init test-asterisk-literal &&
+	(
+		cd test-asterisk-literal &&
+		prepare_test_files &&
+		git add "\*" &&
+		cat >expect <<-EOF &&
+		*
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard **' '
+	git init test-dstar &&
+	(
+		cd test-dstar &&
+		prepare_test_files &&
+		git add "**" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard ?' '
+	git init test-qmark &&
+	(
+		cd test-qmark &&
+		prepare_test_files &&
+		git add "?" &&
+		cat >expect <<-\EOF | sort &&
+		*
+		?
+		a
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard [abc]' '
+	git init test-brackets &&
+	(
+		cd test-brackets &&
+		prepare_test_files &&
+		git add "[abc]" &&
+		cat >expect <<-\EOF | sort &&
+		[abc]
+		a
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f*' '
+	git init test-f-wild &&
+	(
+		cd test-f-wild &&
+		prepare_test_files &&
+		git add "f*" &&
+		cat >expect <<-\EOF | sort &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal f\*' '
+	git init test-f-lit &&
+	(
+		cd test-f-lit &&
+		prepare_test_files &&
+		git add "f\*" &&
+		cat >expect <<-\EOF &&
+		f*
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f**' '
+	git init test-fdstar &&
+	(
+		cd test-fdstar &&
+		prepare_test_files &&
+		git add "f**" &&
+		cat >expect <<-\EOF | sort &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-files | sort >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal f\*\*' '
+	git init test-fdstar-lit &&
+	(
+		cd test-fdstar-lit &&
+		prepare_test_files &&
+		git add "f\*\*" &&
+		cat >expect <<-\EOF &&
+		f**
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard f?z' '
+	git init test-fqz &&
+	(
+		cd test-fqz &&
+		prepare_test_files &&
+		git add "f?z" &&
+		cat >expect <<-\EOF &&
+		f?z
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal \? literal' '
+	git init test-q-lit &&
+	(
+		cd test-q-lit &&
+		prepare_test_files &&
+		git add "\?" &&
+		cat >expect <<-\EOF &&
+		?
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard foo*bar' '
+	git init test-foobar &&
+	(
+		cd test-foobar &&
+		prepare_test_files &&
+		git add "foo*bar" &&
+		cat >expect <<-\EOF &&
+		foo*bar
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add wildcard hello?world' '
+	git init test-hellowild &&
+	(
+		cd test-hellowild &&
+		prepare_test_files &&
+		git add "hello?world" &&
+		cat >expect <<-\EOF &&
+		hello?world
+		hello_world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal hello\?world' '
+	git init test-hellolit &&
+	(
+		cd test-hellolit &&
+		prepare_test_files &&
+		git add "hello\?world" &&
+		cat >expect <<-\EOF &&
+		hello?world
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'add literal [abc]' '
+	git init test-brackets-lit &&
+	(
+		cd test-brackets-lit &&
+		prepare_test_files &&
+		git add "\[abc\]" &&
+		cat >expect <<-\EOF &&
+		[abc]
+		EOF
+		git ls-files >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard *' '
+	git init test-c-asterisk &&
+	(
+		cd test-c-asterisk &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c1" -- "*" &&
+		cat >expect <<-EOF &&
+		*
+		**
+		?
+		[abc]
+		a
+		f*
+		f**
+		f?z
+		foo*bar
+		hello?world
+		hello_world
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal *' '
+	git init test-c-asterisk-lit &&
+	(
+		cd test-c-asterisk-lit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c2" -- "\*" &&
+		cat >expect <<-EOF &&
+		*
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard f*' '
+	git init test-c-fwild &&
+	(
+		cd test-c-fwild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c3" -- "f*" &&
+		cat >expect <<-EOF &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal f\*' '
+	git init test-c-flit &&
+	(
+		cd test-c-flit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c4" -- "f\*" &&
+		cat >expect <<-EOF &&
+		f*
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard pathspec limits commit' '
+	git init test-c-pathlimit &&
+	(
+		cd test-c-pathlimit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c5" -- "f**" &&
+		cat >expect <<-EOF &&
+		f*
+		f**
+		f?z
+		foo*bar
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: literal f\*\*' '
+	git init test-c-fdstar-lit &&
+	(
+		cd test-c-fdstar-lit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c6" -- "f\*\*" &&
+		cat >expect <<-EOF &&
+		f**
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard ?' '
+	git init test-c-qwild &&
+	(
+		cd test-c-qwild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c7" -- "?" &&
+		cat >expect <<-EOF &&
+		*
+		?
+		a
+		EOF
+		git ls-tree -r --name-only HEAD | sort >actual &&
+		sort expect >expect.sorted &&
+		test_cmp expect.sorted actual
+	)
+'
+
+test_expect_success 'commit: literal \?' '
+	git init test-c-qlit &&
+	(
+		cd test-c-qlit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c8" -- "\?" &&
+		cat >expect <<-EOF &&
+		?
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'commit: wildcard hello?world' '
+	git init test-c-hellowild &&
+	(
+		cd test-c-hellowild &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c9" -- "hello?world"  &&
+		cat >expect <<-EOF &&
+		hello?world
+		hello_world
+		EOF
+		git ls-tree -r --name-only HEAD | sort >actual &&
+		sort expect >expect.sorted &&
+		test_cmp expect.sorted actual
+	)
+'
+
+test_expect_success 'commit: literal hello\?world' '
+	git init test-c-hellolit &&
+	(
+		cd test-c-hellolit &&
+		prepare_test_files &&
+		git add . &&
+		git commit -m "c10" -- "hello\?world" &&
+		cat >expect <<-EOF &&
+		hello?world
+		EOF
+		git ls-tree -r --name-only HEAD >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_done
-- 
2.49.GIT

Re: [PATCH] dir.c: literal match with wildcard in pathspec should still glob

[JAYATHEERTH K]

On Mon, May 5, 2025 at 8:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> JAYATHEERTH K <jayatheerthkulkarni2005@gmail.com> writes:
>
> >> +               if (seen && seen[i] == MATCHED_EXACTLY &&
> >> +                       ps->items[i].nowildcard_len == ps->items[i].len)
> >>                         continue;
> >
> > For some reason on my computer when I check the formatted patch the
> > ps-> line
> > align with the (seen && ...) line perfectly
>
> Because the quoted patch in the message I am responding to has tabs
> expanded already, probably by your mailer, I looked at what you
> originally posted, and it has these lines:
>
> -               if (seen && seen[i] == MATCHED_EXACTLY)
> +               if (seen && seen[i] == MATCHED_EXACTLY &&
> +                       ps->items[i].nowildcard_len == ps->items[i].len)
>                         continue;
>
> Removing the prefix '+'/'-'/' ', and replacing HT with ^I for
> visibility, the above looks like this:
>
> ^I^Iif (seen && seen[i] == MATCHED_EXACTLY)
> ^I^Iif (seen && seen[i] == MATCHED_EXACTLY &&
> ^I^I^Ips->items[i].nowildcard_len == ps->items[i].len)
> ^I^I^Icontinue;
>
> As the display width in monospace for "if (" is 4 spaces, "seen" and
> "ps->" would align only if the third HT expands to 4 spaces on your
> system.
>
> Perhaps because you are telling your editor or terminal that your
> tab, unlike everybody else's, is 4-space wide or something?

Thank you for the clarification.
Yes it was the 4 tab thing, my editor has default as 4 therefore the confusion.

-Jayatheerth