threads / patch / 23958

patch, 8 partsrevert: use run_command_v_opt() instead of execv_git_cmd()

Subject: [PATCH 2/8] revert: use run_command_v_opt() instead of execv_git_cmd()

## tl;dr

30 messages between May 31, 2010 and Jun 14, 2010. Diffs are folded; open one to read it.

replies: 29people: 7as markdown or json

Christian Couder· May 31, 2010, 19:42 UTC · lore

[PATCH 0/8] implement cherry-picking many commits

This a patch series to implement cherry-picking and reverting many commits instead of just one.

There is still no way to continue or abort the process when cherry-picking or reverting fails, but this can be implemented later.

Changes since the previous RFC series are the following:
- now use the equivalent of 'git rev-list --no-walk "$@"' to
enumerate the commits (suggested by Junio)
- added a patch to cleanup the code related to the -x option
(suggested by Ram)
- added a commit to change help_msg() and its callers
- added 2 documentation patches 
Christian Couder (8):
  revert: cleanup code for -x option
  revert: use run_command_v_opt() instead of execv_git_cmd()
  revert: refactor code into a do_pick_commit() function
  revert: change help_msg() to take no argument
  revert: allow cherry-picking more than one commit
  revert: add tests to check cherry-picking many commits
  Documentation/cherry-pick: describe passing more than one commit
  Documentation/revert: describe passing more than one commit
 Documentation/git-cherry-pick.txt   |   64 ++++++++++++++++-----
 Documentation/git-revert.txt        |   52 +++++++++++------
 builtin/revert.c                    |  109 ++++++++++++++++++++++-------------
 t/t3508-cherry-pick-many-commits.sh |   95 ++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+), 71 deletions(-)
 create mode 100755 t/t3508-cherry-pick-many-commits.sh
Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 1/8] revert: cleanup code for -x option

There was some dead code and option -x appeared in the short help message of git revert (when running "git revert -h") which was wrong.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)
Show changes to builtin/revert.c +1 −7
diff --git a/builtin/revert.c b/builtin/revert.c
index 7976b5a..5df0d69 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -58,7 +58,6 @@ static void parse_args(int argc, const char **argv)
 	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_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
 		OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
@@ -71,6 +70,7 @@ static void parse_args(int argc, const char **argv)
 
 	if (action == CHERRY_PICK) {
 		struct option cp_extra[] = {
+			OPT_BOOLEAN('x', NULL, &no_replay, "append commit name"),
 			OPT_BOOLEAN(0, "ff", &allow_ff, "allow fast-forward"),
 			OPT_END(),
 		};
@@ -379,10 +379,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	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 (allow_ff) {
 		if (signoff)
 			die("cherry-pick --ff cannot be used with --signoff");
@@ -546,14 +542,12 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	if (isatty(0))
 		edit = 1;
-	no_replay = 1;
 	action = REVERT;
 	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;
 	return revert_or_cherry_pick(argc, argv);
 }
-- 
1.7.1.361.g42de.dirty
Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

This is needed by the following commits, because we are going to cherry pick many commits instead of just one.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to builtin/revert.c +1 −1
diff --git a/builtin/revert.c b/builtin/revert.c
index 5df0d69..9085894 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -530,7 +530,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			args[i++] = defmsg;
 		}
 		args[i] = NULL;
-		return execv_git_cmd(args);
+		return run_command_v_opt(args, RUN_GIT_CMD);
 	}
 	free_message(&msg);
 	free(defmsg);
-- 
1.7.1.361.g42de.dirty
Jonathan Nieder· Jun 1, 2010, 04:01 UTC · re: Christian Couder · lore

Re: [PATCH 2/8] revert: use run_command_v_opt() instead of execv_git_cmd()

Christian Couder wrote:
Show 13 quoted lines
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 5df0d69..9085894 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -530,7 +530,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
>  			args[i++] = defmsg;
>  		}
>  		args[i] = NULL;
> -		return execv_git_cmd(args);
> +		return run_command_v_opt(args, RUN_GIT_CMD);
>  	}
>  	free_message(&msg);
>  	free(defmsg);

Doesn’t this leak msg and defmsg? Maybe it would make sense to free the in-core copy of the commit message before the if (!no_commit) block.

	int ret;
	...
	ret = 0;
	free_message(&msg);
	if (!no_commit)
		ret = commit_for_cherry_pick(signoff, edit, defmsg);
	free(defmsg);
	return ret;
Jonathan
Christian Couder· Jun 1, 2010, 04:33 UTC · re: Jonathan Nieder · lore

Re: [PATCH 2/8] revert: use run_command_v_opt() instead of execv_git_cmd()

On Tuesday 01 June 2010 06:01:15 Jonathan Nieder wrote:
Show 18 quoted lines
> Christian Couder wrote:
> > diff --git a/builtin/revert.c b/builtin/revert.c
> > index 5df0d69..9085894 100644
> > --- a/builtin/revert.c
> > +++ b/builtin/revert.c
> > @@ -530,7 +530,7 @@ static int revert_or_cherry_pick(int argc, const char
> > **argv) args[i++] = defmsg;
> >  		}
> >  		args[i] = NULL;
> > -		return execv_git_cmd(args);
> > +		return run_command_v_opt(args, RUN_GIT_CMD);
> >  	}
> >  	free_message(&msg);
> >  	free(defmsg);
> 
> Doesn’t this leak msg and defmsg?  Maybe it would make sense to free
> the in-core copy of the commit message before the if (!no_commit)
> block.
Yes, I will have a look at it.

Thanks, Christian.

Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 3/8] revert: refactor code into a do_pick_commit() function

This is needed because we are going to make it possible to cherry-pick many commits instead of just one in the following commits. And we will be able to do that by just calling do_pick_commit() once for each commit to cherry-pick.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |   44 +++++++++++++++++++++++++-------------------
 1 files changed, 25 insertions(+), 19 deletions(-)
Show changes to builtin/revert.c +25 −19
diff --git a/builtin/revert.c b/builtin/revert.c
index 9085894..070e02e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -365,7 +365,7 @@ static void do_recursive_merge(struct commit *base, struct commit *next,
 	fprintf(stderr, "Finished one %s.\n", me);
 }
 
-static int revert_or_cherry_pick(int argc, const char **argv)
+static int do_pick_commit()
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
@@ -374,24 +374,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	char *defmsg = NULL;
 	struct strbuf msgbuf = STRBUF_INIT;
 
-	git_config(git_default_config, NULL);
-	me = action == REVERT ? "revert" : "cherry-pick";
-	setenv(GIT_REFLOG_ACTION, me, 0);
-	parse_args(argc, argv);
-
-	if (allow_ff) {
-		if (signoff)
-			die("cherry-pick --ff cannot be used with --signoff");
-		if (no_commit)
-			die("cherry-pick --ff cannot be used with --no-commit");
-		if (no_replay)
-			die("cherry-pick --ff cannot be used with -x");
-		if (edit)
-			die("cherry-pick --ff cannot be used with --edit");
-	}
-
-	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
@@ -538,6 +520,30 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	return 0;
 }
 
