[PATCH v2 0/9] more changes to port rebase -i to C using sequencer code

[PATCH v2 0/9] more changes to port rebase -i to C using sequencer code

[Christian Couder]

This is a reroll of the series I sent 9 days ago.

Patch 5/9 (revert: libify pick) now contains some fixups suggested by
Junio, so it's based on a newer commit in the sequencer repo. 

I also updated the commit message of some following patches so they
display the same commit sha1 from the sequencer repo.

Patch 8/9 has been reworked so that the --cherry-pick option use the
same arguments as other options.

Christian Couder (5):
  sequencer: add "--fast-forward" option to "git sequencer--helper"
  sequencer: let "git sequencer--helper" callers set "allow_dirty"
  rebase -i: use "git sequencer--helper --fast-forward"
  pick: libify "pick_help_msg()"
  rebase -i: use "git sequencer--helper --cherry-pick"

Stephan Beyer (4):
  sequencer: add "do_fast_forward()" to perform a fast forward
  revert: libify pick
  sequencer: add "do_commit()" and related functions
  sequencer: add "--cherry-pick" option to "git sequencer--helper"

 Makefile                    |    2 +
 builtin-revert.c            |  293 ++++++-----------------------------------
 builtin-sequencer--helper.c |  305 +++++++++++++++++++++++++++++++++++++++++--
 git-rebase--interactive.sh  |   22 ++--
 pick.c                      |  232 ++++++++++++++++++++++++++++++++
 pick.h                      |   14 ++
 6 files changed, 599 insertions(+), 269 deletions(-)
 create mode 100644 pick.c
 create mode 100644 pick.h

[PATCH v2 1/9] sequencer: add "do_fast_forward()" to perform a fast forward

[Christian Couder]

From: Stephan Beyer <s-beyer@gmx.net>

This code is taken from the sequencer GSoC project:

    git://repo.or.cz/git/sbeyer.git

    (commit e7b8dab0c2a73ade92017a52bb1405ea1534ef20)

but the messages have been changed to be the same as those
displayed by git-rebase--interactive.sh.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index be030bc..0cd7e98 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -171,6 +171,15 @@ static struct commit *get_commit(const char *arg)
 	return lookup_commit_reference(sha1);
 }
 
+static int do_fast_forward(const unsigned char *sha)
+{
+	if (reset_almost_hard(sha))
+		return error("Cannot fast forward to %s", sha1_to_hex(sha));
+	if (verbosity > 1)
+		printf("Fast forward to %s\n", sha1_to_hex(sha));
+	return 0;
+}
+
 static int set_verbosity(int verbose)
 {
 	char tmp[] = "0";
-- 
1.6.4.271.ge010d

[PATCH v2 2/9] sequencer: add "--fast-forward" option to "git sequencer--helper"

[Christian Couder]

This new option uses the "do_fast_forward()" function to perform
a fast forward.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 0cd7e98..bd72f65 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -19,6 +19,7 @@ static unsigned char head_sha1[20];
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
 	"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>",
+	"git sequencer--helper --fast-forward <commit> <reflog-msg> <verbosity>",
 	NULL
 };
 
@@ -218,11 +219,14 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 {
 	char *patch_commit = NULL;
 	char *reset_commit = NULL;
+	char *ff_commit = NULL;
 	struct option options[] = {
 		OPT_STRING(0, "make-patch", &patch_commit, "commit",
 			   "create a patch from commit"),
 		OPT_STRING(0, "reset-hard", &reset_commit, "commit",
 			   "reset to commit"),
+		OPT_STRING(0, "fast-forward", &ff_commit, "commit",
+			   "fast forward to commit"),
 		OPT_END()
 	};
 
@@ -239,15 +243,16 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 		return 0;
 	}
 
-	if (reset_commit) {
+	if (ff_commit || reset_commit) {
 		unsigned char sha1[20];
+		char *commit = ff_commit ? ff_commit : reset_commit;
 
 		if (argc != 2)
 			usage_with_options(git_sequencer_helper_usage,
 					   options);
 
-		if (get_sha1(reset_commit, sha1)) {
-			error("Could not find '%s'", reset_commit);
+		if (get_sha1(commit, sha1)) {
+			error("Could not find '%s'", commit);
 			return 1;
 		}
 
@@ -258,7 +263,10 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			return 1;
 		}
 
-		return reset_almost_hard(sha1);
+		if (ff_commit)
+			return do_fast_forward(sha1);
+		else
+			return reset_almost_hard(sha1);
 	}
 
 	usage_with_options(git_sequencer_helper_usage, options);
-- 
1.6.4.271.ge010d

[PATCH v2 3/9] sequencer: let "git sequencer--helper" callers set "allow_dirty"

[Christian Couder]

This flag can be set when using --reset-hard or --fast-forward, and
in this case changes in the work tree will be kept.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index bd72f65..71a7fef 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -18,8 +18,10 @@ static unsigned char head_sha1[20];
 
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
-	"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>",
-	"git sequencer--helper --fast-forward <commit> <reflog-msg> <verbosity>",
+	"git sequencer--helper --reset-hard <commit> <reflog-msg> "
+		"<verbosity> [<allow-dirty>]",
+	"git sequencer--helper --fast-forward <commit> <reflog-msg> "
+		"<verbosity> [<allow-dirty>]",
 	NULL
 };
 
@@ -247,7 +249,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 		unsigned char sha1[20];
 		char *commit = ff_commit ? ff_commit : reset_commit;
 
-		if (argc != 2)
+		if (argc != 2 && argc != 3)
 			usage_with_options(git_sequencer_helper_usage,
 					   options);
 
@@ -263,6 +265,9 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			return 1;
 		}
 
+		if (argc == 3 && *argv[2] && strcmp(argv[2], "0"))
+			allow_dirty = 1;
+
 		if (ff_commit)
 			return do_fast_forward(sha1);
 		else
-- 
1.6.4.271.ge010d

[PATCH v2 4/9] rebase -i: use "git sequencer--helper --fast-forward"

[Christian Couder]

when fast forwarding.

Note that in the first hunk of this patch, there was this line:

test "a$1" = a-n && output git reset --soft $current_sha1

