{"thread":{"id":"20305","subject":"[PATCH] revert: libify pick","startedAt":"2009-07-31T03:25:47Z","lastAt":"2009-08-01T15:48:29Z","messageCount":3,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"119209","messageId":"20090731032548.4169.16389.chriscool@tuxfamily.org","threadId":"20305","inReplyTo":null,"subject":"[PATCH] revert: libify pick","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-07-31T03:25:47Z","receivedAt":"2009-07-31T03:25:47Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Stephan Beyer <s-beyer@gmx.net>\n\nThis commit is made of code from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)\n\nThe goal of this commit is to abstract out pick functionnality\ninto a new pick() function made of code from \"builtin-revert.c\".\n\nThe new pick() function is in a new \"pick.c\" file with an\nassociated \"pick.h\".\n\n\"pick.h\" and \"pick.c\" are strictly the same as on the sequencer repo,\nbut a few changes were needed on \"builtin-revert.c\" to keep it up to\ndate with changes on git.git.\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n\n---\n\n\tThis patch is part of trying to port git-rebase--interactive.sh\n\tto C using code from the sequencer GSoC project. But maybe it can\n\tbe seen as a clean up too.\n\n Makefile         |    2 +\n builtin-revert.c |  272 +++++++++---------------------------------------------\n pick.c           |  229 +++++++++++++++++++++++++++++++++++++++++++++\n pick.h           |   13 +++\n 4 files changed, 288 insertions(+), 228 deletions(-)\n create mode 100644 pick.c\n create mode 100644 pick.h\n\ndiff --git a/Makefile b/Makefile\nindex 0adc0cc..478be29 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -435,6 +435,7 @@ LIB_H += pack-refs.h\n LIB_H += pack-revindex.h\n LIB_H += parse-options.h\n LIB_H += patch-ids.h\n+LIB_H += pick.h\n LIB_H += pkt-line.h\n LIB_H += progress.h\n LIB_H += quote.h\n@@ -524,6 +525,7 @@ LIB_OBJS += parse-options.o\n LIB_OBJS += patch-delta.o\n LIB_OBJS += patch-ids.o\n LIB_OBJS += path.o\n+LIB_OBJS += pick.o\n LIB_OBJS += pkt-line.o\n LIB_OBJS += preload-index.o\n LIB_OBJS += pretty.o\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 151aa6a..6dd29a3 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -1,18 +1,14 @@\n #include \"cache.h\"\n #include \"builtin.h\"\n-#include \"object.h\"\n #include \"commit.h\"\n #include \"tag.h\"\n-#include \"wt-status.h\"\n-#include \"run-command.h\"\n #include \"exec_cmd.h\"\n #include \"utf8.h\"\n #include \"parse-options.h\"\n-#include \"cache-tree.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n #include \"rerere.h\"\n-#include \"merge-recursive.h\"\n+#include \"pick.h\"\n \n /*\n  * This implements the builtins revert and cherry-pick.\n@@ -35,25 +31,23 @@ static const char * const cherry_pick_usage[] = {\n \tNULL\n };\n \n-static int edit, no_replay, no_commit, mainline, signoff;\n-static enum { REVERT, CHERRY_PICK } action;\n+static int edit, no_commit, mainline, signoff;\n+static int flags;\n static struct commit *commit;\n \n-static const char *me;\n-\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n static void parse_args(int argc, const char **argv)\n {\n \tconst char * const * usage_str =\n-\t\taction == REVERT ?  revert_usage : cherry_pick_usage;\n+\t\tflags & PICK_REVERSE ? revert_usage : cherry_pick_usage;\n \tunsigned char sha1[20];\n \tconst char *arg;\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('x', NULL, &no_replay, \"append commit name when cherry-picking\"),\n+\t\tOPT_BIT('x', NULL, &flags, \"append commit name when cherry-picking\", PICK_ADD_NOTE),\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@@ -77,42 +71,12 @@ static void parse_args(int argc, const char **argv)\n \t\tdie (\"'%s' does not point to a commit\", arg);\n }\n \n-static char *get_oneline(const char *message)\n-{\n-\tchar *result;\n-\tconst char *p = message, *abbrev, *eol;\n-\tint abbrev_len, oneline_len;\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-\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n-\t\tp++;\n-\n-\tif (*p) {\n-\t\tp += 2;\n-\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n-\t\t\t; /* do nothing */\n-\t} else\n-\t\teol = p;\n-\tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n-\tabbrev_len = strlen(abbrev);\n-\toneline_len = eol - p;\n-\tresult = xmalloc(abbrev_len + 5 + oneline_len);\n-\tmemcpy(result, abbrev, abbrev_len);\n-\tmemcpy(result + abbrev_len, \"... \", 4);\n-\tmemcpy(result + abbrev_len + 4, p, oneline_len);\n-\tresult[abbrev_len + 4 + oneline_len] = '\\0';\n-\treturn result;\n-}\n-\n static char *get_encoding(const char *message)\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\treturn NULL;\n \twhile (*p && *p != '\\n') {\n \t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n \t\t\t; /* do nothing */\n@@ -128,30 +92,6 @@ static char *get_encoding(const char *message)\n \treturn NULL;\n }\n \n-static struct lock_file msg_file;\n-static int msg_fd;\n-\n-static void add_to_msg(const char *string)\n-{\n-\tint len = strlen(string);\n-\tif (write_in_full(msg_fd, string, len) < 0)\n-\t\tdie_errno (\"Could not write to MERGE_MSG\");\n-}\n-\n-static void add_message_to_msg(const char *message)\n-{\n-\tconst char *p = message;\n-\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n-\t\tp++;\n-\n-\tif (!*p)\n-\t\tadd_to_msg(sha1_to_hex(commit->object.sha1));\n-\n-\tp += 2;\n-\tadd_to_msg(p);\n-\treturn;\n-}\n-\n static void set_author_ident_env(const char *message)\n {\n \tconst char *p = message;\n@@ -214,7 +154,7 @@ static char *help_msg(const unsigned char *sha1)\n \t       \"mark the corrected paths with 'git add <paths>' \"\n \t       \"or 'git rm <paths>' and commit the result.\");\n \n-\tif (action == CHERRY_PICK) {\n+\tif (!(flags & PICK_REVERSE)) {\n \t\tsprintf(helpbuf + strlen(helpbuf),\n \t\t\t\"\\nWhen commiting, use the option \"\n \t\t\t\"'-c %s' to retain authorship and message.\",\n@@ -223,187 +163,68 @@ static char *help_msg(const unsigned char *sha1)\n \treturn helpbuf;\n }\n \n-static struct tree *empty_tree(void)\n+static void write_message(struct strbuf *msgbuf, const char *filename)\n {\n-\tstruct tree *tree = xcalloc(1, sizeof(struct tree));\n-\n-\ttree->object.parsed = 1;\n-\ttree->object.type = OBJ_TREE;\n-\tpretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);\n-\treturn tree;\n+\tstruct lock_file msg_file;\n+\tint msg_fd;\n+\tmsg_fd = hold_lock_file_for_update(&msg_file, filename,\n+\t\t\t\t\t   LOCK_DIE_ON_ERROR);\n+\tif (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)\n+\t\tdie_errno(\"Could not write to %s.\", filename);\n+\tstrbuf_release(msgbuf);\n+\tif (commit_lock_file(&msg_file) < 0)\n+\t\tdie(\"Error wrapping up %s\", filename);\n }\n \n static int revert_or_cherry_pick(int argc, const char **argv)\n {\n-\tunsigned char head[20];\n-\tstruct commit *base, *next, *parent;\n-\tint i, index_fd, clean;\n-\tchar *oneline, *reencoded_message = NULL;\n-\tconst char *message, *encoding;\n-\tchar *defmsg = git_pathdup(\"MERGE_MSG\");\n-\tstruct merge_options o;\n-\tstruct tree *result, *next_tree, *base_tree, *head_tree;\n-\tstatic struct lock_file index_lock;\n+\tconst char *me;\n+\tstruct strbuf msgbuf;\n+\tchar *reencoded_message = NULL;\n+\tconst char *encoding;\n+\tint failed;\n \n \tgit_config(git_default_config, NULL);\n-\tme = action == REVERT ? \"revert\" : \"cherry-pick\";\n+\tme = flags & PICK_REVERSE ? \"revert\" : \"cherry-pick\";\n \tsetenv(GIT_REFLOG_ACTION, me, 0);\n \tparse_args(argc, argv);\n \n-\t/* this is copied from the shell script, but it's never triggered... */\n-\tif (action == REVERT && !no_replay)\n-\t\tdie(\"revert is incompatible with replay\");\n-\n \tif (read_cache() < 0)\n \t\tdie(\"git %s: failed to read the index\", me);\n-\tif (no_commit) {\n-\t\t/*\n-\t\t * We do not intend to commit immediately.  We just want to\n-\t\t * merge the differences in, so let's compute the tree\n-\t\t * that represents the \"current\" state for merge-recursive\n-\t\t * to work on.\n-\t\t */\n-\t\tif (write_cache_as_tree(head, 0, NULL))\n-\t\t\tdie (\"Your index file is unmerged.\");\n-\t} else {\n-\t\tif (get_sha1(\"HEAD\", head))\n-\t\t\tdie (\"You do not have a valid HEAD\");\n-\t\tif (index_differs_from(\"HEAD\", 0))\n-\t\t\tdie (\"Dirty index: cannot %s\", me);\n-\t}\n-\tdiscard_cache();\n-\n-\tindex_fd = hold_locked_index(&index_lock, 1);\n+\tif (!no_commit && index_differs_from(\"HEAD\", 0))\n+\t\tdie (\"Dirty index: cannot %s\", me);\n \n-\tif (!commit->parents) {\n-\t\tif (action == REVERT)\n-\t\t\tdie (\"Cannot revert a root commit\");\n-\t\tparent = NULL;\n-\t}\n-\telse if (commit->parents->next) {\n-\t\t/* Reverting or cherry-picking a merge commit */\n-\t\tint cnt;\n-\t\tstruct commit_list *p;\n-\n-\t\tif (!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\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\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-\telse\n-\t\tparent = commit->parents->item;\n-\n-\tif (!(message = commit->buffer))\n-\t\tdie (\"Cannot get commit message for %s\",\n+\tif (!commit->buffer)\n+\t\treturn error(\"Cannot get commit message for %s\",\n \t\t\t\tsha1_to_hex(commit->object.sha1));\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-\n-\t/*\n-\t * \"commit\" is an existing commit.  We would want to apply\n-\t * the difference it introduces since its first parent \"prev\"\n-\t * on top of the current HEAD if we are cherry-pick.  Or the\n-\t * reverse of it if we are revert.\n-\t */\n-\n-\tmsg_fd = hold_lock_file_for_update(&msg_file, defmsg,\n-\t\t\t\t\t   LOCK_DIE_ON_ERROR);\n-\n-\tencoding = get_encoding(message);\n+\tencoding = get_encoding(commit->buffer);\n \tif (!encoding)\n \t\tencoding = \"UTF-8\";\n \tif (!git_commit_encoding)\n \t\tgit_commit_encoding = \"UTF-8\";\n-\tif ((reencoded_message = reencode_string(message,\n+\tif ((reencoded_message = reencode_string(commit->buffer,\n \t\t\t\t\tgit_commit_encoding, encoding)))\n-\t\tmessage = reencoded_message;\n-\n-\toneline = get_oneline(message);\n-\n-\tif (action == REVERT) {\n-\t\tchar *oneline_body = strchr(oneline, ' ');\n+\t\tcommit->buffer = reencoded_message;\n \n-\t\tbase = commit;\n-\t\tnext = parent;\n-\t\tadd_to_msg(\"Revert \\\"\");\n-\t\tadd_to_msg(oneline_body + 1);\n-\t\tadd_to_msg(\"\\\"\\n\\nThis reverts commit \");\n-\t\tadd_to_msg(sha1_to_hex(commit->object.sha1));\n-\n-\t\tif (commit->parents->next) {\n-\t\t\tadd_to_msg(\", reversing\\nchanges made to \");\n-\t\t\tadd_to_msg(sha1_to_hex(parent->object.sha1));\n-\t\t}\n-\t\tadd_to_msg(\".\\n\");\n-\t} else {\n-\t\tbase = parent;\n-\t\tnext = commit;\n-\t\tset_author_ident_env(message);\n-\t\tadd_message_to_msg(message);\n-\t\tif (no_replay) {\n-\t\t\tadd_to_msg(\"(cherry picked from commit \");\n-\t\t\tadd_to_msg(sha1_to_hex(commit->object.sha1));\n-\t\t\tadd_to_msg(\")\\n\");\n-\t\t}\n-\t}\n-\n-\tread_cache();\n-\tinit_merge_options(&o);\n-\to.branch1 = \"HEAD\";\n-\to.branch2 = oneline;\n-\n-\thead_tree = parse_tree_indirect(head);\n-\tnext_tree = next ? next->tree : empty_tree();\n-\tbase_tree = base ? base->tree : empty_tree();\n-\n-\tclean = merge_trees(&o,\n-\t\t\t    head_tree,\n-\t\t\t    next_tree, base_tree, &result);\n-\n-\tif (active_cache_changed &&\n-\t    (write_cache(index_fd, active_cache, active_nr) ||\n-\t     commit_locked_index(&index_lock)))\n-\t\tdie(\"%s: Unable to write new index file\", me);\n-\trollback_lock_file(&index_lock);\n-\n-\tif (!clean) {\n-\t\tadd_to_msg(\"\\nConflicts:\\n\\n\");\n-\t\tfor (i = 0; i < active_nr;) {\n-\t\t\tstruct cache_entry *ce = active_cache[i++];\n-\t\t\tif (ce_stage(ce)) {\n-\t\t\t\tadd_to_msg(\"\\t\");\n-\t\t\t\tadd_to_msg(ce->name);\n-\t\t\t\tadd_to_msg(\"\\n\");\n-\t\t\t\twhile (i < active_nr && !strcmp(ce->name,\n-\t\t\t\t\t\tactive_cache[i]->name))\n-\t\t\t\t\ti++;\n-\t\t\t}\n-\t\t}\n-\t\tif (commit_lock_file(&msg_file) < 0)\n-\t\t\tdie (\"Error wrapping up %s\", defmsg);\n+\tfailed = pick(commit, mainline, flags, &msgbuf);\n+\tif (failed < 0) {\n+\t\texit(1);\n+\t} else if (failed > 0) {\n \t\tfprintf(stderr, \"Automatic %s failed.%s\\n\",\n \t\t\tme, help_msg(commit->object.sha1));\n+\t\twrite_message(&msgbuf, git_path(\"MERGE_MSG\"));\n \t\trerere();\n \t\texit(1);\n \t}\n-\tif (commit_lock_file(&msg_file) < 0)\n-\t\tdie (\"Error wrapping up %s\", defmsg);\n+\tif (!(flags & PICK_REVERSE))\n+\t\tset_author_ident_env(commit->buffer);\n+\tfree(reencoded_message);\n+\n \tfprintf(stderr, \"Finished one %s.\\n\", me);\n \n+\twrite_message(&msgbuf, git_path(\"MERGE_MSG\"));\n+\n \t/*\n-\t *\n \t * If we are cherry-pick, and if the merge did not result in\n \t * hand-editing, we will hit this commit and inherit the original\n \t * author date and name.\n@@ -421,14 +242,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\t\targs[i++] = \"-s\";\n \t\tif (!edit) {\n \t\t\targs[i++] = \"-F\";\n-\t\t\targs[i++] = defmsg;\n+\t\t\targs[i++] = git_path(\"MERGE_MSG\");\n \t\t}\n \t\targs[i] = NULL;\n \t\treturn execv_git_cmd(args);\n \t}\n-\tfree(reencoded_message);\n-\tfree(defmsg);\n-\n \treturn 0;\n }\n \n@@ -436,14 +254,12 @@ int cmd_revert(int argc, const char **argv, const char *prefix)\n {\n \tif (isatty(0))\n \t\tedit = 1;\n-\tno_replay = 1;\n-\taction = REVERT;\n+\tflags = PICK_REVERSE | PICK_ADD_NOTE;\n \treturn revert_or_cherry_pick(argc, argv);\n }\n \n int cmd_cherry_pick(int argc, const char **argv, const char *prefix)\n {\n-\tno_replay = 0;\n-\taction = CHERRY_PICK;\n+\tflags = 0;\n \treturn revert_or_cherry_pick(argc, argv);\n }\ndiff --git a/pick.c b/pick.c\nnew file mode 100644\nindex 0000000..77c7169\n--- /dev/null\n+++ b/pick.c\n@@ -0,0 +1,229 @@\n+#include \"cache.h\"\n+#include \"commit.h\"\n+#include \"run-command.h\"\n+#include \"cache-tree.h\"\n+#include \"pick.h\"\n+#include \"merge-recursive.h\"\n+\n+static struct commit *commit;\n+\n+static char *get_oneline(const char *message)\n+{\n+\tchar *result;\n+\tconst char *p = message, *abbrev, *eol;\n+\tint abbrev_len, oneline_len;\n+\n+\tif (!p)\n+\t\treturn NULL;\n+\twhile (*p && (*p != '\\n' || p[1] != '\\n'))\n+\t\tp++;\n+\n+\tif (*p) {\n+\t\tp += 2;\n+\t\tfor (eol = p + 1; *eol && *eol != '\\n'; eol++)\n+\t\t\t; /* do nothing */\n+\t} else\n+\t\teol = p;\n+\tabbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);\n+\tabbrev_len = strlen(abbrev);\n+\toneline_len = eol - p;\n+\tresult = xmalloc(abbrev_len + 5 + oneline_len);\n+\tmemcpy(result, abbrev, abbrev_len);\n+\tmemcpy(result + abbrev_len, \"... \", 4);\n+\tmemcpy(result + abbrev_len + 4, p, oneline_len);\n+\tresult[abbrev_len + 4 + oneline_len] = '\\0';\n+\treturn result;\n+}\n+\n+static void add_message_to_msg(struct strbuf *msg, const char *message)\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(msg, sha1_to_hex(commit->object.sha1));\n+\n+\tp += 2;\n+\tstrbuf_addstr(msg, p);\n+\treturn;\n+}\n+\n+static struct tree *empty_tree(void)\n+{\n+\tstruct tree *tree = xcalloc(1, sizeof(struct tree));\n+\n+\ttree->object.parsed = 1;\n+\ttree->object.type = OBJ_TREE;\n+\tpretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);\n+\treturn tree;\n+}\n+\n+/*\n+ * Pick changes introduced by pick_commit into current working tree\n+ * and index.\n+ *\n+ * Return 0 on success.\n+ * Return negative value on error before picking,\n+ * and a positive value after picking,\n+ * and return 1 if and only if a conflict occurs but no other error.\n+ */\n+int pick(struct commit *pick_commit, int mainline, int flags,\n+\t\t\t\t\t\tstruct strbuf *msg)\n+{\n+\tunsigned char head[20];\n+\tstruct commit *base, *next, *parent;\n+\tint i, index_fd, clean;\n+\tint ret = 0;\n+\tchar *oneline;\n+\tconst char *message;\n+\tstruct merge_options o;\n+\tstruct tree *result, *next_tree, *base_tree, *head_tree;\n+\tstatic struct lock_file index_lock;\n+\n+\tstrbuf_init(msg, 0);\n+\tcommit = pick_commit;\n+\n+\tif (flags & PICK_REVERSE) /* REVERSE implies ADD_NOTE */\n+\t\tflags |= PICK_ADD_NOTE;\n+\n+\t/*\n+\t * We do not intend to commit immediately.  We just want to\n+\t * merge the differences in, so let's compute the tree\n+\t * that represents the \"current\" state for merge-recursive\n+\t * to work on.\n+\t */\n+\tif (write_cache_as_tree(head, 0, NULL)) {\n+\t\terror(\"Your index file is unmerged.\");\n+\t\treturn -1;\n+\t}\n+\tdiscard_cache();\n+\n+\tindex_fd = hold_locked_index(&index_lock, 0);\n+\tif (index_fd < 0) {\n+\t\terror(\"Unable to create locked index: %s\",\n+\t\t\t\t\t\tstrerror(errno));\n+\t\treturn -1;\n+\t}\n+\n+\tif (!commit->parents) {\n+\t\tif (flags & PICK_REVERSE) {\n+\t\t\terror(\"Cannot revert a root commit\");\n+\t\t\treturn -1;\n+\t\t}\n+\t\tparent = NULL;\n+\t}\n+\telse if (commit->parents->next) {\n+\t\t/* Reverting or cherry-picking a merge commit */\n+\t\tint cnt;\n+\t\tstruct commit_list *p;\n+\n+\t\tif (!mainline) {\n+\t\t\terror(\"Commit %s is a merge but no mainline was given.\",\n+\t\t\t    sha1_to_hex(commit->object.sha1));\n+\t\t\treturn -1;\n+\t\t}\n+\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\terror(\"Commit %s does not have parent %d\",\n+\t\t\t    sha1_to_hex(commit->object.sha1), mainline);\n+\t\t\treturn -1;\n+\t\t}\n+\t\tparent = p->item;\n+\t} else if (0 < mainline) {\n+\t\terror(\"Mainline was specified but commit %s is not a merge.\",\n+\t\t    sha1_to_hex(commit->object.sha1));\n+\t\treturn -1;\n+\t} else\n+\t\tparent = commit->parents->item;\n+\n+\tif (!(message = commit->buffer)) {\n+\t\terror(\"Cannot get commit message for %s\",\n+\t\t\t\tsha1_to_hex(commit->object.sha1));\n+\t\treturn -1;\n+\t}\n+\n+\tif (parent && parse_commit(parent) < 0) {\n+\t\terror(\"Cannot parse parent commit %s\",\n+\t\t      sha1_to_hex(parent->object.sha1));\n+\t\treturn -1;\n+\t}\n+\n+\toneline = get_oneline(message);\n+\n+\tif (flags & PICK_REVERSE) {\n+\t\tchar *oneline_body = strchr(oneline, ' ');\n+\n+\t\tbase = commit;\n+\t\tnext = parent;\n+\t\tstrbuf_addstr(msg, \"Revert \\\"\");\n+\t\tstrbuf_addstr(msg, oneline_body + 1);\n+\t\tstrbuf_addstr(msg, \"\\\"\\n\\nThis reverts commit \");\n+\t\tstrbuf_addstr(msg, sha1_to_hex(commit->object.sha1));\n+\n+\t\tif (commit->parents->next) {\n+\t\t\tstrbuf_addstr(msg, \", reversing\\nchanges made to \");\n+\t\t\tstrbuf_addstr(msg, sha1_to_hex(parent->object.sha1));\n+\t\t}\n+\t\tstrbuf_addstr(msg, \".\\n\");\n+\t} else {\n+\t\tbase = parent;\n+\t\tnext = commit;\n+\t\tadd_message_to_msg(msg, message);\n+\t\tif (flags & PICK_ADD_NOTE) {\n+\t\t\tstrbuf_addstr(msg, \"(cherry picked from commit \");\n+\t\t\tstrbuf_addstr(msg, sha1_to_hex(commit->object.sha1));\n+\t\t\tstrbuf_addstr(msg, \")\\n\");\n+\t\t}\n+\t}\n+\n+\tread_cache();\n+\tinit_merge_options(&o);\n+\to.branch1 = \"HEAD\";\n+\to.branch2 = oneline;\n+\n+\thead_tree = parse_tree_indirect(head);\n+\tnext_tree = next ? next->tree : empty_tree();\n+\tbase_tree = base ? base->tree : empty_tree();\n+\n+\tclean = merge_trees(&o,\n+\t\t\t    head_tree,\n+\t\t\t    next_tree, base_tree, &result);\n+\n+\tif (active_cache_changed &&\n+\t    (write_cache(index_fd, active_cache, active_nr) ||\n+\t     commit_locked_index(&index_lock))) {\n+\t\terror(\"Unable to write new index file\");\n+\t\treturn 2;\n+\t}\n+\trollback_lock_file(&index_lock);\n+\n+\tif (!clean) {\n+\t\tstrbuf_addstr(msg, \"\\nConflicts:\\n\\n\");\n+\t\tfor (i = 0; i < active_nr;) {\n+\t\t\tstruct cache_entry *ce = active_cache[i++];\n+\t\t\tif (ce_stage(ce)) {\n+\t\t\t\tstrbuf_addstr(msg, \"\\t\");\n+\t\t\t\tstrbuf_addstr(msg, ce->name);\n+\t\t\t\tstrbuf_addstr(msg, \"\\n\");\n+\t\t\t\twhile (i < active_nr && !strcmp(ce->name,\n+\t\t\t\t\t\tactive_cache[i]->name))\n+\t\t\t\t\ti++;\n+\t\t\t}\n+\t\t}\n+\t\tret = 1;\n+\t}\n+\tfree(oneline);\n+\n+\tdiscard_cache();\n+\tif (read_cache() < 0) {\n+\t\terror(\"Cannot read the index\");\n+\t\treturn 2;\n+\t}\n+\n+\treturn ret;\n+}\ndiff --git a/pick.h b/pick.h\nnew file mode 100644\nindex 0000000..7eb0d3a\n--- /dev/null\n+++ b/pick.h\n@@ -0,0 +1,13 @@\n+#ifndef PICK_H\n+#define PICK_H\n+\n+#include \"commit.h\"\n+\n+/* Pick flags: */\n+#define PICK_REVERSE   1 /* pick the reverse changes (\"revert\") */\n+#define PICK_ADD_NOTE  2 /* add note about original commit (unless conflict) */\n+/* We don't need a PICK_QUIET. This is done by\n+ *\tsetenv(\"GIT_MERGE_VERBOSITY\", \"0\", 1); */\n+extern int pick(struct commit *pick_commit, int mainline, int flags, struct strbuf *msg);\n+\n+#endif\n-- \n1.6.3.GIT\n"},{"id":"119217","messageId":"7v8wi52uig.fsf@alter.siamese.dyndns.org","threadId":"20305","inReplyTo":"20090731032548.4169.16389.chriscool@tuxfamily.org","subject":"Re: [PATCH] revert: libify pick","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-31T07:15:19Z","receivedAt":"2009-07-31T07:15:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> From: Stephan Beyer <s-beyer@gmx.net>\n>\n> This commit is made of code from the sequencer GSoC project:\n>\n> git://repo.or.cz/git/sbeyer.git\n>\n> (commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)\n>\n> The goal of this commit is to abstract out pick functionnality\n> into a new pick() function made of code from \"builtin-revert.c\".\n>\n> The new pick() function is in a new \"pick.c\" file with an\n> associated \"pick.h\".\n>\n> \"pick.h\" and \"pick.c\" are strictly the same as on the sequencer repo,\n> but a few changes were needed on \"builtin-revert.c\" to keep it up to\n> date with changes on git.git.\n>\n> Mentored-by: Daniel Barkalow <barkalow@iabervon.org>\n> Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>\n> ---\n>\n> \tThis patch is part of trying to port git-rebase--interactive.sh\n> \tto C using code from the sequencer GSoC project. But maybe it can\n> \tbe seen as a clean up too.\n\nThanks.  Why doesn't this have Stephan's sign-off?\n\nThe new \"pick.c\" file seems to be a nicer implementation of the main logic\nof builtin-revert.c and its primary niceness comes from the use of strbuf.\n\nA few minor points and comments.\n\n * error() returns -1.\n\n        error(\"message\");     =>        return error(\"message\");\n        return -1;\n\n * pick() might be a bit too short and abstract name for a generic library\n   function.\n\n * REVERSE is made to imply ADD_NOTE but the codepath that acts on\n   ADD_NOTE never seems to be reached if REVERSE is set.\n\nThe intent of pick() funtion looks like it starts from the current index\n(not HEAD), and allow the effect of one commit replayed (either forward or\nbackward) to that state, leaving the result in the index.\n\nYou do not have to start from a commit, so you can replay many commits to\nthe index in sequence without commiting in between to squash multiple\nsteps if you wanted to.  I think that makes sense as a nice general\ninterface.\n\nThe \"if (no_commit)\" codepath in the original code did things very\ndifferently from the usual \"start from HEAD and replay the effect\"\ncodepath and it warranted the big \"We do not intend to commit immediately\"\ncomment.  For pick() function, however, the \"start from index\" is the\nnormal and only mode of operation.  Keeping the big comment is misleading.\n\nWhen it replays another commit on HEAD, the new code does not read \"HEAD\"\nby hand into head anymore, but it still has the check between the index\nand \"HEAD\" and refuses to run if the index is dirty, which means the tree\nyou get from write_cache_as_tree() is guaranteed to be the same as \"HEAD\",\nso this conversion looks correct.\n"},{"id":"119337","messageId":"200908011748.30032.chriscool@tuxfamily.org","threadId":"20305","inReplyTo":"7v8wi52uig.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] revert: libify pick","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-01T15:48:29Z","receivedAt":"2009-08-01T15:48:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Friday 31 July 2009, Junio C Hamano wrote:\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> >\n> > \tThis patch is part of trying to port git-rebase--interactive.sh\n> > \tto C using code from the sequencer GSoC project. But maybe it can\n> > \tbe seen as a clean up too.\n>\n> Thanks.  Why doesn't this have Stephan's sign-off?\n\nIt has it in the v2 series I will post just after sending this email.\n\nAs usual feel free to squash all or some of the patches together if you \nprefer.\n\n> The new \"pick.c\" file seems to be a nicer implementation of the main\n> logic of builtin-revert.c and its primary niceness comes from the use of\n> strbuf.\n>\n> A few minor points and comments.\n>\n>  * error() returns -1.\n>\n>         error(\"message\");     =>        return error(\"message\");\n>         return -1;\n\nThis change is in patch 2.\n\n>  * pick() might be a bit too short and abstract name for a generic\n> library function.\n\nI changed it to \"pick_commit()\" in patch 3.\n\n>  * REVERSE is made to imply ADD_NOTE but the codepath that acts on\n>    ADD_NOTE never seems to be reached if REVERSE is set.\n\nI removed code that made REVERSE imply ADD_NOTE in patch 4\n\n> The intent of pick() funtion looks like it starts from the current index\n> (not HEAD), and allow the effect of one commit replayed (either forward\n> or backward) to that state, leaving the result in the index.\n>\n> You do not have to start from a commit, so you can replay many commits to\n> the index in sequence without commiting in between to squash multiple\n> steps if you wanted to.  I think that makes sense as a nice general\n> interface.\n>\n> The \"if (no_commit)\" codepath in the original code did things very\n> differently from the usual \"start from HEAD and replay the effect\"\n> codepath and it warranted the big \"We do not intend to commit\n> immediately\" comment.  For pick() function, however, the \"start from\n> index\" is the normal and only mode of operation.  Keeping the big comment\n> is misleading.\n\nI removed the misleading part and added some of your comments in patch 5.\n\n> When it replays another commit on HEAD, the new code does not read \"HEAD\"\n> by hand into head anymore, but it still has the check between the index\n> and \"HEAD\" and refuses to run if the index is dirty, which means the tree\n> you get from write_cache_as_tree() is guaranteed to be the same as\n> \"HEAD\", so this conversion looks correct.\n\nThanks,\nChristian.\n"}]}