+static int revert_or_cherry_pick(int argc, const char **argv)
+{
+	git_config(git_default_config, NULL);
+	me = action == REVERT ? "revert" : "cherry-pick";
+	setenv(GIT_REFLOG_ACTION, me, 0);
+	parse_args(argc, argv);
+
+	if (allow_ff) {
+		if (signoff)
+			die("cherry-pick --ff cannot be used with --signoff");
+		if (no_commit)
+			die("cherry-pick --ff cannot be used with --no-commit");
+		if (no_replay)
+			die("cherry-pick --ff cannot be used with -x");
+		if (edit)
+			die("cherry-pick --ff cannot be used with --edit");
+	}
+
+	if (read_cache() < 0)
+		die("git %s: failed to read the index", me);
+
+	return do_pick_commit();
+}
+
 int cmd_revert(int argc, const char **argv, const char *prefix)
 {
 	if (isatty(0))
-- 
1.7.1.361.g42de.dirty
Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 4/8] revert: change help_msg() to take no argument

This is needed because the following commits will make it possible to cherry-pick many commits instead of just one.

So it will be possible to pass for example ranges of commits to "git cherry-pick" and this means that it will not be possible to use the arguments passed to "git cherry-pick" in the help message.

The help message will have to use the sha1 of the currently processed commit.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to builtin/revert.c +4 −4
diff --git a/builtin/revert.c b/builtin/revert.c
index 070e02e..f0d78e5 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -239,7 +239,7 @@ static void set_author_ident_env(const char *message)
 			sha1_to_hex(commit->object.sha1));
 }
 
-static char *help_msg(const char *name)
+static char *help_msg(void)
 {
 	struct strbuf helpbuf = STRBUF_INIT;
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
@@ -255,7 +255,7 @@ static char *help_msg(const char *name)
 		strbuf_addf(&helpbuf, " with: \n"
 			"\n"
 			"        git commit -c %s\n",
-			name);
+			    sha1_to_hex(commit->object.sha1));
 	}
 	else
 		strbuf_addch(&helpbuf, '.');
@@ -357,7 +357,7 @@ static void do_recursive_merge(struct commit *base, struct commit *next,
 		}
 		write_message(msgbuf, defmsg);
 		fprintf(stderr, "Automatic %s failed.%s\n",
-			me, help_msg(commit_name));
+			me, help_msg());
 		rerere(allow_rerere_auto);
 		exit(1);
 	}
@@ -484,7 +484,7 @@ static int do_pick_commit()
 		free_commit_list(remotes);
 		if (res) {
 			fprintf(stderr, "Automatic %s with strategy %s failed.%s\n",
-				me, strategy, help_msg(commit_name));
+				me, strategy, help_msg());
 			rerere(allow_rerere_auto);
 			exit(1);
 		}
-- 
1.7.1.361.g42de.dirty
Jonathan Nieder· Jun 1, 2010, 05:08 UTC · re: Christian Couder · lore

Re: [PATCH 4/8] revert: change help_msg() to take no argument

Christian Couder wrote:
Show 19 quoted lines
> +++ b/builtin/revert.c
> @@ -239,7 +239,7 @@ static void set_author_ident_env(const char *message)
>  			sha1_to_hex(commit->object.sha1));
>  }
>  
> -static char *help_msg(const char *name)
> +static char *help_msg(void)
>  {
>  	struct strbuf helpbuf = STRBUF_INIT;
>  	char *msg = getenv("GIT_CHERRY_PICK_HELP");
> @@ -255,7 +255,7 @@ static char *help_msg(const char *name)
>  		strbuf_addf(&helpbuf, " with: \n"
>  			"\n"
>  			"        git commit -c %s\n",
> -			name);
> +			    sha1_to_hex(commit->object.sha1));
>  	}
>  	else
>  		strbuf_addch(&helpbuf, '.');
producing a message like
  Automatic cherry-pick failed.
    After resolving the conflicts,
  mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
  and commit the result with: 
  	git commit -c 8a7cdf

Is there any reason not to suggest ‘git commit’ without the -c? This way, the template message includes a helpful Conflicts: string, too.

---
 builtin/revert.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to builtin/revert.c +1 −2
diff --git a/builtin/revert.c b/builtin/revert.c
index f0d78e5..bbafc41 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -254,8 +254,7 @@ static char *help_msg(void)
 	if (action == CHERRY_PICK) {
 		strbuf_addf(&helpbuf, " with: \n"
 			"\n"
-			"        git commit -c %s\n",
-			    sha1_to_hex(commit->object.sha1));
+			"        git commit\n");
 	}
 	else
 		strbuf_addch(&helpbuf, '.');
-- 
1.7.1
Jeff King· Jun 1, 2010, 05:40 UTC · re: Jonathan Nieder · lore

Re: [PATCH 4/8] revert: change help_msg() to take no argument

On Tue, Jun 01, 2010 at 12:08:15AM -0500, Jonathan Nieder wrote:
Show 9 quoted lines
>   Automatic cherry-pick failed.
>     After resolving the conflicts,
>   mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
>   and commit the result with: 
> 
>   	git commit -c 8a7cdf
> 
> Is there any reason not to suggest ‘git commit’ without the -c?  This
> way, the template message includes a helpful Conflicts: string, too.

You cc'd me, which I guess means you git-blame'd the line in question. But you really need to parent-blame about five steps back to find f52463a (cherry-pick: Suggest a better method to retain authorship, 2007-03-04) from Dscho, which introduced the "commit -c" suggestion.

So the answer to your question is that "-c" will retain the proper authorship of the cherry-picked commit. We could instead:

  1. Say only "git commit" when author == committer.
  2. When author and committer do not match, explicitly say "git commit
     --author=...". This retains the "conflicts" information from the
     template.

Those are both easy. Alternatively, we could actually make it stash the original authorship information somewhere (in addition to the commit message template) and then pull it out automatically. That's harder, but probably what the user would want (and it behaves more like a rebase conflict).

-Peff
Jonathan Nieder· Jun 1, 2010, 06:27 UTC · re: Jeff King · lore

Re: [PATCH 4/8] revert: change help_msg() to take no argument

Jeff King wrote:
> You cc'd me, which I guess means you git-blame'd the line in question.
> But you really need to parent-blame about five steps back

Worse, I used ‘git log -- builtin-revert.c’ and stopped reading when it seemed you must know be current maintainer for that line. Sorry about that.

> we could actually make it stash the
> original authorship information somewhere (in addition to the commit
> message template) and then pull it out automatically.
I think that can wait for cherry-pick --continue.

Here’s an ugly patch to use --author. I call it ugly because the relevant part of t3507-cherry-pick-conflict.sh output looks like this:

| Automatic cherry-pick failed.  After resolving the conflicts,
| mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
| and commit the result with: 
| 
|         git commit --author='A U Thor <author@example.com>' \
|                     --date='1112912113 -0700'
In other words, the command is long and the date human-unfriendly.

Anyway, given that cherry-pick --continue is just around the corner, I withdraw my complaint about the ‘commit -c’ version.

Thanks for the pointer.
Show changes to builtin/revert.c +8 −1
diff --git a/builtin/revert.c b/builtin/revert.c
index bbafc41..9b8e7d6 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -252,9 +252,16 @@ static char *help_msg(void)
 		"and commit the result");
 
 	if (action == CHERRY_PICK) {
+		const char *ident = git_author_info(IDENT_ERROR_ON_NO_NAME);
+		const char *ident_end = ident ? strchr(ident, '>') : NULL;
+		if (!ident_end || ident_end[1] != ' ')
+			die("git_author_info returned nonsense");
 		strbuf_addf(&helpbuf, " with: \n"
 			"\n"
-			"        git commit\n");
+			"        git commit --author='%.*s' \\\n"
+			"                    --date='%s'\n",
+			(int) (ident_end + 1 - ident), ident,
+			ident_end + 2);
 	}
 	else
 		strbuf_addch(&helpbuf, '.');
Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 5/8] revert: allow cherry-picking more than one commit

