{"thread":{"id":"28608","subject":"[PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so","startedAt":"2011-10-06T17:48:35Z","lastAt":"2011-10-06T22:02:59Z","messageCount":4,"participants":["Jay Soffian","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"177076","messageId":"1317923315-54940-1-git-send-email-jaysoffian@gmail.com","threadId":"28608","inReplyTo":null,"subject":"[PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-10-06T17:48:35Z","receivedAt":"2011-10-06T17:48:35Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"do_pick_commit() writes out CHERRY_PICK_HEAD before invoking merge (either\nvia do_recursive_merge() or try_merge_command()) on the assumption that if\nthe merge fails it is due to conflict. However, if the tree is dirty, the\nmerge may not even start, aborting before do_pick_commit() can remove\nCHERRY_PICK_HEAD.\n\nInstead, defer writing CHERRY_PICK_HEAD till after merge has returned.\nAt this point we know the merge has either succeeded or failed due\nto conflict. In either case, we want CHERRY_PICK_HEAD to be written\nso that it may be picked up by the subsequent invocation of commit.\n\nNote that do_recursive_merge() aborts if the merge cannot start, while\ntry_merge_command() returns a non-zero value other than 1.\n\nSigned-off-by: Jay Soffian \n---\n builtin/revert.c | 10 ++++++++--\n t/t3507-cherry-pick-conflict.sh | 15 +++++++++++++++\n 2 files changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 3117776c2c..a95b255c86 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -476,8 +476,6 @@ static int do_pick_commit(void)\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n \t\t}\n-\t\tif (!no_commit)\n-\t\t\twrite_cherry_pick_head();\n \t}\n \n \tif (!strategy || !strcmp(strategy, \"recursive\") || action == REVERT) {\n@@ -498,6 +496,14 @@ static int do_pick_commit(void)\n \t\tfree_commit_list(remotes);\n \t}\n \n+\t/* If the merge was clean or if it failed due to conflict, we write\n+\t * CHERRY_PICK_HEAD for the subsequent invocation of commit to use.\n+\t * However, if the merge did not even start, then we don't want to\n+\t * write it at all.\n+\t*/\n+\tif (action == CHERRY_PICK && !no_commit && (res == 0 || res == 1))\n+\t\twrite_cherry_pick_head();\n+\n \tif (res) {\n \t\terror(action == REVERT\n \t\t ? _(\"could not revert %s... %s\")\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 212ec54aaf..7601d0b0d6 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -77,6 +77,21 @@ test_expect_success 'cherry-pick --no-commit does not set CHERRY_PICK_HEAD' '\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n \n+test_expect_success 'cherry-pick w/dirty tree does not set CHERRY_PICK_HEAD' '\n+\tpristine_detach initial &&\n+\techo foo > foo &&\n+\ttest_must_fail git cherry-pick base &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n+'\n+\n+test_expect_success \\\n+'cherry-pick --strategy=resolve w/dirty tree does not set CHERRY_PICK_HEAD' '\n+\tpristine_detach initial &&\n+\techo foo > foo &&\n+\ttest_must_fail git cherry-pick --strategy=resolve base &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n+'\n+\n test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n \t(\n-- \n1.7.7.6.g25c34\n"},{"id":"177078","messageId":"CAG+J_Dw8w9UGBzq4xK+i+QtA4ZuwJ5w1+mPg15mPNcGLuRaXyg@mail.gmail.com","threadId":"28608","inReplyTo":"1317923315-54940-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-10-06T17:58:01Z","receivedAt":"2011-10-06T17:58:01Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Oct 6, 2011 at 1:48 PM, Jay Soffian wrote:\n> Note that do_recursive_merge() aborts if the merge cannot start, while\n> try_merge_command() returns a non-zero value other than 1.\n\nMaybe you want this on-top:\n\ndiff --git i/builtin/revert.c w/builtin/revert.c\nindex a95b255c86..7e4857530b 100644\n--- i/builtin/revert.c\n+++ w/builtin/revert.c\n@@ -223,7 +223,7 @@ static void advise(const char *advice, ...)\n \tva_end(params);\n }\n\n-static void print_advice(void)\n+static void print_advice(int show_hint)\n {\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n\n@@ -238,9 +238,11 @@ static void print_advice(void)\n \t\treturn;\n \t}\n\n-\tadvise(\"after resolving the conflicts, mark the corrected paths\");\n-\tadvise(\"with 'git add ' or 'git rm '\");\n-\tadvise(\"and commit the result with 'git commit'\");\n+\tif (show_hint) {\n+\t\tadvise(\"after resolving the conflicts, mark the corrected paths\");\n+\t\tadvise(\"with 'git add ' or 'git rm '\");\n+\t\tadvise(\"and commit the result with 'git commit'\");\n+\t}\n }\n\n static void write_message(struct strbuf *msgbuf, const char *filename)\n@@ -510,7 +512,7 @@ static int do_pick_commit(void)\n \t\t : _(\"could not apply %s... %s\"),\n \t\t find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n \t\t msg.subject);\n-\t\tprint_advice();\n+\t\tprint_advice(res == 1);\n \t\trerere(allow_rerere_auto);\n \t} else {\n \t\tif (!no_commit)\n\n\nj.\n"},{"id":"177117","messageId":"7vzkhdyecy.fsf@alter.siamese.dyndns.org","threadId":"28608","inReplyTo":"CAG+J_Dw8w9UGBzq4xK+i+QtA4ZuwJ5w1+mPg15mPNcGLuRaXyg@mail.gmail.com","subject":"Re: [PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-06T21:55:25Z","receivedAt":"2011-10-06T21:55:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> On Thu, Oct 6, 2011 at 1:48 PM, Jay Soffian wrote:\n>> Note that do_recursive_merge() aborts if the merge cannot start, while\n>> try_merge_command() returns a non-zero value other than 1.\n>\n> Maybe you want this on-top:\n\nGood thinking.\n\ncommit 4ed15ff067b548011b1eda8b12d46d887c4f056c\nAuthor: Jay Soffian \nDate: Thu Oct 6 13:58:01 2011 -0400\n\n cherry-pick: do not give irrelevant advice when cherry-pick punted\n \n If a cherry-pick did not even start because the working tree had local\n changes that would overlap with the operation, we shouldn't be advising\n the users to resolve conflicts nor to conclude it with \"git commit\".\n \n Signed-off-by: Junio C Hamano \n\nThanks. Care to sign-off?\n"},{"id":"177119","messageId":"CAG+J_DyPThjC1Mt-Hh9nke+U=ZT91AW0uWCO5sFZpZC_LbgDig@mail.gmail.com","threadId":"28608","inReplyTo":"7vzkhdyecy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] revert.c: defer writing CHERRY_PICK_HEAD till it is safe to do so","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2011-10-06T22:02:59Z","receivedAt":"2011-10-06T22:02:59Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Thu, Oct 6, 2011 at 5:55 PM, Junio C Hamano wrote:\n> Jay Soffian writes:\n>\n>> On Thu, Oct 6, 2011 at 1:48 PM, Jay Soffian wrote:\n>>> Note that do_recursive_merge() aborts if the merge cannot start, while\n>>> try_merge_command() returns a non-zero value other than 1.\n>>\n>> Maybe you want this on-top:\n>\n> Good thinking.\n>\n> commit 4ed15ff067b548011b1eda8b12d46d887c4f056c\n> Author: Jay Soffian \n> Date:   Thu Oct 6 13:58:01 2011 -0400\n>\n>    cherry-pick: do not give irrelevant advice when cherry-pick punted\n>\n>    If a cherry-pick did not even start because the working tree had local\n>    changes that would overlap with the operation, we shouldn't be advising\n>    the users to resolve conflicts nor to conclude it with \"git commit\".\n>\n>    Signed-off-by: Junio C Hamano \n>\n> Thanks. Care to sign-off?\n\nSigned-off-by: Jay Soffian \n\nThank you for making it a proper commit. :-)\n\nj.\n"}]}