Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[davi.reis]

Here is how to reproduce the bug:

git init
mkdir prefix && touch prefix/a && git add prefix/a
mkdir prefixdir && touch prefixdir/b && git add prefixdir/b
git commit -a -m "If -r is not given, ls-tree should not show files in subdirs."
git ls-tree --name-only HEAD prefix  # works as expected
git ls-tree --name-only HEAD prefixdir  # works as expected
git ls-tree --name-only HEAD prefix prefixdir  # shows file, not dir

The output of the last command is 

prefix/a
prefixdir

But it should be

prefix
prefixdir

The patch fixes the problem.

[PATCH] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[davi.reis]

From: Davi Reis <davi@davi-macbookpro.local>

---
 builtin/ls-tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index dc86b0d..fa427e2 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
 		speclen = strlen(spec);
 		if (speclen <= len)
 			continue;
+		if (spec[len] != 0 && spec[len] != '/')
+			continue;
 		if (memcmp(pathname, spec, len))
 			continue;
 		return 1;
-- 
1.7.2.2

Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[Matthieu Moy]

davi.reis@gmail.com writes:

> Here is how to reproduce the bug:
>
> git init
> mkdir prefix && touch prefix/a && git add prefix/a
> mkdir prefixdir && touch prefixdir/b && git add prefixdir/b
> git commit -a -m "If -r is not given, ls-tree should not show files in subdirs."
> git ls-tree --name-only HEAD prefix  # works as expected
> git ls-tree --name-only HEAD prefixdir  # works as expected
> git ls-tree --name-only HEAD prefix prefixdir  # shows file, not dir

That's so close to a real test-case... You should incorporate this in
your patch (e.g. in t/t3101-ls-tree-dirname.sh), to make sure such bug
never happens again.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[Davi Reis]

I added the test and used git-sendemail again, but I guess it ended up
in a different thread. Is this good enough or is there some formal
path that I should take? Any help appreciated.

On Wed, Sep 8, 2010 at 11:04 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> davi.reis@gmail.com writes:
>
>> Here is how to reproduce the bug:
>>
>> git init
>> mkdir prefix && touch prefix/a && git add prefix/a
>> mkdir prefixdir && touch prefixdir/b && git add prefixdir/b
>> git commit -a -m "If -r is not given, ls-tree should not show files in subdirs."
>> git ls-tree --name-only HEAD prefix  # works as expected
>> git ls-tree --name-only HEAD prefixdir  # works as expected
>> git ls-tree --name-only HEAD prefix prefixdir  # shows file, not dir
>
> That's so close to a real test-case... You should incorporate this in
> your patch (e.g. in t/t3101-ls-tree-dirname.sh), to make sure such bug
> never happens again.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
>



-- 
[]s
Davi de Castro Reis

Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[Matthieu Moy]

Davi Reis <davi.reis@gmail.com> writes:

> I added the test and used git-sendemail again, but I guess it ended up
> in a different thread.

--in-reply-to is your friend ;-).

> Is this good enough or is there some formal path that I should take?

I'll comment in the other thread.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> That's so close to a real test-case...

Let's do this.

 * t3101 seems somewhat stale; fix the style and add a few missing " &&"
   cascades as a preparatory patch.

 * Add the "mistaken prefix computation causes unwarranted recursion" fix
   with a test.

Here is the first one in such a series.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 11 Sep 2010 10:53:29 -0700
Subject: [PATCH] t3101: modernise style

Also add a few " &&" cascade that were missing.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3101-ls-tree-dirname.sh |  126 ++++++++++++++++++++++---------------------
 1 files changed, 64 insertions(+), 62 deletions(-)

diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh
index 06654c6..026f9f8 100755
--- a/t/t3101-ls-tree-dirname.sh
+++ b/t/t3101-ls-tree-dirname.sh
@@ -21,33 +21,32 @@ entries.  Also test odd filename and missing entries handling.
 '
 . ./test-lib.sh
 
