[PATCH v7 00/12] Improvements to cherry-pick

[PATCH v7 00/12] Improvements to cherry-pick

[Felipe Contreras]

Hi,

In the process of revamping 'git rebase' I found many areas of improvments in
`git cherry-pick`, here are the patches to improve the situation.

These were prettuch sent before already, but this time I dropped the second
part of the series to improve 'git rebase'.


Felipe Contreras (12):
  cherry-pick: don't barf when there's nothing to do
  cherry-pick: add --skip-empty option
  revert/cherry-pick: add --quiet option
  revert/cherry-pick: add --skip option
  cherry-pick: remember rerere-autoupdate
  builtin: add rewrite helper
  cherry-pick: store rewritten commits
  cherry-pick: don't store skipped commit
  builtin: move run_rewrite_hook() to rewrite.c
  builtin: rewrite: add copy_rewrite_notes()
  cherry-pick: add --action-name option
  cherry-pick: copy notes and run hooks

 Documentation/config.txt            |   9 ++-
 Documentation/git-cherry-pick.txt   |  10 ++-
 Documentation/git-revert.txt        |   7 ++-
 Documentation/githooks.txt          |   8 +--
 Documentation/sequencer.txt         |   3 +
 Makefile                            |   2 +
 builtin/commit.c                    |  46 ++------------
 builtin/revert.c                    |  11 ++++
 git-rebase--interactive.sh          |   4 +-
 rewrite.c                           | 121 ++++++++++++++++++++++++++++++++++++
 rewrite.h                           |  20 ++++++
 sequencer.c                         | 105 ++++++++++++++++++++++++++++---
 sequencer.h                         |   8 ++-
 t/t3504-cherry-pick-rerere.sh       |  39 ++++++++++++
 t/t3508-cherry-pick-many-commits.sh |  13 ++++
 t/t3510-cherry-pick-sequence.sh     |  14 ++++-
 t/t3512-revert-cherry-notes.sh      |  18 ++++++
 t/t5407-post-rewrite-hook.sh        |  16 +++++
 18 files changed, 392 insertions(+), 62 deletions(-)
 create mode 100644 rewrite.c
 create mode 100644 rewrite.h
 create mode 100755 t/t3512-revert-cherry-notes.sh

-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 01/12] cherry-pick: don't barf when there's nothing to do

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c                     | 2 +-
 t/t3510-cherry-pick-sequence.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 90cac7b..c94942a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -662,7 +662,7 @@ static void prepare_revs(struct replay_opts *opts)
 		die(_("revision walk setup failed"));
 
 	if (!opts->revs->commits)
-		die(_("empty commit set passed"));
+		error(_("empty commit set passed"));
 }
 
 static void read_and_refresh_cache(struct replay_opts *opts)
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89d..33c5512 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -472,7 +472,7 @@ test_expect_success 'malformed instruction sheet 2' '
 
 test_expect_success 'empty commit set' '
 	pristine_detach initial &&
-	test_expect_code 128 git cherry-pick base..base
+	git cherry-pick base..base
 '
 
 test_expect_success 'malformed instruction sheet 3' '
-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 02/12] 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                    |  2 ++
 sequencer.c                         |  6 ++++++
 sequencer.h                         |  1 +
 t/t3508-cherry-pick-many-commits.sh | 13 +++++++++++++
 5 files changed, 25 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 87659c9..9946ecd 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -95,6 +95,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) {
@@ -104,6 +105,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			OPT_BOOL(0, "allow-empty", &opts->allow_empty, N_("preserve initially empty commits")),
 			OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
 			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
+			OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")),
 			OPT_END(),
 		};
 		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
diff --git a/sequencer.c b/sequencer.c
index c94942a..951f51d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -634,6 +634,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..0cdf07e 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.9.2+fc1.2.gfbaae8c

[PATCH v7 03/12] 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                       | 11 +++++++----
 sequencer.h                       |  1 +
 5 files changed, 19 insertions(+), 6 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 2de67a5..2b51136 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 9946ecd..c8149ca 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -77,6 +77,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	const char *me = action_name(opts);
 	int cmd = 0;
 	struct option options[] = {
+		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
 		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
 		OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
 		OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
diff --git a/sequencer.c b/sequencer.c
index 951f51d..e8239ac 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -396,6 +396,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");
@@ -635,9 +637,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);
@@ -667,7 +670,7 @@ static void prepare_revs(struct replay_opts *opts)
 	if (prepare_revision_walk(opts->revs))
 		die(_("revision walk setup failed"));
 
