[PATCH v3 0/9] cherry-pick: improvements

[PATCH v3 0/9] cherry-pick: improvements

[Felipe Contreras]

Hi,

Sames as v2, plus a few changes from the feedback, and cleanups.

Felipe Contreras (9):
  sequencer: remove useless indentation
  sequencer: trivial fix
  cherry-pick: add --skip-empty option
  cherry-pick: store rewritten commits
  sequencer: run post-rewrite hook
  cherry-pick: add support to copy notes
  revert/cherry-pick: add --quiet option
  revert/cherry-pick: add --skip option
  cherry-pick: don't barf when there's nothing to do

 Documentation/git-cherry-pick.txt   |  10 +-
 Documentation/git-revert.txt        |   7 +-
 Documentation/sequencer.txt         |   3 +
 builtin/revert.c                    |  15 +++
 sequencer.c                         | 230 ++++++++++++++++++++++++++++++++++--
 sequencer.h                         |   6 +-
 t/t3508-cherry-pick-many-commits.sh |  13 ++
 t/t3510-cherry-pick-sequence.sh     |  12 ++
 8 files changed, 280 insertions(+), 16 deletions(-)

-- 
1.8.3.698.g079b096

[PATCH v3 1/9] sequencer: remove useless indentation

[Felipe Contreras]

By using good ol' goto.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index ab6f8a7..b4989ba 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -474,7 +474,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
 	char *defmsg = NULL;
 	struct strbuf msgbuf = STRBUF_INIT;
-	int res, unborn = 0;
+	int res, unborn = 0, allow;
 
 	if (opts->no_commit) {
 		/*
@@ -624,14 +624,16 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		      msg.subject);
 		print_advice(res == 1, opts);
 		rerere(opts->allow_rerere_auto);
-	} else {
-		int allow = allow_empty(opts, commit);
-		if (allow < 0)
-			return allow;
-		if (!opts->no_commit)
-			res = run_git_commit(defmsg, opts, allow);
+		goto leave;
 	}
 
+	allow = allow_empty(opts, commit);
+	if (allow < 0)
+		return allow;
+	if (!opts->no_commit)
+		res = run_git_commit(defmsg, opts, allow);
+
+leave:
 	free_message(&msg);
 	free(defmsg);
 
-- 
1.8.3.698.g079b096

[PATCH v3 2/9] sequencer: trivial fix

[Felipe Contreras]

We should free objects before leaving.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index b4989ba..f7be7d8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -628,8 +628,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	}
 
 	allow = allow_empty(opts, commit);
-	if (allow < 0)
-		return allow;
+	if (allow < 0) {
+		res = allow;
+		goto leave;
+	}
 	if (!opts->no_commit)
 		res = run_git_commit(defmsg, opts, allow);
 
-- 
1.8.3.698.g079b096

[PATCH v3 3/9] cherry-pick: add --skip-empty option

[Felipe Contreras]

Pretty much what it says on the tin.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-cherry-pick.txt   |  3 +++
 builtin/revert.c                    |  8 ++++++++
 sequencer.c                         |  6 ++++++
 sequencer.h                         |  1 +
 t/t3508-cherry-pick-many-commits.sh | 13 +++++++++++++
 5 files changed, 31 insertions(+)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index c205d23..fccd936 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -129,6 +129,9 @@ effect to your index in a row.
 	redundant commits are ignored.  This option overrides that behavior and
 	creates an empty commit object.  Implies `--allow-empty`.
 
+--skip-empty::
+	Instead of failing, skip commits that are or become empty.
+
 --strategy=<strategy>::
 	Use the given merge strategy.  Should only be used once.
 	See the MERGE STRATEGIES section in linkgit:git-merge[1]
diff --git a/builtin/revert.c b/builtin/revert.c
index 0401fdb..5a8453d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -118,6 +118,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		OPT_END(),
 		OPT_END(),
 		OPT_END(),
+		OPT_END(),
 	};
 
 	if (opts->action == REPLAY_PICK) {
@@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
 			OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
 			OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+			OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")),
 			OPT_END(),
 		};
 		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
@@ -144,6 +146,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 				"--abort", rollback,
 				NULL);
 
+	verify_opt_mutually_compatible(me,
+				"--allow-empty", opts->allow_empty,
+				"--skip-empty", opts->skip_empty,
+				"--keep-redundant-commits", opts->keep_redundant_commits,
+				NULL);
+
 	/* implies allow_empty */
 	if (opts->keep_redundant_commits)
 		opts->allow_empty = 1;
diff --git a/sequencer.c b/sequencer.c
index f7be7d8..d3c7d72 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 		goto leave;
 	}
 
