{"thread":{"id":"25050","subject":"Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","startedAt":"2010-09-09T05:40:25Z","lastAt":"2010-09-14T21:22:44Z","messageCount":8,"participants":["davi.reis@gmail.com","Matthieu Moy","Davi Reis","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"150350","messageId":"1284010826-81989-1-git-send-email-davi.reis@gmail.com","threadId":"25050","inReplyTo":null,"subject":"Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T05:40:25Z","receivedAt":"2010-09-09T05:40:25Z","isPatch":false,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"Here is how to reproduce the bug:\n\ngit init\nmkdir prefix && touch prefix/a && git add prefix/a\nmkdir prefixdir && touch prefixdir/b && git add prefixdir/b\ngit commit -a -m \"If -r is not given, ls-tree should not show files in subdirs.\"\ngit ls-tree --name-only HEAD prefix # works as expected\ngit ls-tree --name-only HEAD prefixdir # works as expected\ngit ls-tree --name-only HEAD prefix prefixdir # shows file, not dir\n\nThe output of the last command is \n\nprefix/a\nprefixdir\n\nBut it should be\n\nprefix\nprefixdir\n\nThe patch fixes the problem.\n"},{"id":"150351","messageId":"1284010826-81989-2-git-send-email-davi.reis@gmail.com","threadId":"25050","inReplyTo":"1284010826-81989-1-git-send-email-davi.reis@gmail.com","subject":"[PATCH] Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T05:40:26Z","receivedAt":"2010-09-09T05:40:26Z","isPatch":true,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"From: Davi Reis \n\n---\n builtin/ls-tree.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex dc86b0d..fa427e2 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \t\tspeclen = strlen(spec);\n \t\tif (speclen <= len)\n \t\t\tcontinue;\n+\t\tif (spec[len] != 0 && spec[len] != '/')\n+\t\t\tcontinue;\n \t\tif (memcmp(pathname, spec, len))\n \t\t\tcontinue;\n \t\treturn 1;\n-- \n1.7.2.2\n"},{"id":"150353","messageId":"vpqzkvr5u73.fsf@bauges.imag.fr","threadId":"25050","inReplyTo":"1284010826-81989-1-git-send-email-davi.reis@gmail.com","subject":"Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-09T06:04:16Z","receivedAt":"2010-09-09T06:04:16Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"davi.reis@gmail.com writes:\n\n> Here is how to reproduce the bug:\n>\n> git init\n> mkdir prefix && touch prefix/a && git add prefix/a\n> mkdir prefixdir && touch prefixdir/b && git add prefixdir/b\n> git commit -a -m \"If -r is not given, ls-tree should not show files in subdirs.\"\n> git ls-tree --name-only HEAD prefix # works as expected\n> git ls-tree --name-only HEAD prefixdir # works as expected\n> git ls-tree --name-only HEAD prefix prefixdir # shows file, not dir\n\nThat's so close to a real test-case... You should incorporate this in\nyour patch (e.g. in t/t3101-ls-tree-dirname.sh), to make sure such bug\nnever happens again.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"150380","messageId":"AANLkTikZ11-3QhOxoW096cdA4LryuZQiegg=GLMO0BJB@mail.gmail.com","threadId":"25050","inReplyTo":"vpqzkvr5u73.fsf@bauges.imag.fr","subject":"Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"Davi Reis","fromEmail":"davi.reis@gmail.com","sentAt":"2010-09-09T18:26:09Z","receivedAt":"2010-09-09T18:26:09Z","isPatch":false,"sender":{"key":"davi.reis@gmail.com","avatar":null},"body":"I added the test and used git-sendemail again, but I guess it ended up\nin a different thread. Is this good enough or is there some formal\npath that I should take? Any help appreciated.\n\nOn Wed, Sep 8, 2010 at 11:04 PM, Matthieu Moy\n wrote:\n> davi.reis@gmail.com writes:\n>\n>> Here is how to reproduce the bug:\n>>\n>> git init\n>> mkdir prefix && touch prefix/a && git add prefix/a\n>> mkdir prefixdir && touch prefixdir/b && git add prefixdir/b\n>> git commit -a -m \"If -r is not given, ls-tree should not show files in subdirs.\"\n>> git ls-tree --name-only HEAD prefix  # works as expected\n>> git ls-tree --name-only HEAD prefixdir  # works as expected\n>> git ls-tree --name-only HEAD prefix prefixdir  # shows file, not dir\n>\n> That's so close to a real test-case... You should incorporate this in\n> your patch (e.g. in t/t3101-ls-tree-dirname.sh), to make sure such bug\n> never happens again.\n>\n> --\n> Matthieu Moy\n> http://www-verimag.imag.fr/~moy/\n>\n\n\n\n-- \n[]s\nDavi de Castro Reis\n"},{"id":"150411","messageId":"vpqtylyvcgj.fsf@bauges.imag.fr","threadId":"25050","inReplyTo":"AANLkTikZ11-3QhOxoW096cdA4LryuZQiegg=GLMO0BJB@mail.gmail.com","subject":"Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-09T21:22:52Z","receivedAt":"2010-09-09T21:22:52Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Davi Reis writes:\n\n> I added the test and used git-sendemail again, but I guess it ended up\n> in a different thread.\n\n--in-reply-to is your friend ;-).\n\n> Is this good enough or is there some formal path that I should take?\n\nI'll comment in the other thread.\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"150512","messageId":"7vhbhwf6q5.fsf@alter.siamese.dyndns.org","threadId":"25050","inReplyTo":"vpqzkvr5u73.fsf@bauges.imag.fr","subject":"Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-11T18:57:54Z","receivedAt":"2010-09-11T18:57:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> That's so close to a real test-case...\n\nLet's do this.\n\n * t3101 seems somewhat stale; fix the style and add a few missing \" &&\"\n cascades as a preparatory patch.\n\n * Add the \"mistaken prefix computation causes unwarranted recursion\" fix\n with a test.\n\nHere is the first one in such a series.\n\n-- >8 --\nFrom: Junio C Hamano \nDate: Sat, 11 Sep 2010 10:53:29 -0700\nSubject: [PATCH] t3101: modernise style\n\nAlso add a few \" &&\" cascade that were missing.\n\nSigned-off-by: Junio C Hamano \n---\n t/t3101-ls-tree-dirname.sh | 126 ++++++++++++++++++++++---------------------\n 1 files changed, 64 insertions(+), 62 deletions(-)\n\ndiff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh\nindex 06654c6..026f9f8 100755\n--- a/t/t3101-ls-tree-dirname.sh\n+++ b/t/t3101-ls-tree-dirname.sh\n@@ -21,33 +21,32 @@ entries. Also test odd filename and missing entries handling.\n '\n . ./test-lib.sh\n \n-test_expect_success \\\n- 'setup' \\\n- 'echo 111 >1.txt &&\n- echo 222 >2.txt &&\n- mkdir path0 path0/a path0/a/b path0/a/b/c &&\n- echo 111 >path0/a/b/c/1.txt &&\n- mkdir path1 path1/b path1/b/c &&\n- echo 111 >path1/b/c/1.txt &&\n- mkdir path2 &&\n- echo 111 >path2/1.txt &&\n- mkdir path3 &&\n- echo 111 >path3/1.txt &&\n- echo 222 >path3/2.txt &&\n- find *.txt path* \\( -type f -o -type l \\) -print |\n- xargs git update-index --add &&\n- tree=`git write-tree` &&\n- echo $tree'\n+test_expect_success 'setup' '\n+\techo 111 >1.txt &&\n+\techo 222 >2.txt &&\n+\tmkdir path0 path0/a path0/a/b path0/a/b/c &&\n+\techo 111 >path0/a/b/c/1.txt &&\n+\tmkdir path1 path1/b path1/b/c &&\n+\techo 111 >path1/b/c/1.txt &&\n+\tmkdir path2 &&\n+\techo 111 >path2/1.txt &&\n+\tmkdir path3 &&\n+\techo 111 >path3/1.txt &&\n+\techo 222 >path3/2.txt &&\n+\tfind *.txt path* \\( -type f -o -type l \\) -print |\n+\txargs git update-index --add &&\n+\ttree=`git write-tree` &&\n+\techo $tree\n+'\n \n test_output () {\n- sed -e \"s/ $_x40\t/ X\t/\" check\n- test_cmp expected check\n+\tsed -e \"s/ $_x40\t/ X\t/\" check &&\n+\ttest_cmp expected check\n }\n \n-test_expect_success \\\n- 'ls-tree plain' \\\n- 'git ls-tree $tree >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree plain' '\n+\tgit ls-tree $tree >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\t1.txt\n 100644 blob X\t2.txt\n 040000 tree X\tpath0\n@@ -55,13 +54,13 @@ test_expect_success \\\n 040000 tree X\tpath2\n 040000 tree X\tpath3\n EOF\n- test_output'\n+\ttest_output\n+'\n \n # Recursive does not show tree nodes anymore...\n-test_expect_success \\\n- 'ls-tree recursive' \\\n- 'git ls-tree -r $tree >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree recursive' '\n+\tgit ls-tree -r $tree >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\t1.txt\n 100644 blob X\t2.txt\n 100644 blob X\tpath0/a/b/c/1.txt\n@@ -70,68 +69,71 @@ test_expect_success \\\n 100644 blob X\tpath3/1.txt\n 100644 blob X\tpath3/2.txt\n EOF\n- test_output'\n+\ttest_output\n+'\n \n-test_expect_success \\\n- 'ls-tree filter 1.txt' \\\n- 'git ls-tree $tree 1.txt >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree filter 1.txt' '\n+\tgit ls-tree $tree 1.txt >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\t1.txt\n EOF\n- test_output'\n+\ttest_output\n+'\n \n-test_expect_success \\\n- 'ls-tree filter path1/b/c/1.txt' \\\n- 'git ls-tree $tree path1/b/c/1.txt >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree filter path1/b/c/1.txt' '\n+\tgit ls-tree $tree path1/b/c/1.txt >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\tpath1/b/c/1.txt\n EOF\n- test_output'\n+\ttest_output\n+'\n \n-test_expect_success \\\n- 'ls-tree filter all 1.txt files' \\\n- 'git ls-tree $tree 1.txt path0/a/b/c/1.txt path1/b/c/1.txt path2/1.txt path3/1.txt >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree filter all 1.txt files' '\n+\tgit ls-tree $tree 1.txt path0/a/b/c/1.txt \\\n+\t\tpath1/b/c/1.txt path2/1.txt path3/1.txt >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\t1.txt\n 100644 blob X\tpath0/a/b/c/1.txt\n 100644 blob X\tpath1/b/c/1.txt\n 100644 blob X\tpath2/1.txt\n 100644 blob X\tpath3/1.txt\n EOF\n- test_output'\n+\ttest_output\n+'\n \n # I am not so sure about this one after ls-tree doing pathspec match.\n # Having both path0/a and path0/a/b/c makes path0/a redundant, and\n # it behaves as if path0/a/b/c, path1/b/c, path2 and path3 are specified.\n-test_expect_success \\\n- 'ls-tree filter directories' \\\n- 'git ls-tree $tree path3 path2 path0/a/b/c path1/b/c path0/a >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree filter directories' '\n+\tgit ls-tree $tree path3 path2 path0/a/b/c path1/b/c path0/a >current &&\n+\tcat >expected <<\\EOF &&\n 040000 tree X\tpath0/a/b/c\n 040000 tree X\tpath1/b/c\n 040000 tree X\tpath2\n 040000 tree X\tpath3\n EOF\n- test_output'\n+\ttest_output\n+'\n \n # Again, duplicates are filtered away so this is equivalent to\n # having 1.txt and path3\n-test_expect_success \\\n- 'ls-tree filter odd names' \\\n- 'git ls-tree $tree 1.txt ./1.txt .//1.txt path3/1.txt path3/./1.txt path3 path3// >current &&\n- cat >expected <<\\EOF &&\n+test_expect_success 'ls-tree filter odd names' '\n+\tgit ls-tree $tree 1.txt ./1.txt .//1.txt \\\n+\t\tpath3/1.txt path3/./1.txt path3 path3// >current &&\n+\tcat >expected <<\\EOF &&\n 100644 blob X\t1.txt\n 100644 blob X\tpath3/1.txt\n 100644 blob X\tpath3/2.txt\n EOF\n- test_output'\n+\ttest_output\n+'\n \n-test_expect_success \\\n- 'ls-tree filter missing files and extra slashes' \\\n- 'git ls-tree $tree 1.txt/ abc.txt path3//23.txt path3/2.txt/// >current &&\n- cat >expected <<\\EOF &&\n-EOF\n- test_output'\n+test_expect_success 'ls-tree filter missing files and extra slashes' '\n+\tgit ls-tree $tree 1.txt/ abc.txt \\\n+\t\tpath3//23.txt path3/2.txt/// >current &&\n+\t>expected &&\n+\ttest_output\n+'\n \n test_expect_success 'ls-tree filter is leading path match' '\n \tgit ls-tree $tree pa path3/a >current &&\n@@ -198,7 +200,7 @@ EOF\n '\n \n test_expect_success 'ls-tree --name-only' '\n-\tgit ls-tree --name-only $tree >current\n+\tgit ls-tree --name-only $tree >current &&\n \tcat >expected <<\\EOF &&\n 1.txt\n 2.txt\n@@ -211,7 +213,7 @@ EOF\n '\n \n test_expect_success 'ls-tree --name-only -r' '\n-\tgit ls-tree --name-only -r $tree >current\n+\tgit ls-tree --name-only -r $tree >current &&\n \tcat >expected <<\\EOF &&\n 1.txt\n 2.txt\n-- \n1.7.3.rc1.215.g6997c\n"},{"id":"150513","messageId":"7vd3skf6m4.fsf_-_@alter.siamese.dyndns.org","threadId":"25050","inReplyTo":"7vhbhwf6q5.fsf@alter.siamese.dyndns.org","subject":"[PATCH 2/2] ls-tree $di $dir: do not mistakenly recurse into directories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-11T19:00:19Z","receivedAt":"2010-09-11T19:00:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When applying two pathspecs, one of which is named as a prefix to the\nother, we mistakenly recursed into the shorter one.\n\nNoticed and fixed by David Reis.\n\nSigned-off-by: Junio C Hamano \n---\n\n Junio C Hamano writes:\n\n > Matthieu Moy writes:\n >\n >> That's so close to a real test-case...\n >\n > Let's do this.\n >\n > * t3101 seems somewhat stale; fix the style and add a few missing \" &&\"\n > cascades as a preparatory patch.\n >\n > * Add the \"mistaken prefix computation causes unwarranted recursion\" fix\n > with a test.\n >\n > Here is the first one in such a series.\n\n and here is the second one.\n\n builtin/ls-tree.c | 2 ++\n t/t3101-ls-tree-dirname.sh | 20 +++++++++++++++++++-\n 2 files changed, 21 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex dc86b0d..a818756 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -52,6 +52,8 @@ static int show_recursive(const char *base, int baselen, const char *pathname)\n \t\tspeclen = strlen(spec);\n \t\tif (speclen <= len)\n \t\t\tcontinue;\n+\t\tif (spec[len] && spec[len] != '/')\n+\t\t\tcontinue;\n \t\tif (memcmp(pathname, spec, len))\n \t\t\tcontinue;\n \t\treturn 1;\ndiff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh\nindex 026f9f8..ed8160c 100755\n--- a/t/t3101-ls-tree-dirname.sh\n+++ b/t/t3101-ls-tree-dirname.sh\n@@ -15,6 +15,7 @@ This test runs git ls-tree with the following in a tree.\n path2/1.txt - a file in a directory\n path3/1.txt - a file in a directory\n path3/2.txt - a file in a directory\n+ path30/3.txt - a file in a directory\n \n Test the handling of mulitple directories which have matching file\n entries. Also test odd filename and missing entries handling.\n@@ -24,7 +25,7 @@ entries. Also test odd filename and missing entries handling.\n test_expect_success 'setup' '\n \techo 111 >1.txt &&\n \techo 222 >2.txt &&\n-\tmkdir path0 path0/a path0/a/b path0/a/b/c &&\n+\tmkdir path0 path0/a path0/a/b path0/a/b/c path30 &&\n \techo 111 >path0/a/b/c/1.txt &&\n \tmkdir path1 path1/b path1/b/c &&\n \techo 111 >path1/b/c/1.txt &&\n@@ -33,6 +34,7 @@ test_expect_success 'setup' '\n \tmkdir path3 &&\n \techo 111 >path3/1.txt &&\n \techo 222 >path3/2.txt &&\n+\techo 333 >path30/3.txt &&\n \tfind *.txt path* \\( -type f -o -type l \\) -print |\n \txargs git update-index --add &&\n \ttree=`git write-tree` &&\n@@ -53,6 +55,7 @@ test_expect_success 'ls-tree plain' '\n 040000 tree X\tpath1\n 040000 tree X\tpath2\n 040000 tree X\tpath3\n+040000 tree X\tpath30\n EOF\n \ttest_output\n '\n@@ -68,6 +71,7 @@ test_expect_success 'ls-tree recursive' '\n 100644 blob X\tpath2/1.txt\n 100644 blob X\tpath3/1.txt\n 100644 blob X\tpath3/2.txt\n+100644 blob X\tpath30/3.txt\n EOF\n \ttest_output\n '\n@@ -164,6 +168,7 @@ test_expect_success 'ls-tree --full-tree' '\n 040000 tree X\tpath1\n 040000 tree X\tpath2\n 040000 tree X\tpath3\n+040000 tree X\tpath30\n EOF\n \ttest_output\n '\n@@ -181,6 +186,7 @@ test_expect_success 'ls-tree --full-tree -r' '\n 100644 blob X\tpath2/1.txt\n 100644 blob X\tpath3/1.txt\n 100644 blob X\tpath3/2.txt\n+100644 blob X\tpath30/3.txt\n EOF\n \ttest_output\n '\n@@ -195,6 +201,7 @@ test_expect_success 'ls-tree --abbrev=5' '\n 040000 tree X\tpath1\n 040000 tree X\tpath2\n 040000 tree X\tpath3\n+040000 tree X\tpath30\n EOF\n \ttest_cmp expected check\n '\n@@ -208,6 +215,7 @@ path0\n path1\n path2\n path3\n+path30\n EOF\n \ttest_output\n '\n@@ -222,6 +230,16 @@ path1/b/c/1.txt\n path2/1.txt\n path3/1.txt\n path3/2.txt\n+path30/3.txt\n+EOF\n+\ttest_output\n+'\n+\n+test_expect_success 'ls-tree with two dirnames' '\n+\tgit ls-tree --name-only $tree path3 path30 >current &&\n+\tcat >expected <<\\EOF &&\n+path3\n+path30\n EOF\n \ttest_output\n '\n-- \n1.7.3.rc1.215.g6997c\n"},{"id":"150713","messageId":"vpqmxrk10m3.fsf@bauges.imag.fr","threadId":"25050","inReplyTo":"7vhbhwf6q5.fsf@alter.siamese.dyndns.org","subject":"Re: Do not let lstree output recursively when a directory whose name is a prefix of the others is given in the path list.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-09-14T21:22:44Z","receivedAt":"2010-09-14T21:22:44Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> That's so close to a real test-case...\n>\n> Let's do this.\n>\n> * t3101 seems somewhat stale; fix the style and add a few missing \" &&\"\n> cascades as a preparatory patch.\n>\n> * Add the \"mistaken prefix computation causes unwarranted recursion\" fix\n> with a test.\n\nSounds good, yes.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}