{"thread":{"id":"25843","subject":"[RFC/PATCH 00/18] WIP implement cherry-pick/revert --continue","startedAt":"2010-11-25T21:20:31Z","lastAt":"2010-11-27T03:50:55Z","messageCount":27,"participants":["Christian Couder","Jonathan Nieder","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":18},"messages":[{"id":"156598","messageId":"20101125210138.5188.13115.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":null,"subject":"[RFC/PATCH 00/18] WIP implement cherry-pick/revert --continue","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:31Z","receivedAt":"2010-11-25T21:20:31Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This a a work in progress to show where this is going and to discuss it.\n\nThere are many things missing among other:\n\n * no documentation\n * missing tests, especially for \"revert\"\n * when cherry-pick fails the error message does not advertise --continue\n * resolving a failing cherry-pick is not handled\n * commit messages could be improved a lot\n * ...\n\nMany patches in this series are replacing calls to \"die()\" by\n\"return error()\", because the TODO and DONE files are written\nonly when cherry-pick fails. This is efficient but perhaps it\nwould be simpler and safer to write them before each cherry-pick\njust in case it fails, so that the \"die()\" calls don't need to\nbe removed.\n\nChristian Couder (17):\n advice: add error_resolve_conflict() function\n revert: change many die() calls into \"return error()\" calls\n usage: implement error_errno() the same way as die_errno()\n revert: don't die when write_message() fails\n commit: move reverse_commit_list() into commit.{h,c}\n revert: remove \"commit\" global variable\n revert: put option information in an option struct\n revert: refactor code into a new pick_commits() function\n revert: make pick_commits() return an error on --ff incompatible\n option\n revert: make read_and_refresh_cache() and prepare_revs() return\n errors\n revert: add get_todo_content() and create_todo_file()\n revert: write TODO and DONE files in case of failure\n revert: add option parsing for option --continue\n revert: move global variable \"me\" into \"struct args_info\"\n revert: add NONE action and make parse_args() manage it\n revert: add remaining instructions in todo file\n revert: implement --continue processing\n\nStephan Beyer (1):\n revert: implement parsing TODO and DONE files\n\n advice.c | 25 +-\n advice.h | 1 +\n builtin/revert.c | 692 ++++++++++++++++++++++++++++-------\n commit.c | 11 +\n commit.h | 2 +\n git-compat-util.h | 1 +\n merge-recursive.c | 11 -\n t/t3508-cherry-pick-many-commits.sh | 101 +++++\n usage.c | 28 ++-\n 9 files changed, 717 insertions(+), 155 deletions(-)\n\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156599","messageId":"20101125212050.5188.50630.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 01/18] advice: add error_resolve_conflict() function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:32Z","receivedAt":"2010-11-25T21:20:32Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"in case we don't want to die, but still want the right\nerror message to be printed.\n\nSigned-off-by: Christian Couder \n---\n advice.c | 25 +++++++++++++++++++++----\n advice.h | 1 +\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 0be4b5f..d4ba29f 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -34,6 +34,13 @@ int git_default_advice_config(const char *var, const char *value)\n \treturn 0;\n }\n \n+const char unmerged_file_advice[] =\n+\t\"'%s' is not possible because you have unmerged files.\\n\"\n+\t\"Please, fix them up in the work tree, and then use 'git add/rm ' as\\n\"\n+\t\"appropriate to mark resolution and make a commit, or use 'git commit -a'.\";\n+const char unmerged_file_no_advice[] =\n+\t\"'%s' is not possible because you have unmerged files.\";\n+\n void NORETURN die_resolve_conflict(const char *me)\n {\n \tif (advice_resolve_conflict)\n@@ -41,9 +48,19 @@ void NORETURN die_resolve_conflict(const char *me)\n \t\t * Message used both when 'git commit' fails and when\n \t\t * other commands doing a merge do.\n \t\t */\n-\t\tdie(\"'%s' is not possible because you have unmerged files.\\n\"\n-\t\t \"Please, fix them up in the work tree, and then use 'git add/rm ' as\\n\"\n-\t\t \"appropriate to mark resolution and make a commit, or use 'git commit -a'.\", me);\n+\t\tdie(unmerged_file_advice, me);\n+\telse\n+\t\tdie(unmerged_file_no_advice, me);\n+}\n+\n+int error_resolve_conflict(const char *me)\n+{\n+\tif (advice_resolve_conflict)\n+\t\t/*\n+\t\t * Message used both when 'git commit' fails and when\n+\t\t * other commands doing a merge do.\n+\t\t */\n+\t\treturn error(unmerged_file_advice, me);\n \telse\n-\t\tdie(\"'%s' is not possible because you have unmerged files.\", me);\n+\t\treturn error(unmerged_file_no_advice, me);\n }\ndiff --git a/advice.h b/advice.h\nindex 3244ebb..7b7cea5 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -13,5 +13,6 @@ extern int advice_detached_head;\n int git_default_advice_config(const char *var, const char *value);\n \n extern void NORETURN die_resolve_conflict(const char *me);\n+extern int error_resolve_conflict(const char *me);\n \n #endif /* ADVICE_H */\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156600","messageId":"20101125212050.5188.21758.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 02/18] revert: change many die() calls into \"return error()\" calls","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:33Z","receivedAt":"2010-11-25T21:20:33Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 43 ++++++++++++++++++++++---------------------\n 1 files changed, 22 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex bb6e9e8..9649d37 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -280,16 +280,18 @@ static struct tree *empty_tree(void)\n \treturn tree;\n }\n \n-static NORETURN void die_dirty_index(const char *me)\n+static int error_dirty_index(const char *me)\n {\n \tif (read_cache_unmerged()) {\n-\t\tdie_resolve_conflict(me);\n+\t\treturn error_resolve_conflict(me);\n \t} else {\n \t\tif (advice_commit_before_merge)\n-\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\"\n-\t\t\t \"Please, commit your changes or stash them to proceed.\", me);\n+\t\t\treturn error(\"Your local changes would be overwritten \"\n+\t\t\t\t \"by %s.\\nPlease, commit your changes or \"\n+\t\t\t\t \"stash them to proceed.\", me);\n \t\telse\n-\t\t\tdie(\"Your local changes would be overwritten by %s.\\n\", me);\n+\t\t\treturn error(\"Your local changes would be overwritten \"\n+\t\t\t\t \"by %s.\\n\", me);\n \t}\n }\n \n@@ -339,7 +341,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \tif (active_cache_changed &&\n \t (write_cache(index_fd, active_cache, active_nr) ||\n \t commit_locked_index(&index_lock)))\n-\t\tdie(\"%s: Unable to write new index file\", me);\n+\t\treturn error(\"%s: Unable to write new index file\", me);\n \trollback_lock_file(&index_lock);\n \n \tif (!clean) {\n@@ -405,18 +407,18 @@ static int do_pick_commit(void)\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\t\treturn error(\"Your index file is unmerged.\");\n \t} else {\n \t\tif (get_sha1(\"HEAD\", head))\n-\t\t\tdie (\"You do not have a valid HEAD\");\n+\t\t\treturn error(\"You do not have a valid HEAD\");\n \t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie_dirty_index(me);\n+\t\t\treturn error_dirty_index(me);\n \t}\n \tdiscard_cache();\n \n \tif (!commit->parents) {\n \t\tif (action == REVERT)\n-\t\t\tdie (\"Cannot revert a root commit\");\n+\t\t\treturn error(\"Cannot revert a root commit\");\n \t\tparent = NULL;\n \t}\n \telse if (commit->parents->next) {\n@@ -425,20 +427,19 @@ static int do_pick_commit(void)\n \t\tstruct commit_list *p;\n \n \t\tif (!mainline)\n-\t\t\tdie(\"Commit %s is a merge but no -m option was given.\",\n-\t\t\t sha1_to_hex(commit->object.sha1));\n-\n+\t\t\treturn error(\"Commit %s is a merge but no -m option \"\n+\t\t\t\t \"was given.\", sha1_to_hex(commit->object.sha1));\n \t\tfor (cnt = 1, p = commit->parents;\n \t\t cnt != mainline && p;\n \t\t cnt++)\n \t\t\tp = p->next;\n \t\tif (cnt != mainline || !p)\n-\t\t\tdie(\"Commit %s does not have parent %d\",\n-\t\t\t sha1_to_hex(commit->object.sha1), mainline);\n+\t\t\treturn error(\"Commit %s does not have parent %d\",\n+\t\t\t\t sha1_to_hex(commit->object.sha1), mainline);\n \t\tparent = p->item;\n \t} else if (0 < mainline)\n-\t\tdie(\"Mainline was specified but commit %s is not a merge.\",\n-\t\t sha1_to_hex(commit->object.sha1));\n+\t\treturn error(\"Mainline was specified but commit %s is not a merge.\",\n+\t\t\t sha1_to_hex(commit->object.sha1));\n \telse\n \t\tparent = commit->parents->item;\n \n@@ -446,12 +447,12 @@ static int do_pick_commit(void)\n \t\treturn fast_forward_to(commit->object.sha1, head);\n \n \tif (parent && parse_commit(parent) < 0)\n-\t\tdie(\"%s: cannot parse parent commit %s\",\n-\t\t me, sha1_to_hex(parent->object.sha1));\n+\t\treturn error(\"%s: cannot parse parent commit %s\",\n+\t\t\t me, sha1_to_hex(parent->object.sha1));\n \n \tif (get_message(commit->buffer, &msg) != 0)\n-\t\tdie(\"Cannot get commit message for %s\",\n-\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\treturn error(\"Cannot get commit message for %s\",\n+\t\t\t sha1_to_hex(commit->object.sha1));\n \n \t/*\n \t * \"commit\" is an existing commit. We would want to apply\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156601","messageId":"20101125212050.5188.56613.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 03/18] usage: implement error_errno() the same way as die_errno()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:34Z","receivedAt":"2010-11-25T21:20:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"die_errno() is very useful, but sometimes we don't want to\ndie after printing an error message and the error message\nfrom errno. So let's implement error_errno() that does the\nsame thing as die_errno() except that it calls\nerror_routine() instead of die_routine().\n\nSigned-off-by: Christian Couder \n---\n git-compat-util.h | 1 +\n usage.c | 28 ++++++++++++++++++++++++----\n 2 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 490f969..32294cc 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -230,6 +230,7 @@ extern NORETURN void usage(const char *err);\n extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));\n extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));\n extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));\n+extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));\n extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));\n extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));\n \ndiff --git a/usage.c b/usage.c\nindex ec4cf53..f3ac869 100644\n--- a/usage.c\n+++ b/usage.c\n@@ -69,10 +69,9 @@ void die(const char *err, ...)\n \tva_end(params);\n }\n \n-void die_errno(const char *fmt, ...)\n+static void format_errno(char *fmt_with_err, size_t fmt_with_err_size,\n+\t\t\t const char *fmt)\n {\n-\tva_list params;\n-\tchar fmt_with_err[1024];\n \tchar str_error[256], *err;\n \tint i, j;\n \n@@ -90,13 +89,34 @@ void die_errno(const char *fmt, ...)\n \t\t}\n \t}\n \tstr_error[j] = 0;\n-\tsnprintf(fmt_with_err, sizeof(fmt_with_err), \"%s: %s\", fmt, str_error);\n+\tsnprintf(fmt_with_err, fmt_with_err_size, \"%s: %s\", fmt, str_error);\n+}\n+\n+void die_errno(const char *fmt, ...)\n+{\n+\tva_list params;\n+\tchar fmt_with_err[1024];\n+\n+\tformat_errno(fmt_with_err, sizeof(fmt_with_err), fmt);\n \n \tva_start(params, fmt);\n \tdie_routine(fmt_with_err, params);\n \tva_end(params);\n }\n \n+int error_errno(const char *fmt, ...)\n+{\n+\tva_list params;\n+\tchar fmt_with_err[1024];\n+\n+\tformat_errno(fmt_with_err, sizeof(fmt_with_err), fmt);\n+\n+\tva_start(params, fmt);\n+\terror_routine(fmt_with_err, params);\n+\tva_end(params);\n+\treturn -1;\n+}\n+\n int error(const char *err, ...)\n {\n \tva_list params;\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156602","messageId":"20101125212050.5188.41232.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 04/18] revert: don't die when write_message() fails","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:35Z","receivedAt":"2010-11-25T21:20:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Instead we will just return an error code.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 18 ++++++++++++------\n 1 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 9649d37..947e666 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -257,17 +257,19 @@ static void print_advice(void)\n \t\t find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));\n }\n \n-static void write_message(struct strbuf *msgbuf, const char *filename)\n+static int 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+\t\treturn error_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+\t\treturn error(\"Error wrapping up %s\", filename);\n+\n+\treturn 0;\n }\n \n static struct tree *empty_tree(void)\n@@ -397,7 +399,7 @@ static int do_pick_commit(void)\n \tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n \tchar *defmsg = NULL;\n \tstruct strbuf msgbuf = STRBUF_INIT;\n-\tint res;\n+\tint res, write_res;\n \n \tif (no_commit) {\n \t\t/*\n@@ -495,12 +497,16 @@ static int do_pick_commit(void)\n \tif (!strategy || !strcmp(strategy, \"recursive\") || action == REVERT) {\n \t\tres = do_recursive_merge(base, next, base_label, next_label,\n \t\t\t\t\t head, &msgbuf);\n-\t\twrite_message(&msgbuf, defmsg);\n+\t\twrite_res = write_message(&msgbuf, defmsg);\n+\t\tif (write_res)\n+\t\t\treturn write_res;\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+\t\twrite_res = write_message(&msgbuf, defmsg);\n+\t\tif (write_res)\n+\t\t\treturn write_res;\n \n \t\tcommit_list_insert(base, &common);\n \t\tcommit_list_insert(next, &remotes);\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156604","messageId":"20101125212050.5188.71328.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 05/18] commit: move reverse_commit_list() into commit.{h, c}","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:36Z","receivedAt":"2010-11-25T21:20:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n commit.c | 11 +++++++++++\n commit.h | 2 ++\n merge-recursive.c | 11 -----------\n 3 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 0094ec1..f9d5bf9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -351,6 +351,17 @@ unsigned commit_list_count(const struct commit_list *l)\n \treturn c;\n }\n \n+struct commit_list *reverse_commit_list(struct commit_list *list)\n+{\n+\tstruct commit_list *next = NULL, *current, *backup;\n+\tfor (current = list; current; current = backup) {\n+\t\tbackup = current->next;\n+\t\tcurrent->next = next;\n+\t\tnext = current;\n+\t}\n+\treturn next;\n+}\n+\n void free_commit_list(struct commit_list *list)\n {\n \twhile (list) {\ndiff --git a/commit.h b/commit.h\nindex 9113bbe..6a074a1 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -48,6 +48,8 @@ struct commit_list * commit_list_insert(struct commit *item, struct commit_list\n unsigned commit_list_count(const struct commit_list *l);\n struct commit_list * insert_by_date(struct commit *item, struct commit_list **list);\n \n+struct commit_list *reverse_commit_list(struct commit_list *list);\n+\n void free_commit_list(struct commit_list *list);\n \n void sort_by_date(struct commit_list **list);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 16c2dbe..d02fda6 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1573,17 +1573,6 @@ int merge_trees(struct merge_options *o,\n \treturn clean;\n }\n \n-static struct commit_list *reverse_commit_list(struct commit_list *list)\n-{\n-\tstruct commit_list *next = NULL, *current, *backup;\n-\tfor (current = list; current; current = backup) {\n-\t\tbackup = current->next;\n-\t\tcurrent->next = next;\n-\t\tnext = current;\n-\t}\n-\treturn next;\n-}\n-\n /*\n * Merge the commits h1 and h2, return the resulting virtual\n * commit object and a flag indicating the cleanness of the merge.\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156603","messageId":"20101125212050.5188.99356.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 06/18] revert: remove \"commit\" global variable","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:37Z","receivedAt":"2010-11-25T21:20:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 49 ++++++++++++++++++++++++-------------------------\n 1 files changed, 24 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 947e666..e3dea19 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -38,7 +38,6 @@ static const char * const cherry_pick_usage[] = {\n \n static int edit, no_replay, no_commit, mainline, signoff, allow_ff;\n static enum { REVERT, CHERRY_PICK } action;\n-static struct commit *commit;\n static int commit_argc;\n static const char **commit_argv;\n static int allow_rerere_auto;\n@@ -48,7 +47,7 @@ static const char *strategy;\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n-static char *get_encoding(const char *message);\n+static char *get_encoding(const char *message, const unsigned char *sha1);\n \n static const char * const *revert_or_cherry_pick_usage(void)\n {\n@@ -99,7 +98,8 @@ struct commit_message {\n \tconst char *message;\n };\n \n-static int get_message(const char *raw_message, struct commit_message *out)\n+static int get_message(const char *raw_message, const unsigned char *sha1,\n+\t\t struct commit_message *out)\n {\n \tconst char *encoding;\n \tconst char *abbrev, *subject;\n@@ -108,7 +108,7 @@ static int get_message(const char *raw_message, struct commit_message *out)\n \n \tif (!raw_message)\n \t\treturn -1;\n-\tencoding = get_encoding(raw_message);\n+\tencoding = get_encoding(raw_message, sha1);\n \tif (!encoding)\n \t\tencoding = \"UTF-8\";\n \tif (!git_commit_encoding)\n@@ -122,7 +122,7 @@ static int get_message(const char *raw_message, struct commit_message *out)\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 = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n \tabbrev_len = strlen(abbrev);\n \n \tsubject_len = find_commit_subject(out->message, &subject);\n@@ -146,13 +146,12 @@ static void free_message(struct commit_message *msg)\n \tfree(msg->reencoded_message);\n }\n \n-static char *get_encoding(const char *message)\n+static char *get_encoding(const char *message, const unsigned char *sha1)\n {\n \tconst char *p = message, *eol;\n \n \tif (!p)\n-\t\tdie (\"Could not read commit message of %s\",\n-\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\tdie (\"Could not read commit message of %s\", sha1_to_hex(sha1));\n \twhile (*p && *p != '\\n') {\n \t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n \t\t\t; /* do nothing */\n@@ -168,25 +167,25 @@ static char *get_encoding(const char *message)\n \treturn NULL;\n }\n \n-static void add_message_to_msg(struct strbuf *msgbuf, const char *message)\n+static void add_message_to_msg(struct strbuf *msgbuf, const char *message,\n+\t\t\t const unsigned char *sha1)\n {\n \tconst char *p = message;\n \twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n \t\tp++;\n \n \tif (!*p)\n-\t\tstrbuf_addstr(msgbuf, sha1_to_hex(commit->object.sha1));\n+\t\tstrbuf_addstr(msgbuf, sha1_to_hex(sha1));\n \n \tp += 2;\n \tstrbuf_addstr(msgbuf, p);\n }\n \n-static void set_author_ident_env(const char *message)\n+static void set_author_ident_env(const char *message, const unsigned char *sha1)\n {\n \tconst char *p = message;\n \tif (!p)\n-\t\tdie (\"Could not read commit message of %s\",\n-\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\tdie (\"Could not read commit message of %s\", sha1_to_hex(sha1));\n \twhile (*p && *p != '\\n') {\n \t\tconst char *eol;\n \n@@ -200,7 +199,7 @@ static void set_author_ident_env(const char *message)\n \t\t\temail = strchr(line, '<');\n \t\t\tif (!email)\n \t\t\t\tdie (\"Could not extract author email from %s\",\n-\t\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\t\t\t\tsha1_to_hex(sha1));\n \t\t\tif (email == line)\n \t\t\t\tpend = line;\n \t\t\telse\n@@ -212,7 +211,7 @@ static void set_author_ident_env(const char *message)\n \t\t\ttimestamp = strchr(email, '>');\n \t\t\tif (!timestamp)\n \t\t\t\tdie (\"Could not extract author time from %s\",\n-\t\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\t\t\t\tsha1_to_hex(sha1));\n \t\t\t*timestamp = '\\0';\n \t\t\tfor (timestamp++; *timestamp && isspace(*timestamp);\n \t\t\t\t\ttimestamp++)\n@@ -227,8 +226,7 @@ static void set_author_ident_env(const char *message)\n \t\tif (*p == '\\n')\n \t\t\tp++;\n \t}\n-\tdie (\"No author information found in %s\",\n-\t\t\tsha1_to_hex(commit->object.sha1));\n+\tdie (\"No author information found in %s\", sha1_to_hex(sha1));\n }\n \n static void advise(const char *advice, ...)\n@@ -240,7 +238,7 @@ static void advise(const char *advice, ...)\n \tva_end(params);\n }\n \n-static void print_advice(void)\n+static void print_advice(const unsigned char *sha1)\n {\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n \n@@ -254,7 +252,7 @@ static void print_advice(void)\n \n \tif (action == CHERRY_PICK)\n \t\tadvise(\"and commit the result with 'git commit -c %s'\",\n-\t\t find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));\n+\t\t find_unique_abbrev(sha1, DEFAULT_ABBREV));\n }\n \n static int write_message(struct strbuf *msgbuf, const char *filename)\n@@ -391,7 +389,7 @@ static int run_git_commit(const char *defmsg)\n \treturn run_command_v_opt(args, RUN_GIT_CMD);\n }\n \n-static int do_pick_commit(void)\n+static int do_pick_commit(struct commit *commit)\n {\n \tunsigned char head[20];\n \tstruct commit *base, *next, *parent;\n@@ -452,7 +450,7 @@ static int do_pick_commit(void)\n \t\treturn error(\"%s: cannot parse parent commit %s\",\n \t\t\t me, sha1_to_hex(parent->object.sha1));\n \n-\tif (get_message(commit->buffer, &msg) != 0)\n+\tif (get_message(commit->buffer, commit->object.sha1, &msg) != 0)\n \t\treturn error(\"Cannot get commit message for %s\",\n \t\t\t sha1_to_hex(commit->object.sha1));\n \n@@ -485,8 +483,8 @@ static int do_pick_commit(void)\n \t\tbase_label = msg.parent_label;\n \t\tnext = commit;\n \t\tnext_label = msg.label;\n-\t\tset_author_ident_env(msg.message);\n-\t\tadd_message_to_msg(&msgbuf, msg.message);\n+\t\tset_author_ident_env(msg.message, commit->object.sha1);\n+\t\tadd_message_to_msg(&msgbuf, msg.message, commit->object.sha1);\n \t\tif (no_replay) {\n \t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n@@ -521,7 +519,7 @@ static int do_pick_commit(void)\n \t\t action == REVERT ? \"revert\" : \"apply\",\n \t\t find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n \t\t msg.subject);\n-\t\tprint_advice();\n+\t\tprint_advice(commit->object.sha1);\n \t\trerere(allow_rerere_auto);\n \t} else {\n \t\tif (!no_commit)\n@@ -572,6 +570,7 @@ static void read_and_refresh_cache(const char *me)\n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tstruct rev_info revs;\n+\tstruct commit *commit;\n \n \tgit_config(git_default_config, NULL);\n \tme = action == REVERT ? \"revert\" : \"cherry-pick\";\n@@ -594,7 +593,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \tprepare_revs(&revs);\n \n \twhile ((commit = get_revision(&revs))) {\n-\t\tint res = do_pick_commit();\n+\t\tint res = do_pick_commit(commit);\n \t\tif (res)\n \t\t\treturn res;\n \t}\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156606","messageId":"20101125212050.5188.8316.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 07/18] revert: put option information in an option struct","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:38Z","receivedAt":"2010-11-25T21:20:38Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is needed because we want to reuse the parse_args() function\nso that we can parse options saved in a TODO file.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 148 +++++++++++++++++++++++++++++-------------------------\n 1 files changed, 79 insertions(+), 69 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex e3dea19..443b529 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -36,58 +36,68 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-static int edit, no_replay, no_commit, mainline, signoff, allow_ff;\n-static enum { REVERT, CHERRY_PICK } action;\n-static int commit_argc;\n-static const char **commit_argv;\n-static int allow_rerere_auto;\n-\n static const char *me;\n-static const char *strategy;\n+\n+struct args_info {\n+\tenum { REVERT, CHERRY_PICK } action;\n+\tint edit;\n+\tint no_replay;\n+\tint no_commit;\n+\tint mainline;\n+\tint signoff;\n+\tint allow_ff;\n+\tint allow_rerere_auto;\n+\tconst char *strategy;\n+\tconst char **commit_argv;\n+\tint commit_argc;\n+\tconst char * const * usage_str;\n+};\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n static char *get_encoding(const char *message, const unsigned char *sha1);\n \n-static const char * const *revert_or_cherry_pick_usage(void)\n+static const char * const *revert_or_cherry_pick_usage(struct args_info *info)\n {\n-\treturn action == REVERT ? revert_usage : cherry_pick_usage;\n+\treturn info->action == REVERT ? revert_usage : cherry_pick_usage;\n }\n \n-static void parse_args(int argc, const char **argv)\n+static void parse_args(int argc, const char **argv, struct args_info *info)\n {\n-\tconst char * const * usage_str = revert_or_cherry_pick_usage();\n \tint noop;\n \tstruct option options[] = {\n-\t\tOPT_BOOLEAN('n', \"no-commit\", &no_commit, \"don't automatically commit\"),\n-\t\tOPT_BOOLEAN('e', \"edit\", &edit, \"edit the commit message\"),\n+\t\tOPT_BOOLEAN('n', \"no-commit\", &info->no_commit,\n+\t\t\t \"don't automatically commit\"),\n+\t\tOPT_BOOLEAN('e', \"edit\", &info->edit, \"edit the commit message\"),\n \t\tOPT_BOOLEAN('r', NULL, &noop, \"no-op (backward compatibility)\"),\n-\t\tOPT_BOOLEAN('s', \"signoff\", &signoff, \"add Signed-off-by:\"),\n-\t\tOPT_INTEGER('m', \"mainline\", &mainline, \"parent number\"),\n-\t\tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n-\t\tOPT_STRING(0, \"strategy\", &strategy, \"strategy\", \"merge strategy\"),\n+\t\tOPT_BOOLEAN('s', \"signoff\", &info->signoff, \"add Signed-off-by:\"),\n+\t\tOPT_INTEGER('m', \"mainline\", &info->mainline, \"parent number\"),\n+\t\tOPT_RERERE_AUTOUPDATE(&info->allow_rerere_auto),\n+\t\tOPT_STRING(0, \"strategy\", &info->strategy, \"strategy\",\n+\t\t\t \"merge strategy\"),\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t};\n \n-\tif (action == CHERRY_PICK) {\n+\tif (info->action == CHERRY_PICK) {\n \t\tstruct option cp_extra[] = {\n-\t\t\tOPT_BOOLEAN('x', NULL, &no_replay, \"append commit name\"),\n-\t\t\tOPT_BOOLEAN(0, \"ff\", &allow_ff, \"allow fast-forward\"),\n+\t\t\tOPT_BOOLEAN('x', NULL, &info->no_replay, \"append commit name\"),\n+\t\t\tOPT_BOOLEAN(0, \"ff\", &info->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-\tcommit_argc = parse_options(argc, argv, NULL, options, usage_str,\n-\t\t\t\t PARSE_OPT_KEEP_ARGV0 |\n-\t\t\t\t PARSE_OPT_KEEP_UNKNOWN);\n-\tif (commit_argc < 2)\n-\t\tusage_with_options(usage_str, options);\n+\tinfo->usage_str = revert_or_cherry_pick_usage(info);\n+\tinfo->commit_argc = parse_options(argc, argv, NULL, options, info->usage_str,\n+\t\t\t\t\t PARSE_OPT_KEEP_ARGV0 |\n+\t\t\t\t\t PARSE_OPT_KEEP_UNKNOWN);\n+\tinfo->commit_argv = argv;\n \n-\tcommit_argv = argv;\n+\tif (info->commit_argc < 2)\n+\t\tusage_with_options(info->usage_str, options);\n }\n \n struct commit_message {\n@@ -238,7 +248,7 @@ static void advise(const char *advice, ...)\n \tva_end(params);\n }\n \n-static void print_advice(const unsigned char *sha1)\n+static void print_advice(struct args_info *info, const unsigned char *sha1)\n {\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n \n@@ -250,7 +260,7 @@ static void print_advice(const unsigned char *sha1)\n \tadvise(\"after resolving the conflicts, mark the corrected paths\");\n \tadvise(\"with 'git add ' or 'git rm '\");\n \n-\tif (action == CHERRY_PICK)\n+\tif (info->action == CHERRY_PICK)\n \t\tadvise(\"and commit the result with 'git commit -c %s'\",\n \t\t find_unique_abbrev(sha1, DEFAULT_ABBREV));\n }\n@@ -370,7 +380,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\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)\n+static int run_git_commit(const char *defmsg, struct args_info *info)\n {\n \t/* 6 is max possible length of our args array including NULL */\n \tconst char *args[6];\n@@ -378,9 +388,9 @@ static int run_git_commit(const char *defmsg)\n \n \targs[i++] = \"commit\";\n \targs[i++] = \"-n\";\n-\tif (signoff)\n+\tif (info->signoff)\n \t\targs[i++] = \"-s\";\n-\tif (!edit) {\n+\tif (!info->edit) {\n \t\targs[i++] = \"-F\";\n \t\targs[i++] = defmsg;\n \t}\n@@ -389,7 +399,7 @@ static int run_git_commit(const char *defmsg)\n \treturn run_command_v_opt(args, RUN_GIT_CMD);\n }\n \n-static int do_pick_commit(struct commit *commit)\n+static int do_pick_commit(struct args_info *info, struct commit *commit)\n {\n \tunsigned char head[20];\n \tstruct commit *base, *next, *parent;\n@@ -399,7 +409,7 @@ static int do_pick_commit(struct commit *commit)\n \tstruct strbuf msgbuf = STRBUF_INIT;\n \tint res, write_res;\n \n-\tif (no_commit) {\n+\tif (info->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@@ -417,7 +427,7 @@ static int do_pick_commit(struct commit *commit)\n \tdiscard_cache();\n \n \tif (!commit->parents) {\n-\t\tif (action == REVERT)\n+\t\tif (info->action == REVERT)\n \t\t\treturn error(\"Cannot revert a root commit\");\n \t\tparent = NULL;\n \t}\n@@ -426,24 +436,24 @@ static int do_pick_commit(struct commit *commit)\n \t\tint cnt;\n \t\tstruct commit_list *p;\n \n-\t\tif (!mainline)\n+\t\tif (!info->mainline)\n \t\t\treturn error(\"Commit %s is a merge but no -m option \"\n \t\t\t\t \"was given.\", sha1_to_hex(commit->object.sha1));\n \t\tfor (cnt = 1, p = commit->parents;\n-\t\t cnt != mainline && p;\n+\t\t cnt != info->mainline && p;\n \t\t cnt++)\n \t\t\tp = p->next;\n-\t\tif (cnt != mainline || !p)\n+\t\tif (cnt != info->mainline || !p)\n \t\t\treturn error(\"Commit %s does not have parent %d\",\n-\t\t\t\t sha1_to_hex(commit->object.sha1), mainline);\n+\t\t\t\t sha1_to_hex(commit->object.sha1), info->mainline);\n \t\tparent = p->item;\n-\t} else if (0 < mainline)\n+\t} else if (0 < info->mainline)\n \t\treturn error(\"Mainline was specified but commit %s is not a merge.\",\n \t\t\t sha1_to_hex(commit->object.sha1));\n \telse\n \t\tparent = commit->parents->item;\n \n-\tif (allow_ff && parent && !hashcmp(parent->object.sha1, head))\n+\tif (info->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@@ -463,7 +473,7 @@ static int do_pick_commit(struct commit *commit)\n \n \tdefmsg = git_pathdup(\"MERGE_MSG\");\n \n-\tif (action == REVERT) {\n+\tif (info->action == REVERT) {\n \t\tbase = commit;\n \t\tbase_label = msg.label;\n \t\tnext = parent;\n@@ -485,14 +495,15 @@ static int do_pick_commit(struct commit *commit)\n \t\tnext_label = msg.label;\n \t\tset_author_ident_env(msg.message, commit->object.sha1);\n \t\tadd_message_to_msg(&msgbuf, msg.message, commit->object.sha1);\n-\t\tif (no_replay) {\n+\t\tif (info->no_replay) {\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}\n \n-\tif (!strategy || !strcmp(strategy, \"recursive\") || action == REVERT) {\n+\tif (!info->strategy || !strcmp(info->strategy, \"recursive\") ||\n+\t info->action == REVERT) {\n \t\tres = do_recursive_merge(base, next, base_label, next_label,\n \t\t\t\t\t head, &msgbuf);\n \t\twrite_res = write_message(&msgbuf, defmsg);\n@@ -508,7 +519,7 @@ static int do_pick_commit(struct commit *commit)\n \n \t\tcommit_list_insert(base, &common);\n \t\tcommit_list_insert(next, &remotes);\n-\t\tres = try_merge_command(strategy, common,\n+\t\tres = try_merge_command(info->strategy, common,\n \t\t\t\t\tsha1_to_hex(head), remotes);\n \t\tfree_commit_list(common);\n \t\tfree_commit_list(remotes);\n@@ -516,14 +527,14 @@ static int do_pick_commit(struct commit *commit)\n \n \tif (res) {\n \t\terror(\"could not %s %s... %s\",\n-\t\t action == REVERT ? \"revert\" : \"apply\",\n+\t\t info->action == REVERT ? \"revert\" : \"apply\",\n \t\t find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),\n \t\t msg.subject);\n-\t\tprint_advice(commit->object.sha1);\n-\t\trerere(allow_rerere_auto);\n+\t\tprint_advice(info, commit->object.sha1);\n+\t\trerere(info->allow_rerere_auto);\n \t} else {\n-\t\tif (!no_commit)\n-\t\t\tres = run_git_commit(defmsg);\n+\t\tif (!info->no_commit)\n+\t\t\tres = run_git_commit(defmsg, info);\n \t}\n \n \tfree_message(&msg);\n@@ -532,18 +543,18 @@ static int do_pick_commit(struct commit *commit)\n \treturn res;\n }\n \n-static void prepare_revs(struct rev_info *revs)\n+static void prepare_revs(struct rev_info *revs, struct args_info *info)\n {\n \tint argc;\n \n \tinit_revisions(revs, NULL);\n \trevs->no_walk = 1;\n-\tif (action != REVERT)\n+\tif (info->action != REVERT)\n \t\trevs->reverse = 1;\n \n-\targc = setup_revisions(commit_argc, commit_argv, revs, NULL);\n+\targc = setup_revisions(info->commit_argc, info->commit_argv, revs, NULL);\n \tif (argc > 1)\n-\t\tusage(*revert_or_cherry_pick_usage());\n+\t\tusage(*revert_or_cherry_pick_usage(info));\n \n \tif (prepare_revision_walk(revs))\n \t\tdie(\"revision walk setup failed\");\n@@ -567,33 +578,36 @@ static void read_and_refresh_cache(const char *me)\n \trollback_lock_file(&index_lock);\n }\n \n-static int revert_or_cherry_pick(int argc, const char **argv)\n+static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n {\n+\tstruct args_info infos;\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \n+\tmemset(&infos, 0, sizeof(infos));\n \tgit_config(git_default_config, NULL);\n-\tme = action == REVERT ? \"revert\" : \"cherry-pick\";\n+\tinfos.action = revert ? REVERT : CHERRY_PICK;\n+\tme = revert ? \"revert\" : \"cherry-pick\";\n \tsetenv(GIT_REFLOG_ACTION, me, 0);\n-\tparse_args(argc, argv);\n+\tparse_args(argc, argv, &infos);\n \n-\tif (allow_ff) {\n-\t\tif (signoff)\n+\tif (infos.allow_ff) {\n+\t\tif (infos.signoff)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --signoff\");\n-\t\tif (no_commit)\n+\t\tif (infos.no_commit)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --no-commit\");\n-\t\tif (no_replay)\n+\t\tif (infos.no_replay)\n \t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n-\t\tif (edit)\n+\t\tif (infos.edit)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n \t}\n \n \tread_and_refresh_cache(me);\n \n-\tprepare_revs(&revs);\n+\tprepare_revs(&revs, &infos);\n \n \twhile ((commit = get_revision(&revs))) {\n-\t\tint res = do_pick_commit(commit);\n+\t\tint res = do_pick_commit(&infos, commit);\n \t\tif (res)\n \t\t\treturn res;\n \t}\n@@ -603,14 +617,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \n int cmd_revert(int argc, const char **argv, const char *prefix)\n {\n-\tif (isatty(0))\n-\t\tedit = 1;\n-\taction = REVERT;\n-\treturn revert_or_cherry_pick(argc, argv);\n+\treturn revert_or_cherry_pick(argc, argv, 1, isatty(0));\n }\n \n int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n {\n-\taction = CHERRY_PICK;\n-\treturn revert_or_cherry_pick(argc, argv);\n+\treturn revert_or_cherry_pick(argc, argv, 0, 0);\n }\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156605","messageId":"20101125212050.5188.13304.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 08/18] revert: refactor code into a new pick_commits() function","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:39Z","receivedAt":"2010-11-25T21:20:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 38 ++++++++++++++++++++++----------------\n 1 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 443b529..1f20251 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -578,36 +578,28 @@ static void read_and_refresh_cache(const char *me)\n \trollback_lock_file(&index_lock);\n }\n \n-static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n+static int pick_commits(struct args_info *infos)\n {\n-\tstruct args_info infos;\n \tstruct rev_info revs;\n \tstruct commit *commit;\n \n-\tmemset(&infos, 0, sizeof(infos));\n-\tgit_config(git_default_config, NULL);\n-\tinfos.action = revert ? REVERT : CHERRY_PICK;\n-\tme = revert ? \"revert\" : \"cherry-pick\";\n-\tsetenv(GIT_REFLOG_ACTION, me, 0);\n-\tparse_args(argc, argv, &infos);\n-\n-\tif (infos.allow_ff) {\n-\t\tif (infos.signoff)\n+\tif (infos->allow_ff) {\n+\t\tif (infos->signoff)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --signoff\");\n-\t\tif (infos.no_commit)\n+\t\tif (infos->no_commit)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --no-commit\");\n-\t\tif (infos.no_replay)\n+\t\tif (infos->no_replay)\n \t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n-\t\tif (infos.edit)\n+\t\tif (infos->edit)\n \t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n \t}\n \n \tread_and_refresh_cache(me);\n \n-\tprepare_revs(&revs, &infos);\n+\tprepare_revs(&revs, infos);\n \n \twhile ((commit = get_revision(&revs))) {\n-\t\tint res = do_pick_commit(&infos, commit);\n+\t\tint res = do_pick_commit(infos, commit);\n \t\tif (res)\n \t\t\treturn res;\n \t}\n@@ -615,6 +607,20 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n \treturn 0;\n }\n \n+static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n+{\n+\tstruct args_info infos;\n+\n+\tgit_config(git_default_config, NULL);\n+\tme = revert ? \"revert\" : \"cherry-pick\";\n+\tsetenv(GIT_REFLOG_ACTION, me, 0);\n+\tmemset(&infos, 0, sizeof(infos));\n+\tinfos.action = revert ? REVERT : CHERRY_PICK;\n+\tparse_args(argc, argv, &infos);\n+\n+\treturn pick_commits(&infos);\n+}\n+\n int cmd_revert(int argc, const char **argv, const char *prefix)\n {\n \treturn revert_or_cherry_pick(argc, argv, 1, isatty(0));\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156607","messageId":"20101125212050.5188.43278.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 09/18] revert: make pick_commits() return an error on --ff incompatible option","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:40Z","receivedAt":"2010-11-25T21:20:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"As we want to use pick_commits() many times and write TODO and DONE\nfile in case of errors, we must not die in case of error inside\npick_commits() but return an error.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 32 ++++++++++++++++----------------\n 1 files changed, 16 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 1f20251..57d4300 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -578,33 +578,33 @@ static void read_and_refresh_cache(const char *me)\n \trollback_lock_file(&index_lock);\n }\n \n+static int ff_incompatible(int val, const char *opt)\n+{\n+\treturn val ? error(\"cherry-pick --ff cannot be used with %s\", opt) : 0;\n+}\n+\n static int pick_commits(struct args_info *infos)\n {\n \tstruct rev_info revs;\n \tstruct commit *commit;\n+\tint res = 0;\n \n-\tif (infos->allow_ff) {\n-\t\tif (infos->signoff)\n-\t\t\tdie(\"cherry-pick --ff cannot be used with --signoff\");\n-\t\tif (infos->no_commit)\n-\t\t\tdie(\"cherry-pick --ff cannot be used with --no-commit\");\n-\t\tif (infos->no_replay)\n-\t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n-\t\tif (infos->edit)\n-\t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n-\t}\n+\tif (infos->allow_ff &&\n+\t ((res = ff_incompatible(infos->signoff, \"--signoff\")) ||\n+\t (res = ff_incompatible(infos->no_commit, \"--no_commit\")) ||\n+\t (res = ff_incompatible(infos->no_replay, \"-x\")) ||\n+\t (res = ff_incompatible(infos->edit, \"--edit\"))))\n+\t\t\treturn res;\n \n \tread_and_refresh_cache(me);\n \n \tprepare_revs(&revs, infos);\n \n-\twhile ((commit = get_revision(&revs))) {\n-\t\tint res = do_pick_commit(infos, commit);\n-\t\tif (res)\n-\t\t\treturn res;\n-\t}\n+\twhile ((commit = get_revision(&revs)) &&\n+\t !(res = do_pick_commit(infos, commit)))\n+\t\t; /* do nothing */\n \n-\treturn 0;\n+\treturn res;\n }\n \n static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156610","messageId":"20101125212050.5188.69211.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 10/18] revert: make read_and_refresh_cache() and prepare_revs() return errors","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:41Z","receivedAt":"2010-11-25T21:20:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"They used to \"die\" in case of problems which is bad if we want to\ntake some action in case of error.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 27 ++++++++++++++++-----------\n 1 files changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 57d4300..8b50e0c 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -543,7 +543,7 @@ static int do_pick_commit(struct args_info *info, struct commit *commit)\n \treturn res;\n }\n \n-static void prepare_revs(struct rev_info *revs, struct args_info *info)\n+static int prepare_revs(struct rev_info *revs, struct args_info *info)\n {\n \tint argc;\n \n@@ -553,29 +553,34 @@ static void prepare_revs(struct rev_info *revs, struct args_info *info)\n \t\trevs->reverse = 1;\n \n \targc = setup_revisions(info->commit_argc, info->commit_argv, revs, NULL);\n-\tif (argc > 1)\n-\t\tusage(*revert_or_cherry_pick_usage(info));\n+\tif (argc > 1) {\n+\t\tfprintf(stderr, \"usage: %s\", *revert_or_cherry_pick_usage(info));\n+\t\treturn 129;\n+\t}\n \n \tif (prepare_revision_walk(revs))\n-\t\tdie(\"revision walk setup failed\");\n+\t\treturn error(\"revision walk setup failed\");\n \n \tif (!revs->commits)\n-\t\tdie(\"empty commit set passed\");\n+\t\treturn error(\"empty commit set passed\");\n+\n+\treturn 0;\n }\n \n-static void read_and_refresh_cache(const char *me)\n+static int read_and_refresh_cache(const char *me)\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\", me);\n+\t\treturn error(\"git %s: failed to read the index\", me);\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\", me);\n+\t\t\treturn error(\"git %s: failed to refresh the index\", me);\n \t}\n \trollback_lock_file(&index_lock);\n+\treturn 0;\n }\n \n static int ff_incompatible(int val, const char *opt)\n@@ -596,9 +601,9 @@ static int pick_commits(struct args_info *infos)\n \t (res = ff_incompatible(infos->edit, \"--edit\"))))\n \t\t\treturn res;\n \n-\tread_and_refresh_cache(me);\n-\n-\tprepare_revs(&revs, infos);\n+\tif ((res = read_and_refresh_cache(me)) ||\n+\t (res = prepare_revs(&revs, infos)))\n+\t\treturn res;\n \n \twhile ((commit = get_revision(&revs)) &&\n \t !(res = do_pick_commit(infos, commit)))\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156608","messageId":"20101125212050.5188.30170.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 11/18] revert: add get_todo_content() and create_todo_file()","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:42Z","receivedAt":"2010-11-25T21:20:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"These static functions will make it possible to write \"todo\"\nand \"done\" files. These files will list the actions (cherry\npicks or reverts) that are still to be completed and that\nhave already been done respectively.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 63 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 8b50e0c..7429be2 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -177,6 +177,69 @@ static char *get_encoding(const char *message, const unsigned char *sha1)\n \treturn NULL;\n }\n \n+static void get_todo_content(struct strbuf *buf, struct commit_list *list,\n+\t\t\t const char *line_prefix, struct args_info *info)\n+{\n+\tstruct commit_list *cur = list;\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\n+\tif (line_prefix)\n+\t\tstrbuf_addstr(&cmd, line_prefix);\n+\tstrbuf_addstr(&cmd, info->action == REVERT ? \"revert \" : \"pick \");\n+\tif (info->no_commit)\n+\t\tstrbuf_addstr(&cmd, \"-n \");\n+\tif (info->edit)\n+\t\tstrbuf_addstr(&cmd, \"-e \");\n+\tif (info->signoff)\n+\t\tstrbuf_addstr(&cmd, \"-s \");\n+\tif (info->mainline)\n+\t\tstrbuf_addf(&cmd, \"-m %d \", info->mainline);\n+\tif (info->allow_rerere_auto)\n+\t\tstrbuf_addstr(&cmd, \"--rerere-autoupdate \");\n+\tif (info->strategy)\n+\t\tstrbuf_addf(&cmd, \"--strategy %s \", info->strategy);\n+\tif (info->no_replay)\n+\t\tstrbuf_addstr(&cmd, \"-x \");\n+\tif (info->allow_ff)\n+\t\tstrbuf_addstr(&cmd, \"--ff \");\n+\n+\tfor (; cur; cur = cur->next) {\n+\t\tstruct commit_message msg = { NULL, NULL, NULL, NULL, NULL };\n+\t\tconst unsigned char *sha1 = cur->item->object.sha1;\n+\t\tif (get_message(cur->item->buffer, sha1, &msg) != 0)\n+\t\t\tdie(\"Cannot get commit message for %s\",\n+\t\t\t sha1_to_hex(sha1));\n+\t\tstrbuf_addbuf(buf, &cmd);\n+\t\tstrbuf_addf(buf, \" %s # %s\\n\",\n+\t\t\t find_unique_abbrev(sha1, DEFAULT_ABBREV),\n+\t\t\t msg.subject);\n+\t\tfree_message(&msg);\n+\t}\n+\n+\tstrbuf_release(&cmd);\n+}\n+\n+static void create_todo_file(const char *filepath, int append,\n+\t\t\t struct commit_list *list, const char *line_prefix,\n+\t\t\t struct args_info *info)\n+{\n+\tint fd, flags;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\n+\tget_todo_content(&buf, list, line_prefix, info);\n+\n+\tflags = O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC);\n+\tfd = open(filepath, flags, 0666);\n+\tif (fd < 0)\n+\t\tdie_errno(\"Could not open file '%s' for writing\", filepath);\n+\n+\twrite_or_whine(fd, buf.buf, buf.len, filepath);\n+\n+\tclose(fd);\n+\n+\tstrbuf_release(&buf);\n+}\n+\n static void add_message_to_msg(struct strbuf *msgbuf, const char *message,\n \t\t\t const unsigned char *sha1)\n {\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156609","messageId":"20101125212050.5188.74945.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 12/18] revert: write TODO and DONE files in case of failure","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:43Z","receivedAt":"2010-11-25T21:20:43Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 42 ++++++++++++++++++++++++++++++-----\n t/t3508-cherry-pick-many-commits.sh | 22 ++++++++++++++++++\n 2 files changed, 58 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 7429be2..27e9d6f 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -14,6 +14,7 @@\n #include \"rerere.h\"\n #include \"merge-recursive.h\"\n #include \"refs.h\"\n+#include \"dir.h\"\n \n /*\n * This implements the builtins revert and cherry-pick.\n@@ -55,6 +56,11 @@ struct args_info {\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n+#define SEQ_DIR\t\t\"sequencer\"\n+#define SEQ_PATH\tgit_path(SEQ_DIR)\n+#define TODO_FILE\tgit_path(SEQ_DIR \"/git-cherry-pick-todo\")\n+#define DONE_FILE\tgit_path(SEQ_DIR \"/git-cherry-pick-done\")\n+\n static char *get_encoding(const char *message, const unsigned char *sha1);\n \n static const char * const *revert_or_cherry_pick_usage(struct args_info *info)\n@@ -651,7 +657,25 @@ static int ff_incompatible(int val, const char *opt)\n \treturn val ? error(\"cherry-pick --ff cannot be used with %s\", opt) : 0;\n }\n \n-static int pick_commits(struct args_info *infos)\n+static int save_todo_and_done(int res, struct args_info *infos,\n+\t\t\t struct commit *commit,\n+\t\t\t struct commit_list *todo_list,\n+\t\t\t struct commit_list **done_list)\n+{\n+\tif (res) {\n+\t\tif (!file_exists(SEQ_PATH) && mkdir(SEQ_PATH, 0777))\n+\t\t\tdie_errno(\"Could not create sequencer directory '%s'\",\n+\t\t\t\t SEQ_PATH);\n+\t\tif (commit)\n+\t\t\tcommit_list_insert(commit, &todo_list);\n+\t\tcreate_todo_file(TODO_FILE, 0, todo_list, \"\", infos);\n+\t\t*done_list = reverse_commit_list(*done_list);\n+\t\tcreate_todo_file(DONE_FILE, 0, *done_list, \"\", infos);\n+\t}\n+\treturn res;\n+}\n+\n+static int pick_commits(struct args_info *infos, struct commit_list **done_list)\n {\n \tstruct rev_info revs;\n \tstruct commit *commit;\n@@ -662,22 +686,24 @@ static int pick_commits(struct args_info *infos)\n \t (res = ff_incompatible(infos->no_commit, \"--no_commit\")) ||\n \t (res = ff_incompatible(infos->no_replay, \"-x\")) ||\n \t (res = ff_incompatible(infos->edit, \"--edit\"))))\n-\t\t\treturn res;\n+\t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n \n \tif ((res = read_and_refresh_cache(me)) ||\n \t (res = prepare_revs(&revs, infos)))\n-\t\treturn res;\n+\t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n \n \twhile ((commit = get_revision(&revs)) &&\n \t !(res = do_pick_commit(infos, commit)))\n-\t\t; /* do nothing */\n+\t\tcommit_list_insert(commit, done_list);\n \n-\treturn res;\n+\treturn save_todo_and_done(res, infos, commit, revs.commits, done_list);\n }\n \n static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n {\n \tstruct args_info infos;\n+\tstruct commit_list *done_list = NULL;\n+\tint res;\n \n \tgit_config(git_default_config, NULL);\n \tme = revert ? \"revert\" : \"cherry-pick\";\n@@ -686,7 +712,11 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n \tinfos.action = revert ? REVERT : CHERRY_PICK;\n \tparse_args(argc, argv, &infos);\n \n-\treturn pick_commits(&infos);\n+\tres = pick_commits(&infos, &done_list);\n+\n+\tfree_commit_list(done_list);\n+\n+\treturn res;\n }\n \n int cmd_revert(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex 8e09fd0..9213d59 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -156,4 +156,26 @@ test_expect_success 'cherry-pick --stdin works' '\n \tcheck_head_differs_from fourth\n '\n \n+test_expect_success 'create some files to test --continue' '\n+\tTODO_FILE=\"$(git rev-parse --git-dir)/sequencer/git-cherry-pick-todo\" &&\n+\tDONE_FILE=\"$(git rev-parse --git-dir)/sequencer/git-cherry-pick-done\" &&\n+\n+\tcat <<-EOF >expected_todo &&\n+\tpick $(git rev-parse --short fourth) # fourth\n+\tEOF\n+\n+\tcat <<-EOF >expected_done\n+\tpick $(git rev-parse --short second) # second\n+\tEOF\n+'\n+\n+test_expect_success 'failed cherry-pick produces todo and done files' '\n+\tgit checkout -f master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\ttest_must_fail git cherry-pick fourth~2 fourth &&\n+\ttest_cmp expected_todo \"$TODO_FILE\" &&\n+\ttest_cmp expected_done \"$DONE_FILE\"\n+'\n+\n test_done\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156615","messageId":"20101125212050.5188.59731.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 13/18] revert: add option parsing for option --continue","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:44Z","receivedAt":"2010-11-25T21:20:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Right now this new option does nothing. It is just a start.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 23 +++++++++++++++++++++--\n 1 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 27e9d6f..12a2409 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -29,11 +29,13 @@\n \n static const char * const revert_usage[] = {\n \t\"git revert [options] \",\n+\t\"git revert --continue\",\n \tNULL\n };\n \n static const char * const cherry_pick_usage[] = {\n \t\"git cherry-pick [options] \",\n+\t\"git cherry-pick --continue\",\n \tNULL\n };\n \n@@ -48,6 +50,7 @@ struct args_info {\n \tint signoff;\n \tint allow_ff;\n \tint allow_rerere_auto;\n+\tint continuing;\n \tconst char *strategy;\n \tconst char **commit_argv;\n \tint commit_argc;\n@@ -72,6 +75,8 @@ static void parse_args(int argc, const char **argv, struct args_info *info)\n {\n \tint noop;\n \tstruct option options[] = {\n+\t\tOPT_BOOLEAN(0, \"continue\", &info->continuing,\n+\t\t\t \"continue after resolving a conflict\"),\n \t\tOPT_BOOLEAN('n', \"no-commit\", &info->no_commit,\n \t\t\t \"don't automatically commit\"),\n \t\tOPT_BOOLEAN('e', \"edit\", &info->edit, \"edit the commit message\"),\n@@ -102,7 +107,18 @@ static void parse_args(int argc, const char **argv, struct args_info *info)\n \t\t\t\t\t PARSE_OPT_KEEP_UNKNOWN);\n \tinfo->commit_argv = argv;\n \n-\tif (info->commit_argc < 2)\n+\tif (info->continuing) {\n+\t\tif (info->commit_argc != 1)\n+\t\t\tusage_msg_opt(\"No argument can be passed along \"\n+\t\t\t\t \"with option --continue!\",\n+\t\t\t\t info->usage_str, options);\n+\t\tif (info->no_commit || info->edit || info->signoff ||\n+\t\t info->mainline || info->allow_rerere_auto || info->strategy ||\n+\t\t info->no_replay || info->allow_ff)\n+\t\t\tusage_msg_opt(\"No other option can be passed along \"\n+\t\t\t\t \"with option --continue!\",\n+\t\t\t\t info->usage_str, options);\n+\t} else if (info->commit_argc < 2)\n \t\tusage_with_options(info->usage_str, options);\n }\n \n@@ -712,7 +728,10 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n \tinfos.action = revert ? REVERT : CHERRY_PICK;\n \tparse_args(argc, argv, &infos);\n \n-\tres = pick_commits(&infos, &done_list);\n+\tif (infos.continuing)\n+\t\tres = 0;\n+\telse\n+\t\tres = pick_commits(&infos, &done_list);\n \n \tfree_commit_list(done_list);\n \n-- \n1.7.3.2.504.g59d466\n"},{"id":"156614","messageId":"20101125212050.5188.49554.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 14/18] revert: move global variable \"me\" into \"struct args_info\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:45Z","receivedAt":"2010-11-25T21:20:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 19 ++++++++++---------\n 1 files changed, 10 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 12a2409..7513a00 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -39,8 +39,6 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-static const char *me;\n-\n struct args_info {\n \tenum { REVERT, CHERRY_PICK } action;\n \tint edit;\n@@ -51,6 +49,7 @@ struct args_info {\n \tint allow_ff;\n \tint allow_rerere_auto;\n \tint continuing;\n+\tconst char *me;\n \tconst char *strategy;\n \tconst char **commit_argv;\n \tint commit_argc;\n@@ -91,6 +90,9 @@ static void parse_args(int argc, const char **argv, struct args_info *info)\n \t\tOPT_END(),\n \t};\n \n+\tinfo->me = info->action == REVERT ? \"revert\" : \"cherry-pick\";\n+\tsetenv(GIT_REFLOG_ACTION, info->me, 0);\n+\n \tif (info->action == CHERRY_PICK) {\n \t\tstruct option cp_extra[] = {\n \t\t\tOPT_BOOLEAN('x', NULL, &info->no_replay, \"append commit name\"),\n@@ -403,7 +405,8 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from)\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 unsigned char *head, const char *me,\n+\t\t\t struct strbuf *msgbuf)\n {\n \tstruct merge_options o;\n \tstruct tree *result, *next_tree, *base_tree, *head_tree;\n@@ -507,7 +510,7 @@ static int do_pick_commit(struct args_info *info, struct commit *commit)\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(me);\n+\t\t\treturn error_dirty_index(info->me);\n \t}\n \tdiscard_cache();\n \n@@ -543,7 +546,7 @@ static int do_pick_commit(struct args_info *info, struct commit *commit)\n \n \tif (parent && parse_commit(parent) < 0)\n \t\treturn error(\"%s: cannot parse parent commit %s\",\n-\t\t\t me, sha1_to_hex(parent->object.sha1));\n+\t\t\t info->me, sha1_to_hex(parent->object.sha1));\n \n \tif (get_message(commit->buffer, commit->object.sha1, &msg) != 0)\n \t\treturn error(\"Cannot get commit message for %s\",\n@@ -590,7 +593,7 @@ static int do_pick_commit(struct args_info *info, struct commit *commit)\n \tif (!info->strategy || !strcmp(info->strategy, \"recursive\") ||\n \t info->action == REVERT) {\n \t\tres = do_recursive_merge(base, next, base_label, next_label,\n-\t\t\t\t\t head, &msgbuf);\n+\t\t\t\t\t head, info->me, &msgbuf);\n \t\twrite_res = write_message(&msgbuf, defmsg);\n \t\tif (write_res)\n \t\t\treturn write_res;\n@@ -704,7 +707,7 @@ static int pick_commits(struct args_info *infos, struct commit_list **done_list)\n \t (res = ff_incompatible(infos->edit, \"--edit\"))))\n \t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n \n-\tif ((res = read_and_refresh_cache(me)) ||\n+\tif ((res = read_and_refresh_cache(infos->me)) ||\n \t (res = prepare_revs(&revs, infos)))\n \t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n \n@@ -722,8 +725,6 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n \tint res;\n \n \tgit_config(git_default_config, NULL);\n-\tme = revert ? \"revert\" : \"cherry-pick\";\n-\tsetenv(GIT_REFLOG_ACTION, me, 0);\n \tmemset(&infos, 0, sizeof(infos));\n \tinfos.action = revert ? REVERT : CHERRY_PICK;\n \tparse_args(argc, argv, &infos);\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156616","messageId":"20101125212050.5188.96830.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 15/18] revert: add NONE action and make parse_args() manage it","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:46Z","receivedAt":"2010-11-25T21:20:46Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 13 ++++++++++++-\n 1 files changed, 12 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 7513a00..fee2e38 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -40,7 +40,7 @@ static const char * const cherry_pick_usage[] = {\n };\n \n struct args_info {\n-\tenum { REVERT, CHERRY_PICK } action;\n+\tenum { NONE = 0, REVERT, CHERRY_PICK } action;\n \tint edit;\n \tint no_replay;\n \tint no_commit;\n@@ -90,6 +90,17 @@ static void parse_args(int argc, const char **argv, struct args_info *info)\n \t\tOPT_END(),\n \t};\n \n+\tif (info->action == NONE) {\n+\t\tif (argc < 1)\n+\t\t\tdie(\"no action argument\");\n+\t\tif (!strcasecmp(argv[0], \"p\") || !strcasecmp(argv[0], \"pick\"))\n+\t\t\tinfo->action = CHERRY_PICK;\n+\t\telse if (!strcasecmp(argv[0], \"revert\"))\n+\t\t\tinfo->action = REVERT;\n+\t\telse\n+\t\t\tdie(\"unknown action argument: %s\", argv[0]);\n+\t}\n+\n \tinfo->me = info->action == REVERT ? \"revert\" : \"cherry-pick\";\n \tsetenv(GIT_REFLOG_ACTION, info->me, 0);\n \n-- \n1.7.3.2.504.g59d466\n"},{"id":"156613","messageId":"20101125212050.5188.64875.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 16/18] revert: implement parsing TODO and DONE files","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:47Z","receivedAt":"2010-11-25T21:20:47Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Stephan Beyer \n\nThe code from this patch comes from the git sequencer Google\nSummer of Code 2008 project available here:\n\nhttp://repo.or.cz/w/git/sbeyer.git\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 228 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 228 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex fee2e38..ca65b92 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -70,6 +70,234 @@ static const char * const *revert_or_cherry_pick_usage(struct args_info *info)\n \treturn info->action == REVERT ? revert_usage : cherry_pick_usage;\n }\n \n+/*\n+ * A structure for a parsed instruction line plus a next pointer\n+ * to allow linked list behavior\n+ */\n+struct parsed_insn {\n+\tint argc;\n+\tconst char **argv;\n+\tint line;\n+\tstruct strbuf orig;\n+\tstruct parsed_insn *next;\n+};\n+\n+struct parsed_file {\n+\tsize_t count;\n+\tsize_t total;\n+\tstruct parsed_insn *first;\n+\tstruct parsed_insn *last;\n+\tstruct parsed_insn *cur; /* a versatile helper */\n+};\n+\n+static int parse_line(char *buf, size_t len, int lineno,\n+\t\t struct parsed_insn **line)\n+{\n+\tstatic int alloc = 0;\n+\tstatic struct strbuf arg_sb = STRBUF_INIT;\n+\tstatic enum {\n+\t\tST_START,\n+\t\tST_DELIMITER,\n+\t\tST_ARGUMENT,\n+\t\tST_ESCAPE,\n+\t\tST_DOUBLE_QUOTES,\n+\t\tST_DOUBLE_QUOTES_ESCAPE,\n+\t\tST_SINGLE_QUOTES,\n+\t} state = ST_START;\n+\t/* The current rules are as follows:\n+\t * 1. whitespace at the beginning is ignored\n+\t * 2. insn is everything up to next whitespace or EOL\n+\t * 3. now whitespace acts as delimiter for arguments,\n+\t * except if written in single or double quotes\n+\t * 4. \\ acts as escape inside and outside double quotes.\n+\t * Inside double quotes, this is only useful for \\\".\n+\t * Outside, it is useful for \\', \\\", \\\\ and \\ .\n+\t * 5. single quotes do not have an escape character\n+\t * 6. abort on \"#\" (comments)\n+\t */\n+\n+\tsize_t i, j = 0;\n+\tstruct parsed_insn *ret = *line;\n+\n+\tfor (i = 0; i <= len; ++i) {\n+\t\tswitch (state) {\n+\t\tcase ST_START:\n+\t\t\tswitch (buf[i]) {\n+\t\t\tcase ' ':\n+\t\t\tcase '\\t':\n+\t\t\t\tcontinue;\n+\t\t\tcase 0:\n+\t\t\tcase '#':\n+\t\t\t\tbreak;\n+\t\t\tcase '\\'':\n+\t\t\t\tj = i+1;\n+\t\t\t\tstate = ST_SINGLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tcase '\"':\n+\t\t\t\tj = i+1;\n+\t\t\t\tstate = ST_DOUBLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tj = i;\n+\t\t\t\tstate = ST_ARGUMENT;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\t/* prepare everything */\n+\t\t\tret = xcalloc(1, sizeof(*ret));\n+\t\t\tret->line = lineno;\n+\t\t\tstrbuf_init(&ret->orig, len+2);\n+\t\t\tif (!buf[i] || buf[i] == '#') /* empty/comment */\n+\t\t\t\tgoto finish;\n+\t\t\tbreak;\n+\t\tcase ST_DELIMITER:\n+\t\t\tswitch (buf[i]) {\n+\t\t\tcase ' ':\n+\t\t\tcase '\\t':\n+\t\t\t\tcontinue;\n+\t\t\tcase 0:\n+\t\t\t\tbreak;\n+\t\t\tcase '\\'':\n+\t\t\t\tj = i+1;\n+\t\t\t\tstate = ST_SINGLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tcase '\"':\n+\t\t\t\tj = i+1;\n+\t\t\t\tstate = ST_DOUBLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tj = i;\n+\t\t\t\tstate = ST_ARGUMENT;\n+\t\t\t\tif (buf[i] == '#') /* a comment */\n+\t\t\t\t\tgoto finish;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\t/* prepare next argument */\n+\t\t\tALLOC_GROW(ret->argv, ret->argc + 1, alloc);\n+\t\t\tret->argv[ret->argc++] = strbuf_detach(&arg_sb, NULL);\n+\t\t\tbreak;\n+\t\tcase ST_ARGUMENT:\n+\t\t\tswitch (buf[i]) {\n+\t\t\tcase ' ':\n+\t\t\tcase '\\t':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tstate = ST_DELIMITER;\n+\t\t\t\tbreak;\n+\t\t\tcase '\"':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_DOUBLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tcase '\\'':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_SINGLE_QUOTES;\n+\t\t\t\tbreak;\n+\t\t\tcase '\\\\':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_ESCAPE;\n+\t\t\tdefault:\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase ST_ESCAPE:\n+\t\t\t\tstate = ST_ARGUMENT;\n+\t\t\tbreak;\n+\t\tcase ST_DOUBLE_QUOTES:\n+\t\t\tswitch (buf[i]) {\n+\t\t\tcase '\"':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_ARGUMENT;\n+\t\t\t\tbreak;\n+\t\t\tcase '\\\\':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_DOUBLE_QUOTES_ESCAPE;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase ST_DOUBLE_QUOTES_ESCAPE:\n+\t\t\tstate = ST_DOUBLE_QUOTES;\n+\t\t\tbreak;\n+\t\tcase ST_SINGLE_QUOTES:\n+\t\t\tswitch (buf[i]) {\n+\t\t\tcase '\\'':\n+\t\t\t\tstrbuf_add(&arg_sb, buf+j, i-j);\n+\t\t\t\tj = i + 1;\n+\t\t\t\tstate = ST_ARGUMENT;\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+finish:\n+\t*line = ret;\n+\tswitch(state) {\n+\tcase ST_DOUBLE_QUOTES:\n+\tcase ST_DOUBLE_QUOTES_ESCAPE:\n+\tcase ST_SINGLE_QUOTES:\n+\t\tstrbuf_add(&arg_sb, buf+j, i-j-1);\n+\t\tstrbuf_add(&arg_sb, \"\\n\", 1);\n+\t\treturn 1;\n+\tcase ST_ARGUMENT:\n+\t\tif (i-j > 1)\n+\t\t\tstrbuf_add(&arg_sb, buf+j, i-j-1);\n+\t\tALLOC_GROW(ret->argv, ret->argc + 1, alloc);\n+\t\tret->argv[ret->argc++] = strbuf_detach(&arg_sb, NULL);\n+\tcase ST_DELIMITER:\n+\t\tstate = ST_START;\n+\t\talloc = 0;\n+\tdefault:\n+\t\tstrbuf_addstr(&ret->orig, buf);\n+\t\tstrbuf_addch(&ret->orig, '\\n');\n+\t\treturn 0;\n+\t}\n+}\n+\n+static void add_parsed_line_to_parsed_file(struct parsed_insn *parsed_line,\n+\t\t\t\t\t struct parsed_file *contents)\n+{\n+\tif (!contents->first) {\n+\t\tcontents->first = parsed_line;\n+\t\tcontents->last = parsed_line;\n+\t} else {\n+\t\tcontents->last->next = parsed_line;\n+\t\tcontents->last = parsed_line;\n+\t}\n+\tif (parsed_line->argv)\n+\t\tcontents->total++;\n+}\n+\n+/* Parse a file fp; write result into contents */\n+static void parse_file(const char *filename, struct parsed_file *contents)\n+{\n+\tstruct strbuf str = STRBUF_INIT;\n+\tstruct parsed_insn *parsed_line = NULL;\n+\tint r = 0;\n+\tint lineno = 0;\n+\tFILE *fp = fp = fopen(filename, \"r\");\n+\tif (!fp)\n+\t\tdie_errno(\"Could not open file '%s'\", filename);\n+\n+\tmemset(contents, 0, sizeof(*contents));\n+\n+\twhile (strbuf_getline(&str, fp, '\\n') != EOF) {\n+\t\tlineno++;\n+\t\tr = parse_line(str.buf, str.len, lineno, &parsed_line);\n+\t\tif (!r)\n+\t\t\tadd_parsed_line_to_parsed_file(parsed_line, contents);\n+\t}\n+\tstrbuf_release(&str);\n+\tfclose(fp);\n+\tif (r)\n+\t\tdie(\"Unexpected end of file.\");\n+}\n+\n static void parse_args(int argc, const char **argv, struct args_info *info)\n {\n \tint noop;\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156611","messageId":"20101125212050.5188.28783.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 17/18] revert: add remaining instructions in todo file","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:48Z","receivedAt":"2010-11-25T21:20:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 13 +++++++++----\n 1 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex ca65b92..b51f7ab 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -483,14 +483,19 @@ static void get_todo_content(struct strbuf *buf, struct commit_list *list,\n }\n \n static void create_todo_file(const char *filepath, int append,\n-\t\t\t struct commit_list *list, const char *line_prefix,\n-\t\t\t struct args_info *info)\n+\t\t\t struct commit_list *list, struct parsed_insn *insn,\n+\t\t\t const char *line_prefix, struct args_info *info)\n {\n \tint fd, flags;\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tget_todo_content(&buf, list, line_prefix, info);\n \n+\tif (insn) {\n+\t\twhile ((insn = insn->next))\n+\t\t\tstrbuf_addbuf(&buf, &insn->orig);\n+\t}\n+\n \tflags = O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC);\n \tfd = open(filepath, flags, 0666);\n \tif (fd < 0)\n@@ -926,9 +931,9 @@ static int save_todo_and_done(int res, struct args_info *infos,\n \t\t\t\t SEQ_PATH);\n \t\tif (commit)\n \t\t\tcommit_list_insert(commit, &todo_list);\n-\t\tcreate_todo_file(TODO_FILE, 0, todo_list, \"\", infos);\n+\t\tcreate_todo_file(TODO_FILE, 0, todo_list, NULL, \"\", infos);\n \t\t*done_list = reverse_commit_list(*done_list);\n-\t\tcreate_todo_file(DONE_FILE, 0, *done_list, \"\", infos);\n+\t\tcreate_todo_file(DONE_FILE, 0, *done_list, NULL, \"\", infos);\n \t}\n \treturn res;\n }\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156612","messageId":"20101125212050.5188.26439.chriscool@tuxfamily.org","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"[RFC/PATCH 18/18] revert: implement --continue processing","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2010-11-25T21:20:49Z","receivedAt":"2010-11-25T21:20:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patch adds the pick_continue() and the process_insn() functions\nto process all the instructions from a todo file.\nThe TODO and DONE files are removed at the end if there was no\nerror.\n\nSigned-off-by: Christian Couder \n---\n builtin/revert.c | 54 ++++++++++++++++++----\n t/t3508-cherry-pick-many-commits.sh | 85 +++++++++++++++++++++++++++++++++-\n 2 files changed, 127 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex b51f7ab..46445b0 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -923,7 +923,8 @@ static int ff_incompatible(int val, const char *opt)\n static int save_todo_and_done(int res, struct args_info *infos,\n \t\t\t struct commit *commit,\n \t\t\t struct commit_list *todo_list,\n-\t\t\t struct commit_list **done_list)\n+\t\t\t struct commit_list **done_list,\n+\t\t\t struct parsed_insn *insn)\n {\n \tif (res) {\n \t\tif (!file_exists(SEQ_PATH) && mkdir(SEQ_PATH, 0777))\n@@ -931,14 +932,15 @@ static int save_todo_and_done(int res, struct args_info *infos,\n \t\t\t\t SEQ_PATH);\n \t\tif (commit)\n \t\t\tcommit_list_insert(commit, &todo_list);\n-\t\tcreate_todo_file(TODO_FILE, 0, todo_list, NULL, \"\", infos);\n+\t\tcreate_todo_file(TODO_FILE, 0, todo_list, insn, \"\", infos);\n \t\t*done_list = reverse_commit_list(*done_list);\n-\t\tcreate_todo_file(DONE_FILE, 0, *done_list, NULL, \"\", infos);\n+\t\tcreate_todo_file(DONE_FILE, 1, *done_list, NULL, \"\", infos);\n \t}\n \treturn res;\n }\n \n-static int pick_commits(struct args_info *infos, struct commit_list **done_list)\n+static int pick_commits(struct args_info *infos, struct commit_list **done_list,\n+\t\t\tstruct parsed_insn *insn)\n {\n \tstruct rev_info revs;\n \tstruct commit *commit;\n@@ -949,17 +951,51 @@ static int pick_commits(struct args_info *infos, struct commit_list **done_list)\n \t (res = ff_incompatible(infos->no_commit, \"--no_commit\")) ||\n \t (res = ff_incompatible(infos->no_replay, \"-x\")) ||\n \t (res = ff_incompatible(infos->edit, \"--edit\"))))\n-\t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n+\t return save_todo_and_done(res, infos, NULL, NULL, done_list, insn);\n \n \tif ((res = read_and_refresh_cache(infos->me)) ||\n \t (res = prepare_revs(&revs, infos)))\n-\t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list);\n+\t\treturn save_todo_and_done(res, infos, NULL, NULL, done_list, insn);\n \n \twhile ((commit = get_revision(&revs)) &&\n \t !(res = do_pick_commit(infos, commit)))\n \t\tcommit_list_insert(commit, done_list);\n \n-\treturn save_todo_and_done(res, infos, commit, revs.commits, done_list);\n+\treturn save_todo_and_done(res, infos, commit, revs.commits, done_list, insn);\n+}\n+\n+static int process_insn(struct parsed_insn *cur, struct commit_list **done_list)\n+{\n+\tstruct args_info infos;\n+\tmemset(&infos, 0, sizeof(infos));\n+\tparse_args(cur->argc, cur->argv, &infos);\n+\n+\tif (infos.continuing)\n+\t\treturn error(\"option --continue is not allowed in todo file\");\n+\n+\treturn pick_commits(&infos, done_list, cur);\n+}\n+\n+static int pick_continue(struct commit_list **done_list)\n+{\n+\tstruct parsed_file content;\n+\tstruct parsed_insn *cur;\n+\n+\tif (!file_exists(TODO_FILE))\n+\t\tdie(\"No %s file found, so nothing to continue\", TODO_FILE);\n+\n+\tparse_file(TODO_FILE, &content);\n+\n+\tfor (cur = content.first; cur; cur = cur->next) {\n+\t\tint res = process_insn(cur, done_list);\n+\t\tif (res)\n+\t\t\treturn res;\n+\t}\n+\n+\tunlink(TODO_FILE);\n+\tunlink(DONE_FILE);\n+\n+\treturn 0;\n }\n \n static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n@@ -974,9 +1010,9 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n \tparse_args(argc, argv, &infos);\n \n \tif (infos.continuing)\n-\t\tres = 0;\n+\t\tres = pick_continue(&done_list);\n \telse\n-\t\tres = pick_commits(&infos, &done_list);\n+\t\tres = pick_commits(&infos, &done_list, NULL);\n \n \tfree_commit_list(done_list);\n \ndiff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh\nindex 9213d59..bbc6588 100755\n--- a/t/t3508-cherry-pick-many-commits.sh\n+++ b/t/t3508-cherry-pick-many-commits.sh\n@@ -164,18 +164,97 @@ test_expect_success 'create some files to test --continue' '\n \tpick $(git rev-parse --short fourth) # fourth\n \tEOF\n \n-\tcat <<-EOF >expected_done\n+\tcat <<-EOF >expected_done &&\n \tpick $(git rev-parse --short second) # second\n \tEOF\n+\n+\tcat <<-EOF >new_todo\n+\tpick $(git rev-parse --short third) # third\n+\tpick $(git rev-parse --short fourth) # fourth\n+\tEOF\n+'\n+\n+test_expect_success 'cherry-pick --continue works' '\n+\tgit checkout -f master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\ttest_must_fail git cherry-pick fourth~2 fourth &&\n+\ttest_cmp expected_todo \"$TODO_FILE\" &&\n+\ttest_cmp expected_done \"$DONE_FILE\" &&\n+\tgit reset --merge HEAD &&\n+\tcp new_todo \"$TODO_FILE\" &&\n+\tgit cherry-pick --continue &&\n+\tgit diff --quiet other &&\n+\tgit diff --quiet HEAD other &&\n+\tcheck_head_differs_from fourth &&\n+\t! test -e \"$TODO_FILE\" &&\n+\t! test -e \"$DONE_FILE\"\n+'\n+\n+test_expect_success 'create more files to test --continue' '\n+\tcat <<-EOF >expected_todo_2 &&\n+\tpick $(git rev-parse --short second) # second\n+\tEOF\n+\n+\tcat <<-EOF >expected_done_2 &&\n+\tpick $(git rev-parse --short second) # second\n+\tpick $(git rev-parse --short third) # third\n+\tEOF\n+\n+\tcat <<-EOF >new_todo_2\n+\tpick $(git rev-parse --short third) # third\n+\tpick $(git rev-parse --short second) # second\n+\tEOF\n+'\n+\n+test_expect_success 'TODO and DONE files are ok when --continue fails (1)' '\n+\tgit checkout -f master &&\n+\tgit reset --hard first &&\n+\ttest_tick &&\n+\ttest_must_fail git cherry-pick fourth~2 fourth &&\n+\ttest_cmp expected_todo \"$TODO_FILE\" &&\n+\ttest_cmp expected_done \"$DONE_FILE\" &&\n+\tgit reset --merge HEAD &&\n+\tcp new_todo_2 \"$TODO_FILE\" &&\n+\ttest_must_fail git cherry-pick --continue &&\n+\ttest_cmp expected_todo_2 \"$TODO_FILE\" &&\n+\ttest_cmp expected_done_2 \"$DONE_FILE\" &&\n+\trm \"$TODO_FILE\" &&\n+\trm \"$DONE_FILE\"\n+'\n+\n+test_expect_success 'create again more files to test --continue' '\n+\tcat <<-EOF >expected_todo_3 &&\n+\tpick $(git rev-parse --short second) # second\n+\tpick $(git rev-parse --short fourth) # fourth\n+\tEOF\n+\n+\tcat <<-EOF >expected_done_3 &&\n+\tpick $(git rev-parse --short second) # second\n+\tpick $(git rev-parse --short third) # third\n+\tEOF\n+\n+\tcat <<-EOF >new_todo_3\n+\tpick $(git rev-parse --short third) # third\n+\tpick $(git rev-parse --short second) # second\n+\tpick $(git rev-parse --short fourth) # fourth\n+\tEOF\n '\n \n-test_expect_success 'failed cherry-pick produces todo and done files' '\n+test_expect_success 'TODO and DONE files are ok when --continue fails (2)' '\n \tgit checkout -f master &&\n \tgit reset --hard first &&\n \ttest_tick &&\n \ttest_must_fail git cherry-pick fourth~2 fourth &&\n \ttest_cmp expected_todo \"$TODO_FILE\" &&\n-\ttest_cmp expected_done \"$DONE_FILE\"\n+\ttest_cmp expected_done \"$DONE_FILE\" &&\n+\tgit reset --merge HEAD &&\n+\tcp new_todo_3 \"$TODO_FILE\" &&\n+\ttest_must_fail git cherry-pick --continue &&\n+\ttest_cmp expected_todo_3 \"$TODO_FILE\" &&\n+\ttest_cmp expected_done_3 \"$DONE_FILE\" &&\n+\trm \"$TODO_FILE\" &&\n+\trm \"$DONE_FILE\"\n '\n \n test_done\n-- \n1.7.3.2.504.g59d466\n"},{"id":"156617","messageId":"20101126055656.GB18751@burratino","threadId":"25843","inReplyTo":"20101125212050.5188.50630.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 01/18] advice: add error_resolve_conflict() function","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-26T05:56:57Z","receivedAt":"2010-11-26T05:56:57Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Christian Couder wrote:\n\n> --- a/advice.c\n> +++ b/advice.c\n> @@ -34,6 +34,13 @@ int git_default_advice_config(const char *var, const char *value)\n> \treturn 0;\n> }\n> \n> +const char unmerged_file_advice[] =\n> +\t\"'%s' is not possible because you have unmerged files.\\n\"\n> +\t\"Please, fix them up in the work tree, and then use 'git add/rm ' as\\n\"\n> +\t\"appropriate to mark resolution and make a commit, or use 'git commit -a'.\";\n> +const char unmerged_file_no_advice[] =\n> +\t\"'%s' is not possible because you have unmerged files.\";\n\nstatic?\n\n> --- a/advice.h\n> +++ b/advice.h\n> @@ -13,5 +13,6 @@ extern int advice_detached_head;\n> int git_default_advice_config(const char *var, const char *value);\n> \n> extern void NORETURN die_resolve_conflict(const char *me);\n> +extern int error_resolve_conflict(const char *me);\n\nI like it.\n"},{"id":"156618","messageId":"20101126060543.GC18751@burratino","threadId":"25843","inReplyTo":"20101125212050.5188.21758.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 02/18] revert: change many die() calls into \"return error()\" calls","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-26T06:05:43Z","receivedAt":"2010-11-26T06:05:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Christian Couder wrote:\n\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -280,16 +280,18 @@ static struct tree *empty_tree(void)\n> \treturn tree;\n> }\n> \n> -static NORETURN void die_dirty_index(const char *me)\n> +static int error_dirty_index(const char *me)\n\nIn general sounds to me like a good thing to do. But for your use\ncase (writing out TODO and DONE files when cherry-pick fails),\nwouldn't a set_die_routine() also work?\n\nI am tempted to suggest a series in the following order:\n\n 1. set die routine with the desired behavior\n 2. change die() calls to return error() so the nice stack unwinding\n takes place automatically (this should help with other libification\n work, anyway)\n\nthen maybe:\n\n 3. add an assert(0) to die routine (perhaps protected by a compile-time\n option) so missing die() calls can be noticed\n 4. remove the die routine once it is clear all problematic die calls\n have been eliminated.\n\n... but wait: would all such die calls ever be eliminated? xmalloc,\nxmkstemp, and similar functions are perhaps too convenient to avoid.\nSo it might be simpler to stick to (1) and treat (2) as a separate\ntopic.\n"},{"id":"156619","messageId":"20101126060747.GD18751@burratino","threadId":"25843","inReplyTo":"20101125212050.5188.56613.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 03/18] usage: implement error_errno() the same way as die_errno()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-26T06:07:47Z","receivedAt":"2010-11-26T06:07:47Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Christian Couder wrote:\n\n> die_errno() is very useful, but sometimes we don't want to\n> die after printing an error message and the error message\n> from errno.\n\nYes, please! I have often wished for this helper.\n"},{"id":"156620","messageId":"20101126061831.GE18751@burratino","threadId":"25843","inReplyTo":"20101125212050.5188.8316.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 07/18] revert: put option information in an option struct","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-26T06:18:31Z","receivedAt":"2010-11-26T06:18:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Christian Couder wrote:\n\n> This is needed because we want to reuse the parse_args() function\n> so that we can parse options saved in a TODO file.\n\nWhy couldn't parse_args() write to the globals? I would be more\nconvinced by an explanation like\n\n\tThis helps attain greater sanity by being explicit\n\tabout which functions depend on the parameters passed\n\tto cherry-pick.\n"},{"id":"156621","messageId":"20101126062836.GF18751@burratino","threadId":"25843","inReplyTo":"20101125210138.5188.13115.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 00/18] WIP implement cherry-pick/revert --continue","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-11-26T06:28:36Z","receivedAt":"2010-11-26T06:28:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Christian,\n\nChristian Couder wrote:\n\n> Many patches in this series are replacing calls to \"die()\" by\n> \"return error()\", because the TODO and DONE files are written\n> only when cherry-pick fails. This is efficient but perhaps it\n> would be simpler and safer to write them before each cherry-pick\n> just in case it fails, so that the \"die()\" calls don't need to\n> be removed.\n\nAnother possibility would be to use set_die_routine()/atexit()/\nsigchain_push_common(), but the \"always write\" solution does seem\nsimpler.\n\n> (17):\n\nPerhaps too many. :)\n\nI like where this is going. My main complaint is the commit messages;\ngiven a clear explanation of the design it should not be too hard for\nothers to help write documentation, enhancements, and tests, but\nwithout it is much harder.\n\nNit: the style of commit message in patch 16 is unnecessarily\ndemoralizing. It basically says \"track down the history in this repo\nthat may not exist in 2050 if you want to know what this patch is\nabout\". I think it would be better to say\n\n\tThis code was written as part of the git sequencer\n\tGoogle Summer of Code project, 2008\n\nand let the rest of the commit message tell the important details.\nReaders can google for the detailed history.\n\nThanks for your work.\nJonathan\n"},{"id":"156688","messageId":"7v8w0gq6fn.fsf@alter.siamese.dyndns.org","threadId":"25843","inReplyTo":"20101125212050.5188.56613.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 03/18] usage: implement error_errno() the same way as die_errno()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-26T18:35:24Z","receivedAt":"2010-11-26T18:35:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> die_errno() is very useful, but sometimes we don't want to\n> die after printing an error message and the error message\n> from errno. So let's implement error_errno() that does the\n> same thing as die_errno() except that it calls\n> error_routine() instead of die_routine().\n\nIf this were \"error_errno() is to error() as die_errno() is to die(); iow,\nerror_errno() does the same thing as error() but in addition shows the\nerrno information\", it might be a good thing, but the above makes it sound\nlike you did something different.\n"},{"id":"156689","messageId":"7vzksworiq.fsf@alter.siamese.dyndns.org","threadId":"25843","inReplyTo":"20101125212050.5188.8316.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 07/18] revert: put option information in an option struct","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-11-26T18:42:53Z","receivedAt":"2010-11-26T18:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> This is needed because we want to reuse the parse_args() function\n> so that we can parse options saved in a TODO file.\n\nThis probably is not _needed_ but something you thought would be a\ngood thing to do while at it.\n\nYou forgot to mention something more important, though. You added two\nextra arguments to revert_or_cherry_pick, neither of which I agree with.\n\n * it is a regression to call the first extra argument \"int revert\";\n (action == REVERT) was more readable.\n\n * \"int edit\" is ill thought out; it is about giving the default of \"edit\"\n to revert_or_cherry_pick() depending on what action it is going to\n take. In this particular case, the logic for the default of \"edit\" is\n trivial and localized (it is 0 unless we are interactive revert), so I\n would drop the argument and have default logic immediately after\n \"memset(&info, 0, sizeof(info))\".\n\n If there were many such args-info elements whose default have to be\n different depending on the action, the caller should be passing an\n instance of \"struct args_info\" with the default, and have the parser\n to update the default supplied. I don't think it is warranted in this\n case.\n\nBy the way, \"infos\" is an eyesore at least to me. Any data you work on is\ninformation, naming a variable \"struct args_info info\", unless it\nprimarily works on that \"args_info\" and not any other kinds of info (like\n\"struct rev_info revs\"), is like calling a variable \"var\", adding _no_\nuseful information.\n"},{"id":"156713","messageId":"alpine.LNX.2.00.1011262215540.14365@iabervon.org","threadId":"25843","inReplyTo":"20101125212050.5188.13304.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 08/18] revert: refactor code into a new pick_commits() function","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2010-11-27T03:50:55Z","receivedAt":"2010-11-27T03:50:55Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 25 Nov 2010, Christian Couder wrote:\n\n> Signed-off-by: Christian Couder \n> ---\n> builtin/revert.c | 38 ++++++++++++++++++++++----------------\n> 1 files changed, 22 insertions(+), 16 deletions(-)\n> \n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index 443b529..1f20251 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -578,36 +578,28 @@ static void read_and_refresh_cache(const char *me)\n> \trollback_lock_file(&index_lock);\n> }\n> \n> -static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n> +static int pick_commits(struct args_info *infos)\n> {\n> -\tstruct args_info infos;\n> \tstruct rev_info revs;\n> \tstruct commit *commit;\n> \n> -\tmemset(&infos, 0, sizeof(infos));\n> -\tgit_config(git_default_config, NULL);\n> -\tinfos.action = revert ? REVERT : CHERRY_PICK;\n> -\tme = revert ? \"revert\" : \"cherry-pick\";\n> -\tsetenv(GIT_REFLOG_ACTION, me, 0);\n> -\tparse_args(argc, argv, &infos);\n> -\n> -\tif (infos.allow_ff) {\n> -\t\tif (infos.signoff)\n> +\tif (infos->allow_ff) {\n> +\t\tif (infos->signoff)\n> \t\t\tdie(\"cherry-pick --ff cannot be used with --signoff\");\n> -\t\tif (infos.no_commit)\n> +\t\tif (infos->no_commit)\n> \t\t\tdie(\"cherry-pick --ff cannot be used with --no-commit\");\n> -\t\tif (infos.no_replay)\n> +\t\tif (infos->no_replay)\n> \t\t\tdie(\"cherry-pick --ff cannot be used with -x\");\n> -\t\tif (infos.edit)\n> +\t\tif (infos->edit)\n> \t\t\tdie(\"cherry-pick --ff cannot be used with --edit\");\n> \t}\n> \n> \tread_and_refresh_cache(me);\n> \n> -\tprepare_revs(&revs, &infos);\n> +\tprepare_revs(&revs, infos);\n> \n> \twhile ((commit = get_revision(&revs))) {\n> -\t\tint res = do_pick_commit(&infos, commit);\n> +\t\tint res = do_pick_commit(infos, commit);\n> \t\tif (res)\n> \t\t\treturn res;\n> \t}\n> @@ -615,6 +607,20 @@ static int revert_or_cherry_pick(int argc, const char **argv, int revert, int ed\n> \treturn 0;\n> }\n> \n> +static int revert_or_cherry_pick(int argc, const char **argv, int revert, int edit)\n> +{\n> +\tstruct args_info infos;\n> +\n> +\tgit_config(git_default_config, NULL);\n> +\tme = revert ? \"revert\" : \"cherry-pick\";\n> +\tsetenv(GIT_REFLOG_ACTION, me, 0);\n> +\tmemset(&infos, 0, sizeof(infos));\n> +\tinfos.action = revert ? REVERT : CHERRY_PICK;\n> +\tparse_args(argc, argv, &infos);\n> +\n> +\treturn pick_commits(&infos);\n> +}\n> +\n\nI think it would be more obvious to put this into cmd_revert and \ncmd_cherry_pick, and have them call pick_commits directly. In fact, you \ncould probably make things more clear by calling your \"struct args_info\" \ninstead \"struct pick_commits_args\" (like a lot of other \n\"struct {cmd}_args\" we already have for similar situations).\n\nWhile there's no reason to do it here, pick_commits() is a sensible \noperation that other builtins might want to call, particularly with the \nerror return instead of die(), so it would be nice to name things suitably \nfor that usage. That also avoids Junio's objection to the arguments to \nrevert_or_cherry_pick() by not having the function with the objectionable \narguments at all.\n\nFor that matter, you have a lot of commits in this series that put globals \ninto a struct and pass the struct around and change the arguments to the \nfunctions that actually do things. I think it would be easier to \nunderstand if you squashed all of these together into a single commit, \nwhich does all of the necessary changes to function prototypes. And I \nthink it would be similarly better to have a single commit that makes all \nof the places that call die() not do that, rather than getting some of \nthem in each of several patches.\n\n> int cmd_revert(int argc, const char **argv, const char *prefix)\n> {\n> \treturn revert_or_cherry_pick(argc, argv, 1, isatty(0));\n> -- \n> 1.7.3.2.504.g59d466\n> \n> \n> \n"}]}