+	if (opts->skip_empty && is_index_unchanged() == 1) {
+		warning(_("skipping %s... %s"),
+			find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+			msg.subject);
+		goto leave;
+	}
 	allow = allow_empty(opts, commit);
 	if (allow < 0) {
 		res = allow;
diff --git a/sequencer.h b/sequencer.h
index 1fc22dc..3b04844 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -34,6 +34,7 @@ struct replay_opts {
 	int allow_empty;
 	int allow_empty_message;
 	int keep_redundant_commits;
+	int skip_empty;
 
 	int mainline;
 
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 19c99d7..3dc19c6 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' '
 	check_head_differs_from fourth
 '
 
+test_expect_success 'cherry-pick skip empty' '
+	git clean -fxd &&
+	git checkout -b empty fourth &&
+	git commit --allow-empty -m empty &&
+	test_commit ontop &&
+	git checkout -f master &&
+	git reset --hard fourth &&
+	git cherry-pick --skip-empty fourth..empty &&
+	echo ontop > expected &&
+	git log --format=%s fourth..HEAD > actual
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.3.698.g079b096

[PATCH v3 4/9] cherry-pick: store rewritten commits

[Felipe Contreras]

Will be useful for the next commits.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 sequencer.h |   1 +
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index d3c7d72..76ff2ff 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,18 @@
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
+struct rewritten_list_item {
+	unsigned char from[20];
+	unsigned char to[20];
+};
+
+struct rewritten_list {
+	struct rewritten_list_item *items;
+	unsigned int nr, alloc;
+};
+
+static struct rewritten_list rewritten;
+
 static int is_rfc2822_line(const char *buf, int len)
 {
 	int i;
@@ -102,6 +114,19 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
 	return 1;
 }
 
+static void add_rewritten(unsigned char *from, unsigned char *to)
+{
+	struct rewritten_list_item *item;
+	if (rewritten.nr + 1 >= rewritten.alloc) {
+		rewritten.alloc += 32;
+		rewritten.items = xrealloc(rewritten.items, rewritten.alloc * sizeof(*rewritten.items));
+	}
+	item = &rewritten.items[rewritten.nr];
+	hashcpy(item->from, from);
+	hashcpy(item->to, to);
+	rewritten.nr++;
+}
+
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -641,6 +666,14 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	if (!opts->no_commit)
 		res = run_git_commit(defmsg, opts, allow);
 
+	if (!res && opts->action == REPLAY_PICK) {
+		unsigned char to[20];
+
+		if (read_ref("HEAD", to))
+			goto leave;
+
+		add_rewritten(commit->object.sha1, to);
+	}
 leave:
 	free_message(&msg);
 	free(defmsg);
@@ -786,6 +819,40 @@ static void read_populate_todo(struct commit_list **todo_list,
 		die(_("Unusable instruction sheet: %s"), todo_file);
 }
 
+static void read_populate_rewritten(void)
+{
+	const char *rewritten_file = git_path(SEQ_REWR_FILE);
+	struct strbuf buf = STRBUF_INIT;
+	char *p;
+	int fd;
+
+	fd = open(rewritten_file, O_RDONLY);
+	if (fd < 0)
+		return;
+	if (strbuf_read(&buf, fd, 0) < 0) {
+		close(fd);
+		strbuf_release(&buf);
+		return;
+	}
+	close(fd);
+
+	for (p = buf.buf; *p;) {
+		unsigned char from[20];
+		unsigned char to[20];
+		char *eol = strchrnul(p, '\n');
+		if (eol - p != 81)
+			/* wrong size */
+			break;
+		if (get_sha1_hex(p, from))
+			break;
+		if (get_sha1_hex(p + 41, to))
+			break;
+		add_rewritten(from, to);
+		p = *eol ? eol + 1 : eol;
+	}
+	strbuf_release(&buf);
+}
+
 static int populate_opts_cb(const char *key, const char *value, void *data)
 {
 	struct replay_opts *opts = data;
@@ -958,6 +1025,29 @@ static void save_todo(struct commit_list *todo_list, struct replay_opts *opts)
 	strbuf_release(&buf);
 }
 
+static void save_rewritten(void)
+{
+	const char *todo_file = git_path(SEQ_REWR_FILE);
+	static struct lock_file rewritten_lock;
+	struct strbuf buf = STRBUF_INIT;
+	int fd, i;
+
+	fd = hold_lock_file_for_update(&rewritten_lock, todo_file, LOCK_DIE_ON_ERROR);
+	for (i = 0; i < rewritten.nr; i++) {
+		struct rewritten_list_item *item = &rewritten.items[i];
+		strbuf_addf(&buf, "%s %s\n", sha1_to_hex(item->from), sha1_to_hex(item->to));
+	}
+	if (write_in_full(fd, buf.buf, buf.len) < 0) {
+		strbuf_release(&buf);
+		die_errno(_("Could not write to %s"), todo_file);
+	}
+	if (commit_lock_file(&rewritten_lock) < 0) {
+		strbuf_release(&buf);
+		die(_("Error wrapping up %s."), todo_file);
+	}
+	strbuf_release(&buf);
+}
+
 static void save_opts(struct replay_opts *opts)
 {
 	const char *opts_file = git_path(SEQ_OPTS_FILE);
@@ -1003,8 +1093,11 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 	for (cur = todo_list; cur; cur = cur->next) {
 		save_todo(cur, opts);
 		res = do_pick_commit(cur->item, opts);
-		if (res)
+		if (res) {
+			if (opts->action == REPLAY_PICK)
+				save_rewritten();
 			return res;
+		}
 	}
 
 	/*
@@ -1033,6 +1126,8 @@ static int sequencer_continue(struct replay_opts *opts)
 		return continue_single_pick();
 	read_populate_opts(&opts);
 	read_populate_todo(&todo_list, opts);
+	if (opts->action == REPLAY_PICK)
+		read_populate_rewritten();
 
 	/* Verify that the conflict has been resolved */
 	if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
@@ -1043,6 +1138,11 @@ static int sequencer_continue(struct replay_opts *opts)
 	}
 	if (index_differs_from("HEAD", 0))
 		return error_dirty_index(opts);
+	if (opts->action == REPLAY_PICK) {
+		unsigned char to[20];
+		if (!read_ref("HEAD", to))
+			add_rewritten(todo_list->item->object.sha1, to);
+	}
 	todo_list = todo_list->next;
 	return pick_commits(todo_list, opts);
 }
diff --git a/sequencer.h b/sequencer.h
index 3b04844..84b9957 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -5,6 +5,7 @@
 #define SEQ_HEAD_FILE	"sequencer/head"
 #define SEQ_TODO_FILE	"sequencer/todo"
 #define SEQ_OPTS_FILE	"sequencer/opts"
+#define SEQ_REWR_FILE	"sequencer/rewritten"
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
 
-- 
1.8.3.698.g079b096

[PATCH v3 5/9] sequencer: run post-rewrite hook

[Felipe Contreras]

As we should.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 76ff2ff..74480d7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -127,6 +127,37 @@ static void add_rewritten(unsigned char *from, unsigned char *to)
 	rewritten.nr++;
 }
 