This makes it possible to pass many commits or ranges of commits to "git cherry-pick" and to "git revert" to process many commits instead of just one.

In fact commits are now enumerated with an equivalent of
	git rev-list --no-walk "$@"
so all the following are now possible:
	git cherry-pick master~2..master
	git cherry-pick ^master~2 master
	git cherry-pick master^ master
The following should be possible but does not work:
	git cherry-pick -2 master

because "git rev-list --no-walk -2 master" only outputs one commit as "--no-walk" seems to take over "-2".

And there is currently no way to continue cherry-picking or reverting if there is a problem with one commit. It's also not possible to abort the whole process. Some future work should provide the --continue and --abort options to do just that.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |   49 +++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 39 insertions(+), 10 deletions(-)
Show changes to builtin/revert.c +39 −10
diff --git a/builtin/revert.c b/builtin/revert.c
index f0d78e5..b90955f 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -39,7 +39,8 @@ static const char * const cherry_pick_usage[] = {
 static int edit, no_replay, no_commit, mainline, signoff, allow_ff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
-static const char *commit_name;
+static int commit_argc;
+static const char **commit_argv;
 static int allow_rerere_auto;
 
 static const char *me;
@@ -53,7 +54,6 @@ static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =
 		action == REVERT ?  revert_usage : cherry_pick_usage;
-	unsigned char sha1[20];
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
@@ -78,15 +78,11 @@ static void parse_args(int argc, const char **argv)
 			die("program error");
 	}
 
-	if (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)
+	commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0);
+	if (commit_argc < 1)
 		usage_with_options(usage_str, options);
 
-	commit_name = argv[0];
-	if (get_sha1(commit_name, sha1))
-		die ("Cannot find '%s'", commit_name);
-	commit = lookup_commit_reference(sha1);
-	if (!commit)
-		exit(1);
+	commit_argv = argv;
 }
 
 struct commit_message {
@@ -520,8 +516,33 @@ static int do_pick_commit()
 	return 0;
 }
 
+static void prepare_revs(struct rev_info *revs)
+{
+	int argc = 0;
+	int i;
+	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
+
+	argv[argc++] = NULL;
+	argv[argc++] = "--no-walk";
+	if (action != REVERT)
+		argv[argc++] = "--reverse";
+	for (i = 0; i < commit_argc; i++)
+		argv[argc++] = commit_argv[i];
+	argv[argc++] = NULL;
+
+	init_revisions(revs, NULL);
+	setup_revisions(argc - 1, argv, revs, NULL);
+	if (prepare_revision_walk(revs))
+		die("revision walk setup failed");
+
+	if (!revs->commits)
+		die("empty commit set passed");
+}
+
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
+	struct rev_info revs;
+
 	git_config(git_default_config, NULL);
 	me = action == REVERT ? "revert" : "cherry-pick";
 	setenv(GIT_REFLOG_ACTION, me, 0);
@@ -541,7 +562,15 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	if (read_cache() < 0)
 		die("git %s: failed to read the index", me);
 
-	return do_pick_commit();
+	prepare_revs(&revs);
+
+	while ((commit = get_revision(&revs))) {
+		int res = do_pick_commit();
+		if (res)
+			return res;
+	}
+
+	return 0;
 }
 
 int cmd_revert(int argc, const char **argv, const char *prefix)
-- 
1.7.1.361.g42de.dirty
Sverre Rabbelier· Jun 1, 2010, 07:38 UTC · re: Christian Couder · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

Heya,
On Mon, May 31, 2010 at 21:42, Christian Couder <chriscool@tuxfamily.org> wrote:
> because "git rev-list --no-walk -2 master" only outputs
> one commit as "--no-walk" seems to take over "-2".

I don't understand this, what do you mean with "take over"? Is this a bug/missing feature in rev-list?

-- 
Cheers,

Sverre Rabbelier
Jonathan Nieder· Jun 1, 2010, 08:35 UTC · re: Christian Couder · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

Christian Couder wrote:
Show 6 quoted lines
> The following should be possible but does not work:
> 
> 	git cherry-pick -2 master
> 
> because "git rev-list --no-walk -2 master" only outputs
> one commit as "--no-walk" seems to take over "-2".
Are the semantics of --no-walk documented anywhere?

In the spirit of v1.6.4-rc1~3 (Make 'git show' more useful, 2009-07-13), we could make -n imply --do-walk.

This would change the meaning of git show -5 --all, so I am not too happy with it.

-- 8< --
Subject: DWIM 'git show -5' to 'git show --do-walk -5'
To show the last two commits with one command, one might try
 1) git show -s master~2..
 2) git show -s ^master~2 master
 3) git show -s master^ master
 4) git show -s -2 master

Choice (3) works because both commits are listed on the command line. Choices (1) and (2) have worked ever since v1.6.4-rc~3 (Make 'git show' more useful, 2009-07-13) disabled --no-walk in this case because there is no other useful meaning for them to have. Unfortunately, (4) does not work: it outputs only one commit, because --no-walk stays on.

So disable --no-walk in this case so ‘git show’ and future ‘git cherry-pick’ can behave as expected.

As a side effect, this unfortunately changes the meaning of ‘git log --oneline --decorate --no-walk -5 --all’: instead of listing five refs, after this patch that command would list the five most recent commits.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 revision.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Show changes to revision.c +4 −0
diff --git a/revision.c b/revision.c
index f4b8b38..7b881a8 100644
--- a/revision.c
+++ b/revision.c
@@ -1063,18 +1063,22 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 
 	if (!prefixcmp(arg, "--max-count=")) {
 		revs->max_count = atoi(arg + 12);
+		revs->no_walk = 0;
 	} else if (!prefixcmp(arg, "--skip=")) {
 		revs->skip_count = atoi(arg + 7);
 	} else if ((*arg == '-') && isdigit(arg[1])) {
 	/* accept -<digit>, like traditional "head" */
 		revs->max_count = atoi(arg + 1);
+		revs->no_walk = 0;
 	} else if (!strcmp(arg, "-n")) {
 		if (argc <= 1)
 			return error("-n requires an argument");
 		revs->max_count = atoi(argv[1]);
+		revs->no_walk = 0;
 		return 2;
 	} else if (!prefixcmp(arg, "-n")) {
 		revs->max_count = atoi(arg + 2);
+		revs->no_walk = 0;
 	} else if (!prefixcmp(arg, "--max-age=")) {
 		revs->max_age = atoi(arg + 10);
 	} else if (!prefixcmp(arg, "--since=")) {
-- 
1.7.1.246.g2fc1b.dirty
Junio C Hamano· Jun 2, 2010, 23:37 UTC · re: Jonathan Nieder · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

Jonathan Nieder <jrnieder@gmail.com> writes:
> As a side effect, this unfortunately changes the meaning of
> ‘git log --oneline --decorate --no-walk -5 --all’: instead of listing
> five refs, after this patch that command would list the five most
> recent commits.

It indeed is a change in behaviour, but in this case it is probably not such a big deal.

The order "--all" (or "--branches" for that matter) lists refs doesn't have any logical relationship with the order of importance from the end user's point of view (they're just alphabetical), and asking for "first five" won't make much sense unless you are learning or testing the combinations of options, i.e. not in real life, I think.

Thanks.
Christian Couder· Jun 3, 2010, 04:18 UTC · re: Junio C Hamano · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

On Thursday 03 June 2010 01:37:20 Junio C Hamano wrote:
Show 14 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
> > As a side effect, this unfortunately changes the meaning of
> > ‘git log --oneline --decorate --no-walk -5 --all’: instead of listing
> > five refs, after this patch that command would list the five most
> > recent commits.
> 
> It indeed is a change in behaviour, but in this case it is probably not
> such a big deal.
>
> The order "--all" (or "--branches" for that matter) lists refs doesn't
> have any logical relationship with the order of importance from the end
> user's point of view (they're just alphabetical), and asking for "first
> five" won't make much sense unless you are learning or testing the
> combinations of options, i.e. not in real life, I think.
I agree and I think Jonathan's patch would be nice to have.

Thanks both, Christian.

Jonathan Nieder· Jun 1, 2010, 09:03 UTC · re: Christian Couder · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

Christian Couder wrote:
Show 28 quoted lines
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -520,8 +516,33 @@ static int do_pick_commit()
>  	return 0;
>  }
>  
> +static void prepare_revs(struct rev_info *revs)
> +{
> +	int argc = 0;
> +	int i;
> +	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
> +
> +	argv[argc++] = NULL;
> +	argv[argc++] = "--no-walk";
> +	if (action != REVERT)
> +		argv[argc++] = "--reverse";
> +	for (i = 0; i < commit_argc; i++)
> +		argv[argc++] = commit_argv[i];
> +	argv[argc++] = NULL;
> +
> +	init_revisions(revs, NULL);
> +	setup_revisions(argc - 1, argv, revs, NULL);
> +	if (prepare_revision_walk(revs))
> +		die("revision walk setup failed");
> +
> +	if (!revs->commits)
> +		die("empty commit set passed");
> +}