-	if (!opts->revs->commits)
+	if (!opts->revs->commits && !opts->quiet)
 		error(_("empty commit set passed"));
 }
 
diff --git a/sequencer.h b/sequencer.h
index 3b04844..d37c003 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -35,6 +35,7 @@ struct replay_opts {
 	int allow_empty_message;
 	int keep_redundant_commits;
 	int skip_empty;
+	int quiet;
 
 	int mainline;
 
-- 
1.9.2+fc1.2.gfbaae8c

Re: [PATCH v7 03/12] revert/cherry-pick: add --quiet option

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> @@ -635,9 +637,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);

Personally, I don't see much value in inventing a new option for
suppressing one message.

Re: [PATCH v7 03/12] revert/cherry-pick: add --quiet option

[Felipe Contreras]

Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > @@ -635,9 +637,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);
> 
> Personally, I don't see much value in inventing a new option for
> suppressing one message.

It's not one message; it's two messages from cherry-pick itself, and all the
messages from `git commit`. Didn't we alread discuss this?

-- 
Felipe Contreras

[PATCH v7 04/12] 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                       | 24 ++++++++++++++++++++++++
 sequencer.h                       |  3 ++-
 t/t3510-cherry-pick-sequence.sh   | 12 ++++++++++++
 7 files changed, 49 insertions(+), 1 deletion(-)

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 2b51136..14bd299 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 c8149ca..54fba63 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -76,11 +76,13 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 	const char * const * usage_str = revert_or_cherry_pick_usage(opts);
 	const char *me = action_name(opts);
 	int cmd = 0;
+
 	struct option options[] = {
 		OPT__QUIET(&opts->quiet, N_("suppress progress reporting")),
 		OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'),
 		OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'),
 		OPT_CMDMODE(0, "abort", &cmd, N_("cancel revert or cherry-pick sequence"), 'a'),
+		OPT_CMDMODE(0, "skip", &cmd, N_("skip current commit in the sequence"), 's'),
 		OPT_BOOL('n', "no-commit", &opts->no_commit, N_("don't automatically commit")),
 		OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
 		OPT_NOOP_NOARG('r', NULL),
@@ -128,6 +130,8 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		opts->subcommand = REPLAY_CONTINUE;
 	else if (cmd == 'a')
 		opts->subcommand = REPLAY_ROLLBACK;
+	else if (cmd == 's')
+		opts->subcommand = REPLAY_SKIP;
 	else
 		opts->subcommand = REPLAY_NONE;
 
@@ -138,6 +142,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 e8239ac..c01ad08 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1057,6 +1057,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);
+}
+
 static int single_pick(struct commit *cmit, struct replay_opts *opts)
 {
 	setenv(GIT_REFLOG_ACTION, action_name(opts), 0);
@@ -1087,6 +1109,8 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 		return sequencer_rollback(opts);
 	if (opts->subcommand == REPLAY_CONTINUE)
 		return sequencer_continue(opts);
+	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 d37c003..74d592a 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -17,7 +17,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 33c5512..c43c327 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.9.2+fc1.2.gfbaae8c

Re: [PATCH v7 04/12] revert/cherry-pick: add --skip option

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> Akin to 'am --skip' and 'rebase --skip'.

I don't recall why my original sequencer series didn't include this
option. Perhaps Jonathan remembers?

[PATCH v7 05/12] cherry-pick: remember rerere-autoupdate

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sequencer.c                   | 10 +++++++++-
 t/t3504-cherry-pick-rerere.sh | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index c01ad08..a258627 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -820,7 +820,9 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
 	else if (!strcmp(key, "options.strategy-option")) {
 		ALLOC_GROW(opts->xopts, opts->xopts_nr + 1, opts->xopts_alloc);
 		opts->xopts[opts->xopts_nr++] = xstrdup(value);
-	} else
+	} else if (!strcmp(key, "options.allow-rerere-auto"))
+		opts->allow_rerere_auto = git_config_int(key, value);
+	else
 		return error(_("Invalid key: %s"), key);
 
 	if (!error_flag)