+static void run_rewrite_hook(const char *name)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process proc;
+	const char *argv[3];
+	int code, i;
+
+	argv[0] = find_hook("post-rewrite");
+	if (!argv[0])
+		return;
+
+	argv[1] = name;
+	argv[2] = NULL;
+
+	memset(&proc, 0, sizeof(proc));
+	proc.argv = argv;
+	proc.in = -1;
+	proc.stdout_to_stderr = 1;
+
+	code = start_command(&proc);
+	if (code)
+		return;
+	for (i = 0; i < rewritten.nr; i++) {
+		struct rewritten_list_item *item = &rewritten.items[i];
+		strbuf_addf(&buf, "%s %s\n", sha1_to_hex(item->from), sha1_to_hex(item->to));
+	}
+	write_in_full(proc.in, buf.buf, buf.len);
+	close(proc.in);
+	finish_command(&proc);
+}
+
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -1100,6 +1131,9 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 		}
 	}
 
+	if (opts->action == REPLAY_PICK)
+		run_rewrite_hook("cherry-pick");
+
 	/*
 	 * Sequence of picks finished successfully; cleanup by
 	 * removing the .git/sequencer directory
@@ -1149,8 +1183,14 @@ static int sequencer_continue(struct replay_opts *opts)
 
 static int single_pick(struct commit *cmit, struct replay_opts *opts)
 {
+	int ret;
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
-	return do_pick_commit(cmit, opts);
+	ret = do_pick_commit(cmit, opts);
+	if (ret)
+		return ret;
+	if (opts->action == REPLAY_PICK)
+		run_rewrite_hook("cherry-pick");
+	return 0;
 }
 
 int sequencer_pick_revisions(struct replay_opts *opts)
-- 
1.8.3.698.g079b096

[PATCH v3 6/9] cherry-pick: add support to copy notes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 74480d7..6d13e63 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,6 +14,7 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "builtin.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
@@ -158,6 +159,32 @@ static void run_rewrite_hook(const char *name)
 	finish_command(&proc);
 }
 
+static void copy_notes(const char *name)
+{
+	struct notes_rewrite_cfg *cfg;
+	int i;
+
+	cfg = init_copy_notes_for_rewrite(name);
+	if (!cfg)
+		return;
+
+	for (i = 0; i < rewritten.nr; i++) {
+		struct rewritten_list_item *item = &rewritten.items[i];
+		copy_note_for_rewrite(cfg, item->from, item->to);
+	}
+
+	finish_copy_notes_for_rewrite(cfg);
+}
+
+static void finish(struct replay_opts *opts)
+{
+	if (opts->action != REPLAY_PICK)
+		return;
+
+	run_rewrite_hook("cherry-pick");
+	copy_notes("cherry-pick");
+}
+
 static void remove_sequencer_state(void)
 {
 	struct strbuf seq_dir = STRBUF_INIT;
@@ -1131,8 +1158,7 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 		}
 	}
 
-	if (opts->action == REPLAY_PICK)
-		run_rewrite_hook("cherry-pick");
+	finish(opts);
 
 	/*
 	 * Sequence of picks finished successfully; cleanup by
@@ -1188,8 +1214,7 @@ static int single_pick(struct commit *cmit, struct replay_opts *opts)
 	ret = do_pick_commit(cmit, opts);
 	if (ret)
 		return ret;
-	if (opts->action == REPLAY_PICK)
-		run_rewrite_hook("cherry-pick");
+	finish(opts);
 	return 0;
 }
 
-- 
1.8.3.698.g079b096

[PATCH v3 7/9] revert/cherry-pick: add --quiet option

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-cherry-pick.txt | 6 +++++-
 Documentation/git-revert.txt      | 6 +++++-
 builtin/revert.c                  | 1 +
 sequencer.c                       | 9 ++++++---
 sequencer.h                       | 1 +
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index fccd936..da0bd81 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -8,7 +8,7 @@ git-cherry-pick - Apply the changes introduced by some existing commits
 SYNOPSIS
 --------
 [verse]
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
+'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
 'git cherry-pick' --continue
 'git cherry-pick' --quit
 'git cherry-pick' --abort
@@ -51,6 +51,10 @@ OPTIONS
 	feed all <commit>... arguments to a single revision walk
 	(see a later example that uses 'maint master..next').
 
+-q::
+--quiet::
+	Quiet, suppress feedback messages.
+
 -e::
 --edit::
 	With this option, 'git cherry-pick' will let you edit the commit
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index f79c9d8..98a8e7a 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -8,7 +8,7 @@ git-revert - Revert some existing commits
 SYNOPSIS
 --------
 [verse]
-'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...
+'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...
 'git revert' --continue
 'git revert' --quit
 'git revert' --abort
@@ -40,6 +40,10 @@ OPTIONS
 	default, see linkgit:git-rev-list[1] and its '--no-walk'
 	option.
 
+-q::
+--quiet::
+	Quiet, suppress feedback messages.
+
 -e::
 --edit::
 	With this option, 'git revert' will let you edit the commit
diff --git a/builtin/revert.c b/builtin/revert.c
index 5a8453d..ec83748 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -100,6 +100,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	int contin = 0;
 	int rollback = 0;
 	struct option options[] = {
+		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
 		OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or cherry-pick sequence")),
 		OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or cherry-pick sequence")),
 		OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or cherry-pick sequence")),
diff --git a/sequencer.c b/sequencer.c
index 6d13e63..4b91bd4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -472,6 +472,8 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,
 	argv_array_init(&array);
 	argv_array_push(&array, "commit");
 	argv_array_push(&array, "-n");
+	if (opts->quiet)
+		argv_array_push(&array, "-q");
 
 	if (opts->signoff)
 		argv_array_push(&array, "-s");
@@ -711,9 +713,10 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
 	}
 
 	if (opts->skip_empty && is_index_unchanged() == 1) {
-		warning(_("skipping %s... %s"),
-			find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
-			msg.subject);
+		if (!opts->quiet)
+			warning(_("skipping %s... %s"),
+				find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
+				msg.subject);
 		goto leave;
 	}
 	allow = allow_empty(opts, commit);
diff --git a/sequencer.h b/sequencer.h
index 84b9957..6fc416d 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -36,6 +36,7 @@ struct replay_opts {
 	int allow_empty_message;
 	int keep_redundant_commits;
 	int skip_empty;
+	int quiet;
 
 	int mainline;
 
-- 
1.8.3.698.g079b096

[PATCH v3 8/9] revert/cherry-pick: add --skip option

[Felipe Contreras]

Akin to 'am --skip' and 'rebase --skip'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-cherry-pick.txt |  1 +
 Documentation/git-revert.txt      |  1 +
 Documentation/sequencer.txt       |  3 +++
 builtin/revert.c                  |  6 ++++++
 sequencer.c                       | 32 +++++++++++++++++++++++++++++---
 sequencer.h                       |  3 ++-
 t/t3510-cherry-pick-sequence.sh   | 12 ++++++++++++
 7 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index da0bd81..d95c63c 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git cherry-pick' [-q] [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
 'git cherry-pick' --continue
+'git cherry-pick' --skip
 'git cherry-pick' --quit
 'git cherry-pick' --abort
 
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 98a8e7a..52e146e 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git revert' [-q] [--[no-]edit] [-n] [-m parent-number] [-s] <commit>...
 'git revert' --continue
+'git revert' --skip
 'git revert' --quit
 'git revert' --abort
 
diff --git a/Documentation/sequencer.txt b/Documentation/sequencer.txt
index 5747f44..df2d355 100644
--- a/Documentation/sequencer.txt
+++ b/Documentation/sequencer.txt
@@ -3,6 +3,9 @@
 	'.git/sequencer'.  Can be used to continue after resolving
 	conflicts in a failed cherry-pick or revert.
 
+--skip::
+	Skip the current commit, and then continue.
+
 --quit::
 	Forget about the current operation in progress.  Can be used
 	to clear the sequencer state after a failed cherry-pick or
diff --git a/builtin/revert.c b/builtin/revert.c
index ec83748..d3d5600 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -99,11 +99,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	int remove_state = 0;
 	int contin = 0;
 	int rollback = 0;
+	int skip = 0;
 	struct option options[] = {
 		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
 		OPT_BOOLEAN(0, "quit", &remove_state, N_("end revert or cherry-pick sequence")),
 		OPT_BOOLEAN(0, "continue", &contin, N_("resume revert or cherry-pick sequence")),
 		OPT_BOOLEAN(0, "abort", &rollback, N_("cancel revert or cherry-pick sequence")),
+		OPT_BOOLEAN(0, "skip", &skip, N_("skip current commit in the sequence")),
 		OPT_BOOLEAN('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
 		OPT_BOOLEAN('e', "edit", &opts->edit, N_("edit the commit message")),
 		OPT_NOOP_NOARG('r', NULL),
@@ -164,6 +166,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		opts->subcommand = REPLAY_CONTINUE;
 	else if (rollback)
 		opts->subcommand = REPLAY_ROLLBACK;
+	else if (skip)
+		opts->subcommand = REPLAY_SKIP;
 	else
 		opts->subcommand = REPLAY_NONE;
 
@@ -174,6 +178,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			this_operation = "--quit";
 		else if (opts->subcommand == REPLAY_CONTINUE)
 			this_operation = "--continue";
+		else if (opts->subcommand == REPLAY_SKIP)
+			this_operation = "--skip";
 		else {
 			assert(opts->subcommand == REPLAY_ROLLBACK);
 			this_operation = "--abort";
diff --git a/sequencer.c b/sequencer.c
index 4b91bd4..dd9de49 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1181,7 +1181,9 @@ static int continue_single_pick(void)
 	return run_command_v_opt(argv, RUN_GIT_CMD);
 }
 
-static int sequencer_continue(struct replay_opts *opts)
+static void add_rewritten(unsigned char *from, unsigned char *to);
+
+static int sequencer_continue(struct replay_opts *opts, int skip)
 {
 	struct commit_list *todo_list = NULL;
 
@@ -1201,7 +1203,7 @@ static int sequencer_continue(struct replay_opts *opts)
 	}
 	if (index_differs_from("HEAD", 0))
 		return error_dirty_index(opts);
-	if (opts->action == REPLAY_PICK) {
+	if (opts->action == REPLAY_PICK && !skip) {
 		unsigned char to[20];
 		if (!read_ref("HEAD", to))
 			add_rewritten(todo_list->item->object.sha1, to);
@@ -1210,6 +1212,28 @@ static int sequencer_continue(struct replay_opts *opts)
 	return pick_commits(todo_list, opts);
 }
 
+static int sequencer_skip(struct replay_opts *opts)
+{
+	const char *argv[4]; /* reset --hard HEAD + NULL */
+	struct string_list merge_rr = STRING_LIST_INIT_DUP;
+	int ret;
+
+	if (setup_rerere(&merge_rr, 0) >= 0) {
+		rerere_clear(&merge_rr);
+		string_list_clear(&merge_rr, 1);
+	}
+
+	argv[0] = "reset";
+	argv[1] = "--hard";
+	argv[2] = "HEAD";
+	argv[3] = NULL;
+	ret = run_command_v_opt(argv, RUN_GIT_CMD);
+	if (ret)
+		return ret;
+
+	return sequencer_continue(opts, 1);
+}
+
 static int single_pick(struct commit *cmit, struct replay_opts *opts)
 {
 	int ret;
@@ -1244,7 +1268,9 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	if (opts->subcommand == REPLAY_ROLLBACK)
 		return sequencer_rollback(opts);
 	if (opts->subcommand == REPLAY_CONTINUE)
-		return sequencer_continue(opts);
+		return sequencer_continue(opts, 0);
+	if (opts->subcommand == REPLAY_SKIP)
+		return sequencer_skip(opts);
 
 	for (i = 0; i < opts->revs->pending.nr; i++) {
 		unsigned char sha1[20];
diff --git a/sequencer.h b/sequencer.h
index 6fc416d..efec1b5 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -18,7 +18,8 @@ enum replay_subcommand {
 	REPLAY_NONE,
 	REPLAY_REMOVE_STATE,
 	REPLAY_CONTINUE,
-	REPLAY_ROLLBACK
+	REPLAY_ROLLBACK,
+	REPLAY_SKIP
 };
 
 struct replay_opts {
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89d..f58a83d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -511,4 +511,16 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
 	test_line_count = 4 commits
 '
 
+test_expect_success 'skip' '
+	pristine_detach conflicting &&
+	test_must_fail git cherry-pick initial..picked &&
+
+	git checkout HEAD -- unrelated &&
+	test_must_fail git cherry-pick --continue &&
+	git cherry-pick --skip &&
+
+	git rev-list initial..HEAD >commits &&
+	test_line_count = 3 commits
+'
+
 test_done
-- 
1.8.3.698.g079b096

[PATCH v3 9/9] cherry-pick: don't barf when there's nothing to do

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index dd9de49..c3a17f8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -754,8 +754,8 @@ static void prepare_revs(struct replay_opts *opts)
 	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!opts->revs->commits)
-		die(_("empty commit set passed"));
+	if (!opts->revs->commits && !opts->quiet)
+		error(_("empty commit set passed"));
 }
 
 static void read_and_refresh_cache(struct replay_opts *opts)
-- 
1.8.3.698.g079b096

Re: [PATCH v3 6/9] cherry-pick: add support to copy notes

[Felipe Contreras]

On Thu, Jun 6, 2013 at 3:59 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  sequencer.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 74480d7..6d13e63 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -14,6 +14,7 @@
>  #include "merge-recursive.h"
>  #include "refs.h"
>  #include "argv-array.h"
> +#include "builtin.h"
>
>  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> @@ -158,6 +159,32 @@ static void run_rewrite_hook(const char *name)
>         finish_command(&proc);
>  }
>
> +static void copy_notes(const char *name)
> +{
> +       struct notes_rewrite_cfg *cfg;
> +       int i;
> +
> +       cfg = init_copy_notes_for_rewrite(name);
> +       if (!cfg)
> +               return;
> +
> +       for (i = 0; i < rewritten.nr; i++) {
> +               struct rewritten_list_item *item = &rewritten.items[i];
> +               copy_note_for_rewrite(cfg, item->from, item->to);
> +       }
> +
> +       finish_copy_notes_for_rewrite(cfg);
> +}