-test_expect_success \
-    'setup' \
-    'echo 111 >1.txt &&
-     echo 222 >2.txt &&
-     mkdir path0 path0/a path0/a/b path0/a/b/c &&
-     echo 111 >path0/a/b/c/1.txt &&
-     mkdir path1 path1/b path1/b/c &&
-     echo 111 >path1/b/c/1.txt &&
-     mkdir path2 &&
-     echo 111 >path2/1.txt &&
-     mkdir path3 &&
-     echo 111 >path3/1.txt &&
-     echo 222 >path3/2.txt &&
-     find *.txt path* \( -type f -o -type l \) -print |
-     xargs git update-index --add &&
-     tree=`git write-tree` &&
-     echo $tree'
+test_expect_success 'setup' '
+	echo 111 >1.txt &&
+	echo 222 >2.txt &&
+	mkdir path0 path0/a path0/a/b path0/a/b/c &&
+	echo 111 >path0/a/b/c/1.txt &&
+	mkdir path1 path1/b path1/b/c &&
+	echo 111 >path1/b/c/1.txt &&
+	mkdir path2 &&
+	echo 111 >path2/1.txt &&
+	mkdir path3 &&
+	echo 111 >path3/1.txt &&
+	echo 222 >path3/2.txt &&
+	find *.txt path* \( -type f -o -type l \) -print |
+	xargs git update-index --add &&
+	tree=`git write-tree` &&
+	echo $tree
+'
 
 test_output () {
-    sed -e "s/ $_x40	/ X	/" <current >check
-    test_cmp expected check
+	sed -e "s/ $_x40	/ X	/" <current >check &&
+	test_cmp expected check
 }
 
-test_expect_success \
-    'ls-tree plain' \
-    'git ls-tree $tree >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree plain' '
+	git ls-tree $tree >current &&
+	cat >expected <<\EOF &&
 100644 blob X	1.txt
 100644 blob X	2.txt
 040000 tree X	path0
@@ -55,13 +54,13 @@ test_expect_success \
 040000 tree X	path2
 040000 tree X	path3
 EOF
-     test_output'
+	test_output
+'
 
 # Recursive does not show tree nodes anymore...
-test_expect_success \
-    'ls-tree recursive' \
-    'git ls-tree -r $tree >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree recursive' '
+	git ls-tree -r $tree >current &&
+	cat >expected <<\EOF &&
 100644 blob X	1.txt
 100644 blob X	2.txt
 100644 blob X	path0/a/b/c/1.txt
@@ -70,68 +69,71 @@ test_expect_success \
 100644 blob X	path3/1.txt
 100644 blob X	path3/2.txt
 EOF
-     test_output'
+	test_output
+'
 
-test_expect_success \
-    'ls-tree filter 1.txt' \
-    'git ls-tree $tree 1.txt >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree filter 1.txt' '
+	git ls-tree $tree 1.txt >current &&
+	cat >expected <<\EOF &&
 100644 blob X	1.txt
 EOF
-     test_output'
+	test_output
+'
 
-test_expect_success \
-    'ls-tree filter path1/b/c/1.txt' \
-    'git ls-tree $tree path1/b/c/1.txt >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree filter path1/b/c/1.txt' '
+	git ls-tree $tree path1/b/c/1.txt >current &&
+	cat >expected <<\EOF &&
 100644 blob X	path1/b/c/1.txt
 EOF
-     test_output'
+	test_output
+'
 
-test_expect_success \
-    'ls-tree filter all 1.txt files' \
-    'git ls-tree $tree 1.txt path0/a/b/c/1.txt path1/b/c/1.txt path2/1.txt path3/1.txt >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree filter all 1.txt files' '
+	git ls-tree $tree 1.txt path0/a/b/c/1.txt \
+		path1/b/c/1.txt path2/1.txt path3/1.txt >current &&
+	cat >expected <<\EOF &&
 100644 blob X	1.txt
 100644 blob X	path0/a/b/c/1.txt
 100644 blob X	path1/b/c/1.txt
 100644 blob X	path2/1.txt
 100644 blob X	path3/1.txt
 EOF
