{"thread":{"id":"12559","subject":"[PATCH 0/4] Re-fix get_pathspec()","startedAt":"2008-03-07T08:38:35Z","lastAt":"2008-03-08T09:37:46Z","messageCount":7,"participants":["Junio C Hamano","Johannes Schindelin","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"71325","messageId":"1204879119-7528-1-git-send-email-gitster@pobox.com","threadId":"12559","inReplyTo":null,"subject":"[PATCH 0/4] Re-fix get_pathspec()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-07T08:38:35Z","receivedAt":"2008-03-07T08:38:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"After 1.5.4, we improved get_pathspec() to allow absolute paths, because\npeople wanted to cut&paste from file managers and such, so that you can\nfeed absolute paths to \"git add\" and friends.\n\nHowever, the paths still must be inside work tree, so there must be a way\nto ensure that. d089eba (setup: sanitize absolute and funny paths in\nget_pathspec()) botched the interface, by making the check the\nresponsibility of the callers, while keeping the interface to the function\nintact, which meant that the callers needed to count returned pathspecs\nand compared it with the number of paths fed to the function.\n\nThis miniseries cleans up the interface by making get_pathspec() to die\nagain.\n\nJunio C Hamano (4):\n get_pathspec(): die when an out-of-tree path is given\n Revert part of 744dacd (builtin-mv: minimum fix to avoid losing files)\n Revert part of 1abf095 (git-add: adjust to the get_pathspec() changes)\n Revert part of d089eba (setup: sanitize absolute and funny paths in\n get_pathspec())\n\n builtin-add.c | 12 ------------\n builtin-ls-files.c | 11 +----------\n builtin-mv.c | 6 +-----\n setup.c | 2 ++\n t/t3101-ls-tree-dirname.sh | 2 +-\n t/t7010-setup.sh | 7 ++++---\n 6 files changed, 9 insertions(+), 31 deletions(-)\n\n"},{"id":"71326","messageId":"1204879119-7528-2-git-send-email-gitster@pobox.com","threadId":"12559","inReplyTo":"1204879119-7528-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/4] get_pathspec(): die when an out-of-tree path is given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-07T08:38:36Z","receivedAt":"2008-03-07T08:38:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"An earlier commit d089ebaa (setup: sanitize absolute and funny paths) made\nget_pathspec() aware of absolute paths, but with a botched interface that\nforced the callers to count the resulting pathspecs in order to detect\nan error of giving a path that is outside the work tree.\n\nThis fixes it, by dying inside the function.\n\nWe had ls-tree test that relied on a misfeature in the original\nimplementation of its pathspec handling. Leading slashes were silently\nremoved from them. However we allow giving absolute pathnames (people\nwant to cut and paste from elsewhere) that are inside work tree these\ndays, so a pathspec that begin with slash _should_ be treated as a full\npath. The test is adjusted to match the updated rule for get_pathspec().\n\nEarlier I mistook three tests given by Robin that they should succeed, but\nthese are attempts to add path outside work tree, which should fail\nloudly. These tests also have been fixed.\n\nSigned-off-by: Junio C Hamano \n---\n setup.c | 2 ++\n t/t3101-ls-tree-dirname.sh | 2 +-\n t/t7010-setup.sh | 7 ++++---\n 3 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 89c81e5..c7b1080 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -202,6 +202,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)\n \t\tconst char *p = prefix_path(prefix, prefixlen, *src);\n \t\tif (p)\n \t\t\t*(dst++) = p;\n+\t\telse\n+\t\t\texit(128); /* error message already given */\n \t\tsrc++;\n \t}\n \t*dst = NULL;\ndiff --git a/t/t3101-ls-tree-dirname.sh b/t/t3101-ls-tree-dirname.sh\nindex 39fe267..70f9ce9 100755\n--- a/t/t3101-ls-tree-dirname.sh\n+++ b/t/t3101-ls-tree-dirname.sh\n@@ -120,7 +120,7 @@ EOF\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//1.txt path3 /path3/ path3// >current &&\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 100644 blob X\t1.txt\n 100644 blob X\tpath3/1.txt\ndiff --git a/t/t7010-setup.sh b/t/t7010-setup.sh\nindex e809e0e..bc8ab6a 100755\n--- a/t/t7010-setup.sh\n+++ b/t/t7010-setup.sh\n@@ -142,15 +142,16 @@ test_expect_success 'setup deeper work tree' '\n test_expect_success 'add a directory outside the work tree' '(\n \tcd tester &&\n \td1=\"$(cd .. ; pwd)\" &&\n-\tgit add \"$d1\"\n+\ttest_must_fail git add \"$d1\"\n )'\n \n+\n test_expect_success 'add a file outside the work tree, nasty case 1' '(\n \tcd tester &&\n \tf=\"$(pwd)x\" &&\n \techo \"$f\" &&\n \ttouch \"$f\" &&\n-\tgit add \"$f\"\n+\ttest_must_fail git add \"$f\"\n )'\n \n test_expect_success 'add a file outside the work tree, nasty case 2' '(\n@@ -158,7 +159,7 @@ test_expect_success 'add a file outside the work tree, nasty case 2' '(\n \tf=\"$(pwd | sed \"s/.$//\")x\" &&\n \techo \"$f\" &&\n \ttouch \"$f\" &&\n-\tgit add \"$f\"\n+\ttest_must_fail git add \"$f\"\n )'\n \n test_done\n-- \n1.5.4.3.587.g0bdd73\n\n"},{"id":"71336","messageId":"alpine.LSU.1.00.0803071252240.19395@racer.site","threadId":"12559","inReplyTo":"1204879119-7528-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/4] Re-fix get_pathspec()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-07T11:53:02Z","receivedAt":"2008-03-07T11:53:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 7 Mar 2008, Junio C Hamano wrote:\n\n> After 1.5.4, we improved get_pathspec() to allow absolute paths, because\n> people wanted to cut&paste from file managers and such, so that you can\n> feed absolute paths to \"git add\" and friends.\n> \n> However, the paths still must be inside work tree, so there must be a \n> way to ensure that. d089eba (setup: sanitize absolute and funny paths \n> in get_pathspec()) botched the interface, by making the check the \n> responsibility of the callers, while keeping the interface to the \n> function intact, which meant that the callers needed to count returned \n> pathspecs and compared it with the number of paths fed to the function.\n> \n> This miniseries cleans up the interface by making get_pathspec() to die\n> again.\n\nWe have a few places where you have to pass a \"die_on_error\" flag. Why \nnot imitate that?\n\nCiao,\nDscho\n\n"},{"id":"71362","messageId":"7v1w6m2zoq.fsf@gitster.siamese.dyndns.org","threadId":"12559","inReplyTo":"alpine.LSU.1.00.0803071252240.19395@racer.site","subject":"Re: [PATCH 0/4] Re-fix get_pathspec()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-07T17:48:05Z","receivedAt":"2008-03-07T17:48:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Fri, 7 Mar 2008, Junio C Hamano wrote:\n>\n>> This miniseries cleans up the interface by making get_pathspec() to die\n>> again.\n>\n> We have a few places where you have to pass a \"die_on_error\" flag. Why \n> not imitate that?\n\nBefore get_pathspec() was made absolute-path capable, it died on a path\nthat is outside the work tree, so we know die() there is what the callers\nhave expected all along. This miniseries is about fixing the earlier one\nthat broke that expectation, nothing more.\n\nIf enough callers want different behaviours on error, it probably is worth\nan API enhancement so that the callers can choose to do their own thing,\nbut that is a separate topic I haven't seen the need for, and definitely\nis not part of this fix.\n\n"},{"id":"71369","messageId":"200803071929.42499.robin.rosenberg@dewire.com","threadId":"12559","inReplyTo":"1204879119-7528-2-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 1/4] get_pathspec(): die when an out-of-tree path is given","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-03-07T18:29:42Z","receivedAt":"2008-03-07T18:29:42Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"My gcc refuses to cooperate, giving me SEGV at line 666 or combine-diff, so \nI'll ask instead of testing.\n\nDo absolute paths to diff still work without exiting?\n\n-- robin\n"},{"id":"71372","messageId":"7vskz21i7f.fsf@gitster.siamese.dyndns.org","threadId":"12559","inReplyTo":"200803071929.42499.robin.rosenberg@dewire.com","subject":"Re: [PATCH 1/4] get_pathspec(): die when an out-of-tree path is given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-07T18:51:00Z","receivedAt":"2008-03-07T18:51:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg writes:\n\n> My gcc refuses to cooperate, giving me SEGV at line 666 or combine-diff, so \n> I'll ask instead of testing.\n>\n> Do absolute paths to diff still work without exiting?\n\nThey should, as long as they name inside work tree paths.\n\nAs far as I know, the bolted-on \"work on files outside\" code does not feed\nthe paths to get_pathspec() (and it shouldn't --- that mode is not a git\noperation to begin with, and should work even without having a work tree,\nso there is no valid base to make the paths relative to).\n\nBut it needs to be checked.\n\n"},{"id":"71409","messageId":"7vve3xv9n9.fsf@gitster.siamese.dyndns.org","threadId":"12559","inReplyTo":"1204879119-7528-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/4] Re-fix get_pathspec()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-08T09:37:46Z","receivedAt":"2008-03-08T09:37:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> ...\n> This miniseries cleans up the interface by making get_pathspec() to die\n> again.\n\nBy the way, this is meant to also fix the breakage of \"git clean\" run with\na path outside the work tree. With two patches from Dmitry:\n\n Make private quote_path() in wt-status.c available as quote_path_relative())\n git-clean: correct printing relative path)\n\non top of the series, the following tests now pass.\n\n-- >8 --\ngit-clean: add tests for relative path\n\nThis adds tests for recent change by Dmitry to fix the report \"git\nclean\" gives on removed paths, and also makes sure the command detects\npaths that is outside working tree.\n\nSigned-off-by: Junio C Hamano \n---\n t/t7300-clean.sh | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 52 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7300-clean.sh b/t/t7300-clean.sh\nindex 4037142..afccfc9 100755\n--- a/t/t7300-clean.sh\n+++ b/t/t7300-clean.sh\n@@ -89,6 +89,58 @@ test_expect_success 'git-clean with prefix' '\n \ttest -f build/lib.so\n \n '\n+\n+test_expect_success 'git-clean with relative prefix' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&\n+\twould_clean=$(\n+\t\tcd docs &&\n+\t\tgit clean -n ../src |\n+\t\tsed -n -e \"s|^Would remove ||p\"\n+\t) &&\n+\ttest \"$would_clean\" = ../src/part3.c || {\n+\t\techo \"OOps <$would_clean>\"\n+\t\tfalse\n+\t}\n+'\n+\n+test_expect_success 'git-clean with absolute path' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&\n+\twould_clean=$(\n+\t\tcd docs &&\n+\t\tgit clean -n $(pwd)/../src |\n+\t\tsed -n -e \"s|^Would remove ||p\"\n+\t) &&\n+\ttest \"$would_clean\" = ../src/part3.c || {\n+\t\techo \"OOps <$would_clean>\"\n+\t\tfalse\n+\t}\n+'\n+\n+test_expect_success 'git-clean with out of work tree relative path' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&\n+\t(\n+\t\tcd docs &&\n+\t\ttest_must_fail git clean -n ../..\n+\t)\n+'\n+\n+test_expect_success 'git-clean with out of work tree absolute path' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c docs/manual.txt obj.o build/lib.so &&\n+\tdd=$(cd .. && pwd) &&\n+\t(\n+\t\tcd docs &&\n+\t\ttest_must_fail git clean -n $dd\n+\t)\n+'\n+\n test_expect_success 'git-clean -d with prefix and path' '\n \n \tmkdir -p build docs src/feature &&\n-- \n1.5.4.3.587.g0bdd73\n\n\n\n\n\n"}]}