Unfortunately this doesn't work, because all these functions are part
of the builtins, not git lib.

-- 
Felipe Contreras

Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Pretty much what it says on the tin.

And a bit more, isn't it?

The --keep-redundant-commits option implies the --allow-empty option
and it was perfectly acceptable to give both.  By making sure that
only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
usage.

"It is implied so there is no *need* to give it redundantly" is
different from "It is implied so you shouldn't give it redundantly".

Like 7 and 8 that adds --quiet/--skip, don't we want this also for
revert?

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/git-cherry-pick.txt   |  3 +++
>  builtin/revert.c                    |  8 ++++++++
>  sequencer.c                         |  6 ++++++
>  sequencer.h                         |  1 +
>  t/t3508-cherry-pick-many-commits.sh | 13 +++++++++++++
>  5 files changed, 31 insertions(+)
>
> diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
> index c205d23..fccd936 100644
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -129,6 +129,9 @@ effect to your index in a row.
>  	redundant commits are ignored.  This option overrides that behavior and
>  	creates an empty commit object.  Implies `--allow-empty`.
>  
> +--skip-empty::
> +	Instead of failing, skip commits that are or become empty.
> +
>  --strategy=<strategy>::
>  	Use the given merge strategy.  Should only be used once.
>  	See the MERGE STRATEGIES section in linkgit:git-merge[1]
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 0401fdb..5a8453d 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -118,6 +118,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  		OPT_END(),
>  		OPT_END(),
>  		OPT_END(),
> +		OPT_END(),
>  	};
>  
>  	if (opts->action == REPLAY_PICK) {
> @@ -127,6 +128,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  			OPT_BOOLEAN(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
>  			OPT_BOOLEAN(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
>  			OPT_BOOLEAN(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
> +			OPT_BOOLEAN(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")),
>  			OPT_END(),
>  		};
>  		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
> @@ -144,6 +146,12 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
>  				"--abort", rollback,
>  				NULL);
>  
> +	verify_opt_mutually_compatible(me,
> +				"--allow-empty", opts->allow_empty,
> +				"--skip-empty", opts->skip_empty,
> +				"--keep-redundant-commits", opts->keep_redundant_commits,
> +				NULL);
> +
>  	/* implies allow_empty */
>  	if (opts->keep_redundant_commits)
>  		opts->allow_empty = 1;
> diff --git a/sequencer.c b/sequencer.c
> index f7be7d8..d3c7d72 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -627,6 +627,12 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
>  		goto leave;
>  	}
>  
> +	if (opts->skip_empty && is_index_unchanged() == 1) {
> +		warning(_("skipping %s... %s"),
> +			find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV),
> +			msg.subject);
> +		goto leave;
> +	}
>  	allow = allow_empty(opts, commit);
>  	if (allow < 0) {
>  		res = allow;
> diff --git a/sequencer.h b/sequencer.h
> index 1fc22dc..3b04844 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -34,6 +34,7 @@ struct replay_opts {
>  	int allow_empty;
>  	int allow_empty_message;
>  	int keep_redundant_commits;
> +	int skip_empty;
>  
>  	int mainline;
>  
> diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
> index 19c99d7..3dc19c6 100755
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -187,4 +187,17 @@ test_expect_success 'cherry-pick --stdin works' '
>  	check_head_differs_from fourth
>  '
>  
> +test_expect_success 'cherry-pick skip empty' '
> +	git clean -fxd &&
> +	git checkout -b empty fourth &&
> +	git commit --allow-empty -m empty &&
> +	test_commit ontop &&
> +	git checkout -f master &&
> +	git reset --hard fourth &&
> +	git cherry-pick --skip-empty fourth..empty &&
> +	echo ontop > expected &&
> +	git log --format=%s fourth..HEAD > actual
> +	test_cmp expected actual
> +'
> +
>  test_done

Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Felipe Contreras]

On Thu, Jun 6, 2013 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Pretty much what it says on the tin.
>
> And a bit more, isn't it?
>
> The --keep-redundant-commits option implies the --allow-empty option
> and it was perfectly acceptable to give both.  By making sure that
> only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
> usage.
>
> "It is implied so there is no *need* to give it redundantly" is
> different from "It is implied so you shouldn't give it redundantly".

Remove that line then.

> Like 7 and 8 that adds --quiet/--skip, don't we want this also for
> revert?

If we don't want --allow-empty in revert, we don't want --skip-empty
either. That is a separate patch.

-- 
Felipe Contreras

Re: [PATCH v3 5/9] sequencer: run post-rewrite hook

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> As we should.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  sequencer.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 76ff2ff..74480d7 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -127,6 +127,37 @@ static void add_rewritten(unsigned char *from, unsigned char *to)
>  	rewritten.nr++;
>  }
>  
> +static void run_rewrite_hook(const char *name)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	struct child_process proc;
> +	const char *argv[3];
> +	int code, i;
> +
> +	argv[0] = find_hook("post-rewrite");
> +	if (!argv[0])
> +		return;
> +
> +	argv[1] = name;
> +	argv[2] = NULL;
> +
> +	memset(&proc, 0, sizeof(proc));
> +	proc.argv = argv;
> +	proc.in = -1;
> +	proc.stdout_to_stderr = 1;
> +
> +	code = start_command(&proc);
> +	if (code)
> +		return;
> +	for (i = 0; i < rewritten.nr; i++) {
> +		struct rewritten_list_item *item = &rewritten.items[i];
> +		strbuf_addf(&buf, "%s %s\n", sha1_to_hex(item->from), sha1_to_hex(item->to));
> +	}
> +	write_in_full(proc.in, buf.buf, buf.len);
> +	close(proc.in);
> +	finish_command(&proc);
> +}

