* 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.