Re: Expected behaviour for pathspecs matching attributes in subdirectories

[Junio C Hamano <gitster@pobox.com>]

Matthew Hughes <mhughes@uw.co.uk> writes:

> I'd be happy to submit a patch adding those tests if you'd like. Though I would
> like to just confirm that in the patch I shared it is not a bug that:
>
>     git ls-files ":(attr:otherLabel)sub/" >actual &&
>     test_must_be_empty actual
>
> I.e. that no files are listed here even tough `sub/fileSetLabel` has the
> attribute `otherLabel`?

I do not think it is a good idea to cast in stone the behaviour,
which we do not know if it is sensible, with a new test, before
knowning what behaviour we want.

I think in this case the common prefix optimization in "ls-files.c"
is broken.  If we disable it like the attached illustration patch,
we will see that pathspecs that end with "sub" or "sub/" behave the
same way, which is what I think people would expect.

The code change in this illustration is not a "fix", of course ;-).

Thanks.

 builtin/ls-files.c             |  2 +-
 t/t6135-pathspec-with-attrs.sh | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git c/builtin/ls-files.c w/builtin/ls-files.c
index a0229c3277..17baed30ca 100644
--- c/builtin/ls-files.c
+++ w/builtin/ls-files.c
@@ -724,7 +724,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	 * submodule entry because the pathspec may match something inside the
 	 * submodule.
 	 */
-	if (recurse_submodules)
+	if (!!"disable common prefix optimization" || recurse_submodules)
 		max_prefix = NULL;
 	else
 		max_prefix = common_prefix(&pathspec);
diff --git c/t/t6135-pathspec-with-attrs.sh w/t/t6135-pathspec-with-attrs.sh
index 457cc167c7..a805fa132b 100755
--- c/t/t6135-pathspec-with-attrs.sh
+++ w/t/t6135-pathspec-with-attrs.sh
@@ -253,4 +253,21 @@ test_expect_success 'backslash cannot be used as a value' '
 	test_i18ngrep "for value matching" actual
 '
 
+test_expect_success 'reading from .gitattributes in a subdirectory' '
+	test_when_finished "rm -f sub/.gitattributes" &&
+	test_write_lines "fileSetLabel label1" >sub/.gitattributes &&
+
+	git ls-files ":(attr:label1)" >actual &&
+	test_write_lines "sub/fileSetLabel" >expect &&
+	test_cmp expect actual &&
+
+	git ls-files ":(attr:label1)sub" >actual &&
+	test_write_lines "sub/fileSetLabel" >expect &&
+	test_cmp expect actual &&
+
+	git ls-files ":(attr:label1)sub/" >actual &&
+	test_write_lines "sub/fileSetLabel" >expect &&
+	test_cmp expect actual
+'
+
 test_done