but the test always failed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-rebase--interactive.sh |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0041994..7651fd6 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -154,11 +154,8 @@ pick_one () {
 		die "Could not get the parent of $sha1"
 	current_sha1=$(git rev-parse --verify HEAD)
 	if test "$no_ff$current_sha1" = "$parent_sha1"; then
-		git sequencer--helper --reset-hard $sha1 \
+		git sequencer--helper --fast-forward $sha1 \
 			"$GIT_REFLOG_ACTION" "$VERBOSE"
-		test "a$1" = a-n && output git reset --soft $current_sha1
-		sha1=$(git rev-parse --short $sha1)
-		output warn Fast forward to $sha1
 	else
 		output git cherry-pick "$@"
 	fi
@@ -238,10 +235,8 @@ pick_one_preserving_merges () {
 	done
 	case $fast_forward in
 	t)
-		output warn "Fast forward to $sha1"
-		git sequencer--helper --reset-hard $sha1 \
-			"$GIT_REFLOG_ACTION" "$VERBOSE" ||
-			die "Cannot fast forward to $sha1"
+		git sequencer--helper --fast-forward $sha1 \
+			"$GIT_REFLOG_ACTION" "$VERBOSE" || exit
 		;;
 	f)
 		first_parent=$(expr "$new_parents" : ' \([^ ]*\)')
-- 
1.6.4.271.ge010d

[PATCH v2 5/9] revert: libify pick

[Christian Couder]

From: Stephan Beyer <s-beyer@gmx.net>

This commit is made of code from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

The goal of this commit is to abstract out pick functionnality
into a new pick() function made of code from "builtin-revert.c".

The new pick() function is in a new "pick.c" file with an
associated "pick.h".

This commit contains some changes suggested by Junio.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile         |    2 +
 builtin-revert.c |  272 +++++++++---------------------------------------------
 pick.c           |  210 +++++++++++++++++++++++++++++++++++++++++
 pick.h           |   13 +++
 4 files changed, 269 insertions(+), 228 deletions(-)
 create mode 100644 pick.c
 create mode 100644 pick.h

diff --git a/Makefile b/Makefile
index 98dd01d..5e76083 100644
--- a/Makefile
+++ b/Makefile
@@ -445,6 +445,7 @@ LIB_H += pack-refs.h
 LIB_H += pack-revindex.h
 LIB_H += parse-options.h
 LIB_H += patch-ids.h
+LIB_H += pick.h
 LIB_H += pkt-line.h
 LIB_H += progress.h
 LIB_H += quote.h
@@ -533,6 +534,7 @@ LIB_OBJS += parse-options.o
 LIB_OBJS += patch-delta.o
 LIB_OBJS += patch-ids.o
 LIB_OBJS += path.o
+LIB_OBJS += pick.o
 LIB_OBJS += pkt-line.o
 LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
diff --git a/builtin-revert.c b/builtin-revert.c
index 151aa6a..4797ac5 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -1,18 +1,14 @@
 #include "cache.h"
 #include "builtin.h"
-#include "object.h"
 #include "commit.h"
 #include "tag.h"
-#include "wt-status.h"
-#include "run-command.h"
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "parse-options.h"
-#include "cache-tree.h"
 #include "diff.h"
 #include "revision.h"
 #include "rerere.h"
-#include "merge-recursive.h"
+#include "pick.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -35,25 +31,23 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff;
-static enum { REVERT, CHERRY_PICK } action;
+static int edit, no_commit, mainline, signoff;
+static int flags;
 static struct commit *commit;
 
-static const char *me;
-
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =
-		action == REVERT ?  revert_usage : cherry_pick_usage;
+		flags & PICK_REVERSE ? revert_usage : cherry_pick_usage;
 	unsigned char sha1[20];
 	const char *arg;
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
 		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
-		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry-picking"),
+		OPT_BIT('x', NULL, &flags, "append commit name when cherry-picking", PICK_ADD_NOTE),
 		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
@@ -77,42 +71,12 @@ static void parse_args(int argc, const char **argv)
 		die ("'%s' does not point to a commit", arg);
 }
 