@@ -997,6 +999,12 @@ static void save_opts(struct replay_opts *opts)
 							"options.strategy-option",
 							opts->xopts[i], "^$", 0);
 	}
+	if (opts->allow_rerere_auto) {
+		struct strbuf buf = STRBUF_INIT;
+		strbuf_addf(&buf, "%d", opts->allow_rerere_auto);
+		git_config_set_in_file(opts_file, "options.allow-rerere-auto", buf.buf);
+		strbuf_release(&buf);
+	}
 }
 
 static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
index e6a6481..274b2cc 100755
--- a/t/t3504-cherry-pick-rerere.sh
+++ b/t/t3504-cherry-pick-rerere.sh
@@ -42,4 +42,43 @@ test_expect_success 'cherry-pick conflict without rerere' '
 	test_must_fail test_cmp expect foo
 '
 
+test_expect_success 'cherry-pick --rerere-autoupdate' '
+	test_when_finished "rm -rf rerere" &&
+	(
+	git init rerere &&
+	cd rerere &&
+	test_config rerere.enabled true &&
+	echo one > content-a && git add content-a &&
+	echo one > content-b && git add content-b &&
+	git commit -m one &&
+	git checkout -b conflict master &&
+	echo conflict-a > content-a &&
+	git commit -a -m conflict-a &&
+	echo conflict-b > content-b &&
+	git commit -a -m conflict-b &&
+	git checkout master &&
+	echo two > content-a &&
+	echo two > content-b &&
+	git commit -a -m two &&
+	git checkout -b test &&
+	test_must_fail git cherry-pick master..conflict &&
+	echo resolved-a > content-a &&
+	git add content-a &&
+	test_must_fail git cherry-pick --continue &&
+	echo resolved-b > content-b &&
+	git add content-b &&
+	git cherry-pick --continue &&
+	git reset --hard master
+	git log --oneline --all --decorate --graph &&
+	test_must_fail git cherry-pick --rerere-autoupdate master..conflict &&
+	git log --oneline --all --decorate --graph &&
+	echo resolved-a > expected &&
+	test_cmp expected content-a
+	test_must_fail git cherry-pick --continue &&
+	echo resolved-b > expected &&
+	test_cmp expected content-b &&
+	git cherry-pick --continue
+	)
+'
+
 test_done
-- 
1.9.2+fc1.2.gfbaae8c

Re: [PATCH v7 05/12] cherry-pick: remember rerere-autoupdate

[Eric Sunshine]

On Wed, Apr 23, 2014 at 10:44 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
> index e6a6481..274b2cc 100755
> --- a/t/t3504-cherry-pick-rerere.sh
> +++ b/t/t3504-cherry-pick-rerere.sh
> @@ -42,4 +42,43 @@ test_expect_success 'cherry-pick conflict without rerere' '
>         test_must_fail test_cmp expect foo
>  '
>
> +test_expect_success 'cherry-pick --rerere-autoupdate' '
> +       test_when_finished "rm -rf rerere" &&
> +       (
> +       git init rerere &&
> +       cd rerere &&
> +       test_config rerere.enabled true &&
> +       echo one > content-a && git add content-a &&
> +       echo one > content-b && git add content-b &&
> +       git commit -m one &&
> +       git checkout -b conflict master &&
> +       echo conflict-a > content-a &&
> +       git commit -a -m conflict-a &&
> +       echo conflict-b > content-b &&
> +       git commit -a -m conflict-b &&
> +       git checkout master &&
> +       echo two > content-a &&
> +       echo two > content-b &&
> +       git commit -a -m two &&
> +       git checkout -b test &&
> +       test_must_fail git cherry-pick master..conflict &&
> +       echo resolved-a > content-a &&
> +       git add content-a &&
> +       test_must_fail git cherry-pick --continue &&
> +       echo resolved-b > content-b &&
> +       git add content-b &&
> +       git cherry-pick --continue &&
> +       git reset --hard master

Broken &&-chain.

> +       git log --oneline --all --decorate --graph &&
> +       test_must_fail git cherry-pick --rerere-autoupdate master..conflict &&
> +       git log --oneline --all --decorate --graph &&
> +       echo resolved-a > expected &&
> +       test_cmp expected content-a

Ditto.

> +       test_must_fail git cherry-pick --continue &&
> +       echo resolved-b > expected &&
> +       test_cmp expected content-b &&
> +       git cherry-pick --continue
> +       )
> +'
> +
>  test_done
> --
> 1.9.2+fc1.2.gfbaae8c