It probably is worth inserting a commit before 4/9 that adds
rewrite.[ch], and

 - introduces "struct rewritten_list[_item]";

 - moves run_rewrite_hook() in builtin/commit.c to rewrite.c;

 - changes its function signature so that it takes "char
   *action_name" and "struct rewritten *" as parameters; and

 - adjust its sole call site in cmd_commit() to feed a single-item
   rewritten_list to it.

Then 4/9 can teach cherry-pick to prepare the rewritten-list and
probably this commit can be part of that to call run_rewrite_hook().

Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Jun 6, 2013 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Pretty much what it says on the tin.
>>
>> And a bit more, isn't it?
>>
>> The --keep-redundant-commits option implies the --allow-empty option
>> and it was perfectly acceptable to give both.  By making sure that
>> only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
>> usage.
>>
>> "It is implied so there is no *need* to give it redundantly" is
>> different from "It is implied so you shouldn't give it redundantly".
>
> Remove that line then.
>
>> Like 7 and 8 that adds --quiet/--skip, don't we want this also for
>> revert?
>
> If we don't want --allow-empty in revert, we don't want --skip-empty
> either. That is a separate patch.

OK.

Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Jun 6, 2013 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Pretty much what it says on the tin.
>>
>> And a bit more, isn't it?
>>
>> The --keep-redundant-commits option implies the --allow-empty option
>> and it was perfectly acceptable to give both.  By making sure that
>> only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
>> usage.
>>
>> "It is implied so there is no *need* to give it redundantly" is
>> different from "It is implied so you shouldn't give it redundantly".
>
> Remove that line then.