Tiny one-time leak. Maybe avoiding it will make debugging tools happier.

The rest of the patch of course looks good.  Thanks.
Show changes to builtin/revert.c +1 −0
diff --git a/builtin/revert.c b/builtin/revert.c
index b90955f..a73f66b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -532,6 +532,7 @@ static void prepare_revs(struct rev_info *revs)
 
 	init_revisions(revs, NULL);
 	setup_revisions(argc - 1, argv, revs, NULL);
+	free(argv);
 	if (prepare_revision_walk(revs))
 		die("revision walk setup failed");
 
Christian Couder· Jun 2, 2010, 05:57 UTC · re: Jonathan Nieder · lore

Re: [PATCH 5/8] revert: allow cherry-picking more than one commit

On Tuesday 01 June 2010 11:03:17 Jonathan Nieder wrote:
Show 32 quoted lines
> Christian Couder wrote:
> > --- a/builtin/revert.c
> > +++ b/builtin/revert.c
> > @@ -520,8 +516,33 @@ static int do_pick_commit()
> >  	return 0;
> >  }
> >
> > +static void prepare_revs(struct rev_info *revs)
> > +{
> > +	int argc = 0;
> > +	int i;
> > +	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
> > +
> > +	argv[argc++] = NULL;
> > +	argv[argc++] = "--no-walk";
> > +	if (action != REVERT)
> > +		argv[argc++] = "--reverse";
> > +	for (i = 0; i < commit_argc; i++)
> > +		argv[argc++] = commit_argv[i];
> > +	argv[argc++] = NULL;
> > +
> > +	init_revisions(revs, NULL);
> > +	setup_revisions(argc - 1, argv, revs, NULL);
> > +	if (prepare_revision_walk(revs))
> > +		die("revision walk setup failed");
> > +
> > +	if (!revs->commits)
> > +		die("empty commit set passed");
> > +}
> 
> Tiny one-time leak.  Maybe avoiding it will make debugging tools
> happier.
I added a "free(argv)".

Thanks, Christian.

Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 6/8] revert: add tests to check cherry-picking many commits

Note that there is an expected failure when running:
	git cherry-pick -3 fourth
that's because:
	git rev-list --no-walk -3 fourth

produce only one commit and not 3 as "--no-walk" seems to take over "-3".

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t3508-cherry-pick-many-commits.sh |   95 +++++++++++++++++++++++++++++++++++
 1 files changed, 95 insertions(+), 0 deletions(-)
 create mode 100755 t/t3508-cherry-pick-many-commits.sh
Show changes to t/t3508-cherry-pick-many-commits.sh +95 −0
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
new file mode 100755
index 0000000..3b87efe
--- /dev/null
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -0,0 +1,95 @@
+#!/bin/sh
+
+test_description='test cherry-picking many commits'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo first > file1 &&
+	git add file1 &&
+	test_tick &&
+	git commit -m "first" &&
+	git tag first &&
+
+	git checkout -b other &&
+	for val in second third fourth
+	do
+		echo $val >> file1 &&
+		git add file1 &&
+		test_tick &&
+		git commit -m "$val" &&
+		git tag $val
+	done
+'
+
+test_expect_success 'cherry-pick first..fourth works' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick first..fourth &&
+	git diff --quiet other &&
+	git diff --quiet HEAD other &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+'
+
+test_expect_success 'cherry-pick --ff first..fourth works' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick --ff first..fourth &&
+	git diff --quiet other &&
+	git diff --quiet HEAD other &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify fourth)"
+'
+
+test_expect_success 'cherry-pick -n first..fourth works' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick -n first..fourth &&
+	git diff --quiet other &&
+	git diff --cached --quiet other &&
+	git diff --quiet HEAD first
+'
+
+test_expect_success 'revert first..fourth works' '
+	git checkout master &&
+	git reset --hard fourth &&
+	test_tick &&
+	git revert first..fourth &&
+	git diff --quiet first &&
+	git diff --cached --quiet first &&
+	git diff --quiet HEAD first
+'
+
+test_expect_success 'revert ^first fourth works' '
+	git checkout master &&
+	git reset --hard fourth &&
+	test_tick &&
+	git revert ^first fourth &&
+	git diff --quiet first &&
+	git diff --cached --quiet first &&
+	git diff --quiet HEAD first
+'
+
+test_expect_success 'revert fourth fourth~1 fourth~2 works' '
+	git checkout master &&
+	git reset --hard fourth &&
+	test_tick &&
+	git revert fourth fourth~1 fourth~2 &&
+	git diff --quiet first &&
+	git diff --cached --quiet first &&
+	git diff --quiet HEAD first
+'
+
+test_expect_failure 'cherry-pick -3 fourth works' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick -3 fourth &&
+	git diff --quiet other &&
+	git diff --quiet HEAD other &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
+'
+
+test_done
-- 
1.7.1.361.g42de.dirty
Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

And while at it, add an "Examples" section.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-cherry-pick.txt |   64 +++++++++++++++++++++++++++++--------
 1 files changed, 50 insertions(+), 14 deletions(-)
Show changes to Documentation/git-cherry-pick.txt +50 −14
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d71607a..df2742a 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -3,24 +3,29 @@ git-cherry-pick(1)
 
 NAME
 ----
-git-cherry-pick - Apply the change introduced by an existing commit
+git-cherry-pick - Apply the change introduced by some existing commits
 
 SYNOPSIS
 --------
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>
+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>...
 
 DESCRIPTION
 -----------