-static char *get_oneline(const char *message)
-{
-	char *result;
-	const char *p = message, *abbrev, *eol;
-	int abbrev_len, oneline_len;
-
-	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (*p) {
-		p += 2;
-		for (eol = p + 1; *eol && *eol != '\n'; eol++)
-			; /* do nothing */
-	} else
-		eol = p;
-	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
-	abbrev_len = strlen(abbrev);
-	oneline_len = eol - p;
-	result = xmalloc(abbrev_len + 5 + oneline_len);
-	memcpy(result, abbrev, abbrev_len);
-	memcpy(result + abbrev_len, "... ", 4);
-	memcpy(result + abbrev_len + 4, p, oneline_len);
-	result[abbrev_len + 4 + oneline_len] = '\0';
-	return result;
-}
-
 static char *get_encoding(const char *message)
 {
 	const char *p = message, *eol;
 
 	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
+		return NULL;
 	while (*p && *p != '\n') {
 		for (eol = p + 1; *eol && *eol != '\n'; eol++)
 			; /* do nothing */
@@ -128,30 +92,6 @@ static char *get_encoding(const char *message)
 	return NULL;
 }
 
-static struct lock_file msg_file;
-static int msg_fd;
-
-static void add_to_msg(const char *string)
-{
-	int len = strlen(string);
-	if (write_in_full(msg_fd, string, len) < 0)
-		die_errno ("Could not write to MERGE_MSG");
-}
-
-static void add_message_to_msg(const char *message)
-{
-	const char *p = message;
-	while (*p && (*p != '\n' || p[1] != '\n'))
-		p++;
-
-	if (!*p)
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-	p += 2;
-	add_to_msg(p);
-	return;
-}
-
 static void set_author_ident_env(const char *message)
 {
 	const char *p = message;
@@ -214,7 +154,7 @@ static char *help_msg(const unsigned char *sha1)
 	       "mark the corrected paths with 'git add <paths>' "
 	       "or 'git rm <paths>' and commit the result.");
 
-	if (action == CHERRY_PICK) {
+	if (!(flags & PICK_REVERSE)) {
 		sprintf(helpbuf + strlen(helpbuf),
 			"\nWhen commiting, use the option "
 			"'-c %s' to retain authorship and message.",
@@ -223,187 +163,68 @@ static char *help_msg(const unsigned char *sha1)
 	return helpbuf;
 }
 
-static struct tree *empty_tree(void)
+static void write_message(struct strbuf *msgbuf, const char *filename)
 {
-	struct tree *tree = xcalloc(1, sizeof(struct tree));
-
-	tree->object.parsed = 1;
-	tree->object.type = OBJ_TREE;
-	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
-	return tree;
+	struct lock_file msg_file;
+	int msg_fd;
+	msg_fd = hold_lock_file_for_update(&msg_file, filename,
+					   LOCK_DIE_ON_ERROR);
+	if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
+		die_errno("Could not write to %s.", filename);
+	strbuf_release(msgbuf);
+	if (commit_lock_file(&msg_file) < 0)
+		die("Error wrapping up %s", filename);
 }
 
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
-	unsigned char head[20];
-	struct commit *base, *next, *parent;
-	int i, index_fd, clean;
-	char *oneline, *reencoded_message = NULL;
-	const char *message, *encoding;
-	char *defmsg = git_pathdup("MERGE_MSG");
-	struct merge_options o;
-	struct tree *result, *next_tree, *base_tree, *head_tree;
-	static struct lock_file index_lock;
+	const char *me;
+	struct strbuf msgbuf;
+	char *reencoded_message = NULL;
+	const char *encoding;
+	int failed;
 
 	git_config(git_default_config, NULL);
-	me = action == REVERT ? "revert" : "cherry-pick";
+	me = flags & PICK_REVERSE ? "revert" : "cherry-pick";
 	setenv(GIT_REFLOG_ACTION, me, 0);
 	parse_args(argc, argv);
 
-	/* this is copied from the shell script, but it's never triggered... */
-	if (action == REVERT && !no_replay)
-		die("revert is incompatible with replay");
-
 	if (read_cache() < 0)
 		die("git %s: failed to read the index", me);
-	if (no_commit) {
-		/*
-		 * We do not intend to commit immediately.  We just want to
-		 * merge the differences in, so let's compute the tree
-		 * that represents the "current" state for merge-recursive
-		 * to work on.
-		 */
-		if (write_cache_as_tree(head, 0, NULL))
-			die ("Your index file is unmerged.");
-	} else {
-		if (get_sha1("HEAD", head))
-			die ("You do not have a valid HEAD");
-		if (index_differs_from("HEAD", 0))
-			die ("Dirty index: cannot %s", me);
-	}
-	discard_cache();
-
-	index_fd = hold_locked_index(&index_lock, 1);
+	if (!no_commit && index_differs_from("HEAD", 0))
+		die ("Dirty index: cannot %s", me);
 
-	if (!commit->parents) {
-		if (action == REVERT)
-			die ("Cannot revert a root commit");
-		parent = NULL;
-	}
-	else if (commit->parents->next) {
-		/* Reverting or cherry-picking a merge commit */
-		int cnt;
-		struct commit_list *p;
-
-		if (!mainline)
-			die("Commit %s is a merge but no -m option was given.",
-			    sha1_to_hex(commit->object.sha1));
-
-		for (cnt = 1, p = commit->parents;
-		     cnt != mainline && p;
-		     cnt++)
-			p = p->next;
-		if (cnt != mainline || !p)
-			die("Commit %s does not have parent %d",
-			    sha1_to_hex(commit->object.sha1), mainline);
-		parent = p->item;
-	} else if (0 < mainline)
-		die("Mainline was specified but commit %s is not a merge.",
-		    sha1_to_hex(commit->object.sha1));
-	else
-		parent = commit->parents->item;
-
-	if (!(message = commit->buffer))
-		die ("Cannot get commit message for %s",
+	if (!commit->buffer)
+		return error("Cannot get commit message for %s",
 				sha1_to_hex(commit->object.sha1));
-
-	if (parent && parse_commit(parent) < 0)
-		die("%s: cannot parse parent commit %s",
-		    me, sha1_to_hex(parent->object.sha1));
-
-	/*
-	 * "commit" is an existing commit.  We would want to apply
-	 * the difference it introduces since its first parent "prev"
-	 * on top of the current HEAD if we are cherry-pick.  Or the
-	 * reverse of it if we are revert.
-	 */
-
-	msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
-					   LOCK_DIE_ON_ERROR);
-
-	encoding = get_encoding(message);
+	encoding = get_encoding(commit->buffer);
 	if (!encoding)
 		encoding = "UTF-8";
 	if (!git_commit_encoding)
 		git_commit_encoding = "UTF-8";
-	if ((reencoded_message = reencode_string(message,
+	if ((reencoded_message = reencode_string(commit->buffer,
 					git_commit_encoding, encoding)))
-		message = reencoded_message;
-
-	oneline = get_oneline(message);
-
-	if (action == REVERT) {
-		char *oneline_body = strchr(oneline, ' ');
+		commit->buffer = reencoded_message;
 
-		base = commit;
-		next = parent;
-		add_to_msg("Revert \"");
-		add_to_msg(oneline_body + 1);
-		add_to_msg("\"\n\nThis reverts commit ");
-		add_to_msg(sha1_to_hex(commit->object.sha1));
-
-		if (commit->parents->next) {
-			add_to_msg(", reversing\nchanges made to ");
-			add_to_msg(sha1_to_hex(parent->object.sha1));
-		}
-		add_to_msg(".\n");
-	} else {
-		base = parent;
-		next = commit;
-		set_author_ident_env(message);
-		add_message_to_msg(message);
-		if (no_replay) {
-			add_to_msg("(cherry picked from commit ");
-			add_to_msg(sha1_to_hex(commit->object.sha1));
-			add_to_msg(")\n");
-		}
-	}
-
-	read_cache();
-	init_merge_options(&o);
-	o.branch1 = "HEAD";
-	o.branch2 = oneline;
-
-	head_tree = parse_tree_indirect(head);
-	next_tree = next ? next->tree : empty_tree();
-	base_tree = base ? base->tree : empty_tree();
-
-	clean = merge_trees(&o,
-			    head_tree,
-			    next_tree, base_tree, &result);
-
-	if (active_cache_changed &&
-	    (write_cache(index_fd, active_cache, active_nr) ||
-	     commit_locked_index(&index_lock)))
-		die("%s: Unable to write new index file", me);
-	rollback_lock_file(&index_lock);
-
-	if (!clean) {
-		add_to_msg("\nConflicts:\n\n");
-		for (i = 0; i < active_nr;) {
-			struct cache_entry *ce = active_cache[i++];
-			if (ce_stage(ce)) {
-				add_to_msg("\t");
-				add_to_msg(ce->name);
-				add_to_msg("\n");
-				while (i < active_nr && !strcmp(ce->name,
-						active_cache[i]->name))
-					i++;
-			}
-		}
-		if (commit_lock_file(&msg_file) < 0)
-			die ("Error wrapping up %s", defmsg);
+	failed = pick_commit(commit, mainline, flags, &msgbuf);
+	if (failed < 0) {
+		exit(1);
+	} else if (failed > 0) {
 		fprintf(stderr, "Automatic %s failed.%s\n",
 			me, help_msg(commit->object.sha1));
+		write_message(&msgbuf, git_path("MERGE_MSG"));
 		rerere();
 		exit(1);
 	}
-	if (commit_lock_file(&msg_file) < 0)
-		die ("Error wrapping up %s", defmsg);
+	if (!(flags & PICK_REVERSE))
+		set_author_ident_env(commit->buffer);
+	free(reencoded_message);
+
 	fprintf(stderr, "Finished one %s.\n", me);
 
+	write_message(&msgbuf, git_path("MERGE_MSG"));
+
 	/*
-	 *
 	 * If we are cherry-pick, and if the merge did not result in
 	 * hand-editing, we will hit this commit and inherit the original
 	 * author date and name.
@@ -421,14 +242,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			args[i++] = "-s";
 		if (!edit) {
 			args[i++] = "-F";
-			args[i++] = defmsg;
+			args[i++] = git_path("MERGE_MSG");
 		}
 		args[i] = NULL;
 		return execv_git_cmd(args);
 	}
-	free(reencoded_message);
-	free(defmsg);
-
 	return 0;
 }
 
@@ -436,14 +254,12 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	if (isatty(0))
 		edit = 1;
-	no_replay = 1;
-	action = REVERT;
+	flags = PICK_REVERSE | PICK_ADD_NOTE;
 	return revert_or_cherry_pick(argc, argv);
 }
 
 int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 {
-	no_replay = 0;
-	action = CHERRY_PICK;
+	flags = 0;
 	return revert_or_cherry_pick(argc, argv);
 }
diff --git a/pick.c b/pick.c
new file mode 100644
index 0000000..058b877
--- /dev/null
+++ b/pick.c
@@ -0,0 +1,210 @@
+#include "cache.h"
+#include "commit.h"
+#include "run-command.h"
+#include "cache-tree.h"
+#include "pick.h"
+#include "merge-recursive.h"
+
+static struct commit *commit;
+
+static char *get_oneline(const char *message)
+{
+	char *result;
+	const char *p = message, *abbrev, *eol;
+	int abbrev_len, oneline_len;
+
+	if (!p)
+		return NULL;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (*p) {
+		p += 2;
+		for (eol = p + 1; *eol && *eol != '\n'; eol++)
+			; /* do nothing */
+	} else
+		eol = p;
+	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+	abbrev_len = strlen(abbrev);
+	oneline_len = eol - p;
+	result = xmalloc(abbrev_len + 5 + oneline_len);
+	memcpy(result, abbrev, abbrev_len);
+	memcpy(result + abbrev_len, "... ", 4);
+	memcpy(result + abbrev_len + 4, p, oneline_len);
+	result[abbrev_len + 4 + oneline_len] = '\0';
+	return result;
+}
+
+static void add_message_to_msg(struct strbuf *msg, const char *message)
+{
+	const char *p = message;
+	while (*p && (*p != '\n' || p[1] != '\n'))
+		p++;
+
+	if (!*p)
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+	p += 2;
+	strbuf_addstr(msg, p);
+	return;
+}
+
+static struct tree *empty_tree(void)
+{
+	struct tree *tree = xcalloc(1, sizeof(struct tree));
+
+	tree->object.parsed = 1;
+	tree->object.type = OBJ_TREE;
+	pretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);
+	return tree;
+}
+
+/*
+ * Pick changes introduced by "commit" argument into current working
+ * tree and index.
+ *
+ * Return 0 on success.
+ * Return negative value on error before picking,
+ * and a positive value after picking,
+ * and return 1 if and only if a conflict occurs but no other error.
+ */
+int pick_commit(struct commit *pick_commit, int mainline, int flags,
+		struct strbuf *msg)
+{
+	unsigned char head[20];
+	struct commit *base, *next, *parent;
+	int i, index_fd, clean;
+	int ret = 0;
+	char *oneline;
+	const char *message;
+	struct merge_options o;
+	struct tree *result, *next_tree, *base_tree, *head_tree;
+	static struct lock_file index_lock;
+
+	strbuf_init(msg, 0);
+	commit = pick_commit;
+
+	/*
+	 * Let's compute the tree that represents the "current" state
+	 * for merge-recursive to work on.
+	 */
+	if (write_cache_as_tree(head, 0, NULL))
+		return error("Your index file is unmerged.");
+	discard_cache();
+
+	index_fd = hold_locked_index(&index_lock, 0);
+	if (index_fd < 0)
+		return error("Unable to create locked index: %s",
+			     strerror(errno));
+
+	if (!commit->parents) {
+		if (flags & PICK_REVERSE)
+			return error("Cannot revert a root commit");
+		parent = NULL;
+	}
+	else if (commit->parents->next) {
+		/* Reverting or cherry-picking a merge commit */
+		int cnt;
+		struct commit_list *p;
+
+		if (!mainline)
+			return error("Commit %s is a merge but no mainline was given.",
+				     sha1_to_hex(commit->object.sha1));
+
+		for (cnt = 1, p = commit->parents;
+		     cnt != mainline && p;
+		     cnt++)
+			p = p->next;
+		if (cnt != mainline || !p)
+			return error("Commit %s does not have parent %d",
+				     sha1_to_hex(commit->object.sha1),
+				     mainline);
+		parent = p->item;
+	} else if (0 < mainline)
+		return error("Mainline was specified but commit %s is not a merge.",
+			     sha1_to_hex(commit->object.sha1));
+	else
+		parent = commit->parents->item;
+
+	if (!(message = commit->buffer))
+		return error("Cannot get commit message for %s",
+			     sha1_to_hex(commit->object.sha1));
+
+	if (parent && parse_commit(parent) < 0)
+		return error("Cannot parse parent commit %s",
+			     sha1_to_hex(parent->object.sha1));
+
+	oneline = get_oneline(message);
+
+	if (flags & PICK_REVERSE) {
+		char *oneline_body = strchr(oneline, ' ');
+
+		base = commit;
+		next = parent;
+		strbuf_addstr(msg, "Revert \"");
+		strbuf_addstr(msg, oneline_body + 1);
+		strbuf_addstr(msg, "\"\n\nThis reverts commit ");
+		strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+
+		if (commit->parents->next) {
+			strbuf_addstr(msg, ", reversing\nchanges made to ");
+			strbuf_addstr(msg, sha1_to_hex(parent->object.sha1));
+		}
+		strbuf_addstr(msg, ".\n");
+	} else {
+		base = parent;
+		next = commit;
+		add_message_to_msg(msg, message);
+		if (flags & PICK_ADD_NOTE) {
+			strbuf_addstr(msg, "(cherry picked from commit ");
+			strbuf_addstr(msg, sha1_to_hex(commit->object.sha1));
+			strbuf_addstr(msg, ")\n");
+		}
+	}
+
+	read_cache();
+	init_merge_options(&o);
+	o.branch1 = "HEAD";
+	o.branch2 = oneline;
+
+	head_tree = parse_tree_indirect(head);
+	next_tree = next ? next->tree : empty_tree();
+	base_tree = base ? base->tree : empty_tree();
+
+	clean = merge_trees(&o,
+			    head_tree,
+			    next_tree, base_tree, &result);
+
+	if (active_cache_changed &&
+	    (write_cache(index_fd, active_cache, active_nr) ||
+	     commit_locked_index(&index_lock))) {
+		error("Unable to write new index file");
+		return 2;
+	}
+	rollback_lock_file(&index_lock);
+
+	if (!clean) {
+		strbuf_addstr(msg, "\nConflicts:\n\n");
+		for (i = 0; i < active_nr;) {
+			struct cache_entry *ce = active_cache[i++];
+			if (ce_stage(ce)) {
+				strbuf_addstr(msg, "\t");
+				strbuf_addstr(msg, ce->name);
+				strbuf_addstr(msg, "\n");
+				while (i < active_nr && !strcmp(ce->name,
+						active_cache[i]->name))
+					i++;
+			}
+		}
+		ret = 1;
+	}
+	free(oneline);
+
+	discard_cache();
+	if (read_cache() < 0) {
+		error("Cannot read the index");
+		return 2;
+	}
+
+	return ret;
+}
diff --git a/pick.h b/pick.h
new file mode 100644
index 0000000..7a74ad8
--- /dev/null
+++ b/pick.h
@@ -0,0 +1,13 @@
+#ifndef PICK_H
+#define PICK_H
+
+#include "commit.h"
+
+/* Pick flags: */
+#define PICK_REVERSE   1 /* pick the reverse changes ("revert") */
+#define PICK_ADD_NOTE  2 /* add note about original commit (unless conflict) */
+/* We don't need a PICK_QUIET. This is done by
+ *	setenv("GIT_MERGE_VERBOSITY", "0", 1); */
+extern int pick_commit(struct commit *commit, int mainline, int flags, struct strbuf *msg);
+
+#endif
-- 
1.6.4.271.ge010d

[PATCH v2 6/9] pick: libify "pick_help_msg()"

[Christian Couder]

This function gives an help message when pick or revert failed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-revert.c |   23 +----------------------
 pick.c           |   22 ++++++++++++++++++++++
 pick.h           |    1 +
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/builtin-revert.c b/builtin-revert.c
index 4797ac5..e5250bd 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -142,27 +142,6 @@ static void set_author_ident_env(const char *message)
 			sha1_to_hex(commit->object.sha1));
 }
 
-static char *help_msg(const unsigned char *sha1)
-{
-	static char helpbuf[1024];
-	char *msg = getenv("GIT_CHERRY_PICK_HELP");
-
-	if (msg)
-		return msg;
-
-	strcpy(helpbuf, "  After resolving the conflicts,\n"
-	       "mark the corrected paths with 'git add <paths>' "
-	       "or 'git rm <paths>' and commit the result.");
-
-	if (!(flags & PICK_REVERSE)) {
-		sprintf(helpbuf + strlen(helpbuf),
-			"\nWhen commiting, use the option "
-			"'-c %s' to retain authorship and message.",
-			find_unique_abbrev(sha1, DEFAULT_ABBREV));
-	}
-	return helpbuf;
-}
-
 static void write_message(struct strbuf *msgbuf, const char *filename)
 {
 	struct lock_file msg_file;
@@ -211,7 +190,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		exit(1);
 	} else if (failed > 0) {
 		fprintf(stderr, "Automatic %s failed.%s\n",
-			me, help_msg(commit->object.sha1));
+			me, pick_help_msg(commit->object.sha1, flags));
 		write_message(&msgbuf, git_path("MERGE_MSG"));
 		rerere();
 		exit(1);
diff --git a/pick.c b/pick.c
index 058b877..4f882bb 100644
--- a/pick.c
+++ b/pick.c
@@ -208,3 +208,25 @@ int pick_commit(struct commit *pick_commit, int mainline, int flags,
 
 	return ret;
 }
+
+char *pick_help_msg(const unsigned char *sha1, int flags)
+{
+	static char helpbuf[1024];
+	char *msg = getenv("GIT_CHERRY_PICK_HELP");
+
+	if (msg)
+		return msg;
+
+	strcpy(helpbuf, "  After resolving the conflicts,\n"
+	       "mark the corrected paths with 'git add <paths>' "
+	       "or 'git rm <paths>' and commit the result.");
+
+	if (!(flags & PICK_REVERSE)) {
+		sprintf(helpbuf + strlen(helpbuf),
+			"\nWhen commiting, use the option "
+			"'-c %s' to retain authorship and message.",
+			find_unique_abbrev(sha1, DEFAULT_ABBREV));
+	}
+	return helpbuf;
+}
+
diff --git a/pick.h b/pick.h
index 7a74ad8..115541a 100644
--- a/pick.h
+++ b/pick.h
@@ -9,5 +9,6 @@
 /* We don't need a PICK_QUIET. This is done by
  *	setenv("GIT_MERGE_VERBOSITY", "0", 1); */
 extern int pick_commit(struct commit *commit, int mainline, int flags, struct strbuf *msg);
+extern char *pick_help_msg(const unsigned char *sha1, int flags);
 
 #endif
-- 
1.6.4.271.ge010d

[PATCH v2 7/9] sequencer: add "do_commit()" and related functions

[Christian Couder]

From: Stephan Beyer <s-beyer@gmx.net>

This patch adds some code that comes from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

It adds "struct commit_info", the "next_commit" static variable and
the following functions:

        - do_commit()
        - set_author_info()
        - set_message_source()
        - set_pick_subject()
        - write_commit_summary_into()

This makes it possible to prepare and perform a commit.

Compared to the sequencer project, the only change is that "mark"
related (3 lines long) code has been removed from do_commit().

    Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
    Mentored-by: Christian Couder <chriscool@tuxfamily.org>
    Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
    Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |  214 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 214 insertions(+), 0 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 71a7fef..61a8f2e 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -5,26 +5,69 @@
 #include "refs.h"
 #include "diff.h"
 #include "unpack-trees.h"
+#include "string-list.h"
+#include "pick.h"
+#include "rerere.h"
+#include "dir.h"
+#include "cache-tree.h"
+#include "utf8.h"
 
 #define SEQ_DIR "rebase-merge"
 
 #define PATCH_FILE	git_path(SEQ_DIR "/patch")
+#define MERGE_MSG	git_path("MERGE_MSG")
+#define SQUASH_MSG	git_path("SQUASH_MSG")
+
+/**********************************************************************
+ * Data structures
+ */
+
+struct user_info {
+	const char *name;
+	const char *mail;
+	const char *time; /* "<timestamp> <timezone>" */
+};
+
+struct commit_info {
+	struct user_info author; /* author info */
+	struct user_info committer; /* not used, but for easy extendability */
+	const char *encoding; /* encoding */
+	char *subject; /* basically the first line of the summary */
+	struct strbuf summary; /* the commit message */
+	char *source; /* source of the commit message, either
+		       * "message", "merge", "squash" or a commit SHA1 */
+	char *patch; /* a patch */
+	struct string_list parents; /* list of parents' hex'ed sha1 ids */
+};
+
+/**********************************************************************
+ * Global variables
+ */
 
 static char *reflog;
 
+static int squash_count = 0;
+
 static int allow_dirty = 0, verbosity = 1, advice = 1;
 
 static unsigned char head_sha1[20];
 
+static struct commit_info next_commit;
+
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
 	"git sequencer--helper --reset-hard <commit> <reflog-msg> "
 		"<verbosity> [<allow-dirty>]",
 	"git sequencer--helper --fast-forward <commit> <reflog-msg> "
 		"<verbosity> [<allow-dirty>]",
+	"git sequencer--helper --cherry-pick <commit> [<do-not-commit>]",
 	NULL
 };
 
+/**********************************************************************
+ * Sequencer functions
+ */
+
 static int parse_and_init_tree_desc(const unsigned char *sha1,
 				    struct tree_desc *desc)
 {
@@ -162,6 +205,157 @@ static void make_patch(struct commit *commit)
 	free(args);
 }
 
+/* Commit current index with information next_commit onto parent_sha1. */
+static int do_commit(unsigned char *parent_sha1)
+{
+	int failed;
+	unsigned char tree_sha1[20];
+	unsigned char commit_sha1[20];
+	struct strbuf sbuf;
+	const char *reencoded = NULL;
+
+	if (squash_count) {
+		squash_count = 0;
+		if (file_exists(SQUASH_MSG))
+			unlink(SQUASH_MSG);
+	}
+
+	if (!index_differs_from("HEAD", 0) &&
+	    !next_commit.parents.nr)
+		return error("No changes! Do you really want an empty commit?");
+
+	if (!next_commit.author.name || !next_commit.author.mail)
+		return error("Internal error: Author information not set properly.");
+
+	if (write_cache_as_tree(tree_sha1, 0, NULL))
+		return 1;
+
+	if (!next_commit.encoding)
+		next_commit.encoding = xstrdup("utf-8");
+	if (!git_commit_encoding)
+		git_commit_encoding = "utf-8";
+
+	strbuf_init(&sbuf, 8192); /* should avoid reallocs for the headers */
+	strbuf_addf(&sbuf, "tree %s\n", sha1_to_hex(tree_sha1));
+	if (parent_sha1)
+		strbuf_addf(&sbuf, "parent %s\n", sha1_to_hex(parent_sha1));
+	if (next_commit.parents.nr) {
+		int i;
+		for (i = 0; i < next_commit.parents.nr; ++i)
+			strbuf_addf(&sbuf, "parent %s\n",
+					next_commit.parents.items[i].string);
+	}
+	if (!next_commit.author.time) {
+		char time[50];
+		datestamp(time, sizeof(time));
+		next_commit.author.time = xstrdup(time);
+	}
+
+	stripspace(&next_commit.summary, 1);
+
+	/* if encodings differ, reencode whole buffer */
+	if (strcasecmp(git_commit_encoding, next_commit.encoding)) {
+		if ((reencoded = reencode_string(next_commit.author.name,
+				git_commit_encoding, next_commit.encoding))) {
+			free((void *)next_commit.author.name);
+			next_commit.author.name = reencoded;
+		}
+		if ((reencoded = reencode_string(next_commit.summary.buf,
+				git_commit_encoding, next_commit.encoding))) {
+			strbuf_reset(&next_commit.summary);
+			strbuf_addstr(&next_commit.summary, reencoded);
+		}
+	}
+	strbuf_addf(&sbuf, "author %s <%s> %s\n", next_commit.author.name,
+			next_commit.author.mail, next_commit.author.time);
+	strbuf_addf(&sbuf, "committer %s\n", git_committer_info(0));
+	if (!is_encoding_utf8(git_commit_encoding))
+		strbuf_addf(&sbuf, "encoding %s\n", git_commit_encoding);
+	strbuf_addch(&sbuf, '\n');
+	strbuf_addbuf(&sbuf, &next_commit.summary);
+	if (sbuf.buf[sbuf.len-1] != '\n')
+		strbuf_addch(&sbuf, '\n');
+
+	failed = write_sha1_file(sbuf.buf, sbuf.len, commit_type, commit_sha1);
+	strbuf_release(&sbuf);
+	if (failed)
+		return 1;
+
+	if (verbosity > 1)
+		printf("Created %scommit %s\n",
+			parent_sha1 || next_commit.parents.nr ? "" : "initial ",
+			sha1_to_hex(commit_sha1));
+
+	if (update_ref(reflog, "HEAD", commit_sha1, NULL, 0, 0))
+		return error("Could not update HEAD to %s.",
+						sha1_to_hex(commit_sha1));
+
+	return 0;
+}
+
+/*
+ * Fill next_commit.author according to ident.
+ * Ident may have one of the following forms:
+ * 	"name <e-mail> timestamp timezone\n..."
+ * 	"name <e-mail> timestamp timezone"
+ * 	"name <e-mail>"
+ */
+static void set_author_info(const char *ident)
+{
+	const char *tmp1 = strstr(ident, " <");
+	const char *tmp2;
+	char *data;
+	if (!tmp1)
+		return;
+	tmp2 = strstr(tmp1+2, ">");
+	if (!tmp2)
+		return;
+	if (tmp2[1] != 0 && tmp2[1] != ' ')
+		return;
+
+	data = xmalloc(strlen(ident)); /* a trivial upper bound */
+
+	snprintf(data, tmp1-ident+1, "%s", ident);
+	next_commit.author.name = xstrdup(data);
+	snprintf(data, tmp2-tmp1-1, "%s", tmp1+2);
+	next_commit.author.mail = xstrdup(data);
+
+	if (tmp2[1] == 0) {
+		free(data);
+		return;
+	}
+
+	tmp1 = strpbrk(tmp2+2, "\r\n");
+	if (!tmp1)
+		tmp1 = tmp2 + strlen(tmp2);
+
+	snprintf(data, tmp1-tmp2-1, "%s", tmp2+2);
+	next_commit.author.time = xstrdup(data);
+	free(data);
+}
+
+static void set_message_source(const char *source)
+{
+	if (next_commit.source)
+		free(next_commit.source);
+	next_commit.source = xstrdup(source);
+}
+
+/* Set subject, an information for the case of conflict */
+static void set_pick_subject(const char *hex, struct commit *commit)
+{
+	const char *tmp = strstr(commit->buffer, "\n\n");
+	if (tmp) {
+		const char *eol;
+		int len = strlen(hex);
+		tmp += 2;
+		eol = strchrnul(tmp, '\n');
+		next_commit.subject = xmalloc(eol - tmp + len + 5);
+		snprintf(next_commit.subject, eol - tmp + len + 5, "%s... %s",
+								hex, tmp);
+	}
+}
+
 /* Return a commit object of "arg" */
 static struct commit *get_commit(const char *arg)
 {
@@ -198,6 +392,26 @@ static int set_verbosity(int verbose)
 	return 0;
 }
 
+static int write_commit_summary_into(const char *filename)
+{
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+	int fd = hold_lock_file_for_update(lock, filename, 0);
+	if (fd < 0)
+		return error("Unable to create '%s.lock': %s", filename,
+							strerror(errno));
+	if (write_in_full(fd, next_commit.summary.buf,
+			      next_commit.summary.len) < 0)
+		return error("Could not write to %s: %s",
+						filename, strerror(errno));
+	if (commit_lock_file(lock) < 0)
+		return error("Error wrapping up %s", filename);
+	return 0;
+}
+
+/**********************************************************************
+ * Builtin sequencer helper functions
+ */
+
 /* v should be "" or "t" or "\d" */
 static int parse_verbosity(const char *v)
 {
-- 
1.6.4.271.ge010d

[PATCH v2 8/9] sequencer: add "--cherry-pick" option to "git sequencer--helper"

[Christian Couder]

From: Stephan Beyer <s-beyer@gmx.net>

This patch adds some code that comes from the sequencer GSoC project:

git://repo.or.cz/git/sbeyer.git

(commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)

Most of the code from do_cherry_pick() is taken from the
sequencer insn_pick_act() function.

Mentored-by: Daniel Barkalow <barkalow@iabervon.org>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   71 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 61a8f2e..291ba18 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -60,7 +60,8 @@ static const char * const git_sequencer_helper_usage[] = {
 		"<verbosity> [<allow-dirty>]",
 	"git sequencer--helper --fast-forward <commit> <reflog-msg> "
 		"<verbosity> [<allow-dirty>]",
-	"git sequencer--helper --cherry-pick <commit> [<do-not-commit>]",
+	"git sequencer--helper --cherry-pick <commit> <reflog-msg> "
+		"<verbosity> [<do-not-commit>]",
 	NULL
 };
 
@@ -288,7 +289,7 @@ static int do_commit(unsigned char *parent_sha1)
 
 	if (update_ref(reflog, "HEAD", commit_sha1, NULL, 0, 0))
 		return error("Could not update HEAD to %s.",
-						sha1_to_hex(commit_sha1));
+			     sha1_to_hex(commit_sha1));
 
 	return 0;
 }
