{"thread":{"id":"28989","subject":"cherry-pick/revert error messages","startedAt":"2011-11-20T07:30:59Z","lastAt":"2012-04-05T12:15:08Z","messageCount":52,"participants":["Jonathan Nieder","Ramkumar Ramachandra","Junio C Hamano","Jakub Narebski","Thiago Farina","Johannes Sixt","Alex Riesen","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"179756","messageId":"20111120073059.GA2278@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":null,"subject":"cherry-pick/revert error messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-20T07:30:59Z","receivedAt":"2011-11-20T07:30:59Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Ram,\n\nToday I encountered the following error message:\n\n\t$ git cherry-pick foo..bar\n\terror: .git/sequencer already exists.\n\terror: A cherry-pick or revert is in progress.\n\thint: Use --continue to continue the operation\n\thint: or --reset to forget about it\n\tfatal: cherry-pick failed\n\t$\n\nThe double \"error\" seemed a little weird. Also, the capital letters\nand periods feel out of place, when compared with other messages\nemited by git with an \"error:\" prefix. The final \"fatal: cherry-pick\nfailed\" seems redundant, too. I guess I would have expected something\nlike the following instead:\n\n\t$ git cherry-pick foo..bar\n\tfatal: a cherry-pick or revert is already in progress\n\thint: try \"git cherry-pick (--continue | --quit)\"\n\t$\n\nWhat do you think?\n"},{"id":"179757","messageId":"CALkWK0=45OwcBoH2TorsgwTbaXjnffVuh0mGxh2+ShN9cuF-=A@mail.gmail.com","threadId":"28989","inReplyTo":"20111120073059.GA2278@elie.hsd1.il.comcast.net","subject":"Re: cherry-pick/revert error messages","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-11-20T08:02:01Z","receivedAt":"2011-11-20T08:02:01Z","isPatch":false,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder wrote:\n> Today I encountered the following error message:\n> [...]\n\n> I guess I would have expected something\n> like the following instead:\n>\n>        $ git cherry-pick foo..bar\n>        fatal: a cherry-pick or revert is already in progress\n>        hint: try \"git cherry-pick (--continue | --quit)\"\n>        $\n>\n> What do you think?\n\nLooks much better! Yes, a series pretty'fying error messages would be\nreally nice. I'm busy with exams this week, and \"New sequencer\nworkflow! v2\" has been pending for some time -- would you like to\nhandle this bit?\n\nThanks.\n\n-- Ram\n"},{"id":"179760","messageId":"20111120094650.GB2278@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"CALkWK0=45OwcBoH2TorsgwTbaXjnffVuh0mGxh2+ShN9cuF-=A@mail.gmail.com","subject":"[RFC/PATCH 0/3] Re: cherry-pick/revert error messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-20T09:46:50Z","receivedAt":"2011-11-20T09:46:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Looks much better! Yes, a series pretty'fying error messages would be\n> really nice.\n\nGood to hear. Here's a rough one.\n\nI realize patch 1/3 might not be conservative enough, even though\nthere hasn't been much time for people to get used to --reset yet. So\nI might be sending a second version that treats --reset as a synonym.\nBut please forget I said that when judging this version (i.e., if it\nlooks like a problem, I do want to know).\n\nThoughts?\n\nJonathan Nieder (3):\n revert: rename --reset option to --quit\n revert: restructure pick_revisions() for clarity\n revert: improve error message for cherry-pick during cherry-pick\n\n Documentation/git-cherry-pick.txt | 2 +-\n Documentation/git-revert.txt | 2 +-\n Documentation/sequencer.txt | 10 +++---\n builtin/revert.c | 71 ++++++++++++++++++-------------------\n sequencer.h | 2 +-\n t/t3510-cherry-pick-sequence.sh | 10 +++---\n t/t7106-reset-sequence.sh | 2 +-\n 7 files changed, 49 insertions(+), 50 deletions(-)\n"},{"id":"179761","messageId":"20111120094824.GC2278@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111120094650.GB2278@elie.hsd1.il.comcast.net","subject":"[PATCH 1/3] revert: rename --reset option to --quit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-20T09:48:24Z","receivedAt":"2011-11-20T09:48:24Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The option to \"git cherry-pick\" and \"git revert\" to discard the\nsequencer state introduced by v1.7.8-rc0~141^2~6 (revert: Introduce\n--reset to remove sequencer state, 2011-08-04) has a confusing name.\nChange it now, while we still have the time.\n\nMechanics:\n\nThis commit removes the \"git cherry-pick --reset\" option. Hopefully\nnobody was using it. If somebody was, we can add it back again as a\nsynonym.\n\nThe new name for \"cherry-pick, please get out of my way, since I've\nlong forgotten about the sequence of commits I was cherry-picking when\nyou wrote that old .git/sequencer directory\" is --quit. Mnemonic:\nthis is analagous to quiting a program the user is no longer using ---\nwe just want to get out of the multiple-command cherry-pick procedure\nand not to reset HEAD or restore any other old state.\n\nAdjust documentation and tests to match. While at it, let's clarify\nthe short descriptions of these operations in \"-h\" output.\n\nBefore:\n\n\t--reset\t\tforget the current operation\n\t--continue\tcontinue the current operation\n\nAfter:\n\n\t--quit\t\tend revert or cherry-pick sequence\n\t--continue\tresume revert or cherry-pick sequence\n\nNoticed-by: Phil Hord \nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-cherry-pick.txt | 2 +-\n Documentation/git-revert.txt | 2 +-\n Documentation/sequencer.txt | 10 +++++-----\n builtin/revert.c | 22 +++++++++++-----------\n sequencer.h | 2 +-\n t/t3510-cherry-pick-sequence.sh | 10 +++++-----\n t/t7106-reset-sequence.sh | 2 +-\n 7 files changed, 25 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 2660a842..21998b82 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -9,8 +9,8 @@ SYNOPSIS\n --------\n [verse]\n 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ...\n-'git cherry-pick' --reset\n 'git cherry-pick' --continue\n+'git cherry-pick' --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex f3519413..b0fcabc8 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -9,8 +9,8 @@ SYNOPSIS\n --------\n [verse]\n 'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] ...\n-'git revert' --reset\n 'git revert' --continue\n+'git revert' --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 3e6df338..75f8e869 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -1,9 +1,9 @@\n---reset::\n-\tForget about the current operation in progress. Can be used\n-\tto clear the sequencer state after a failed cherry-pick or\n-\trevert.\n-\n --continue::\n \tContinue the operation in progress using the information in\n \t'.git/sequencer'. Can be used to continue after resolving\n \tconflicts in a failed cherry-pick or revert.\n+\n+--quit::\n+\tForget about the current operation in progress. Can be used\n+\tto clear the sequencer state after a failed cherry-pick or\n+\trevert.\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 544e8c30..b59dd68c 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {\n };\n \n enum replay_action { REVERT, CHERRY_PICK };\n-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };\n+enum replay_subcommand { REPLAY_NONE, REPLAY_REMOVE_STATE, REPLAY_CONTINUE };\n \n struct replay_opts {\n \tenum replay_action action;\n@@ -133,11 +133,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n \tconst char *me = action_name(opts);\n-\tint reset = 0;\n+\tint remove_state = 0;\n \tint contin = 0;\n \tstruct option options[] = {\n-\t\tOPT_BOOLEAN(0, \"reset\", &reset, \"forget the current operation\"),\n-\t\tOPT_BOOLEAN(0, \"continue\", &contin, \"continue the current operation\"),\n+\t\tOPT_BOOLEAN(0, \"quit\", &remove_state, \"end revert or cherry-pick sequence\"),\n+\t\tOPT_BOOLEAN(0, \"continue\", &contin, \"resume revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n \t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -168,13 +168,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \n \t/* Check for incompatible subcommands */\n \tverify_opt_mutually_compatible(me,\n-\t\t\t\t\"--reset\", reset,\n+\t\t\t\t\"--quit\", remove_state,\n \t\t\t\t\"--continue\", contin,\n \t\t\t\tNULL);\n \n \t/* Set the subcommand */\n-\tif (reset)\n-\t\topts->subcommand = REPLAY_RESET;\n+\tif (remove_state)\n+\t\topts->subcommand = REPLAY_REMOVE_STATE;\n \telse if (contin)\n \t\topts->subcommand = REPLAY_CONTINUE;\n \telse\n@@ -183,8 +183,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t/* Check for incompatible command line arguments */\n \tif (opts->subcommand != REPLAY_NONE) {\n \t\tchar *this_operation;\n-\t\tif (opts->subcommand == REPLAY_RESET)\n-\t\t\tthis_operation = \"--reset\";\n+\t\tif (opts->subcommand == REPLAY_REMOVE_STATE)\n+\t\t\tthis_operation = \"--quit\";\n \t\telse\n \t\t\tthis_operation = \"--continue\";\n \n@@ -965,7 +965,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t * cherry-pick should be handled differently from an existing\n \t * one that is being continued\n \t */\n-\tif (opts->subcommand == REPLAY_RESET) {\n+\tif (opts->subcommand == REPLAY_REMOVE_STATE) {\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n \t} else if (opts->subcommand == REPLAY_CONTINUE) {\n@@ -988,7 +988,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t\tif (create_seq_dir() < 0) {\n \t\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n \t\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\t\tadvise(_(\"or --reset to forget about it\"));\n+\t\t\tadvise(_(\"or --quit to forget about it\"));\n \t\t\treturn -1;\n \t\t}\n \t\tif (get_sha1(\"HEAD\", sha1)) {\ndiff --git a/sequencer.h b/sequencer.h\nindex 905d2950..f435fdb4 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -13,7 +13,7 @@\n *\n * With the aggressive flag, it additionally removes SEQ_OLD_DIR,\n * ignoring any errors. Inteded to be used by the sequencer's\n- * '--reset' subcommand.\n+ * '--quit' subcommand.\n */\n void remove_sequencer_state(int aggressive);\n \ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 3bca2b3d..913435e2 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -13,7 +13,7 @@ test_description='Test cherry-pick continuation features\n . ./test-lib.sh\n \n pristine_detach () {\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \tgit checkout -f \"$1^0\" &&\n \tgit read-tree -u --reset HEAD &&\n \tgit clean -d -f -f -q -x\n@@ -70,15 +70,15 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '\n \ttest_path_is_missing .git/sequencer\n '\n \n-test_expect_success '--reset does not complain when no cherry-pick is in progress' '\n+test_expect_success '--quit does not complain when no cherry-pick is in progress' '\n \tpristine_detach initial &&\n-\tgit cherry-pick --reset\n+\tgit cherry-pick --quit\n '\n \n-test_expect_success '--reset cleans up sequencer state' '\n+test_expect_success '--quit cleans up sequencer state' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \ttest_path_is_missing .git/sequencer\n '\n \ndiff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh\nindex 4956caaf..3f86e8c5 100755\n--- a/t/t7106-reset-sequence.sh\n+++ b/t/t7106-reset-sequence.sh\n@@ -12,7 +12,7 @@ test_description='Test interaction of reset --hard with sequencer\n . ./test-lib.sh\n \n pristine_detach () {\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \tgit checkout -f \"$1^0\" &&\n \tgit read-tree -u --reset HEAD &&\n \tgit clean -d -f -f -q -x\n-- \n1.7.8.rc3\n"},{"id":"179762","messageId":"20111120095010.GD2278@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111120094650.GB2278@elie.hsd1.il.comcast.net","subject":"[PATCH 2/3] revert: rearrange pick_revisions() for clarity","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-20T09:50:10Z","receivedAt":"2011-11-20T09:50:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Deal completely with \"cherry-pick --quit\" and --continue at the\nbeginning of pick_revisions(), leaving the rest of the function for\nthe more interesting \"git cherry-pick \" case.\n\nNo functional change intended. The impact is just to unindent the\ncode a little.\n\nSigned-off-by: Jonathan Nieder \n---\nThis also paves the way to factoring out the REPLAY_CONTINUE case\ninto a separate function.\n\n builtin/revert.c | 48 ++++++++++++++++++++++++------------------------\n 1 files changed, 24 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex b59dd68c..dd072ce6 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -968,40 +968,40 @@ static int pick_revisions(struct replay_opts *opts)\n \tif (opts->subcommand == REPLAY_REMOVE_STATE) {\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n-\t} else if (opts->subcommand == REPLAY_CONTINUE) {\n+\t}\n+\tif (opts->subcommand == REPLAY_CONTINUE) {\n \t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n-\t\t\tgoto error;\n+\t\t\treturn error(_(\"No %s in progress\"), action_name(opts));\n \t\tread_populate_opts(&opts);\n \t\tread_populate_todo(&todo_list, opts);\n \n \t\t/* Verify that the conflict has been resolved */\n \t\tif (!index_differs_from(\"HEAD\", 0))\n \t\t\ttodo_list = todo_list->next;\n-\t} else {\n-\t\t/*\n-\t\t * Start a new cherry-pick/ revert sequence; but\n-\t\t * first, make sure that an existing one isn't in\n-\t\t * progress\n-\t\t */\n+\t\treturn pick_commits(todo_list, opts);\n+\t}\n+\n+\t/*\n+\t * Start a new cherry-pick/ revert sequence; but\n+\t * first, make sure that an existing one isn't in\n+\t * progress\n+\t */\n \n-\t\twalk_revs_populate_todo(&todo_list, opts);\n-\t\tif (create_seq_dir() < 0) {\n-\t\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n-\t\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\t\tadvise(_(\"or --quit to forget about it\"));\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (get_sha1(\"HEAD\", sha1)) {\n-\t\t\tif (opts->action == REVERT)\n-\t\t\t\treturn error(_(\"Can't revert as initial commit\"));\n-\t\t\treturn error(_(\"Can't cherry-pick into empty head\"));\n-\t\t}\n-\t\tsave_head(sha1_to_hex(sha1));\n-\t\tsave_opts(opts);\n+\twalk_revs_populate_todo(&todo_list, opts);\n+\tif (create_seq_dir() < 0) {\n+\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n+\t\tadvise(_(\"Use --continue to continue the operation\"));\n+\t\tadvise(_(\"or --quit to forget about it\"));\n+\t\treturn -1;\n+\t}\n+\tif (get_sha1(\"HEAD\", sha1)) {\n+\t\tif (opts->action == REVERT)\n+\t\t\treturn error(_(\"Can't revert as initial commit\"));\n+\t\treturn error(_(\"Can't cherry-pick into empty head\"));\n \t}\n+\tsave_head(sha1_to_hex(sha1));\n+\tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n-error:\n-\treturn error(_(\"No %s in progress\"), action_name(opts));\n }\n \n int cmd_revert(int argc, const char **argv, const char *prefix)\n-- \n1.7.8.rc3\n"},{"id":"179763","messageId":"20111120095151.GE2278@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111120094650.GB2278@elie.hsd1.il.comcast.net","subject":"[PATCH 3/3] revert: improve error message for cherry-pick during cherry-pick","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-20T09:51:51Z","receivedAt":"2011-11-20T09:51:51Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In the spirit of v1.6.3.3~3^2 (refuse to merge during a merge,\n2009-07-01), \"git cherry-pick\" refuses to start a new cherry-pick when\nin the middle of an existing conflicted cherry-pick in the following\nsequence:\n\n 1. git cherry-pick HEAD..origin\n 2. resolve conflicts\n 3. git cherry-pick HEAD..origin (instead of \"git cherry-pick\n --continue\", by mistake)\n\nGood. However, the error message on attempting step 3 is more\nconvoluted than necessary:\n\n $ git cherry-pick HEAD..origin\n error: .git/sequencer already exists.\n error: A cherry-pick or revert is in progress.\n hint: Use --continue to continue the operation\n hint: or --quit to forget about it\n fatal: cherry-pick failed\n\nClarify by removing the double redundant first \"error:\" message,\nsimplifying the advice, and using lower-case and no full stops to be\nconsistent with other commands that prefix their messages with\n\"error:\", so it becomes\n\n error: a cherry-pick or revert is already in progress\n hint: try \"git cherry-pick (--continue | --quit)\"\n fatal: cherry-pick failed\n\nThe \"fatal: cherry-pick failed\" line seems unnecessary, too, but\nthat can be fixed some other day.\n\nSigned-off-by: Jonathan Nieder \n---\nThat's the end of the series. There are more error messages, but\nlet's start with this one.\n\nThanks for reading.\n\n builtin/revert.c | 13 ++++++-------\n 1 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex dd072ce6..9db4c1e4 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -843,8 +843,11 @@ static int create_seq_dir(void)\n {\n \tconst char *seq_dir = git_path(SEQ_DIR);\n \n-\tif (file_exists(seq_dir))\n-\t\treturn error(_(\"%s already exists.\"), seq_dir);\n+\tif (file_exists(seq_dir)) {\n+\t\terror(_(\"a cherry-pick or revert is already in progress\"));\n+\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit)\\\"\"));\n+\t\treturn -1;\n+\t}\n \telse if (mkdir(seq_dir, 0777) < 0)\n \t\tdie_errno(_(\"Could not create sequencer directory %s\"), seq_dir);\n \treturn 0;\n@@ -988,12 +991,8 @@ static int pick_revisions(struct replay_opts *opts)\n \t */\n \n \twalk_revs_populate_todo(&todo_list, opts);\n-\tif (create_seq_dir() < 0) {\n-\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n-\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\tadvise(_(\"or --quit to forget about it\"));\n+\tif (create_seq_dir() < 0)\n \t\treturn -1;\n-\t}\n \tif (get_sha1(\"HEAD\", sha1)) {\n \t\tif (opts->action == REVERT)\n \t\t\treturn error(_(\"Can't revert as initial commit\"));\n-- \n1.7.8.rc3\n"},{"id":"179801","messageId":"7vpqglfc1k.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111120094824.GC2278@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 1/3] revert: rename --reset option to --quit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-21T20:36:23Z","receivedAt":"2011-11-21T20:36:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> The option to \"git cherry-pick\" and \"git revert\" to discard the\n> sequencer state introduced by v1.7.8-rc0~141^2~6 (revert: Introduce\n> --reset to remove sequencer state, 2011-08-04) has a confusing name.\n> Change it now, while we still have the time.\n>\n> Mechanics:\n>\n> This commit removes the \"git cherry-pick --reset\" option. Hopefully\n> nobody was using it. If somebody was, we can add it back again as a\n> synonym.\n>\n> The new name for \"cherry-pick, please get out of my way, since I've\n> long forgotten about the sequence of commits I was cherry-picking when\n> you wrote that old .git/sequencer directory\" is --quit. \n\nWouldn't it match other commands better if we called this --abort instead\nof --quit?\n\nOther than that I think I agree with the reasoning (and I think I too had\nencountered the irritation with the \"sequencer state\").\n"},{"id":"179807","messageId":"m3bos5m7ez.fsf@localhost.localdomain","threadId":"28989","inReplyTo":"7vpqglfc1k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/3] revert: rename --reset option to --quit","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-11-21T22:35:33Z","receivedAt":"2011-11-21T22:35:33Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n> Jonathan Nieder writes:\n> \n> > The option to \"git cherry-pick\" and \"git revert\" to discard the\n> > sequencer state introduced by v1.7.8-rc0~141^2~6 (revert: Introduce\n> > --reset to remove sequencer state, 2011-08-04) has a confusing name.\n> > Change it now, while we still have the time.\n> >\n> > Mechanics:\n> >\n> > This commit removes the \"git cherry-pick --reset\" option. Hopefully\n> > nobody was using it. If somebody was, we can add it back again as a\n> > synonym.\n> >\n> > The new name for \"cherry-pick, please get out of my way, since I've\n> > long forgotten about the sequence of commits I was cherry-picking when\n> > you wrote that old .git/sequencer directory\" is --quit. \n> \n> Wouldn't it match other commands better if we called this --abort instead\n> of --quit?\n\nActually from what I understand --reset / --quit has to have different\nmeaning than --abort. While for multi-commit operation --abort goes back\nto the state before last operation, --reset / --quit just clears sequencer\nstate, but does not change working area, nor index, not HEAD. This is to\nbe used when encounering stale old rebase / am / cherry-pick / revert.\n\nBTW. I think that '--clear' (or '--clear-state') would be a better name\nfor this option.\n \n> Other than that I think I agree with the reasoning (and I think I too had\n> encountered the irritation with the \"sequencer state\").\n\n-- \nJakub Narębski\n"},{"id":"179808","messageId":"20111121224353.GC19682@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"m3bos5m7ez.fsf@localhost.localdomain","subject":"Re: [PATCH 1/3] revert: rename --reset option to --quit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-21T22:43:53Z","receivedAt":"2011-11-21T22:43:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jakub Narebski wrote:\n\n> Actually from what I understand --reset / --quit has to have different\n> meaning than --abort.\n\nRight. I'll send a strawman series that introduces a \"cherry-pick\n--abort\" option soon to clarify this.\n\n[...]\n> BTW. I think that '--clear' (or '--clear-state') would be a better name\n> for this option.\n\nCould you explain why? When I run \"git cherry-pick --clear\", I'd\nexpect that it removes some files. Problems:\n\n (1) It's not clear to the novice _which_ files it will remove. Maybe\n \".rej\" files or something?\n\n (2) That the sequencer state is stored in files is an implementation\n detail.\n\n>> Other than that I think I agree with the reasoning (and I think I too had\n>> encountered the irritation with the \"sequencer state\").\n\nThanks for the quick feedback, both of you.\n"},{"id":"179820","messageId":"20111122111207.GA7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111120094650.GB2278@elie.hsd1.il.comcast.net","subject":"[PATCH v2 0/3] Re: cherry-pick/revert error messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:12:08Z","receivedAt":"2011-11-22T11:12:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi again,\n\nJonathan Nieder wrote:\n> Ramkumar Ramachandra wrote:\n\n>> Looks much better! Yes, a series pretty'fying error messages would be\n>> really nice.\n>\n> Good to hear. Here's a rough one.\n\nHere's a less rough version. Same scope.\n\nPatch 1 renames --reset to --quit, keeping --reset as a synonym for\nbackward compatibility this time. I don't suspect anyone was using\n--reset, but splitting out the backward-incompatible changes seems\nlike a useful habit to follow nonetheless.\n\nPatch 2 is a small code cleanup in preparation for patch 3, unchanged\nfrom v1.\n\nThe change description for patch 3 said 'double redundant first\n\"error:\" message', which was kind of amusing. This time it says\n'redundant first \"error:\" message'.\n\nMore notably, this version has some bonus patches:\n\nPatch 4 introduces the REVERT_HEAD pseudoref, which is analagous to\nCHERRY_PICK_HEAD except it is populated by \"git revert\" and does not\ninfluence the behavior of \"git commit\". This could allow tools like\n__git_ps1 to detect when we are in the middle of a conflicted revert,\nthough the series does not include a patch to do that. I'm kind of\nfond of it.\n\nPatch 5 introduces a \"git cherry-pick --abort\" command that rolls back\nthe series of commits made so far in a conflicted cherry-pick.\nCompare --quit, which tells cherry-pick to leave HEAD alone and get\nout of the way.\n\nPatch 6 removes the --reset compatibility option. There might not be\nmuch reason to do that, aside from removing clutter and saving a few\nbytes. The alternate motivation of removing the option to prevent\npeople from relying on it seems less compelling after patch 1 made it\nharder to stumble upon the option.\n\nMay the series can provide some amusement.\n\nThanks to Junio and Jakub for the useful feedback so far.\n\nJonathan Nieder (3):\n revert: rename --reset option to --quit\n revert: rearrange pick_revisions() for clarity\n revert: improve error message for cherry-pick during cherry-pick\n\n Documentation/git-cherry-pick.txt | 2 +-\n Documentation/git-revert.txt | 2 +-\n Documentation/sequencer.txt | 10 ++--\n builtin/revert.c | 74 +++++++++++++++++++------------------\n sequencer.h | 2 +-\n t/t3510-cherry-pick-sequence.sh | 31 +++++++++++++--\n t/t7106-reset-sequence.sh | 2 +-\n 7 files changed, 73 insertions(+), 50 deletions(-)\n"},{"id":"179821","messageId":"20111122111429.GB7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 1/3] revert: rename --reset option to --quit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:14:29Z","receivedAt":"2011-11-22T11:14:29Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The option to \"git cherry-pick\" and \"git revert\" to discard the\nsequencer state introduced by v1.7.8-rc0~141^2~6 (revert: Introduce\n--reset to remove sequencer state, 2011-08-04) has a confusing name.\nChange it now, while we still have the time.\n\nThe new name for \"cherry-pick, please get out of my way, since I've\nlong forgotten about the sequence of commits I was cherry-picking when\nyou wrote that old .git/sequencer directory\" is --quit. Mnemonic:\nthis is analagous to quiting a program the user is no longer using ---\nwe just want to get out of the multiple-command cherry-pick procedure\nand not to reset HEAD or rewind any other old state.\n\nThe \"--reset\" option is kept as a synonym to minimize the impact. We\nmight consider dropping it for simplicity in a separate patch, though.\n\nAdjust documentation and tests to use the newly preferred name (--quit)\ninstead of --reset. While at it, let's clarify the short descriptions\nof these operations in \"-h\" output.\n\nBefore:\n\n\t--reset\t\tforget the current operation\n\t--continue\tcontinue the current operation\n\nAfter:\n\n\t--quit\t\tend revert or cherry-pick sequence\n\t--continue\tresume revert or cherry-pick sequence\n\nNoticed-by: Phil Hord \nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-cherry-pick.txt | 2 +-\n Documentation/git-revert.txt | 2 +-\n Documentation/sequencer.txt | 10 +++++-----\n builtin/revert.c | 25 ++++++++++++++-----------\n sequencer.h | 2 +-\n t/t3510-cherry-pick-sequence.sh | 31 ++++++++++++++++++++++++++-----\n t/t7106-reset-sequence.sh | 2 +-\n 7 files changed, 49 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 2660a842..21998b82 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -9,8 +9,8 @@ SYNOPSIS\n --------\n [verse]\n 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ...\n-'git cherry-pick' --reset\n 'git cherry-pick' --continue\n+'git cherry-pick' --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex f3519413..b0fcabc8 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -9,8 +9,8 @@ SYNOPSIS\n --------\n [verse]\n 'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] ...\n-'git revert' --reset\n 'git revert' --continue\n+'git revert' --quit\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 3e6df338..75f8e869 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -1,9 +1,9 @@\n---reset::\n-\tForget about the current operation in progress. Can be used\n-\tto clear the sequencer state after a failed cherry-pick or\n-\trevert.\n-\n --continue::\n \tContinue the operation in progress using the information in\n \t'.git/sequencer'. Can be used to continue after resolving\n \tconflicts in a failed cherry-pick or revert.\n+\n+--quit::\n+\tForget about the current operation in progress. Can be used\n+\tto clear the sequencer state after a failed cherry-pick or\n+\trevert.\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 544e8c30..e109fb11 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {\n };\n \n enum replay_action { REVERT, CHERRY_PICK };\n-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };\n+enum replay_subcommand { REPLAY_NONE, REPLAY_REMOVE_STATE, REPLAY_CONTINUE };\n \n struct replay_opts {\n \tenum replay_action action;\n@@ -133,11 +133,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n {\n \tconst char * const * usage_str = revert_or_cherry_pick_usage(opts);\n \tconst char *me = action_name(opts);\n-\tint reset = 0;\n+\tint remove_state = 0;\n \tint contin = 0;\n \tstruct option options[] = {\n-\t\tOPT_BOOLEAN(0, \"reset\", &reset, \"forget the current operation\"),\n-\t\tOPT_BOOLEAN(0, \"continue\", &contin, \"continue the current operation\"),\n+\t\tOPT_BOOLEAN(0, \"quit\", &remove_state, \"end revert or cherry-pick sequence\"),\n+\t\tOPT_BOOLEAN(0, \"continue\", &contin, \"resume revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n \t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -147,6 +147,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_STRING(0, \"strategy\", &opts->strategy, \"strategy\", \"merge strategy\"),\n \t\tOPT_CALLBACK('X', \"strategy-option\", &opts, \"option\",\n \t\t\t\"option for merge strategy\", option_parse_x),\n+\t\t{ OPTION_BOOLEAN, 0, \"reset\", &remove_state, NULL,\n+\t\t\t\"alias for --quit (deprecated)\",\n+\t\t\tPARSE_OPT_HIDDEN | PARSE_OPT_NOARG },\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n@@ -168,13 +171,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \n \t/* Check for incompatible subcommands */\n \tverify_opt_mutually_compatible(me,\n-\t\t\t\t\"--reset\", reset,\n+\t\t\t\t\"--quit\", remove_state,\n \t\t\t\t\"--continue\", contin,\n \t\t\t\tNULL);\n \n \t/* Set the subcommand */\n-\tif (reset)\n-\t\topts->subcommand = REPLAY_RESET;\n+\tif (remove_state)\n+\t\topts->subcommand = REPLAY_REMOVE_STATE;\n \telse if (contin)\n \t\topts->subcommand = REPLAY_CONTINUE;\n \telse\n@@ -183,8 +186,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t/* Check for incompatible command line arguments */\n \tif (opts->subcommand != REPLAY_NONE) {\n \t\tchar *this_operation;\n-\t\tif (opts->subcommand == REPLAY_RESET)\n-\t\t\tthis_operation = \"--reset\";\n+\t\tif (opts->subcommand == REPLAY_REMOVE_STATE)\n+\t\t\tthis_operation = \"--quit\";\n \t\telse\n \t\t\tthis_operation = \"--continue\";\n \n@@ -965,7 +968,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t * cherry-pick should be handled differently from an existing\n \t * one that is being continued\n \t */\n-\tif (opts->subcommand == REPLAY_RESET) {\n+\tif (opts->subcommand == REPLAY_REMOVE_STATE) {\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n \t} else if (opts->subcommand == REPLAY_CONTINUE) {\n@@ -988,7 +991,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t\tif (create_seq_dir() < 0) {\n \t\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n \t\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\t\tadvise(_(\"or --reset to forget about it\"));\n+\t\t\tadvise(_(\"or --quit to forget about it\"));\n \t\t\treturn -1;\n \t\t}\n \t\tif (get_sha1(\"HEAD\", sha1)) {\ndiff --git a/sequencer.h b/sequencer.h\nindex 905d2950..f435fdb4 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -13,7 +13,7 @@\n *\n * With the aggressive flag, it additionally removes SEQ_OLD_DIR,\n * ignoring any errors. Inteded to be used by the sequencer's\n- * '--reset' subcommand.\n+ * '--quit' subcommand.\n */\n void remove_sequencer_state(int aggressive);\n \ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 3bca2b3d..bb67cdcb 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -13,7 +13,7 @@ test_description='Test cherry-pick continuation features\n . ./test-lib.sh\n \n pristine_detach () {\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \tgit checkout -f \"$1^0\" &&\n \tgit read-tree -u --reset HEAD &&\n \tgit clean -d -f -f -q -x\n@@ -70,18 +70,39 @@ test_expect_success 'cherry-pick cleans up sequencer state upon success' '\n \ttest_path_is_missing .git/sequencer\n '\n \n-test_expect_success '--reset does not complain when no cherry-pick is in progress' '\n+test_expect_success '--quit does not complain when no cherry-pick is in progress' '\n \tpristine_detach initial &&\n-\tgit cherry-pick --reset\n+\tgit cherry-pick --quit\n '\n \n-test_expect_success '--reset cleans up sequencer state' '\n+test_expect_success '--quit cleans up sequencer state' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \ttest_path_is_missing .git/sequencer\n '\n \n+test_expect_success 'cherry-pick --reset (another name for --quit)' '\n+\tpristine_detach initial &&\n+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tunrelated\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tfoo\n+\t:000000 100644 OBJID OBJID A\tunrelated\n+\tEOF\n+\ttest_must_fail git cherry-pick base..picked &&\n+\tgit cherry-pick --reset &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_must_fail git update-index --refresh &&\n+\t{\n+\t\tgit rev-list HEAD |\n+\t\tgit diff-tree --root --stdin |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\ndiff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh\nindex 4956caaf..3f86e8c5 100755\n--- a/t/t7106-reset-sequence.sh\n+++ b/t/t7106-reset-sequence.sh\n@@ -12,7 +12,7 @@ test_description='Test interaction of reset --hard with sequencer\n . ./test-lib.sh\n \n pristine_detach () {\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \tgit checkout -f \"$1^0\" &&\n \tgit read-tree -u --reset HEAD &&\n \tgit clean -d -f -f -q -x\n-- \n1.7.8.rc3\n"},{"id":"179822","messageId":"20111122111510.GC7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 2/3] revert: rearrange pick_revisions() for clarity","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:15:10Z","receivedAt":"2011-11-22T11:15:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Deal completely with \"cherry-pick --quit\" and --continue at the\nbeginning of pick_revisions(), leaving the rest of the function for\nthe more interesting \"git cherry-pick \" case.\n\nNo functional change intended. The impact is just to unindent the\ncode a little.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\n builtin/revert.c | 48 ++++++++++++++++++++++++------------------------\n 1 files changed, 24 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex e109fb11..2346cafb 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -971,40 +971,40 @@ static int pick_revisions(struct replay_opts *opts)\n \tif (opts->subcommand == REPLAY_REMOVE_STATE) {\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n-\t} else if (opts->subcommand == REPLAY_CONTINUE) {\n+\t}\n+\tif (opts->subcommand == REPLAY_CONTINUE) {\n \t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n-\t\t\tgoto error;\n+\t\t\treturn error(_(\"No %s in progress\"), action_name(opts));\n \t\tread_populate_opts(&opts);\n \t\tread_populate_todo(&todo_list, opts);\n \n \t\t/* Verify that the conflict has been resolved */\n \t\tif (!index_differs_from(\"HEAD\", 0))\n \t\t\ttodo_list = todo_list->next;\n-\t} else {\n-\t\t/*\n-\t\t * Start a new cherry-pick/ revert sequence; but\n-\t\t * first, make sure that an existing one isn't in\n-\t\t * progress\n-\t\t */\n+\t\treturn pick_commits(todo_list, opts);\n+\t}\n+\n+\t/*\n+\t * Start a new cherry-pick/ revert sequence; but\n+\t * first, make sure that an existing one isn't in\n+\t * progress\n+\t */\n \n-\t\twalk_revs_populate_todo(&todo_list, opts);\n-\t\tif (create_seq_dir() < 0) {\n-\t\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n-\t\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\t\tadvise(_(\"or --quit to forget about it\"));\n-\t\t\treturn -1;\n-\t\t}\n-\t\tif (get_sha1(\"HEAD\", sha1)) {\n-\t\t\tif (opts->action == REVERT)\n-\t\t\t\treturn error(_(\"Can't revert as initial commit\"));\n-\t\t\treturn error(_(\"Can't cherry-pick into empty head\"));\n-\t\t}\n-\t\tsave_head(sha1_to_hex(sha1));\n-\t\tsave_opts(opts);\n+\twalk_revs_populate_todo(&todo_list, opts);\n+\tif (create_seq_dir() < 0) {\n+\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n+\t\tadvise(_(\"Use --continue to continue the operation\"));\n+\t\tadvise(_(\"or --quit to forget about it\"));\n+\t\treturn -1;\n+\t}\n+\tif (get_sha1(\"HEAD\", sha1)) {\n+\t\tif (opts->action == REVERT)\n+\t\t\treturn error(_(\"Can't revert as initial commit\"));\n+\t\treturn error(_(\"Can't cherry-pick into empty head\"));\n \t}\n+\tsave_head(sha1_to_hex(sha1));\n+\tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n-error:\n-\treturn error(_(\"No %s in progress\"), action_name(opts));\n }\n \n int cmd_revert(int argc, const char **argv, const char *prefix)\n-- \n1.7.8.rc3\n"},{"id":"179823","messageId":"20111122111547.GD7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 3/3] revert: improve error message for cherry-pick during cherry-pick","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:15:47Z","receivedAt":"2011-11-22T11:15:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In the spirit of v1.6.3.3~3^2 (refuse to merge during a merge,\n2009-07-01), \"git cherry-pick\" refuses to start a new cherry-pick when\nin the middle of an existing conflicted cherry-pick in the following\nsequence:\n\n 1. git cherry-pick HEAD..origin\n 2. resolve conflicts\n 3. git cherry-pick HEAD..origin (instead of \"git cherry-pick\n --continue\", by mistake)\n\nGood. However, the error message on attempting step 3 is more\nconvoluted than necessary:\n\n $ git cherry-pick HEAD..origin\n error: .git/sequencer already exists.\n error: A cherry-pick or revert is in progress.\n hint: Use --continue to continue the operation\n hint: or --quit to forget about it\n fatal: cherry-pick failed\n\nClarify by removing the redundant first \"error:\" message, simplifying\nthe advice, and using lower-case and no full stops to be consistent\nwith other commands that prefix their messages with \"error:\", so it\nbecomes\n\n error: a cherry-pick or revert is already in progress\n hint: try \"git cherry-pick (--continue | --quit)\"\n fatal: cherry-pick failed\n\nThe \"fatal: cherry-pick failed\" line seems unnecessary, too, but\nthat can be fixed some other day.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Junio C Hamano \n---\n builtin/revert.c | 13 ++++++-------\n 1 files changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 2346cafb..1d112e4c 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -846,8 +846,11 @@ static int create_seq_dir(void)\n {\n \tconst char *seq_dir = git_path(SEQ_DIR);\n \n-\tif (file_exists(seq_dir))\n-\t\treturn error(_(\"%s already exists.\"), seq_dir);\n+\tif (file_exists(seq_dir)) {\n+\t\terror(_(\"a cherry-pick or revert is already in progress\"));\n+\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit)\\\"\"));\n+\t\treturn -1;\n+\t}\n \telse if (mkdir(seq_dir, 0777) < 0)\n \t\tdie_errno(_(\"Could not create sequencer directory %s\"), seq_dir);\n \treturn 0;\n@@ -991,12 +994,8 @@ static int pick_revisions(struct replay_opts *opts)\n \t */\n \n \twalk_revs_populate_todo(&todo_list, opts);\n-\tif (create_seq_dir() < 0) {\n-\t\terror(_(\"A cherry-pick or revert is in progress.\"));\n-\t\tadvise(_(\"Use --continue to continue the operation\"));\n-\t\tadvise(_(\"or --quit to forget about it\"));\n+\tif (create_seq_dir() < 0)\n \t\treturn -1;\n-\t}\n \tif (get_sha1(\"HEAD\", sha1)) {\n \t\tif (opts->action == REVERT)\n \t\t\treturn error(_(\"Can't revert as initial commit\"));\n-- \n1.7.8.rc3\n"},{"id":"179824","messageId":"20111122111736.GE7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 4/3] revert: write REVERT_HEAD pseudoref during conflicted revert","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:17:36Z","receivedAt":"2011-11-22T11:17:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When conflicts are encountered while reverting a commit, it can be\nhandy to have the name of that commit easily available. For example,\nto produce a copy of the patch to refer to while resolving conflicts:\n\n\t$ git revert 2eceb2a8\n\terror: could not revert 2eceb2a8... awesome, buggy feature\n\t$ git show -R REVERT_HEAD >the-patch\n\t$ edit $(git diff --name-only)\n\nSet a REVERT_HEAD pseudoref when \"git revert\" does not make a commit,\nfor cases like this. This also makes it possible for scripts to\ndistinguish between a revert that encountered conflicts and other\nsources of an unmerged index.\n\nAfter successfully committing, resetting with \"git reset\", or moving\nto another commit with \"git checkout\" or \"git reset\", the pseudoref is\nno longer useful, so remove it.\n\nSigned-off-by: Jonathan Nieder \n---\n branch.c | 1 +\n builtin/commit.c | 1 +\n builtin/revert.c | 8 +++--\n t/t3507-cherry-pick-conflict.sh | 54 +++++++++++++++++++++++++++++++++++++++\n 4 files changed, 61 insertions(+), 3 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex d8098762..025a97be 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -241,6 +241,7 @@ void create_branch(const char *head,\n void remove_branch_state(void)\n {\n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n+\tunlink(git_path(\"REVERT_HEAD\"));\n \tunlink(git_path(\"MERGE_HEAD\"));\n \tunlink(git_path(\"MERGE_RR\"));\n \tunlink(git_path(\"MERGE_MSG\"));\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c46f2d18..8f2bebec 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1514,6 +1514,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t}\n \n \tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n+\tunlink(git_path(\"REVERT_HEAD\"));\n \tunlink(git_path(\"MERGE_HEAD\"));\n \tunlink(git_path(\"MERGE_MSG\"));\n \tunlink(git_path(\"MERGE_MODE\"));\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 1d112e4c..f5ba67a5 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -289,7 +289,7 @@ static char *get_encoding(const char *message)\n \treturn NULL;\n }\n \n-static void write_cherry_pick_head(struct commit *commit)\n+static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)\n {\n \tconst char *filename;\n \tint fd;\n@@ -297,7 +297,7 @@ static void write_cherry_pick_head(struct commit *commit)\n \n \tstrbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n \n-\tfilename = git_path(\"CHERRY_PICK_HEAD\");\n+\tfilename = git_path(pseudoref);\n \tfd = open(filename, O_WRONLY | O_CREAT, 0666);\n \tif (fd < 0)\n \t\tdie_errno(_(\"Could not open '%s' for writing\"), filename);\n@@ -597,7 +597,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t * write it at all.\n \t */\n \tif (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))\n-\t\twrite_cherry_pick_head(commit);\n+\t\twrite_cherry_pick_head(commit, \"CHERRY_PICK_HEAD\");\n+\tif (opts->action == REVERT && ((opts->no_commit && res == 0) || res == 1))\n+\t\twrite_cherry_pick_head(commit, \"REVERT_HEAD\");\n \n \tif (res) {\n \t\terror(opts->action == REVERT\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex cb45574a..ee1659c1 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -253,6 +253,60 @@ test_expect_success 'revert also handles conflicts sanely' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'failed revert sets REVERT_HEAD' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git revert picked &&\n+\ttest_cmp_rev picked REVERT_HEAD\n+'\n+\n+test_expect_success 'successful revert does not set REVERT_HEAD' '\n+\tpristine_detach base &&\n+\tgit revert base &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n+\ttest_must_fail git rev-parse --verify REVERT_HEAD\n+'\n+\n+test_expect_success 'revert --no-commit sets REVERT_HEAD' '\n+\tpristine_detach base &&\n+\tgit revert --no-commit base &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n+\ttest_cmp_rev base REVERT_HEAD\n+'\n+\n+test_expect_success 'revert w/dirty tree does not set REVERT_HEAD' '\n+\tpristine_detach base &&\n+\techo foo > foo &&\n+\ttest_must_fail git revert base &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n+\ttest_must_fail git rev-parse --verify REVERT_HEAD\n+'\n+\n+test_expect_success 'GIT_CHERRY_PICK_HELP does not suppress REVERT_HEAD' '\n+\tpristine_detach initial &&\n+\t(\n+\t\tGIT_CHERRY_PICK_HELP=\"and then do something else\" &&\n+\t\tGIT_REVERT_HELP=\"and then do something else, again\" &&\n+\t\texport GIT_CHERRY_PICK_HELP GIT_REVERT_HELP &&\n+\t\ttest_must_fail git revert picked\n+\t) &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n+\ttest_cmp_rev picked REVERT_HEAD\n+'\n+\n+test_expect_success 'git reset clears REVERT_HEAD' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git revert picked &&\n+\tgit reset &&\n+\ttest_must_fail git rev-parse --verify REVERT_HEAD\n+'\n+\n+test_expect_success 'failed commit does not clear REVERT_HEAD' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git revert picked &&\n+\ttest_must_fail git commit &&\n+\ttest_cmp_rev picked REVERT_HEAD\n+'\n+\n test_expect_success 'revert conflict, diff3 -m style' '\n \tpristine_detach initial &&\n \tgit config merge.conflictstyle diff3 &&\n-- \n1.7.8.rc3\n"},{"id":"179825","messageId":"20111122112001.GF7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 5/3] revert: introduce --abort to cancel a failed cherry-pick","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:20:02Z","receivedAt":"2011-11-22T11:20:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"After running some ill-advised command like \"git cherry-pick\nHEAD..linux-next\", the bewildered novice may want to return to more\nfamiliar territory. Introduce a \"git cherry-pick --abort\" command\nthat rolls back the entire cherry-pick sequence and places the\nrepository back on solid ground.\n\nJust like \"git merge --abort\", this internally uses \"git reset\n--merge\", so local changes not involved in the conflict resolution are\npreserved.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-cherry-pick.txt | 1 +\n Documentation/git-revert.txt | 1 +\n Documentation/sequencer.txt | 3 +\n builtin/revert.c | 87 ++++++++++++++++++++++++++++++-\n t/t3510-cherry-pick-sequence.sh | 102 ++++++++++++++++++++++++++++++++++++-\n 5 files changed, 189 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 21998b82..fed5097e 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ...\n 'git cherry-pick' --continue\n 'git cherry-pick' --quit\n+'git cherry-pick' --abort\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex b0fcabc8..b699a345 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n 'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] ...\n 'git revert' --continue\n 'git revert' --quit\n+'git revert' --abort\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 75f8e869..5747f442 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -7,3 +7,6 @@\n \tForget about the current operation in progress. Can be used\n \tto clear the sequencer state after a failed cherry-pick or\n \trevert.\n+\n+--abort::\n+\tCancel the operation and return to the pre-sequence state.\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex f5ba67a5..70a5fbb6 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -40,7 +40,12 @@ static const char * const cherry_pick_usage[] = {\n };\n \n enum replay_action { REVERT, CHERRY_PICK };\n-enum replay_subcommand { REPLAY_NONE, REPLAY_REMOVE_STATE, REPLAY_CONTINUE };\n+enum replay_subcommand {\n+\tREPLAY_NONE,\n+\tREPLAY_REMOVE_STATE,\n+\tREPLAY_CONTINUE,\n+\tREPLAY_ROLLBACK\n+};\n \n struct replay_opts {\n \tenum replay_action action;\n@@ -135,9 +140,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tconst char *me = action_name(opts);\n \tint remove_state = 0;\n \tint contin = 0;\n+\tint rollback = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN(0, \"quit\", &remove_state, \"end revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN(0, \"continue\", &contin, \"resume revert or cherry-pick sequence\"),\n+\t\tOPT_BOOLEAN(0, \"abort\", &rollback, \"cancel revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n \t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -173,6 +180,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tverify_opt_mutually_compatible(me,\n \t\t\t\t\"--quit\", remove_state,\n \t\t\t\t\"--continue\", contin,\n+\t\t\t\t\"--abort\", rollback,\n \t\t\t\tNULL);\n \n \t/* Set the subcommand */\n@@ -180,6 +188,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->subcommand = REPLAY_REMOVE_STATE;\n \telse if (contin)\n \t\topts->subcommand = REPLAY_CONTINUE;\n+\telse if (rollback)\n+\t\topts->subcommand = REPLAY_ROLLBACK;\n \telse\n \t\topts->subcommand = REPLAY_NONE;\n \n@@ -188,8 +198,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tchar *this_operation;\n \t\tif (opts->subcommand == REPLAY_REMOVE_STATE)\n \t\t\tthis_operation = \"--quit\";\n-\t\telse\n+\t\telse if (opts->subcommand == REPLAY_CONTINUE)\n \t\t\tthis_operation = \"--continue\";\n+\t\telse {\n+\t\t\tassert(opts->subcommand == REPLAY_ROLLBACK);\n+\t\t\tthis_operation = \"--abort\";\n+\t\t}\n \n \t\tverify_opt_compatible(me, this_operation,\n \t\t\t\t\"--no-commit\", opts->no_commit,\n@@ -850,7 +864,7 @@ static int create_seq_dir(void)\n \n \tif (file_exists(seq_dir)) {\n \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n-\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit)\\\"\"));\n+\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n \t\treturn -1;\n \t}\n \telse if (mkdir(seq_dir, 0777) < 0)\n@@ -873,6 +887,71 @@ static void save_head(const char *head)\n \t\tdie(_(\"Error wrapping up %s.\"), head_file);\n }\n \n+static int reset_for_rollback(const unsigned char *sha1)\n+{\n+\tconst char *argv[4];\t/* reset --merge + NULL */\n+\targv[0] = \"reset\";\n+\targv[1] = \"--merge\";\n+\targv[2] = sha1_to_hex(sha1);\n+\targv[3] = NULL;\n+\treturn run_command_v_opt(argv, RUN_GIT_CMD);\n+}\n+\n+static int rollback_single_pick(void)\n+{\n+\tunsigned char head_sha1[20];\n+\n+\tif (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n+\t !file_exists(git_path(\"REVERT_HEAD\")))\n+\t\treturn error(_(\"no cherry-pick or revert in progress\"));\n+\tif (!resolve_ref(\"HEAD\", head_sha1, 0, NULL))\n+\t\treturn error(_(\"cannot resolve HEAD\"));\n+\tif (is_null_sha1(head_sha1))\n+\t\treturn error(_(\"cannot abort from a branch yet to be born\"));\n+\treturn reset_for_rollback(head_sha1);\n+}\n+\n+static int sequencer_rollback(struct replay_opts *opts)\n+{\n+\tconst char *filename;\n+\tFILE *f;\n+\tunsigned char sha1[20];\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tfilename = git_path(SEQ_HEAD_FILE);\n+\tf = fopen(filename, \"r\");\n+\tif (!f && errno == ENOENT) {\n+\t\t/*\n+\t\t * There is no multiple-cherry-pick in progress.\n+\t\t * If CHERRY_PICK_HEAD or REVERT_HEAD indicates\n+\t\t * a single-cherry-pick in progress, abort that.\n+\t\t */\n+\t\treturn rollback_single_pick();\n+\t}\n+\tif (!f)\n+\t\treturn error(_(\"cannot open %s: %s\"), filename,\n+\t\t\t\t\t\tstrerror(errno));\n+\tif (strbuf_getline(&buf, f, '\\n')) {\n+\t\terror(_(\"cannot read %s: %s\"), filename, ferror(f) ?\n+\t\t\tstrerror(errno) : _(\"unexpected end of file\"));\n+\t\tgoto fail;\n+\t}\n+\tif (get_sha1_hex(buf.buf, sha1) || buf.buf[40] != '\\0') {\n+\t\terror(_(\"stored pre-cherry-pick HEAD file '%s' is corrupt\"),\n+\t\t\tfilename);\n+\t\tgoto fail;\n+\t}\n+\tif (reset_for_rollback(sha1))\n+\t\tgoto fail;\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\treturn 0;\n+fail:\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\treturn -1;\n+}\n+\n static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n {\n \tconst char *todo_file = git_path(SEQ_TODO_FILE);\n@@ -977,6 +1056,8 @@ static int pick_revisions(struct replay_opts *opts)\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n \t}\n+\tif (opts->subcommand == REPLAY_ROLLBACK)\n+\t\treturn sequencer_rollback(opts);\n \tif (opts->subcommand == REPLAY_CONTINUE) {\n \t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n \t\t\treturn error(_(\"No %s in progress\"), action_name(opts));\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex bb67cdcb..63297ad6 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -6,7 +6,7 @@ test_description='Test cherry-pick continuation features\n + picked: rewrites foo to c\n + unrelatedpick: rewrites unrelated to reallyunrelated\n + base: rewrites foo to b\n- + initial: writes foo as a, unrelated as unrelated\n+ + initial: writes foo as a, unrelated as unrelated, bar as bar\n \n '\n \n@@ -19,9 +19,16 @@ pristine_detach () {\n \tgit clean -d -f -f -q -x\n }\n \n+test_cmp_rev () {\n+\tgit rev-parse --verify \"$1\" >expect.rev &&\n+\tgit rev-parse --verify \"$2\" >actual.rev &&\n+\ttest_cmp expect.rev actual.rev\n+}\n+\n test_expect_success setup '\n \techo unrelated >unrelated &&\n-\tgit add unrelated &&\n+\techo bar >bar &&\n+\tgit add unrelated bar &&\n \ttest_commit initial foo a &&\n \ttest_commit base foo b &&\n \ttest_commit unrelatedpick unrelated reallyunrelated &&\n@@ -75,6 +82,11 @@ test_expect_success '--quit does not complain when no cherry-pick is in progress\n \tgit cherry-pick --quit\n '\n \n+test_expect_success '--abort requires cherry-pick in progress' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick --abort\n+'\n+\n test_expect_success '--quit cleans up sequencer state' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n@@ -88,6 +100,7 @@ test_expect_success 'cherry-pick --reset (another name for --quit)' '\n \tOBJID\n \t:100644 100644 OBJID OBJID M\tunrelated\n \tOBJID\n+\t:000000 100644 OBJID OBJID A\tbar\n \t:000000 100644 OBJID OBJID A\tfoo\n \t:000000 100644 OBJID OBJID A\tunrelated\n \tEOF\n@@ -103,6 +116,79 @@ test_expect_success 'cherry-pick --reset (another name for --quit)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--abort to cancel multiple cherry-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success '--abort to cancel single cherry-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success 'cherry-pick --abort to cancel multiple revert' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert base..picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success 'revert --abort works, too' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert base..picked &&\n+\tgit revert --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD\n+'\n+\n+test_expect_success '--abort to cancel single revert' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert picked &&\n+\tgit revert --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success '--abort keeps unrelated change, easy case' '\n+\tpristine_detach initial &&\n+\techo changed >expect &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo changed >bar &&\n+\tgit cherry-pick --abort &&\n+\ttest_cmp expect bar\n+'\n+\n+test_expect_success '--abort refuses to clobber unrelated change, harder case' '\n+\tpristine_detach initial &&\n+\techo changed >expect &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo changed >unrelated &&\n+\ttest_must_fail git cherry-pick --abort &&\n+\ttest_cmp expect unrelated &&\n+\tgit rev-list HEAD >log &&\n+\ttest_line_count = 2 log &&\n+\ttest_must_fail git update-index --refresh &&\n+\n+\tgit checkout unrelated &&\n+\tgit cherry-pick --abort &&\n+\ttest_cmp_rev initial HEAD\n+'\n+\n test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n@@ -121,12 +207,23 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le\n \tOBJID\n \t:100644 100644 OBJID OBJID M\tunrelated\n \tOBJID\n+\t:000000 100644 OBJID OBJID A\tbar\n \t:000000 100644 OBJID OBJID A\tfoo\n \t:000000 100644 OBJID OBJID A\tunrelated\n \tEOF\n \ttest_cmp expect actual\n '\n \n+test_expect_failure '--abort after last commit in sequence' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..anotherpick &&\n@@ -168,6 +265,7 @@ test_expect_success '--continue continues after conflicts are resolved' '\n \tOBJID\n \t:100644 100644 OBJID OBJID M\tunrelated\n \tOBJID\n+\t:000000 100644 OBJID OBJID A\tbar\n \t:000000 100644 OBJID OBJID A\tfoo\n \t:000000 100644 OBJID OBJID A\tunrelated\n \tEOF\n-- \n1.7.8.rc3\n"},{"id":"179826","messageId":"20111122112046.GG7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"[PATCH 6/3] revert: remove --reset compatibility option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:20:46Z","receivedAt":"2011-11-22T11:20:46Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Remove the \"git cherry-pick --reset\" option, which has a different\npreferred spelling nowadays (\"--quit\"). Luckily the old --reset name\nwas not around long enough for anyone to get used to it.\n\nSigned-off-by: Jonathan Nieder \n---\nThanks for reading.\n\n builtin/revert.c | 3 ---\n t/t3510-cherry-pick-sequence.sh | 4 ++--\n 2 files changed, 2 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 70a5fbb6..0c61668b 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -154,9 +154,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_STRING(0, \"strategy\", &opts->strategy, \"strategy\", \"merge strategy\"),\n \t\tOPT_CALLBACK('X', \"strategy-option\", &opts, \"option\",\n \t\t\t\"option for merge strategy\", option_parse_x),\n-\t\t{ OPTION_BOOLEAN, 0, \"reset\", &remove_state, NULL,\n-\t\t\t\"alias for --quit (deprecated)\",\n-\t\t\tPARSE_OPT_HIDDEN | PARSE_OPT_NOARG },\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 63297ad6..f28a4e8a 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -94,7 +94,7 @@ test_expect_success '--quit cleans up sequencer state' '\n \ttest_path_is_missing .git/sequencer\n '\n \n-test_expect_success 'cherry-pick --reset (another name for --quit)' '\n+test_expect_success '--quit keeps HEAD and conflicted index intact' '\n \tpristine_detach initial &&\n \tcat >expect <<-\\EOF &&\n \tOBJID\n@@ -105,7 +105,7 @@ test_expect_success 'cherry-pick --reset (another name for --quit)' '\n \t:000000 100644 OBJID OBJID A\tunrelated\n \tEOF\n \ttest_must_fail git cherry-pick base..picked &&\n-\tgit cherry-pick --reset &&\n+\tgit cherry-pick --quit &&\n \ttest_path_is_missing .git/sequencer &&\n \ttest_must_fail git update-index --refresh &&\n \t{\n-- \n1.7.8.rc3\n"},{"id":"179828","messageId":"20111122112700.GH7399@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111122111207.GA7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH v2 0/3] Re: cherry-pick/revert error messages","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T11:27:00Z","receivedAt":"2011-11-22T11:27:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Errata:\n\nJonathan Nieder wrote:\n\n> May the series can provide some amusement.\n\n\"Maybe\".\n\n[...]\n> Jonathan Nieder (3):\n> revert: rename --reset option to --quit\n> revert: rearrange pick_revisions() for clarity\n> revert: improve error message for cherry-pick during cherry-pick\n\nI took these from pu before tweaking them, so in a sense it's\nappropriate that Junio's sign-off is there, but I do not mean to claim\nhe looked at and acked the tweaked form. I should have just left my\nsign-off.\n\nSorry for the noise.\n"},{"id":"179850","messageId":"CACnwZYd3ocb2dx9-2iHH4xmP6_O0M9gRdr3RvDpaAyZq5wPZ0w@mail.gmail.com","threadId":"28989","inReplyTo":"20111122111736.GE7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 4/3] revert: write REVERT_HEAD pseudoref during conflicted revert","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-11-22T21:40:06Z","receivedAt":"2011-11-22T21:40:06Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"There seems to be something wrong with the subject. I says [4/3],\ninstead of [4/6]?\n\nOn Tue, Nov 22, 2011 at 9:17 AM, Jonathan Nieder wrote:\n> When conflicts are encountered while reverting a commit, it can be\n> handy to have the name of that commit easily available.  For example,\n> to produce a copy of the patch to refer to while resolving conflicts:\n>\n>        $ git revert 2eceb2a8\n>        error: could not revert 2eceb2a8... awesome, buggy feature\n>        $ git show -R REVERT_HEAD >the-patch\n>        $ edit $(git diff --name-only)\n>\n> Set a REVERT_HEAD pseudoref when \"git revert\" does not make a commit,\n> for cases like this.  This also makes it possible for scripts to\n> distinguish between a revert that encountered conflicts and other\n> sources of an unmerged index.\n>\n> After successfully committing, resetting with \"git reset\", or moving\n> to another commit with \"git checkout\" or \"git reset\", the pseudoref is\n> no longer useful, so remove it.\n>\n> Signed-off-by: Jonathan Nieder \n> ---\n>  branch.c                        |    1 +\n>  builtin/commit.c                |    1 +\n>  builtin/revert.c                |    8 +++--\n>  t/t3507-cherry-pick-conflict.sh |   54 +++++++++++++++++++++++++++++++++++++++\n>  4 files changed, 61 insertions(+), 3 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index d8098762..025a97be 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -241,6 +241,7 @@ void create_branch(const char *head,\n>  void remove_branch_state(void)\n>  {\n>        unlink(git_path(\"CHERRY_PICK_HEAD\"));\n> +       unlink(git_path(\"REVERT_HEAD\"));\n>        unlink(git_path(\"MERGE_HEAD\"));\n>        unlink(git_path(\"MERGE_RR\"));\n>        unlink(git_path(\"MERGE_MSG\"));\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index c46f2d18..8f2bebec 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1514,6 +1514,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>        }\n>\n>        unlink(git_path(\"CHERRY_PICK_HEAD\"));\n> +       unlink(git_path(\"REVERT_HEAD\"));\n>        unlink(git_path(\"MERGE_HEAD\"));\n>        unlink(git_path(\"MERGE_MSG\"));\n>        unlink(git_path(\"MERGE_MODE\"));\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 1d112e4c..f5ba67a5 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -289,7 +289,7 @@ static char *get_encoding(const char *message)\n>        return NULL;\n>  }\n>\n> -static void write_cherry_pick_head(struct commit *commit)\n> +static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)\n>  {\n>        const char *filename;\n>        int fd;\n> @@ -297,7 +297,7 @@ static void write_cherry_pick_head(struct commit *commit)\n>\n>        strbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n>\n> -       filename = git_path(\"CHERRY_PICK_HEAD\");\n> +       filename = git_path(pseudoref);\n>        fd = open(filename, O_WRONLY | O_CREAT, 0666);\n>        if (fd < 0)\n>                die_errno(_(\"Could not open '%s' for writing\"), filename);\n> @@ -597,7 +597,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>         * write it at all.\n>         */\n>        if (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))\n> -               write_cherry_pick_head(commit);\n> +               write_cherry_pick_head(commit, \"CHERRY_PICK_HEAD\");\n> +       if (opts->action == REVERT && ((opts->no_commit && res == 0) || res == 1))\n> +               write_cherry_pick_head(commit, \"REVERT_HEAD\");\n>\n>        if (res) {\n>                error(opts->action == REVERT\n> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> index cb45574a..ee1659c1 100755\n> --- a/t/t3507-cherry-pick-conflict.sh\n> +++ b/t/t3507-cherry-pick-conflict.sh\n> @@ -253,6 +253,60 @@ test_expect_success 'revert also handles conflicts sanely' '\n>        test_cmp expected actual\n>  '\n>\n> +test_expect_success 'failed revert sets REVERT_HEAD' '\n> +       pristine_detach initial &&\n> +       test_must_fail git revert picked &&\n> +       test_cmp_rev picked REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'successful revert does not set REVERT_HEAD' '\n> +       pristine_detach base &&\n> +       git revert base &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n> +       test_must_fail git rev-parse --verify REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'revert --no-commit sets REVERT_HEAD' '\n> +       pristine_detach base &&\n> +       git revert --no-commit base &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n> +       test_cmp_rev base REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'revert w/dirty tree does not set REVERT_HEAD' '\n> +       pristine_detach base &&\n> +       echo foo > foo &&\n> +       test_must_fail git revert base &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n> +       test_must_fail git rev-parse --verify REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'GIT_CHERRY_PICK_HELP does not suppress REVERT_HEAD' '\n> +       pristine_detach initial &&\n> +       (\n> +               GIT_CHERRY_PICK_HELP=\"and then do something else\" &&\n> +               GIT_REVERT_HELP=\"and then do something else, again\" &&\n> +               export GIT_CHERRY_PICK_HELP GIT_REVERT_HELP &&\n> +               test_must_fail git revert picked\n> +       ) &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n> +       test_cmp_rev picked REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'git reset clears REVERT_HEAD' '\n> +       pristine_detach initial &&\n> +       test_must_fail git revert picked &&\n> +       git reset &&\n> +       test_must_fail git rev-parse --verify REVERT_HEAD\n> +'\n> +\n> +test_expect_success 'failed commit does not clear REVERT_HEAD' '\n> +       pristine_detach initial &&\n> +       test_must_fail git revert picked &&\n> +       test_must_fail git commit &&\n> +       test_cmp_rev picked REVERT_HEAD\n> +'\n> +\n>  test_expect_success 'revert conflict, diff3 -m style' '\n>        pristine_detach initial &&\n>        git config merge.conflictstyle diff3 &&\n> --\n> 1.7.8.rc3\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"179851","messageId":"7vzkfnde05.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111122112046.GG7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 6/3] revert: remove --reset compatibility option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-22T21:49:14Z","receivedAt":"2011-11-22T21:49:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Remove the \"git cherry-pick --reset\" option, which has a different\n> preferred spelling nowadays (\"--quit\"). Luckily the old --reset name\n> was not around long enough for anyone to get used to it.\n\nIf we want to reach this point eventually, given that --reset was not in\nany released version, and also given that we are deep in pre-release\nphase, we probably should either just apply the first one (and perhaps\nthis one) from the series immediately before 1.7.8 final, or delay the\nfinal and swallow the entire series.\n\nI am not quite convinced that quit is *that* superiour over reset\nthough. The description in 1/3 \"has a confusing name\" does not say why it\nis confusing, either, to help readers agree with the conclusion.\n"},{"id":"179852","messageId":"20111122231145.GE6274@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"7vzkfnde05.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/3] revert: remove --reset compatibility option","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-22T23:11:45Z","receivedAt":"2011-11-22T23:11:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> If we want to reach this point eventually, given that --reset was not in\n> any released version, and also given that we are deep in pre-release\n> phase, we probably should either just apply the first one (and perhaps\n> this one) from the series immediately before 1.7.8 final, or delay the\n> final and swallow the entire series.\n\nMakes sense.\n\n> I am not quite convinced that quit is *that* superiour over reset\n> though. The description in 1/3 \"has a confusing name\" does not say why it\n> is confusing, either, to help readers agree with the conclusion.\n\nOk, a few words about this.\n\nI should say that I am not deeply attached to --quit. If some other\nword for \"abandon\" (or some other concept entirely) is more obvious,\nI'd be happy to see it used. From a couple of days of practice, I can\nsay that --quit feels fine in a way that --reset didn't, but not much\nmore.\n\n--reset is a cognate to \"git reset\", the command to make some\ncombination of the HEAD, index, and worktree just so. In other\ncontexts --- rebooting a machine, resetting an alarm clock --- to\nreset something also means to return it to a known state. By\ncontrast, the \"git cherry-pick --abandon\" command is about leaving the\nstate alone as much as possible while escaping from the context of a\ncherry-pick sequence.\n\nHow does that do harm, though? My main fear is that people would not\nnotice that --reset is the option they want to use and would opt to\n\"rm -r .git/sequencer\" instead, which is a kind of error-prone\nwizardry that should not be required.\n\n(I assume cherry-pick --reset was named by analogy with \"git bisect\nreset\", the command to abandon a bisection and return to the branch\nyou were on before bisection, or a branch or commit named on the\ncommand line. Which is not analagous to cherry-pick --abandon at\nall. The command people often run, \"git bisect reset HEAD\", is a\ncloser analog if you squint right.)\n"},{"id":"179853","messageId":"7vvcqbd8yk.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111122231145.GE6274@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 6/3] revert: remove --reset compatibility option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-22T23:38:11Z","receivedAt":"2011-11-22T23:38:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n>\n>> If we want to reach this point eventually, given that --reset was not in\n>> any released version, and also given that we are deep in pre-release\n>> phase, we probably should either just apply the first one (and perhaps\n>> this one) from the series immediately before 1.7.8 final, or delay the\n>> final and swallow the entire series.\n>\n> Makes sense.\n\nOk, then let's do the latter, and make tomorrow's pushout a 1.7.8-rc4; I\nread the series over and they seem to make sense.\n"},{"id":"179854","messageId":"7vr50zd5x0.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111122112001.GF7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 5/3] revert: introduce --abort to cancel a failed cherry-pick","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-23T00:43:55Z","receivedAt":"2011-11-23T00:43:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> After running some ill-advised command like \"git cherry-pick\n> HEAD..linux-next\", the bewildered novice may want to return to more\n> familiar territory. Introduce a \"git cherry-pick --abort\" command\n> that rolls back the entire cherry-pick sequence and places the\n> repository back on solid ground.\n>\n> Just like \"git merge --abort\", this internally uses \"git reset\n> --merge\", so local changes not involved in the conflict resolution are\n> preserved.\n>\n> Signed-off-by: Jonathan Nieder \n> ---\n> ...\n> @@ -168,6 +265,7 @@ test_expect_success '--continue continues after conflicts are resolved' '\n> \tOBJID\n> \t:100644 100644 OBJID OBJID M\tunrelated\n> \tOBJID\n> +\t:000000 100644 OBJID OBJID A\tbar\n> \t:000000 100644 OBJID OBJID A\tfoo\n> \t:000000 100644 OBJID OBJID A\tunrelated\n> \tEOF\n\nWhat is this hunk about? Don't you also need another one to the test\nafter that one? When merged with rr/revert-cherry-pick series, t3510 seems\nto misbehave around this area.\n"},{"id":"179855","messageId":"20111123012721.GA14217@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"7vr50zd5x0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/3] revert: introduce --abort to cancel a failed cherry-pick","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-23T01:27:21Z","receivedAt":"2011-11-23T01:27:21Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> ...\n>> @@ -168,6 +265,7 @@ test_expect_success '--continue continues after conflicts are resolved' '\n>> \tOBJID\n>> \t:100644 100644 OBJID OBJID M\tunrelated\n>> \tOBJID\n>> +\t:000000 100644 OBJID OBJID A\tbar\n>> \t:000000 100644 OBJID OBJID A\tfoo\n>> \t:000000 100644 OBJID OBJID A\tunrelated\n>> \tEOF\n>\n> What is this hunk about?\n\nJust laziness. It would have been more polite for changes to the\nscript's setup to be made in such a way as not to require changing\nunrelated tests, like this.\n\n-- >8 --\nSubject: revert: introduce --abort to cancel a failed cherry-pick\n\nAfter running some ill-advised command like \"git cherry-pick\nHEAD..linux-next\", the bewildered novice may want to return to more\nfamiliar territory. Introduce a \"git cherry-pick --abort\" command\nthat rolls back the entire cherry-pick sequence and places the\nrepository back on solid ground.\n\nJust like \"git merge --abort\", this internally uses \"git reset\n--merge\", so local changes not involved in the conflict resolution are\npreserved.\n\nSigned-off-by: Jonathan Nieder \n---\n Documentation/git-cherry-pick.txt | 1 +\n Documentation/git-revert.txt | 1 +\n Documentation/sequencer.txt | 3 +\n builtin/revert.c | 87 ++++++++++++++++++++++++++++++++-\n t/t3510-cherry-pick-sequence.sh | 96 +++++++++++++++++++++++++++++++++++++\n 5 files changed, 185 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 21998b82..fed5097e 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n 'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] ...\n 'git cherry-pick' --continue\n 'git cherry-pick' --quit\n+'git cherry-pick' --abort\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt\nindex b0fcabc8..b699a345 100644\n--- a/Documentation/git-revert.txt\n+++ b/Documentation/git-revert.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n 'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] ...\n 'git revert' --continue\n 'git revert' --quit\n+'git revert' --abort\n \n DESCRIPTION\n -----------\ndiff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\nindex 75f8e869..5747f442 100644\n--- a/Documentation/sequencer.txt\n+++ b/Documentation/sequencer.txt\n@@ -7,3 +7,6 @@\n \tForget about the current operation in progress. Can be used\n \tto clear the sequencer state after a failed cherry-pick or\n \trevert.\n+\n+--abort::\n+\tCancel the operation and return to the pre-sequence state.\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex f5ba67a5..70a5fbb6 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -40,7 +40,12 @@ static const char * const cherry_pick_usage[] = {\n };\n \n enum replay_action { REVERT, CHERRY_PICK };\n-enum replay_subcommand { REPLAY_NONE, REPLAY_REMOVE_STATE, REPLAY_CONTINUE };\n+enum replay_subcommand {\n+\tREPLAY_NONE,\n+\tREPLAY_REMOVE_STATE,\n+\tREPLAY_CONTINUE,\n+\tREPLAY_ROLLBACK\n+};\n \n struct replay_opts {\n \tenum replay_action action;\n@@ -135,9 +140,11 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tconst char *me = action_name(opts);\n \tint remove_state = 0;\n \tint contin = 0;\n+\tint rollback = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN(0, \"quit\", &remove_state, \"end revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN(0, \"continue\", &contin, \"resume revert or cherry-pick sequence\"),\n+\t\tOPT_BOOLEAN(0, \"abort\", &rollback, \"cancel revert or cherry-pick sequence\"),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n \t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n \t\tOPT_NOOP_NOARG('r', NULL),\n@@ -173,6 +180,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tverify_opt_mutually_compatible(me,\n \t\t\t\t\"--quit\", remove_state,\n \t\t\t\t\"--continue\", contin,\n+\t\t\t\t\"--abort\", rollback,\n \t\t\t\tNULL);\n \n \t/* Set the subcommand */\n@@ -180,6 +188,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->subcommand = REPLAY_REMOVE_STATE;\n \telse if (contin)\n \t\topts->subcommand = REPLAY_CONTINUE;\n+\telse if (rollback)\n+\t\topts->subcommand = REPLAY_ROLLBACK;\n \telse\n \t\topts->subcommand = REPLAY_NONE;\n \n@@ -188,8 +198,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tchar *this_operation;\n \t\tif (opts->subcommand == REPLAY_REMOVE_STATE)\n \t\t\tthis_operation = \"--quit\";\n-\t\telse\n+\t\telse if (opts->subcommand == REPLAY_CONTINUE)\n \t\t\tthis_operation = \"--continue\";\n+\t\telse {\n+\t\t\tassert(opts->subcommand == REPLAY_ROLLBACK);\n+\t\t\tthis_operation = \"--abort\";\n+\t\t}\n \n \t\tverify_opt_compatible(me, this_operation,\n \t\t\t\t\"--no-commit\", opts->no_commit,\n@@ -850,7 +864,7 @@ static int create_seq_dir(void)\n \n \tif (file_exists(seq_dir)) {\n \t\terror(_(\"a cherry-pick or revert is already in progress\"));\n-\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit)\\\"\"));\n+\t\tadvise(_(\"try \\\"git cherry-pick (--continue | --quit | --abort)\\\"\"));\n \t\treturn -1;\n \t}\n \telse if (mkdir(seq_dir, 0777) < 0)\n@@ -873,6 +887,71 @@ static void save_head(const char *head)\n \t\tdie(_(\"Error wrapping up %s.\"), head_file);\n }\n \n+static int reset_for_rollback(const unsigned char *sha1)\n+{\n+\tconst char *argv[4];\t/* reset --merge + NULL */\n+\targv[0] = \"reset\";\n+\targv[1] = \"--merge\";\n+\targv[2] = sha1_to_hex(sha1);\n+\targv[3] = NULL;\n+\treturn run_command_v_opt(argv, RUN_GIT_CMD);\n+}\n+\n+static int rollback_single_pick(void)\n+{\n+\tunsigned char head_sha1[20];\n+\n+\tif (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n+\t !file_exists(git_path(\"REVERT_HEAD\")))\n+\t\treturn error(_(\"no cherry-pick or revert in progress\"));\n+\tif (!resolve_ref(\"HEAD\", head_sha1, 0, NULL))\n+\t\treturn error(_(\"cannot resolve HEAD\"));\n+\tif (is_null_sha1(head_sha1))\n+\t\treturn error(_(\"cannot abort from a branch yet to be born\"));\n+\treturn reset_for_rollback(head_sha1);\n+}\n+\n+static int sequencer_rollback(struct replay_opts *opts)\n+{\n+\tconst char *filename;\n+\tFILE *f;\n+\tunsigned char sha1[20];\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tfilename = git_path(SEQ_HEAD_FILE);\n+\tf = fopen(filename, \"r\");\n+\tif (!f && errno == ENOENT) {\n+\t\t/*\n+\t\t * There is no multiple-cherry-pick in progress.\n+\t\t * If CHERRY_PICK_HEAD or REVERT_HEAD indicates\n+\t\t * a single-cherry-pick in progress, abort that.\n+\t\t */\n+\t\treturn rollback_single_pick();\n+\t}\n+\tif (!f)\n+\t\treturn error(_(\"cannot open %s: %s\"), filename,\n+\t\t\t\t\t\tstrerror(errno));\n+\tif (strbuf_getline(&buf, f, '\\n')) {\n+\t\terror(_(\"cannot read %s: %s\"), filename, ferror(f) ?\n+\t\t\tstrerror(errno) : _(\"unexpected end of file\"));\n+\t\tgoto fail;\n+\t}\n+\tif (get_sha1_hex(buf.buf, sha1) || buf.buf[40] != '\\0') {\n+\t\terror(_(\"stored pre-cherry-pick HEAD file '%s' is corrupt\"),\n+\t\t\tfilename);\n+\t\tgoto fail;\n+\t}\n+\tif (reset_for_rollback(sha1))\n+\t\tgoto fail;\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\treturn 0;\n+fail:\n+\tstrbuf_release(&buf);\n+\tfclose(f);\n+\treturn -1;\n+}\n+\n static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n {\n \tconst char *todo_file = git_path(SEQ_TODO_FILE);\n@@ -977,6 +1056,8 @@ static int pick_revisions(struct replay_opts *opts)\n \t\tremove_sequencer_state(1);\n \t\treturn 0;\n \t}\n+\tif (opts->subcommand == REPLAY_ROLLBACK)\n+\t\treturn sequencer_rollback(opts);\n \tif (opts->subcommand == REPLAY_CONTINUE) {\n \t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n \t\t\treturn error(_(\"No %s in progress\"), action_name(opts));\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex bb67cdcb..e97397f9 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test cherry-pick continuation features\n \n+ + yetanotherpick: rewrites foo to e\n + anotherpick: rewrites foo to d\n + picked: rewrites foo to c\n + unrelatedpick: rewrites unrelated to reallyunrelated\n@@ -19,6 +20,12 @@ pristine_detach () {\n \tgit clean -d -f -f -q -x\n }\n \n+test_cmp_rev () {\n+\tgit rev-parse --verify \"$1\" >expect.rev &&\n+\tgit rev-parse --verify \"$2\" >actual.rev &&\n+\ttest_cmp expect.rev actual.rev\n+}\n+\n test_expect_success setup '\n \techo unrelated >unrelated &&\n \tgit add unrelated &&\n@@ -27,6 +34,7 @@ test_expect_success setup '\n \ttest_commit unrelatedpick unrelated reallyunrelated &&\n \ttest_commit picked foo c &&\n \ttest_commit anotherpick foo d &&\n+\ttest_commit yetanotherpick foo e &&\n \tgit config advice.detachedhead false\n \n '\n@@ -75,6 +83,11 @@ test_expect_success '--quit does not complain when no cherry-pick is in progress\n \tgit cherry-pick --quit\n '\n \n+test_expect_success '--abort requires cherry-pick in progress' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick --abort\n+'\n+\n test_expect_success '--quit cleans up sequencer state' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n@@ -103,6 +116,79 @@ test_expect_success 'cherry-pick --reset (another name for --quit)' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--abort to cancel multiple cherry-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success '--abort to cancel single cherry-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success 'cherry-pick --abort to cancel multiple revert' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert base..picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success 'revert --abort works, too' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert base..picked &&\n+\tgit revert --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD\n+'\n+\n+test_expect_success '--abort to cancel single revert' '\n+\tpristine_detach anotherpick &&\n+\ttest_must_fail git revert picked &&\n+\tgit revert --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev anotherpick HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n+test_expect_success '--abort keeps unrelated change, easy case' '\n+\tpristine_detach unrelatedpick &&\n+\techo changed >expect &&\n+\ttest_must_fail git cherry-pick picked..yetanotherpick &&\n+\techo changed >unrelated &&\n+\tgit cherry-pick --abort &&\n+\ttest_cmp expect unrelated\n+'\n+\n+test_expect_success '--abort refuses to clobber unrelated change, harder case' '\n+\tpristine_detach initial &&\n+\techo changed >expect &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo changed >unrelated &&\n+\ttest_must_fail git cherry-pick --abort &&\n+\ttest_cmp expect unrelated &&\n+\tgit rev-list HEAD >log &&\n+\ttest_line_count = 2 log &&\n+\ttest_must_fail git update-index --refresh &&\n+\n+\tgit checkout unrelated &&\n+\tgit cherry-pick --abort &&\n+\ttest_cmp_rev initial HEAD\n+'\n+\n test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n@@ -127,6 +213,16 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le\n \ttest_cmp expect actual\n '\n \n+test_expect_failure '--abort after last commit in sequence' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..picked &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_cmp_rev initial HEAD &&\n+\tgit update-index --refresh &&\n+\tgit diff-index --exit-code HEAD\n+'\n+\n test_expect_success 'cherry-pick does not implicitly stomp an existing operation' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..anotherpick &&\n-- \n1.7.8.rc3\n"},{"id":"179866","messageId":"4ECCB3A2.5030102@viscovery.net","threadId":"28989","inReplyTo":"20111123012721.GA14217@elie.hsd1.il.comcast.net","subject":"[PATCH] Fix revert --abort on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-11-23T08:49:38Z","receivedAt":"2011-11-23T08:49:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nOn Windows, it is not possible to rename or remove a directory that has\nopen files. 'revert --abort' renamed .git/sequencer when it still had\n.git/sequencer/head open. Close the file as early as possible to allow\nthe rename operation on Windows.\n\nSigned-off-by: Johannes Sixt \n---\n I guess it's too late to squash this in. ;)\n\n -- Hannes\n\n builtin/revert.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 0c61668..5dedb51 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -931,8 +931,10 @@ static int sequencer_rollback(struct replay_opts *opts)\n \tif (strbuf_getline(&buf, f, '\\n')) {\n \t\terror(_(\"cannot read %s: %s\"), filename, ferror(f) ?\n \t\t\tstrerror(errno) : _(\"unexpected end of file\"));\n+\t\tfclose(f);\n \t\tgoto fail;\n \t}\n+\tfclose(f);\n \tif (get_sha1_hex(buf.buf, sha1) || buf.buf[40] != '\\0') {\n \t\terror(_(\"stored pre-cherry-pick HEAD file '%s' is corrupt\"),\n \t\t\tfilename);\n@@ -941,11 +943,9 @@ static int sequencer_rollback(struct replay_opts *opts)\n \tif (reset_for_rollback(sha1))\n \t\tgoto fail;\n \tstrbuf_release(&buf);\n-\tfclose(f);\n \treturn 0;\n fail:\n \tstrbuf_release(&buf);\n-\tfclose(f);\n \treturn -1;\n }\n \n-- \n1.7.8.rc3.1164.gba869.dirty\n"},{"id":"179872","messageId":"20111123100452.GA30629@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"4ECCB3A2.5030102@viscovery.net","subject":"Re: [PATCH] Fix revert --abort on Windows","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-11-23T10:04:52Z","receivedAt":"2011-11-23T10:04:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Johannes Sixt wrote:\n\n> From: Johannes Sixt \n>\n> On Windows, it is not possible to rename or remove a directory that has\n> open files. 'revert --abort' renamed .git/sequencer when it still had\n> .git/sequencer/head open. Close the file as early as possible to allow\n> the rename operation on Windows.\n\nNice catch.\n\nAcked-by: Jonathan Nieder \n\nSpeaking of which, it doesn't make a lot of sense for \"git revert\n--abort\" to be leaving a .git/sequencer-old directory around. How\nabout this on top?\n\n-- >8 --\nSubject: revert --abort: do not leave behind useless sequencer-old directory\n\nThe \"git cherry-pick --abort\" command currently renames the\n.git/sequencer directory to .git/sequencer-old instead of removing it\non success due to an accident. cherry-pick --abort is designed to\nwork in three steps:\n\n 1) find which commit to roll back to\n 2) call \"git reset --merge \" to move to that commit\n 3) remove the .git/sequencer directory\n\nBut the careless author forgot step 3 entirely. The only reason the\ncommand worked anyway is that \"git reset --merge \" renames the\n.git/sequencer directory as a secondary effect --- after moving to\n, or so the logic goes, it is unlikely but possible that the\ncaller of git reset wants to continue the series of cherry-picks that\nwas in progress, so git renames the sequencer state to\n.git/sequencer-old to be helpful while allowing the cherry-pick to be\nresumed if the caller did not want to end the sequence after all.\n\nBy running \"git cherry-pick --abort\", the operator has clearly\nindicated that she is not planning to continue cherry-picking. Remove\nthe (renamed) .git/sequencer directory as intended all along.\n\nSigned-off-by: Jonathan Nieder \n---\nBy the way, as the length of the second-to-last paragraph above might\nhave hinted, I am not convinced that allowing \"git reset --hard\" as an\nescape route from a cherry-pick sequence was very sensible. It\n_would_ be nice to have a command to return to a known state,\ndiscarding progress in all pending multiple-command guided workflows\n(am, rebase, bisect), but git reset is not that command.\n\n builtin/revert.c | 1 +\n t/t7106-reset-sequence.sh | 8 ++++++++\n 2 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 5dedb51c..818b4abb 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -942,6 +942,7 @@ static int sequencer_rollback(struct replay_opts *opts)\n \t}\n \tif (reset_for_rollback(sha1))\n \t\tgoto fail;\n+\tremove_sequencer_state(1);\n \tstrbuf_release(&buf);\n \treturn 0;\n fail:\ndiff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh\nindex 3f86e8c5..83f7ea59 100755\n--- a/t/t7106-reset-sequence.sh\n+++ b/t/t7106-reset-sequence.sh\n@@ -41,4 +41,12 @@ test_expect_success 'reset --hard cleans up sequencer state, providing one-level\n \ttest_path_is_missing .git/sequencer-old\n '\n \n+test_expect_success 'cherry-pick --abort does not leave sequencer-old dir' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\tgit cherry-pick --abort &&\n+\ttest_path_is_missing .git/sequencer &&\n+\ttest_path_is_missing .git/sequencer-old\n+'\n+\n test_done\n-- \n1.7.8.rc3\n"},{"id":"179873","messageId":"4ECCC935.7010407@viscovery.net","threadId":"28989","inReplyTo":"20111123100452.GA30629@elie.hsd1.il.comcast.net","subject":"Re: [PATCH] Fix revert --abort on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2011-11-23T10:21:41Z","receivedAt":"2011-11-23T10:21:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11/23/2011 11:04, schrieb Jonathan Nieder:\n> ... \"git reset --merge \" renames the\n> .git/sequencer directory as a secondary effect --- after moving to\n> , or so the logic goes, it is unlikely but possible that the\n> caller of git reset wants to continue the series of cherry-picks that\n> was in progress, so git renames the sequencer state to\n> .git/sequencer-old to be helpful while allowing the cherry-pick to be\n> resumed if the caller did not want to end the sequence after all.\n> ...\n> By the way, as the length of [this paragraph] might\n> have hinted, I am not convinced that allowing \"git reset --hard\" as an\n> escape route from a cherry-pick sequence was very sensible. It\n> _would_ be nice to have a command to return to a known state,\n> discarding progress in all pending multiple-command guided workflows\n> (am, rebase, bisect), but git reset is not that command.\n\nIMO, it doesn't make sense that git-reset aborts a cherry-pick sequence:\nWhen I messed up a difficult conflict in the middle of a cherry-pick\nsequence, it might be useful to be able to 'git reset --hard && git\ncherry-pick that-one-commit' to restart the conflict resolution.\n\n(But does a single-commit cherry-pick during a multi-commit cherry-pick\nwork to begin with?)\n\n-- Hannes\n"},{"id":"179894","messageId":"CALxABCZ=phbFwUPNX+z8CeLGSsr03kULcxX3E7zepZDOGyb2jA@mail.gmail.com","threadId":"28989","inReplyTo":"4ECCB3A2.5030102@viscovery.net","subject":"Re: [PATCH] Fix revert --abort on Windows","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2011-11-23T17:23:08Z","receivedAt":"2011-11-23T17:23:08Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Wed, Nov 23, 2011 at 09:49, Johannes Sixt wrote:\n> From: Johannes Sixt \n>\n> On Windows, it is not possible to rename or remove a directory that has\n> open files. 'revert --abort' renamed .git/sequencer when it still had\n> .git/sequencer/head open. Close the file as early as possible to allow\n> the rename operation on Windows.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n>  I guess it's too late to squash this in. ;)\n>\n\nJust made a patch for this of my own (noticed it on Cygwin), should have\nlooked in the archives first. Thanks!\n\nThe minority platforms can wait, I guess :)\n"},{"id":"180171","messageId":"7vty5lp6jc.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111122112001.GF7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 5/3] revert: introduce --abort to cancel a failed cherry-pick","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-30T22:52:23Z","receivedAt":"2011-11-30T22:52:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> After running some ill-advised command like \"git cherry-pick\n> HEAD..linux-next\", the bewildered novice may want to return to more\n> familiar territory. Introduce a \"git cherry-pick --abort\" command\n> that rolls back the entire cherry-pick sequence and places the\n> repository back on solid ground.\n\nThis is confusing; if you have many commits in the range, and a handful of\nthem replayed without conflicts and then you hit a conflict, where should\n(I am not asking \"where does ... with your patch\") abort take us? The\nstate after the random commit that happened to have replayed successfully?\nThe state before the entire cherry-pick sequence started? \"back on solid\nground\" does not tell us which one you meant.\n\nI am assuming that it is the latter.\n\n> diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt\n> index 75f8e869..5747f442 100644\n> --- a/Documentation/sequencer.txt\n> +++ b/Documentation/sequencer.txt\n> @@ -7,3 +7,6 @@\n> \tForget about the current operation in progress. Can be used\n> \tto clear the sequencer state after a failed cherry-pick or\n> \trevert.\n> +\n> +--abort::\n> +\tCancel the operation and return to the pre-sequence state.\n\nOk, it is the latter.\n\n> +static int reset_for_rollback(const unsigned char *sha1)\n> +{\n> +\tconst char *argv[4];\t/* reset --merge + NULL */\n> +\targv[0] = \"reset\";\n> +\targv[1] = \"--merge\";\n> +\targv[2] = sha1_to_hex(sha1);\n> +\targv[3] = NULL;\n> +\treturn run_command_v_opt(argv, RUN_GIT_CMD);\n> +}\n\nSo you give the value of the HEAD before the sequence started to this\nfunction and all should go well. Where do you read that value from?\n\n> +static int rollback_single_pick(void)\n> +{\n> +\tunsigned char head_sha1[20];\n> +\n> +\tif (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n> +\t !file_exists(git_path(\"REVERT_HEAD\")))\n> +\t\treturn error(_(\"no cherry-pick or revert in progress\"));\n> +\tif (!resolve_ref(\"HEAD\", head_sha1, 0, NULL))\n> +\t\treturn error(_(\"cannot resolve HEAD\"));\n> +\tif (is_null_sha1(head_sha1))\n> +\t\treturn error(_(\"cannot abort from a branch yet to be born\"));\n\nOk, this is for single-pick so HEAD is where we came from. Good.\n\n> +\treturn reset_for_rollback(head_sha1);\n> +}\n> +\n> +static int sequencer_rollback(struct replay_opts *opts)\n> +{\n> +\tconst char *filename;\n> +\tFILE *f;\n> +\tunsigned char sha1[20];\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\n> +\tfilename = git_path(SEQ_HEAD_FILE);\n> +\tf = fopen(filename, \"r\");\n> +\tif (!f && errno == ENOENT) {\n> +\t\t/*\n> +\t\t * There is no multiple-cherry-pick in progress.\n> +\t\t * If CHERRY_PICK_HEAD or REVERT_HEAD indicates\n> +\t\t * a single-cherry-pick in progress, abort that.\n> +\t\t */\n> +\t\treturn rollback_single_pick();\n> +\t}\n> +\tif (!f)\n> +\t\treturn error(_(\"cannot open %s: %s\"), filename,\n> +\t\t\t\t\t\tstrerror(errno));\n> +\tif (strbuf_getline(&buf, f, '\\n')) {\n> +\t\terror(_(\"cannot read %s: %s\"), filename, ferror(f) ?\n> +\t\t\tstrerror(errno) : _(\"unexpected end of file\"));\n> +\t\tgoto fail;\n> +\t}\n\nAnd when we are in multi-pick, SEQ_HEAD_FILE has it.\n\nLooks good from a cursory review. Thanks.\n"},{"id":"180196","messageId":"CALkWK0mp4-nxLsf=wjDwHOhvoWFNzNz-Rc+Q+S_Er5rpFW2k=Q@mail.gmail.com","threadId":"28989","inReplyTo":"20111122111736.GE7399@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 4/3] revert: write REVERT_HEAD pseudoref during conflicted revert","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-01T09:34:43Z","receivedAt":"2011-12-01T09:34:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> [...]\n> Set a REVERT_HEAD pseudoref when \"git revert\" does not make a commit,\n> for cases like this.  This also makes it possible for scripts to\n> distinguish between a revert that encountered conflicts and other\n> sources of an unmerged index.\n\nSounds a lot like CHERRY_PICK_HEAD counterpart. Let's read ahead and\nsee if there are any unexpected differences.\n\n> diff --git a/branch.c b/branch.c\n> index d8098762..025a97be 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -241,6 +241,7 @@ void create_branch(const char *head,\n>  void remove_branch_state(void)\n>  {\n>        unlink(git_path(\"CHERRY_PICK_HEAD\"));\n> +       unlink(git_path(\"REVERT_HEAD\"));\n>        unlink(git_path(\"MERGE_HEAD\"));\n>        unlink(git_path(\"MERGE_RR\"));\n>        unlink(git_path(\"MERGE_MSG\"));\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index c46f2d18..8f2bebec 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1514,6 +1514,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>        }\n>\n>        unlink(git_path(\"CHERRY_PICK_HEAD\"));\n> +       unlink(git_path(\"REVERT_HEAD\"));\n>        unlink(git_path(\"MERGE_HEAD\"));\n>        unlink(git_path(\"MERGE_MSG\"));\n>        unlink(git_path(\"MERGE_MODE\"));\n\nSo far so good. Behaves exactly like CHERRY_PICK_HEAD.\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 1d112e4c..f5ba67a5 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -289,7 +289,7 @@ static char *get_encoding(const char *message)\n>        return NULL;\n>  }\n>\n> -static void write_cherry_pick_head(struct commit *commit)\n> +static void write_cherry_pick_head(struct commit *commit, const char *pseudoref)\n>  {\n>        const char *filename;\n>        int fd;\n> @@ -297,7 +297,7 @@ static void write_cherry_pick_head(struct commit *commit)\n>\n>        strbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n>\n> -       filename = git_path(\"CHERRY_PICK_HEAD\");\n> +       filename = git_path(pseudoref);\n>        fd = open(filename, O_WRONLY | O_CREAT, 0666);\n>        if (fd < 0)\n>                die_errno(_(\"Could not open '%s' for writing\"), filename);\n> @@ -597,7 +597,9 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>         * write it at all.\n>         */\n>        if (opts->action == CHERRY_PICK && !opts->no_commit && (res == 0 || res == 1))\n> -               write_cherry_pick_head(commit);\n> +               write_cherry_pick_head(commit, \"CHERRY_PICK_HEAD\");\n> +       if (opts->action == REVERT && ((opts->no_commit && res == 0) || res == 1))\n> +               write_cherry_pick_head(commit, \"REVERT_HEAD\");\n>\n>        if (res) {\n>                error(opts->action == REVERT\n\nInteresting. This additional symmetry will probably make life much\neasier for my pending \"New Sequencer Workflow! v2\" series.\n\n> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> index cb45574a..ee1659c1 100755\n> --- a/t/t3507-cherry-pick-conflict.sh\n> +++ b/t/t3507-cherry-pick-conflict.sh\n> [...]\n\nAnd some tests. Nice.\n\nThanks.\n\n-- Ram\n"},{"id":"180785","messageId":"20111210124644.GA22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"4ECCC935.7010407@viscovery.net","subject":"[RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T12:46:44Z","receivedAt":"2011-12-10T12:46:44Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(-cc: Phil)\nJohannes Sixt wrote:\n\n> IMO, it doesn't make sense that git-reset aborts a cherry-pick sequence:\n> When I messed up a difficult conflict in the middle of a cherry-pick\n> sequence, it might be useful to be able to 'git reset --hard && git\n> cherry-pick that-one-commit' to restart the conflict resolution.\n>\n> (But does a single-commit cherry-pick during a multi-commit cherry-pick\n> work to begin with?)\n\nIt should, I think.\n\nHere are patches to address some UI warts (such as that one) in\ncurrent cherry-pick code.\n\nPatch 1 cleans up to prepare for patch 2, which in turn makes \"git\ncherry-pick --continue\" act like \"git rebase --continue\" by commiting\na conflict resolution if it has not already been committed. This\nbrings us closer to a less confusing world in which all commands that\ncan exit and ask the user to help to get closer to their goal provide\na --continue option as a standard interface to resume (I guess \"git\nmerge --continue\" is all that's left to do afterwards).\n\nPatch 3 is from Ram's rr/revert-cherry-pick series. It doesn't have\nmuch to do with this series, but I'd rather work on a codebase with\nthis particular patch applied, so I applied it before working on\npatch 4.\n\nPatch 4 uses Junio's cmdline_info API to distinguish single-picks\nfrom multi-picks. This is the title feature. For a while I thought\nsomething like this was the only sane thing to do, but laziness won\nout.\n\nPatches 5-7 remove hacks that patch 4 makes superfluous.\n\nPatch 7 has the downside that if anyone had a .git/sequencer-old\ndirectory lying around, then git will not care after applying the\npatch and it will sit just taking up its few bytes and distracting\npeople that run \"ls .git\". I'm not sure whether that's worth fixing,\nand if so how.\n\nAnyway, I hope you enjoy the series. Thoughts and bug reports\nappreciated as usual.\n\nJonathan Nieder (7):\n revert: give --continue handling its own function\n revert: allow cherry-pick --continue to commit before resuming\n revert: pass around rev-list args in already-parsed form\n revert: allow single-pick in the middle of cherry-pick sequence\n revert: do not remove state until sequence is finished\n Revert \"reset: Make reset remove the sequencer state\"\n revert: stop creating or removing sequencer-old directory\n\n branch.c | 2 -\n builtin/revert.c | 138 ++++++++++++++++++++++-----------\n sequencer.c | 10 +--\n sequencer.h | 12 +---\n t/t3510-cherry-pick-sequence.sh | 162 +++++++++++++++++++++++++++++++++++++--\n t/t7106-reset-sequence.sh | 52 -------------\n 6 files changed, 251 insertions(+), 125 deletions(-)\n delete mode 100755 t/t7106-reset-sequence.sh\n"},{"id":"180786","messageId":"20111210124736.GB22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 1/7] revert: give --continue handling its own function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T12:47:36Z","receivedAt":"2011-12-10T12:47:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This makes pick_revisions() a little shorter and easier to read\nstraight through.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/revert.c | 28 +++++++++++++++++-----------\n 1 files changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 1ea525c1..9f6c85c1 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -1038,6 +1038,21 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \treturn 0;\n }\n \n+static int sequencer_continue(struct replay_opts *opts)\n+{\n+\tstruct commit_list *todo_list = NULL;\n+\n+\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n+\t\treturn error(_(\"No %s in progress\"), action_name(opts));\n+\tread_populate_opts(&opts);\n+\tread_populate_todo(&todo_list, opts);\n+\n+\t/* Verify that the conflict has been resolved */\n+\tif (!index_differs_from(\"HEAD\", 0))\n+\t\ttodo_list = todo_list->next;\n+\treturn pick_commits(todo_list, opts);\n+}\n+\n static int pick_revisions(struct replay_opts *opts)\n {\n \tstruct commit_list *todo_list = NULL;\n@@ -1056,17 +1071,8 @@ static int pick_revisions(struct replay_opts *opts)\n \t}\n \tif (opts->subcommand == REPLAY_ROLLBACK)\n \t\treturn sequencer_rollback(opts);\n-\tif (opts->subcommand == REPLAY_CONTINUE) {\n-\t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n-\t\t\treturn error(_(\"No %s in progress\"), action_name(opts));\n-\t\tread_populate_opts(&opts);\n-\t\tread_populate_todo(&todo_list, opts);\n-\n-\t\t/* Verify that the conflict has been resolved */\n-\t\tif (!index_differs_from(\"HEAD\", 0))\n-\t\t\ttodo_list = todo_list->next;\n-\t\treturn pick_commits(todo_list, opts);\n-\t}\n+\tif (opts->subcommand == REPLAY_CONTINUE)\n+\t\treturn sequencer_continue(opts);\n \n \t/*\n \t * Start a new cherry-pick/ revert sequence; but\n-- \n1.7.8.rc3\n"},{"id":"180787","messageId":"20111210124925.GC22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 2/7] revert: allow cherry-pick --continue to commit before resuming","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T12:49:25Z","receivedAt":"2011-12-10T12:49:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When \"git cherry-pick ..bar\" encounters conflicts, permit the operator\nto use cherry-pick --continue after resolving them as a shortcut for\n\"git commit && git cherry-pick --continue\" to record the resolution\nand carry on with the rest of the sequence.\n\nThis improves the analogy with \"git rebase\" (in olden days --continue\nwas the way to preserve authorship when a rebase encountered\nconflicts) and fits well with a general UI goal of making \"git cmd\n--continue\" save humans the trouble of deciding what to do next.\n\nExample: after encountering a conflict from running \"git cherry-pick\nfoo bar baz\":\n\n\tCONFLICT (content): Merge conflict in main.c\n\terror: could not apply f78a8d98c... bar!\n\thint: after resolving the conflicts, mark the corrected paths\n\thint: with 'git add ' or 'git rm '\n\thint: and commit the result with 'git commit'\n\nWe edit main.c to resolve the conflict, mark it acceptable with \"git\nadd main.c\", and can run \"cherry-pick --continue\" to resume the\nsequence.\n\n\t$ git cherry-pick --continue\n\t[editor opens to confirm commit message]\n\t[master 78c8a8c98] bar!\n\t 1 files changed, 1 insertions(+), 1 deletions(-)\n\t[master 87ca8798c] baz!\n\t 1 files changed, 1 insertions(+), 1 deletions(-)\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/revert.c | 23 ++++++-\n t/t3510-cherry-pick-sequence.sh | 139 ++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 156 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 9f6c85c1..a43b4d85 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -1038,18 +1038,35 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \treturn 0;\n }\n \n+static int continue_single_pick(void)\n+{\n+\tconst char *argv[] = { \"commit\", NULL };\n+\n+\tif (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n+\t !file_exists(git_path(\"REVERT_HEAD\")))\n+\t\treturn error(_(\"no cherry-pick or revert in progress\"));\n+\treturn run_command_v_opt(argv, RUN_GIT_CMD);\n+}\n+\n static int sequencer_continue(struct replay_opts *opts)\n {\n \tstruct commit_list *todo_list = NULL;\n \n \tif (!file_exists(git_path(SEQ_TODO_FILE)))\n-\t\treturn error(_(\"No %s in progress\"), action_name(opts));\n+\t\treturn continue_single_pick();\n \tread_populate_opts(&opts);\n \tread_populate_todo(&todo_list, opts);\n \n \t/* Verify that the conflict has been resolved */\n-\tif (!index_differs_from(\"HEAD\", 0))\n-\t\ttodo_list = todo_list->next;\n+\tif (file_exists(git_path(\"CHERRY_PICK_HEAD\")) ||\n+\t file_exists(git_path(\"REVERT_HEAD\"))) {\n+\t\tint ret = continue_single_pick();\n+\t\tif (ret)\n+\t\t\treturn ret;\n+\t}\n+\tif (index_differs_from(\"HEAD\", 0))\n+\t\treturn error_dirty_index(opts);\n+\ttodo_list = todo_list->next;\n \treturn pick_commits(todo_list, opts);\n }\n \ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 2c4c1c85..4d1883b7 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -2,6 +2,7 @@\n \n test_description='Test cherry-pick continuation features\n \n+ + conflicting: rewrites unrelated to conflicting\n + yetanotherpick: rewrites foo to e\n + anotherpick: rewrites foo to d\n + picked: rewrites foo to c\n@@ -27,6 +28,7 @@ test_cmp_rev () {\n }\n \n test_expect_success setup '\n+\tgit config advice.detachedhead false\n \techo unrelated >unrelated &&\n \tgit add unrelated &&\n \ttest_commit initial foo a &&\n@@ -35,8 +37,8 @@ test_expect_success setup '\n \ttest_commit picked foo c &&\n \ttest_commit anotherpick foo d &&\n \ttest_commit yetanotherpick foo e &&\n-\tgit config advice.detachedhead false\n-\n+\tpristine_detach initial &&\n+\ttest_commit conflicting unrelated\n '\n \n test_expect_success 'cherry-pick persists data on failure' '\n@@ -243,7 +245,66 @@ test_expect_success '--continue complains when there are unresolved conflicts' '\n \ttest_must_fail git cherry-pick --continue\n '\n \n-test_expect_success '--continue continues after conflicts are resolved' '\n+test_expect_success '--continue of single cherry-pick' '\n+\tpristine_detach initial &&\n+\techo c >expect &&\n+\ttest_must_fail git cherry-pick picked &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\n+\ttest_cmp expect foo &&\n+\ttest_cmp_rev initial HEAD^ &&\n+\tgit diff --exit-code HEAD &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n+'\n+\n+test_expect_success '--continue of single revert' '\n+\tpristine_detach initial &&\n+\techo resolved >expect &&\n+\techo \"Revert \\\"picked\\\"\" >expect.msg &&\n+\ttest_must_fail git revert picked &&\n+\techo resolved >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\n+\tgit diff --exit-code HEAD &&\n+\ttest_cmp expect foo &&\n+\ttest_cmp_rev initial HEAD^ &&\n+\tgit diff-tree -s --pretty=tformat:%s HEAD >msg &&\n+\ttest_cmp expect.msg msg &&\n+\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n+\ttest_must_fail git rev-parse --verify REVERT_HEAD\n+'\n+\n+test_expect_success '--continue after resolving conflicts' '\n+\tpristine_detach initial &&\n+\techo d >expect &&\n+\tcat >expect.log <<-\\EOF &&\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tfoo\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tfoo\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tunrelated\n+\tOBJID\n+\t:000000 100644 OBJID OBJID A\tfoo\n+\t:000000 100644 OBJID OBJID A\tunrelated\n+\tEOF\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\t{\n+\t\tgit rev-list HEAD |\n+\t\tgit diff-tree --root --stdin |\n+\t\tsed \"s/$_x40/OBJID/g\"\n+\t} >actual.log &&\n+\ttest_cmp expect foo &&\n+\ttest_cmp expect.log actual.log\n+'\n+\n+test_expect_success '--continue after resolving conflicts and committing' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..anotherpick &&\n \techo \"c\" >foo &&\n@@ -270,6 +331,29 @@ test_expect_success '--continue continues after conflicts are resolved' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--continue asks for help after resolving patch to nil' '\n+\tpristine_detach conflicting &&\n+\ttest_must_fail git cherry-pick initial..picked &&\n+\n+\ttest_cmp_rev unrelatedpick CHERRY_PICK_HEAD &&\n+\tgit checkout HEAD -- unrelated &&\n+\ttest_must_fail git cherry-pick --continue 2>msg &&\n+\ttest_i18ngrep \"The previous cherry-pick is now empty\" msg\n+'\n+\n+test_expect_failure 'follow advice and skip nil patch' '\n+\tpristine_detach conflicting &&\n+\ttest_must_fail git cherry-pick initial..picked &&\n+\n+\tgit checkout HEAD -- unrelated &&\n+\ttest_must_fail git cherry-pick --continue &&\n+\tgit reset &&\n+\tgit cherry-pick --continue &&\n+\n+\tgit rev-list initial..HEAD >commits &&\n+\ttest_line_count = 3 commits\n+'\n+\n test_expect_success '--continue respects opts' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -x base..anotherpick &&\n@@ -288,6 +372,29 @@ test_expect_success '--continue respects opts' '\n \tgrep \"cherry picked from\" anotherpick_msg\n '\n \n+test_expect_success '--continue of single-pick respects -x' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -x picked &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\ttest_path_is_missing .git/sequencer &&\n+\tgit cat-file commit HEAD >msg &&\n+\tgrep \"cherry picked from\" msg\n+'\n+\n+test_expect_success '--continue respects -x in first commit in multi-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -x picked anotherpick &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\ttest_path_is_missing .git/sequencer &&\n+\tgit cat-file commit HEAD^ >msg &&\n+\tpicked=$(git rev-parse --verify picked) &&\n+\tgrep \"cherry picked from.*$picked\" msg\n+'\n+\n test_expect_success '--signoff is not automatically propagated to resolved conflict' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick --signoff base..anotherpick &&\n@@ -306,6 +413,32 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \n+test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked anotherpick &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\n+\tgit diff --exit-code HEAD &&\n+\ttest_cmp_rev initial HEAD^^ &&\n+\tgit cat-file commit HEAD^ >msg &&\n+\t! grep Signed-off-by: msg\n+'\n+\n+test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked &&\n+\techo c >foo &&\n+\tgit add foo &&\n+\tgit cherry-pick --continue &&\n+\n+\tgit diff --exit-code HEAD &&\n+\ttest_cmp_rev initial HEAD^ &&\n+\tgit cat-file commit HEAD >msg &&\n+\t! grep Signed-off-by: msg\n+'\n+\n test_expect_success 'malformed instruction sheet 1' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..anotherpick &&\n-- \n1.7.8.rc3\n"},{"id":"180791","messageId":"20111210125825.GD22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 3/7] revert: pass around rev-list args in already-parsed form","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T12:58:25Z","receivedAt":"2011-12-10T12:58:25Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Date: Sat, 13 Aug 2011 12:06:23 -0500\n\nSince 7e2bfd3f (revert: allow cherry-picking more than one commit,\n2010-07-02), the pick/revert machinery has kept track of the set of\ncommits to be cherry-picked or reverted using commit_argc and\ncommit_argv variables, storing the corresponding command-line\nparameters.\n\nFuture callers as other commands are built in (am, rebase, sequencer)\nmay find it easier to pass rev-list options to this machinery in\nalready-parsed form. Teach cmd_cherry_pick and cmd_revert to parse\nthe rev-list arguments in advance and pass the commit set to\npick_revisions() as a rev_info structure.\n\nOriginal patch by Jonathan, tweaks and test from Ram.\n\nSigned-off-by: Jonathan Nieder \nImproved-by: Ramkumar Ramachandra \n---\n builtin/revert.c | 53 +++++++++++++++++++++-----------------\n t/t3510-cherry-pick-sequence.sh | 5 +++\n 2 files changed, 34 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex a43b4d85..71570357 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -60,13 +60,14 @@ struct replay_opts {\n \tint allow_rerere_auto;\n \n \tint mainline;\n-\tint commit_argc;\n-\tconst char **commit_argv;\n \n \t/* Merge strategy */\n \tconst char *strategy;\n \tconst char **xopts;\n \tsize_t xopts_nr, xopts_alloc;\n+\n+\t/* Only used by REPLAY_NONE */\n+\tstruct rev_info *revs;\n };\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n@@ -169,9 +170,9 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tdie(_(\"program error\"));\n \t}\n \n-\topts->commit_argc = parse_options(argc, argv, NULL, options, usage_str,\n-\t\t\t\t\tPARSE_OPT_KEEP_ARGV0 |\n-\t\t\t\t\tPARSE_OPT_KEEP_UNKNOWN);\n+\targc = parse_options(argc, argv, NULL, options, usage_str,\n+\t\t\tPARSE_OPT_KEEP_ARGV0 |\n+\t\t\tPARSE_OPT_KEEP_UNKNOWN);\n \n \t/* Check for incompatible subcommands */\n \tverify_opt_mutually_compatible(me,\n@@ -213,9 +214,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\t\tNULL);\n \t}\n \n-\telse if (opts->commit_argc < 2)\n-\t\tusage_with_options(usage_str, options);\n-\n \tif (opts->allow_ff)\n \t\tverify_opt_compatible(me, \"--ff\",\n \t\t\t\t\"--signoff\", opts->signoff,\n@@ -223,7 +221,20 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\t\t\"-x\", opts->record_origin,\n \t\t\t\t\"--edit\", opts->edit,\n \t\t\t\tNULL);\n-\topts->commit_argv = argv;\n+\n+\tif (opts->subcommand != REPLAY_NONE) {\n+\t\topts->revs = NULL;\n+\t} else {\n+\t\topts->revs = xmalloc(sizeof(*opts->revs));\n+\t\tinit_revisions(opts->revs, NULL);\n+\t\topts->revs->no_walk = 1;\n+\t\tif (argc < 2)\n+\t\t\tusage_with_options(usage_str, options);\n+\t\targc = setup_revisions(argc, argv, opts->revs, NULL);\n+\t}\n+\n+\tif (argc > 1)\n+\t\tusage_with_options(usage_str, options);\n }\n \n struct commit_message {\n@@ -631,23 +642,15 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \treturn res;\n }\n \n-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)\n+static void prepare_revs(struct replay_opts *opts)\n {\n-\tint argc;\n-\n-\tinit_revisions(revs, NULL);\n-\trevs->no_walk = 1;\n \tif (opts->action != REVERT)\n-\t\trevs->reverse = 1;\n+\t\topts->revs->reverse ^= 1;\n \n-\targc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);\n-\tif (argc > 1)\n-\t\tusage(*revert_or_cherry_pick_usage(opts));\n-\n-\tif (prepare_revision_walk(revs))\n+\tif (prepare_revision_walk(opts->revs))\n \t\tdie(_(\"revision walk setup failed\"));\n \n-\tif (!revs->commits)\n+\tif (!opts->revs->commits)\n \t\tdie(_(\"empty commit set passed\"));\n }\n \n@@ -844,14 +847,13 @@ static void read_populate_opts(struct replay_opts **opts_ptr)\n static void walk_revs_populate_todo(struct commit_list **todo_list,\n \t\t\t\tstruct replay_opts *opts)\n {\n-\tstruct rev_info revs;\n \tstruct commit *commit;\n \tstruct commit_list **next;\n \n-\tprepare_revs(&revs, opts);\n+\tprepare_revs(opts);\n \n \tnext = todo_list;\n-\twhile ((commit = get_revision(&revs)))\n+\twhile ((commit = get_revision(opts->revs)))\n \t\tnext = commit_list_append(commit, next);\n }\n \n@@ -1075,6 +1077,9 @@ static int pick_revisions(struct replay_opts *opts)\n \tstruct commit_list *todo_list = NULL;\n \tunsigned char sha1[20];\n \n+\tif (opts->subcommand == REPLAY_NONE)\n+\t\tassert(opts->revs);\n+\n \tread_and_refresh_cache(opts);\n \n \t/*\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 4d1883b7..56c95ec1 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -461,4 +461,9 @@ test_expect_success 'malformed instruction sheet 2' '\n \ttest_must_fail git cherry-pick --continue\n '\n \n+test_expect_success 'empty commit set' '\n+\tpristine_detach initial &&\n+\ttest_expect_code 128 git cherry-pick base..base\n+'\n+\n test_done\n-- \n1.7.8.rc3\n"},{"id":"180792","messageId":"20111210125948.GE22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T12:59:48Z","receivedAt":"2011-12-10T12:59:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When I messed up a difficult conflict in the middle of a cherry-pick\nsequence, it can be useful to be able to 'git checkout HEAD . && git\ncherry-pick that-one-commit' to restart the conflict resolution.\n\nSuggested-by: Johannes Sixt \nSigned-off-by: Jonathan Nieder \n---\nMaybe this should come after patch 6/7, so the use case could use\n\"git reset --hard\".\n\n builtin/revert.c | 26 ++++++++++++++++++++++++++\n t/t3510-cherry-pick-sequence.sh | 12 ++++++++++++\n 2 files changed, 38 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 71570357..dcb69904 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -1072,6 +1072,12 @@ static int sequencer_continue(struct replay_opts *opts)\n \treturn pick_commits(todo_list, opts);\n }\n \n+static int single_pick(struct commit *cmit, struct replay_opts *opts)\n+{\n+\tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n+\treturn do_pick_commit(cmit, opts);\n+}\n+\n static int pick_revisions(struct replay_opts *opts)\n {\n \tstruct commit_list *todo_list = NULL;\n@@ -1097,6 +1103,26 @@ static int pick_revisions(struct replay_opts *opts)\n \t\treturn sequencer_continue(opts);\n \n \t/*\n+\t * If we were called as \"git cherry-pick \", just\n+\t * cherry-pick/revert it, set CHERRY_PICK_HEAD /\n+\t * REVERT_HEAD, and don't touch the sequencer state.\n+\t * This means it is possible to cherry-pick in the middle\n+\t * of a cherry-pick sequence.\n+\t */\n+\tif (opts->revs->cmdline.nr == 1 &&\n+\t opts->revs->cmdline.rev->whence == REV_CMD_REV &&\n+\t opts->revs->no_walk &&\n+\t !opts->revs->cmdline.rev->flags) {\n+\t\tstruct commit *cmit;\n+\t\tif (prepare_revision_walk(opts->revs))\n+\t\t\tdie(_(\"revision walk setup failed\"));\n+\t\tcmit = get_revision(opts->revs);\n+\t\tif (!cmit || get_revision(opts->revs))\n+\t\t\tdie(\"BUG: expected exactly one commit from walk\");\n+\t\treturn single_pick(cmit, opts);\n+\t}\n+\n+\t/*\n \t * Start a new cherry-pick/ revert sequence; but\n \t * first, make sure that an existing one isn't in\n \t * progress\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 56c95ec1..98a27a23 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -50,6 +50,18 @@ test_expect_success 'cherry-pick persists data on failure' '\n \ttest_path_is_file .git/sequencer/opts\n '\n \n+test_expect_success 'cherry-pick mid-cherry-pick-sequence' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\ttest_cmp_rev picked CHERRY_PICK_HEAD &&\n+\t# \"oops, I forgot that these patches rely on the change from base\"\n+\tgit checkout HEAD foo &&\n+\tgit cherry-pick base &&\n+\tgit cherry-pick picked &&\n+\tgit cherry-pick --continue &&\n+\tgit diff --exit-code anotherpick\n+'\n+\n test_expect_success 'cherry-pick persists opts correctly' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s -m 1 --strategy=recursive -X patience -X ours base..anotherpick &&\n-- \n1.7.8.rc3\n"},{"id":"180793","messageId":"20111210130212.GF22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 5/7] revert: do not remove state until sequence is finished","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T13:02:12Z","receivedAt":"2011-12-10T13:02:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"As v1.7.8-rc0~141^2~4 (2011-08-04) explains, git cherry-pick removes\nthe sequencer state just before applying the final patch. In the\nsingle-pick case, that was a good thing, since --abort and --continue\nwork fine without access to such state and removing it provides a\nsignal that git should not complain about the need to clobber it (\"a\ncherry-pick or revert is already in progress\") in sequences like the\nfollowing:\n\n\tgit cherry-pick foo\n\tgit read-tree -m -u HEAD; # forget that; let's try a different one\n\tgit cherry-pick bar\n\nAfter the recent patch \"allow single-pick in the middle of cherry-pick\nsequence\" we don't need that hack any more. In the new regime, a\ntraditional \"git cherry-pick \" command never looks at\n.git/sequencer, so we do not need to cripple \"git cherry-pick\n..\" for it any more.\n\nSo now you can run \"git cherry-pick --abort\" near the end of a\nmulti-pick sequence and it will abort the entire sequence, instead of\nmisbehaving and aborting just the final commit.\n\nSigned-off-by: Jonathan Nieder \n---\n builtin/revert.c | 12 +-----------\n t/t3510-cherry-pick-sequence.sh | 6 +++---\n 2 files changed, 4 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex dcb69904..28deb85b 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -1018,18 +1018,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \tfor (cur = todo_list; cur; cur = cur->next) {\n \t\tsave_todo(cur, opts);\n \t\tres = do_pick_commit(cur->item, opts);\n-\t\tif (res) {\n-\t\t\tif (!cur->next)\n-\t\t\t\t/*\n-\t\t\t\t * An error was encountered while\n-\t\t\t\t * picking the last commit; the\n-\t\t\t\t * sequencer state is useless now --\n-\t\t\t\t * the user simply needs to resolve\n-\t\t\t\t * the conflict and commit\n-\t\t\t\t */\n-\t\t\t\tremove_sequencer_state(0);\n+\t\tif (res)\n \t\t\treturn res;\n-\t\t}\n \t}\n \n \t/*\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 98a27a23..851b147f 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -203,10 +203,10 @@ test_expect_success '--abort refuses to clobber unrelated change, harder case' '\n \ttest_cmp_rev initial HEAD\n '\n \n-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '\n+test_expect_success 'cherry-pick still writes sequencer state when one commit is left' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n-\ttest_path_is_missing .git/sequencer &&\n+\ttest_path_is_dir .git/sequencer &&\n \techo \"resolved\" >foo &&\n \tgit add foo &&\n \tgit commit &&\n@@ -227,7 +227,7 @@ test_expect_success 'cherry-pick cleans up sequencer state when one commit is le\n \ttest_cmp expect actual\n '\n \n-test_expect_failure '--abort after last commit in sequence' '\n+test_expect_success '--abort after last commit in sequence' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n \tgit cherry-pick --abort &&\n-- \n1.7.8.rc3\n"},{"id":"180794","messageId":"20111210130348.GG22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 6/7] Revert \"reset: Make reset remove the sequencer state\"","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T13:03:48Z","receivedAt":"2011-12-10T13:03:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This reverts commit 95eb88d8ee588d89b4f06d2753ed4d16ab13b39f, which\nwas a UI experiment that did not reflect how \"git reset\" actually gets\nused. The reversion also fixes a test, indicated in the patch.\n\nEncouraged-by: Johannes Sixt \nSigned-off-by: Jonathan Nieder \n---\n branch.c | 2 -\n t/t3510-cherry-pick-sequence.sh | 2 +-\n t/t7106-reset-sequence.sh | 52 ---------------------------------------\n 3 files changed, 1 insertions(+), 55 deletions(-)\n delete mode 100755 t/t7106-reset-sequence.sh\n\ndiff --git a/branch.c b/branch.c\nindex 025a97be..a6b6722e 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -3,7 +3,6 @@\n #include \"refs.h\"\n #include \"remote.h\"\n #include \"commit.h\"\n-#include \"sequencer.h\"\n \n struct tracking {\n \tstruct refspec spec;\n@@ -247,5 +246,4 @@ void remove_branch_state(void)\n \tunlink(git_path(\"MERGE_MSG\"));\n \tunlink(git_path(\"MERGE_MODE\"));\n \tunlink(git_path(\"SQUASH_MSG\"));\n-\tremove_sequencer_state(0);\n }\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 851b147f..e80050e1 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -353,7 +353,7 @@ test_expect_success '--continue asks for help after resolving patch to nil' '\n \ttest_i18ngrep \"The previous cherry-pick is now empty\" msg\n '\n \n-test_expect_failure 'follow advice and skip nil patch' '\n+test_expect_success 'follow advice and skip nil patch' '\n \tpristine_detach conflicting &&\n \ttest_must_fail git cherry-pick initial..picked &&\n \ndiff --git a/t/t7106-reset-sequence.sh b/t/t7106-reset-sequence.sh\ndeleted file mode 100755\nindex 83f7ea59..00000000\n--- a/t/t7106-reset-sequence.sh\n+++ /dev/null\n@@ -1,52 +0,0 @@\n-#!/bin/sh\n-\n-test_description='Test interaction of reset --hard with sequencer\n-\n- + anotherpick: rewrites foo to d\n- + picked: rewrites foo to c\n- + unrelatedpick: rewrites unrelated to reallyunrelated\n- + base: rewrites foo to b\n- + initial: writes foo as a, unrelated as unrelated\n-'\n-\n-. ./test-lib.sh\n-\n-pristine_detach () {\n-\tgit cherry-pick --quit &&\n-\tgit checkout -f \"$1^0\" &&\n-\tgit read-tree -u --reset HEAD &&\n-\tgit clean -d -f -f -q -x\n-}\n-\n-test_expect_success setup '\n-\techo unrelated >unrelated &&\n-\tgit add unrelated &&\n-\ttest_commit initial foo a &&\n-\ttest_commit base foo b &&\n-\ttest_commit unrelatedpick unrelated reallyunrelated &&\n-\ttest_commit picked foo c &&\n-\ttest_commit anotherpick foo d &&\n-\tgit config advice.detachedhead false\n-\n-'\n-\n-test_expect_success 'reset --hard cleans up sequencer state, providing one-level undo' '\n-\tpristine_detach initial &&\n-\ttest_must_fail git cherry-pick base..anotherpick &&\n-\ttest_path_is_dir .git/sequencer &&\n-\tgit reset --hard &&\n-\ttest_path_is_missing .git/sequencer &&\n-\ttest_path_is_dir .git/sequencer-old &&\n-\tgit reset --hard &&\n-\ttest_path_is_missing .git/sequencer-old\n-'\n-\n-test_expect_success 'cherry-pick --abort does not leave sequencer-old dir' '\n-\tpristine_detach initial &&\n-\ttest_must_fail git cherry-pick base..anotherpick &&\n-\tgit cherry-pick --abort &&\n-\ttest_path_is_missing .git/sequencer &&\n-\ttest_path_is_missing .git/sequencer-old\n-'\n-\n-test_done\n-- \n1.7.8.rc3\n"},{"id":"180795","messageId":"20111210130612.GH22035@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"[PATCH 7/7] revert: stop creating and removing sequencer-old directory","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-10T13:06:12Z","receivedAt":"2011-12-10T13:06:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Now that \"git reset\" no longer implicitly removes .git/sequencer that\nthe operator may or may not have wanted to keep, the logic to write a\nbackup copy of .git/sequencer and remove it when stale is not needed\nany more. Simplify the sequencer API and repository layout by\ndropping it.\n\nSigned-off-by: Jonathan Nieder \n---\nThat's the end. I hope the patches provided some amusement, and\nadvice towards making them more useful would be welcome.\n\nGood night,\nJonathan\n\n builtin/revert.c | 6 +++---\n sequencer.c | 10 ++--------\n sequencer.h | 12 ++----------\n 3 files changed, 7 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 28deb85b..444df413 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -944,7 +944,7 @@ static int sequencer_rollback(struct replay_opts *opts)\n \t}\n \tif (reset_for_rollback(sha1))\n \t\tgoto fail;\n-\tremove_sequencer_state(1);\n+\tremove_sequencer_state();\n \tstrbuf_release(&buf);\n \treturn 0;\n fail:\n@@ -1026,7 +1026,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \t * Sequence of picks finished successfully; cleanup by\n \t * removing the .git/sequencer directory\n \t */\n-\tremove_sequencer_state(1);\n+\tremove_sequencer_state();\n \treturn 0;\n }\n \n@@ -1084,7 +1084,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t * one that is being continued\n \t */\n \tif (opts->subcommand == REPLAY_REMOVE_STATE) {\n-\t\tremove_sequencer_state(1);\n+\t\tremove_sequencer_state();\n \t\treturn 0;\n \t}\n \tif (opts->subcommand == REPLAY_ROLLBACK)\ndiff --git a/sequencer.c b/sequencer.c\nindex bc2c046a..d1f28a69 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -3,17 +3,11 @@\n #include \"strbuf.h\"\n #include \"dir.h\"\n \n-void remove_sequencer_state(int aggressive)\n+void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n-\tstruct strbuf seq_old_dir = STRBUF_INIT;\n \n \tstrbuf_addf(&seq_dir, \"%s\", git_path(SEQ_DIR));\n-\tstrbuf_addf(&seq_old_dir, \"%s\", git_path(SEQ_OLD_DIR));\n-\tremove_dir_recursively(&seq_old_dir, 0);\n-\trename(git_path(SEQ_DIR), git_path(SEQ_OLD_DIR));\n-\tif (aggressive)\n-\t\tremove_dir_recursively(&seq_old_dir, 0);\n+\tremove_dir_recursively(&seq_dir, 0);\n \tstrbuf_release(&seq_dir);\n-\tstrbuf_release(&seq_old_dir);\n }\ndiff --git a/sequencer.h b/sequencer.h\nindex f435fdb4..2d4528f2 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -2,19 +2,11 @@\n #define SEQUENCER_H\n \n #define SEQ_DIR\t\t\"sequencer\"\n-#define SEQ_OLD_DIR\t\"sequencer-old\"\n #define SEQ_HEAD_FILE\t\"sequencer/head\"\n #define SEQ_TODO_FILE\t\"sequencer/todo\"\n #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n \n-/*\n- * Removes SEQ_OLD_DIR and renames SEQ_DIR to SEQ_OLD_DIR, ignoring\n- * any errors. Intended to be used by 'git reset'.\n- *\n- * With the aggressive flag, it additionally removes SEQ_OLD_DIR,\n- * ignoring any errors. Inteded to be used by the sequencer's\n- * '--quit' subcommand.\n- */\n-void remove_sequencer_state(int aggressive);\n+/* Removes SEQ_DIR. */\n+extern void remove_sequencer_state(void);\n \n #endif\n-- \n1.7.8.rc3\n"},{"id":"180830","messageId":"20111211195836.GA25482@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-11T19:58:36Z","receivedAt":"2011-12-11T19:58:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Here are patches to address some UI warts\n[... rambling cover letter snipped ...]\n\nLet's try that again. :)\n\nCurrent git has a somewhat odd behavior when cherry-picking multiple\ncommits and running into a conflict in the _last_ commit of the\nseries. Imagine the following sequence of operations:\n\n 1. git cherry-pick simplething simplethingtwo complexthing\n 2. CONFLICT.\n 3. git cherry-pick --abort\n\nIt would be most consistent for the entire cherry-pick sequence to be\nrolled back, so the user can come up with some other sequence of\ncommits to try. After all, that's what happens if a conflict is\nencountered applying simplethingtwo and the user asks to abort.\n\nInstead, by the time complexthing is being applied, git has forgotten\nabout the multi-pick sequence entirely. And the --abort does not even\nwarn about this weird state --- it just cancels complexthing and\nleaves the earlier commits applied.\n\nThis is an edge case, but I think it's worth fixing. Patch 5/7 does\nso.\n\nIn the same vein, now imagine a different sequence of operations:\n\n 1. git cherry-pick simplething complexthing morecommits...\n 2. CONFLICT.\n 3. git reset --merge\n 4. git cherry-pick --continue\n\nIt would be sensible for this to remove the conflicted patch and go\non with the remaining ones, right? But instead, \"git reset\"\nautomagically removes the sequencer state, so you can't even use\ngit cherry-pick --abort any more. Well, you can, if you say the\nmagic words \"mv .git/sequencer-old .git/sequencer\", but nobody\nactually tells you that.\n\nHow did we ever let this in? \"git reset\" already has well defined\nsemantics that have nothing to do with this. Patches 6/7 and 7/7\nwould help us forget this UI mistake (and I believe it was a mistake)\never happened.\n\nPatch 2 makes cherry-pick --continue behave a little more like\nrebase --continue, for people who like to learn by analogy.\n\nPatches 1 and 3 are just code style things.\n\nPatch 4 is the basic building block that makes patch 5 possible: it\nteaches \"git cherry-pick\" to treat picks of a single commit named on\nthe command line differently from the more complex multi-picks\nrequested with general rev-list arguments. Single-pick is all that\ngit cherry-pick originally supported, and in some details it has to\ndiffer from multi-pick (for example, \"git commit\" after resolving\nconflicts after a conflicted single-pick needs to be enough to clear\nthe state). As a side-benefit, we get the ability to do a single-pick\nin the middle of a multi-pick, which is kind of cool and handy from\ntime to time.\n\nI am interested in sanity checking of the patches and testing. It\nwould be pleasant to find comments like \"yeah, that looks good\" or\n\"what are you thinking?! I ran into the following bug\" arriving in my\ninbox.\n\nIncidentally, I'd like to apologize about not protesting more about\nthese things (and even having suggested some of them) as they\nhappened. Instead of exercising careful oversight over the sanity of\npatches that passed through my mailbox, I had some strange idea of\nusing the Socratic method to help others learn to explore the design\nspace and sanity-check findings themselves...\n\nThanks for reading.\n\nCiao,\nJonathan\n"},{"id":"180905","messageId":"7vzkey6w9f.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111211195836.GA25482@elie.hsd1.il.comcast.net","subject":"Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T08:15:40Z","receivedAt":"2011-12-12T08:15:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> How did we ever let this in? \"git reset\" already has well defined\n> semantics that have nothing to do with this. Patches 6/7 and 7/7\n> would help us forget this UI mistake (and I believe it was a mistake)\n> ever happened.\n\nThanks for catching this. Let's review this quickly and queue it for\nmaintenance track as necessary.\n"},{"id":"180973","messageId":"7v39cp32av.fsf@alter.siamese.dyndns.org","threadId":"28989","inReplyTo":"20111210124644.GA22035@elie.hsd1.il.comcast.net","subject":"Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T21:31:04Z","receivedAt":"2011-12-12T21:31:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Overall, I think this series is a more correct and faithful implementation\nof the design we discussed earlier in the thread\n\n http://thread.gmane.org/gmane.comp.version-control.git/179304/focus=179625\n\nI saw a few minor nits in the log messages but otherwise nothing\nobjectionable jumped at me from my initial reading of the series.\n\nThanks.\n"},{"id":"181123","messageId":"20111214095749.GA19953@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"7v39cp32av.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC/PATCH 0/7] some sequencer loose ends (Re: Fix revert --abort on Windows)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-14T09:57:49Z","receivedAt":"2011-12-14T09:57:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n\n> I saw a few minor nits in the log messages but otherwise nothing\n> objectionable jumped at me from my initial reading of the series.\n\nI was tempted to send a reroll with slightly better log messages once\nI could see your corrections, but it looks like the series has been\nmerged to \"next\" and your corrections already leave me happy enough.\n\nThanks for looking it over.\n"},{"id":"181128","messageId":"CALkWK0nQm9fCNgP5koo9YUp_jXTzrqZNnGpf81MAbT-K=qRhYQ@mail.gmail.com","threadId":"28989","inReplyTo":"20111210124736.GB22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 1/7] revert: give --continue handling its own function","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T13:16:43Z","receivedAt":"2011-12-14T13:16:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder wrote:\n> This makes pick_revisions() a little shorter and easier to read\n> straight through.\n\nAh, yes: you've asked about this earlier. Sounds sane; let's read\nahead and see if anything jumps out.\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 1ea525c1..9f6c85c1 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -1038,6 +1038,21 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n> [...]\n> +static int sequencer_continue(struct replay_opts *opts)\n> +{\n> +       struct commit_list *todo_list = NULL;\n> [...]\n\n>  static int pick_revisions(struct replay_opts *opts)\n>  {\n>        struct commit_list *todo_list = NULL;\n> [...]\n\nThis is the only detailed that jumped out- you're filling up two\ndifferent commit_list structures, depending on whether we're\nperforming a fresh operation or continuing an existing one. Okay.\n\nThanks.\n\np.s- Sorry about the delay; just returned from a short vacation.\n\n-- Ram\n"},{"id":"181138","messageId":"CALkWK0miBzT4BXRDYOhz8JqF2jeyz1L3=pwaGKVm654oHtQbtQ@mail.gmail.com","threadId":"28989","inReplyTo":"20111210124925.GC22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 2/7] revert: allow cherry-pick --continue to commit before resuming","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T14:26:20Z","receivedAt":"2011-12-14T14:26:20Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nJonathan Nieder wrote:\n> When \"git cherry-pick ..bar\" encounters conflicts, permit the operator\n> to use cherry-pick --continue after resolving them as a shortcut for\n> \"git commit && git cherry-pick --continue\" to record the resolution\n> and carry on with the rest of the sequence.\n\nSounds good. I remember my implementation being quite complicated;\nlet's see how you've done this.\n\n> Example: after encountering a conflict from running \"git cherry-pick\n> foo bar baz\":\n>\n>        CONFLICT (content): Merge conflict in main.c\n>        error: could not apply f78a8d98c... bar!\n>        hint: after resolving the conflicts, mark the corrected paths\n>        hint: with 'git add ' or 'git rm '\n>        hint: and commit the result with 'git commit'\n>\n> We edit main.c to resolve the conflict, mark it acceptable with \"git\n> add main.c\", and can run \"cherry-pick --continue\" to resume the\n> sequence.\n>\n>        $ git cherry-pick --continue\n>        [editor opens to confirm commit message]\n>        [master 78c8a8c98] bar!\n>         1 files changed, 1 insertions(+), 1 deletions(-)\n>        [master 87ca8798c] baz!\n>         1 files changed, 1 insertions(+), 1 deletions(-)\n\nI like the presentation of this example: much clearer than my examples.\n\n>  builtin/revert.c                |   23 ++++++-\n>  t/t3510-cherry-pick-sequence.sh |  139 ++++++++++++++++++++++++++++++++++++++-\n>  2 files changed, 156 insertions(+), 6 deletions(-)\n\nOh, good -- lots of new tests :)\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 9f6c85c1..a43b4d85 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -1038,18 +1038,35 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n>        return 0;\n>  }\n>\n> +static int continue_single_pick(void)\n> +{\n> +       const char *argv[] = { \"commit\", NULL };\n> +\n> +       if (!file_exists(git_path(\"CHERRY_PICK_HEAD\")) &&\n> +           !file_exists(git_path(\"REVERT_HEAD\")))\n> +               return error(_(\"no cherry-pick or revert in progress\"));\n> +       return run_command_v_opt(argv, RUN_GIT_CMD);\n> +}\n\nVery nice! I can see how the introduction of REVERT_HEAD simplifies things :)\nI'm totally embarrassed by the horribly convoluted logic in the \"New\nsequencer workflow!\" I posted earlier.\n\nNote to self: don't capitalize error() messages.\n\n>  static int sequencer_continue(struct replay_opts *opts)\n>  {\n>        struct commit_list *todo_list = NULL;\n>\n>        if (!file_exists(git_path(SEQ_TODO_FILE)))\n> -               return error(_(\"No %s in progress\"), action_name(opts));\n> +               return continue_single_pick();\n>        read_populate_opts(&opts);\n>        read_populate_todo(&todo_list, opts);\n>\n>        /* Verify that the conflict has been resolved */\n> -       if (!index_differs_from(\"HEAD\", 0))\n> -               todo_list = todo_list->next;\n> +       if (file_exists(git_path(\"CHERRY_PICK_HEAD\")) ||\n> +           file_exists(git_path(\"REVERT_HEAD\"))) {\n> +               int ret = continue_single_pick();\n> +               if (ret)\n> +                       return ret;\n> +       }\n> +       if (index_differs_from(\"HEAD\", 0))\n> +               return error_dirty_index(opts);\n> +       todo_list = todo_list->next;\n>        return pick_commits(todo_list, opts);\n>  }\n\nVery nicely done. I can see why 1/7 makes so much sense now: it\nhelps think of different operations independently.\n\n> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n> index 2c4c1c85..4d1883b7 100755\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -2,6 +2,7 @@\n>\n>  test_description='Test cherry-pick continuation features\n>\n> + +  conflicting: rewrites unrelated to conflicting\n>   + yetanotherpick: rewrites foo to e\n>   + anotherpick: rewrites foo to d\n>   + picked: rewrites foo to c\n\nNote to self: this list of commits is becoming quite unwieldy. We\nshould probably refactor these sometime.\n\n> @@ -27,6 +28,7 @@ test_cmp_rev () {\n>  }\n>\n>  test_expect_success setup '\n> +       git config advice.detachedhead false\n>        echo unrelated >unrelated &&\n>        git add unrelated &&\n>        test_commit initial foo a &&\n\nHuh, why are you moving this line up? Oh, right: there are\n\"test_commit\" statements in the setup- good catch. This is unrelated\nto your patch and should be a separate commit though.\n\n> @@ -35,8 +37,8 @@ test_expect_success setup '\n>        test_commit picked foo c &&\n>        test_commit anotherpick foo d &&\n>        test_commit yetanotherpick foo e &&\n> -       git config advice.detachedhead false\n> -\n> +       pristine_detach initial &&\n> +       test_commit conflicting unrelated\n>  '\n\nLooks fishy- I wonder why you're doing this. Let's read ahead and find out.\n\n> @@ -243,7 +245,66 @@ test_expect_success '--continue complains when there are unresolved conflicts' '\n>        test_must_fail git cherry-pick --continue\n>  '\n>\n> -test_expect_success '--continue continues after conflicts are resolved' '\n> +test_expect_success '--continue of single cherry-pick' '\n> +       pristine_detach initial &&\n> +       echo c >expect &&\n> +       test_must_fail git cherry-pick picked &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +\n> +       test_cmp expect foo &&\n> +       test_cmp_rev initial HEAD^ &&\n> +       git diff --exit-code HEAD &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n> +'\n\nBeautiful. The tests I wrote are ugly in comparison :\\\n\n> +test_expect_success '--continue of single revert' '\n> +       pristine_detach initial &&\n> +       echo resolved >expect &&\n> +       echo \"Revert \\\"picked\\\"\" >expect.msg &&\n> +       test_must_fail git revert picked &&\n> +       echo resolved >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n\nHuh? You're continuing a \"git revert\" with a a \"git cherry-pick\n--continue\"? The current 'master' still uses a commit_list, and\ndoesn't allow mixed \"pick\" and \"revert\" instructions yet.\n\n> +       git diff --exit-code HEAD &&\n> +       test_cmp expect foo &&\n> +       test_cmp_rev initial HEAD^ &&\n> +       git diff-tree -s --pretty=tformat:%s HEAD >msg &&\n> +       test_cmp expect.msg msg &&\n> +       test_must_fail git rev-parse --verify CHERRY_PICK_HEAD &&\n> +       test_must_fail git rev-parse --verify REVERT_HEAD\n> +'\n\nA couple of notes:\n\n1. I haven't used the \"-s\" flag of \"git diff-tree\" before, so I opened\nup the documentation to find this:\n\n By default, 'git diff-tree --stdin' shows differences,\n either in machine-readable form (without '-p') or in patch\n form (with '-p'). This output can be suppressed. It is\n only useful with '-v' flag.\n\nVery misleading. TODO: Fix this.\n\n2. Why did you use \"diff-tree\" to get the commit message? Isn't\n\"cat-file commit\" much more straightforward?\n\n> +test_expect_success '--continue after resolving conflicts' '\n> +       pristine_detach initial &&\n> +       echo d >expect &&\n> +       cat >expect.log <<-\\EOF &&\n> +       OBJID\n> +       :100644 100644 OBJID OBJID M    foo\n> +       OBJID\n> +       :100644 100644 OBJID OBJID M    foo\n> +       OBJID\n> +       :100644 100644 OBJID OBJID M    unrelated\n> +       OBJID\n> +       :000000 100644 OBJID OBJID A    foo\n> +       :000000 100644 OBJID OBJID A    unrelated\n> +       EOF\n> +       test_must_fail git cherry-pick base..anotherpick &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +       {\n> +               git rev-list HEAD |\n> +               git diff-tree --root --stdin |\n> +               sed \"s/$_x40/OBJID/g\"\n> +       } >actual.log &&\n> +       test_cmp expect foo &&\n> +       test_cmp expect.log actual.log\n> +'\n\nUnchanged from the original: I suspect you've moved the generation of\nexpectation messages up to produce a clean diff.\n\n> +test_expect_success '--continue after resolving conflicts and committing' '\n>        pristine_detach initial &&\n>        test_must_fail git cherry-pick base..anotherpick &&\n>        echo \"c\" >foo &&\n\nOkay, the diff isn't all that clean :P\n\n> +test_expect_success '--continue asks for help after resolving patch to nil' '\n> +       pristine_detach conflicting &&\n> +       test_must_fail git cherry-pick initial..picked &&\n> +\n> +       test_cmp_rev unrelatedpick CHERRY_PICK_HEAD &&\n> +       git checkout HEAD -- unrelated &&\n> +       test_must_fail git cherry-pick --continue 2>msg &&\n> +       test_i18ngrep \"The previous cherry-pick is now empty\" msg\n> +'\n\nI thought it was a bad idea to grep for specific output messages,\nbecause of their volatile nature? Remind me what this test has to do\nwith the rest of your patch?\n\n> +test_expect_failure 'follow advice and skip nil patch' '\n> +       pristine_detach conflicting &&\n> +       test_must_fail git cherry-pick initial..picked &&\n> +\n> +       git checkout HEAD -- unrelated &&\n> +       test_must_fail git cherry-pick --continue &&\n> +       git reset &&\n> +       git cherry-pick --continue &&\n> +\n> +       git rev-list initial..HEAD >commits &&\n> +       test_line_count = 3 commits\n> +'\n\nAgain, what does this test have to do with the rest of your patch?\n\n>  test_expect_success '--continue respects opts' '\n>        pristine_detach initial &&\n>        test_must_fail git cherry-pick -x base..anotherpick &&\n> @@ -288,6 +372,29 @@ test_expect_success '--continue respects opts' '\n>        grep \"cherry picked from\" anotherpick_msg\n>  '\n>\n> +test_expect_success '--continue of single-pick respects -x' '\n> +       pristine_detach initial &&\n> +       test_must_fail git cherry-pick -x picked &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +       test_path_is_missing .git/sequencer &&\n> +       git cat-file commit HEAD >msg &&\n> +       grep \"cherry picked from\" msg\n> +'\n\nI'd have liked s/respects -x/respects opts/ here for symmetry with the\nprevious test.\n\n> +test_expect_success '--continue respects -x in first commit in multi-pick' '\n> +       pristine_detach initial &&\n> +       test_must_fail git cherry-pick -x picked anotherpick &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +       test_path_is_missing .git/sequencer &&\n> +       git cat-file commit HEAD^ >msg &&\n> +       picked=$(git rev-parse --verify picked) &&\n> +       grep \"cherry picked from.*$picked\" msg\n> +'\n\nCan you explain why \"first commit in a multi-pick\" is a special case?\n\n> @@ -306,6 +413,32 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n>        grep \"Signed-off-by:\" anotherpick_msg\n>  '\n>\n> +test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n> +       pristine_detach initial &&\n> +       test_must_fail git cherry-pick -s picked anotherpick &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +\n> +       git diff --exit-code HEAD &&\n> +       test_cmp_rev initial HEAD^^ &&\n> +       git cat-file commit HEAD^ >msg &&\n> +       ! grep Signed-off-by: msg\n> +'\n\nUnrelated.\n\n> +test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n> +       pristine_detach initial &&\n> +       test_must_fail git cherry-pick -s picked &&\n> +       echo c >foo &&\n> +       git add foo &&\n> +       git cherry-pick --continue &&\n> +\n> +       git diff --exit-code HEAD &&\n> +       test_cmp_rev initial HEAD^ &&\n> +       git cat-file commit HEAD >msg &&\n> +       ! grep Signed-off-by: msg\n> +'\n\nIf the previous test were a separate patch preceding this one, this'd\nbelong here.\n\nThanks for working on this.\n\n-- Ram\n"},{"id":"181141","messageId":"CALkWK0kb8NzS+PWtvucfpAOMu+X8yZ1hbTVu+ZurhNJcdesc2w@mail.gmail.com","threadId":"28989","inReplyTo":"20111210125825.GD22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 3/7] revert: pass around rev-list args in already-parsed form","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T14:51:14Z","receivedAt":"2011-12-14T14:51:14Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hey,\n\nJonathan Nieder wrote:\n> [...]\n> Original patch by Jonathan, tweaks and test from Ram.\n>\n> Signed-off-by: Jonathan Nieder \n> Improved-by: Ramkumar Ramachandra \n\nI've already seen this, so let's see how it fits into the rest of the series.\n\n-- Ram\n"},{"id":"181152","messageId":"CALkWK0mt03SSNT-svUO1wHdq5OpM=0xQO3FHkSGGEDuW-jUEXA@mail.gmail.com","threadId":"28989","inReplyTo":"20111210125948.GE22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T15:48:45Z","receivedAt":"2011-12-14T15:48:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> When I messed up a difficult conflict in the middle of a cherry-pick\n> sequence, it can be useful to be able to 'git checkout HEAD . && git\n> cherry-pick that-one-commit' to restart the conflict resolution.\n\nI was about to complain about the commit message until I noticed that\nJunio already fixed it in `next`:\n\n revert: allow single-pick in the middle of cherry-pick sequence\n\n After messing up a difficult conflict resolution in the middle of a\n cherry-pick sequence, it can be useful to be able to\n\n git checkout HEAD . && git cherry-pick that-one-commit\n\n to restart the conflict resolution. The current code however errors out\n saying that another cherry-pick is already in progress.\n\nInteresting concept; let's see how it's implemented.\n\n> Suggested-by: Johannes Sixt \n\nCould you link to the corresponding thread with Johannes?\n\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 71570357..dcb69904 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -1072,6 +1072,12 @@ static int sequencer_continue(struct replay_opts *opts)\n>        return pick_commits(todo_list, opts);\n>  }\n>\n> +static int single_pick(struct commit *cmit, struct replay_opts *opts)\n> +{\n> +       setenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n> +       return do_pick_commit(cmit, opts);\n> +}\n\nsingle_pick as opposed to the continue_single_pick introduced in 2/7.\n\n>  static int pick_revisions(struct replay_opts *opts)\n>  {\n>        struct commit_list *todo_list = NULL;\n> @@ -1097,6 +1103,26 @@ static int pick_revisions(struct replay_opts *opts)\n>                return sequencer_continue(opts);\n>\n>        /*\n> +        * If we were called as \"git cherry-pick \", just\n> +        * cherry-pick/revert it, set CHERRY_PICK_HEAD /\n> +        * REVERT_HEAD, and don't touch the sequencer state.\n> +        * This means it is possible to cherry-pick in the middle\n> +        * of a cherry-pick sequence.\n> +        */\n\nConceptually all very good. What I'm really interested in seeing is\nhow you persist opts for \"cherry-pick --continue\" when a single-commit\npick fails: in other words, how you manage to get \" --continue of\nsingle-pick respects -x\" to pass.\n\n> +       if (opts->revs->cmdline.nr == 1 &&\n> +           opts->revs->cmdline.rev->whence == REV_CMD_REV &&\n> +           opts->revs->no_walk &&\n> +           !opts->revs->cmdline.rev->flags) {\n\nYuck, seriously.\n1. I'd have expected you to check opts->revs->commits, not\nopts->revs->cmdline.nr. Okay, you're using the cmdline because the\nrevision walk hasn't happened yet.\n2. Why are you using opts->revs->cmdline.rev->whence as opposed to\nopts->action? Why do you want to expose the underlying revision\nwalking mechanism?\n3. When will the opts->revs->no_walk condition not be satisfied? Only\nwhen you explicitly set it to 0 or NULL, right -- where is this\nhappening in revert.c?\n4. Why are you checking flags? When is this condition not going to be\nsatisfied?\n\nSince 3 and 4 indicate that you're being overly defensive, consistency\nrequires you to guarantee that this code will work no matter what the\nrev_info struct is filled up with prior to this segment.\nIs this true?\n\n> +               struct commit *cmit;\n> +               if (prepare_revision_walk(opts->revs))\n> +                       die(_(\"revision walk setup failed\"));\n> +               cmit = get_revision(opts->revs);\n> +               if (!cmit || get_revision(opts->revs))\n> +                       die(\"BUG: expected exactly one commit from walk\");\n> +               return single_pick(cmit, opts);\n> +       }\n\nI'd have expected you to reuse prepare_revs().\n\n> +       /*\n>         * Start a new cherry-pick/ revert sequence; but\n>         * first, make sure that an existing one isn't in\n>         * progress\n\nSince all your new code is a special case of \"Start a new cherry-pick/\nrevert sequence\", you don't check the sequencer state in the first\nplace.\n\n> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n> index 56c95ec1..98a27a23 100755\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -50,6 +50,18 @@ test_expect_success 'cherry-pick persists data on failure' '\n>        test_path_is_file .git/sequencer/opts\n>  '\n>\n> +test_expect_success 'cherry-pick mid-cherry-pick-sequence' '\n> +       pristine_detach initial &&\n> +       test_must_fail git cherry-pick base..anotherpick &&\n> +       test_cmp_rev picked CHERRY_PICK_HEAD &&\n> +       # \"oops, I forgot that these patches rely on the change from base\"\n> +       git checkout HEAD foo &&\n> +       git cherry-pick base &&\n> +       git cherry-pick picked &&\n> +       git cherry-pick --continue &&\n> +       git diff --exit-code anotherpick\n> +'\n\nCute feature, although I don't ever recall needing it personally. Why\ndoes this relatively esoteric \"feature\" belong along with the other\n\"maintenance patches\" in jn/maint-sequencer-fixes?\n\n-- Ram\n"},{"id":"181154","messageId":"CALkWK0nn2mhaP5pwUob2_1ADzNkfP9_ZtUqWnhqWfn7pR9aj7Q@mail.gmail.com","threadId":"28989","inReplyTo":"20111210130212.GF22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 5/7] revert: do not remove state until sequence is finished","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T16:02:32Z","receivedAt":"2011-12-14T16:02:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hey,\n\nJonathan Nieder wrote:\n> [...]\n> After the recent patch \"allow single-pick in the middle of cherry-pick\n> sequence\" we don't need that hack any more.  In the new regime, a\n> traditional \"git cherry-pick \" command never looks at\n> .git/sequencer, so we do not need to cripple \"git cherry-pick\n> ..\" for it any more.\n\nSo this is why you needed that \"relatively esoteric feature\" :P\nThis approach competes with the approach I presented in \"New sequencer\nworkflow!\" [1], where I use a special case to side-step various\nsequencer state files: this approach wins on the grounds of\nsimplicity, and I can't see any potential long-term issues with my\nlimited foresight. Do you have any other points of comparison?\n\n[1]: https://github.com/artagnon/git sequencer #; 8a08d09b9\n\n-- Ram\n"},{"id":"181156","messageId":"CALkWK0=22=3grVo57QiJ0ZJ_yF3Tzw2bUzqmQ3UbjM0NKLUO_Q@mail.gmail.com","threadId":"28989","inReplyTo":"20111210130348.GG22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 6/7] Revert \"reset: Make reset remove the sequencer state\"","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T16:06:19Z","receivedAt":"2011-12-14T16:06:19Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nJonathan Nieder wrote:\n> This reverts commit 95eb88d8ee588d89b4f06d2753ed4d16ab13b39f, which\n> was a UI experiment that did not reflect how \"git reset\" actually gets\n> used.  The reversion also fixes a test, indicated in the patch.\n\nLooks good.\n\nThanks.\n\n-- Ram\n"},{"id":"181157","messageId":"CALkWK0n_Weyr3Y97h9P+bRTizZjdxeQRgP8tG0ssiBpksBtaKg@mail.gmail.com","threadId":"28989","inReplyTo":"20111210130612.GH22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 7/7] revert: stop creating and removing sequencer-old directory","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-12-14T16:10:45Z","receivedAt":"2011-12-14T16:10:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder wrote:\n> That's the end.\n\nApart from the few minor details, I'm happy with the series overall.\n\n>  I hope the patches provided some amusement, and\n> advice towards making them more useful would be welcome.\n\nOh, yes. There were quite a few :facepalm: moments in there for me ;)\n\nThanks for the enjoyable read.\n\n-- Ram\n"},{"id":"181159","messageId":"20111214162148.GA481@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"CALkWK0mt03SSNT-svUO1wHdq5OpM=0xQO3FHkSGGEDuW-jUEXA@mail.gmail.com","subject":"Re: [PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-14T16:21:48Z","receivedAt":"2011-12-14T16:21:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(out of order for convenience)\nRamkumar Ramachandra wrote:\n> Jonathan Nieder wrote:\n\n>> Suggested-by: Johannes Sixt \n>\n> Could you link to the corresponding thread with Johannes?\n\nNo, I prefer not to. If I did a good job, the commit message would\nexplain enough already, and in exceptional cases, the interested\nreader can look up the mailing list message the commit comes from and\nwalk upthread, no?\n\n> Cute feature, although I don't ever recall needing it personally. Why\n> does this relatively esoteric \"feature\" belong along with the other\n> \"maintenance patches\" in jn/maint-sequencer-fixes?\n\nRead ahead in the series, or read the cover letter. :)\n\n> What I'm really interested in seeing is\n> how you persist opts for \"cherry-pick --continue\" when a single-commit\n> pick fails: in other words, how you manage to get \" --continue of\n> single-pick respects -x\" to pass.\n\nThat's a good question. I did the lazy thing and let the existing\n\"git cherry-pick\" logic take care of it (it writes MERGE_MSG).\n\n>> +               struct commit *cmit;\n>> +               if (prepare_revision_walk(opts->revs))\n>> +                       die(_(\"revision walk setup failed\"));\n>> +               cmit = get_revision(opts->revs);\n>> +               if (!cmit || get_revision(opts->revs))\n>> +                       die(\"BUG: expected exactly one commit from walk\");\n>> +               return single_pick(cmit, opts);\n>> +       }\n>\n> I'd have expected you to reuse prepare_revs().\n\nWhy? The purposes do not overlap much.\n\n>> +       if (opts->revs->cmdline.nr == 1 &&\n>> +           opts->revs->cmdline.rev->whence == REV_CMD_REV &&\n>> +           opts->revs->no_walk &&\n>> +           !opts->revs->cmdline.rev->flags) {\n>\n> Yuck, seriously.\n> 1. I'd have expected you to check opts->revs->commits, not\n> opts->revs->cmdline.nr. Okay, you're using the cmdline because the\n> revision walk hasn't happened yet.\n\nIt would have been easy to do a revision walk and count and I'm using\nthe cmdline instead deliberately --- the goal really is \"anything more\ncomplicated than a simple rev on the command line should trip the\nmulti-pick logic\".\n\nI admit though that I'm not too familiar with the new cmdline_info\nAPI. I'd welcome a simpler expression with the same effect.\n\nAlso, I probably should have included a test that does some\n\n\t\tgit cherry-pick picked^..picked\n\nthing and verifies that this is treated as a multi-pick. And\ndocumented this. :)\n\nThanks for pointing out the questionable bits. I am tempted to reroll\nto put this after patch 6/7, which would make it possible to use \"git\nreset --merge\" in the commit message for a more natural explanation.\n\nThat would also provide an opportunity to reuse some text from [1],\nwhich in hindsight seems to have explained some aspects of each patch\na little more clearly.\n\nThanks, and hoping that clarifies a little,\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/185716/focus=186811\n"},{"id":"181161","messageId":"20111214164826.GB481@elie.hsd1.il.comcast.net","threadId":"28989","inReplyTo":"CALkWK0miBzT4BXRDYOhz8JqF2jeyz1L3=pwaGKVm654oHtQbtQ@mail.gmail.com","subject":"Re: [PATCH 2/7] revert: allow cherry-pick --continue to commit before resuming","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-12-14T16:48:26Z","receivedAt":"2011-12-14T16:48:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Sounds good. I remember my implementation being quite complicated;\n> let's see how you've done this.\n\nI have to confess that I don't remember the implementation you are\nreferring to. Maybe I could have taken inspiration from it.\n\nThe rest of this message is about tests.\n\n[...]\n> Jonathan Nieder wrote:\n\n>> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\n>> index 2c4c1c85..4d1883b7 100755\n>> --- a/t/t3510-cherry-pick-sequence.sh\n>> +++ b/t/t3510-cherry-pick-sequence.sh\n>> @@ -2,6 +2,7 @@\n>>\n>>  test_description='Test cherry-pick continuation features\n>>\n>> + +  conflicting: rewrites unrelated to conflicting\n>>   + yetanotherpick: rewrites foo to e\n>>   + anotherpick: rewrites foo to d\n>>   + picked: rewrites foo to c\n>\n> Note to self: this list of commits is becoming quite unwieldy. We\n> should probably refactor these sometime.\n\nTo clarify, \"conflicting\" is sitting on a separate branch from the\nrest of the commits. This --help text uses \"git show-branch\"-style\noutput, which is perhaps out of fashion but compact and used by some\nexisting tests.\n\n>> @@ -27,6 +28,7 @@ test_cmp_rev () {\n>>  }\n>>\n>>  test_expect_success setup '\n>> +       git config advice.detachedhead false\n>>        echo unrelated >unrelated &&\n>>        git add unrelated &&\n>>        test_commit initial foo a &&\n>\n> Huh, why are you moving this line up? Oh, right: there are\n> \"test_commit\" statements in the setup- good catch.\n\nNah, it's for the pristine_detach, not the test_commit.\n\nI did miss an &&, though. Not enough to justify rerolling on its own\nbut seems worth fixing if resending anyway.\n\n>> @@ -35,8 +37,8 @@ test_expect_success setup '\n>>        test_commit picked foo c &&\n>>        test_commit anotherpick foo d &&\n>>        test_commit yetanotherpick foo e &&\n>> -       git config advice.detachedhead false\n>> -\n>> +       pristine_detach initial &&\n>> +       test_commit conflicting unrelated\n>>  '\n>\n> Looks fishy- I wonder why you're doing this. Let's read ahead and find out.\n\nDo you mean that you'd prefer this \"conflicting\" commit not to be\npart of the setup shared between tests?\n\n[...]\n>> +test_expect_success '--continue of single revert' '\n>> +       pristine_detach initial &&\n>> +       echo resolved >expect &&\n>> +       echo \"Revert \\\"picked\\\"\" >expect.msg &&\n>> +       test_must_fail git revert picked &&\n>> +       echo resolved >foo &&\n>> +       git add foo &&\n>> +       git cherry-pick --continue &&\n>\n> Huh? You're continuing a \"git revert\" with a a \"git cherry-pick\n> --continue\"?\n\nYep, works fine.\n\n[...]\n> 1. I haven't used the \"-s\" flag of \"git diff-tree\" before, so I opened\n> up the documentation to find this:\n\nYeah, that documentation sucks. I'll keep this message marked as a\nreminder to look at it.\n\nJust like \"git show\" is the porcelain command to show a commit, \"git\ndiff-tree\" is the corresponding plumbing.\n\n[...]\n>> +test_expect_success '--continue after resolving conflicts' '\n[...]\n> Unchanged from the original: I suspect you've moved the generation of\n> expectation messages up to produce a clean diff.\n\nIt's just a new test. If rerolling, I'll make it imitate the style of\nthe existing test following it better.\n\n[...]\n>> +test_expect_success '--continue asks for help after resolving patch to nil' '\n>> +       pristine_detach conflicting &&\n>> +       test_must_fail git cherry-pick initial..picked &&\n>> +\n>> +       test_cmp_rev unrelatedpick CHERRY_PICK_HEAD &&\n>> +       git checkout HEAD -- unrelated &&\n>> +       test_must_fail git cherry-pick --continue 2>msg &&\n>> +       test_i18ngrep \"The previous cherry-pick is now empty\" msg\n>> +'\n>\n> I thought it was a bad idea to grep for specific output messages,\n> because of their volatile nature?\n\nThis test is about --continue asking for help instead of succeeding or\nfailing in some uncontrolled way, so it seemed useful to check that\nthe message actually pertains to that.\n\n> Remind me what this test has to do\n> with the rest of your patch?\n\nWith this change in how --continue works, I wanted to make sure the\nsemantics that were not supposed to be changed were still intact.\n\n[...]\n>> +test_expect_failure 'follow advice and skip nil patch' '\n[...]\n> Again, what does this test have to do with the rest of your patch?\n\nLikewise.\n\n[...]\n>> +test_expect_success '--continue of single-pick respects -x' '\n[...]\n> I'd have liked s/respects -x/respects opts/ here for symmetry with the\n> previous test.\n\nMaybe the previous one should say \"respects -x\".\n\nI am not sure what it would mean for --continue of a single-pick to\nrespect --strategy, for example.\n\n>> +test_expect_success '--continue respects -x in first commit in multi-pick' '\n[...]\n> Can you explain why \"first commit in a multi-pick\" is a special case?\n\nI guess you mean \"how does this differ from the existing '--continue\nrespects opts' test?\".\n\nGood question. In the existing \"--continue respects opts\" test, we\nexplicitly run \"git commit\" before \"git cherry-pick --continue\". This\ntest checks that running \"git cherry-pick --continue\" without\ncommiting first does not cause the commit message to be clobbered.\n\n[...]\n>> @@ -306,6 +413,32 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n>>        grep \"Signed-off-by:\" anotherpick_msg\n>>  '\n>>\n>> +test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n[...]\n> Unrelated.\n[...]\n>> +test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n\nThere was no implicit commit of resolution before this patch, so how\ncan it be unrelated?\n\n[...]\n> Thanks for working on this.\n\nThanks for your attention to detail.\n\nSincerely,\nJonathan\n"},{"id":"188555","messageId":"CACBZZX53dhb2SYMcMrQnhm3nsdTvZOm_0BCbSdPEPNdT0vxgTA@mail.gmail.com","threadId":"28989","inReplyTo":"20111210125948.GE22035@elie.hsd1.il.comcast.net","subject":"Re: [PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-04-05T11:49:42Z","receivedAt":"2012-04-05T11:49:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Dec 10, 2011 at 13:59, Jonathan Nieder wrote:\n>        /*\n> +        * If we were called as \"git cherry-pick \", just\n> +        * cherry-pick/revert it, set CHERRY_PICK_HEAD /\n> +        * REVERT_HEAD, and don't touch the sequencer state.\n> +        * This means it is possible to cherry-pick in the middle\n> +        * of a cherry-pick sequence.\n> +        */\n> +       if (opts->revs->cmdline.nr == 1 &&\n> +           opts->revs->cmdline.rev->whence == REV_CMD_REV &&\n> +           opts->revs->no_walk &&\n> +           !opts->revs->cmdline.rev->flags) {\n> +               struct commit *cmit;\n> +               if (prepare_revision_walk(opts->revs))\n> +                       die(_(\"revision walk setup failed\"));\n> +               cmit = get_revision(opts->revs);\n> +               if (!cmit || get_revision(opts->revs))\n> +                       die(\"BUG: expected exactly one commit from walk\");\n> +               return single_pick(cmit, opts);\n> +       }\n> +\n> +       /*\n>         * Start a new cherry-pick/ revert sequence; but\n\nThis might be an issue introduced later in Ramkumar's code when he\nmoved this around, but on git.git's e5056c0 I get this:\n\n $ ./git revert --author=Ævar :/i18n\n fatal: BUG: expected exactly one commit from walk\n\nThat should find and revert this, right:\n\n $ git --no-pager log --author=Ævar --grep=\"i18n\" -1\n commit 5eb660e\n Author: Ævar Arnfjörð Bjarmason \n Date: Sat Mar 10 12:29:35 2012 +0000\n\n perl/Makefile: install Git::I18N under NO_PERL_MAKEMAKER\n\n When I added the i18n infrastructure in v1.7.8-rc2-1-g5e9637c I forgot\n to install Git::I18N also when NO_PERL_MAKEMAKER=YesPlease was\n set. Change the generation of the fallback perl.mak file to do that.\n\n Now Git/I18N.pm is installed alongside Git.pm in such a way that\n anything that uses GITPERLLIB will find it.\n\n Reported-by: Tom G. Christensen \n Signed-off-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Junio C Hamano \n\n?\n"},{"id":"188556","messageId":"20120405121507.GA5175@burratino","threadId":"28989","inReplyTo":"CACBZZX53dhb2SYMcMrQnhm3nsdTvZOm_0BCbSdPEPNdT0vxgTA@mail.gmail.com","subject":"Re: [PATCH 4/7] revert: allow single-pick in the middle of cherry-pick sequence","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-04-05T12:15:08Z","receivedAt":"2012-04-05T12:15:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> This might be an issue introduced later in Ramkumar's code when he\n> moved this around, but on git.git's e5056c0 I get this:\n>\n> $ ./git revert --author=Ævar :/i18n\n> fatal: BUG: expected exactly one commit from walk\n\nThis seems buggy on two counts:\n\n 1. The \":/\" magic should probably imply --do-walk so that\n\n\tgit show --author=Ævar :/i18n\n\n does the right thing.\n\n 2.\n\n\t$ git cherry-pick --author=Ævar origin/pu\n\tfatal: BUG: expected exactly one commit from walk\n\n The single-pick code does not understand that such a\n simple revision specifier can return no revisions. A more\n appropriate error message would be\n\n\tfatal: empty commit set passed\n\ndiff --git i/sequencer.c w/sequencer.c\nindex a37846a5..736ccd57 100644\n--- i/sequencer.c\n+++ w/sequencer.c\n@@ -908,7 +908,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \t\tif (prepare_revision_walk(opts->revs))\n \t\t\tdie(_(\"revision walk setup failed\"));\n \t\tcmit = get_revision(opts->revs);\n-\t\tif (!cmit || get_revision(opts->revs))\n+\t\tif (!cmit)\n+\t\t\t/* e.g. \"git cherry-pick --author=nobody \" */\n+\t\t\tdie(_(\"empty commit set passed\"));\n+\t\tif (get_revision(opts->revs))\n \t\t\tdie(\"BUG: expected exactly one commit from walk\");\n \t\treturn single_pick(cmit, opts);\n \t}\n"}]}