{"thread":{"id":"20695","subject":"[PATCH v3 0/9] more changes to port rebase -i to C using sequencer code","startedAt":"2009-08-22T04:16:06Z","lastAt":"2009-08-22T20:15:34Z","messageCount":13,"participants":["Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":9},"messages":[{"id":"121491","messageId":"20090822041157.4261.92491.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":null,"subject":"[PATCH v3 0/9] more changes to port rebase -i to C using sequencer code","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:06Z","receivedAt":"2009-08-22T04:16:06Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This is another reroll of the series I first sent on August the 12th.\n\nThe only changes since v2 are in the commit messages. References to the\npick() function have been replaced by references to pick_commit(), and I\ntried to describe the purpose of the patch first. \n\nChristian Couder (5):\n  sequencer: add \"--fast-forward\" option to \"git sequencer--helper\"\n  sequencer: let \"git sequencer--helper\" callers set \"allow_dirty\"\n  rebase -i: use \"git sequencer--helper --fast-forward\"\n  pick: libify \"pick_help_msg()\"\n  rebase -i: use \"git sequencer--helper --cherry-pick\"\n\nStephan Beyer (4):\n  sequencer: add \"do_fast_forward()\" to perform a fast forward\n  revert: libify pick\n  sequencer: add \"do_commit()\" and related functions\n  sequencer: add \"--cherry-pick\" option to \"git sequencer--helper\"\n\n Makefile                    |    2 +\n builtin-revert.c            |  293 ++++++-----------------------------------\n builtin-sequencer--helper.c |  305 +++++++++++++++++++++++++++++++++++++++++--\n git-rebase--interactive.sh  |   22 ++--\n pick.c                      |  232 ++++++++++++++++++++++++++++++++\n pick.h                      |   14 ++\n 6 files changed, 599 insertions(+), 269 deletions(-)\n create mode 100644 pick.c\n create mode 100644 pick.h\n"},{"id":"121492","messageId":"20090822041616.4261.40442.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 1/9] sequencer: add \"do_fast_forward()\" to perform a fast forward","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:07Z","receivedAt":"2009-08-22T04:16:07Z","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 code is taken from the sequencer GSoC project:\n\n    git://repo.or.cz/git/sbeyer.git\n\n    (commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)\n\nbut the messages have been changed to be the same as those\ndisplayed by git-rebase--interactive.sh.\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-sequencer--helper.c |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c\nindex be030bc..0cd7e98 100644\n--- a/builtin-sequencer--helper.c\n+++ b/builtin-sequencer--helper.c\n@@ -171,6 +171,15 @@ static struct commit *get_commit(const char *arg)\n \treturn lookup_commit_reference(sha1);\n }\n \n+static int do_fast_forward(const unsigned char *sha)\n+{\n+\tif (reset_almost_hard(sha))\n+\t\treturn error(\"Cannot fast forward to %s\", sha1_to_hex(sha));\n+\tif (verbosity > 1)\n+\t\tprintf(\"Fast forward to %s\\n\", sha1_to_hex(sha));\n+\treturn 0;\n+}\n+\n static int set_verbosity(int verbose)\n {\n \tchar tmp[] = \"0\";\n-- \n1.6.4.271.ge010d\n"},{"id":"121493","messageId":"20090822041616.4261.45315.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 2/9] sequencer: add \"--fast-forward\" option to \"git sequencer--helper\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:08Z","receivedAt":"2009-08-22T04:16:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This new option uses the \"do_fast_forward()\" function to perform\na fast forward.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-sequencer--helper.c |   16 ++++++++++++----\n 1 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c\nindex 0cd7e98..bd72f65 100644\n--- a/builtin-sequencer--helper.c\n+++ b/builtin-sequencer--helper.c\n@@ -19,6 +19,7 @@ static unsigned char head_sha1[20];\n static const char * const git_sequencer_helper_usage[] = {\n \t\"git sequencer--helper --make-patch <commit>\",\n \t\"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>\",\n+\t\"git sequencer--helper --fast-forward <commit> <reflog-msg> <verbosity>\",\n \tNULL\n };\n \n@@ -218,11 +219,14 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n {\n \tchar *patch_commit = NULL;\n \tchar *reset_commit = NULL;\n+\tchar *ff_commit = NULL;\n \tstruct option options[] = {\n \t\tOPT_STRING(0, \"make-patch\", &patch_commit, \"commit\",\n \t\t\t   \"create a patch from commit\"),\n \t\tOPT_STRING(0, \"reset-hard\", &reset_commit, \"commit\",\n \t\t\t   \"reset to commit\"),\n+\t\tOPT_STRING(0, \"fast-forward\", &ff_commit, \"commit\",\n+\t\t\t   \"fast forward to commit\"),\n \t\tOPT_END()\n \t};\n \n@@ -239,15 +243,16 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\n \t}\n \n-\tif (reset_commit) {\n+\tif (ff_commit || reset_commit) {\n \t\tunsigned char sha1[20];\n+\t\tchar *commit = ff_commit ? ff_commit : reset_commit;\n \n \t\tif (argc != 2)\n \t\t\tusage_with_options(git_sequencer_helper_usage,\n \t\t\t\t\t   options);\n \n-\t\tif (get_sha1(reset_commit, sha1)) {\n-\t\t\terror(\"Could not find '%s'\", reset_commit);\n+\t\tif (get_sha1(commit, sha1)) {\n+\t\t\terror(\"Could not find '%s'\", commit);\n \t\t\treturn 1;\n \t\t}\n \n@@ -258,7 +263,10 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn 1;\n \t\t}\n \n-\t\treturn reset_almost_hard(sha1);\n+\t\tif (ff_commit)\n+\t\t\treturn do_fast_forward(sha1);\n+\t\telse\n+\t\t\treturn reset_almost_hard(sha1);\n \t}\n \n \tusage_with_options(git_sequencer_helper_usage, options);\n-- \n1.6.4.271.ge010d\n"},{"id":"121495","messageId":"20090822041616.4261.91862.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 3/9] sequencer: let \"git sequencer--helper\" callers set \"allow_dirty\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:09Z","receivedAt":"2009-08-22T04:16:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This flag can be set when using --reset-hard or --fast-forward, and\nin this case changes in the work tree will be kept.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-sequencer--helper.c |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c\nindex bd72f65..71a7fef 100644\n--- a/builtin-sequencer--helper.c\n+++ b/builtin-sequencer--helper.c\n@@ -18,8 +18,10 @@ static unsigned char head_sha1[20];\n \n static const char * const git_sequencer_helper_usage[] = {\n \t\"git sequencer--helper --make-patch <commit>\",\n-\t\"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>\",\n-\t\"git sequencer--helper --fast-forward <commit> <reflog-msg> <verbosity>\",\n+\t\"git sequencer--helper --reset-hard <commit> <reflog-msg> \"\n+\t\t\"<verbosity> [<allow-dirty>]\",\n+\t\"git sequencer--helper --fast-forward <commit> <reflog-msg> \"\n+\t\t\"<verbosity> [<allow-dirty>]\",\n \tNULL\n };\n \n@@ -247,7 +249,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\tunsigned char sha1[20];\n \t\tchar *commit = ff_commit ? ff_commit : reset_commit;\n \n-\t\tif (argc != 2)\n+\t\tif (argc != 2 && argc != 3)\n \t\t\tusage_with_options(git_sequencer_helper_usage,\n \t\t\t\t\t   options);\n \n@@ -263,6 +265,9 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\t\treturn 1;\n \t\t}\n \n+\t\tif (argc == 3 && *argv[2] && strcmp(argv[2], \"0\"))\n+\t\t\tallow_dirty = 1;\n+\n \t\tif (ff_commit)\n \t\t\treturn do_fast_forward(sha1);\n \t\telse\n-- \n1.6.4.271.ge010d\n"},{"id":"121494","messageId":"20090822041616.4261.52894.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 4/9] rebase -i: use \"git sequencer--helper --fast-forward\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:10Z","receivedAt":"2009-08-22T04:16:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"when fast forwarding.\n\nNote that in the first hunk of this patch, there was this line:\n\ntest \"a$1\" = a-n && output git reset --soft $current_sha1\n\nbut the test always failed.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-rebase--interactive.sh |   11 +++--------\n 1 files changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 0041994..7651fd6 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -154,11 +154,8 @@ pick_one () {\n \t\tdie \"Could not get the parent of $sha1\"\n \tcurrent_sha1=$(git rev-parse --verify HEAD)\n \tif test \"$no_ff$current_sha1\" = \"$parent_sha1\"; then\n-\t\tgit sequencer--helper --reset-hard $sha1 \\\n+\t\tgit sequencer--helper --fast-forward $sha1 \\\n \t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\"\n-\t\ttest \"a$1\" = a-n && output git reset --soft $current_sha1\n-\t\tsha1=$(git rev-parse --short $sha1)\n-\t\toutput warn Fast forward to $sha1\n \telse\n \t\toutput git cherry-pick \"$@\"\n \tfi\n@@ -238,10 +235,8 @@ pick_one_preserving_merges () {\n \tdone\n \tcase $fast_forward in\n \tt)\n-\t\toutput warn \"Fast forward to $sha1\"\n-\t\tgit sequencer--helper --reset-hard $sha1 \\\n-\t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\" ||\n-\t\t\tdie \"Cannot fast forward to $sha1\"\n+\t\tgit sequencer--helper --fast-forward $sha1 \\\n+\t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\" || exit\n \t\t;;\n \tf)\n \t\tfirst_parent=$(expr \"$new_parents\" : ' \\([^ ]*\\)')\n-- \n1.6.4.271.ge010d\n"},{"id":"121496","messageId":"20090822041616.4261.3709.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 5/9] revert: libify pick","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:11Z","receivedAt":"2009-08-22T04:16:11Z","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\nThe goal of this commit is to abstract out pick functionnality\ninto a new pick_commit() function made of code from\n\"builtin-revert.c\".\n\nThe new pick_commit() function is in a new \"pick.c\" file with an\nassociated \"pick.h\".\n\nThis commit is made of code from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)\n\nAnd it contains some changes suggested by Junio.\n\nThe original commit in the sequencer project that introduced\nthis change is: 94a568a78d243d7a6c13778bc6b7ac1eb46e48cc\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Makefile         |    2 +\n builtin-revert.c |  272 +++++++++---------------------------------------------\n pick.c           |  210 +++++++++++++++++++++++++++++++++++++++++\n pick.h           |   13 +++\n 4 files changed, 269 insertions(+), 228 deletions(-)\n create mode 100644 pick.c\n create mode 100644 pick.h\n\ndiff --git a/Makefile b/Makefile\nindex 98dd01d..5e76083 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -445,6 +445,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@@ -533,6 +534,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..4797ac5 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(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..058b877\n--- /dev/null\n+++ b/pick.c\n@@ -0,0 +1,210 @@\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 \"commit\" argument into current working\n+ * tree 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_commit(struct commit *pick_commit, int mainline, int flags,\n+\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+\t/*\n+\t * Let's compute the tree that represents the \"current\" state\n+\t * for merge-recursive to work on.\n+\t */\n+\tif (write_cache_as_tree(head, 0, NULL))\n+\t\treturn error(\"Your index file is unmerged.\");\n+\tdiscard_cache();\n+\n+\tindex_fd = hold_locked_index(&index_lock, 0);\n+\tif (index_fd < 0)\n+\t\treturn error(\"Unable to create locked index: %s\",\n+\t\t\t     strerror(errno));\n+\n+\tif (!commit->parents) {\n+\t\tif (flags & PICK_REVERSE)\n+\t\t\treturn error(\"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\treturn error(\"Commit %s is a merge but no mainline was given.\",\n+\t\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\treturn error(\"Commit %s does not have parent %d\",\n+\t\t\t\t     sha1_to_hex(commit->object.sha1),\n+\t\t\t\t     mainline);\n+\t\tparent = p->item;\n+\t} else if (0 < 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 (!(message = commit->buffer))\n+\t\treturn error(\"Cannot get commit message for %s\",\n+\t\t\t     sha1_to_hex(commit->object.sha1));\n+\n+\tif (parent && parse_commit(parent) < 0)\n+\t\treturn error(\"Cannot parse parent commit %s\",\n+\t\t\t     sha1_to_hex(parent->object.sha1));\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..7a74ad8\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_commit(struct commit *commit, int mainline, int flags, struct strbuf *msg);\n+\n+#endif\n-- \n1.6.4.271.ge010d\n"},{"id":"121499","messageId":"20090822041616.4261.59306.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 6/9] pick: libify \"pick_help_msg()\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:12Z","receivedAt":"2009-08-22T04:16:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This function gives an help message when pick or revert failed.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-revert.c |   23 +----------------------\n pick.c           |   22 ++++++++++++++++++++++\n pick.h           |    1 +\n 3 files changed, 24 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 4797ac5..e5250bd 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -142,27 +142,6 @@ static void set_author_ident_env(const char *message)\n \t\t\tsha1_to_hex(commit->object.sha1));\n }\n \n-static char *help_msg(const unsigned char *sha1)\n-{\n-\tstatic char helpbuf[1024];\n-\tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n-\n-\tif (msg)\n-\t\treturn msg;\n-\n-\tstrcpy(helpbuf, \"  After resolving the conflicts,\\n\"\n-\t       \"mark the corrected paths with 'git add <paths>' \"\n-\t       \"or 'git rm <paths>' and commit the result.\");\n-\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-\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n-\t}\n-\treturn helpbuf;\n-}\n-\n static void write_message(struct strbuf *msgbuf, const char *filename)\n {\n \tstruct lock_file msg_file;\n@@ -211,7 +190,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\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\t\tme, pick_help_msg(commit->object.sha1, flags));\n \t\twrite_message(&msgbuf, git_path(\"MERGE_MSG\"));\n \t\trerere();\n \t\texit(1);\ndiff --git a/pick.c b/pick.c\nindex 058b877..4f882bb 100644\n--- a/pick.c\n+++ b/pick.c\n@@ -208,3 +208,25 @@ int pick_commit(struct commit *pick_commit, int mainline, int flags,\n \n \treturn ret;\n }\n+\n+char *pick_help_msg(const unsigned char *sha1, int flags)\n+{\n+\tstatic char helpbuf[1024];\n+\tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n+\n+\tif (msg)\n+\t\treturn msg;\n+\n+\tstrcpy(helpbuf, \"  After resolving the conflicts,\\n\"\n+\t       \"mark the corrected paths with 'git add <paths>' \"\n+\t       \"or 'git rm <paths>' and commit the result.\");\n+\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+\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n+\t}\n+\treturn helpbuf;\n+}\n+\ndiff --git a/pick.h b/pick.h\nindex 7a74ad8..115541a 100644\n--- a/pick.h\n+++ b/pick.h\n@@ -9,5 +9,6 @@\n /* We don't need a PICK_QUIET. This is done by\n  *\tsetenv(\"GIT_MERGE_VERBOSITY\", \"0\", 1); */\n extern int pick_commit(struct commit *commit, int mainline, int flags, struct strbuf *msg);\n+extern char *pick_help_msg(const unsigned char *sha1, int flags);\n \n #endif\n-- \n1.6.4.271.ge010d\n"},{"id":"121500","messageId":"20090822041616.4261.65829.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 7/9] sequencer: add \"do_commit()\" and related functions","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:13Z","receivedAt":"2009-08-22T04:16:13Z","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\nIt adds \"struct commit_info\", the \"next_commit\" static variable and\nthe following functions:\n\n        - do_commit()\n        - set_author_info()\n        - set_message_source()\n        - set_pick_subject()\n        - write_commit_summary_into()\n\nThis makes it possible to prepare and perform a commit (without\nforking and execing \"git commit\").\n\nThis patch adds some code that comes from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)\n\nCompared to the sequencer project, the only change is that \"mark\"\nrelated (3 lines long) code has been removed from do_commit().\n\n    Mentored-by: Daniel Barkalow <barkalow@iabervon.org>\n    Mentored-by: Christian Couder <chriscool@tuxfamily.org>\n    Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-sequencer--helper.c |  214 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 214 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c\nindex 71a7fef..61a8f2e 100644\n--- a/builtin-sequencer--helper.c\n+++ b/builtin-sequencer--helper.c\n@@ -5,26 +5,69 @@\n #include \"refs.h\"\n #include \"diff.h\"\n #include \"unpack-trees.h\"\n+#include \"string-list.h\"\n+#include \"pick.h\"\n+#include \"rerere.h\"\n+#include \"dir.h\"\n+#include \"cache-tree.h\"\n+#include \"utf8.h\"\n \n #define SEQ_DIR \"rebase-merge\"\n \n #define PATCH_FILE\tgit_path(SEQ_DIR \"/patch\")\n+#define MERGE_MSG\tgit_path(\"MERGE_MSG\")\n+#define SQUASH_MSG\tgit_path(\"SQUASH_MSG\")\n+\n+/**********************************************************************\n+ * Data structures\n+ */\n+\n+struct user_info {\n+\tconst char *name;\n+\tconst char *mail;\n+\tconst char *time; /* \"<timestamp> <timezone>\" */\n+};\n+\n+struct commit_info {\n+\tstruct user_info author; /* author info */\n+\tstruct user_info committer; /* not used, but for easy extendability */\n+\tconst char *encoding; /* encoding */\n+\tchar *subject; /* basically the first line of the summary */\n+\tstruct strbuf summary; /* the commit message */\n+\tchar *source; /* source of the commit message, either\n+\t\t       * \"message\", \"merge\", \"squash\" or a commit SHA1 */\n+\tchar *patch; /* a patch */\n+\tstruct string_list parents; /* list of parents' hex'ed sha1 ids */\n+};\n+\n+/**********************************************************************\n+ * Global variables\n+ */\n \n static char *reflog;\n \n+static int squash_count = 0;\n+\n static int allow_dirty = 0, verbosity = 1, advice = 1;\n \n static unsigned char head_sha1[20];\n \n+static struct commit_info next_commit;\n+\n static const char * const git_sequencer_helper_usage[] = {\n \t\"git sequencer--helper --make-patch <commit>\",\n \t\"git sequencer--helper --reset-hard <commit> <reflog-msg> \"\n \t\t\"<verbosity> [<allow-dirty>]\",\n \t\"git sequencer--helper --fast-forward <commit> <reflog-msg> \"\n \t\t\"<verbosity> [<allow-dirty>]\",\n+\t\"git sequencer--helper --cherry-pick <commit> [<do-not-commit>]\",\n \tNULL\n };\n \n+/**********************************************************************\n+ * Sequencer functions\n+ */\n+\n static int parse_and_init_tree_desc(const unsigned char *sha1,\n \t\t\t\t    struct tree_desc *desc)\n {\n@@ -162,6 +205,157 @@ static void make_patch(struct commit *commit)\n \tfree(args);\n }\n \n+/* Commit current index with information next_commit onto parent_sha1. */\n+static int do_commit(unsigned char *parent_sha1)\n+{\n+\tint failed;\n+\tunsigned char tree_sha1[20];\n+\tunsigned char commit_sha1[20];\n+\tstruct strbuf sbuf;\n+\tconst char *reencoded = NULL;\n+\n+\tif (squash_count) {\n+\t\tsquash_count = 0;\n+\t\tif (file_exists(SQUASH_MSG))\n+\t\t\tunlink(SQUASH_MSG);\n+\t}\n+\n+\tif (!index_differs_from(\"HEAD\", 0) &&\n+\t    !next_commit.parents.nr)\n+\t\treturn error(\"No changes! Do you really want an empty commit?\");\n+\n+\tif (!next_commit.author.name || !next_commit.author.mail)\n+\t\treturn error(\"Internal error: Author information not set properly.\");\n+\n+\tif (write_cache_as_tree(tree_sha1, 0, NULL))\n+\t\treturn 1;\n+\n+\tif (!next_commit.encoding)\n+\t\tnext_commit.encoding = xstrdup(\"utf-8\");\n+\tif (!git_commit_encoding)\n+\t\tgit_commit_encoding = \"utf-8\";\n+\n+\tstrbuf_init(&sbuf, 8192); /* should avoid reallocs for the headers */\n+\tstrbuf_addf(&sbuf, \"tree %s\\n\", sha1_to_hex(tree_sha1));\n+\tif (parent_sha1)\n+\t\tstrbuf_addf(&sbuf, \"parent %s\\n\", sha1_to_hex(parent_sha1));\n+\tif (next_commit.parents.nr) {\n+\t\tint i;\n+\t\tfor (i = 0; i < next_commit.parents.nr; ++i)\n+\t\t\tstrbuf_addf(&sbuf, \"parent %s\\n\",\n+\t\t\t\t\tnext_commit.parents.items[i].string);\n+\t}\n+\tif (!next_commit.author.time) {\n+\t\tchar time[50];\n+\t\tdatestamp(time, sizeof(time));\n+\t\tnext_commit.author.time = xstrdup(time);\n+\t}\n+\n+\tstripspace(&next_commit.summary, 1);\n+\n+\t/* if encodings differ, reencode whole buffer */\n+\tif (strcasecmp(git_commit_encoding, next_commit.encoding)) {\n+\t\tif ((reencoded = reencode_string(next_commit.author.name,\n+\t\t\t\tgit_commit_encoding, next_commit.encoding))) {\n+\t\t\tfree((void *)next_commit.author.name);\n+\t\t\tnext_commit.author.name = reencoded;\n+\t\t}\n+\t\tif ((reencoded = reencode_string(next_commit.summary.buf,\n+\t\t\t\tgit_commit_encoding, next_commit.encoding))) {\n+\t\t\tstrbuf_reset(&next_commit.summary);\n+\t\t\tstrbuf_addstr(&next_commit.summary, reencoded);\n+\t\t}\n+\t}\n+\tstrbuf_addf(&sbuf, \"author %s <%s> %s\\n\", next_commit.author.name,\n+\t\t\tnext_commit.author.mail, next_commit.author.time);\n+\tstrbuf_addf(&sbuf, \"committer %s\\n\", git_committer_info(0));\n+\tif (!is_encoding_utf8(git_commit_encoding))\n+\t\tstrbuf_addf(&sbuf, \"encoding %s\\n\", git_commit_encoding);\n+\tstrbuf_addch(&sbuf, '\\n');\n+\tstrbuf_addbuf(&sbuf, &next_commit.summary);\n+\tif (sbuf.buf[sbuf.len-1] != '\\n')\n+\t\tstrbuf_addch(&sbuf, '\\n');\n+\n+\tfailed = write_sha1_file(sbuf.buf, sbuf.len, commit_type, commit_sha1);\n+\tstrbuf_release(&sbuf);\n+\tif (failed)\n+\t\treturn 1;\n+\n+\tif (verbosity > 1)\n+\t\tprintf(\"Created %scommit %s\\n\",\n+\t\t\tparent_sha1 || next_commit.parents.nr ? \"\" : \"initial \",\n+\t\t\tsha1_to_hex(commit_sha1));\n+\n+\tif (update_ref(reflog, \"HEAD\", commit_sha1, NULL, 0, 0))\n+\t\treturn error(\"Could not update HEAD to %s.\",\n+\t\t\t\t\t\tsha1_to_hex(commit_sha1));\n+\n+\treturn 0;\n+}\n+\n+/*\n+ * Fill next_commit.author according to ident.\n+ * Ident may have one of the following forms:\n+ * \t\"name <e-mail> timestamp timezone\\n...\"\n+ * \t\"name <e-mail> timestamp timezone\"\n+ * \t\"name <e-mail>\"\n+ */\n+static void set_author_info(const char *ident)\n+{\n+\tconst char *tmp1 = strstr(ident, \" <\");\n+\tconst char *tmp2;\n+\tchar *data;\n+\tif (!tmp1)\n+\t\treturn;\n+\ttmp2 = strstr(tmp1+2, \">\");\n+\tif (!tmp2)\n+\t\treturn;\n+\tif (tmp2[1] != 0 && tmp2[1] != ' ')\n+\t\treturn;\n+\n+\tdata = xmalloc(strlen(ident)); /* a trivial upper bound */\n+\n+\tsnprintf(data, tmp1-ident+1, \"%s\", ident);\n+\tnext_commit.author.name = xstrdup(data);\n+\tsnprintf(data, tmp2-tmp1-1, \"%s\", tmp1+2);\n+\tnext_commit.author.mail = xstrdup(data);\n+\n+\tif (tmp2[1] == 0) {\n+\t\tfree(data);\n+\t\treturn;\n+\t}\n+\n+\ttmp1 = strpbrk(tmp2+2, \"\\r\\n\");\n+\tif (!tmp1)\n+\t\ttmp1 = tmp2 + strlen(tmp2);\n+\n+\tsnprintf(data, tmp1-tmp2-1, \"%s\", tmp2+2);\n+\tnext_commit.author.time = xstrdup(data);\n+\tfree(data);\n+}\n+\n+static void set_message_source(const char *source)\n+{\n+\tif (next_commit.source)\n+\t\tfree(next_commit.source);\n+\tnext_commit.source = xstrdup(source);\n+}\n+\n+/* Set subject, an information for the case of conflict */\n+static void set_pick_subject(const char *hex, struct commit *commit)\n+{\n+\tconst char *tmp = strstr(commit->buffer, \"\\n\\n\");\n+\tif (tmp) {\n+\t\tconst char *eol;\n+\t\tint len = strlen(hex);\n+\t\ttmp += 2;\n+\t\teol = strchrnul(tmp, '\\n');\n+\t\tnext_commit.subject = xmalloc(eol - tmp + len + 5);\n+\t\tsnprintf(next_commit.subject, eol - tmp + len + 5, \"%s... %s\",\n+\t\t\t\t\t\t\t\thex, tmp);\n+\t}\n+}\n+\n /* Return a commit object of \"arg\" */\n static struct commit *get_commit(const char *arg)\n {\n@@ -198,6 +392,26 @@ static int set_verbosity(int verbose)\n \treturn 0;\n }\n \n+static int write_commit_summary_into(const char *filename)\n+{\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tint fd = hold_lock_file_for_update(lock, filename, 0);\n+\tif (fd < 0)\n+\t\treturn error(\"Unable to create '%s.lock': %s\", filename,\n+\t\t\t\t\t\t\tstrerror(errno));\n+\tif (write_in_full(fd, next_commit.summary.buf,\n+\t\t\t      next_commit.summary.len) < 0)\n+\t\treturn error(\"Could not write to %s: %s\",\n+\t\t\t\t\t\tfilename, strerror(errno));\n+\tif (commit_lock_file(lock) < 0)\n+\t\treturn error(\"Error wrapping up %s\", filename);\n+\treturn 0;\n+}\n+\n+/**********************************************************************\n+ * Builtin sequencer helper functions\n+ */\n+\n /* v should be \"\" or \"t\" or \"\\d\" */\n static int parse_verbosity(const char *v)\n {\n-- \n1.6.4.271.ge010d\n"},{"id":"121497","messageId":"20090822041616.4261.14355.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 8/9] sequencer: add \"--cherry-pick\" option to \"git sequencer--helper\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:14Z","receivedAt":"2009-08-22T04:16:14Z","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 patch adds some code that comes from the sequencer GSoC project:\n\ngit://repo.or.cz/git/sbeyer.git\n\n(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)\n\nMost of the code from do_cherry_pick() is taken from the\nsequencer insn_pick_act() function.\n\nMentored-by: Daniel Barkalow <barkalow@iabervon.org>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n builtin-sequencer--helper.c |   71 +++++++++++++++++++++++++++++++++++++------\n 1 files changed, 61 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c\nindex 61a8f2e..291ba18 100644\n--- a/builtin-sequencer--helper.c\n+++ b/builtin-sequencer--helper.c\n@@ -60,7 +60,8 @@ static const char * const git_sequencer_helper_usage[] = {\n \t\t\"<verbosity> [<allow-dirty>]\",\n \t\"git sequencer--helper --fast-forward <commit> <reflog-msg> \"\n \t\t\"<verbosity> [<allow-dirty>]\",\n-\t\"git sequencer--helper --cherry-pick <commit> [<do-not-commit>]\",\n+\t\"git sequencer--helper --cherry-pick <commit> <reflog-msg> \"\n+\t\t\"<verbosity> [<do-not-commit>]\",\n \tNULL\n };\n \n@@ -288,7 +289,7 @@ static int do_commit(unsigned char *parent_sha1)\n \n \tif (update_ref(reflog, \"HEAD\", commit_sha1, NULL, 0, 0))\n \t\treturn error(\"Could not update HEAD to %s.\",\n-\t\t\t\t\t\tsha1_to_hex(commit_sha1));\n+\t\t\t     sha1_to_hex(commit_sha1));\n \n \treturn 0;\n }\n@@ -408,6 +409,46 @@ static int write_commit_summary_into(const char *filename)\n \treturn 0;\n }\n \n+static int do_cherry_pick(char *cp_commit, int no_commit)\n+{\n+\tstruct commit *commit;\n+\tint failed;\n+\tconst char *author;\n+\n+\tif (get_sha1(\"HEAD\", head_sha1))\n+\t\treturn error(\"You do not have a valid HEAD.\");\n+\n+\tcommit = get_commit(cp_commit);\n+\tif (!commit)\n+\t\treturn 1;\n+\n+\tset_pick_subject(cp_commit, commit);\n+\n+\tfailed = pick_commit(commit, 0, 0, &next_commit.summary);\n+\n+\tset_message_source(sha1_to_hex(commit->object.sha1));\n+\tauthor = strstr(commit->buffer, \"\\nauthor \");\n+\tif (author)\n+\t\tset_author_info(author + 8);\n+\n+\t/* We do not want extra Conflicts: lines on cherry-pick,\n+\t   so just take the old commit message. */\n+\tif (failed) {\n+\t\tstrbuf_setlen(&next_commit.summary, 0);\n+\t\tstrbuf_addstr(&next_commit.summary,\n+\t\t\t      strstr(commit->buffer, \"\\n\\n\") + 2);\n+\t\trerere();\n+\t\tmake_patch(commit);\n+\t\twrite_commit_summary_into(MERGE_MSG);\n+\t\treturn error(pick_help_msg(commit->object.sha1, 0));\n+\t}\n+\n+\tif (!no_commit && do_commit(head_sha1))\n+\t\treturn error(\"Could not commit.\");\n+\n+\treturn 0;\n+}\n+\n /**********************************************************************\n  * Builtin sequencer helper functions\n  */\n@@ -436,6 +477,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \tchar *patch_commit = NULL;\n \tchar *reset_commit = NULL;\n \tchar *ff_commit = NULL;\n+\tchar *cp_commit = NULL;\n \tstruct option options[] = {\n \t\tOPT_STRING(0, \"make-patch\", &patch_commit, \"commit\",\n \t\t\t   \"create a patch from commit\"),\n@@ -443,6 +485,8 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\t\t   \"reset to commit\"),\n \t\tOPT_STRING(0, \"fast-forward\", &ff_commit, \"commit\",\n \t\t\t   \"fast forward to commit\"),\n+\t\tOPT_STRING(0, \"cherry-pick\", &cp_commit, \"commit\",\n+\t\t\t   \"cherry pick commit\"),\n \t\tOPT_END()\n \t};\n \n@@ -459,19 +503,15 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\treturn 0;\n \t}\n \n-\tif (ff_commit || reset_commit) {\n+\tif (cp_commit || ff_commit || reset_commit) {\n \t\tunsigned char sha1[20];\n-\t\tchar *commit = ff_commit ? ff_commit : reset_commit;\n+\t\tchar *commit;\n+\t\tint opt_arg = 0;\n \n \t\tif (argc != 2 && argc != 3)\n \t\t\tusage_with_options(git_sequencer_helper_usage,\n \t\t\t\t\t   options);\n \n-\t\tif (get_sha1(commit, sha1)) {\n-\t\t\terror(\"Could not find '%s'\", commit);\n-\t\t\treturn 1;\n-\t\t}\n-\n \t\treflog = (char *)argv[0];\n \n \t\tif (parse_verbosity(argv[1])) {\n@@ -480,7 +520,18 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (argc == 3 && *argv[2] && strcmp(argv[2], \"0\"))\n-\t\t\tallow_dirty = 1;\n+\t\t\topt_arg = 1;\n+\n+\t\tif (cp_commit)\n+\t\t\treturn do_cherry_pick(cp_commit, opt_arg);\n+\n+\t\tallow_dirty = opt_arg;\n+\n+\t\tcommit = ff_commit ? ff_commit : reset_commit;\n+\t\tif (get_sha1(commit, sha1)) {\n+\t\t\terror(\"Could not find '%s'\", commit);\n+\t\t\treturn 1;\n+\t\t}\n \n \t\tif (ff_commit)\n \t\t\treturn do_fast_forward(sha1);\n-- \n1.6.4.271.ge010d\n"},{"id":"121498","messageId":"20090822041616.4261.12763.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"20090822041157.4261.92491.chriscool@tuxfamily.org","subject":"[PATCH v3 9/9] rebase -i: use \"git sequencer--helper --cherry-pick\"","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T04:16:15Z","receivedAt":"2009-08-22T04:16:15Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"instead of \"git cherry-pick\", as this will make it easier to\nport \"git-rebase--interactive.sh\" to C.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n git-rebase--interactive.sh |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 7651fd6..349ca50 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -147,7 +147,8 @@ pick_one () {\n \t\tpick_one_preserving_merges \"$@\" && return\n \tif test ! -z \"$REBASE_ROOT\"\n \tthen\n-\t\toutput git cherry-pick \"$@\"\n+\t\tgit sequencer--helper --cherry-pick $sha1 \\\n+\t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\" $no_ff\n \t\treturn\n \tfi\n \tparent_sha1=$(git rev-parse --verify $sha1^) ||\n@@ -157,7 +158,8 @@ pick_one () {\n \t\tgit sequencer--helper --fast-forward $sha1 \\\n \t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\"\n \telse\n-\t\toutput git cherry-pick \"$@\"\n+\t\tgit sequencer--helper --cherry-pick $sha1 \\\n+\t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\" $no_ff\n \tfi\n }\n \n@@ -269,7 +271,10 @@ pick_one_preserving_merges () {\n \t\t\tfi\n \t\t\t;;\n \t\t*)\n-\t\t\toutput git cherry-pick \"$@\" ||\n+\t\t\tno_commit=\n+\t\t\ttest \"a$1\" = \"a-n\" && no_commit=t\n+\t\t\tgit sequencer--helper --cherry-pick $sha1 \\\n+\t\t\t\t\"$GIT_REFLOG_ACTION\" \"$VERBOSE\" $no_commit ||\n \t\t\t\tdie_with_patch $sha1 \"Could not pick $sha1\"\n \t\t\t;;\n \t\tesac\n-- \n1.6.4.271.ge010d\n"},{"id":"121505","messageId":"7vab1s5mtc.fsf@alter.siamese.dyndns.org","threadId":"20695","inReplyTo":"20090822041616.4261.40442.chriscool@tuxfamily.org","subject":"Re: [PATCH v3 1/9] sequencer: add \"do_fast_forward()\" to perform a fast forward","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-22T07:29:03Z","receivedAt":"2009-08-22T07:29:03Z","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 code is taken from the sequencer GSoC project:\n>\n>     git://repo.or.cz/git/sbeyer.git\n>\n>     (commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)\n>\n> but the messages have been changed to be the same as those\n> displayed by git-rebase--interactive.sh.\n\nHmm, forgot to amend, or perhaps you sent out a wrong series?\n\nThe log message does not explain:\n\n - why the patch adds a new static function that nobody calls;\n - what the new function is good for;\n\nwhich are the most important things in order to defend the change.\n\n\"The messages have been changed to...\" hints that the original commit by\nStephan had different messages produced, perhaps so that it can be used in\na different context.  I hoped, in an ideal world, perhaps Stephan defended\nwhy the change was relevant to his project in some way, and because you\nare using it in a different context that needs modification of the\nmessage, perhaps Stephan's defense of his commit could be reworded to\ndefend your change here.\n\nSo I decided to take a look at the quoted commit to see if I can reword\nthis mess.\n\nBut the quoted commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20 does not\neven seem to be a commit that corresponds to this change.  It is a merge\nfrom upstream.\n\n    commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20\n    Merge: 0c73ae7 99ddd24\n    Author: Stephan Beyer <s-beyer@gmx.net>\n    Date:   Wed May 20 10:54:37 2009 +0200\n\n        Merge branch 'junio/master' into seq-builtin-dev\n\n        After this merge f79d4c8 \"teach git-am to apply a patch to an unborn\n        branch\" has to be reimplemented in sequencer by allowing the \"patch\"\n        insn on unborn branches.\n        The related test in t/t4150-am.sh is set to test_expect_failure.\n\n        Conflicts:\n            git-am.sh\n\nIt does not help that the function that is crucial to the implemention of\nthis new function, reset_almost_hard(), is not explained at all in the\nearlier commit in the previous series (36f692b (sequencer: add\n\"reset_almost_hard()\" and related functions, 2009-08-05).  \n\nThe log message to the commit does not even hint in what sense \"almost\"\nthe function is, iow, in what situation it behaves exactly like \"reset\n--hard\", and in what other situation it doesn't, and more importantly why\nthat distinction is there.  I thought I asked these questions when the\nprevious series was submitted, but I do not remember ever seeing\nsatisfactory answers to them.\n\nI am afraid that the whole cc/sequencer-rebase-i series needs a serious\nreroll before it gets near 'next'.\n\nBefore giving up, I'll quickly re-review how (un)readable the log of each\ncommit is in the series.  The following comments are mostly about the log\nmessages, which are supposed to entice people to review the code, and more\nimportantly, used as part of the release notes to summarize what the newly\nadded toys are about.  If they are horrible, the code has little chance to\nbe even read, and I'll have a hard time merging the series up into a new\nrelease.\n\n\n\n6db6551 sequencer: add \"builtin-sequencer--helper.c\"\n\nGood.\n\nb512803 sequencer: add \"make_patch\" function to save a patch\n\nPassably okay, but the limitation that it always writes into a file with a\nfixed name \"$SEQ_DIR/patch\" should be noted in the log.\n\n0ccc92b sequencer: free memory used in \"make_patch\" function\n\nShould be squashed to the previous.\n\nf121b06 rebase -i: use \"git sequencer--helper --make-patch\"\n\nGood.\n\n36f692b sequencer: add \"reset_almost_hard()\" and related functions\n\nHorrible.  See above.\n\n9d41fbd sequencer: add comments about reset_almost_hard()\n\nShould be squashed to the previous---lift some text to justify the\nexistence of the function in the commit log message.  Even though\nallow_dirty is referred to in the comment as affecting the behaviour, it\nis unclear who sets that global variable using what interface, making the\nreader suspect that maybe it should be a function parameter instead of a\nglobal (but the other parts of the helper may also look at allow_dirty\nand the internal implementation might be--I am just guessing--simpler this\nway, in which case _that_ should be explained and justified).\n\n022a9e7 sequencer: add \"--reset-hard\" option to \"git sequencer--helper\"\n\nThis by itself is Okay, provided if 36f692b were made readable.  Then you\ncan expect the reader to know why reset_almost_hard() needs to be there,\nand you need an interface to that function.  Until then, it is totally\nunclear why you need this, instead of using \"reset --hard\" itself.\n\nad28459 rebase -i: use \"git sequencer--helper --reset-hard\"\n\nDitto.\n\ne4b3f0f sequencer: add \"do_fast_forward()\" to perform a fast forward\n\nSee above.\n\n1d88073 sequencer: add \"--fast-forward\" option to \"git sequencer--helper\"\n\nOkay.\n\n6eff656 sequencer: let \"git sequencer--helper\" callers set \"allow_dirty\"\n\nWhy?  What for?\n\n877ddc1 rebase -i: use \"git sequencer--helper --fast-forward\"\n\nIt is unclear how this relates to the previous one, nor why it is more\nappropriate than \"reset-hard\" it replaces.\n\nff312f0 revert: libify pick\n\nAlmost good.\n\nab67716 pick: libify \"pick_help_msg()\"\n\nGood.\n\nd871b0e sequencer: add \"do_commit()\" and related functions\n\nWe can see from \"git show\" what static functions that are never called in\nthis commit are added, but nobody explains why they are needed.  For\nexample, do_commit() may create a new commit object, but does not share\nthe code with what \"git commit\" and/or \"git commit-tree\" do?  If so, how?\nIf not, why not?\n\nac5fc4d sequencer: add \"--cherry-pick\" option to \"git sequencer--helper\"\n\nPassably okay.  I can see ff312f0 made about a half of cherry-pick\naccessible to the sequencer, and this patch uses it to finish the other\nhalf, although that is not explained in the log message.  Also it is\nunclear why the resulting \"libified\" code does not share more\ninfrastructure with \"git cherry-pick\" itself (and \"git revert\").\n\n664c7ab rebase -i: use \"git sequencer--helper --cherry-pick\"\n\nPassably okay, even though it is not quite convincing why using\nsequencer-helper's cherry-pick option makes it easier to later port the\nscript, than keeping calls to cherry-pick.\n"},{"id":"121511","messageId":"200908221319.37737.chriscool@tuxfamily.org","threadId":"20695","inReplyTo":"7vab1s5mtc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 1/9] sequencer: add \"do_fast_forward()\" to perform a fast forward","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-08-22T11:19:37Z","receivedAt":"2009-08-22T11:19:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Saturday 22 August 2009, Junio C Hamano wrote:\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > From: Stephan Beyer <s-beyer@gmx.net>\n> >\n> > This code is taken from the sequencer GSoC project:\n> >\n> >     git://repo.or.cz/git/sbeyer.git\n> >\n> >     (commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)\n> >\n> > but the messages have been changed to be the same as those\n> > displayed by git-rebase--interactive.sh.\n>\n> Hmm, forgot to amend, or perhaps you sent out a wrong series?\n\nYour comments on the v2 series were in a message replying to patch 5/9, so I \namended only 5/9 and after it, because I thought that you had already \nreviewed those before 5/9 and they were ok.\n\n> The log message does not explain:\n>\n>  - why the patch adds a new static function that nobody calls;\n>  - what the new function is good for;\n>\n> which are the most important things in order to defend the change.\n\nYes, but the function contains only 5 lines of code and it mostly only calls \nreset_almost_hard() that is already documented by a big comment before its \ndefinition.\n\n> \"The messages have been changed to...\" hints that the original commit by\n> Stephan had different messages produced, perhaps so that it can be used\n> in a different context.  I hoped, in an ideal world, perhaps Stephan\n> defended why the change was relevant to his project in some way, and\n> because you are using it in a different context that needs modification\n> of the message, perhaps Stephan's defense of his commit could be reworded\n> to defend your change here.\n>\n> So I decided to take a look at the quoted commit to see if I can reword\n> this mess.\n>\n> But the quoted commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20 does not\n> even seem to be a commit that corresponds to this change.  It is a merge\n> from upstream.\n>\n>     commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20\n>     Merge: 0c73ae7 99ddd24\n>     Author: Stephan Beyer <s-beyer@gmx.net>\n>     Date:   Wed May 20 10:54:37 2009 +0200\n>\n>         Merge branch 'junio/master' into seq-builtin-dev\n>\n>         After this merge f79d4c8 \"teach git-am to apply a patch to an\n> unborn branch\" has to be reimplemented in sequencer by allowing the\n> \"patch\" insn on unborn branches.\n>         The related test in t/t4150-am.sh is set to test_expect_failure.\n>\n>         Conflicts:\n>             git-am.sh\n\nThis is the commit at the point where I took the code of the function, not \nthe commit that introduce the function. This is because some functions like \npick() in patch 5/9 evolve a lot in the sequencer repo after they are \nintroduced.\n\n> It does not help that the function that is crucial to the implemention of\n> this new function, reset_almost_hard(), is not explained at all in the\n> earlier commit in the previous series (36f692b (sequencer: add\n> \"reset_almost_hard()\" and related functions, 2009-08-05).\n>\n> The log message to the commit does not even hint in what sense \"almost\"\n> the function is, iow, in what situation it behaves exactly like \"reset\n> --hard\", and in what other situation it doesn't, and more importantly why\n> that distinction is there.  I thought I asked these questions when the\n> previous series was submitted, but I do not remember ever seeing\n> satisfactory answers to them.\n\nYou asked questions about reset_almost_hard() and I added the big comment \nbefore its definition, see:\n\nhttp://git.kernel.org/?p=git/git.git;a=commitdiff;h=9d41fbd28f1ba3d07cd2e0f547521f9dced4cfd2\n\nAs you merged the series in pu, I thought that it was ok.\n\n> I am afraid that the whole cc/sequencer-rebase-i series needs a serious\n> reroll before it gets near 'next'.\n\nOk, I will reroll everything to try to improve commit messages.\n\n> Before giving up, I'll quickly re-review how (un)readable the log of each\n> commit is in the series.  The following comments are mostly about the log\n> messages, which are supposed to entice people to review the code, and\n> more importantly, used as part of the release notes to summarize what the\n> newly added toys are about.  If they are horrible, the code has little\n> chance to be even read, and I'll have a hard time merging the series up\n> into a new release.\n\nAbout the release notes, as \"git sequencer--helper\" is not for public use, \nand as many functions like reset_almost_hard() are static, perhaps it would \nbe ok to have only something like:\n\n(developers)\n\n- parts of \"git rebase -i\" have been ported to C using \"git \nsequencer--helper\"\n\n- cherry-pick and revert functionality is available using new functions \ndeclared in \"pick.h\"\n\n> 6db6551 sequencer: add \"builtin-sequencer--helper.c\"\n>\n> Good.\n>\n> b512803 sequencer: add \"make_patch\" function to save a patch\n>\n> Passably okay, but the limitation that it always writes into a file with\n> a fixed name \"$SEQ_DIR/patch\" should be noted in the log.\n\nOk.\n\n> 0ccc92b sequencer: free memory used in \"make_patch\" function\n>\n> Should be squashed to the previous.\n\nWill do.\n\n> f121b06 rebase -i: use \"git sequencer--helper --make-patch\"\n>\n> Good.\n>\n> 36f692b sequencer: add \"reset_almost_hard()\" and related functions\n>\n> Horrible.  See above.\n>\n> 9d41fbd sequencer: add comments about reset_almost_hard()\n>\n> Should be squashed to the previous---lift some text to justify the\n> existence of the function in the commit log message.\n\nWill do.\n\n> Even though \n> allow_dirty is referred to in the comment as affecting the behaviour, it\n> is unclear who sets that global variable using what interface, making the\n> reader suspect that maybe it should be a function parameter instead of a\n> global (but the other parts of the helper may also look at allow_dirty\n> and the internal implementation might be--I am just guessing--simpler\n> this way, in which case _that_ should be explained and justified).\n\nWill have a look.\n\n> 022a9e7 sequencer: add \"--reset-hard\" option to \"git sequencer--helper\"\n>\n> This by itself is Okay, provided if 36f692b were made readable.  Then you\n> can expect the reader to know why reset_almost_hard() needs to be there,\n> and you need an interface to that function.  Until then, it is totally\n> unclear why you need this, instead of using \"reset --hard\" itself.\n\nOk.\n\n> ad28459 rebase -i: use \"git sequencer--helper --reset-hard\"\n>\n> Ditto.\n>\n> e4b3f0f sequencer: add \"do_fast_forward()\" to perform a fast forward\n>\n> See above.\n>\n> 1d88073 sequencer: add \"--fast-forward\" option to \"git sequencer--helper\"\n>\n> Okay.\n>\n> 6eff656 sequencer: let \"git sequencer--helper\" callers set \"allow_dirty\"\n>\n> Why?  What for?\n\nDaniel asked me to make it available so people interested can test. I will \nstate this in the commit message.\n\n> 877ddc1 rebase -i: use \"git sequencer--helper --fast-forward\"\n>\n> It is unclear how this relates to the previous one, nor why it is more\n> appropriate than \"reset-hard\" it replaces.\n\nIt is more appropriate because it makes the rebase -i shell code a little \nbit shorter. I will add that to the commit message.\n\n> ff312f0 revert: libify pick\n>\n> Almost good.\n\nWill try to improve.\n\n> ab67716 pick: libify \"pick_help_msg()\"\n>\n> Good.\n>\n> d871b0e sequencer: add \"do_commit()\" and related functions\n>\n> We can see from \"git show\" what static functions that are never called in\n> this commit are added, but nobody explains why they are needed.  For\n> example, do_commit() may create a new commit object, but does not share\n> the code with what \"git commit\" and/or \"git commit-tree\" do?  If so, how?\n> If not, why not?\n\nWill have a look.\n\n> ac5fc4d sequencer: add \"--cherry-pick\" option to \"git sequencer--helper\"\n>\n> Passably okay.  I can see ff312f0 made about a half of cherry-pick\n> accessible to the sequencer, and this patch uses it to finish the other\n> half, although that is not explained in the log message.  Also it is\n> unclear why the resulting \"libified\" code does not share more\n> infrastructure with \"git cherry-pick\" itself (and \"git revert\").\n\nWill have a look.\n\n> 664c7ab rebase -i: use \"git sequencer--helper --cherry-pick\"\n>\n> Passably okay, even though it is not quite convincing why using\n> sequencer-helper's cherry-pick option makes it easier to later port the\n> script, than keeping calls to cherry-pick.\n\nWill have a look.\n\nThanks,\nChristian.\n"},{"id":"121523","messageId":"7viqgf38rd.fsf@alter.siamese.dyndns.org","threadId":"20695","inReplyTo":"200908221319.37737.chriscool@tuxfamily.org","subject":"Re: [PATCH v3 1/9] sequencer: add \"do_fast_forward()\" to perform a fast forward","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-22T20:15:34Z","receivedAt":"2009-08-22T20:15:34Z","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>> Hmm, forgot to amend, or perhaps you sent out a wrong series?\n>\n> Your comments on the v2 series were in a message replying to patch 5/9, so I \n> amended only 5/9 and after it, because I thought that you had already \n> reviewed those before 5/9 and they were ok.\n\nSorry, but in general I expect readers of this list to be intelligent\nenough and capable of extrapolating, so that I do not have to spell out\nthe same issue over and over again ;-)\n\n> As you merged the series in pu, I thought that it was ok.\n\nHeh, being queued to 'pu' does not mean any more than just that we do not\nwant to lose what the author spent efforts to produce, and we want to give\nreviewers easier access to the end result.  If it is Ok, it would have\nbeen in 'next'.\n\nThat's why topics initially queued in 'pu' sometimes move to 'stalled'\nstate in \"What's in\" report and then eventually get ejected.\n\nThe comment you added only says what it does, namely, that it does two\ndifferent things, based on the allow_dirty global variable that is\nunderdocumented.  It does not explain _why_ it is a good thing, in what\ncircumstance the caller would want \"almost\" semantics and in what other\ncircumstance the caller would want a \"real\" hard reset.\n\n>> I am afraid that the whole cc/sequencer-rebase-i series needs a serious\n>> reroll before it gets near 'next'.\n>\n> Ok, I will reroll everything to try to improve commit messages.\n\nIt would be a good idea; it may expose a wrong abstraction or an\ninsufficient refactoring (e.g. adding yet another function to make a\ncommit object that runs parallel to what we already have).\n\nThanks.\n"}]}