[PATCH v7 06/12] builtin: add rewrite helper

[Felipe Contreras]

So that we can load and store rewrites, as well as other operations on a
list of rewritten commits.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Makefile  |  2 ++
 rewrite.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 rewrite.h | 18 ++++++++++++++++
 3 files changed, 91 insertions(+)
 create mode 100644 rewrite.c
 create mode 100644 rewrite.h

diff --git a/Makefile b/Makefile
index 33aa15b..d315b71 100644
--- a/Makefile
+++ b/Makefile
@@ -710,6 +710,7 @@ LIB_H += remote.h
 LIB_H += rerere.h
 LIB_H += resolve-undo.h
 LIB_H += revision.h
+LIB_H += rewrite.h
 LIB_H += run-command.h
 LIB_H += send-pack.h
 LIB_H += sequencer.h
@@ -855,6 +856,7 @@ LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
 LIB_OBJS += revision.o
+LIB_OBJS += rewrite.o
 LIB_OBJS += run-command.o
 LIB_OBJS += send-pack.o
 LIB_OBJS += sequencer.o
diff --git a/rewrite.c b/rewrite.c
new file mode 100644
index 0000000..436d54a
--- /dev/null
+++ b/rewrite.c
@@ -0,0 +1,71 @@
+#include "cache.h"
+#include "rewrite.h"
+
+void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to)
+{
+	struct rewritten_item *item;
+	ALLOC_GROW(list->items, list->nr + 1, list->alloc);
+	item = &list->items[list->nr];
+	hashcpy(item->from, from);
+	hashcpy(item->to, to);
+	list->nr++;
+}
+
+int store_rewritten(struct rewritten *list, const char *file)
+{
+	static struct lock_file lock;
+	struct strbuf buf = STRBUF_INIT;
+	int fd, i, ret = 0;
+
+	fd = hold_lock_file_for_update(&lock, file, LOCK_DIE_ON_ERROR);
+	for (i = 0; i < list->nr; i++) {
+		struct rewritten_item *item = &list->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) {
+		error(_("Could not write to %s"), file);
+		ret = 1;
+		goto leave;
+	}
+	if (commit_lock_file(&lock) < 0) {
+		error(_("Error wrapping up %s."), file);
+		ret = 1;
+		goto leave;
+	}
+leave:
+	strbuf_release(&buf);
+	return ret;
+}
+
+void load_rewritten(struct rewritten *list, const char *file)
+{
+	struct strbuf buf = STRBUF_INIT;
+	char *p;
+	int fd;
+
+	fd = open(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(list, from, to);
+		p = *eol ? eol + 1 : eol;
+	}
+	strbuf_release(&buf);
+}
diff --git a/rewrite.h b/rewrite.h
new file mode 100644
index 0000000..09e7222
--- /dev/null
+++ b/rewrite.h
@@ -0,0 +1,18 @@
+#ifndef REWRITE_H
+#define REWRITE_H
+
+struct rewritten_item {
+	unsigned char from[20];
+	unsigned char to[20];
+};
+
+struct rewritten {
+	struct rewritten_item *items;
+	unsigned int nr, alloc;
+};
+
+void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to);
+int store_rewritten(struct rewritten *list, const char *file);
+void load_rewritten(struct rewritten *list, const char *file);
+
+#endif
-- 
1.9.2+fc1.2.gfbaae8c

Re: [PATCH v7 06/12] builtin: add rewrite helper

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> So that we can load and store rewrites, as well as other operations on a
> list of rewritten commits.

Please elaborate. Explain why this code shouldn't go in sequencer.c.

Re: [PATCH v7 06/12] builtin: add rewrite helper

[Felipe Contreras]

Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > So that we can load and store rewrites, as well as other operations on a
> > list of rewritten commits.
> 
> Please elaborate. Explain why this code shouldn't go in sequencer.c.

Isn't it obvious? Because sequencer.c wouldn't be the only user.

-- 
Felipe Contreras

[PATCH v7 07/12] 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 | 22 +++++++++++++++++++++-
 sequencer.h |  1 +
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index a258627..426fddd 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -14,11 +14,13 @@
 #include "merge-recursive.h"
 #include "refs.h"
 #include "argv-array.h"