-Given one existing commit, apply the change the patch introduces, and record a
-new commit that records it.  This requires your working tree to be clean (no
-modifications from the HEAD commit).
+
+Given one or more existing commits, apply the changes that the related
+patches introduce, and record some new commits that record them.  This
+requires your working tree to be clean (no modifications from the HEAD
+commit).
 
 OPTIONS
 -------
-<commit>::
-	Commit to cherry-pick.
+<commit>...::
+	Commits to cherry-pick.
 	For a more complete list of ways to spell commits, see the
 	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
+	Sets of commits can also be given but no traversal is done
+	by default, see linkgit:git-rev-list[1] and its '--no-walk'
+	option.
 
 -e::
 --edit::
@@ -55,13 +60,12 @@ OPTIONS
 
 -n::
 --no-commit::
-	Usually the command automatically creates a commit.
-	This flag applies the change necessary to cherry-pick
-	the named commit to your working tree and the index,
-	but does not make the commit.  In addition, when this
-	option is used, your index does not have to match the
-	HEAD commit.  The cherry-pick is done against the
-	beginning state of your index.
+	Usually the command automatically creates some commits.  This
+	flag applies the change necessary to cherry-pick the named
+	commits to your working tree and the index, but does not make
+	the commits.  In addition, when this option is used, your
+	index does not have to match the HEAD commit.  The cherry-pick
+	is done against the beginning state of your index.
 +
 This is useful when cherry-picking more than one commits'
 effect to your index in a row.
@@ -75,6 +79,38 @@ effect to your index in a row.
 	cherry-pick'ed commit, then a fast forward to this commit will
 	be performed.
 
+Examples
+--------
+git cherry-pick master::
+
+	Apply the changes specified by the commit pointed to by master
+	and create a new commit with these changes.
+
+git cherry-pick master~3..master::
+git cherry-pick ^master~3 master::
+
+	Apply the changes specified by the last 3 commits pointed to
+	by master and create 3 new commits with these changes.
+
+git cherry-pick master\~3 master~2::
+
+	Apply the changes specified by the fourth and third last
+	commits pointed to by master and create 2 new commits with
+	these changes.
+
+git cherry-pick -n master~1 next::
+
+	Apply to the working tree and the index the changes specified
+	by the second last commit pointed to by master and by the last
+	commit pointed to by next, but do not create any commit with
+	these changes.
+
+git cherry-pick --ff ..next::
+
+	If possible fast forward to the existing commits already in
+	next but not in HEAD, otherwise apply the changes specified by
+	these commits and create new commits with these changes.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com>
-- 
1.7.1.361.g42de.dirty
Ramkumar Ramachandra· Jun 1, 2010, 09:29 UTC · re: Christian Couder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

Hi,
Christian Couder <chriscool@tuxfamily.org> wrote:
Show 16 quoted lines
>  -n::
>  --no-commit::
> -       Usually the command automatically creates a commit.
> -       This flag applies the change necessary to cherry-pick
> -       the named commit to your working tree and the index,
> -       but does not make the commit.  In addition, when this
> -       option is used, your index does not have to match the
> -       HEAD commit.  The cherry-pick is done against the
> -       beginning state of your index.
> +       Usually the command automatically creates some commits.  This
> +       flag applies the change necessary to cherry-pick the named
> +       commits to your working tree and the index, but does not make
> +       the commits.  In addition, when this option is used, your
> +       index does not have to match the HEAD commit.  The cherry-pick
> +       is done against the beginning state of your index.
>  +

Avoid re-wrapping the text so it's easy to see the changes that you have introduced in subsequent blames- you've just changed the word "commit" to "commits".

> +Examples
> +--------
Style nitpick: shouldn't the word "Examples" be in all-caps?
-- Ram
Christian Couder· Jun 2, 2010, 05:57 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

On Tuesday 01 June 2010 11:29:32 Ramkumar Ramachandra wrote:
Show 23 quoted lines
> Hi,
> 
> Christian Couder <chriscool@tuxfamily.org> wrote:
> >  -n::
> >  --no-commit::
> > -       Usually the command automatically creates a commit.
> > -       This flag applies the change necessary to cherry-pick
> > -       the named commit to your working tree and the index,
> > -       but does not make the commit.  In addition, when this
> > -       option is used, your index does not have to match the
> > -       HEAD commit.  The cherry-pick is done against the
> > -       beginning state of your index.
> > +       Usually the command automatically creates some commits.  This
> > +       flag applies the change necessary to cherry-pick the named
> > +       commits to your working tree and the index, but does not make
> > +       the commits.  In addition, when this option is used, your
> > +       index does not have to match the HEAD commit.  The cherry-pick
> > +       is done against the beginning state of your index.
> >  +
> 
> Avoid re-wrapping the text so it's easy to see the changes that you
> have introduced in subsequent blames- you've just changed the word
> "commit" to "commits".
Ok, avoided the re-wrapping.
> > +Examples
> > +--------
> 
> Style nitpick: shouldn't the word "Examples" be in all-caps?