-     test_output'
+	test_output
+'
 
 # I am not so sure about this one after ls-tree doing pathspec match.
 # Having both path0/a and path0/a/b/c makes path0/a redundant, and
 # it behaves as if path0/a/b/c, path1/b/c, path2 and path3 are specified.
-test_expect_success \
-    'ls-tree filter directories' \
-    'git ls-tree $tree path3 path2 path0/a/b/c path1/b/c path0/a >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree filter directories' '
+	git ls-tree $tree path3 path2 path0/a/b/c path1/b/c path0/a >current &&
+	cat >expected <<\EOF &&
 040000 tree X	path0/a/b/c
 040000 tree X	path1/b/c
 040000 tree X	path2
 040000 tree X	path3
 EOF
-     test_output'
+	test_output
+'
 
 # Again, duplicates are filtered away so this is equivalent to
 # having 1.txt and path3
-test_expect_success \
-    'ls-tree filter odd names' \
-    'git ls-tree $tree 1.txt ./1.txt .//1.txt path3/1.txt path3/./1.txt path3 path3// >current &&
-     cat >expected <<\EOF &&
+test_expect_success 'ls-tree filter odd names' '
+	git ls-tree $tree 1.txt ./1.txt .//1.txt \
+		path3/1.txt path3/./1.txt path3 path3// >current &&
+	cat >expected <<\EOF &&
 100644 blob X	1.txt
 100644 blob X	path3/1.txt
 100644 blob X	path3/2.txt
 EOF
-     test_output'
+	test_output
+'
 
-test_expect_success \
-    'ls-tree filter missing files and extra slashes' \
-    'git ls-tree $tree 1.txt/ abc.txt path3//23.txt path3/2.txt/// >current &&
-     cat >expected <<\EOF &&
-EOF
-     test_output'
+test_expect_success 'ls-tree filter missing files and extra slashes' '
+	git ls-tree $tree 1.txt/ abc.txt \
+		path3//23.txt path3/2.txt/// >current &&
+	>expected &&
+	test_output
+'
 
 test_expect_success 'ls-tree filter is leading path match' '
 	git ls-tree $tree pa path3/a >current &&
@@ -198,7 +200,7 @@ EOF
 '
 
 test_expect_success 'ls-tree --name-only' '
-	git ls-tree --name-only $tree >current
+	git ls-tree --name-only $tree >current &&
 	cat >expected <<\EOF &&
 1.txt
 2.txt
@@ -211,7 +213,7 @@ EOF
 '
 
 test_expect_success 'ls-tree --name-only -r' '
-	git ls-tree --name-only -r $tree >current
+	git ls-tree --name-only -r $tree >current &&
 	cat >expected <<\EOF &&
 1.txt
 2.txt
-- 
1.7.3.rc1.215.g6997c

[PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories

[Junio C Hamano]

When applying two pathspecs, one of which is named as a prefix to the
other, we mistakenly recursed into the shorter one.

Noticed and fixed by David Reis.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Junio C Hamano <gitster@pobox.com> writes:

  > Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
  >
  >> That's so close to a real test-case...
  >
  > Let's do this.
  >
  >  * t3101 seems somewhat stale; fix the style and add a few missing " &&"
  >    cascades as a preparatory patch.
  >
  >  * Add the "mistaken prefix computation causes unwarranted recursion" fix
  >    with a test.
  >
  > Here is the first one in such a series.

  and here is the second one.

 builtin/ls-tree.c          |    2 ++
 t/t3101-ls-tree-dirname.sh |   20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index dc86b0d..a818756 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
 		speclen = strlen(spec);
 		if (speclen <= len)
 			continue;
+		if (spec[len] && spec[len] != '/')
+			continue;
 		if (memcmp(pathname, spec, len))
 			continue;
 		return 1;
diff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh
index 026f9f8..ed8160c 100755
--- a/t/t3101-ls-tree-dirname.sh
+++ b/t/t3101-ls-tree-dirname.sh
@@ -15,6 +15,7 @@ This test runs git ls-tree with the following in a tree.
     path2/1.txt        - a file in a directory
     path3/1.txt        - a file in a directory
     path3/2.txt        - a file in a directory
+    path30/3.txt       - a file in a directory
 
 Test the handling of mulitple directories which have matching file
 entries.  Also test odd filename and missing entries handling.
@@ -24,7 +25,7 @@ entries.  Also test odd filename and missing entries handling.
 test_expect_success 'setup' '
 	echo 111 >1.txt &&
 	echo 222 >2.txt &&
-	mkdir path0 path0/a path0/a/b path0/a/b/c &&
+	mkdir path0 path0/a path0/a/b path0/a/b/c path30 &&
 	echo 111 >path0/a/b/c/1.txt &&
 	mkdir path1 path1/b path1/b/c &&
 	echo 111 >path1/b/c/1.txt &&
@@ -33,6 +34,7 @@ test_expect_success 'setup' '
 	mkdir path3 &&
 	echo 111 >path3/1.txt &&
 	echo 222 >path3/2.txt &&
+	echo 333 >path30/3.txt &&
 	find *.txt path* \( -type f -o -type l \) -print |
 	xargs git update-index --add &&
 	tree=`git write-tree` &&
@@ -53,6 +55,7 @@ test_expect_success 'ls-tree plain' '
 040000 tree X	path1
 040000 tree X	path2
 040000 tree X	path3
+040000 tree X	path30
 EOF
 	test_output
 '
@@ -68,6 +71,7 @@ test_expect_success 'ls-tree recursive' '
 100644 blob X	path2/1.txt
 100644 blob X	path3/1.txt
 100644 blob X	path3/2.txt
+100644 blob X	path30/3.txt
 EOF
 	test_output
 '
@@ -164,6 +168,7 @@ test_expect_success 'ls-tree --full-tree' '
 040000 tree X	path1
 040000 tree X	path2
 040000 tree X	path3
+040000 tree X	path30
 EOF
 	test_output
 '
@@ -181,6 +186,7 @@ test_expect_success 'ls-tree --full-tree -r' '
 100644 blob X	path2/1.txt
 100644 blob X	path3/1.txt
 100644 blob X	path3/2.txt
+100644 blob X	path30/3.txt
 EOF
 	test_output
 '
@@ -195,6 +201,7 @@ test_expect_success 'ls-tree --abbrev=5' '
 040000 tree X	path1
 040000 tree X	path2
 040000 tree X	path3
+040000 tree X	path30
 EOF
 	test_cmp expected check
 '
@@ -208,6 +215,7 @@ path0
 path1
 path2
 path3
+path30
 EOF
 	test_output
 '
@@ -222,6 +230,16 @@ path1/b/c/1.txt
 path2/1.txt
 path3/1.txt
 path3/2.txt
+path30/3.txt
+EOF
+	test_output
+'
+
+test_expect_success 'ls-tree with two dirnames' '
+	git ls-tree --name-only $tree path3 path30 >current &&
+	cat >expected <<\EOF &&
+path3
+path30
 EOF
 	test_output
 '
-- 
1.7.3.rc1.215.g6997c

Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> That's so close to a real test-case...
>
> Let's do this.
>
>  * t3101 seems somewhat stale; fix the style and add a few missing " &&"
>    cascades as a preparatory patch.
>
>  * Add the "mistaken prefix computation causes unwarranted recursion" fix
>    with a test.

Sounds good, yes.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/