* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).