I had a quick look at Documentation/*.txt and found 22 files with "Examples" and 32 files with "EXAMPLES", so I changed to "EXAMPLES". But if you really care I suggest you to submit a patch to changes the 22 files that contains "EXAMPLES".

Thanks, Christian.

Jonathan Nieder· Jun 1, 2010, 09:51 UTC · re: Christian Couder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

Christian Couder wrote:
Show 8 quoted lines
> --- a/Documentation/git-cherry-pick.txt
> +++ b/Documentation/git-cherry-pick.txt
> @@ -3,24 +3,29 @@ git-cherry-pick(1)
>  
>  NAME
>  ----
> -git-cherry-pick - Apply the change introduced by an existing commit
> +git-cherry-pick - Apply the change introduced by some existing commits
Maybe s/change/changes/.
Show 10 quoted lines
>  DESCRIPTION
>  -----------
> -Given one existing commit, apply the change the patch introduces, and record a
> -new commit that records it.  This requires your working tree to be clean (no
> -modifications from the HEAD commit).
> +
> +Given one or more existing commits, apply the changes that the related
> +patches introduce, and record some new commits that record them.  This
> +requires your working tree to be clean (no modifications from the HEAD
> +commit).

"Related" made me think "related how"? "Record some new commits" sounds oddly vague.

Maybe:
	Given one or more existing commits, apply the change each one
	introduces, recording a new commit for each.  This requires
	your working tree to be clean (no modifications from the HEAD
	commit).
Show 11 quoted lines
>  OPTIONS
>  -------
> -<commit>::
> -	Commit to cherry-pick.
> +<commit>...::
> +	Commits to cherry-pick.
>  	For a more complete list of ways to spell commits, see the
>  	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
> +	Sets of commits can also be given but no traversal is done
> +	by default, see linkgit:git-rev-list[1] and its '--no-walk'
> +	option.

--no-walk can be a bit confusing. I think we should try to avoid relying on the reader understanding it.

 <commit>...::
	Commits to cherry-pick.
	For a more complete list of ways to spell commits, see the
	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
	Revision ranges are interpreted as though specified with
	the `--no-walk` option; see the "Examples" section below.
Show 19 quoted lines
>  -e::
>  --edit::
> @@ -55,13 +60,12 @@ OPTIONS
>  
>  -n::
>  --no-commit::
> -	Usually the command automatically creates a commit.
> -	This flag applies the change necessary to cherry-pick
> -	the named commit to your working tree and the index,
> -	but does not make the commit.  In addition, when this
> -	option is used, your index does not have to match the
> -	HEAD commit.  The cherry-pick is done against the
> -	beginning state of your index.
> +	Usually the command automatically creates some commits.  This
> +	flag applies the change necessary to cherry-pick the named
> +	commits to your working tree and the index, but does not make
> +	the commits.  In addition, when this option is used, your
> +	index does not have to match the HEAD commit.  The cherry-pick
> +	is done against the beginning state of your index.
This switches between singular and plural.
 -n::
 --no-commit::
	Usually 'git cherry-pick' automatically creates a sequence
	of commits.  This option can be used to apply the changes
	necessary to cherry-pick each named commit to your working
	tree and index without making any commits.  In addition,
	when this option is used, your index does not have to match
	the HEAD commit: the cherry-pick is done against the
	beginning state of the index.
>  +
>  This is useful when cherry-picking more than one commits'
>  effect to your index in a row.
	This is useful for cherry-picking multiple commits
	to produce a single new commit.
Show 6 quoted lines
> @@ -75,6 +79,38 @@ effect to your index in a row.
>  	cherry-pick'ed commit, then a fast forward to this commit will
>  	be performed.
>  
> +Examples
> +--------
These are a little repetitive.
> +git cherry-pick master::
> +
> +	Apply the changes specified by the commit pointed to by master
> +	and create a new commit with these changes.
git cherry-pick master::
	Apply the change introduced by the commit at the tip of the
	master branch and create a new commit.
Show 6 quoted lines
> +
> +git cherry-pick master~3..master::
> +git cherry-pick ^master~3 master::
> +
> +	Apply the changes specified by the last 3 commits pointed to
> +	by master and create 3 new commits with these changes.

git cherry-pick ..master:: git cherry-pick ^HEAD master::

	Apply the changes introduced by all commits that are ancestors
	of master but not HEAD to produce new commits on the current
	branch.
Show 5 quoted lines
> +git cherry-pick master\~3 master~2::
> +
> +	Apply the changes specified by the fourth and third last
> +	commits pointed to by master and create 2 new commits with
> +	these changes.
git cherry-pick master~5 master~2::
	Apply the changes introduced by the fifth- and second-generation
	grandparents of master to HEAD as new commits.
Show 7 quoted lines
> +
> +git cherry-pick -n master~1 next::
> +
> +	Apply to the working tree and the index the changes specified
> +	by the second last commit pointed to by master and by the last
> +	commit pointed to by next, but do not create any commit with
> +	these changes.
git rev-list master -- README | git cherry-pick -n --stdin::
	Apply the changes introduced by all commits on the master
	branch that touched README to the working tree and index,
	so the result can be inspected and made into a single new
	commit if suitable.
Show 6 quoted lines
> +
> +git cherry-pick --ff ..next::
> +
> +	If possible fast forward to the existing commits already in
> +	next but not in HEAD, otherwise apply the changes specified by
> +	these commits and create new commits with these changes.
git cherry-pick --ff ..next::
	If history is linear and HEAD is an ancestor of next, update
	the working tree and advance the HEAD pointer to match next.
	Otherwise, apply the changes introduced by those commits that
	are in next but not HEAD to the current branch, creating a new
	commit for each new change.

(This is not precisely right, since it doesn’t describe the behavior in some cases of nonlinear history:

   . --- . --- . --- . ---. next
  /                      /
 . HEAD             .---.

In this case, assuming time flows left-to-right, the HEAD would fast-forward three commits, then cherry-pick the other three. Not sure how to word this nicely.)

Hope that helps, Jonathan

Ramkumar Ramachandra· Jun 1, 2010, 10:26 UTC · re: Jonathan Nieder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

Hi,
Jonathan Nieder wrote:
Show 5 quoted lines
> Christian Couder wrote:
>> +Examples
>> +--------
>
> These are a little repetitive.

Since you also posted an "Examples" section for the git-revert, you could especially showcase options that aren't in git-revert (--ff and -x) here. Also, you might want to include a reference to git-revert in this document, in a "See Also" section perhaps?

-- Ram
Christian Couder· Jun 2, 2010, 05:57 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

Hi,
On Tuesday 01 June 2010 12:26:01 Ramkumar Ramachandra wrote:
Show 12 quoted lines
> Hi,
> 
> Jonathan Nieder wrote:
> > Christian Couder wrote:
> >> +Examples
> >> +--------
> >
> > These are a little repetitive.
> 
> Since you also posted an "Examples" section for the git-revert, you
> could especially showcase options that aren't in git-revert (--ff and
> -x) here.

There is already an example with --ff and I think -x may not deserve an example. (But feel free to propose a patch to add one if you think it's worth it.)

> Also, you might want to include a reference to git-revert in this
> document, in a "See Also" section perhaps?

Ok, I added a "SEE ALSO" section to both git-cherry-pick.txt and git- revert.txt.

Thanks, Christian.

Christian Couder· Jun 2, 2010, 05:57 UTC · re: Jonathan Nieder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

On Tuesday 01 June 2010 11:51:24 Jonathan Nieder wrote:
Show 11 quoted lines
> Christian Couder wrote:
> > --- a/Documentation/git-cherry-pick.txt
> > +++ b/Documentation/git-cherry-pick.txt
> > @@ -3,24 +3,29 @@ git-cherry-pick(1)
> >
> >  NAME
> >  ----
> > -git-cherry-pick - Apply the change introduced by an existing commit
> > +git-cherry-pick - Apply the change introduced by some existing commits
> 
> Maybe s/change/changes/.
Ok.
Show 20 quoted lines
> >  DESCRIPTION
> >  -----------
> > -Given one existing commit, apply the change the patch introduces, and
> > record a -new commit that records it.  This requires your working tree to
> > be clean (no -modifications from the HEAD commit).
> > +
> > +Given one or more existing commits, apply the changes that the related
> > +patches introduce, and record some new commits that record them.  This
> > +requires your working tree to be clean (no modifications from the HEAD
> > +commit).
> 
> "Related" made me think "related how"?  "Record some new commits"
> sounds oddly vague.
> 
> Maybe:
> 
> 	Given one or more existing commits, apply the change each one
> 	introduces, recording a new commit for each.  This requires
> 	your working tree to be clean (no modifications from the HEAD
> 	commit).
Ok.
Show 21 quoted lines
> >  OPTIONS
> >  -------
> > -<commit>::
> > -	Commit to cherry-pick.
> > +<commit>...::
> > +	Commits to cherry-pick.
> >  	For a more complete list of ways to spell commits, see the
> >  	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
> > +	Sets of commits can also be given but no traversal is done
> > +	by default, see linkgit:git-rev-list[1] and its '--no-walk'
> > +	option.
> 
> --no-walk can be a bit confusing.  I think we should try to avoid
> relying on the reader understanding it.
> 
>  <commit>...::
> 	Commits to cherry-pick.
> 	For a more complete list of ways to spell commits, see the
> 	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
> 	Revision ranges are interpreted as though specified with
> 	the `--no-walk` option; see the "Examples" section below.
I put the following:
<commit>...::
	Commits to cherry-pick.
	For a more complete list of ways to spell commits, see the
	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
	Sets of commits can be passed but no traversal is done by
	default, as if the '--no-walk' option was specified, see
	linkgit:git-rev-list[1].
Show 31 quoted lines
> >  -e::
> >  --edit::
> > @@ -55,13 +60,12 @@ OPTIONS
> >
> >  -n::
> >  --no-commit::
> > -	Usually the command automatically creates a commit.
> > -	This flag applies the change necessary to cherry-pick
> > -	the named commit to your working tree and the index,
> > -	but does not make the commit.  In addition, when this
> > -	option is used, your index does not have to match the
> > -	HEAD commit.  The cherry-pick is done against the
> > -	beginning state of your index.
> > +	Usually the command automatically creates some commits.  This
> > +	flag applies the change necessary to cherry-pick the named
> > +	commits to your working tree and the index, but does not make
> > +	the commits.  In addition, when this option is used, your
> > +	index does not have to match the HEAD commit.  The cherry-pick
> > +	is done against the beginning state of your index.
> 
> This switches between singular and plural.
> 
>  -n::
>  --no-commit::
> 	Usually 'git cherry-pick' automatically creates a sequence
> 	of commits.  This option can be used to apply the changes
> 	necessary to cherry-pick each named commit to your working
> 	tree and index without making any commits.  In addition,
> 	when this option is used, your index does not have to match
> 	the HEAD commit: the cherry-pick is done against the
> 	beginning state of the index.
I put the following:
-n::
--no-commit::
	Usually the command automatically creates a sequence of commits.
	This flag applies the changes necessary to cherry-pick
	each named commit to your working tree and the index,
	without making any commit.  In addition, when this
	option is used, your index does not have to match the
	HEAD commit.  The cherry-pick is done against the
	beginning state of your index.
Show 6 quoted lines
> >  +
> >  This is useful when cherry-picking more than one commits'
> >  effect to your index in a row.
> 
> 	This is useful for cherry-picking multiple commits
> 	to produce a single new commit.
I don't think any change is really needed here, so I left it as it was.
Show 18 quoted lines
> > @@ -75,6 +79,38 @@ effect to your index in a row.
> >  	cherry-pick'ed commit, then a fast forward to this commit will
> >  	be performed.
> >
> > +Examples
> > +--------
> 
> These are a little repetitive.
> 
> > +git cherry-pick master::
> > +
> > +	Apply the changes specified by the commit pointed to by master
> > +	and create a new commit with these changes.
> 
> git cherry-pick master::
> 
> 	Apply the change introduced by the commit at the tip of the
> 	master branch and create a new commit.
I put:
git cherry-pick master::
	Apply the change introduced by the commit at the tip of the
	master branch and create a new commit with this change.
Show 13 quoted lines
> > +
> > +git cherry-pick master~3..master::
> > +git cherry-pick ^master~3 master::
> > +
> > +	Apply the changes specified by the last 3 commits pointed to
> > +	by master and create 3 new commits with these changes.
> 
> git cherry-pick ..master::
> git cherry-pick ^HEAD master::
> 
> 	Apply the changes introduced by all commits that are ancestors
> 	of master but not HEAD to produce new commits on the current
> 	branch.
I put:

git cherry-pick ..master:: git cherry-pick ^HEAD master::

	Apply the changes introduced by all commits that are ancestors
	of master but not of HEAD to produce new commits.
Show 10 quoted lines
> > +git cherry-pick master\~3 master~2::
> > +
> > +	Apply the changes specified by the fourth and third last
> > +	commits pointed to by master and create 2 new commits with
> > +	these changes.
> 
> git cherry-pick master~5 master~2::
> 
> 	Apply the changes introduced by the fifth- and second-generation
> 	grandparents of master to HEAD as new commits.
I put:
git cherry-pick master\~4 master~2::
	Apply the changes introduced by the fifth and third last
	commits pointed to by master and create 2 new commits with
	these changes.
 
Show 14 quoted lines
> > +
> > +git cherry-pick -n master~1 next::
> > +
> > +	Apply to the working tree and the index the changes specified
> > +	by the second last commit pointed to by master and by the last
> > +	commit pointed to by next, but do not create any commit with
> > +	these changes.
> 
> git rev-list master -- README | git cherry-pick -n --stdin::
> 
> 	Apply the changes introduced by all commits on the master
> 	branch that touched README to the working tree and index,
> 	so the result can be inspected and made into a single new
> 	commit if suitable.

I think "--reverse" is needed after "rev-list" and cherry-pick has no --stdin option. So I put:

git cherry-pick -n master~1 next::
	Apply to the working tree and the index the changes introduced
	by the second last commit pointed to by master and by the last
	commit pointed to by next, but do not create any commit with
	these changes.
Show 14 quoted lines
> > +
> > +git cherry-pick --ff ..next::
> > +
> > +	If possible fast forward to the existing commits already in
> > +	next but not in HEAD, otherwise apply the changes specified by
> > +	these commits and create new commits with these changes.
> 
> git cherry-pick --ff ..next::
> 
> 	If history is linear and HEAD is an ancestor of next, update
> 	the working tree and advance the HEAD pointer to match next.
> 	Otherwise, apply the changes introduced by those commits that
> 	are in next but not HEAD to the current branch, creating a new
> 	commit for each new change.
Ok.

Thanks, Christian.

Jonathan Nieder· Jun 2, 2010, 06:14 UTC · re: Christian Couder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

Christian Couder wrote:
> On Tuesday 01 June 2010 11:51:24 Jonathan Nieder wrote:
Show 9 quoted lines
>> git rev-list master -- README | git cherry-pick -n --stdin::
>> 
>> 	Apply the changes introduced by all commits on the master
>> 	branch that touched README to the working tree and index,
>> 	so the result can be inspected and made into a single new
>> 	commit if suitable.
>
> I think "--reverse" is needed after "rev-list" and cherry-pick has no --stdin 
> option.
That’s too bad. :(  git show inherits it from rev-list.

Your new series looks good. Maybe I will obfuscate the examples some other day.

Thanks, Jonathan

Christian Couder· Jun 14, 2010, 03:33 UTC · re: Jonathan Nieder · lore

Re: [PATCH 7/8] Documentation/cherry-pick: describe passing more than one commit

On Wednesday 02 June 2010 08:14:52 Jonathan Nieder wrote:
Show 16 quoted lines
> Christian Couder wrote:
> > On Tuesday 01 June 2010 11:51:24 Jonathan Nieder wrote:
> >> git rev-list master -- README | git cherry-pick -n --stdin::
> >>
> >> 	Apply the changes introduced by all commits on the master
> >> 	branch that touched README to the working tree and index,
> >> 	so the result can be inspected and made into a single new
> >> 	commit if suitable.
> >
> > I think "--reverse" is needed after "rev-list" and cherry-pick has no
> > --stdin option.
> 
> That’s too bad. :(  git show inherits it from rev-list.
> 
> Your new series looks good.  Maybe I will obfuscate the examples some
> other day.

I will send a patch to add option --stdin to revert/cherry-pick. And it adds your fixed obfuscating example :-)

Thanks, Christian.

Christian Couder· May 31, 2010, 19:42 UTC · re: Christian Couder · lore

[PATCH 8/8] Documentation/revert: describe passing more than one commit

And while at it, add an "Examples" section.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-revert.txt |   52 ++++++++++++++++++++++++++++-------------
 1 files changed, 35 insertions(+), 17 deletions(-)
Show changes to Documentation/git-revert.txt +35 −17
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index c66bf80..5740f37 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -3,20 +3,22 @@ git-revert(1)
 
 NAME
 ----
-git-revert - Revert an existing commit
+git-revert - Revert some existing commits
 
 SYNOPSIS
 --------
-'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>
+'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
 
 DESCRIPTION
 -----------
-Given one existing commit, revert the change the patch introduces, and record a
-new commit that records it.  This requires your working tree to be clean (no
-modifications from the HEAD commit).
 
-Note: 'git revert' is used to record a new commit to reverse the
-effect of an earlier commit (often a faulty one).  If you want to
+Given one or more existing commits, revert the changes that the
+related patches introduce, and record some new commits that record
+them.  This requires your working tree to be clean (no modifications
+from the HEAD commit).
+
+Note: 'git revert' is used to record some new commits to reverse the
+effect of some earlier commits (often only a faulty one).  If you want to
 throw away all uncommitted changes in your working directory, you
 should see linkgit:git-reset[1], particularly the '--hard' option.  If
 you want to extract specific files as they were in another commit, you
@@ -26,10 +28,13 @@ both will discard uncommitted changes in your working directory.
 
 OPTIONS
 -------
-<commit>::
-	Commit to revert.
+<commit>...::
+	Commits to revert.
 	For a more complete list of ways to spell commit names, see
 	"SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
+	Sets of commits can also be given but no traversal is done by
+	default, see linkgit:git-rev-list[1] and its '--no-walk'
+	option.
 
 -e::
 --edit::
@@ -59,14 +64,13 @@ more details.
 
 -n::
 --no-commit::
-	Usually the command automatically creates a commit with
-	a commit log message stating which commit was
-	reverted.  This flag applies the change necessary
-	to revert the named commit to your working tree
-	and the index, but does not make the commit.  In addition,
-	when this option is used, your index does not have to match
-	the HEAD commit.  The revert is done against the
-	beginning state of your index.
+	Usually the command automatically creates some commits with
+	commit log messages stating which commits were reverted.  This
+	flag applies the changes necessary to revert the named commits
+	to your working tree and the index, but does not make the
+	commits.  In addition, when this option is used, your index
+	does not have to match the HEAD commit.  The revert is done
+	against the beginning state of your index.
 +
 This is useful when reverting more than one commits'
 effect to your index in a row.
@@ -75,6 +79,20 @@ effect to your index in a row.
 --signoff::
 	Add Signed-off-by line at the end of the commit message.
 
+Examples
+--------
+git revert HEAD~3::
+
+	Revert the changes specified by the fourth last commit in HEAD
+	and create a new commit with the reverted changes.
+
+git revert -n master\~5..master~2::
+
+	Revert the changes done by commits from the fiveth last commit
+	in master (included) to the third last commit in master
+	(included), but do not create any commit with the reverted
+	changes. The revert only modifies the working tree and the
+	index.
 
 Author
 ------
-- 
1.7.1.361.g42de.dirty
Antriksh Pany· Jun 1, 2010, 13:28 UTC · re: Christian Couder · lore

Re: [PATCH 8/8] Documentation/revert: describe passing more than one commit

On Mon, May 31, 2010 at 8:42 PM, Christian Couder <chriscool@tuxfamily.org> wrote:

Show 91 quoted lines
> And while at it, add an "Examples" section.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/git-revert.txt |   52 ++++++++++++++++++++++++++++-------------
>  1 files changed, 35 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
> index c66bf80..5740f37 100644
> --- a/Documentation/git-revert.txt
> +++ b/Documentation/git-revert.txt
> @@ -3,20 +3,22 @@ git-revert(1)
>
>  NAME
>  ----
> -git-revert - Revert an existing commit
> +git-revert - Revert some existing commits
>
>  SYNOPSIS
>  --------
> -'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>
> +'git revert' [--edit | --no-edit] [-n] [-m parent-number] [-s] <commit>...
>
>  DESCRIPTION
>  -----------
> -Given one existing commit, revert the change the patch introduces, and record a
> -new commit that records it.  This requires your working tree to be clean (no
> -modifications from the HEAD commit).
>
> -Note: 'git revert' is used to record a new commit to reverse the
> -effect of an earlier commit (often a faulty one).  If you want to
> +Given one or more existing commits, revert the changes that the
> +related patches introduce, and record some new commits that record
> +them.  This requires your working tree to be clean (no modifications
> +from the HEAD commit).
> +
> +Note: 'git revert' is used to record some new commits to reverse the
> +effect of some earlier commits (often only a faulty one).  If you want to
>  throw away all uncommitted changes in your working directory, you
>  should see linkgit:git-reset[1], particularly the '--hard' option.  If
>  you want to extract specific files as they were in another commit, you
> @@ -26,10 +28,13 @@ both will discard uncommitted changes in your working directory.
>
>  OPTIONS
>  -------
> -<commit>::
> -       Commit to revert.
> +<commit>...::
> +       Commits to revert.
>        For a more complete list of ways to spell commit names, see
>        "SPECIFYING REVISIONS" section in linkgit:git-rev-parse[1].
> +       Sets of commits can also be given but no traversal is done by
> +       default, see linkgit:git-rev-list[1] and its '--no-walk'
> +       option.
>
>  -e::
>  --edit::
> @@ -59,14 +64,13 @@ more details.
>
>  -n::
>  --no-commit::
> -       Usually the command automatically creates a commit with
> -       a commit log message stating which commit was
> -       reverted.  This flag applies the change necessary
> -       to revert the named commit to your working tree
> -       and the index, but does not make the commit.  In addition,
> -       when this option is used, your index does not have to match
> -       the HEAD commit.  The revert is done against the
> -       beginning state of your index.
> +       Usually the command automatically creates some commits with
> +       commit log messages stating which commits were reverted.  This
> +       flag applies the changes necessary to revert the named commits
> +       to your working tree and the index, but does not make the
> +       commits.  In addition, when this option is used, your index
> +       does not have to match the HEAD commit.  The revert is done
> +       against the beginning state of your index.
>  +
>  This is useful when reverting more than one commits'
>  effect to your index in a row.
> @@ -75,6 +79,20 @@ effect to your index in a row.
>  --signoff::
>        Add Signed-off-by line at the end of the commit message.
>
> +Examples
> +--------
> +git revert HEAD~3::
> +
> +       Revert the changes specified by the fourth last commit in HEAD
> +       and create a new commit with the reverted changes.
> +
> +git revert -n master\~5..master~2::
Is the backslash correct?
> +
> +       Revert the changes done by commits from the fiveth last commit
'fifth' instead of 'fiveth' may be more appropriate :)
Show 15 quoted lines
> +       in master (included) to the third last commit in master
> +       (included), but do not create any commit with the reverted
> +       changes. The revert only modifies the working tree and the
> +       index.
>
>  Author
>  ------
> --
> 1.7.1.361.g42de.dirty
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Christian Couder· Jun 2, 2010, 05:57 UTC · re: Antriksh Pany · lore

Re: [PATCH 8/8] Documentation/revert: describe passing more than one commit

On Tuesday 01 June 2010 15:28:57 Antriksh Pany wrote:
> 
> > +git revert -n master\~5..master~2::
> 
> Is the backslash correct?
Yes, otherwise asciidoc does something strange with the text between both "~".
> > +
> > +       Revert the changes done by commits from the fiveth last commit
> 
> 'fifth' instead of 'fiveth' may be more appropriate :)
Yes!

Thanks, Christian.

← back to recent threads