@@ -408,6 +409,46 @@ static int write_commit_summary_into(const char *filename)
 	return 0;
 }
 
+static int do_cherry_pick(char *cp_commit, int no_commit)
+{
+	struct commit *commit;
+	int failed;
+	const char *author;
+
+	if (get_sha1("HEAD", head_sha1))
+		return error("You do not have a valid HEAD.");
+
+	commit = get_commit(cp_commit);
+	if (!commit)
+		return 1;
+
+	set_pick_subject(cp_commit, commit);
+
+	failed = pick_commit(commit, 0, 0, &next_commit.summary);
+
+	set_message_source(sha1_to_hex(commit->object.sha1));
+	author = strstr(commit->buffer, "\nauthor ");
+	if (author)
+		set_author_info(author + 8);
+
+	/* We do not want extra Conflicts: lines on cherry-pick,
+	   so just take the old commit message. */
+	if (failed) {
+		strbuf_setlen(&next_commit.summary, 0);
+		strbuf_addstr(&next_commit.summary,
+			      strstr(commit->buffer, "\n\n") + 2);
+		rerere();
+		make_patch(commit);
+		write_commit_summary_into(MERGE_MSG);
+		return error(pick_help_msg(commit->object.sha1, 0));
+	}
+
+	if (!no_commit && do_commit(head_sha1))
+		return error("Could not commit.");
+
+	return 0;
+}
+
 /**********************************************************************
  * Builtin sequencer helper functions
  */