That's what the submitter does.

Thanks.

Re: [PATCH v3 3/9] cherry-pick: add --skip-empty option

[Felipe Contreras]

On Thu, Jun 6, 2013 at 2:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, Jun 6, 2013 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> Pretty much what it says on the tin.
>>>
>>> And a bit more, isn't it?
>>>
>>> The --keep-redundant-commits option implies the --allow-empty option
>>> and it was perfectly acceptable to give both.  By making sure that
>>> only at most one of -k-r-d, -a-e or -s-e is given, this forbids that
>>> usage.
>>>
>>> "It is implied so there is no *need* to give it redundantly" is
>>> different from "It is implied so you shouldn't give it redundantly".
>>
>> Remove that line then.
>
> That's what the submitter does.

Apparently "the submitter" does everything. It seems sending 800
patches in the past 3 months is not enough work for you.

And what's the point? If nobody is interested in shifting from shell
scripts to C, neither am I.

Consider this series abandoned.

-- 
Felipe Contreras

Re: [PATCH v3 5/9] sequencer: run post-rewrite hook

[Felipe Contreras]

On Thu, Jun 6, 2013 at 1:40 PM, Junio C Hamano <gitster@pobox.com> wrote:

> It probably is worth inserting a commit before 4/9 that adds
> rewrite.[ch], and
>
>  - introduces "struct rewritten_list[_item]";
>
>  - moves run_rewrite_hook() in builtin/commit.c to rewrite.c;
>
>  - changes its function signature so that it takes "char
>    *action_name" and "struct rewritten *" as parameters; and
>
>  - adjust its sole call site in cmd_commit() to feed a single-item
>    rewritten_list to it.
>
> Then 4/9 can teach cherry-pick to prepare the rewritten-list and
> probably this commit can be part of that to call run_rewrite_hook().

Done. I'll send the patches in a moment.

-- 
Felipe Contreras