{"thread":{"id":"28072","subject":"[PATCH 0/6] Towards a generalized sequencer","startedAt":"2011-08-11T18:51:39Z","lastAt":"2011-08-13T20:54:52Z","messageCount":31,"participants":["Ramkumar Ramachandra","Jonathan Nieder","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"173301","messageId":"1313088705-32222-1-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":null,"subject":"[PATCH 0/6] Towards a generalized sequencer","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:39Z","receivedAt":"2011-08-11T18:51:39Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nI've prepared a nicer series after Jonathan's feedback on the previous\none.  No new ideas; just the same ideas implemented in a more sane\nway.  The first three patches fix some minor annoyances and don't\ndontribute much to the series in general.  The fourth patch implements\na very important idea: the ability to parse a general (action,\noperand) pair in the instruction sheet.  The fifth patch may come as a\nreal shocker; I was shocked myself, but I'm now convinced that this is\nthe right way forward.  Since it's only code movement, it should be\nvery easy to review.  The final patch solves a long-standing problem\nby introducing tighter coupling between 'git commit' and the\nsequencer.\n\nThanks for lending a ear.  Enjoy reading the rest!\n\nNote: I didn't know what to do with the license header in the fifth\npatch.  I just assumed that it was some historical cruft and removed\nit.\n\nRamkumar Ramachandra (6):\n  revert: Don't remove the sequencer state on error\n  revert: Free memory after get_message call\n  revert: Parse instruction sheet more cautiously\n  revert: Allow mixed pick and revert instructions\n  sequencer: Expose API to cherry-picking machinery\n  sequencer: Remove sequencer state after final commit\n\n builtin/commit.c                |    7 +-\n builtin/revert.c                | 1012 +--------------------------------------\n sequencer.c                     |  962 +++++++++++++++++++++++++++++++++++++-\n sequencer.h                     |   37 ++\n t/t3510-cherry-pick-sequence.sh |   91 ++++-\n 5 files changed, 1099 insertions(+), 1010 deletions(-)\n\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173302","messageId":"1313088705-32222-2-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/6] revert: Don't remove the sequencer state on error","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:40Z","receivedAt":"2011-08-11T18:51:40Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The cherry-pick/ revert machinery now removes the sequencer state when\ndo_pick_commit returns a non-zero, and when only one instruction is\nleft in the todo_list.  Since do_pick_commit has a way to distinguish\nerrors from conflicts using the signed-ness of the return value,\nutilize this to ensure that the sequencer state is only removed when\nthere's a conflict and there is only one instruction left in the\ntodo_list.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/revert.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 8b452e8..a548a14 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -929,10 +929,10 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\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\tif (!cur->next && res > 0)\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 * A conflict 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-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173303","messageId":"1313088705-32222-3-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/6] revert: Free memory after get_message call","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:41Z","receivedAt":"2011-08-11T18:51:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The format_todo function leaks memory because it forgets to call\nfree_message after get_message.  Fix this.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/revert.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex a548a14..1a4187a 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -688,6 +688,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,\n \t\t\treturn error(_(\"Cannot get commit message for %s\"), sha1_abbrev);\n \t\tstrbuf_addf(buf, \"%s %s %s\\n\", action_str, sha1_abbrev, msg.subject);\n \t}\n+\tfree_message(&msg);\n \treturn 0;\n }\n \n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173304","messageId":"1313088705-32222-4-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/6] revert: Parse instruction sheet more cautiously","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:42Z","receivedAt":"2011-08-11T18:51:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Fix a buffer overflow bug by checking that parsed SHA-1 hex will fit\nin the buffer we've created for it.  Also change the instruction sheet\nformat subtly so that a description of the commit after the object\nname is optional.  So now, an instruction sheet like this is perfectly\nvalid:\n\n  pick 35b0426\n  pick fbd5bbcbc2e\n  pick 7362160f\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/revert.c                |   20 +++++++++-----------\n t/t3510-cherry-pick-sequence.sh |   29 +++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 1a4187a..f44f749 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -697,26 +697,24 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)\n \tunsigned char commit_sha1[20];\n \tchar sha1_abbrev[40];\n \tenum replay_action action;\n-\tint insn_len = 0;\n-\tchar *p, *q;\n+\tchar *p = start, *q, *end = strchrnul(start, '\\n');\n \n \tif (!prefixcmp(start, \"pick \")) {\n \t\taction = CHERRY_PICK;\n-\t\tinsn_len = strlen(\"pick\");\n-\t\tp = start + insn_len + 1;\n+\t\tp += strlen(\"pick \");\n \t} else if (!prefixcmp(start, \"revert \")) {\n \t\taction = REVERT;\n-\t\tinsn_len = strlen(\"revert\");\n-\t\tp = start + insn_len + 1;\n+\t\tp += strlen(\"revert \");\n \t} else\n \t\treturn NULL;\n \n-\tq = strchr(p, ' ');\n-\tif (!q)\n+\tq = strchrnul(p, ' ');\n+\tif (q > end)\n+\t\tq = end;\n+\tif (q - p + 1 > sizeof(sha1_abbrev))\n \t\treturn NULL;\n-\tq++;\n-\n-\tstrlcpy(sha1_abbrev, p, q - p);\n+\tmemcpy(sha1_abbrev, p, q - p);\n+\tsha1_abbrev[q - p] = '\\0';\n \n \t/*\n \t * Verify that the action matches up with the one in\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 3bca2b3..bc5f0b8 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -211,4 +211,33 @@ test_expect_success 'malformed instruction sheet 2' '\n \ttest_must_fail git cherry-pick --continue\n '\n \n+test_expect_success 'missing commit descriptions in instruction sheet' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo \"c\" >foo &&\n+\tgit add foo &&\n+\tgit commit &&\n+\tcut -d\" \" -f1,2 .git/sequencer/todo >new_sheet &&\n+\tcp new_sheet .git/sequencer/todo &&\n+\tgit cherry-pick --continue &&\n+\ttest_path_is_missing .git/sequencer &&\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+\tcat >expect <<-\\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_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173305","messageId":"1313088705-32222-5-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/6] revert: Allow mixed pick and revert instructions","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:43Z","receivedAt":"2011-08-11T18:51:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Change the way the instruction parser works, allowing arbitrary\n(action, operand) pairs to be parsed.  So now, you can do:\n\n  pick fdc0b12 picked\n  revert 965fed4 anotherpick\n\nFor cherry-pick and revert, this means that a 'git cherry-pick\n--continue' can continue an ongoing revert operation and viceversa.\nThis patch lays the foundation for extending the parser to support\nmore actions so 'git rebase -i' can reuse this machinery in the\nfuture.\n\nHelped-by: Jonathan Nieder <jrnider@gmail.com>\nAcked-by: Jonathan Nieder <jrnider@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/revert.c                |  128 ++++++++++++++++++--------------------\n sequencer.h                     |    8 +++\n t/t3510-cherry-pick-sequence.sh |   58 ++++++++++++++++++\n 3 files changed, 127 insertions(+), 67 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex f44f749..483c957 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -39,7 +39,6 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-enum replay_action { REVERT, CHERRY_PICK };\n enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };\n \n struct replay_opts {\n@@ -68,14 +67,14 @@ struct replay_opts {\n \n static const char *action_name(const struct replay_opts *opts)\n {\n-\treturn opts->action == REVERT ? \"revert\" : \"cherry-pick\";\n+\treturn opts->action == REPLAY_REVERT ? \"revert\" : \"cherry-pick\";\n }\n \n static char *get_encoding(const char *message);\n \n static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)\n {\n-\treturn opts->action == REVERT ? revert_usage : cherry_pick_usage;\n+\treturn opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;\n }\n \n static int option_parse_x(const struct option *opt,\n@@ -154,7 +153,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_END(),\n \t};\n \n-\tif (opts->action == CHERRY_PICK) {\n+\tif (opts->action == REPLAY_PICK) {\n \t\tstruct option cp_extra[] = {\n \t\t\tOPT_BOOLEAN('x', NULL, &opts->record_origin, \"append commit name\"),\n \t\t\tOPT_BOOLEAN(0, \"ff\", &opts->allow_ff, \"allow fast-forward\"),\n@@ -353,7 +352,7 @@ static int error_dirty_index(struct replay_opts *opts)\n \t\treturn error_resolve_conflict(action_name(opts));\n \n \t/* Different translation strings for cherry-pick and revert */\n-\tif (opts->action == CHERRY_PICK)\n+\tif (opts->action == REPLAY_PICK)\n \t\terror(_(\"Your local changes would be overwritten by cherry-pick.\"));\n \telse\n \t\terror(_(\"Your local changes would be overwritten by revert.\"));\n@@ -457,7 +456,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)\n \treturn run_command_v_opt(args, RUN_GIT_CMD);\n }\n \n-static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n+static int do_pick_commit(struct commit *commit, enum replay_action action,\n+\t\t\tstruct replay_opts *opts)\n {\n \tunsigned char head[20];\n \tstruct commit *base, *next, *parent;\n@@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t/* TRANSLATORS: The first %s will be \"revert\" or\n \t\t   \"cherry-pick\", the second %s a SHA1 */\n \t\treturn error(_(\"%s: cannot parse parent commit %s\"),\n-\t\t\taction_name(opts), sha1_to_hex(parent->object.sha1));\n+\t\t\taction == REPLAY_REVERT ? \"revert\" : \"cherry-pick\",\n+\t\t\tsha1_to_hex(parent->object.sha1));\n \n \tif (get_message(commit, &msg) != 0)\n \t\treturn error(_(\"Cannot get commit message for %s\"),\n@@ -532,7 +533,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \n \tdefmsg = git_pathdup(\"MERGE_MSG\");\n \n-\tif (opts->action == REVERT) {\n+\tif (action == REPLAY_REVERT) {\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n@@ -575,7 +576,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t\twrite_cherry_pick_head(commit);\n \t}\n \n-\tif (!opts->strategy || !strcmp(opts->strategy, \"recursive\") || opts->action == REVERT) {\n+\tif (!opts->strategy || !strcmp(opts->strategy, \"recursive\") || action == REPLAY_REVERT) {\n \t\tres = do_recursive_merge(base, next, base_label, next_label,\n \t\t\t\t\t head, &msgbuf, opts);\n \t\twrite_message(&msgbuf, defmsg);\n@@ -594,7 +595,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t}\n \n \tif (res) {\n-\t\terror(opts->action == REVERT\n+\t\terror(action == REPLAY_REVERT\n \t\t      ? _(\"could not revert %s... %s\")\n \t\t      : _(\"could not apply %s... %s\"),\n \t\t      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n@@ -618,7 +619,7 @@ static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)\n \n \tinit_revisions(revs, NULL);\n \trevs->no_walk = 1;\n-\tif (opts->action != REVERT)\n+\tif (opts->action != REPLAY_REVERT)\n \t\trevs->reverse = 1;\n \n \targc = setup_revisions(opts->commit_argc, opts->commit_argv, revs, NULL);\n@@ -664,88 +665,81 @@ static void read_and_refresh_cache(struct replay_opts *opts)\n  *     assert(commit_list_count(list) == 2);\n  *     return list;\n  */\n-struct commit_list **commit_list_append(struct commit *commit,\n-\t\t\t\t\tstruct commit_list **next)\n+struct replay_insn_list **replay_insn_list_append(enum replay_action action,\n+\t\t\t\t\t\tstruct commit *operand,\n+\t\t\t\t\t\tstruct replay_insn_list **next)\n {\n-\tstruct commit_list *new = xmalloc(sizeof(struct commit_list));\n-\tnew->item = commit;\n+\tstruct replay_insn_list *new = xmalloc(sizeof(*new));\n+\tnew->action = action;\n+\tnew->operand = operand;\n \t*next = new;\n \tnew->next = NULL;\n \treturn &new->next;\n }\n \n-static int format_todo(struct strbuf *buf, struct commit_list *todo_list,\n-\t\tstruct replay_opts *opts)\n+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)\n {\n-\tstruct commit_list *cur = NULL;\n-\tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n-\tconst char *sha1_abbrev = NULL;\n-\tconst char *action_str = opts->action == REVERT ? \"revert\" : \"pick\";\n+\tstruct replay_insn_list *cur;\n \n \tfor (cur = todo_list; cur; cur = cur->next) {\n-\t\tsha1_abbrev = find_unique_abbrev(cur->item->object.sha1, DEFAULT_ABBREV);\n-\t\tif (get_message(cur->item, &msg))\n+\t\tconst char *sha1_abbrev, *action_str;\n+\t\tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n+\n+\t\taction_str = cur->action == REPLAY_REVERT ? \"revert\" : \"pick\";\n+\t\tsha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);\n+\t\tif (get_message(cur->operand, &msg))\n \t\t\treturn error(_(\"Cannot get commit message for %s\"), sha1_abbrev);\n \t\tstrbuf_addf(buf, \"%s %s %s\\n\", action_str, sha1_abbrev, msg.subject);\n+\t\tfree_message(&msg);\n \t}\n-\tfree_message(&msg);\n \treturn 0;\n }\n \n-static struct commit *parse_insn_line(char *start, struct replay_opts *opts)\n+static int parse_insn_line(char *start, struct replay_insn_list *item)\n {\n \tunsigned char commit_sha1[20];\n \tchar sha1_abbrev[40];\n-\tenum replay_action action;\n \tchar *p = start, *q, *end = strchrnul(start, '\\n');\n \n \tif (!prefixcmp(start, \"pick \")) {\n-\t\taction = CHERRY_PICK;\n+\t\titem->action = REPLAY_PICK;\n \t\tp += strlen(\"pick \");\n \t} else if (!prefixcmp(start, \"revert \")) {\n-\t\taction = REVERT;\n+\t\titem->action = REPLAY_REVERT;\n \t\tp += strlen(\"revert \");\n \t} else\n-\t\treturn NULL;\n+\t\treturn error(_(\"Unrecognized action: %s\"), start);\n \n \tq = strchrnul(p, ' ');\n \tif (q > end)\n \t\tq = end;\n \tif (q - p + 1 > sizeof(sha1_abbrev))\n-\t\treturn NULL;\n+\t\treturn error(_(\"Object name too large: %s\"), p);\n \tmemcpy(sha1_abbrev, p, q - p);\n \tsha1_abbrev[q - p] = '\\0';\n \n-\t/*\n-\t * Verify that the action matches up with the one in\n-\t * opts; we don't support arbitrary instructions\n-\t */\n-\tif (action != opts->action) {\n-\t\tconst char *action_str;\n-\t\taction_str = action == REVERT ? \"revert\" : \"cherry-pick\";\n-\t\terror(_(\"Cannot %s during a %s\"), action_str, action_name(opts));\n-\t\treturn NULL;\n-\t}\n-\n \tif (get_sha1(sha1_abbrev, commit_sha1) < 0)\n-\t\treturn NULL;\n+\t\treturn error(_(\"Malformed object name: %s\"), sha1_abbrev);\n+\n+\titem->operand = lookup_commit_reference(commit_sha1);\n+\tif (!item->operand)\n+\t\treturn error(_(\"Not a valid commit: %s\"), sha1_abbrev);\n \n-\treturn lookup_commit_reference(commit_sha1);\n+\titem->next = NULL;\n+\treturn 0;\n }\n \n-static int parse_insn_buffer(char *buf, struct commit_list **todo_list,\n-\t\t\tstruct replay_opts *opts)\n+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)\n {\n-\tstruct commit_list **next = todo_list;\n-\tstruct commit *commit;\n+\tstruct replay_insn_list **next = todo_list;\n+\tstruct replay_insn_list item = {0, NULL, NULL};\n \tchar *p = buf;\n \tint i;\n \n \tfor (i = 1; *p; i++) {\n-\t\tcommit = parse_insn_line(p, opts);\n-\t\tif (!commit)\n+\t\tif (parse_insn_line(p, &item) < 0)\n \t\t\treturn error(_(\"Could not parse line %d.\"), i);\n-\t\tnext = commit_list_append(commit, next);\n+\t\tnext = replay_insn_list_append(item.action, item.operand, next);\n \t\tp = strchrnul(p, '\\n');\n \t\tif (*p)\n \t\t\tp++;\n@@ -755,8 +749,7 @@ static int parse_insn_buffer(char *buf, struct commit_list **todo_list,\n \treturn 0;\n }\n \n-static void read_populate_todo(struct commit_list **todo_list,\n-\t\t\tstruct replay_opts *opts)\n+static void read_populate_todo(struct replay_insn_list **todo_list)\n {\n \tconst char *todo_file = git_path(SEQ_TODO_FILE);\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -772,7 +765,7 @@ static void read_populate_todo(struct commit_list **todo_list,\n \t}\n \tclose(fd);\n \n-\tres = parse_insn_buffer(buf.buf, todo_list, opts);\n+\tres = parse_insn_buffer(buf.buf, todo_list);\n \tstrbuf_release(&buf);\n \tif (res)\n \t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n@@ -821,18 +814,18 @@ static void read_populate_opts(struct replay_opts **opts_ptr)\n \t\tdie(_(\"Malformed options sheet: %s\"), opts_file);\n }\n \n-static void walk_revs_populate_todo(struct commit_list **todo_list,\n+static void walk_revs_populate_todo(struct replay_insn_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+\tstruct replay_insn_list **next;\n \n \tprepare_revs(&revs, opts);\n \n \tnext = todo_list;\n \twhile ((commit = get_revision(&revs)))\n-\t\tnext = commit_list_append(commit, next);\n+\t\tnext = replay_insn_list_append(opts->action, commit, next);\n }\n \n static int create_seq_dir(void)\n@@ -861,7 +854,7 @@ static void save_head(const char *head)\n \t\tdie(_(\"Error wrapping up %s.\"), head_file);\n }\n \n-static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n+static void save_todo(struct replay_insn_list *todo_list)\n {\n \tconst char *todo_file = git_path(SEQ_TODO_FILE);\n \tstatic struct lock_file todo_lock;\n@@ -869,7 +862,7 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)\n \tint fd;\n \n \tfd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);\n-\tif (format_todo(&buf, todo_list, opts) < 0)\n+\tif (format_todo(&buf, todo_list) < 0)\n \t\tdie(_(\"Could not format %s.\"), todo_file);\n \tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n \t\tstrbuf_release(&buf);\n@@ -913,9 +906,10 @@ static void save_opts(struct replay_opts *opts)\n \t}\n }\n \n-static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n+static int pick_commits(struct replay_insn_list *todo_list,\n+\t\t\tstruct replay_opts *opts)\n {\n-\tstruct commit_list *cur;\n+\tstruct replay_insn_list *cur;\n \tint res;\n \n \tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n@@ -925,8 +919,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \tread_and_refresh_cache(opts);\n \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\tsave_todo(cur);\n+\t\tres = do_pick_commit(cur->operand, cur->action, opts);\n \t\tif (res) {\n \t\t\tif (!cur->next && res > 0)\n \t\t\t\t/*\n@@ -951,7 +945,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)\n \n static int pick_revisions(struct replay_opts *opts)\n {\n-\tstruct commit_list *todo_list = NULL;\n+\tstruct replay_insn_list *todo_list = NULL;\n \tunsigned char sha1[20];\n \n \tread_and_refresh_cache(opts);\n@@ -968,7 +962,7 @@ static int pick_revisions(struct replay_opts *opts)\n \t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n \t\t\tgoto error;\n \t\tread_populate_opts(&opts);\n-\t\tread_populate_todo(&todo_list, opts);\n+\t\tread_populate_todo(&todo_list);\n \n \t\t/* Verify that the conflict has been resolved */\n \t\tif (!index_differs_from(\"HEAD\", 0))\n@@ -988,7 +982,7 @@ static int pick_revisions(struct replay_opts *opts)\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\tif (opts->action == REPLAY_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@@ -1008,7 +1002,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n \tmemset(&opts, 0, sizeof(opts));\n \tif (isatty(0))\n \t\topts.edit = 1;\n-\topts.action = REVERT;\n+\topts.action = REPLAY_REVERT;\n \tgit_config(git_default_config, NULL);\n \tparse_args(argc, argv, &opts);\n \tres = pick_revisions(&opts);\n@@ -1023,7 +1017,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n \tint res;\n \n \tmemset(&opts, 0, sizeof(opts));\n-\topts.action = CHERRY_PICK;\n+\topts.action = REPLAY_PICK;\n \tgit_config(git_default_config, NULL);\n \tparse_args(argc, argv, &opts);\n \tres = pick_revisions(&opts);\ndiff --git a/sequencer.h b/sequencer.h\nindex 905d295..f4db257 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -7,6 +7,14 @@\n #define SEQ_TODO_FILE\t\"sequencer/todo\"\n #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n \n+enum replay_action { REPLAY_REVERT, REPLAY_PICK };\n+\n+struct replay_insn_list {\n+\tenum replay_action action;\n+\tstruct commit *operand;\n+\tstruct replay_insn_list *next;\n+};\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'.\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex bc5f0b8..bc7fb13 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -240,4 +240,62 @@ test_expect_success 'missing commit descriptions in instruction sheet' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'revert --continue continues after cherry-pick' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo \"c\" >foo &&\n+\tgit add foo &&\n+\tgit commit &&\n+\tgit revert --continue &&\n+\ttest_path_is_missing .git/sequencer &&\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+\tcat >expect <<-\\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_cmp expect actual\n+'\n+\n+test_expect_success 'mixed pick and revert instructions' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick base..anotherpick &&\n+\techo \"c\" >foo &&\n+\tgit add foo &&\n+\tgit commit &&\n+\toldsha=`git rev-parse --short HEAD~1` &&\n+\techo \"revert $oldsha unrelatedpick\" >>.git/sequencer/todo &&\n+\tgit cherry-pick --continue &&\n+\ttest_path_is_missing .git/sequencer &&\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+\tcat >expect <<-\\EOF &&\n+\tOBJID\n+\t:100644 100644 OBJID OBJID M\tunrelated\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_cmp expect actual\n+'\n+\n test_done\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173307","messageId":"1313088705-32222-6-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:44Z","receivedAt":"2011-08-11T18:51:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Move code from builtin/revert.c to sequencer.c and expose a public API\nwithout making any functional changes.  Although it is useful only to\nexisting callers of cherry-pick and revert now, this patch lays the\nfoundation for future expansion.\n\nHelped-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/revert.c | 1001 +-----------------------------------------------------\n sequencer.c      |  958 +++++++++++++++++++++++++++++++++++++++++++++++++++-\n sequencer.h      |   28 ++\n 3 files changed, 989 insertions(+), 998 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 483c957..fc818fd 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -1,999 +1,6 @@\n #include \"cache.h\"\n-#include \"builtin.h\"\n-#include \"object.h\"\n-#include \"commit.h\"\n-#include \"tag.h\"\n-#include \"run-command.h\"\n-#include \"exec_cmd.h\"\n-#include \"utf8.h\"\n-#include \"parse-options.h\"\n-#include \"cache-tree.h\"\n-#include \"diff.h\"\n-#include \"revision.h\"\n-#include \"rerere.h\"\n-#include \"merge-recursive.h\"\n-#include \"refs.h\"\n-#include \"dir.h\"\n #include \"sequencer.h\"\n \n-/*\n- * This implements the builtins revert and cherry-pick.\n- *\n- * Copyright (c) 2007 Johannes E. Schindelin\n- *\n- * Based on git-revert.sh, which is\n- *\n- * Copyright (c) 2005 Linus Torvalds\n- * Copyright (c) 2005 Junio C Hamano\n- */\n-\n-static const char * const revert_usage[] = {\n-\t\"git revert [options] <commit-ish>\",\n-\t\"git revert <subcommand>\",\n-\tNULL\n-};\n-\n-static const char * const cherry_pick_usage[] = {\n-\t\"git cherry-pick [options] <commit-ish>\",\n-\t\"git cherry-pick <subcommand>\",\n-\tNULL\n-};\n-\n-enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };\n-\n-struct replay_opts {\n-\tenum replay_action action;\n-\tenum replay_subcommand subcommand;\n-\n-\t/* Boolean options */\n-\tint edit;\n-\tint record_origin;\n-\tint no_commit;\n-\tint signoff;\n-\tint allow_ff;\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-\n-#define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n-\n-static const char *action_name(const struct replay_opts *opts)\n-{\n-\treturn opts->action == REPLAY_REVERT ? \"revert\" : \"cherry-pick\";\n-}\n-\n-static char *get_encoding(const char *message);\n-\n-static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)\n-{\n-\treturn opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;\n-}\n-\n-static int option_parse_x(const struct option *opt,\n-\t\t\t  const char *arg, int unset)\n-{\n-\tstruct replay_opts **opts_ptr = opt->value;\n-\tstruct replay_opts *opts = *opts_ptr;\n-\n-\tif (unset)\n-\t\treturn 0;\n-\n-\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n-\topts->xopts[opts->xopts_nr++] = xstrdup(arg);\n-\treturn 0;\n-}\n-\n-static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n-{\n-\tconst char *this_opt;\n-\tva_list ap;\n-\n-\tva_start(ap, base_opt);\n-\twhile ((this_opt = va_arg(ap, const char *))) {\n-\t\tif (va_arg(ap, int))\n-\t\t\tbreak;\n-\t}\n-\tva_end(ap);\n-\n-\tif (this_opt)\n-\t\tdie(_(\"%s: %s cannot be used with %s\"), me, this_opt, base_opt);\n-}\n-\n-static void verify_opt_mutually_compatible(const char *me, ...)\n-{\n-\tconst char *opt1, *opt2;\n-\tva_list ap;\n-\n-\tva_start(ap, me);\n-\twhile ((opt1 = va_arg(ap, const char *))) {\n-\t\tif (va_arg(ap, int))\n-\t\t\tbreak;\n-\t}\n-\tif (opt1) {\n-\t\twhile ((opt2 = va_arg(ap, const char *))) {\n-\t\t\tif (va_arg(ap, int))\n-\t\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tif (opt1 && opt2)\n-\t\tdie(_(\"%s: %s cannot be used with %s\"),\tme, opt1, opt2);\n-}\n-\n-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 noop;\n-\tint reset = 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('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n-\t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n-\t\t{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, \"no-op (backward compatibility)\",\n-\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 0 },\n-\t\tOPT_BOOLEAN('s', \"signoff\", &opts->signoff, \"add Signed-off-by:\"),\n-\t\tOPT_INTEGER('m', \"mainline\", &opts->mainline, \"parent number\"),\n-\t\tOPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),\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\tOPT_END(),\n-\t\tOPT_END(),\n-\t\tOPT_END(),\n-\t};\n-\n-\tif (opts->action == REPLAY_PICK) {\n-\t\tstruct option cp_extra[] = {\n-\t\t\tOPT_BOOLEAN('x', NULL, &opts->record_origin, \"append commit name\"),\n-\t\t\tOPT_BOOLEAN(0, \"ff\", &opts->allow_ff, \"allow fast-forward\"),\n-\t\t\tOPT_END(),\n-\t\t};\n-\t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\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-\n-\t/* Check for incompatible subcommands */\n-\tverify_opt_mutually_compatible(me,\n-\t\t\t\t\"--reset\", reset,\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-\telse if (contin)\n-\t\topts->subcommand = REPLAY_CONTINUE;\n-\telse\n-\t\topts->subcommand = REPLAY_NONE;\n-\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\telse\n-\t\t\tthis_operation = \"--continue\";\n-\n-\t\tverify_opt_compatible(me, this_operation,\n-\t\t\t\t\"--no-commit\", opts->no_commit,\n-\t\t\t\t\"--signoff\", opts->signoff,\n-\t\t\t\t\"--mainline\", opts->mainline,\n-\t\t\t\t\"--strategy\", opts->strategy ? 1 : 0,\n-\t\t\t\t\"--strategy-option\", opts->xopts ? 1 : 0,\n-\t\t\t\t\"-x\", opts->record_origin,\n-\t\t\t\t\"--ff\", opts->allow_ff,\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-\t\t\t\t\"--no-commit\", opts->no_commit,\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-\n-struct commit_message {\n-\tchar *parent_label;\n-\tconst char *label;\n-\tconst char *subject;\n-\tchar *reencoded_message;\n-\tconst char *message;\n-};\n-\n-static int get_message(struct commit *commit, struct commit_message *out)\n-{\n-\tconst char *encoding;\n-\tconst char *abbrev, *subject;\n-\tint abbrev_len, subject_len;\n-\tchar *q;\n-\n-\tif (!commit->buffer)\n-\t\treturn -1;\n-\tencoding = get_encoding(commit->buffer);\n-\tif (!encoding)\n-\t\tencoding = \"UTF-8\";\n-\tif (!git_commit_encoding)\n-\t\tgit_commit_encoding = \"UTF-8\";\n-\n-\tout->reencoded_message = NULL;\n-\tout->message = commit->buffer;\n-\tif (strcmp(encoding, git_commit_encoding))\n-\t\tout->reencoded_message = reencode_string(commit->buffer,\n-\t\t\t\t\tgit_commit_encoding, encoding);\n-\tif (out->reencoded_message)\n-\t\tout->message = out->reencoded_message;\n-\n-\tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n-\tabbrev_len = strlen(abbrev);\n-\n-\tsubject_len = find_commit_subject(out->message, &subject);\n-\n-\tout->parent_label = xmalloc(strlen(\"parent of \") + abbrev_len +\n-\t\t\t      strlen(\"... \") + subject_len + 1);\n-\tq = out->parent_label;\n-\tq = mempcpy(q, \"parent of \", strlen(\"parent of \"));\n-\tout->label = q;\n-\tq = mempcpy(q, abbrev, abbrev_len);\n-\tq = mempcpy(q, \"... \", strlen(\"... \"));\n-\tout->subject = q;\n-\tq = mempcpy(q, subject, subject_len);\n-\t*q = '\\0';\n-\treturn 0;\n-}\n-\n-static void free_message(struct commit_message *msg)\n-{\n-\tfree(msg->parent_label);\n-\tfree(msg->reencoded_message);\n-}\n-\n-static char *get_encoding(const char *message)\n-{\n-\tconst char *p = message, *eol;\n-\n-\twhile (*p && *p != '\\n') {\n-\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n-\t\t\t; /* do nothing */\n-\t\tif (!prefixcmp(p, \"encoding \")) {\n-\t\t\tchar *result = xmalloc(eol - 8 - p);\n-\t\t\tstrlcpy(result, p + 9, eol - 8 - p);\n-\t\t\treturn result;\n-\t\t}\n-\t\tp = eol;\n-\t\tif (*p == '\\n')\n-\t\t\tp++;\n-\t}\n-\treturn NULL;\n-}\n-\n-static void write_cherry_pick_head(struct commit *commit)\n-{\n-\tint fd;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\n-\tstrbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n-\n-\tfd = open(git_path(\"CHERRY_PICK_HEAD\"), O_WRONLY | O_CREAT, 0666);\n-\tif (fd < 0)\n-\t\tdie_errno(_(\"Could not open '%s' for writing\"),\n-\t\t\t  git_path(\"CHERRY_PICK_HEAD\"));\n-\tif (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))\n-\t\tdie_errno(_(\"Could not write to '%s'\"), git_path(\"CHERRY_PICK_HEAD\"));\n-\tstrbuf_release(&buf);\n-}\n-\n-static void print_advice(void)\n-{\n-\tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n-\n-\tif (msg) {\n-\t\tfprintf(stderr, \"%s\\n\", msg);\n-\t\t/*\n-\t\t * A conflict has occured but the porcelain\n-\t\t * (typically rebase --interactive) wants to take care\n-\t\t * of the commit itself so remove CHERRY_PICK_HEAD\n-\t\t */\n-\t\tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n-\t\treturn;\n-\t}\n-\n-\tadvise(\"after resolving the conflicts, mark the corrected paths\");\n-\tadvise(\"with 'git add <paths>' or 'git rm <paths>'\");\n-\tadvise(\"and commit the result with 'git commit'\");\n-}\n-\n-static void write_message(struct strbuf *msgbuf, const char *filename)\n-{\n-\tstatic struct lock_file msg_file;\n-\n-\tint msg_fd = hold_lock_file_for_update(&msg_file, filename,\n-\t\t\t\t\t       LOCK_DIE_ON_ERROR);\n-\tif (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)\n-\t\tdie_errno(_(\"Could not write to %s.\"), filename);\n-\tstrbuf_release(msgbuf);\n-\tif (commit_lock_file(&msg_file) < 0)\n-\t\tdie(_(\"Error wrapping up %s\"), filename);\n-}\n-\n-static struct tree *empty_tree(void)\n-{\n-\tstruct tree *tree = xcalloc(1, sizeof(struct tree));\n-\n-\ttree->object.parsed = 1;\n-\ttree->object.type = OBJ_TREE;\n-\tpretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);\n-\treturn tree;\n-}\n-\n-static int error_dirty_index(struct replay_opts *opts)\n-{\n-\tif (read_cache_unmerged())\n-\t\treturn error_resolve_conflict(action_name(opts));\n-\n-\t/* Different translation strings for cherry-pick and revert */\n-\tif (opts->action == REPLAY_PICK)\n-\t\terror(_(\"Your local changes would be overwritten by cherry-pick.\"));\n-\telse\n-\t\terror(_(\"Your local changes would be overwritten by revert.\"));\n-\n-\tif (advice_commit_before_merge)\n-\t\tadvise(_(\"Commit your changes or stash them to proceed.\"));\n-\treturn -1;\n-}\n-\n-static int fast_forward_to(const unsigned char *to, const unsigned char *from)\n-{\n-\tstruct ref_lock *ref_lock;\n-\n-\tread_cache();\n-\tif (checkout_fast_forward(from, to))\n-\t\texit(1); /* the callee should have complained already */\n-\tref_lock = lock_any_ref_for_update(\"HEAD\", from, 0);\n-\treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n-}\n-\n-static int do_recursive_merge(struct commit *base, struct commit *next,\n-\t\t\t      const char *base_label, const char *next_label,\n-\t\t\t      unsigned char *head, struct strbuf *msgbuf,\n-\t\t\t      struct replay_opts *opts)\n-{\n-\tstruct merge_options o;\n-\tstruct tree *result, *next_tree, *base_tree, *head_tree;\n-\tint clean, index_fd;\n-\tconst char **xopt;\n-\tstatic struct lock_file index_lock;\n-\n-\tindex_fd = hold_locked_index(&index_lock, 1);\n-\n-\tread_cache();\n-\n-\tinit_merge_options(&o);\n-\to.ancestor = base ? base_label : \"(empty tree)\";\n-\to.branch1 = \"HEAD\";\n-\to.branch2 = next ? next_label : \"(empty tree)\";\n-\n-\thead_tree = parse_tree_indirect(head);\n-\tnext_tree = next ? next->tree : empty_tree();\n-\tbase_tree = base ? base->tree : empty_tree();\n-\n-\tfor (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)\n-\t\tparse_merge_opt(&o, *xopt);\n-\n-\tclean = merge_trees(&o,\n-\t\t\t    head_tree,\n-\t\t\t    next_tree, base_tree, &result);\n-\n-\tif (active_cache_changed &&\n-\t    (write_cache(index_fd, active_cache, active_nr) ||\n-\t     commit_locked_index(&index_lock)))\n-\t\t/* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n-\t\tdie(_(\"%s: Unable to write new index file\"), action_name(opts));\n-\trollback_lock_file(&index_lock);\n-\n-\tif (!clean) {\n-\t\tint i;\n-\t\tstrbuf_addstr(msgbuf, \"\\nConflicts:\\n\\n\");\n-\t\tfor (i = 0; i < active_nr;) {\n-\t\t\tstruct cache_entry *ce = active_cache[i++];\n-\t\t\tif (ce_stage(ce)) {\n-\t\t\t\tstrbuf_addch(msgbuf, '\\t');\n-\t\t\t\tstrbuf_addstr(msgbuf, ce->name);\n-\t\t\t\tstrbuf_addch(msgbuf, '\\n');\n-\t\t\t\twhile (i < active_nr && !strcmp(ce->name,\n-\t\t\t\t\t\tactive_cache[i]->name))\n-\t\t\t\t\ti++;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\treturn !clean;\n-}\n-\n-/*\n- * If we are cherry-pick, and if the merge did not result in\n- * hand-editing, we will hit this commit and inherit the original\n- * author date and name.\n- * If we are revert, or if our cherry-pick results in a hand merge,\n- * we had better say that the current user is responsible for that.\n- */\n-static int run_git_commit(const char *defmsg, struct replay_opts *opts)\n-{\n-\t/* 6 is max possible length of our args array including NULL */\n-\tconst char *args[6];\n-\tint i = 0;\n-\n-\targs[i++] = \"commit\";\n-\targs[i++] = \"-n\";\n-\tif (opts->signoff)\n-\t\targs[i++] = \"-s\";\n-\tif (!opts->edit) {\n-\t\targs[i++] = \"-F\";\n-\t\targs[i++] = defmsg;\n-\t}\n-\targs[i] = NULL;\n-\n-\treturn run_command_v_opt(args, RUN_GIT_CMD);\n-}\n-\n-static int do_pick_commit(struct commit *commit, enum replay_action action,\n-\t\t\tstruct replay_opts *opts)\n-{\n-\tunsigned char head[20];\n-\tstruct commit *base, *next, *parent;\n-\tconst char *base_label, *next_label;\n-\tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n-\tchar *defmsg = NULL;\n-\tstruct strbuf msgbuf = STRBUF_INIT;\n-\tint res;\n-\n-\tif (opts->no_commit) {\n-\t\t/*\n-\t\t * We do not intend to commit immediately.  We just want to\n-\t\t * merge the differences in, so let's compute the tree\n-\t\t * that represents the \"current\" state for merge-recursive\n-\t\t * to work on.\n-\t\t */\n-\t\tif (write_cache_as_tree(head, 0, NULL))\n-\t\t\tdie (_(\"Your index file is unmerged.\"));\n-\t} else {\n-\t\tif (get_sha1(\"HEAD\", head))\n-\t\t\treturn error(_(\"You do not have a valid HEAD\"));\n-\t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\treturn error_dirty_index(opts);\n-\t}\n-\tdiscard_cache();\n-\n-\tif (!commit->parents) {\n-\t\tparent = NULL;\n-\t}\n-\telse if (commit->parents->next) {\n-\t\t/* Reverting or cherry-picking a merge commit */\n-\t\tint cnt;\n-\t\tstruct commit_list *p;\n-\n-\t\tif (!opts->mainline)\n-\t\t\treturn error(_(\"Commit %s is a merge but no -m option was given.\"),\n-\t\t\t\tsha1_to_hex(commit->object.sha1));\n-\n-\t\tfor (cnt = 1, p = commit->parents;\n-\t\t     cnt != opts->mainline && p;\n-\t\t     cnt++)\n-\t\t\tp = p->next;\n-\t\tif (cnt != opts->mainline || !p)\n-\t\t\treturn error(_(\"Commit %s does not have parent %d\"),\n-\t\t\t\tsha1_to_hex(commit->object.sha1), opts->mainline);\n-\t\tparent = p->item;\n-\t} else if (0 < opts->mainline)\n-\t\treturn error(_(\"Mainline was specified but commit %s is not a merge.\"),\n-\t\t\tsha1_to_hex(commit->object.sha1));\n-\telse\n-\t\tparent = commit->parents->item;\n-\n-\tif (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))\n-\t\treturn fast_forward_to(commit->object.sha1, head);\n-\n-\tif (parent && parse_commit(parent) < 0)\n-\t\t/* TRANSLATORS: The first %s will be \"revert\" or\n-\t\t   \"cherry-pick\", the second %s a SHA1 */\n-\t\treturn error(_(\"%s: cannot parse parent commit %s\"),\n-\t\t\taction == REPLAY_REVERT ? \"revert\" : \"cherry-pick\",\n-\t\t\tsha1_to_hex(parent->object.sha1));\n-\n-\tif (get_message(commit, &msg) != 0)\n-\t\treturn error(_(\"Cannot get commit message for %s\"),\n-\t\t\tsha1_to_hex(commit->object.sha1));\n-\n-\t/*\n-\t * \"commit\" is an existing commit.  We would want to apply\n-\t * the difference it introduces since its first parent \"prev\"\n-\t * on top of the current HEAD if we are cherry-pick.  Or the\n-\t * reverse of it if we are revert.\n-\t */\n-\n-\tdefmsg = git_pathdup(\"MERGE_MSG\");\n-\n-\tif (action == REPLAY_REVERT) {\n-\t\tbase = commit;\n-\t\tbase_label = msg.label;\n-\t\tnext = parent;\n-\t\tnext_label = msg.parent_label;\n-\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n-\t\tstrbuf_addstr(&msgbuf, msg.subject);\n-\t\tstrbuf_addstr(&msgbuf, \"\\\"\\n\\nThis reverts commit \");\n-\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n-\n-\t\tif (commit->parents && commit->parents->next) {\n-\t\t\tstrbuf_addstr(&msgbuf, \", reversing\\nchanges made to \");\n-\t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));\n-\t\t}\n-\t\tstrbuf_addstr(&msgbuf, \".\\n\");\n-\t} else {\n-\t\tconst char *p;\n-\n-\t\tbase = parent;\n-\t\tbase_label = msg.parent_label;\n-\t\tnext = commit;\n-\t\tnext_label = msg.label;\n-\n-\t\t/*\n-\t\t * Append the commit log message to msgbuf; it starts\n-\t\t * after the tree, parent, author, committer\n-\t\t * information followed by \"\\n\\n\".\n-\t\t */\n-\t\tp = strstr(msg.message, \"\\n\\n\");\n-\t\tif (p) {\n-\t\t\tp += 2;\n-\t\t\tstrbuf_addstr(&msgbuf, p);\n-\t\t}\n-\n-\t\tif (opts->record_origin) {\n-\t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n-\t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n-\t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n-\t\t}\n-\t\tif (!opts->no_commit)\n-\t\t\twrite_cherry_pick_head(commit);\n-\t}\n-\n-\tif (!opts->strategy || !strcmp(opts->strategy, \"recursive\") || action == REPLAY_REVERT) {\n-\t\tres = do_recursive_merge(base, next, base_label, next_label,\n-\t\t\t\t\t head, &msgbuf, opts);\n-\t\twrite_message(&msgbuf, defmsg);\n-\t} else {\n-\t\tstruct commit_list *common = NULL;\n-\t\tstruct commit_list *remotes = NULL;\n-\n-\t\twrite_message(&msgbuf, defmsg);\n-\n-\t\tcommit_list_insert(base, &common);\n-\t\tcommit_list_insert(next, &remotes);\n-\t\tres = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,\n-\t\t\t\t\tcommon, sha1_to_hex(head), remotes);\n-\t\tfree_commit_list(common);\n-\t\tfree_commit_list(remotes);\n-\t}\n-\n-\tif (res) {\n-\t\terror(action == REPLAY_REVERT\n-\t\t      ? _(\"could not revert %s... %s\")\n-\t\t      : _(\"could not apply %s... %s\"),\n-\t\t      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n-\t\t      msg.subject);\n-\t\tprint_advice();\n-\t\trerere(opts->allow_rerere_auto);\n-\t} else {\n-\t\tif (!opts->no_commit)\n-\t\t\tres = run_git_commit(defmsg, opts);\n-\t}\n-\n-\tfree_message(&msg);\n-\tfree(defmsg);\n-\n-\treturn res;\n-}\n-\n-static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)\n-{\n-\tint argc;\n-\n-\tinit_revisions(revs, NULL);\n-\trevs->no_walk = 1;\n-\tif (opts->action != REPLAY_REVERT)\n-\t\trevs->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-\t\tdie(_(\"revision walk setup failed\"));\n-\n-\tif (!revs->commits)\n-\t\tdie(_(\"empty commit set passed\"));\n-}\n-\n-static void read_and_refresh_cache(struct replay_opts *opts)\n-{\n-\tstatic struct lock_file index_lock;\n-\tint index_fd = hold_locked_index(&index_lock, 0);\n-\tif (read_index_preload(&the_index, NULL) < 0)\n-\t\tdie(_(\"git %s: failed to read the index\"), action_name(opts));\n-\trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n-\tif (the_index.cache_changed) {\n-\t\tif (write_index(&the_index, index_fd) ||\n-\t\t    commit_locked_index(&index_lock))\n-\t\t\tdie(_(\"git %s: failed to refresh the index\"), action_name(opts));\n-\t}\n-\trollback_lock_file(&index_lock);\n-}\n-\n-/*\n- * Append a commit to the end of the commit_list.\n- *\n- * next starts by pointing to the variable that holds the head of an\n- * empty commit_list, and is updated to point to the \"next\" field of\n- * the last item on the list as new commits are appended.\n- *\n- * Usage example:\n- *\n- *     struct commit_list *list;\n- *     struct commit_list **next = &list;\n- *\n- *     next = commit_list_append(c1, next);\n- *     next = commit_list_append(c2, next);\n- *     assert(commit_list_count(list) == 2);\n- *     return list;\n- */\n-struct replay_insn_list **replay_insn_list_append(enum replay_action action,\n-\t\t\t\t\t\tstruct commit *operand,\n-\t\t\t\t\t\tstruct replay_insn_list **next)\n-{\n-\tstruct replay_insn_list *new = xmalloc(sizeof(*new));\n-\tnew->action = action;\n-\tnew->operand = operand;\n-\t*next = new;\n-\tnew->next = NULL;\n-\treturn &new->next;\n-}\n-\n-static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)\n-{\n-\tstruct replay_insn_list *cur;\n-\n-\tfor (cur = todo_list; cur; cur = cur->next) {\n-\t\tconst char *sha1_abbrev, *action_str;\n-\t\tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n-\n-\t\taction_str = cur->action == REPLAY_REVERT ? \"revert\" : \"pick\";\n-\t\tsha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);\n-\t\tif (get_message(cur->operand, &msg))\n-\t\t\treturn error(_(\"Cannot get commit message for %s\"), sha1_abbrev);\n-\t\tstrbuf_addf(buf, \"%s %s %s\\n\", action_str, sha1_abbrev, msg.subject);\n-\t\tfree_message(&msg);\n-\t}\n-\treturn 0;\n-}\n-\n-static int parse_insn_line(char *start, struct replay_insn_list *item)\n-{\n-\tunsigned char commit_sha1[20];\n-\tchar sha1_abbrev[40];\n-\tchar *p = start, *q, *end = strchrnul(start, '\\n');\n-\n-\tif (!prefixcmp(start, \"pick \")) {\n-\t\titem->action = REPLAY_PICK;\n-\t\tp += strlen(\"pick \");\n-\t} else if (!prefixcmp(start, \"revert \")) {\n-\t\titem->action = REPLAY_REVERT;\n-\t\tp += strlen(\"revert \");\n-\t} else\n-\t\treturn error(_(\"Unrecognized action: %s\"), start);\n-\n-\tq = strchrnul(p, ' ');\n-\tif (q > end)\n-\t\tq = end;\n-\tif (q - p + 1 > sizeof(sha1_abbrev))\n-\t\treturn error(_(\"Object name too large: %s\"), p);\n-\tmemcpy(sha1_abbrev, p, q - p);\n-\tsha1_abbrev[q - p] = '\\0';\n-\n-\tif (get_sha1(sha1_abbrev, commit_sha1) < 0)\n-\t\treturn error(_(\"Malformed object name: %s\"), sha1_abbrev);\n-\n-\titem->operand = lookup_commit_reference(commit_sha1);\n-\tif (!item->operand)\n-\t\treturn error(_(\"Not a valid commit: %s\"), sha1_abbrev);\n-\n-\titem->next = NULL;\n-\treturn 0;\n-}\n-\n-static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)\n-{\n-\tstruct replay_insn_list **next = todo_list;\n-\tstruct replay_insn_list item = {0, NULL, NULL};\n-\tchar *p = buf;\n-\tint i;\n-\n-\tfor (i = 1; *p; i++) {\n-\t\tif (parse_insn_line(p, &item) < 0)\n-\t\t\treturn error(_(\"Could not parse line %d.\"), i);\n-\t\tnext = replay_insn_list_append(item.action, item.operand, next);\n-\t\tp = strchrnul(p, '\\n');\n-\t\tif (*p)\n-\t\t\tp++;\n-\t}\n-\tif (!*todo_list)\n-\t\treturn error(_(\"No commits parsed.\"));\n-\treturn 0;\n-}\n-\n-static void read_populate_todo(struct replay_insn_list **todo_list)\n-{\n-\tconst char *todo_file = git_path(SEQ_TODO_FILE);\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint fd, res;\n-\n-\tfd = open(todo_file, O_RDONLY);\n-\tif (fd < 0)\n-\t\tdie_errno(_(\"Could not open %s.\"), todo_file);\n-\tif (strbuf_read(&buf, fd, 0) < 0) {\n-\t\tclose(fd);\n-\t\tstrbuf_release(&buf);\n-\t\tdie(_(\"Could not read %s.\"), todo_file);\n-\t}\n-\tclose(fd);\n-\n-\tres = parse_insn_buffer(buf.buf, todo_list);\n-\tstrbuf_release(&buf);\n-\tif (res)\n-\t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n-}\n-\n-static int populate_opts_cb(const char *key, const char *value, void *data)\n-{\n-\tstruct replay_opts *opts = data;\n-\tint error_flag = 1;\n-\n-\tif (!value)\n-\t\terror_flag = 0;\n-\telse if (!strcmp(key, \"options.no-commit\"))\n-\t\topts->no_commit = git_config_bool_or_int(key, value, &error_flag);\n-\telse if (!strcmp(key, \"options.edit\"))\n-\t\topts->edit = git_config_bool_or_int(key, value, &error_flag);\n-\telse if (!strcmp(key, \"options.signoff\"))\n-\t\topts->signoff = git_config_bool_or_int(key, value, &error_flag);\n-\telse if (!strcmp(key, \"options.record-origin\"))\n-\t\topts->record_origin = git_config_bool_or_int(key, value, &error_flag);\n-\telse if (!strcmp(key, \"options.allow-ff\"))\n-\t\topts->allow_ff = git_config_bool_or_int(key, value, &error_flag);\n-\telse if (!strcmp(key, \"options.mainline\"))\n-\t\topts->mainline = git_config_int(key, value);\n-\telse if (!strcmp(key, \"options.strategy\"))\n-\t\tgit_config_string(&opts->strategy, key, value);\n-\telse if (!strcmp(key, \"options.strategy-option\")) {\n-\t\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n-\t\topts->xopts[opts->xopts_nr++] = xstrdup(value);\n-\t} else\n-\t\treturn error(_(\"Invalid key: %s\"), key);\n-\n-\tif (!error_flag)\n-\t\treturn error(_(\"Invalid value for %s: %s\"), key, value);\n-\n-\treturn 0;\n-}\n-\n-static void read_populate_opts(struct replay_opts **opts_ptr)\n-{\n-\tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n-\n-\tif (!file_exists(opts_file))\n-\t\treturn;\n-\tif (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)\n-\t\tdie(_(\"Malformed options sheet: %s\"), opts_file);\n-}\n-\n-static void walk_revs_populate_todo(struct replay_insn_list **todo_list,\n-\t\t\t\tstruct replay_opts *opts)\n-{\n-\tstruct rev_info revs;\n-\tstruct commit *commit;\n-\tstruct replay_insn_list **next;\n-\n-\tprepare_revs(&revs, opts);\n-\n-\tnext = todo_list;\n-\twhile ((commit = get_revision(&revs)))\n-\t\tnext = replay_insn_list_append(opts->action, commit, next);\n-}\n-\n-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-\telse if (mkdir(seq_dir, 0777) < 0)\n-\t\tdie_errno(_(\"Could not create sequencer directory '%s'.\"), seq_dir);\n-\treturn 0;\n-}\n-\n-static void save_head(const char *head)\n-{\n-\tconst char *head_file = git_path(SEQ_HEAD_FILE);\n-\tstatic struct lock_file head_lock;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint fd;\n-\n-\tfd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);\n-\tstrbuf_addf(&buf, \"%s\\n\", head);\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0)\n-\t\tdie_errno(_(\"Could not write to %s.\"), head_file);\n-\tif (commit_lock_file(&head_lock) < 0)\n-\t\tdie(_(\"Error wrapping up %s.\"), head_file);\n-}\n-\n-static void save_todo(struct replay_insn_list *todo_list)\n-{\n-\tconst char *todo_file = git_path(SEQ_TODO_FILE);\n-\tstatic struct lock_file todo_lock;\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint fd;\n-\n-\tfd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);\n-\tif (format_todo(&buf, todo_list) < 0)\n-\t\tdie(_(\"Could not format %s.\"), todo_file);\n-\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n-\t\tstrbuf_release(&buf);\n-\t\tdie_errno(_(\"Could not write to %s.\"), todo_file);\n-\t}\n-\tif (commit_lock_file(&todo_lock) < 0) {\n-\t\tstrbuf_release(&buf);\n-\t\tdie(_(\"Error wrapping up %s.\"), todo_file);\n-\t}\n-\tstrbuf_release(&buf);\n-}\n-\n-static void save_opts(struct replay_opts *opts)\n-{\n-\tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n-\n-\tif (opts->no_commit)\n-\t\tgit_config_set_in_file(opts_file, \"options.no-commit\", \"true\");\n-\tif (opts->edit)\n-\t\tgit_config_set_in_file(opts_file, \"options.edit\", \"true\");\n-\tif (opts->signoff)\n-\t\tgit_config_set_in_file(opts_file, \"options.signoff\", \"true\");\n-\tif (opts->record_origin)\n-\t\tgit_config_set_in_file(opts_file, \"options.record-origin\", \"true\");\n-\tif (opts->allow_ff)\n-\t\tgit_config_set_in_file(opts_file, \"options.allow-ff\", \"true\");\n-\tif (opts->mainline) {\n-\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tstrbuf_addf(&buf, \"%d\", opts->mainline);\n-\t\tgit_config_set_in_file(opts_file, \"options.mainline\", buf.buf);\n-\t\tstrbuf_release(&buf);\n-\t}\n-\tif (opts->strategy)\n-\t\tgit_config_set_in_file(opts_file, \"options.strategy\", opts->strategy);\n-\tif (opts->xopts) {\n-\t\tint i;\n-\t\tfor (i = 0; i < opts->xopts_nr; i++)\n-\t\t\tgit_config_set_multivar_in_file(opts_file,\n-\t\t\t\t\t\t\t\"options.strategy-option\",\n-\t\t\t\t\t\t\topts->xopts[i], \"^$\", 0);\n-\t}\n-}\n-\n-static int pick_commits(struct replay_insn_list *todo_list,\n-\t\t\tstruct replay_opts *opts)\n-{\n-\tstruct replay_insn_list *cur;\n-\tint res;\n-\n-\tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n-\tif (opts->allow_ff)\n-\t\tassert(!(opts->signoff || opts->no_commit ||\n-\t\t\t\topts->record_origin || opts->edit));\n-\tread_and_refresh_cache(opts);\n-\n-\tfor (cur = todo_list; cur; cur = cur->next) {\n-\t\tsave_todo(cur);\n-\t\tres = do_pick_commit(cur->operand, cur->action, opts);\n-\t\tif (res) {\n-\t\t\tif (!cur->next && res > 0)\n-\t\t\t\t/*\n-\t\t\t\t * A conflict 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\t\treturn res;\n-\t\t}\n-\t}\n-\n-\t/*\n-\t * Sequence of picks finished successfully; cleanup by\n-\t * removing the .git/sequencer directory\n-\t */\n-\tremove_sequencer_state(1);\n-\treturn 0;\n-}\n-\n-static int pick_revisions(struct replay_opts *opts)\n-{\n-\tstruct replay_insn_list *todo_list = NULL;\n-\tunsigned char sha1[20];\n-\n-\tread_and_refresh_cache(opts);\n-\n-\t/*\n-\t * Decide what to do depending on the arguments; a fresh\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-\t\tremove_sequencer_state(1);\n-\t\treturn 0;\n-\t} else if (opts->subcommand == REPLAY_CONTINUE) {\n-\t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n-\t\t\tgoto error;\n-\t\tread_populate_opts(&opts);\n-\t\tread_populate_todo(&todo_list);\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-\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 --reset 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 == REPLAY_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-\t}\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 {\n \tstruct replay_opts opts;\n@@ -1004,8 +11,8 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n \t\topts.edit = 1;\n \topts.action = REPLAY_REVERT;\n \tgit_config(git_default_config, NULL);\n-\tparse_args(argc, argv, &opts);\n-\tres = pick_revisions(&opts);\n+\tsequencer_parse_args(argc, argv, &opts);\n+\tres = sequencer_pick_revisions(&opts);\n \tif (res < 0)\n \t\tdie(_(\"revert failed\"));\n \treturn res;\n@@ -1019,8 +26,8 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n \tmemset(&opts, 0, sizeof(opts));\n \topts.action = REPLAY_PICK;\n \tgit_config(git_default_config, NULL);\n-\tparse_args(argc, argv, &opts);\n-\tres = pick_revisions(&opts);\n+\tsequencer_parse_args(argc, argv, &opts);\n+\tres = sequencer_pick_revisions(&opts);\n \tif (res < 0)\n \t\tdie(_(\"cherry-pick failed\"));\n \treturn res;\ndiff --git a/sequencer.c b/sequencer.c\nindex bc2c046..e72618c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1,8 +1,809 @@\n #include \"cache.h\"\n #include \"sequencer.h\"\n-#include \"strbuf.h\"\n+#include \"builtin.h\"\n+#include \"object.h\"\n+#include \"commit.h\"\n+#include \"tag.h\"\n+#include \"run-command.h\"\n+#include \"exec_cmd.h\"\n+#include \"utf8.h\"\n+#include \"parse-options.h\"\n+#include \"cache-tree.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n+#include \"rerere.h\"\n+#include \"merge-recursive.h\"\n+#include \"refs.h\"\n #include \"dir.h\"\n \n+static const char * const revert_usage[] = {\n+\t\"git revert [options] <commit-ish>\",\n+\t\"git revert <subcommand>\",\n+\tNULL\n+};\n+\n+static const char * const cherry_pick_usage[] = {\n+\t\"git cherry-pick [options] <commit-ish>\",\n+\t\"git cherry-pick <subcommand>\",\n+\tNULL\n+};\n+\n+#define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n+\n+static const char *action_name(const struct replay_opts *opts)\n+{\n+\treturn opts->action == REPLAY_REVERT ? \"revert\" : \"cherry-pick\";\n+}\n+\n+static char *get_encoding(const char *message);\n+\n+static const char * const *revert_or_cherry_pick_usage(struct replay_opts *opts)\n+{\n+\treturn opts->action == REPLAY_REVERT ? revert_usage : cherry_pick_usage;\n+}\n+\n+static int option_parse_x(const struct option *opt,\n+\t\t\t  const char *arg, int unset)\n+{\n+\tstruct replay_opts **opts_ptr = opt->value;\n+\tstruct replay_opts *opts = *opts_ptr;\n+\n+\tif (unset)\n+\t\treturn 0;\n+\n+\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n+\topts->xopts[opts->xopts_nr++] = xstrdup(arg);\n+\treturn 0;\n+}\n+\n+static void verify_opt_compatible(const char *me, const char *base_opt, ...)\n+{\n+\tconst char *this_opt;\n+\tva_list ap;\n+\n+\tva_start(ap, base_opt);\n+\twhile ((this_opt = va_arg(ap, const char *))) {\n+\t\tif (va_arg(ap, int))\n+\t\t\tbreak;\n+\t}\n+\tva_end(ap);\n+\n+\tif (this_opt)\n+\t\tdie(_(\"%s: %s cannot be used with %s\"), me, this_opt, base_opt);\n+}\n+\n+static void verify_opt_mutually_compatible(const char *me, ...)\n+{\n+\tconst char *opt1, *opt2;\n+\tva_list ap;\n+\n+\tva_start(ap, me);\n+\twhile ((opt1 = va_arg(ap, const char *))) {\n+\t\tif (va_arg(ap, int))\n+\t\t\tbreak;\n+\t}\n+\tif (opt1) {\n+\t\twhile ((opt2 = va_arg(ap, const char *))) {\n+\t\t\tif (va_arg(ap, int))\n+\t\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (opt1 && opt2)\n+\t\tdie(_(\"%s: %s cannot be used with %s\"),\tme, opt1, opt2);\n+}\n+\n+void sequencer_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 noop;\n+\tint reset = 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('n', \"no-commit\", &opts->no_commit, \"don't automatically commit\"),\n+\t\tOPT_BOOLEAN('e', \"edit\", &opts->edit, \"edit the commit message\"),\n+\t\t{ OPTION_BOOLEAN, 'r', NULL, &noop, NULL, \"no-op (backward compatibility)\",\n+\t\t  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 0 },\n+\t\tOPT_BOOLEAN('s', \"signoff\", &opts->signoff, \"add Signed-off-by:\"),\n+\t\tOPT_INTEGER('m', \"mainline\", &opts->mainline, \"parent number\"),\n+\t\tOPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),\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\tOPT_END(),\n+\t\tOPT_END(),\n+\t\tOPT_END(),\n+\t};\n+\n+\tif (opts->action == REPLAY_PICK) {\n+\t\tstruct option cp_extra[] = {\n+\t\t\tOPT_BOOLEAN('x', NULL, &opts->record_origin, \"append commit name\"),\n+\t\t\tOPT_BOOLEAN(0, \"ff\", &opts->allow_ff, \"allow fast-forward\"),\n+\t\t\tOPT_END(),\n+\t\t};\n+\t\tif (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))\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+\n+\t/* Check for incompatible subcommands */\n+\tverify_opt_mutually_compatible(me,\n+\t\t\t\t\"--reset\", reset,\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+\telse if (contin)\n+\t\topts->subcommand = REPLAY_CONTINUE;\n+\telse\n+\t\topts->subcommand = REPLAY_NONE;\n+\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\telse\n+\t\t\tthis_operation = \"--continue\";\n+\n+\t\tverify_opt_compatible(me, this_operation,\n+\t\t\t\t\"--no-commit\", opts->no_commit,\n+\t\t\t\t\"--signoff\", opts->signoff,\n+\t\t\t\t\"--mainline\", opts->mainline,\n+\t\t\t\t\"--strategy\", opts->strategy ? 1 : 0,\n+\t\t\t\t\"--strategy-option\", opts->xopts ? 1 : 0,\n+\t\t\t\t\"-x\", opts->record_origin,\n+\t\t\t\t\"--ff\", opts->allow_ff,\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+\t\t\t\t\"--no-commit\", opts->no_commit,\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+\n+struct commit_message {\n+\tchar *parent_label;\n+\tconst char *label;\n+\tconst char *subject;\n+\tchar *reencoded_message;\n+\tconst char *message;\n+};\n+\n+static int get_message(struct commit *commit, struct commit_message *out)\n+{\n+\tconst char *encoding;\n+\tconst char *abbrev, *subject;\n+\tint abbrev_len, subject_len;\n+\tchar *q;\n+\n+\tif (!commit->buffer)\n+\t\treturn -1;\n+\tencoding = get_encoding(commit->buffer);\n+\tif (!encoding)\n+\t\tencoding = \"UTF-8\";\n+\tif (!git_commit_encoding)\n+\t\tgit_commit_encoding = \"UTF-8\";\n+\n+\tout->reencoded_message = NULL;\n+\tout->message = commit->buffer;\n+\tif (strcmp(encoding, git_commit_encoding))\n+\t\tout->reencoded_message = reencode_string(commit->buffer,\n+\t\t\t\t\tgit_commit_encoding, encoding);\n+\tif (out->reencoded_message)\n+\t\tout->message = out->reencoded_message;\n+\n+\tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n+\tabbrev_len = strlen(abbrev);\n+\n+\tsubject_len = find_commit_subject(out->message, &subject);\n+\n+\tout->parent_label = xmalloc(strlen(\"parent of \") + abbrev_len +\n+\t\t\t      strlen(\"... \") + subject_len + 1);\n+\tq = out->parent_label;\n+\tq = mempcpy(q, \"parent of \", strlen(\"parent of \"));\n+\tout->label = q;\n+\tq = mempcpy(q, abbrev, abbrev_len);\n+\tq = mempcpy(q, \"... \", strlen(\"... \"));\n+\tout->subject = q;\n+\tq = mempcpy(q, subject, subject_len);\n+\t*q = '\\0';\n+\treturn 0;\n+}\n+\n+static void free_message(struct commit_message *msg)\n+{\n+\tfree(msg->parent_label);\n+\tfree(msg->reencoded_message);\n+}\n+\n+static char *get_encoding(const char *message)\n+{\n+\tconst char *p = message, *eol;\n+\n+\twhile (*p && *p != '\\n') {\n+\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n+\t\t\t; /* do nothing */\n+\t\tif (!prefixcmp(p, \"encoding \")) {\n+\t\t\tchar *result = xmalloc(eol - 8 - p);\n+\t\t\tstrlcpy(result, p + 9, eol - 8 - p);\n+\t\t\treturn result;\n+\t\t}\n+\t\tp = eol;\n+\t\tif (*p == '\\n')\n+\t\t\tp++;\n+\t}\n+\treturn NULL;\n+}\n+\n+static void write_cherry_pick_head(struct commit *commit)\n+{\n+\tint fd;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tstrbuf_addf(&buf, \"%s\\n\", sha1_to_hex(commit->object.sha1));\n+\n+\tfd = open(git_path(\"CHERRY_PICK_HEAD\"), O_WRONLY | O_CREAT, 0666);\n+\tif (fd < 0)\n+\t\tdie_errno(_(\"Could not open '%s' for writing\"),\n+\t\t\t  git_path(\"CHERRY_PICK_HEAD\"));\n+\tif (write_in_full(fd, buf.buf, buf.len) != buf.len || close(fd))\n+\t\tdie_errno(_(\"Could not write to '%s'\"), git_path(\"CHERRY_PICK_HEAD\"));\n+\tstrbuf_release(&buf);\n+}\n+\n+static void print_advice(void)\n+{\n+\tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n+\n+\tif (msg) {\n+\t\tfprintf(stderr, \"%s\\n\", msg);\n+\t\t/*\n+\t\t * A conflict has occured but the porcelain\n+\t\t * (typically rebase --interactive) wants to take care\n+\t\t * of the commit itself so remove CHERRY_PICK_HEAD\n+\t\t */\n+\t\tunlink(git_path(\"CHERRY_PICK_HEAD\"));\n+\t\treturn;\n+\t}\n+\n+\tadvise(\"after resolving the conflicts, mark the corrected paths\");\n+\tadvise(\"with 'git add <paths>' or 'git rm <paths>'\");\n+\tadvise(\"and commit the result with 'git commit'\");\n+}\n+\n+static void write_message(struct strbuf *msgbuf, const char *filename)\n+{\n+\tstatic struct lock_file msg_file;\n+\n+\tint msg_fd = hold_lock_file_for_update(&msg_file, filename,\n+\t\t\t\t\t       LOCK_DIE_ON_ERROR);\n+\tif (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)\n+\t\tdie_errno(_(\"Could not write to %s.\"), filename);\n+\tstrbuf_release(msgbuf);\n+\tif (commit_lock_file(&msg_file) < 0)\n+\t\tdie(_(\"Error wrapping up %s\"), filename);\n+}\n+\n+static struct tree *empty_tree(void)\n+{\n+\tstruct tree *tree = xcalloc(1, sizeof(struct tree));\n+\n+\ttree->object.parsed = 1;\n+\ttree->object.type = OBJ_TREE;\n+\tpretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);\n+\treturn tree;\n+}\n+\n+static int error_dirty_index(struct replay_opts *opts)\n+{\n+\tif (read_cache_unmerged())\n+\t\treturn error_resolve_conflict(action_name(opts));\n+\n+\t/* Different translation strings for cherry-pick and revert */\n+\tif (opts->action == REPLAY_PICK)\n+\t\terror(_(\"Your local changes would be overwritten by cherry-pick.\"));\n+\telse\n+\t\terror(_(\"Your local changes would be overwritten by revert.\"));\n+\n+\tif (advice_commit_before_merge)\n+\t\tadvise(_(\"Commit your changes or stash them to proceed.\"));\n+\treturn -1;\n+}\n+\n+static int fast_forward_to(const unsigned char *to, const unsigned char *from)\n+{\n+\tstruct ref_lock *ref_lock;\n+\n+\tread_cache();\n+\tif (checkout_fast_forward(from, to))\n+\t\texit(1); /* the callee should have complained already */\n+\tref_lock = lock_any_ref_for_update(\"HEAD\", from, 0);\n+\treturn write_ref_sha1(ref_lock, to, \"cherry-pick\");\n+}\n+\n+static int do_recursive_merge(struct commit *base, struct commit *next,\n+\t\t\t      const char *base_label, const char *next_label,\n+\t\t\t      unsigned char *head, struct strbuf *msgbuf,\n+\t\t\t      struct replay_opts *opts)\n+{\n+\tstruct merge_options o;\n+\tstruct tree *result, *next_tree, *base_tree, *head_tree;\n+\tint clean, index_fd;\n+\tconst char **xopt;\n+\tstatic struct lock_file index_lock;\n+\n+\tindex_fd = hold_locked_index(&index_lock, 1);\n+\n+\tread_cache();\n+\n+\tinit_merge_options(&o);\n+\to.ancestor = base ? base_label : \"(empty tree)\";\n+\to.branch1 = \"HEAD\";\n+\to.branch2 = next ? next_label : \"(empty tree)\";\n+\n+\thead_tree = parse_tree_indirect(head);\n+\tnext_tree = next ? next->tree : empty_tree();\n+\tbase_tree = base ? base->tree : empty_tree();\n+\n+\tfor (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)\n+\t\tparse_merge_opt(&o, *xopt);\n+\n+\tclean = merge_trees(&o,\n+\t\t\t    head_tree,\n+\t\t\t    next_tree, base_tree, &result);\n+\n+\tif (active_cache_changed &&\n+\t    (write_cache(index_fd, active_cache, active_nr) ||\n+\t     commit_locked_index(&index_lock)))\n+\t\t/* TRANSLATORS: %s will be \"revert\" or \"cherry-pick\" */\n+\t\tdie(_(\"%s: Unable to write new index file\"), action_name(opts));\n+\trollback_lock_file(&index_lock);\n+\n+\tif (!clean) {\n+\t\tint i;\n+\t\tstrbuf_addstr(msgbuf, \"\\nConflicts:\\n\\n\");\n+\t\tfor (i = 0; i < active_nr;) {\n+\t\t\tstruct cache_entry *ce = active_cache[i++];\n+\t\t\tif (ce_stage(ce)) {\n+\t\t\t\tstrbuf_addch(msgbuf, '\\t');\n+\t\t\t\tstrbuf_addstr(msgbuf, ce->name);\n+\t\t\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\t\t\twhile (i < active_nr && !strcmp(ce->name,\n+\t\t\t\t\t\tactive_cache[i]->name))\n+\t\t\t\t\ti++;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn !clean;\n+}\n+\n+/*\n+ * If we are cherry-pick, and if the merge did not result in\n+ * hand-editing, we will hit this commit and inherit the original\n+ * author date and name.\n+ * If we are revert, or if our cherry-pick results in a hand merge,\n+ * we had better say that the current user is responsible for that.\n+ */\n+static int run_git_commit(const char *defmsg, struct replay_opts *opts)\n+{\n+\t/* 6 is max possible length of our args array including NULL */\n+\tconst char *args[6];\n+\tint i = 0;\n+\n+\targs[i++] = \"commit\";\n+\targs[i++] = \"-n\";\n+\tif (opts->signoff)\n+\t\targs[i++] = \"-s\";\n+\tif (!opts->edit) {\n+\t\targs[i++] = \"-F\";\n+\t\targs[i++] = defmsg;\n+\t}\n+\targs[i] = NULL;\n+\n+\treturn run_command_v_opt(args, RUN_GIT_CMD);\n+}\n+\n+static int do_pick_commit(struct commit *commit, enum replay_action action,\n+\t\t\tstruct replay_opts *opts)\n+{\n+\tunsigned char head[20];\n+\tstruct commit *base, *next, *parent;\n+\tconst char *base_label, *next_label;\n+\tstruct commit_message msg = COMMIT_MESSAGE_INIT;\n+\tchar *defmsg = NULL;\n+\tstruct strbuf msgbuf = STRBUF_INIT;\n+\tint res;\n+\n+\tif (opts->no_commit) {\n+\t\t/*\n+\t\t * We do not intend to commit immediately.  We just want to\n+\t\t * merge the differences in, so let's compute the tree\n+\t\t * that represents the \"current\" state for merge-recursive\n+\t\t * to work on.\n+\t\t */\n+\t\tif (write_cache_as_tree(head, 0, NULL))\n+\t\t\tdie (_(\"Your index file is unmerged.\"));\n+\t} else {\n+\t\tif (get_sha1(\"HEAD\", head))\n+\t\t\treturn error(_(\"You do not have a valid HEAD\"));\n+\t\tif (index_differs_from(\"HEAD\", 0))\n+\t\t\treturn error_dirty_index(opts);\n+\t}\n+\tdiscard_cache();\n+\n+\tif (!commit->parents) {\n+\t\tparent = NULL;\n+\t}\n+\telse if (commit->parents->next) {\n+\t\t/* Reverting or cherry-picking a merge commit */\n+\t\tint cnt;\n+\t\tstruct commit_list *p;\n+\n+\t\tif (!opts->mainline)\n+\t\t\treturn error(_(\"Commit %s is a merge but no -m option was given.\"),\n+\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\n+\t\tfor (cnt = 1, p = commit->parents;\n+\t\t     cnt != opts->mainline && p;\n+\t\t     cnt++)\n+\t\t\tp = p->next;\n+\t\tif (cnt != opts->mainline || !p)\n+\t\t\treturn error(_(\"Commit %s does not have parent %d\"),\n+\t\t\t\tsha1_to_hex(commit->object.sha1), opts->mainline);\n+\t\tparent = p->item;\n+\t} else if (0 < opts->mainline)\n+\t\treturn error(_(\"Mainline was specified but commit %s is not a merge.\"),\n+\t\t\tsha1_to_hex(commit->object.sha1));\n+\telse\n+\t\tparent = commit->parents->item;\n+\n+\tif (opts->allow_ff && parent && !hashcmp(parent->object.sha1, head))\n+\t\treturn fast_forward_to(commit->object.sha1, head);\n+\n+\tif (parent && parse_commit(parent) < 0)\n+\t\t/* TRANSLATORS: The first %s will be \"revert\" or\n+\t\t   \"cherry-pick\", the second %s a SHA1 */\n+\t\treturn error(_(\"%s: cannot parse parent commit %s\"),\n+\t\t\taction == REPLAY_REVERT ? \"revert\" : \"cherry-pick\",\n+\t\t\tsha1_to_hex(parent->object.sha1));\n+\n+\tif (get_message(commit, &msg) != 0)\n+\t\treturn error(_(\"Cannot get commit message for %s\"),\n+\t\t\tsha1_to_hex(commit->object.sha1));\n+\n+\t/*\n+\t * \"commit\" is an existing commit.  We would want to apply\n+\t * the difference it introduces since its first parent \"prev\"\n+\t * on top of the current HEAD if we are cherry-pick.  Or the\n+\t * reverse of it if we are revert.\n+\t */\n+\n+\tdefmsg = git_pathdup(\"MERGE_MSG\");\n+\n+\tif (action == REPLAY_REVERT) {\n+\t\tbase = commit;\n+\t\tbase_label = msg.label;\n+\t\tnext = parent;\n+\t\tnext_label = msg.parent_label;\n+\t\tstrbuf_addstr(&msgbuf, \"Revert \\\"\");\n+\t\tstrbuf_addstr(&msgbuf, msg.subject);\n+\t\tstrbuf_addstr(&msgbuf, \"\\\"\\n\\nThis reverts commit \");\n+\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n+\n+\t\tif (commit->parents && commit->parents->next) {\n+\t\t\tstrbuf_addstr(&msgbuf, \", reversing\\nchanges made to \");\n+\t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(parent->object.sha1));\n+\t\t}\n+\t\tstrbuf_addstr(&msgbuf, \".\\n\");\n+\t} else {\n+\t\tconst char *p;\n+\n+\t\tbase = parent;\n+\t\tbase_label = msg.parent_label;\n+\t\tnext = commit;\n+\t\tnext_label = msg.label;\n+\n+\t\t/*\n+\t\t * Append the commit log message to msgbuf; it starts\n+\t\t * after the tree, parent, author, committer\n+\t\t * information followed by \"\\n\\n\".\n+\t\t */\n+\t\tp = strstr(msg.message, \"\\n\\n\");\n+\t\tif (p) {\n+\t\t\tp += 2;\n+\t\t\tstrbuf_addstr(&msgbuf, p);\n+\t\t}\n+\n+\t\tif (opts->record_origin) {\n+\t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n+\t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n+\t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n+\t\t}\n+\t\tif (!opts->no_commit)\n+\t\t\twrite_cherry_pick_head(commit);\n+\t}\n+\n+\tif (!opts->strategy || !strcmp(opts->strategy, \"recursive\") || action == REPLAY_REVERT) {\n+\t\tres = do_recursive_merge(base, next, base_label, next_label,\n+\t\t\t\t\t head, &msgbuf, opts);\n+\t\twrite_message(&msgbuf, defmsg);\n+\t} else {\n+\t\tstruct commit_list *common = NULL;\n+\t\tstruct commit_list *remotes = NULL;\n+\n+\t\twrite_message(&msgbuf, defmsg);\n+\n+\t\tcommit_list_insert(base, &common);\n+\t\tcommit_list_insert(next, &remotes);\n+\t\tres = try_merge_command(opts->strategy, opts->xopts_nr, opts->xopts,\n+\t\t\t\t\tcommon, sha1_to_hex(head), remotes);\n+\t\tfree_commit_list(common);\n+\t\tfree_commit_list(remotes);\n+\t}\n+\n+\tif (res) {\n+\t\terror(action == REPLAY_REVERT\n+\t\t      ? _(\"could not revert %s... %s\")\n+\t\t      : _(\"could not apply %s... %s\"),\n+\t\t      find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n+\t\t      msg.subject);\n+\t\tprint_advice();\n+\t\trerere(opts->allow_rerere_auto);\n+\t} else {\n+\t\tif (!opts->no_commit)\n+\t\t\tres = run_git_commit(defmsg, opts);\n+\t}\n+\n+\tfree_message(&msg);\n+\tfree(defmsg);\n+\n+\treturn res;\n+}\n+\n+static void prepare_revs(struct rev_info *revs, struct replay_opts *opts)\n+{\n+\tint argc;\n+\n+\tinit_revisions(revs, NULL);\n+\trevs->no_walk = 1;\n+\tif (opts->action != REPLAY_REVERT)\n+\t\trevs->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+\t\tdie(_(\"revision walk setup failed\"));\n+\n+\tif (!revs->commits)\n+\t\tdie(_(\"empty commit set passed\"));\n+}\n+\n+static void read_and_refresh_cache(struct replay_opts *opts)\n+{\n+\tstatic struct lock_file index_lock;\n+\tint index_fd = hold_locked_index(&index_lock, 0);\n+\tif (read_index_preload(&the_index, NULL) < 0)\n+\t\tdie(_(\"git %s: failed to read the index\"), action_name(opts));\n+\trefresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);\n+\tif (the_index.cache_changed) {\n+\t\tif (write_index(&the_index, index_fd) ||\n+\t\t    commit_locked_index(&index_lock))\n+\t\t\tdie(_(\"git %s: failed to refresh the index\"), action_name(opts));\n+\t}\n+\trollback_lock_file(&index_lock);\n+}\n+\n+/*\n+ * Append a commit to the end of the commit_list.\n+ *\n+ * next starts by pointing to the variable that holds the head of an\n+ * empty commit_list, and is updated to point to the \"next\" field of\n+ * the last item on the list as new commits are appended.\n+ *\n+ * Usage example:\n+ *\n+ *     struct commit_list *list;\n+ *     struct commit_list **next = &list;\n+ *\n+ *     next = commit_list_append(c1, next);\n+ *     next = commit_list_append(c2, next);\n+ *     assert(commit_list_count(list) == 2);\n+ *     return list;\n+ */\n+struct replay_insn_list **replay_insn_list_append(enum replay_action action,\n+\t\t\t\t\t\tstruct commit *operand,\n+\t\t\t\t\t\tstruct replay_insn_list **next)\n+{\n+\tstruct replay_insn_list *new = xmalloc(sizeof(*new));\n+\tnew->action = action;\n+\tnew->operand = operand;\n+\t*next = new;\n+\tnew->next = NULL;\n+\treturn &new->next;\n+}\n+\n+static int format_todo(struct strbuf *buf, struct replay_insn_list *todo_list)\n+{\n+\tstruct replay_insn_list *cur;\n+\n+\tfor (cur = todo_list; cur; cur = cur->next) {\n+\t\tconst char *sha1_abbrev, *action_str;\n+\t\tstruct commit_message msg = COMMIT_MESSAGE_INIT;\n+\n+\t\taction_str = cur->action == REPLAY_REVERT ? \"revert\" : \"pick\";\n+\t\tsha1_abbrev = find_unique_abbrev(cur->operand->object.sha1, DEFAULT_ABBREV);\n+\t\tif (get_message(cur->operand, &msg))\n+\t\t\treturn error(_(\"Cannot get commit message for %s\"), sha1_abbrev);\n+\t\tstrbuf_addf(buf, \"%s %s %s\\n\", action_str, sha1_abbrev, msg.subject);\n+\t\tfree_message(&msg);\n+\t}\n+\treturn 0;\n+}\n+\n+static int parse_insn_line(char *start, struct replay_insn_list *item)\n+{\n+\tunsigned char commit_sha1[20];\n+\tchar sha1_abbrev[40];\n+\tchar *p = start, *q, *end = strchrnul(start, '\\n');\n+\n+\tif (!prefixcmp(start, \"pick \")) {\n+\t\titem->action = REPLAY_PICK;\n+\t\tp += strlen(\"pick \");\n+\t} else if (!prefixcmp(start, \"revert \")) {\n+\t\titem->action = REPLAY_REVERT;\n+\t\tp += strlen(\"revert \");\n+\t} else\n+\t\treturn error(_(\"Unrecognized action: %s\"), start);\n+\n+\tq = strchrnul(p, ' ');\n+\tif (q > end)\n+\t\tq = end;\n+\tif (q - p + 1 > sizeof(sha1_abbrev))\n+\t\treturn error(_(\"Object name too large: %s\"), p);\n+\tmemcpy(sha1_abbrev, p, q - p);\n+\tsha1_abbrev[q - p] = '\\0';\n+\n+\tif (get_sha1(sha1_abbrev, commit_sha1) < 0)\n+\t\treturn error(_(\"Malformed object name: %s\"), sha1_abbrev);\n+\n+\titem->operand = lookup_commit_reference(commit_sha1);\n+\tif (!item->operand)\n+\t\treturn error(_(\"Not a valid commit: %s\"), sha1_abbrev);\n+\n+\titem->next = NULL;\n+\treturn 0;\n+}\n+\n+static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)\n+{\n+\tstruct replay_insn_list **next = todo_list;\n+\tstruct replay_insn_list item = {0, NULL, NULL};\n+\tchar *p = buf;\n+\tint i;\n+\n+\tfor (i = 1; *p; i++) {\n+\t\tif (parse_insn_line(p, &item) < 0)\n+\t\t\treturn error(_(\"Could not parse line %d.\"), i);\n+\t\tnext = replay_insn_list_append(item.action, item.operand, next);\n+\t\tp = strchrnul(p, '\\n');\n+\t\tif (*p)\n+\t\t\tp++;\n+\t}\n+\tif (!*todo_list)\n+\t\treturn error(_(\"No commits parsed.\"));\n+\treturn 0;\n+}\n+\n+static void read_populate_todo(struct replay_insn_list **todo_list)\n+{\n+\tconst char *todo_file = git_path(SEQ_TODO_FILE);\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint fd, res;\n+\n+\tfd = open(todo_file, O_RDONLY);\n+\tif (fd < 0)\n+\t\tdie_errno(_(\"Could not open %s.\"), todo_file);\n+\tif (strbuf_read(&buf, fd, 0) < 0) {\n+\t\tclose(fd);\n+\t\tstrbuf_release(&buf);\n+\t\tdie(_(\"Could not read %s.\"), todo_file);\n+\t}\n+\tclose(fd);\n+\n+\tres = parse_insn_buffer(buf.buf, todo_list);\n+\tstrbuf_release(&buf);\n+\tif (res)\n+\t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n+}\n+\n+static int populate_opts_cb(const char *key, const char *value, void *data)\n+{\n+\tstruct replay_opts *opts = data;\n+\tint error_flag = 1;\n+\n+\tif (!value)\n+\t\terror_flag = 0;\n+\telse if (!strcmp(key, \"options.no-commit\"))\n+\t\topts->no_commit = git_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.edit\"))\n+\t\topts->edit = git_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.signoff\"))\n+\t\topts->signoff = git_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.record-origin\"))\n+\t\topts->record_origin = git_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.allow-ff\"))\n+\t\topts->allow_ff = git_config_bool_or_int(key, value, &error_flag);\n+\telse if (!strcmp(key, \"options.mainline\"))\n+\t\topts->mainline = git_config_int(key, value);\n+\telse if (!strcmp(key, \"options.strategy\"))\n+\t\tgit_config_string(&opts->strategy, key, value);\n+\telse if (!strcmp(key, \"options.strategy-option\")) {\n+\t\tALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);\n+\t\topts->xopts[opts->xopts_nr++] = xstrdup(value);\n+\t} else\n+\t\treturn error(_(\"Invalid key: %s\"), key);\n+\n+\tif (!error_flag)\n+\t\treturn error(_(\"Invalid value for %s: %s\"), key, value);\n+\n+\treturn 0;\n+}\n+\n+static void read_populate_opts(struct replay_opts **opts_ptr)\n+{\n+\tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n+\n+\tif (!file_exists(opts_file))\n+\t\treturn;\n+\tif (git_config_from_file(populate_opts_cb, opts_file, *opts_ptr) < 0)\n+\t\tdie(_(\"Malformed options sheet: %s\"), opts_file);\n+}\n+\n+static void walk_revs_populate_todo(struct replay_insn_list **todo_list,\n+\t\t\t\tstruct replay_opts *opts)\n+{\n+\tstruct rev_info revs;\n+\tstruct commit *commit;\n+\tstruct replay_insn_list **next;\n+\n+\tprepare_revs(&revs, opts);\n+\n+\tnext = todo_list;\n+\twhile ((commit = get_revision(&revs)))\n+\t\tnext = replay_insn_list_append(opts->action, commit, next);\n+}\n+\n+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+\telse if (mkdir(seq_dir, 0777) < 0)\n+\t\tdie_errno(_(\"Could not create sequencer directory '%s'.\"), seq_dir);\n+\treturn 0;\n+}\n+\n void remove_sequencer_state(int aggressive)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -17,3 +818,158 @@ void remove_sequencer_state(int aggressive)\n \tstrbuf_release(&seq_dir);\n \tstrbuf_release(&seq_old_dir);\n }\n+\n+static void save_head(const char *head)\n+{\n+\tconst char *head_file = git_path(SEQ_HEAD_FILE);\n+\tstatic struct lock_file head_lock;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint fd;\n+\n+\tfd = hold_lock_file_for_update(&head_lock, head_file, LOCK_DIE_ON_ERROR);\n+\tstrbuf_addf(&buf, \"%s\\n\", head);\n+\tif (write_in_full(fd, buf.buf, buf.len) < 0)\n+\t\tdie_errno(_(\"Could not write to %s.\"), head_file);\n+\tif (commit_lock_file(&head_lock) < 0)\n+\t\tdie(_(\"Error wrapping up %s.\"), head_file);\n+}\n+\n+static void save_todo(struct replay_insn_list *todo_list)\n+{\n+\tconst char *todo_file = git_path(SEQ_TODO_FILE);\n+\tstatic struct lock_file todo_lock;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint fd;\n+\n+\tfd = hold_lock_file_for_update(&todo_lock, todo_file, LOCK_DIE_ON_ERROR);\n+\tif (format_todo(&buf, todo_list) < 0)\n+\t\tdie(_(\"Could not format %s.\"), todo_file);\n+\tif (write_in_full(fd, buf.buf, buf.len) < 0) {\n+\t\tstrbuf_release(&buf);\n+\t\tdie_errno(_(\"Could not write to %s.\"), todo_file);\n+\t}\n+\tif (commit_lock_file(&todo_lock) < 0) {\n+\t\tstrbuf_release(&buf);\n+\t\tdie(_(\"Error wrapping up %s.\"), todo_file);\n+\t}\n+\tstrbuf_release(&buf);\n+}\n+\n+static void save_opts(struct replay_opts *opts)\n+{\n+\tconst char *opts_file = git_path(SEQ_OPTS_FILE);\n+\n+\tif (opts->no_commit)\n+\t\tgit_config_set_in_file(opts_file, \"options.no-commit\", \"true\");\n+\tif (opts->edit)\n+\t\tgit_config_set_in_file(opts_file, \"options.edit\", \"true\");\n+\tif (opts->signoff)\n+\t\tgit_config_set_in_file(opts_file, \"options.signoff\", \"true\");\n+\tif (opts->record_origin)\n+\t\tgit_config_set_in_file(opts_file, \"options.record-origin\", \"true\");\n+\tif (opts->allow_ff)\n+\t\tgit_config_set_in_file(opts_file, \"options.allow-ff\", \"true\");\n+\tif (opts->mainline) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstrbuf_addf(&buf, \"%d\", opts->mainline);\n+\t\tgit_config_set_in_file(opts_file, \"options.mainline\", buf.buf);\n+\t\tstrbuf_release(&buf);\n+\t}\n+\tif (opts->strategy)\n+\t\tgit_config_set_in_file(opts_file, \"options.strategy\", opts->strategy);\n+\tif (opts->xopts) {\n+\t\tint i;\n+\t\tfor (i = 0; i < opts->xopts_nr; i++)\n+\t\t\tgit_config_set_multivar_in_file(opts_file,\n+\t\t\t\t\t\t\t\"options.strategy-option\",\n+\t\t\t\t\t\t\topts->xopts[i], \"^$\", 0);\n+\t}\n+}\n+\n+static int pick_commits(struct replay_insn_list *todo_list,\n+\t\t\tstruct replay_opts *opts)\n+{\n+\tstruct replay_insn_list *cur;\n+\tint res;\n+\n+\tsetenv(GIT_REFLOG_ACTION, action_name(opts), 0);\n+\tif (opts->allow_ff)\n+\t\tassert(!(opts->signoff || opts->no_commit ||\n+\t\t\t\topts->record_origin || opts->edit));\n+\tread_and_refresh_cache(opts);\n+\n+\tfor (cur = todo_list; cur; cur = cur->next) {\n+\t\tsave_todo(cur);\n+\t\tres = do_pick_commit(cur->operand, cur->action, opts);\n+\t\tif (res) {\n+\t\t\tif (!cur->next && res > 0)\n+\t\t\t\t/*\n+\t\t\t\t * A conflict 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\t\treturn res;\n+\t\t}\n+\t}\n+\n+\t/*\n+\t * Sequence of picks finished successfully; cleanup by\n+\t * removing the .git/sequencer directory\n+\t */\n+\tremove_sequencer_state(1);\n+\treturn 0;\n+}\n+\n+int sequencer_pick_revisions(struct replay_opts *opts)\n+{\n+\tstruct replay_insn_list *todo_list = NULL;\n+\tunsigned char sha1[20];\n+\n+\tread_and_refresh_cache(opts);\n+\n+\t/*\n+\t * Decide what to do depending on the arguments; a fresh\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+\t\tremove_sequencer_state(1);\n+\t\treturn 0;\n+\t} else if (opts->subcommand == REPLAY_CONTINUE) {\n+\t\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n+\t\t\tgoto error;\n+\t\tread_populate_opts(&opts);\n+\t\tread_populate_todo(&todo_list);\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+\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 --reset 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 == REPLAY_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+\t}\n+\treturn pick_commits(todo_list, opts);\n+error:\n+\treturn error(_(\"No %s in progress\"), action_name(opts));\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex f4db257..ebf20cb 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -7,7 +7,32 @@\n #define SEQ_TODO_FILE\t\"sequencer/todo\"\n #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n \n+#define COMMIT_MESSAGE_INIT  { NULL, NULL, NULL, NULL, NULL };\n+\n enum replay_action { REPLAY_REVERT, REPLAY_PICK };\n+enum replay_subcommand { REPLAY_NONE, REPLAY_RESET, REPLAY_CONTINUE };\n+\n+struct replay_opts {\n+\tenum replay_action action;\n+\tenum replay_subcommand subcommand;\n+\n+\t/* Boolean options */\n+\tint edit;\n+\tint record_origin;\n+\tint no_commit;\n+\tint signoff;\n+\tint allow_ff;\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 \n struct replay_insn_list {\n \tenum replay_action action;\n@@ -25,4 +50,7 @@ struct replay_insn_list {\n  */\n void remove_sequencer_state(int aggressive);\n \n+void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts);\n+int sequencer_pick_revisions(struct replay_opts *opts);\n+\n #endif\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173306","messageId":"1313088705-32222-7-git-send-email-artagnon@gmail.com","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 6/6] sequencer: Remove sequencer state after final commit","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-11T18:51:45Z","receivedAt":"2011-08-11T18:51:45Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Since d3f4628e (revert: Remove sequencer state when no commits are\npending, 2011-07-06), the sequencer removes the sequencer state before\nthe final commit is actually completed.  This design is inherently\nflawed, as it will not allow the user to abort the sequencer operation\nat that stage.  Instead, write and expose a new function to count the\nnumber of commits left in the instruction sheet; use this in\nbuiltin/commit.c to remove the sequencer state when a commit has\nsuccessfully completed and there is only one instruction left in the\nsheet.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n builtin/commit.c                |    7 ++++++-\n sequencer.c                     |   26 +++++++++++++++-----------\n sequencer.h                     |    1 +\n t/t3510-cherry-pick-sequence.sh |    4 ++--\n 4 files changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e1af9b1..4a5af9a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -26,6 +26,7 @@\n #include \"unpack-trees.h\"\n #include \"quote.h\"\n #include \"submodule.h\"\n+#include \"sequencer.h\"\n \n static const char * const builtin_commit_usage[] = {\n \t\"git commit [options] [--] <filepattern>...\",\n@@ -1521,7 +1522,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \tunlink(git_path(\"MERGE_MODE\"));\n \tunlink(git_path(\"SQUASH_MSG\"));\n \n-\tif (commit_index_files())\n+\t/* Remove sequencer state if we just finished the last insn */\n+\tif (sequencer_count_todo() == 1)\n+\t\tremove_sequencer_state(1);\n+\n+       if (commit_index_files())\n \t\tdie (_(\"Repository has been updated, but unable to write\\n\"\n \t\t     \"new_index file. Check that disk is not full or quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\ndiff --git a/sequencer.c b/sequencer.c\nindex e72618c..783b4a9 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -736,6 +736,20 @@ static void read_populate_todo(struct replay_insn_list **todo_list)\n \t\tdie(_(\"Unusable instruction sheet: %s\"), todo_file);\n }\n \n+int sequencer_count_todo(void)\n+{\n+\tstruct replay_insn_list *todo_list = NULL;\n+\tstruct replay_insn_list *cur;\n+\tint insn_count = 0;\n+\n+\tif (!file_exists(git_path(SEQ_TODO_FILE)))\n+\t\treturn 0;\n+\tread_populate_todo(&todo_list);\n+\tfor (cur = todo_list; cur; cur = cur->next)\n+\t\tinsn_count += 1;\n+\treturn insn_count;\n+}\n+\n static int populate_opts_cb(const char *key, const char *value, void *data)\n {\n \tstruct replay_opts *opts = data;\n@@ -901,18 +915,8 @@ static int pick_commits(struct replay_insn_list *todo_list,\n \tfor (cur = todo_list; cur; cur = cur->next) {\n \t\tsave_todo(cur);\n \t\tres = do_pick_commit(cur->operand, cur->action, opts);\n-\t\tif (res) {\n-\t\t\tif (!cur->next && res > 0)\n-\t\t\t\t/*\n-\t\t\t\t * A conflict 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/sequencer.h b/sequencer.h\nindex ebf20cb..c64ba91 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -52,5 +52,6 @@ void remove_sequencer_state(int aggressive);\n \n void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts);\n int sequencer_pick_revisions(struct replay_opts *opts);\n+int sequencer_count_todo(void);\n \n #endif\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex bc7fb13..57e9e7c 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -82,13 +82,13 @@ test_expect_success '--reset cleans up sequencer state' '\n \ttest_path_is_missing .git/sequencer\n '\n \n-test_expect_success 'cherry-pick cleans up sequencer state when one commit is left' '\n+test_expect_success 'final commit cleans up sequencer state' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick base..picked &&\n-\ttest_path_is_missing .git/sequencer &&\n \techo \"resolved\" >foo &&\n \tgit add foo &&\n \tgit commit &&\n+\ttest_path_is_missing .git/sequencer &&\n \t{\n \t\tgit rev-list HEAD |\n \t\tgit diff-tree --root --stdin |\n-- \n1.7.6.351.gb35ac.dirty\n"},{"id":"173308","messageId":"20110811190312.GD2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/6] Towards a generalized sequencer","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T19:03:12Z","receivedAt":"2011-08-11T19:03:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> No new ideas; just the same ideas implemented in a more sane\n> way.\n\nThanks!  Will try it out.\n\n> Note: I didn't know what to do with the license header in the fifth\n> patch.  I just assumed that it was some historical cruft and removed\n> it.\n\nPlease don't.  Technically it's allowed by the license if I understand\ncorrectly (since the copyright notices are not accompanied by a\ndisclaimer of warranty) but it's almost always the wrong thing to do\nto remove a copyright notice without the author's permission.\n"},{"id":"173309","messageId":"20110811192047.GE2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/6] revert: Don't remove the sequencer state on error","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T19:20:47Z","receivedAt":"2011-08-11T19:20:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The cherry-pick/ revert machinery now removes the sequencer state when\n> do_pick_commit returns a non-zero, and when only one instruction is\n> left in the todo_list.  Since do_pick_commit has a way to distinguish\n> errors from conflicts using the signed-ness of the return value,\n> utilize this to ensure that the sequencer state is only removed when\n> there's a conflict and there is only one instruction left in the\n> todo_list.\n\nI'm having trouble parsing this.  Is the idea of this one to mitigate\nsome of the problems with the \"remove sequencer state when a conflict\nis encountered in the last commit\" hack, by suppressing such behavior\nwhen there is an internal error rather than a conflict?  Why bother,\nwhen the behavior is suppressed altogether later in the series?\n"},{"id":"173310","messageId":"20110811192403.GF2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/6] revert: Free memory after get_message call","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T19:24:03Z","receivedAt":"2011-08-11T19:24:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> The format_todo function leaks memory because it forgets to call\n> free_message after get_message.  Fix this.\n>\n> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n\nThat's \"Reported-by\", I think. :)\n\nIs this a big leak or a small one?  Is it one-time or in a loop?\n\n> ---\n>  builtin/revert.c |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index a548a14..1a4187a 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -688,6 +688,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,\n>  \t\t\treturn error(_(\"Cannot get commit message for %s\"), sha1_abbrev);\n>  \t\tstrbuf_addf(buf, \"%s %s %s\\n\", action_str, sha1_abbrev, msg.subject);\n>  \t}\n> +\tfree_message(&msg);\n>  \treturn 0;\n>  }\n\nI don't see how this could work.  Since there an xmalloc() in each\nloop iteration, I would have expected the free() to be in the loop\nbody, too.\n"},{"id":"173311","messageId":"20110811194708.GG2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/6] revert: Parse instruction sheet more cautiously","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T19:47:08Z","receivedAt":"2011-08-11T19:47:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Fix a buffer overflow bug by checking that parsed SHA-1 hex will fit\n> in the buffer we've created for it.\n\nNit: it seems best to either describe the behavior or the code change.\nSo, for example:\n\n\tDo not overflow a buffer when the second word in a \"pick\"\n\tor \"revert\" line in .git/sequencer/todo is very long.\n\nOr:\n\n\tCheck that the commit name argument to a \"pick\" or \"revert\"\n\taction is not too long, to avoid overflowing an on-stack\n\tbuffer.\n\nIt would also be comforting to readers to mention when the bug was\nintroduced, so they don't start worrying about protecting their\ninstallations against privilege escalation:\n\n\tThis fixes a regression introduced by <...>, which has not\n\tescaped into the wild yet, luckily.\n\nCould we have a test for this?\n\n> Also change the instruction sheet format\n\nHow is this \"Also\" part related to the earlier bit?  Does one make the\nother easier, or is it more of a \"While we're changing this code\" kind\nof thing?\n\n> subtly so that a description of the commit after the object\n> name is optional.  So now, an instruction sheet like this is perfectly\n> valid:\n>\n>   pick 35b0426\n>   pick fbd5bbcbc2e\n>   pick 7362160f\n\nSounds convenient. :)  Thanks.\n\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -697,26 +697,24 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts)\n>  \tunsigned char commit_sha1[20];\n>  \tchar sha1_abbrev[40];\n>  \tenum replay_action action;\n> -\tint insn_len = 0;\n> -\tchar *p, *q;\n> +\tchar *p = start, *q, *end = strchrnul(start, '\\n');\n\nBy the way, why are these non-const?  (Not about this patch.)\n\nI don't know why, but I would be more comfortable reading something\nlike this:\n\n\tconst char *p, *q;\n\n\tp = start;\n\tif (!prefixcmp(p, \"pick \")) {\n\t\taction = CHERRY_PICK;\n\t\tp += strlen(\"pick \");\n\t} else if (...) {\n\t\t...\n\t} ...\n\n\tq = p + strcspn(p, \" \\n\");\n\tif (q - p + 1 > sizeof(...))\n\t\t...\n\t...\n\nMaybe because I can imagine how the pointers move, always forward and\nnever past the end of the line.\n\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -211,4 +211,33 @@ test_expect_success 'malformed instruction sheet 2' '\n>  \ttest_must_fail git cherry-pick --continue\n>  '\n>  \n> +test_expect_success 'missing commit descriptions in instruction sheet' '\n\nWhat assertion does this test check?  \"commit descriptions in insn\nsheet are optional\"?\n\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick base..anotherpick &&\n\nA failed cherry-pick.\n\n> +\techo \"c\" >foo &&\n> +\tgit add foo &&\n> +\tgit commit &&\n\nResolving the conflict.\n\n> +\tcut -d\" \" -f1,2 .git/sequencer/todo >new_sheet &&\n> +\tcp new_sheet .git/sequencer/todo &&\n\nMucking about with the insn sheet.\n\n> +\tgit cherry-pick --continue &&\n\nContinuing.\n\n> +\ttest_path_is_missing .git/sequencer &&\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> +\tcat >expect <<-\\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_cmp expect actual\n\nChecking that the cherry-pick succeeded and the resulting list of\ncommits.  How is this expected to potentially fail?  Maybe something\nlike\n\n\tgit rev-list HEAD >commits &&\n\ttest_line_count = 4 commits\n\nor\n\n\tgit diff --exit-code <something>\n\nwould make what this is intended to check clearer.  As hinted above,\nsome blank lines or comments might make the earlier part easier to\nread.\n\nThanks, this patch does two good things.  For what it's worth, with\nthe changes hinted at above and a split into two patches if that seems\nsensible,\nAcked-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"173313","messageId":"20110811201245.GH2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/6] revert: Allow mixed pick and revert instructions","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T20:12:45Z","receivedAt":"2011-08-11T20:12:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Change the way the instruction parser works, allowing arbitrary\n> (action, operand) pairs to be parsed.\n\nThe first part of this sentence is not very satisfying.  Maybe it\nmeans something like\n\n\tParse the instruction list in .git/sequencer/todo as a list\n\tof (action, operand) pairs, instead of assuming all instructions\n\tuse the same action.\n\n[...]\n> This patch lays the foundation for extending the parser to support\n> more actions so 'git rebase -i' can reuse this machinery in the\n> future.\n\nExciting stuff. :)\n\n[...]\n> +++ b/builtin/revert.c\n[...]\n> @@ -457,7 +456,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts)\n>  \treturn run_command_v_opt(args, RUN_GIT_CMD);\n>  }\n>  \n> -static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n> +static int do_pick_commit(struct commit *commit, enum replay_action action,\n> +\t\t\tstruct replay_opts *opts)\n[...]\n> @@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>  \t\t/* TRANSLATORS: The first %s will be \"revert\" or\n>  \t\t   \"cherry-pick\", the second %s a SHA1 */\n>  \t\treturn error(_(\"%s: cannot parse parent commit %s\"),\n> -\t\t\taction_name(opts), sha1_to_hex(parent->object.sha1));\n> +\t\t\taction == REPLAY_REVERT ? \"revert\" : \"cherry-pick\",\n> +\t\t\tsha1_to_hex(parent->object.sha1));\n\nMy first thought was \"why stop using the helper function action_name\"?\nBut now I see that it previously came from \"opts\" (i.e., the command\nline) and now comes from the todo file.\n\nThe command name there was never really important except when\ncherry-pick or revert is being called by a script, and the message\nindicates which command was having trouble parsing the commit.  If I\nam using \"git cherry-pick --continue\" to continue after a failed\nrevert, I suspect action_name(opts) [\"cherry-pick: \"] would actually be\nmore sensible than the command name corresponding to the particular\npick/revert line.\n\n[...]\n> -\t\treturn NULL;\n> +\t\treturn error(_(\"Unrecognized action: %s\"), start);\n\nProbably should be mentioned in the commit message.  Doesn't this\nprint the problematic line and all lines after it?\n\nMaybe something like\n\n\tlen = strchrnul(p, '\\n') - p;\n\tif (len > 255)\n\t\tlen = 255;\n\treturn error(_(\"Unrecognized action: %.*s\"), (int) len, p);\n\nwould do.\n\n[...]\n> -static int parse_insn_buffer(char *buf, struct commit_list **todo_list,\n> -\t\t\tstruct replay_opts *opts)\n> +static int parse_insn_buffer(char *buf, struct replay_insn_list **todo_list)\n>  {\n> -\tstruct commit_list **next = todo_list;\n> -\tstruct commit *commit;\n> +\tstruct replay_insn_list **next = todo_list;\n> +\tstruct replay_insn_list item = {0, NULL, NULL};\n>  \tchar *p = buf;\n>  \tint i;\n>  \n>  \tfor (i = 1; *p; i++) {\n> -\t\tcommit = parse_insn_line(p, opts);\n> -\t\tif (!commit)\n> +\t\tif (parse_insn_line(p, &item) < 0)\n>  \t\t\treturn error(_(\"Could not parse line %d.\"), i);\n\nCould we can make this error message more clearly suggest that it's\ngiving context to the error above it?  For example, something vaguely\nlike\n\n\terror: unrecognized action: reset c78a78c9 Going back\n\terror: on line 7\n\tfatal: unusable instruction sheet \".git/sequencer/todo\"\n\thint: to continue after fixing it, use \"git cherry-pick --continue\"\n\thint: or to bail out, use \"git cherry-pick --abort\"\n\nDoes a \"cherry-pick --continue\" in this scenario skip the first commit\nin the todo list?  Should it?\n\n> --- a/t/t3510-cherry-pick-sequence.sh\n> +++ b/t/t3510-cherry-pick-sequence.sh\n> @@ -240,4 +240,62 @@ test_expect_success 'missing commit descriptions in instruction sheet' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'revert --continue continues after cherry-pick' '\n\nHaven't read the tests yet.  The general idea of this patch still\nseems very sane.\n"},{"id":"173314","messageId":"20110811201604.GI2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T20:16:04Z","receivedAt":"2011-08-11T20:16:04Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -7,7 +7,32 @@\n>  #define SEQ_TODO_FILE\t\"sequencer/todo\"\n>  #define SEQ_OPTS_FILE\t\"sequencer/opts\"\n>  \n> +#define COMMIT_MESSAGE_INIT  { NULL, NULL, NULL, NULL, NULL };\n\nI don't think this should be exposed.  The rest seems pretty sane,\nthough I haven't read the patch carefully.\n"},{"id":"173315","messageId":"20110811201755.GJ2277@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-7-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 6/6] sequencer: Remove sequencer state after final commit","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T20:17:55Z","receivedAt":"2011-08-11T20:17:55Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRamkumar Ramachandra wrote:\n\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -26,6 +26,7 @@\n>  #include \"unpack-trees.h\"\n>  #include \"quote.h\"\n>  #include \"submodule.h\"\n> +#include \"sequencer.h\"\n>  \n>  static const char * const builtin_commit_usage[] = {\n>  \t\"git commit [options] [--] <filepattern>...\",\n> @@ -1521,7 +1522,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>  \tunlink(git_path(\"MERGE_MODE\"));\n>  \tunlink(git_path(\"SQUASH_MSG\"));\n>  \n> -\tif (commit_index_files())\n> +\t/* Remove sequencer state if we just finished the last insn */\n> +\tif (sequencer_count_todo() == 1)\n> +\t\tremove_sequencer_state(1);\n> +\n> +       if (commit_index_files())\n>  \t\tdie (_(\"Repository has been updated, but unable to write\\n\"\n\nWhitespace damage?\n\nI think even this is too much knowledge of sequencer details on \"git\ncommit\"'s part.  It should be enough to say\n\n\tDear sequencer, I'm commiting!\n\nand let the sequencer take care of the rest.\n"},{"id":"173318","messageId":"20110811215650.GA13839@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"1313088705-32222-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-11T21:56:50Z","receivedAt":"2011-08-11T21:56:50Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> +++ b/sequencer.c\n> @@ -1,8 +1,809 @@\n[...]\n> +static const char * const revert_usage[] = {\n> +\t\"git revert [options] <commit-ish>\",\n> +\t\"git revert <subcommand>\",\n> +\tNULL\n> +};\n[...]\n> +++ b/sequencer.h\n[...]\n> @@ -25,4 +50,7 @@ struct replay_insn_list {\n>   */\n>  void remove_sequencer_state(int aggressive);\n>\n> +void sequencer_parse_args(int argc, const char **argv, struct replay_opts *opts);\n\nAnother thought.  I wonder if it's possible to leave\nsequencer_parse_args() private to builtin/revert.c, making the split\na little more logical:\n\n - the builtin takes responsibility for its commandline interface\n - the library takes over once the builtin has figured out what the\n   user wanted.\n\n(If another command wants to reuse some subset of cherry-pick/revert's\ncommandline options, an appropriate function can always be exposed\nlater.)\n"},{"id":"173328","messageId":"7vk4ajo6hk.fsf@alter.siamese.dyndns.org","threadId":"28072","inReplyTo":"20110811215650.GA13839@elie.gateway.2wire.net","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-11T23:47:51Z","receivedAt":"2011-08-11T23:47:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Another thought.  I wonder if it's possible to leave\n> sequencer_parse_args() private to builtin/revert.c, making the split\n> a little more logical:\n>\n>  - the builtin takes responsibility for its commandline interface\n>  - the library takes over once the builtin has figured out what the\n>    user wanted.\n\nThat's a very natural separation of tasks.\n"},{"id":"173329","messageId":"CALkWK0nf0ScbKdykVzp3s+gK9nTPLA3xUOWzLNwVEO83YiLkvg@mail.gmail.com","threadId":"28072","inReplyTo":"20110811192403.GF2277@elie.gateway.2wire.net","subject":"Re: [PATCH 2/6] revert: Free memory after get_message call","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-12T02:07:43Z","receivedAt":"2011-08-12T02:07:43Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> I don't see how this could work.  Since there an xmalloc() in each\n> loop iteration, I would have expected the free() to be in the loop\n> body, too.\n\nOops, fixed.  Thanks.\n\n-- Ram\n"},{"id":"173330","messageId":"CALkWK0nXAU+jFwyzmYC6XuPGy8Cdmbpis0Nvb-M+tQSNk_PXiQ@mail.gmail.com","threadId":"28072","inReplyTo":"20110811190312.GD2277@elie.gateway.2wire.net","subject":"Re: [PATCH 0/6] Towards a generalized sequencer","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-12T02:14:12Z","receivedAt":"2011-08-12T02:14:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n>> Note: I didn't know what to do with the license header in the fifth\n>> patch.  I just assumed that it was some historical cruft and removed\n>> it.\n>\n> Please don't.  Technically it's allowed by the license if I understand\n> correctly (since the copyright notices are not accompanied by a\n> disclaimer of warranty) but it's almost always the wrong thing to do\n> to remove a copyright notice without the author's permission.\n\nUm, ok.  I still don't know what to do:\n1. Should I leave it in builtin/revert.c?  There are only a few tiny\nfunctions left there.\n2. Should I move it to sequencer.c and and modify it accordingly?\nIt'll read Copyright (C) 2011 <Me>, based on builtin/revert.c which is\nCopyright Dscho, which in turn is based on git-revert.sh Copyright\nLinus and Junio?  Isn't this information redundant? Can't `log` and\n`blame -M -C` do a much better job?\n\nThanks.\n\n-- Ram\n"},{"id":"173331","messageId":"CALkWK0m68_N3nJD91q=WRvBmGefGYX6vs-exgxkhXajDfzvKxA@mail.gmail.com","threadId":"28072","inReplyTo":"20110811201755.GJ2277@elie.gateway.2wire.net","subject":"Re: [PATCH 6/6] sequencer: Remove sequencer state after final commit","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-12T02:19:35Z","receivedAt":"2011-08-12T02:19:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nJonathan Nieder writes:\n>        Dear sequencer, I'm commiting!\n\nInteresting.  I was worried that sequencer_count_todo() might be a bit\nof an overkill.  I can't find any other usecase for counting the\nnumber of commits in the instruction sheet right now, and it's\nunlikely that I'll find any callers for it in the future either.  I\nsuppose we can save time by just counting until we figure out that\nmore than one commit exists -- there's no need to find the exact\nnumber.\n\nThanks.\n\n-- Ram\n"},{"id":"173332","messageId":"20110812023323.GA672@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"CALkWK0nXAU+jFwyzmYC6XuPGy8Cdmbpis0Nvb-M+tQSNk_PXiQ@mail.gmail.com","subject":"Re: [PATCH 0/6] Towards a generalized sequencer","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-12T02:33:23Z","receivedAt":"2011-08-12T02:33:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> 2. Should I move it to sequencer.c and and modify it accordingly?\n> It'll read Copyright (C) 2011 <Me>, based on builtin/revert.c which is\n> Copyright Dscho, which in turn is based on git-revert.sh\n[...]\n\nIsn't it still the same built-in cherry-pick code, originally by Dscho\nand improved over time by others?  The filename is irrelevant.\n\nIf you want to add your and Christian's names to reflect\nmultiple-cherry-pick and the restructuring, I guess that would be ok.\nBetter yet, if you want to remove Dscho's name to encourage people to\nlook at the commit log and get a richer story, just ask Dscho.\n\nAsking would (1) show that you value his contribution and respect his\nwishes and (2) allow him to say, \"no, I really prefer to keep my name\nthere for such-and-such copyright-related reason\".  Of course these\nare only my own thoughts.  Maybe I'm too picky.\n"},{"id":"173333","messageId":"CALkWK0=z20TMJAMnkkjrYrDVmpf9gvXxhBgeOTDY7yNNU39Buw@mail.gmail.com","threadId":"28072","inReplyTo":"20110812023323.GA672@elie.gateway.2wire.net","subject":"Re: [PATCH 0/6] Towards a generalized sequencer","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-12T03:17:20Z","receivedAt":"2011-08-12T03:17:20Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\n[+CC: Johannes Schindelin]\n\nJonathan Nieder writes:\n> Isn't it still the same built-in cherry-pick code, originally by Dscho\n> and improved over time by others?  The filename is irrelevant.\n\nTrue, but we can't have \"This implements the builtins revert and\ncherry-pick.\" in sequencer.c, no?\n\n> If you want to add your and Christian's names to reflect\n> multiple-cherry-pick and the restructuring, I guess that would be ok.\n> Better yet, if you want to remove Dscho's name to encourage people to\n> look at the commit log and get a richer story, just ask Dscho.\n\nI thought about this a bit.  I'm completely against the former option\n-- I merely meant it as a \"I'm confused; help!\" rather than a\nsuggestion; adding contributors' names to the list is a bad idea.  I\nlooked at some copyright notices in other files and re-evaluated: it's\na cute historical note to have. Unless Johannes wants to remove it\nnow, I'd like to put the following in sequencer.c:\n\n/*\n * Used to implement the builtins revert and cherry-pick.\n *\n * Copyright (c) 2007 Johannes E. Schindelin\n *\n * Based on git-revert.sh, which is\n *\n * Copyright (c) 2005 Linus Torvalds\n * Copyright (c) 2005 Junio C Hamano\n */\n\nSorry about the confusion. Feel free to suggest a better \"title line\".\n\nThanks.\n\n-- Ram\n"},{"id":"173446","messageId":"CALkWK0mP50YPAuskBueokUpAHaPBRSh7yc8sR6OQ8cVM4hQppg@mail.gmail.com","threadId":"28072","inReplyTo":"20110811192047.GE2277@elie.gateway.2wire.net","subject":"Re: [PATCH 1/6] revert: Don't remove the sequencer state on error","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T12:33:32Z","receivedAt":"2011-08-13T12:33:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nJonathan Nieder writes:\n> Why bother,\n> when the behavior is suppressed altogether later in the series?\n\nActually, this patch helped me while debugging:  I'd originally\nintended for it to go into 'maint'.  Things have changed though- the\n'sequencer-stable' is close to hitting 'next', and this series seems\nto be close behind*; I'll drop this patch altogether during the\nre-roll.\n\nThanks for making me explain this :)\n\n-- Ram\n"},{"id":"173448","messageId":"CALkWK0migSRUmhPp0069O_NiRs3gQJbrU8QLdwUJ-kUYAsLz4Q@mail.gmail.com","threadId":"28072","inReplyTo":"20110811215650.GA13839@elie.gateway.2wire.net","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T14:00:03Z","receivedAt":"2011-08-13T14:00:03Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi again,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n>> --- a/sequencer.h\n>> +++ b/sequencer.h\n>> @@ -7,7 +7,32 @@\n>>  #define SEQ_TODO_FILE        \"sequencer/todo\"\n>>  #define SEQ_OPTS_FILE        \"sequencer/opts\"\n>>\n>> +#define COMMIT_MESSAGE_INIT  { NULL, NULL, NULL, NULL, NULL };\n>\n> I don't think this should be exposed.  The rest seems pretty sane,\n> though I haven't read the patch carefully.\n\nGah, my stupidity.  What sense does it make to expose\nCOMMIT_MESSAGE_INIT when 'struct commit_message' itself isn't?  Moved\nto sequencer.c now, thanks.\n\n> Another thought.  I wonder if it's possible to leave\n> sequencer_parse_args() private to builtin/revert.c, making the split\n> a little more logical:\n\nYes, I'd like this too.  However, there are two new issues:\nrevert_or_cherry_pick_usage and action_name.  The former has two\ncallsites: one in prepare_revs (in sequencer.c) and another in\nparse_args (in builtin/revert.c).  Unfortunately, action_name is even\nmore complicated to get rid of: the information from it is used all\nover the place.  Attempting to attack the problems one by one:\n1. Make prepare_revs and walk_revs_populate_todo return errors to be\nhandled by callers.  This is a fairly small patch that can come before\nthe big \"code moving patch\".\n2. Duplicate action_name in both files.  I don't think it's too\nserious, and we can fix this later.\n\nIt has been enormously annoying to work with this \"code moving patch\":\neverytime I make some changes to the earlier patches, I have to\nrecreate this one by hand; rebase offers no help whatsoever.  After\nthrowing away code based on this patch several times, I learnt my\nlesson and restricted my series to avoid building on this patch.  I\nconsider this a very serious glitch* and I'm interested in fixing it.\nThoughts?\n\nThanks.\n\n* We don't track renames, and I fully subscribe to that design.\nHowever, that doesn't prevent us from building small helpers.\n\n-- Ram\n"},{"id":"173449","messageId":"CALkWK0=1cVNrHMvMqrEhFYcdV8FkETk8qT1i1oGUJxsWEH9Zww@mail.gmail.com","threadId":"28072","inReplyTo":"20110811201245.GH2277@elie.gateway.2wire.net","subject":"Re: [PATCH 4/6] revert: Allow mixed pick and revert instructions","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T16:07:51Z","receivedAt":"2011-08-13T16:07:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n>> Change the way the instruction parser works, allowing arbitrary\n>> (action, operand) pairs to be parsed.\n>\n>        Parse the instruction list in .git/sequencer/todo as a list\n>        of (action, operand) pairs, instead of assuming all instructions\n>        use the same action.\n\nFixed.  Thanks :)\n\n>> @@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n>>               /* TRANSLATORS: The first %s will be \"revert\" or\n>>                  \"cherry-pick\", the second %s a SHA1 */\n>>               return error(_(\"%s: cannot parse parent commit %s\"),\n>> -                     action_name(opts), sha1_to_hex(parent->object.sha1));\n>> +                     action == REPLAY_REVERT ? \"revert\" : \"cherry-pick\",\n>> +                     sha1_to_hex(parent->object.sha1));\n>\n> My first thought was \"why stop using the helper function action_name\"?\n> But now I see that it previously came from \"opts\" (i.e., the command\n> line) and now comes from the todo file.\n>\n> The command name there was never really important except when\n> cherry-pick or revert is being called by a script, and the message\n> indicates which command was having trouble parsing the commit.  If I\n> am using \"git cherry-pick --continue\" to continue after a failed\n> revert, I suspect action_name(opts) [\"cherry-pick: \"] would actually be\n> more sensible than the command name corresponding to the particular\n> pick/revert line.\n\nYou're right.  Removed this hunk.\n\n> Maybe something like\n>\n>        len = strchrnul(p, '\\n') - p;\n>        if (len > 255)\n>                len = 255;\n>        return error(_(\"Unrecognized action: %.*s\"), (int) len, p);\n>\n> would do.\n\nExcellent idea!  I fixed the buffer overflow message to do this too.\nBy the way, shouldn't error() do this?\n\n> Could we can make this error message more clearly suggest that it's\n> giving context to the error above it?  For example, something vaguely\n> like\n>\n>        error: unrecognized action: reset c78a78c9 Going back\n>        error: on line 7\n\nGood suggestion.  Fixed.\n\n> Does a \"cherry-pick --continue\" in this scenario skip the first commit\n> in the todo list?  Should it?\n\nIt shouldn't and it doesn't.  See how read_populate_opts and\nread_populate_todo are called before the segment that drops the first\ncommit.\n\n-- Ram\n"},{"id":"173451","messageId":"alpine.LNX.2.00.1108131215170.2056@iabervon.org","threadId":"28072","inReplyTo":"CALkWK0migSRUmhPp0069O_NiRs3gQJbrU8QLdwUJ-kUYAsLz4Q@mail.gmail.com","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2011-08-13T16:45:37Z","receivedAt":"2011-08-13T16:45:37Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sat, 13 Aug 2011, Ramkumar Ramachandra wrote:\n\n> Hi again,\n> \n> Jonathan Nieder writes:\n> > Another thought.  I wonder if it's possible to leave\n> > sequencer_parse_args() private to builtin/revert.c, making the split\n> > a little more logical:\n> \n> Yes, I'd like this too.  However, there are two new issues:\n> revert_or_cherry_pick_usage and action_name.  The former has two\n> callsites: one in prepare_revs (in sequencer.c) and another in\n> parse_args (in builtin/revert.c).  Unfortunately, action_name is even\n> more complicated to get rid of: the information from it is used all\n> over the place.\n\nOne thing to consider is that sequencer.c will be used by all sorts of \ndifferent builtins, which are each implementing instructions given \ndifferently by the user. Just because a message makes sense as output from \nrevert.c doesn't mean it will make sense from (for example) bisect*.\n\n> Attempting to attack the problems one by one:\n> 1. Make prepare_revs and walk_revs_populate_todo return errors to be\n> handled by callers.  This is a fairly small patch that can come before\n> the big \"code moving patch\".\n\nThis makes sense. If you type \"git log --stat=foo\", you don't get a \ndiff usage message, even though it's an error parsing options that were \noriginally part of diff.\n\n> 2. Duplicate action_name in both files.  I don't think it's too\n> serious, and we can fix this later.\n\nThis is actually probably even a good idea, because the two functions \nmight actually want to give different results. Maybe revert.c will end up \ndoing different sequencer operations depending on whether the commit is a \nmerge, but if revert.c has to give an error, it would call it the same \nthing either way, because the difference doesn't matter at the level of \ndetail the revert.c works at; on the other hand, sequencer.c would want to \ndistinguish the cases so that it is explaining exactly what it's trying to \ndo in this step because it matters to how the issue would be resolved.\n\nOf course, at the point where you move the code, you only have one piece \nof code that you're starting from, so they'll be the same. But you might \nwant to name them differently.\n\n> It has been enormously annoying to work with this \"code moving patch\":\n> everytime I make some changes to the earlier patches, I have to\n> recreate this one by hand; rebase offers no help whatsoever.  After\n> throwing away code based on this patch several times, I learnt my\n> lesson and restricted my series to avoid building on this patch.  I\n> consider this a very serious glitch and I'm interested in fixing it.\n> Thoughts?\n\nIt's a hard problem, although likely worthwhile to solve. But only when \nyou're not doing anything else, because it's complicated and will take you \nfar afield. Essentially, what you need to do is implement a diff algorithm \nthat can detect reorganization (or copying) of sections; this isn't \nsomething you can represent in unified diff output, but that's okay \nbecause you're not going to output it. You merge two of these results and \napply the result to the base, which gives you a file (potentially with \nconflicts, which is another interesting issue because you have to \nrepresent and explain them somehow).\n\n*: bisect could be using sequencer in order to handle the situation where \nthe user has said \"commit A is good, commit B is bad, commit C breaks my \nsystem in a way that's unrelated\"; the system should then be able to check \nout a maybe-bad commit and revert C from it, but it would be doing this in \nresponse to an instruction from the user: \"give me something to test \nnext\", and would have to present errors differently.\n\n\t-Daniel\n*This .sig left intentionally blank*"},{"id":"173452","messageId":"20110813170623.GB1494@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"CALkWK0migSRUmhPp0069O_NiRs3gQJbrU8QLdwUJ-kUYAsLz4Q@mail.gmail.com","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-13T17:06:23Z","receivedAt":"2011-08-13T17:06:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Jonathan Nieder writes:\n\n>> Another thought.  I wonder if it's possible to leave\n>> sequencer_parse_args() private to builtin/revert.c, making the split\n>> a little more logical:\n>\n> Yes, I'd like this too.  However, there are two new issues:\n> revert_or_cherry_pick_usage and action_name.  The former has two\n> callsites: one in prepare_revs (in sequencer.c) and another in\n> parse_args (in builtin/revert.c).\n\nSo it sounds like the answer is \"no, it's not possible without\nfurther changes\".  Alas. :)  Thanks for checking.\n\n(Q. Wait, what further changes?\n\n A. The above suggests that the setup_revisions call should also be\n    the responsibility of the builtin, and that it could communicate\n    revs and other rev-list options using a\n\n\tstruct rev_info *revs;\n\n    instead of\n\n\tint commit_argc;\n\tconst char **commit_argv;\n\n    Like this, maybe:\n\n builtin/revert.c |   52 ++++++++++++++++++++++++++++++----------------------\n 1 files changed, 30 insertions(+), 22 deletions(-)\n\ndiff --git i/builtin/revert.c w/builtin/revert.c\nindex 8b452e81..f602ece0 100644\n--- i/builtin/revert.c\n+++ w/builtin/revert.c\n@@ -55,13 +55,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 the default subcommand (\"git revert <revs>\") */\n+\tstruct rev_info *revs;\n };\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n@@ -164,7 +165,7 @@ 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+\targc = 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 \n@@ -201,9 +202,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@@ -211,7 +209,23 @@ 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\tif (argc > 1)\n+\t\t\tusage_with_options(usage_str, options);\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+\n+\t\tif (argc < 2)\n+\t\t\tusage_with_options(usage_str, options);\n+\n+\t\targc = setup_revisions(argc, argv, opts->revs, NULL);\n+\t\tif (argc > 1)\n+\t\t\tusage_with_options(usage_str, options);\n+\t}\n }\n \n struct commit_message {\n@@ -612,23 +626,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@@ -825,14 +831,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@@ -955,6 +960,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/*\n-- \n)\n"},{"id":"173455","messageId":"CALkWK0=seEY+O8qmNKoPyYPW-QT9zpwGsh8SB89qcd6kUQRdKg@mail.gmail.com","threadId":"28072","inReplyTo":"alpine.LNX.2.00.1108131215170.2056@iabervon.org","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T17:40:35Z","receivedAt":"2011-08-13T17:40:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Daniel,\n\nDaniel Barkalow writes:\n>> 1. Make prepare_revs and walk_revs_populate_todo return errors to be\n>> handled by callers.  This is a fairly small patch that can come before\n>> the big \"code moving patch\".\n>\n> This makes sense. If you type \"git log --stat=foo\", you don't get a\n> diff usage message, even though it's an error parsing options that were\n> originally part of diff.\n\nYes :) I really liked the \"revert: Propagate errors upwards from\ndo_pick_commit\" patch.  I hope we improve the error handling in other\nparts of Git soon.\n\n>> 2. Duplicate action_name in both files.  I don't think it's too\n>> serious, and we can fix this later.\n>\n> This is actually probably even a good idea, because the two functions\n> might actually want to give different results. Maybe revert.c will end up\n> doing different sequencer operations depending on whether the commit is a\n> merge, but if revert.c has to give an error, it would call it the same\n> thing either way, because the difference doesn't matter at the level of\n> detail the revert.c works at; on the other hand, sequencer.c would want to\n> distinguish the cases so that it is explaining exactly what it's trying to\n> do in this step because it matters to how the issue would be resolved.\n>\n> Of course, at the point where you move the code, you only have one piece\n> of code that you're starting from, so they'll be the same. But you might\n> want to name them differently.\n\nExcellent suggestion- I want to create a differentiation between the\nsequencer \"pick\" action and the builtin \"cherry-pick\" operation.  I\nmentioned earlier that I wanted the sequencer to be more than a fast\ngit-shell.  It'll be interesting to see the kinds of composite\n\"actions\" we invent later; especially ones that only make sense in the\ncontext of sequencing commits.\n\n>> It has been enormously annoying to work with this \"code moving patch\":\n>> everytime I make some changes to the earlier patches, I have to\n>> recreate this one by hand; rebase offers no help whatsoever.  After\n>> throwing away code based on this patch several times, I learnt my\n>> lesson and restricted my series to avoid building on this patch.  I\n>> consider this a very serious glitch and I'm interested in fixing it.\n>> Thoughts?\n>\n> It's a hard problem, although likely worthwhile to solve. But only when\n> you're not doing anything else, because it's complicated and will take you\n> far afield. Essentially, what you need to do is implement a diff algorithm\n> that can detect reorganization (or copying) of sections; this isn't\n> something you can represent in unified diff output, but that's okay\n> because you're not going to output it. You merge two of these results and\n> apply the result to the base, which gives you a file (potentially with\n> conflicts, which is another interesting issue because you have to\n> represent and explain them somehow).\n\nHm, I was actually thinking of a much less ambitious helper that would\nkick in when the heuristic correlation between two files is above a\ncertain threshold.  Your idea is better!  It would be totally awesome\nif we could modify the diffing algorithm to work between files,\nalthough I can't even imagine where to start.  Oh, and I think we have\nto go far beyond the traditional in-file conflict markers to resolve\nconflicts.  Pretty insane challenge :D  I'm definitely not experienced\nenough to take this on now, although I hope to be good enough someday.\n\n> *: bisect could be using sequencer in order to handle the situation where\n> the user has said \"commit A is good, commit B is bad, commit C breaks my\n> system in a way that's unrelated\"; the system should then be able to check\n> out a maybe-bad commit and revert C from it, but it would be doing this in\n> response to an instruction from the user: \"give me something to test\n> next\", and would have to present errors differently.\n\nThanks for the interesting sidenote.\n\n-- Ram\n"},{"id":"173456","messageId":"20110813175012.GD1494@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"CALkWK0=seEY+O8qmNKoPyYPW-QT9zpwGsh8SB89qcd6kUQRdKg@mail.gmail.com","subject":"Reusing changes after renaming a file (Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-13T17:50:13Z","receivedAt":"2011-08-13T17:50:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> Hm, I was actually thinking of a much less ambitious helper that would\n> kick in when the heuristic correlation between two files is above a\n> certain threshold.\n\nDoesn't that already work?  For example:\n\n\tgit cherry-pick -Xrename-threshold=50 foo\n"},{"id":"173457","messageId":"CALkWK0mDdYTahhDx9_7-4AP8iuvkWMu1wHhQnmUcatEXiDiuTA@mail.gmail.com","threadId":"28072","inReplyTo":"20110813175012.GD1494@elie.gateway.2wire.net","subject":"Re: Reusing changes after renaming a file (Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery)","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T18:20:42Z","receivedAt":"2011-08-13T18:20:42Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi Jonathan,\n\nDisclaimer: I'm sorry if I sound incredibly stupid.  I know nothing\nabout how the diffing algorithm works.\n\nJonathan Nieder writes:\n> Ramkumar Ramachandra wrote:\n>> Hm, I was actually thinking of a much less ambitious helper that would\n>> kick in when the heuristic correlation between two files is above a\n>> certain threshold.\n>\n> Doesn't that already work?  For example:\n>\n>        git cherry-pick -Xrename-threshold=50 foo\n\nOkay, I'm still trying to figure out what this does.  It's supposed to\n\"influence the rename detection\" according to the 1.7.4 release notes.\n I tried:\n\n$ git cherry-pick -Xrename-threshold=90 sequencer~1\n[test 7400609] sequencer: Expose API to cherry-picking machinery\n 3 files changed, 256 insertions(+), 1247 deletions(-)\n rewrite builtin/revert.c (81%)\n rename builtin/revert.c => sequencer.c (81%)\n\nIs it supposed to influence the diffstat?  I don't see any conflict\nmarkers in sequencer.c.  I can see lots of unintelligent (and useless)\nconflict markers in builtin/revert.c.  What am I supposed to be\nlooking at?\n\n-- Ram\n"},{"id":"173458","messageId":"20110813183211.GF1494@elie.gateway.2wire.net","threadId":"28072","inReplyTo":"CALkWK0mDdYTahhDx9_7-4AP8iuvkWMu1wHhQnmUcatEXiDiuTA@mail.gmail.com","subject":"Re: Reusing changes after renaming a file (Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery)","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-13T18:32:11Z","receivedAt":"2011-08-13T18:32:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> $ git cherry-pick -Xrename-threshold=90 sequencer~1\n> [test 7400609] sequencer: Expose API to cherry-picking machinery\n>  3 files changed, 256 insertions(+), 1247 deletions(-)\n>  rewrite builtin/revert.c (81%)\n>  rename builtin/revert.c => sequencer.c (81%)\n>\n> Is it supposed to influence the diffstat?  I don't see any conflict\n> markers in sequencer.c.  I can see lots of unintelligent (and useless)\n> conflict markers in builtin/revert.c.\n\nPerhaps \"git merge-recursive\" and its callers don't use the equivalent\nof -B, so it doesn't look like a rename to them.  Cases involving\nplain renames work fine for me.\n"},{"id":"173464","messageId":"CALkWK0mNaK5mysf76LhzkZPXX5eKkNGezFQrBZjru9G96SqbYw@mail.gmail.com","threadId":"28072","inReplyTo":"20110813170623.GB1494@elie.gateway.2wire.net","subject":"Re: [PATCH 5/6] sequencer: Expose API to cherry-picking machinery","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2011-08-13T20:54:52Z","receivedAt":"2011-08-13T20:54:52Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nJonathan Nieder writes:\n>  builtin/revert.c |   52 ++++++++++++++++++++++++++++++----------------------\n>  1 files changed, 30 insertions(+), 22 deletions(-)\n\nNice! That fixes everything :)\nI'll send out a fresh iteration in the morning.\n\nThanks.\n\n-- Ram\n"}]}