+#include "rewrite.h"
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+static struct rewritten rewritten;
 
 static int is_rfc2822_line(const char *buf, int len)
 {
@@ -651,6 +653,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(&rewritten, commit->object.sha1, to);
+	}
 leave:
 	free_message(&msg);
 	free(defmsg);
@@ -1021,8 +1031,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)
+				store_rewritten(&rewritten, git_path(SEQ_REWR_FILE));
 			return res;
+		}
 	}
 
 	/*
@@ -1051,6 +1064,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)
+		load_rewritten(&rewritten, git_path(SEQ_REWR_FILE));
 
 	/* Verify that the conflict has been resolved */
 	if (file_exists(git_path("CHERRY_PICK_HEAD")) ||
@@ -1061,6 +1076,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(&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 74d592a..efec1b5 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.9.2+fc1.2.gfbaae8c

[PATCH v7 08/12] cherry-pick: don't store skipped commit

[Felipe Contreras]

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

diff --git a/sequencer.c b/sequencer.c
index 426fddd..fc0dd04 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1056,7 +1056,7 @@ static int continue_single_pick(void)
 	return run_command_v_opt(argv, RUN_GIT_CMD);
 }
 
-static int sequencer_continue(struct replay_opts *opts)
+static int sequencer_continue(struct replay_opts *opts, int skip)
 {
 	struct commit_list *todo_list = NULL;
 
@@ -1076,7 +1076,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(&rewritten, todo_list->item->object.sha1, to);
@@ -1104,7 +1104,7 @@ static int sequencer_skip(struct replay_opts *opts)
 	if (ret)
 		return ret;
 
-	return sequencer_continue(opts);
+	return sequencer_continue(opts, 1);
 }
 
 static int single_pick(struct commit *cmit, struct replay_opts *opts)
@@ -1136,7 +1136,7 @@ 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);
 
-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 09/12] builtin: move run_rewrite_hook() to rewrite.c

[Felipe Contreras]

And use struct rewrite.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/commit.c | 38 +++++---------------------------------
 rewrite.c        | 32 ++++++++++++++++++++++++++++++++
 rewrite.h        |  1 +
 3 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 7d711ea..ea42f22 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -31,6 +31,7 @@
 #include "sequencer.h"
 #include "notes-utils.h"
 #include "mailmap.h"
+#include "rewrite.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [options] [--] <pathspec>..."),
@@ -1411,38 +1412,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);
 }
 
-static int run_rewrite_hook(const unsigned char *oldsha1,
-			    const unsigned char *newsha1)
-{
-	/* oldsha1 SP newsha1 LF NUL */
-	static char buf[2*40 + 3];
-	struct child_process proc;
-	const char *argv[3];
-	int code;
-	size_t n;
-
-	argv[0] = find_hook("post-rewrite");
-	if (!argv[0])
-		return 0;
-
-	argv[1] = "amend";
-	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 code;
-	n = snprintf(buf, sizeof(buf), "%s %s\n",
-		     sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
-	write_in_full(proc.in, buf, n);
-	close(proc.in);
-	return finish_command(&proc);
-}
-
 int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...)
 {
 	const char *hook_env[3] =  { NULL };
@@ -1693,13 +1662,16 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		struct notes_rewrite_cfg *cfg;
+		struct rewritten rewrite;
+		memset(&rewrite, 0, sizeof(rewrite));
 		cfg = init_copy_notes_for_rewrite("amend");
 		if (cfg) {
 			/* we are amending, so current_head is not NULL */
 			copy_note_for_rewrite(cfg, current_head->object.sha1, sha1);
 			finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
 		}
-		run_rewrite_hook(current_head->object.sha1, sha1);
+		add_rewritten(&rewrite, current_head->object.sha1, sha1);
+		run_rewrite_hook(&rewrite, "amend");
 	}
 	if (!quiet)
 		print_summary(prefix, sha1, !current_head);
diff --git a/rewrite.c b/rewrite.c
index 436d54a..56617ce 100644
--- a/rewrite.c
+++ b/rewrite.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "rewrite.h"
+#include "run-command.h"
 
 void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to)
 {
@@ -69,3 +70,34 @@ void load_rewritten(struct rewritten *list, const char *file)
 	}
 	strbuf_release(&buf);
 }
