* Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
@ 2010-09-09 5:40 davi.reis
2010-09-09 5:40 ` [PATCH] " davi.reis
2010-09-09 6:04 ` Matthieu Moy
0 siblings, 2 replies; 8+ messages in thread
From: davi.reis @ 2010-09-09 5:40 UTC (permalink / raw)
To: git
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 5:40 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
@ 2010-09-09 5:40 ` davi.reis
2010-09-09 6:04 ` Matthieu Moy
1 sibling, 0 replies; 8+ messages in thread
From: davi.reis @ 2010-09-09 5:40 UTC (permalink / raw)
To: git; +Cc: 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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 5:40 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
2010-09-09 5:40 ` [PATCH] " davi.reis
@ 2010-09-09 6:04 ` Matthieu Moy
2010-09-09 18:26 ` Davi Reis
2010-09-11 18:57 ` Junio C Hamano
1 sibling, 2 replies; 8+ messages in thread
From: Matthieu Moy @ 2010-09-09 6:04 UTC (permalink / raw)
To: davi.reis; +Cc: git
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/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 6:04 ` Matthieu Moy
@ 2010-09-09 18:26 ` Davi Reis
2010-09-09 21:22 ` Matthieu Moy
2010-09-11 18:57 ` Junio C Hamano
1 sibling, 1 reply; 8+ messages in thread
From: Davi Reis @ 2010-09-09 18:26 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 18:26 ` Davi Reis
@ 2010-09-09 21:22 ` Matthieu Moy
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Moy @ 2010-09-09 21:22 UTC (permalink / raw)
To: Davi Reis; +Cc: git
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/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-09 6:04 ` Matthieu Moy
2010-09-09 18:26 ` Davi Reis
@ 2010-09-11 18:57 ` Junio C Hamano
2010-09-11 19:00 ` [PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories Junio C Hamano
2010-09-14 21:22 ` 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
1 sibling, 2 replies; 8+ messages in thread
From: Junio C Hamano @ 2010-09-11 18:57 UTC (permalink / raw)
To: Matthieu Moy; +Cc: davi.reis, git
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories
2010-09-11 18:57 ` Junio C Hamano
@ 2010-09-11 19:00 ` Junio C Hamano
2010-09-14 21:22 ` 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
1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2010-09-11 19:00 UTC (permalink / raw)
To: Matthieu Moy; +Cc: davi.reis, git
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.
2010-09-11 18:57 ` Junio C Hamano
2010-09-11 19:00 ` [PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories Junio C Hamano
@ 2010-09-14 21:22 ` Matthieu Moy
1 sibling, 0 replies; 8+ messages in thread
From: Matthieu Moy @ 2010-09-14 21:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: davi.reis, git
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/
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-09-14 21:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-09 5:40 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
2010-09-09 5:40 ` [PATCH] " davi.reis
2010-09-09 6:04 ` Matthieu Moy
2010-09-09 18:26 ` Davi Reis
2010-09-09 21:22 ` Matthieu Moy
2010-09-11 18:57 ` Junio C Hamano
2010-09-11 19:00 ` [PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories Junio C Hamano
2010-09-14 21:22 ` 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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.