@@ -436,6 +477,7 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 	char *patch_commit = NULL;
 	char *reset_commit = NULL;
 	char *ff_commit = NULL;
+	char *cp_commit = NULL;
 	struct option options[] = {
 		OPT_STRING(0, "make-patch", &patch_commit, "commit",
 			   "create a patch from commit"),
@@ -443,6 +485,8 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 			   "reset to commit"),
 		OPT_STRING(0, "fast-forward", &ff_commit, "commit",
 			   "fast forward to commit"),
+		OPT_STRING(0, "cherry-pick", &cp_commit, "commit",
+			   "cherry pick commit"),
 		OPT_END()
 	};
 
@@ -459,19 +503,15 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 		return 0;
 	}
 
-	if (ff_commit || reset_commit) {
+	if (cp_commit || ff_commit || reset_commit) {
 		unsigned char sha1[20];
-		char *commit = ff_commit ? ff_commit : reset_commit;
+		char *commit;
+		int opt_arg = 0;
 
 		if (argc != 2 && argc != 3)
 			usage_with_options(git_sequencer_helper_usage,
 					   options);
 
-		if (get_sha1(commit, sha1)) {
-			error("Could not find '%s'", commit);
-			return 1;
-		}
-
 		reflog = (char *)argv[0];
 
 		if (parse_verbosity(argv[1])) {
@@ -480,7 +520,18 @@ int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 		}
 
 		if (argc == 3 && *argv[2] && strcmp(argv[2], "0"))
-			allow_dirty = 1;
+			opt_arg = 1;
+
+		if (cp_commit)
+			return do_cherry_pick(cp_commit, opt_arg);
+
+		allow_dirty = opt_arg;
+
+		commit = ff_commit ? ff_commit : reset_commit;
+		if (get_sha1(commit, sha1)) {
+			error("Could not find '%s'", commit);
+			return 1;
+		}
 
 		if (ff_commit)
 			return do_fast_forward(sha1);
-- 
1.6.4.271.ge010d

[PATCH v2 9/9] rebase -i: use "git sequencer--helper --cherry-pick"

[Christian Couder]

instead of "git cherry-pick", as this will make it easier to
port "git-rebase--interactive.sh" to C.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-rebase--interactive.sh |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7651fd6..349ca50 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -147,7 +147,8 @@ pick_one () {
 		pick_one_preserving_merges "$@" && return
 	if test ! -z "$REBASE_ROOT"
 	then
-		output git cherry-pick "$@"
+		git sequencer--helper --cherry-pick $sha1 \
+			"$GIT_REFLOG_ACTION" "$VERBOSE" $no_ff
 		return
 	fi
 	parent_sha1=$(git rev-parse --verify $sha1^) ||
@@ -157,7 +158,8 @@ pick_one () {
 		git sequencer--helper --fast-forward $sha1 \
 			"$GIT_REFLOG_ACTION" "$VERBOSE"
 	else
-		output git cherry-pick "$@"
+		git sequencer--helper --cherry-pick $sha1 \
+			"$GIT_REFLOG_ACTION" "$VERBOSE" $no_ff
 	fi
 }
 
@@ -269,7 +271,10 @@ pick_one_preserving_merges () {
 			fi
 			;;
 		*)
-			output git cherry-pick "$@" ||
+			no_commit=
+			test "a$1" = "a-n" && no_commit=t
+			git sequencer--helper --cherry-pick $sha1 \
+				"$GIT_REFLOG_ACTION" "$VERBOSE" $no_commit ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
-- 
1.6.4.271.ge010d

Re: [PATCH v2 5/9] revert: libify pick

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> From: Stephan Beyer <s-beyer@gmx.net>
>
> This commit is made of code from the sequencer GSoC project:
>
> git://repo.or.cz/git/sbeyer.git
>
> (commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
>
> The goal of this commit is to abstract out pick functionnality
> into a new pick() function made of code from "builtin-revert.c".
>
> The new pick() function is in a new "pick.c" file with an
> associated "pick.h".

Is it pick() or pick_commit()?

By the way, this comment applies to other messages in the series, but do
these original commits by Stephan lack any meaningful messages?  I would
actually have expected to see a commit log message that looks more like:

	From: Stephan Beyer <...>

        Meaningful log message Stephan describes what this commit does in
        sufficient detail, perhaps with clarification and rewording by
        Christian.

	Second and subsequent paragraph to explain it as necessary.

	[cc: this is based on 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079
        from Stephan's git://repo.or.cz/git/sbeyer.git]

        Signed-off-by: ...

The point is that the "origin info" would be a side-note, not the first
and the most important thing in the message.

Re: [PATCH v2 5/9] revert: libify pick

[Christian Couder]

On Friday 21 August 2009, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > From: Stephan Beyer <s-beyer@gmx.net>
> >
> > This commit is made of code from the sequencer GSoC project:
> >
> > git://repo.or.cz/git/sbeyer.git
> >
> > (commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
> >
> > The goal of this commit is to abstract out pick functionnality
> > into a new pick() function made of code from "builtin-revert.c".
> >
> > The new pick() function is in a new "pick.c" file with an
> > associated "pick.h".
>
> Is it pick() or pick_commit()?

It is pick_commit(). Sorry I didn't properly update the commit message.

> By the way, this comment applies to other messages in the series, but do
> these original commits by Stephan lack any meaningful messages?

Well, in Stephan's repo the commit 
(94a568a78d243d7a6c13778bc6b7ac1eb46e48cc) that created the pick.c and 
pick.h files had this message:

------
Libify cherry-pick and revert.

A poor start.
------

(Note that I have seen far worse commit messages in other GSoC students' 
repo.)
And then quite a few other commits improved on this one, and the whole gives 
the code that is in this patch.

> I would 
> actually have expected to see a commit log message that looks more like:
>
> 	From: Stephan Beyer <...>
>
>         Meaningful log message Stephan describes what this commit does in
>         sufficient detail, perhaps with clarification and rewording by
>         Christian.
>
> 	Second and subsequent paragraph to explain it as necessary.
>
> 	[cc: this is based on 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079
>         from Stephan's git://repo.or.cz/git/sbeyer.git]
>
>         Signed-off-by: ...
>
> The point is that the "origin info" would be a side-note, not the first
> and the most important thing in the message.

I tried to improve some commit messages like this in the v3 that will follow 
this email.

Thanks,
Christian.