+
+int run_rewrite_hook(struct rewritten *list, 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 0;
+
+	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 code;
+	for (i = 0; i < list->nr; i++) {
+		struct rewritten_item *item = &list->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);
+	return finish_command(&proc);
+}
diff --git a/rewrite.h b/rewrite.h
index 09e7222..fd00e66 100644
--- a/rewrite.h
+++ b/rewrite.h
@@ -14,5 +14,6 @@ struct rewritten {
 void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to);
 int store_rewritten(struct rewritten *list, const char *file);
 void load_rewritten(struct rewritten *list, const char *file);
+int run_rewrite_hook(struct rewritten *list, const char *name);
 
 #endif
-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 10/12] builtin: rewrite: add copy_rewrite_notes()

[Felipe Contreras]

And use it on commit.c.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/commit.c |  8 +-------
 rewrite.c        | 18 ++++++++++++++++++
 rewrite.h        |  1 +
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index ea42f22..b5287d6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1661,16 +1661,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	rerere(0);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
-		struct notes_rewrite_cfg *cfg;
 		struct rewritten rewrite;
 		memset(&rewrite, 0, sizeof(rewrite));
-		cfg = init_copy_notes_for_rewrite("amend");
-		if (cfg) {
-			/* we are amending, so current_head is not NULL */
-			copy_note_for_rewrite(cfg, current_head->object.sha1, sha1);
-			finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
-		}
 		add_rewritten(&rewrite, current_head->object.sha1, sha1);
+		copy_rewrite_notes(&rewrite, "amend", "Notes added by 'git commit --amend'");
 		run_rewrite_hook(&rewrite, "amend");
 	}
 	if (!quiet)
diff --git a/rewrite.c b/rewrite.c
index 56617ce..59b8be7 100644
--- a/rewrite.c
+++ b/rewrite.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "rewrite.h"
 #include "run-command.h"
+#include "notes-utils.h"
 
 void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *to)
 {
@@ -101,3 +102,20 @@ int run_rewrite_hook(struct rewritten *list, const char *name)
 	close(proc.in);
 	return finish_command(&proc);
 }
+
+void copy_rewrite_notes(struct rewritten *list, const char *name, const char *msg)
+{
+	struct notes_rewrite_cfg *cfg;
+	int i;
+
+	cfg = init_copy_notes_for_rewrite(name);
+	if (!cfg)
+		return;
+
+	for (i = 0; i < list->nr; i++) {
+		struct rewritten_item *item = &list->items[i];
+		copy_note_for_rewrite(cfg, item->from, item->to);
+	}
+
+	finish_copy_notes_for_rewrite(cfg, msg);
+}
diff --git a/rewrite.h b/rewrite.h
index fd00e66..fdc7055 100644
--- a/rewrite.h
+++ b/rewrite.h
@@ -15,5 +15,6 @@ void add_rewritten(struct rewritten *list, unsigned char *from, unsigned char *t
 int store_rewritten(struct rewritten *list, const char *file);
 void load_rewritten(struct rewritten *list, const char *file);
 int run_rewrite_hook(struct rewritten *list, const char *name);
+void copy_rewrite_notes(struct rewritten *list, const char *name, const char *msg);
 
 #endif
-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 11/12] cherry-pick: add --action-name option

[Felipe Contreras]

So it can be used by other tools (e.g. git rebase), and the right action
is passed to the hooks and notes rewrite stuff.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/revert.c | 2 ++
 sequencer.c      | 4 ++++
 sequencer.h      | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/builtin/revert.c b/builtin/revert.c
index 54fba63..a24b689 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -99,6 +99,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) {
@@ -109,6 +110,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 			OPT_BOOL(0, "allow-empty-message", &opts->allow_empty_message, N_("allow commits with empty messages")),
 			OPT_BOOL(0, "keep-redundant-commits", &opts->keep_redundant_commits, N_("keep redundant, empty commits")),
 			OPT_BOOL(0, "skip-empty", &opts->skip_empty, N_("skip empty commits")),
+			OPT_STRING(0, "action-name", &opts->action_name, N_("name"), N_("action name")),
 			OPT_END(),
 		};
 		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
diff --git a/sequencer.c b/sequencer.c
index fc0dd04..ed59d75 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -832,6 +832,8 @@ static int populate_opts_cb(const char *key, const char *value, void *data)
 		opts->xopts[opts->xopts_nr++] = xstrdup(value);
 	} else if (!strcmp(key, "options.allow-rerere-auto"))
 		opts->allow_rerere_auto = git_config_int(key, value);
