{"thread":{"id":"29265","subject":"[PATCH] stash: Don't fail if work dir contains file named 'HEAD'","startedAt":"2011-12-29T20:47:44Z","lastAt":"2012-01-03T19:44:41Z","messageCount":4,"participants":["Jonathon Mah","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"181786","messageId":"913BB2F9-3C51-44D0-BFEC-3A49A5EC9E15@JonathonMah.com","threadId":"29265","inReplyTo":null,"subject":"[PATCH] stash: Don't fail if work dir contains file named 'HEAD'","fromName":"Jonathon Mah","fromEmail":"me@jonathonmah.com","sentAt":"2011-12-29T20:47:44Z","receivedAt":"2011-12-29T20:47:44Z","isPatch":true,"sender":{"key":"me@jonathonmah.com","avatar":"https://avatars.githubusercontent.com/u/2748?v=4"},"body":"When performing a plain \"git stash\" (without --patch), git-diff would fail\nwith \"fatal: ambiguous argument 'HEAD': both revision and filename\". The\noutput was piped into git-update-index, masking the failed exit status.\nThe output is now sent to a temporary file (which is cleaned up by\nexisting code), and the exit status is checked. The \"HEAD\" arg to the\ngit-diff invocation has been disambiguated too, of course.\n\nIn patch mode, \"git stash -p\" would fail harmlessly, leaving the working\ndir untouched.\n\nSigned-off-by: Jonathon Mah <me@JonathonMah.com>\n---\n git-stash.sh                       |    5 ++-\n t/t3903-stash.sh                   |   25 +++++++++++++++++++\n t/t3904-stash-patch.sh             |   47 ++++++++++++++++++++++-------------\n t/t3905-stash-include-untracked.sh |   13 +++++++++-\n 4 files changed, 69 insertions(+), 21 deletions(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c766692..a46f32a 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -115,7 +115,8 @@ create_stash () {\n \t\t\tgit read-tree --index-output=\"$TMPindex\" -m $i_tree &&\n \t\t\tGIT_INDEX_FILE=\"$TMPindex\" &&\n \t\t\texport GIT_INDEX_FILE &&\n-\t\t\tgit diff --name-only -z HEAD | git update-index -z --add --remove --stdin &&\n+\t\t\tgit diff --name-only -z HEAD -- > \"$TMP-stagenames\" &&\n+\t\t\tgit update-index -z --add --remove --stdin < \"$TMP-stagenames\" &&\n \t\t\tgit write-tree &&\n \t\t\trm -f \"$TMPindex\"\n \t\t) ) ||\n@@ -134,7 +135,7 @@ create_stash () {\n \t\tw_tree=$(GIT_INDEX_FILE=\"$TMP-index\" git write-tree) ||\n \t\tdie \"$(gettext \"Cannot save the current worktree state\")\"\n \n-\t\tgit diff-tree -p HEAD $w_tree > \"$TMP-patch\" &&\n+\t\tgit diff-tree -p HEAD $w_tree -- > \"$TMP-patch\" &&\n \t\ttest -s \"$TMP-patch\" ||\n \t\tdie \"$(gettext \"No changes selected\")\"\n \ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex fcdb182..8f1d07a 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -601,4 +601,29 @@ test_expect_success 'stash apply shows status same as git status (relative to cu\n \ttest_cmp expect actual\n '\n \n+cat > expect << EOF\n+diff --git a/HEAD b/HEAD\n+new file mode 100644\n+index 0000000..fe0cbee\n+--- /dev/null\n++++ b/HEAD\n+@@ -0,0 +1 @@\n++file-not-a-ref\n+EOF\n+\n+test_expect_success 'stash where working directory contains \"HEAD\" file' '\n+\tgit stash clear &&\n+\tgit reset --hard &&\n+\techo file-not-a-ref > HEAD &&\n+\tgit add HEAD &&\n+\tgit stash &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --cached --quiet HEAD &&\n+\ttest_tick &&\n+\ttest $(git rev-parse stash^) = $(git rev-parse HEAD) &&\n+\tgit diff stash^..stash > output &&\n+\ttest_cmp output expect &&\n+\tgit stash drop\n+'\n+\n test_done\ndiff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\nindex 781fd71..70655c1 100755\n--- a/t/t3904-stash-patch.sh\n+++ b/t/t3904-stash-patch.sh\n@@ -7,7 +7,8 @@ test_expect_success PERL 'setup' '\n \tmkdir dir &&\n \techo parent > dir/foo &&\n \techo dummy > bar &&\n-\tgit add bar dir/foo &&\n+\techo committed > HEAD &&\n+\tgit add bar dir/foo HEAD &&\n \tgit commit -m initial &&\n \ttest_tick &&\n \ttest_commit second dir/foo head &&\n@@ -17,47 +18,57 @@ test_expect_success PERL 'setup' '\n \tsave_head\n '\n \n-# note: bar sorts before dir, so the first 'n' is always to skip 'bar'\n+# note: order of files with unstaged changes: HEAD bar dir/foo\n \n test_expect_success PERL 'saying \"n\" does nothing' '\n+\tset_state HEAD HEADfile_work HEADfile_index &&\n \tset_state dir/foo work index &&\n-\t(echo n; echo n) | test_must_fail git stash save -p &&\n-\tverify_state dir/foo work index &&\n-\tverify_saved_state bar\n+\t(echo n; echo n; echo n) | test_must_fail git stash save -p &&\n+\tverify_state HEAD HEADfile_work HEADfile_index &&\n+\tverify_saved_state bar &&\n+\tverify_state dir/foo work index\n '\n \n test_expect_success PERL 'git stash -p' '\n-\t(echo n; echo y) | git stash save -p &&\n-\tverify_state dir/foo head index &&\n+\t(echo y; echo n; echo y) | git stash save -p &&\n+\tverify_state HEAD committed HEADfile_index &&\n \tverify_saved_state bar &&\n+\tverify_state dir/foo head index &&\n \tgit reset --hard &&\n \tgit stash apply &&\n-\tverify_state dir/foo work head &&\n-\tverify_state bar dummy dummy\n+\tverify_state HEAD HEADfile_work committed &&\n+\tverify_state bar dummy dummy &&\n+\tverify_state dir/foo work head\n '\n \n test_expect_success PERL 'git stash -p --no-keep-index' '\n-\tset_state dir/foo work index &&\n+\tset_state HEAD HEADfile_work HEADfile_index &&\n \tset_state bar bar_work bar_index &&\n-\t(echo n; echo y) | git stash save -p --no-keep-index &&\n-\tverify_state dir/foo head head &&\n+\tset_state dir/foo work index &&\n+\t(echo y; echo n; echo y) | git stash save -p --no-keep-index &&\n+\tverify_state HEAD committed committed &&\n \tverify_state bar bar_work dummy &&\n+\tverify_state dir/foo head head &&\n \tgit reset --hard &&\n \tgit stash apply --index &&\n-\tverify_state dir/foo work index &&\n-\tverify_state bar dummy bar_index\n+\tverify_state HEAD HEADfile_work HEADfile_index &&\n+\tverify_state bar dummy bar_index &&\n+\tverify_state dir/foo work index\n '\n \n test_expect_success PERL 'git stash --no-keep-index -p' '\n-\tset_state dir/foo work index &&\n+\tset_state HEAD HEADfile_work HEADfile_index &&\n \tset_state bar bar_work bar_index &&\n-\t(echo n; echo y) | git stash save --no-keep-index -p &&\n+\tset_state dir/foo work index &&\n+\t(echo y; echo n; echo y) | git stash save --no-keep-index -p &&\n+\tverify_state HEAD committed committed &&\n \tverify_state dir/foo head head &&\n \tverify_state bar bar_work dummy &&\n \tgit reset --hard &&\n \tgit stash apply --index &&\n-\tverify_state dir/foo work index &&\n-\tverify_state bar dummy bar_index\n+\tverify_state HEAD HEADfile_work HEADfile_index &&\n+\tverify_state bar dummy bar_index &&\n+\tverify_state dir/foo work index\n '\n \n test_expect_success PERL 'none of this moved HEAD' '\ndiff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\nindex ef44fb2..7f75622 100755\n--- a/t/t3905-stash-include-untracked.sh\n+++ b/t/t3905-stash-include-untracked.sh\n@@ -17,6 +17,7 @@ test_expect_success 'stash save --include-untracked some dirty working directory\n \techo 3 > file &&\n \ttest_tick &&\n \techo 1 > file2 &&\n+\techo 1 > HEAD &&\n \tmkdir untracked &&\n \techo untracked >untracked/untracked &&\n \tgit stash --include-untracked &&\n@@ -35,6 +36,13 @@ test_expect_success 'stash save --include-untracked cleaned the untracked files'\n '\n \n cat > expect.diff <<EOF\n+diff --git a/HEAD b/HEAD\n+new file mode 100644\n+index 0000000..d00491f\n+--- /dev/null\n++++ b/HEAD\n+@@ -0,0 +1 @@\n++1\n diff --git a/file2 b/file2\n new file mode 100644\n index 0000000..d00491f\n@@ -51,6 +59,7 @@ index 0000000..5a72eb2\n +untracked\n EOF\n cat > expect.lstree <<EOF\n+HEAD\n file2\n untracked\n EOF\n@@ -58,7 +67,8 @@ EOF\n test_expect_success 'stash save --include-untracked stashed the untracked files' '\n \ttest \"!\" -f file2 &&\n \ttest ! -e untracked &&\n-\tgit diff HEAD stash^3 -- file2 untracked >actual &&\n+\ttest \"!\" -f HEAD &&\n+\tgit diff HEAD stash^3 -- HEAD file2 untracked >actual &&\n \ttest_cmp expect.diff actual &&\n \tgit ls-tree --name-only stash^3: >actual &&\n \ttest_cmp expect.lstree actual\n@@ -75,6 +85,7 @@ git clean --force --quiet\n \n cat > expect <<EOF\n  M file\n+?? HEAD\n ?? actual\n ?? expect\n ?? file2\n-- \n1.7.8\n\n\n\nJonathon Mah\nme@JonathonMah.com\n"},{"id":"181799","messageId":"8739c28iwh.fsf@thomas.inf.ethz.ch","threadId":"29265","inReplyTo":"913BB2F9-3C51-44D0-BFEC-3A49A5EC9E15@JonathonMah.com","subject":"Re: [PATCH] stash: Don't fail if work dir contains file named 'HEAD'","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2011-12-30T10:15:42Z","receivedAt":"2011-12-30T10:15:42Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jonathon Mah <me@JonathonMah.com> writes:\n> When performing a plain \"git stash\" (without --patch), git-diff would fail\n> with \"fatal: ambiguous argument 'HEAD': both revision and filename\". The\n> output was piped into git-update-index, masking the failed exit status.\n> The output is now sent to a temporary file (which is cleaned up by\n> existing code), and the exit status is checked. The \"HEAD\" arg to the\n> git-diff invocation has been disambiguated too, of course.\n\nThanks, good catch.\n\n> In patch mode, \"git stash -p\" would fail harmlessly, leaving the working\n> dir untouched.\n\nNote that this only affects stash -p, not add/reset/commit -p, because\nit is the only one that does an extra patch dance on top of the\ngit-add--interactive work.  stash -p uses a 'diff-index -p HEAD'\ninvocation in the %patch_modes of git-add--interactive, but diff-index\ndoesn't need disambiguation as the first argument is always the (sole)\ntree-ish.\n\nI had to look and verify, so perhaps you can put a paragraph to this\neffect in the commit message.\n\n> -\t\t\tgit diff --name-only -z HEAD | git update-index -z --add --remove --stdin &&\n> +\t\t\tgit diff --name-only -z HEAD -- > \"$TMP-stagenames\" &&\n> +\t\t\tgit update-index -z --add --remove --stdin < \"$TMP-stagenames\" &&\n\nStyle nit: we usually spell it >foo.  I saw that git-stash is already\ninconsistent, but let's at least not make it worse.\n\nWhile reading this I also wondered if there was a good reason it didn't\njust use 'add -u', and indeed: 7aa5d43 (stash: Don't overwrite files\nthat have gone from the index, 2010-04-18) changed it *away* from add -u\nbecause that was broken.\n\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n[...]\n> +test_expect_success 'stash where working directory contains \"HEAD\" file' '\n> +\tgit stash clear &&\n> +\tgit reset --hard &&\n> +\techo file-not-a-ref > HEAD &&\n> +\tgit add HEAD &&\n> +\tgit stash &&\n> +\tgit diff-files --quiet &&\n> +\tgit diff-index --cached --quiet HEAD &&\n> +\ttest_tick &&\n\nWhat's the tick good for if you don't create any commits after it?  You\nshould put it immediately before the 'git stash'.\n\n> +\ttest $(git rev-parse stash^) = $(git rev-parse HEAD) &&\n\nThis should probably have its arguments quoted, to avoid confusing\n'test' if something goes horribly wrong (e.g. stash^ is not valid).\nThere are plenty of existing lines of this form in other tests however.\n\n> +\tgit diff stash^..stash > output &&\n> +\ttest_cmp output expect &&\n> +\tgit stash drop\n\nPerhaps this could go after the 'git stash' as a\n\n  test_when_finished \"git stash drop\"\n\n> +'\n\n> diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\n[...]\n> -# note: bar sorts before dir, so the first 'n' is always to skip 'bar'\n> +# note: order of files with unstaged changes: HEAD bar dir/foo\n>  \n>  test_expect_success PERL 'saying \"n\" does nothing' '\n> +\tset_state HEAD HEADfile_work HEADfile_index &&\n>  \tset_state dir/foo work index &&\n> -\t(echo n; echo n) | test_must_fail git stash save -p &&\n> -\tverify_state dir/foo work index &&\n> -\tverify_saved_state bar\n> +\t(echo n; echo n; echo n) | test_must_fail git stash save -p &&\n> +\tverify_state HEAD HEADfile_work HEADfile_index &&\n> +\tverify_saved_state bar &&\n> +\tverify_state dir/foo work index\n>  '\n\nOther reviewers may want to read these hunks in word diff mode, where it\nis far easier to verify that the functionality tested is a superset:\n\n  test_expect_success PERL 'saying \"n\" does nothing' '\n          {+set_state HEAD HEADfile_work HEADfile_index &&+}\n          set_state dir/foo work index &&\n          (echo n; echo {+n; echo+} n) | test_must_fail git stash save -p &&\n          verify_state [-dir/foo work index-]{+HEAD HEADfile_work HEADfile_index+} &&\n          verify_saved_state bar {+&&+}\n  {+      verify_state dir/foo work index+}\n  '\n\n> diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh\n[...]\n>  test_expect_success 'stash save --include-untracked stashed the untracked files' '\n>  \ttest \"!\" -f file2 &&\n>  \ttest ! -e untracked &&\n> -\tgit diff HEAD stash^3 -- file2 untracked >actual &&\n> +\ttest \"!\" -f HEAD &&\n> +\tgit diff HEAD stash^3 -- HEAD file2 untracked >actual &&\n\nAgain not something you started, but we may want to clean this up to say\n\n  test_path_is_missing file2 &&\n  test_path_is_missing untracked &&\n  test_path_is_missing HEAD &&\n\netc.  This is nicer (gives a message) if the test fails.  It also barfs\nif there is a freak bug that made HEAD a device or some such :-)\n\n\nAnyway, except for the test_tick those were just style nits.  You can\nadd\n\n  Acked-by: Thomas Rast <trast@student.ethz.ch>\n\nwhen you reroll.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"181811","messageId":"117AA87B-FDCA-48B3-B5A3-02AF392332E9@JonathonMah.com","threadId":"29265","inReplyTo":"8739c28iwh.fsf@thomas.inf.ethz.ch","subject":"Re: [PATCH] stash: Don't fail if work dir contains file named 'HEAD'","fromName":"Jonathon Mah","fromEmail":"me@jonathonmah.com","sentAt":"2011-12-31T00:01:53Z","receivedAt":"2011-12-31T00:01:53Z","isPatch":true,"sender":{"key":"me@jonathonmah.com","avatar":"https://avatars.githubusercontent.com/u/2748?v=4"},"body":"Thanks for the feedback, Thomas. I should note, this bug initially came up on #git several days ago.\n\nI've tried to take on all your suggestions; patch v2 imminent.\n\nOn 2011-12-30, at 02:15, Thomas Rast wrote:\n\n> Jonathon Mah <me@JonathonMah.com> writes:\n>> diff --git a/t/t3904-stash-patch.sh b/t/t3904-stash-patch.sh\n> [...]\n> \n> Other reviewers may want to read these hunks in word diff mode, where it\n> is far easier to verify that the functionality tested is a superset:\n> \n>  test_expect_success PERL 'saying \"n\" does nothing' '\n>          {+set_state HEAD HEADfile_work HEADfile_index &&+}\n>          set_state dir/foo work index &&\n>          (echo n; echo {+n; echo+} n) | test_must_fail git stash save -p &&\n>          verify_state [-dir/foo work index-]{+HEAD HEADfile_work HEADfile_index+} &&\n>          verify_saved_state bar {+&&+}\n>  {+      verify_state dir/foo work index+}\n>  '\n\nI added a note to the message: In t3904, checks and operations on each file are in the order they'll appear when interactively staging.\n\nThat is, \"echo y/n; echo y/n; ...\" for the three files corresponds to the surrounding checks.\n\n\n\nJonathon Mah\nme@JonathonMah.com\n"},{"id":"181881","messageId":"7vwr98tvti.fsf@alter.siamese.dyndns.org","threadId":"29265","inReplyTo":"913BB2F9-3C51-44D0-BFEC-3A49A5EC9E15@JonathonMah.com","subject":"Re: [PATCH] stash: Don't fail if work dir contains file named 'HEAD'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-01-03T19:44:41Z","receivedAt":"2012-01-03T19:44:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathon Mah <me@JonathonMah.com> writes:\n\n> When performing a plain \"git stash\" (without --patch), git-diff would fail\n> with \"fatal: ambiguous argument 'HEAD': both revision and filename\". The\n> output was piped into git-update-index, masking the failed exit status.\n> The output is now sent to a temporary file (which is cleaned up by\n> existing code), and the exit status is checked. The \"HEAD\" arg to the\n> git-diff invocation has been disambiguated too, of course.\n>\n> In patch mode, \"git stash -p\" would fail harmlessly, leaving the working\n> dir untouched.\n>\n> Signed-off-by: Jonathon Mah <me@JonathonMah.com>\n> ---\n\nThanks.\n"}]}