{"thread":{"id":"38244","subject":"[PATCH 1/5] t2004: modernize style","startedAt":"2014-12-24T09:43:11Z","lastAt":"2014-12-29T18:58:42Z","messageCount":8,"participants":["Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"254071","messageId":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":null,"subject":"[PATCH 0/5] resolve checkout-index --temp path mangling","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:11Z","receivedAt":"2014-12-24T09:43:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"This patch series fixes a path mangling bug in checkout-index --temp\nreported by Russ Cox[1]. As a bonus, it also squashes a bug in which\ncheckout-index was accessing and printing memory beyond end-of-string.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/261739\n\nEric Sunshine (5):\n  t2004: modernize style\n  t2004: drop unnecessary write-tree/read-tree\n  t2004: standardize file naming in symlink test\n  t2004: demonstrate broken relative path printing\n  checkout-index: fix --temp relative path mangling\n\n builtin/checkout-index.c       |  16 +-\n t/t2004-checkout-cache-temp.sh | 400 +++++++++++++++++++++--------------------\n 2 files changed, 213 insertions(+), 203 deletions(-)\n\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254070","messageId":"1419414196-58587-2-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 1/5] t2004: modernize style","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:12Z","receivedAt":"2014-12-24T09:43:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"In particular:\n\n* indent test body\n* place test description on same line as test_expect_*\n* place closing quote on its own line\n* name output file \"actual\" rather than \"out\"\n* name setup test \"setup\" rather than \"preparation\"\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t2004-checkout-cache-temp.sh | 391 +++++++++++++++++++++--------------------\n 1 file changed, 196 insertions(+), 195 deletions(-)\n\ndiff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh\nindex f171a55..28e50d6 100755\n--- a/t/t2004-checkout-cache-temp.sh\n+++ b/t/t2004-checkout-cache-temp.sh\n@@ -10,202 +10,203 @@ rather than the tracked path.'\n \n . ./test-lib.sh\n \n-test_expect_success \\\n-'preparation' '\n-mkdir asubdir &&\n-echo tree1path0 >path0 &&\n-echo tree1path1 >path1 &&\n-echo tree1path3 >path3 &&\n-echo tree1path4 >path4 &&\n-echo tree1asubdir/path5 >asubdir/path5 &&\n-git update-index --add path0 path1 path3 path4 asubdir/path5 &&\n-t1=$(git write-tree) &&\n-rm -f path* .merge_* out .git/index &&\n-echo tree2path0 >path0 &&\n-echo tree2path1 >path1 &&\n-echo tree2path2 >path2 &&\n-echo tree2path4 >path4 &&\n-git update-index --add path0 path1 path2 path4 &&\n-t2=$(git write-tree) &&\n-rm -f path* .merge_* out .git/index &&\n-echo tree2path0 >path0 &&\n-echo tree3path1 >path1 &&\n-echo tree3path2 >path2 &&\n-echo tree3path3 >path3 &&\n-git update-index --add path0 path1 path2 path3 &&\n-t3=$(git write-tree)'\n-\n-test_expect_success \\\n-'checkout one stage 0 to temporary file' '\n-rm -f path* .merge_* out .git/index &&\n-git read-tree $t1 &&\n-git checkout-index --temp -- path1 >out &&\n-test_line_count = 1 out &&\n-test $(cut \"-d\t\" -f2 out) = path1 &&\n-p=$(cut \"-d\t\" -f1 out) &&\n-test -f $p &&\n-test $(cat $p) = tree1path1'\n-\n-test_expect_success \\\n-'checkout all stage 0 to temporary files' '\n-rm -f path* .merge_* out .git/index &&\n-git read-tree $t1 &&\n-git checkout-index -a --temp >out &&\n-test_line_count = 5 out &&\n-for f in path0 path1 path3 path4 asubdir/path5\n-do\n-\ttest $(grep $f out | cut \"-d\t\" -f2) = $f &&\n-\tp=$(grep $f out | cut \"-d\t\" -f1) &&\n+test_expect_success 'setup' '\n+\tmkdir asubdir &&\n+\techo tree1path0 >path0 &&\n+\techo tree1path1 >path1 &&\n+\techo tree1path3 >path3 &&\n+\techo tree1path4 >path4 &&\n+\techo tree1asubdir/path5 >asubdir/path5 &&\n+\tgit update-index --add path0 path1 path3 path4 asubdir/path5 &&\n+\tt1=$(git write-tree) &&\n+\trm -f path* .merge_* actual .git/index &&\n+\techo tree2path0 >path0 &&\n+\techo tree2path1 >path1 &&\n+\techo tree2path2 >path2 &&\n+\techo tree2path4 >path4 &&\n+\tgit update-index --add path0 path1 path2 path4 &&\n+\tt2=$(git write-tree) &&\n+\trm -f path* .merge_* actual .git/index &&\n+\techo tree2path0 >path0 &&\n+\techo tree3path1 >path1 &&\n+\techo tree3path2 >path2 &&\n+\techo tree3path3 >path3 &&\n+\tgit update-index --add path0 path1 path2 path3 &&\n+\tt3=$(git write-tree)\n+'\n+\n+test_expect_success 'checkout one stage 0 to temporary file' '\n+\trm -f path* .merge_* actual .git/index &&\n+\tgit read-tree $t1 &&\n+\tgit checkout-index --temp -- path1 >actual &&\n+\ttest_line_count = 1 actual &&\n+\ttest $(cut \"-d\t\" -f2 actual) = path1 &&\n+\tp=$(cut \"-d\t\" -f1 actual) &&\n \ttest -f $p &&\n-\ttest $(cat $p) = tree1$f\n-done'\n-\n-test_expect_success \\\n-'prepare 3-way merge' '\n-rm -f path* .merge_* out .git/index &&\n-git read-tree -m $t1 $t2 $t3'\n-\n-test_expect_success \\\n-'checkout one stage 2 to temporary file' '\n-rm -f path* .merge_* out &&\n-git checkout-index --stage=2 --temp -- path1 >out &&\n-test_line_count = 1 out &&\n-test $(cut \"-d\t\" -f2 out) = path1 &&\n-p=$(cut \"-d\t\" -f1 out) &&\n-test -f $p &&\n-test $(cat $p) = tree2path1'\n-\n-test_expect_success \\\n-'checkout all stage 2 to temporary files' '\n-rm -f path* .merge_* out &&\n-git checkout-index --all --stage=2 --temp >out &&\n-test_line_count = 3 out &&\n-for f in path1 path2 path4\n-do\n-\ttest $(grep $f out | cut \"-d\t\" -f2) = $f &&\n-\tp=$(grep $f out | cut \"-d\t\" -f1) &&\n+\ttest $(cat $p) = tree1path1\n+'\n+\n+test_expect_success 'checkout all stage 0 to temporary files' '\n+\trm -f path* .merge_* actual .git/index &&\n+\tgit read-tree $t1 &&\n+\tgit checkout-index -a --temp >actual &&\n+\ttest_line_count = 5 actual &&\n+\tfor f in path0 path1 path3 path4 asubdir/path5\n+\tdo\n+\t\ttest $(grep $f actual | cut \"-d\t\" -f2) = $f &&\n+\t\tp=$(grep $f actual | cut \"-d\t\" -f1) &&\n+\t\ttest -f $p &&\n+\t\ttest $(cat $p) = tree1$f\n+\tdone\n+'\n+\n+test_expect_success 'setup 3-way merge' '\n+\trm -f path* .merge_* actual .git/index &&\n+\tgit read-tree -m $t1 $t2 $t3\n+'\n+\n+test_expect_success 'checkout one stage 2 to temporary file' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index --stage=2 --temp -- path1 >actual &&\n+\ttest_line_count = 1 actual &&\n+\ttest $(cut \"-d\t\" -f2 actual) = path1 &&\n+\tp=$(cut \"-d\t\" -f1 actual) &&\n \ttest -f $p &&\n-\ttest $(cat $p) = tree2$f\n-done'\n-\n-test_expect_success \\\n-'checkout all stages/one file to nothing' '\n-rm -f path* .merge_* out &&\n-git checkout-index --stage=all --temp -- path0 >out &&\n-test_line_count = 0 out'\n-\n-test_expect_success \\\n-'checkout all stages/one file to temporary files' '\n-rm -f path* .merge_* out &&\n-git checkout-index --stage=all --temp -- path1 >out &&\n-test_line_count = 1 out &&\n-test $(cut \"-d\t\" -f2 out) = path1 &&\n-cut \"-d\t\" -f1 out | (read s1 s2 s3 &&\n-test -f $s1 &&\n-test -f $s2 &&\n-test -f $s3 &&\n-test $(cat $s1) = tree1path1 &&\n-test $(cat $s2) = tree2path1 &&\n-test $(cat $s3) = tree3path1)'\n-\n-test_expect_success \\\n-'checkout some stages/one file to temporary files' '\n-rm -f path* .merge_* out &&\n-git checkout-index --stage=all --temp -- path2 >out &&\n-test_line_count = 1 out &&\n-test $(cut \"-d\t\" -f2 out) = path2 &&\n-cut \"-d\t\" -f1 out | (read s1 s2 s3 &&\n-test $s1 = . &&\n-test -f $s2 &&\n-test -f $s3 &&\n-test $(cat $s2) = tree2path2 &&\n-test $(cat $s3) = tree3path2)'\n-\n-test_expect_success \\\n-'checkout all stages/all files to temporary files' '\n-rm -f path* .merge_* out &&\n-git checkout-index -a --stage=all --temp >out &&\n-test_line_count = 5 out'\n-\n-test_expect_success \\\n-'-- path0: no entry' '\n-test x$(grep path0 out | cut \"-d\t\" -f2) = x'\n-\n-test_expect_success \\\n-'-- path1: all 3 stages' '\n-test $(grep path1 out | cut \"-d\t\" -f2) = path1 &&\n-grep path1 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n-test -f $s1 &&\n-test -f $s2 &&\n-test -f $s3 &&\n-test $(cat $s1) = tree1path1 &&\n-test $(cat $s2) = tree2path1 &&\n-test $(cat $s3) = tree3path1)'\n-\n-test_expect_success \\\n-'-- path2: no stage 1, have stage 2 and 3' '\n-test $(grep path2 out | cut \"-d\t\" -f2) = path2 &&\n-grep path2 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n-test $s1 = . &&\n-test -f $s2 &&\n-test -f $s3 &&\n-test $(cat $s2) = tree2path2 &&\n-test $(cat $s3) = tree3path2)'\n-\n-test_expect_success \\\n-'-- path3: no stage 2, have stage 1 and 3' '\n-test $(grep path3 out | cut \"-d\t\" -f2) = path3 &&\n-grep path3 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n-test -f $s1 &&\n-test $s2 = . &&\n-test -f $s3 &&\n-test $(cat $s1) = tree1path3 &&\n-test $(cat $s3) = tree3path3)'\n-\n-test_expect_success \\\n-'-- path4: no stage 3, have stage 1 and 3' '\n-test $(grep path4 out | cut \"-d\t\" -f2) = path4 &&\n-grep path4 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n-test -f $s1 &&\n-test -f $s2 &&\n-test $s3 = . &&\n-test $(cat $s1) = tree1path4 &&\n-test $(cat $s2) = tree2path4)'\n-\n-test_expect_success \\\n-'-- asubdir/path5: no stage 2 and 3 have stage 1' '\n-test $(grep asubdir/path5 out | cut \"-d\t\" -f2) = asubdir/path5 &&\n-grep asubdir/path5 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n-test -f $s1 &&\n-test $s2 = . &&\n-test $s3 = . &&\n-test $(cat $s1) = tree1asubdir/path5)'\n-\n-test_expect_success \\\n-'checkout --temp within subdir' '\n-(cd asubdir &&\n- git checkout-index -a --stage=all >out &&\n- test_line_count = 1 out &&\n- test $(grep path5 out | cut \"-d\t\" -f2) = path5 &&\n- grep path5 out | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n- test -f ../$s1 &&\n- test $s2 = . &&\n- test $s3 = . &&\n- test $(cat ../$s1) = tree1asubdir/path5)\n-)'\n-\n-test_expect_success \\\n-'checkout --temp symlink' '\n-rm -f path* .merge_* out .git/index &&\n-test_ln_s_add b a &&\n-t4=$(git write-tree) &&\n-rm -f .git/index &&\n-git read-tree $t4 &&\n-git checkout-index --temp -a >out &&\n-test_line_count = 1 out &&\n-test $(cut \"-d\t\" -f2 out) = a &&\n-p=$(cut \"-d\t\" -f1 out) &&\n-test -f $p &&\n-test $(cat $p) = b'\n+\ttest $(cat $p) = tree2path1\n+'\n+\n+test_expect_success 'checkout all stage 2 to temporary files' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index --all --stage=2 --temp >actual &&\n+\ttest_line_count = 3 actual &&\n+\tfor f in path1 path2 path4\n+\tdo\n+\t\ttest $(grep $f actual | cut \"-d\t\" -f2) = $f &&\n+\t\tp=$(grep $f actual | cut \"-d\t\" -f1) &&\n+\t\ttest -f $p &&\n+\t\ttest $(cat $p) = tree2$f\n+\tdone\n+'\n+\n+test_expect_success 'checkout all stages/one file to nothing' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index --stage=all --temp -- path0 >actual &&\n+\ttest_line_count = 0 actual\n+'\n+\n+test_expect_success 'checkout all stages/one file to temporary files' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index --stage=all --temp -- path1 >actual &&\n+\ttest_line_count = 1 actual &&\n+\ttest $(cut \"-d\t\" -f2 actual) = path1 &&\n+\tcut \"-d\t\" -f1 actual | (read s1 s2 s3 &&\n+\ttest -f $s1 &&\n+\ttest -f $s2 &&\n+\ttest -f $s3 &&\n+\ttest $(cat $s1) = tree1path1 &&\n+\ttest $(cat $s2) = tree2path1 &&\n+\ttest $(cat $s3) = tree3path1)\n+'\n+\n+test_expect_success 'checkout some stages/one file to temporary files' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index --stage=all --temp -- path2 >actual &&\n+\ttest_line_count = 1 actual &&\n+\ttest $(cut \"-d\t\" -f2 actual) = path2 &&\n+\tcut \"-d\t\" -f1 actual | (read s1 s2 s3 &&\n+\ttest $s1 = . &&\n+\ttest -f $s2 &&\n+\ttest -f $s3 &&\n+\ttest $(cat $s2) = tree2path2 &&\n+\ttest $(cat $s3) = tree3path2)\n+'\n+\n+test_expect_success 'checkout all stages/all files to temporary files' '\n+\trm -f path* .merge_* actual &&\n+\tgit checkout-index -a --stage=all --temp >actual &&\n+\ttest_line_count = 5 actual\n+'\n+\n+test_expect_success '-- path0: no entry' '\n+\ttest x$(grep path0 actual | cut \"-d\t\" -f2) = x\n+'\n+\n+test_expect_success '-- path1: all 3 stages' '\n+\ttest $(grep path1 actual | cut \"-d\t\" -f2) = path1 &&\n+\tgrep path1 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\ttest -f $s1 &&\n+\ttest -f $s2 &&\n+\ttest -f $s3 &&\n+\ttest $(cat $s1) = tree1path1 &&\n+\ttest $(cat $s2) = tree2path1 &&\n+\ttest $(cat $s3) = tree3path1)\n+'\n+\n+test_expect_success '-- path2: no stage 1, have stage 2 and 3' '\n+\ttest $(grep path2 actual | cut \"-d\t\" -f2) = path2 &&\n+\tgrep path2 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\ttest $s1 = . &&\n+\ttest -f $s2 &&\n+\ttest -f $s3 &&\n+\ttest $(cat $s2) = tree2path2 &&\n+\ttest $(cat $s3) = tree3path2)\n+'\n+\n+test_expect_success '-- path3: no stage 2, have stage 1 and 3' '\n+\ttest $(grep path3 actual | cut \"-d\t\" -f2) = path3 &&\n+\tgrep path3 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\ttest -f $s1 &&\n+\ttest $s2 = . &&\n+\ttest -f $s3 &&\n+\ttest $(cat $s1) = tree1path3 &&\n+\ttest $(cat $s3) = tree3path3)\n+'\n+\n+test_expect_success '-- path4: no stage 3, have stage 1 and 3' '\n+\ttest $(grep path4 actual | cut \"-d\t\" -f2) = path4 &&\n+\tgrep path4 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\ttest -f $s1 &&\n+\ttest -f $s2 &&\n+\ttest $s3 = . &&\n+\ttest $(cat $s1) = tree1path4 &&\n+\ttest $(cat $s2) = tree2path4)\n+'\n+\n+test_expect_success '-- asubdir/path5: no stage 2 and 3 have stage 1' '\n+\ttest $(grep asubdir/path5 actual | cut \"-d\t\" -f2) = asubdir/path5 &&\n+\tgrep asubdir/path5 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\ttest -f $s1 &&\n+\ttest $s2 = . &&\n+\ttest $s3 = . &&\n+\ttest $(cat $s1) = tree1asubdir/path5)\n+'\n+\n+test_expect_success 'checkout --temp within subdir' '\n+\t(\n+\t\tcd asubdir &&\n+\t\tgit checkout-index -a --stage=all >actual &&\n+\t\ttest_line_count = 1 actual &&\n+\t\ttest $(grep path5 actual | cut \"-d\t\" -f2) = path5 &&\n+\t\tgrep path5 actual | cut \"-d\t\" -f1 | (read s1 s2 s3 &&\n+\t\ttest -f ../$s1 &&\n+\t\ttest $s2 = . &&\n+\t\ttest $s3 = . &&\n+\t\ttest $(cat ../$s1) = tree1asubdir/path5)\n+\t)\n+'\n+\n+test_expect_success 'checkout --temp symlink' '\n+\trm -f path* .merge_* actual .git/index &&\n+\ttest_ln_s_add b a &&\n+\tt4=$(git write-tree) &&\n+\trm -f .git/index &&\n+\tgit read-tree $t4 &&\n+\tgit checkout-index --temp -a >actual &&\n+\ttest_line_count = 1 actual &&\n+\ttest $(cut \"-d\t\" -f2 actual) = a &&\n+\tp=$(cut \"-d\t\" -f1 actual) &&\n+\ttest -f $p &&\n+\ttest $(cat $p) = b\n+'\n \n test_done\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254075","messageId":"1419414196-58587-3-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 2/5] t2004: drop unnecessary write-tree/read-tree","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:13Z","receivedAt":"2014-12-24T09:43:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Unlike earlier tests which reference several trees prepared by \"setup\",\nno other tests utilize the tree from the \"symlink\" test, so there is no\nneed to write it (or read it back immediately).\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t2004-checkout-cache-temp.sh | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh\nindex 28e50d6..22d1a24 100755\n--- a/t/t2004-checkout-cache-temp.sh\n+++ b/t/t2004-checkout-cache-temp.sh\n@@ -198,9 +198,6 @@ test_expect_success 'checkout --temp within subdir' '\n test_expect_success 'checkout --temp symlink' '\n \trm -f path* .merge_* actual .git/index &&\n \ttest_ln_s_add b a &&\n-\tt4=$(git write-tree) &&\n-\trm -f .git/index &&\n-\tgit read-tree $t4 &&\n \tgit checkout-index --temp -a >actual &&\n \ttest_line_count = 1 actual &&\n \ttest $(cut \"-d\t\" -f2 actual) = a &&\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254073","messageId":"1419414196-58587-4-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 3/5] t2004: standardize file naming in symlink test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:14Z","receivedAt":"2014-12-24T09:43:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Update \"symlink\" test to use the common file naming scheme so that its\ntemporary files can be cleaned up by the \"rm -f path*\" idiom employed by\nother tests in this script.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t2004-checkout-cache-temp.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh\nindex 22d1a24..602fbc7 100755\n--- a/t/t2004-checkout-cache-temp.sh\n+++ b/t/t2004-checkout-cache-temp.sh\n@@ -197,13 +197,13 @@ test_expect_success 'checkout --temp within subdir' '\n \n test_expect_success 'checkout --temp symlink' '\n \trm -f path* .merge_* actual .git/index &&\n-\ttest_ln_s_add b a &&\n+\ttest_ln_s_add path7 path6 &&\n \tgit checkout-index --temp -a >actual &&\n \ttest_line_count = 1 actual &&\n-\ttest $(cut \"-d\t\" -f2 actual) = a &&\n+\ttest $(cut \"-d\t\" -f2 actual) = path6 &&\n \tp=$(cut \"-d\t\" -f1 actual) &&\n \ttest -f $p &&\n-\ttest $(cat $p) = b\n+\ttest $(cat $p) = path7\n '\n \n test_done\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254074","messageId":"1419414196-58587-5-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 4/5] t2004: demonstrate broken relative path printing","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:15Z","receivedAt":"2014-12-24T09:43:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"checkout-index --temp only properly prints relative paths which are\ndescendants of the current directory. Paths in ancestor or sibling\ndirectories (or their children) are often printed in mangled form. For\nexample:\n\n    mkdir a bbb &&\n    >file &&\n    >bbb/file &&\n    git update-index --add file bbb/file &&\n    cd a &&\n    git checkout-index --temp ../file ../bbb/file\n\nprints:\n\n    .merge_file_ooblek  le\n    .merge_file_igloo0  b/file\n\nrather than the correct:\n\n    .merge_file_ooblek  ../file\n    .merge_file_igloo0  ../bbb/file\n\nUnfortunately, testing is complicated slightly by relative paths\nsometimes _appearing_ to be printed correctly, but this is an accident\nof implementation in which a \"correct\" copy of the string exists in\nmemory beyond the end of the real string, and that \"correct\" copy gets\nprinted. This test takes care to avoid the accidentally \"correct\"\nbehavior by testing with a filename longer than the directory name in\nwhich checkout-index is invoked.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n t/t2004-checkout-cache-temp.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh\nindex 602fbc7..3c2b2b9 100755\n--- a/t/t2004-checkout-cache-temp.sh\n+++ b/t/t2004-checkout-cache-temp.sh\n@@ -206,4 +206,16 @@ test_expect_success 'checkout --temp symlink' '\n \ttest $(cat $p) = path7\n '\n \n+test_expect_failure 'emit well-formed relative path' '\n+\trm -f path* .merge_* actual .git/index &&\n+\t>path0123456789 &&\n+\tgit update-index --add path0123456789 &&\n+\t(\n+\t\tcd asubdir &&\n+\t\tgit checkout-index --temp -- ../path0123456789 >actual &&\n+\t\ttest_line_count = 1 actual &&\n+\t\ttest $(cut \"-d\t\" -f2 actual) = ../path0123456789\n+\t)\n+'\n+\n test_done\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254072","messageId":"1419414196-58587-6-git-send-email-sunshine@sunshineco.com","threadId":"38244","inReplyTo":"1419414196-58587-1-git-send-email-sunshine@sunshineco.com","subject":"[PATCH 5/5] checkout-index: fix --temp relative path mangling","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-24T09:43:16Z","receivedAt":"2014-12-24T09:43:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"checkout-index --temp only properly prints relative paths which are\ndescendants of the current directory. Paths in ancestor or sibling\ndirectories (or their children) are often printed in mangled form. For\nexample:\n\n    mkdir a bbb &&\n    >file &&\n    >bbb/file &&\n    git update-index --add file bbb/file &&\n    cd a &&\n    git checkout-index --temp ../file ../bbb/file\n\nprints:\n\n    .merge_file_ooblek  le\n    .merge_file_igloo0  b/file\n\nrather than the correct:\n\n    .merge_file_ooblek  ../file\n    .merge_file_igloo0  ../bbb/file\n\nInternally, given the above example, checkout-index prefixes each input\nargument with the name of the current directory (\"a/\", in this case),\nand then assumes that it can simply skip forward by strlen(\"a/\") bytes\nto recover the original name. This works for files in the current\ndirectory or its descendants, but fails for files in ancestors or\nsiblings (or their children) due to path normalization.\n\nFor instance, given \"../file\", \"a/\" is appended, giving \"a/../file\".\nPath normalization folds out \"a/../\", resulting in \"file\". Attempting\nto recover the original name by skipping strlen(\"a/\") bytes gives the\nincorrect \"le\" rather than the desired \"../file\".\n\nFix this by taking advantage of write_name_quoted_relative() to recover\nthe original name properly, rather than assuming that it can be\nrecovered by skipping strlen(prefix) bytes.\n\nAs a bonus, this also fixes a bug in which checkout-index --temp\naccessed and printed memory beyond the end-of-string. For instance,\nwithin a subdirectory named \"subdirectory\", and given argument\n\"../file\", prefixing would give \"subdirectory/../file\", which would\nbecome \"file\" after normalization. checkout-index would then attempt to\nrecover the original name by skipping strlen(\"subdirectory/\") bytes of\n\"file\", which placed it well beyond end-of-string. Despite this error,\nit often appeared to give the correct result, but only due to an\naccident of implementation which left an apparently correct copy of the\npath in memory following the normalized value. In particular, handed\n\"subdirectory/../file\", in-place processing by normalize_path_copy_len()\nresulted in \"file\\0rectory/../file\". When checkout-index skipped\nstrlen(\"subdirectory/\") bytes, it ended up back at \"../file\" and thus\nappeared to give the correct answer, despite being past end-of-string.\n\nReported-by: Russ Cox <rsc@golang.org>\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n builtin/checkout-index.c       | 16 ++++++++--------\n t/t2004-checkout-cache-temp.sh |  2 +-\n 2 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex 383dccf..031780f 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -18,7 +18,7 @@ static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];\n \n static struct checkout state;\n \n-static void write_tempfile_record(const char *name, int prefix_length)\n+static void write_tempfile_record(const char *name, const char *prefix)\n {\n \tint i;\n \n@@ -35,14 +35,14 @@ static void write_tempfile_record(const char *name, int prefix_length)\n \t\tfputs(topath[checkout_stage], stdout);\n \n \tputchar('\\t');\n-\twrite_name_quoted(name + prefix_length, stdout, line_termination);\n+\twrite_name_quoted_relative(name, prefix, stdout, line_termination);\n \n \tfor (i = 0; i < 4; i++) {\n \t\ttopath[i][0] = 0;\n \t}\n }\n \n-static int checkout_file(const char *name, int prefix_length)\n+static int checkout_file(const char *name, const char *prefix)\n {\n \tint namelen = strlen(name);\n \tint pos = cache_name_pos(name, namelen);\n@@ -71,7 +71,7 @@ static int checkout_file(const char *name, int prefix_length)\n \n \tif (did_checkout) {\n \t\tif (to_tempfile)\n-\t\t\twrite_tempfile_record(name, prefix_length);\n+\t\t\twrite_tempfile_record(name, prefix);\n \t\treturn errs > 0 ? -1 : 0;\n \t}\n \n@@ -106,7 +106,7 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\tif (last_ce && to_tempfile) {\n \t\t\tif (ce_namelen(last_ce) != ce_namelen(ce)\n \t\t\t    || memcmp(last_ce->name, ce->name, ce_namelen(ce)))\n-\t\t\t\twrite_tempfile_record(last_ce->name, prefix_length);\n+\t\t\t\twrite_tempfile_record(last_ce->name, prefix);\n \t\t}\n \t\tif (checkout_entry(ce, &state,\n \t\t    to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)\n@@ -114,7 +114,7 @@ static void checkout_all(const char *prefix, int prefix_length)\n \t\tlast_ce = ce;\n \t}\n \tif (last_ce && to_tempfile)\n-\t\twrite_tempfile_record(last_ce->name, prefix_length);\n+\t\twrite_tempfile_record(last_ce->name, prefix);\n \tif (errs)\n \t\t/* we have already done our error reporting.\n \t\t * exit with the same code as die().\n@@ -248,7 +248,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\tif (read_from_stdin)\n \t\t\tdie(\"git checkout-index: don't mix '--stdin' and explicit filenames\");\n \t\tp = prefix_path(prefix, prefix_length, arg);\n-\t\tcheckout_file(p, prefix_length);\n+\t\tcheckout_file(p, prefix);\n \t\tif (p < arg || p > arg + strlen(arg))\n \t\t\tfree((char *)p);\n \t}\n@@ -268,7 +268,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t\t\t\tstrbuf_swap(&buf, &nbuf);\n \t\t\t}\n \t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n-\t\t\tcheckout_file(p, prefix_length);\n+\t\t\tcheckout_file(p, prefix);\n \t\t\tif (p < buf.buf || p > buf.buf + buf.len)\n \t\t\t\tfree((char *)p);\n \t\t}\ndiff --git a/t/t2004-checkout-cache-temp.sh b/t/t2004-checkout-cache-temp.sh\nindex 3c2b2b9..a12afe9 100755\n--- a/t/t2004-checkout-cache-temp.sh\n+++ b/t/t2004-checkout-cache-temp.sh\n@@ -206,7 +206,7 @@ test_expect_success 'checkout --temp symlink' '\n \ttest $(cat $p) = path7\n '\n \n-test_expect_failure 'emit well-formed relative path' '\n+test_expect_success 'emit well-formed relative path' '\n \trm -f path* .merge_* actual .git/index &&\n \t>path0123456789 &&\n \tgit update-index --add path0123456789 &&\n-- \n2.2.1.267.g0ad48de\n"},{"id":"254128","messageId":"CAPig+cTCQgdX0FgZCiL921bAycEoceeKXT5UpnD7_Ro=NJH0Yg@mail.gmail.com","threadId":"38244","inReplyTo":"1419414196-58587-6-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 5/5] checkout-index: fix --temp relative path mangling","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-12-28T03:19:48Z","receivedAt":"2014-12-28T03:19:48Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Dec 24, 2014 at 4:43 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:\n> checkout-index --temp only properly prints relative paths which are\n> descendants of the current directory. Paths in ancestor or sibling\n> directories (or their children) are often printed in mangled form. For\n> example:\n>\n>     mkdir a bbb &&\n>     >file &&\n>     >bbb/file &&\n>     git update-index --add file bbb/file &&\n>     cd a &&\n>     git checkout-index --temp ../file ../bbb/file\n>\n> prints:\n>\n>     .merge_file_ooblek  le\n>     .merge_file_igloo0  b/file\n>\n> rather than the correct:\n>\n>     .merge_file_ooblek  ../file\n>     .merge_file_igloo0  ../bbb/file\n>\n> Internally, given the above example, checkout-index prefixes each input\n> argument with the name of the current directory (\"a/\", in this case),\n> and then assumes that it can simply skip forward by strlen(\"a/\") bytes\n> to recover the original name. This works for files in the current\n> directory or its descendants, but fails for files in ancestors or\n> siblings (or their children) due to path normalization.\n>\n> For instance, given \"../file\", \"a/\" is appended, giving \"a/../file\".\n\ns/appended/prepended/\n\n> Path normalization folds out \"a/../\", resulting in \"file\". Attempting\n> to recover the original name by skipping strlen(\"a/\") bytes gives the\n> incorrect \"le\" rather than the desired \"../file\".\n>\n> Fix this by taking advantage of write_name_quoted_relative() to recover\n> the original name properly, rather than assuming that it can be\n> recovered by skipping strlen(prefix) bytes.\n>\n> As a bonus, this also fixes a bug in which checkout-index --temp\n> accessed and printed memory beyond the end-of-string. For instance,\n> within a subdirectory named \"subdirectory\", and given argument\n> \"../file\", prefixing would give \"subdirectory/../file\", which would\n> become \"file\" after normalization. checkout-index would then attempt to\n> recover the original name by skipping strlen(\"subdirectory/\") bytes of\n> \"file\", which placed it well beyond end-of-string. Despite this error,\n> it often appeared to give the correct result, but only due to an\n> accident of implementation which left an apparently correct copy of the\n> path in memory following the normalized value. In particular, handed\n> \"subdirectory/../file\", in-place processing by normalize_path_copy_len()\n> resulted in \"file\\0rectory/../file\". When checkout-index skipped\n> strlen(\"subdirectory/\") bytes, it ended up back at \"../file\" and thus\n> appeared to give the correct answer, despite being past end-of-string.\n>\n> Reported-by: Russ Cox <rsc@golang.org>\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n"},{"id":"254148","messageId":"xmqqlhlqz3j1.fsf@gitster.dls.corp.google.com","threadId":"38244","inReplyTo":"1419414196-58587-6-git-send-email-sunshine@sunshineco.com","subject":"Re: [PATCH 5/5] checkout-index: fix --temp relative path mangling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-29T18:58:42Z","receivedAt":"2014-12-29T18:58:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> Fix this by taking advantage of write_name_quoted_relative() to recover\n> the original name properly, rather than assuming that it can be\n> recovered by skipping strlen(prefix) bytes.\n\nNice; I was wondering if we already had that helper when I saw the\ninitial report of the problem.\n\nThanks for a nicely constructed series.  Will queue.\n"}]}