+	else if (!strcmp(key, "options.action-name"))
+		git_config_string(&opts->action_name, key, value);
 	else
 		return error(_("Invalid key: %s"), key);
 
@@ -1015,6 +1017,8 @@ static void save_opts(struct replay_opts *opts)
 		git_config_set_in_file(opts_file, "options.allow-rerere-auto", buf.buf);
 		strbuf_release(&buf);
 	}
+	if (opts->action_name)
+		git_config_set_in_file(opts_file, "options.action-name", opts->action_name);
 }
 
 static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
diff --git a/sequencer.h b/sequencer.h
index efec1b5..6dfffaa 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -48,6 +48,8 @@ struct replay_opts {
 
 	/* Only used by REPLAY_NONE */
 	struct rev_info *revs;
+
+	const char *action_name;
 };
 
 int sequencer_pick_revisions(struct replay_opts *opts);
-- 
1.9.2+fc1.2.gfbaae8c

[PATCH v7 12/12] cherry-pick: copy notes and run hooks

[Felipe Contreras]

If no action-name is specified, nothing is done.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/config.txt       |  9 ++++-----
 Documentation/githooks.txt     |  8 ++++----
 git-rebase--interactive.sh     |  4 ++--
 sequencer.c                    | 28 +++++++++++++++++++++++++++-
 t/t3512-revert-cherry-notes.sh | 18 ++++++++++++++++++
 t/t5407-post-rewrite-hook.sh   | 16 ++++++++++++++++
 6 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100755 t/t3512-revert-cherry-notes.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c26a7c8..c328d33 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1752,11 +1752,10 @@ GIT_NOTES_REF) is also implicitly added to the list of refs to be
 displayed.
 
 notes.rewrite.<command>::
-	When rewriting commits with <command> (currently `amend` or
-	`rebase`) and this variable is set to `true`, Git
-	automatically copies your notes from the original to the
-	rewritten commit.  Defaults to `true`, but see
-	"notes.rewriteRef" below.
+	When rewriting commits with <command> (currently `amend`, `rebase`, or
+	`cherry-pick`) and this variable is set to `true`, Git automatically
+	copies your notes from the original to the rewritten commit.  Defaults
+	to `true`, but see "notes.rewriteRef" below.
 
 notes.rewriteMode::
 	When copying notes during a rewrite (see the
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d954bf6..2402e7d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -352,10 +352,10 @@ post-rewrite
 ~~~~~~~~~~~~
 
 This hook is invoked by commands that rewrite commits (`git commit
---amend`, 'git-rebase'; currently 'git-filter-branch' does 'not' call
-it!).  Its first argument denotes the command it was invoked by:
-currently one of `amend` or `rebase`.  Further command-dependent
-arguments may be passed in the future.
+--amend`, `git rebase`, `git cherry-pick`; currently `git filter-branch` does
+'not' call it!).  Its first argument denotes the command it was invoked by
+(e.g. `rebase`).  Further command-dependent arguments may be passed in the
+future.
 
 The hook receives a list of the rewritten commits on stdin, in the
 format
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43631b4..fd085e1 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -248,7 +248,7 @@ pick_one () {
 
 	test -d "$rewritten" &&
 		pick_one_preserving_merges "$@" && return
-	output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
+	output eval git cherry-pick "--action-name ''" "$strategy_args" $empty_args $ff "$@"
 }
 
 pick_one_preserving_merges () {
@@ -359,7 +359,7 @@ pick_one_preserving_merges () {
 			echo "$sha1 $(git rev-parse HEAD^0)" >> "$rewritten_list"
 			;;
 		*)
-			output eval git cherry-pick "$strategy_args" "$@" ||
+			output eval git cherry-pick "--action-name ''" "$strategy_args" "$@" ||
 				die_with_patch $sha1 "Could not pick $sha1"
 			;;
 		esac
diff --git a/sequencer.c b/sequencer.c
index ed59d75..2eb6ce8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -22,6 +22,25 @@ const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 static struct rewritten rewritten;
 
+static void finish(struct replay_opts *opts)
+{
+	const char *name;
+	struct strbuf msg = STRBUF_INIT;
+
+	if (opts->action != REPLAY_PICK)
+		return;
+
+	name = opts->action_name ? opts->action_name : "cherry-pick";
+
+	if (!*name)
+		return;
+
+	strbuf_addf(&msg, "Notes added by 'git %s'", name);
+	copy_rewrite_notes(&rewritten, name, msg.buf);
+	run_rewrite_hook(&rewritten, name);
+	strbuf_release(&msg);
+}
+
 static int is_rfc2822_line(const char *buf, int len)
 {
 	int i;
@@ -1042,6 +1061,8 @@ static int pick_commits(struct commit_list *todo_list, struct replay_opts *opts)
 		}
 	}
 
+	finish(opts);
+
 	/*
 	 * Sequence of picks finished successfully; cleanup by
 	 * removing the .git/sequencer directory
@@ -1113,8 +1134,13 @@ static int sequencer_skip(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;
+	finish(opts);
+	return 0;
 }
 
 int sequencer_pick_revisions(struct replay_opts *opts)
diff --git a/t/t3512-revert-cherry-notes.sh b/t/t3512-revert-cherry-notes.sh
new file mode 100755
index 0000000..9e9e611
--- /dev/null
+++ b/t/t3512-revert-cherry-notes.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+
+test_description='test cherry-pick and revert notes'
+
+. ./test-lib.sh
+
+test_expect_success 'notes are rewritten' '
+	test_config notes.rewrite.cherry-pick true &&
+	test_config notes.rewriteRef "refs/notes/*" &&
+	test_commit n1 &&
+	test_commit n2 &&
+	git notes add -m "a note" n2 &&
+	git checkout n1 &&
+	git cherry-pick n2 &&
+	git notes show HEAD
+'
+
+test_done
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index ea2e0d4..53d7ade 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -53,6 +53,22 @@ test_expect_success 'git commit --amend --no-post-rewrite' '
 	test ! -f post-rewrite.data
 '
 
+test_expect_success 'git cherry-pick' '
+	git reset --hard D &&
+	clear_hook_input &&
+	git checkout A &&
+	test_must_fail git cherry-pick B..D &&
+	echo C > foo &&
+	git add foo &&
+	git cherry-pick --continue &&
+	echo cherry-pick >expected.args &&
+	cat >expected.data <<EOF &&
+$(git rev-parse C) $(git rev-parse HEAD^)
+$(git rev-parse D) $(git rev-parse HEAD)
+EOF
+	verify_hook_input
+'
+
 test_expect_success 'git rebase' '
 	git reset --hard D &&
 	clear_hook_input &&
-- 
1.9.2+fc1.2.gfbaae8c

Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> If no action-name is specified, nothing is done.

Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic?

Otherwise, I agree with the goal of making notes.rewrite.<command>
work for cherry-pick.

Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

[Felipe Contreras]

Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > If no action-name is specified, nothing is done.
> 
> Why? Is it because git-rebase implements its own notes-copy-on-rewrite logic?

Yes, and `git rebase` uses `git cherry-pick`.

-- 
Felipe Contreras

Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

[Ramkumar Ramachandra]

Felipe Contreras wrote:
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 43631b4..fd085e1 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -248,7 +248,7 @@ pick_one () {
>
>         test -d "$rewritten" &&
>                 pick_one_preserving_merges "$@" && return
> -       output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
> +       output eval git cherry-pick "--action-name ''" "$strategy_args" $empty_args $ff "$@"

Passing an empty action-name looks quite ugly. Is there a better way
to achieve this?

Re: [PATCH v7 12/12] cherry-pick: copy notes and run hooks

[Felipe Contreras]

Ramkumar Ramachandra wrote:
> Felipe Contreras wrote:
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 43631b4..fd085e1 100644
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -248,7 +248,7 @@ pick_one () {
> >
> >         test -d "$rewritten" &&
> >                 pick_one_preserving_merges "$@" && return
> > -       output eval git cherry-pick "$strategy_args" $empty_args $ff "$@"
> > +       output eval git cherry-pick "--action-name ''" "$strategy_args" $empty_args $ff "$@"
> 
> Passing an empty action-name looks quite ugly. Is there a better way
> to achieve this?

I want `git cherry-pick` to be able to do two things:

1) Omit the whole notes and hooks code
2) Specify a name other than "cherry-pick" to use for those

The --action-name argument achieves both.

-- 
Felipe Contreras