[PATCH] revert: add --stdin option to read commits from stdin

[PATCH] revert: add --stdin option to read commits from stdin

[Christian Couder]

This can be useful to do something like:

git rev-list --reverse master -- README | git cherry-pick -n --stdin

without using xargs.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
	This applies on top of pu as it is related to my cherry-pick
	many commits series.

 Documentation/git-cherry-pick.txt   |   14 +++++++++++++-
 Documentation/git-revert.txt        |    6 +++++-
 builtin/revert.c                    |    9 ++++++---
 t/t3508-cherry-pick-many-commits.sh |   10 ++++++++++
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index bcb4c75..54e6833 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -7,7 +7,8 @@ git-cherry-pick - Apply the changes 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] [--stdin]
+		<commit>...
 
 DESCRIPTION
 -----------
@@ -79,6 +80,10 @@ effect to your index in a row.
 	cherry-pick'ed commit, then a fast forward to this commit will
 	be performed.
 
+--stdin::
+	In addition to the '<commit>' listed on the command
+	line, read them from the standard input.
+
 EXAMPLES
 --------
 git cherry-pick master::
@@ -113,6 +118,13 @@ git cherry-pick --ff ..next::
 	are in next but not HEAD to the current branch, creating a new
 	commit for each new change.
 
+git rev-list --reverse 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.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com>
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index dea4f53..84b4a68 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -7,7 +7,7 @@ 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] [--stdin] <commit>...
 
 DESCRIPTION
 -----------
@@ -80,6 +80,10 @@ effect to your index in a row.
 --signoff::
 	Add Signed-off-by line at the end of the commit message.
 
+--stdin::
+	In addition to the '<commit>' listed on the command
+	line, read them from the standard input.
+
 EXAMPLES
 --------
 git revert HEAD~3::
diff --git a/builtin/revert.c b/builtin/revert.c
index 853e9e4..2b3d5a5 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -41,7 +41,7 @@ static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
 static int commit_argc;
 static const char **commit_argv;
-static int allow_rerere_auto;
+static int allow_rerere_auto, read_stdin;
 
 static const char *me;
 static const char *strategy;
@@ -63,6 +63,7 @@ static void parse_args(int argc, const char **argv)
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 		OPT_STRING(0, "strategy", &strategy, "strategy", "merge strategy"),
+		OPT_BOOLEAN(0, "stdin", &read_stdin, "read commits from stdin"),
 		OPT_END(),
 		OPT_END(),
 		OPT_END(),
@@ -79,7 +80,7 @@ static void parse_args(int argc, const char **argv)
 	}
 
 	commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0);
-	if (commit_argc < 1)
+	if (commit_argc < 1 && !read_stdin)
 		usage_with_options(usage_str, options);
 
 	commit_argv = argv;
@@ -527,10 +528,12 @@ static void prepare_revs(struct rev_info *revs)
 {
 	int argc = 0;
 	int i;
-	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
+	const char **argv = xmalloc((commit_argc + 5) * sizeof(*argv));
 
 	argv[argc++] = NULL;
 	argv[argc++] = "--no-walk";
+	if (read_stdin)
+		argv[argc++] = "--stdin";
 	if (action != REVERT)
 		argv[argc++] = "--reverse";
 	for (i = 0; i < commit_argc; i++)
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 3b87efe..27096f1 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -92,4 +92,14 @@ test_expect_failure 'cherry-pick -3 fourth works' '
 	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
 '
 
+test_expect_success 'cherry-pick --stdin works' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git rev-list --reverse first..fourth | git cherry-pick --stdin &&
+	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.468.g77401.dirty

Re: [PATCH] revert: add --stdin option to read commits from stdin

[Jonathan Nieder]

Christian Couder wrote:

> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -92,4 +92,14 @@ test_expect_failure 'cherry-pick -3 fourth works' '
>  	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
>  '
>  
> +test_expect_success 'cherry-pick --stdin works' '
> +	git checkout master &&
> +	git reset --hard first &&
[...]

This test fails for me as written, since the previous test leaves some
files in an unmerged state.  Patch 1 below works around that.

> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -79,7 +80,7 @@ static void parse_args(int argc, const char **argv)
>  	}
>  
>  	commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0);
[...]
> @@ -527,10 +528,12 @@ static void prepare_revs(struct rev_info *revs)
>  {
>  	int argc = 0;
>  	int i;
> -	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
> +	const char **argv = xmalloc((commit_argc + 5) * sizeof(*argv));
>  
>  	argv[argc++] = NULL;
>  	argv[argc++] = "--no-walk";
> +	if (read_stdin)
> +		argv[argc++] = "--stdin";

Ah, I see the problem now.  But it would be even nicer to allow arbitrary
rev-list options, so a person could ‘git cherry-pick --reverse a..b’,
for example.

In other words, how about something like patch 2 below?

Patch 3 is a small cleanup, as a bonus.

Christian Couder (1):
  revert: accept arbitrary rev-list options

Jonathan Nieder (2):
  t3508 (cherry-pick): futureproof against unmerged files
  revert: do not rebuild argv on heap

 Documentation/git-cherry-pick.txt   |    7 ++++++
 builtin/revert.c                    |   37 ++++++++++++++++++-----------------
 t/t3501-revert-cherry-pick.sh       |   18 +++++++++++++++++
 t/t3508-cherry-pick-many-commits.sh |   24 ++++++++++++++++------
 4 files changed, 61 insertions(+), 25 deletions(-)

[PATCH 1/3] t3508 (cherry-pick): futureproof against unmerged files

[Jonathan Nieder]

Each of the tests in t3508 begins by navigating to a sane state:

	git checkout master &&
	git reset --hard $commit

If a previous test left unmerged files around, they are untouched and
the checkout fails, causing later tests to fail, too.  This is not a
problem in practice because no test except the final one produces
unmerged files.

But as a futureproofing measure, it is still best to avoid the problem
with 'checkout -f'.  In particular, this is needed for new tests to be
added to the end of the script.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t3508-cherry-pick-many-commits.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 3b87efe..26a8730 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -23,7 +23,7 @@ test_expect_success setup '
 '
 
 test_expect_success 'cherry-pick first..fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
 	git cherry-pick first..fourth &&
@@ -33,7 +33,7 @@ test_expect_success 'cherry-pick first..fourth works' '
 '
 
 test_expect_success 'cherry-pick --ff first..fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
 	git cherry-pick --ff first..fourth &&
@@ -43,7 +43,7 @@ test_expect_success 'cherry-pick --ff first..fourth works' '
 '
 
 test_expect_success 'cherry-pick -n first..fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
 	git cherry-pick -n first..fourth &&
@@ -53,7 +53,7 @@ test_expect_success 'cherry-pick -n first..fourth works' '
 '
 
 test_expect_success 'revert first..fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard fourth &&
 	test_tick &&
 	git revert first..fourth &&
@@ -63,7 +63,7 @@ test_expect_success 'revert first..fourth works' '
 '
 
 test_expect_success 'revert ^first fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard fourth &&
 	test_tick &&
 	git revert ^first fourth &&
@@ -73,7 +73,7 @@ test_expect_success 'revert ^first fourth works' '
 '
 
 test_expect_success 'revert fourth fourth~1 fourth~2 works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard fourth &&
 	test_tick &&
 	git revert fourth fourth~1 fourth~2 &&
@@ -83,7 +83,7 @@ test_expect_success 'revert fourth fourth~1 fourth~2 works' '
 '
 
 test_expect_failure 'cherry-pick -3 fourth works' '
-	git checkout master &&
+	git checkout -f master &&
 	git reset --hard first &&
 	test_tick &&
 	git cherry-pick -3 fourth &&
-- 
1.7.1.246.g398e5.dirty

[PATCH 2/3] revert: accept arbitrary rev-list options

[Jonathan Nieder]

From: Christian Couder <chriscool@tuxfamily.org>

This can be useful to do something like:

git rev-list --reverse master -- README | git cherry-pick -n --stdin

without using xargs.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/git-cherry-pick.txt   |    7 +++++++
 builtin/revert.c                    |    3 ++-
 t/t3508-cherry-pick-many-commits.sh |   10 ++++++++++
 3 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index bcb4c75..f047739 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -113,6 +113,13 @@ git cherry-pick --ff ..next::
 	are in next but not HEAD to the current branch, creating a new
 	commit for each new change.
 
+git rev-list --reverse 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.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com>
diff --git a/builtin/revert.c b/builtin/revert.c
index 853e9e4..67e191b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -78,7 +78,8 @@ static void parse_args(int argc, const char **argv)
 			die("program error");
 	}
 
-	commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0);
+	commit_argc = parse_options(argc, argv, NULL, options, usage_str,
+	                            PARSE_OPT_KEEP_UNKNOWN);
 	if (commit_argc < 1)
 		usage_with_options(usage_str, options);
 
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 26a8730..93d7189 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -92,4 +92,14 @@ test_expect_failure 'cherry-pick -3 fourth works' '
 	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify fourth)"
 '
 
+test_expect_success 'cherry-pick --stdin works' '
+	git checkout -f master &&
+	git reset --hard first &&
+	test_tick &&
+	git rev-list --reverse first..fourth | git cherry-pick --stdin &&
+	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.246.g398e5.dirty

[PATCH 3/3] revert: do not rebuild argv on heap

[Jonathan Nieder]

Set options in struct rev_info directly so we can reuse the
arguments collected from parse_options without modification.

This is just a cleanup; no noticeable change is intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks, Christian.

 builtin/revert.c              |   34 +++++++++++++++++-----------------
 t/t3501-revert-cherry-pick.sh |   18 ++++++++++++++++++
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 67e191b..88b7501 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -50,10 +50,14 @@ static const char *strategy;
 
 static char *get_encoding(const char *message);
 
+static const char * const *revert_or_cherry_pick_usage(void)
+{
+	return action == REVERT ? revert_usage : cherry_pick_usage;
+}
+
 static void parse_args(int argc, const char **argv)
 {
-	const char * const * usage_str =
-		action == REVERT ?  revert_usage : cherry_pick_usage;
+	const char * const * usage_str = revert_or_cherry_pick_usage();
 	int noop;
 	struct option options[] = {
 		OPT_BOOLEAN('n', "no-commit", &no_commit, "don't automatically commit"),
@@ -79,8 +83,9 @@ static void parse_args(int argc, const char **argv)
 	}
 
 	commit_argc = parse_options(argc, argv, NULL, options, usage_str,
+	                            PARSE_OPT_KEEP_ARGV0 |
 	                            PARSE_OPT_KEEP_UNKNOWN);
-	if (commit_argc < 1)
+	if (commit_argc < 2)
 		usage_with_options(usage_str, options);
 
 	commit_argv = argv;
@@ -526,27 +531,22 @@ static int do_pick_commit(void)
 
 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;
+	int argc;
 
 	init_revisions(revs, NULL);
-	setup_revisions(argc - 1, argv, revs, NULL);
+	revs->no_walk = 1;
+	if (action != REVERT)
+		revs->reverse = 1;
+
+	argc = setup_revisions(commit_argc, commit_argv, revs, NULL);
+	if (argc > 1)
+		usage(*revert_or_cherry_pick_usage());
+
 	if (prepare_revision_walk(revs))
 		die("revision walk setup failed");
 
 	if (!revs->commits)
 		die("empty commit set passed");
-
-	free(argv);
 }
 
 static int revert_or_cherry_pick(int argc, const char **argv)
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 7f85815..f61c54d 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -41,6 +41,24 @@ test_expect_success setup '
 	git tag rename2
 '
 
+test_expect_success 'cherry-pick --nonsense' '
+
+	pos=$(git rev-parse HEAD) &&
+	git diff --exit-code HEAD &&
+	test_must_fail git cherry-pick --nonsense 2>msg &&
+	git diff --exit-code HEAD "$pos" &&
+	grep '[Uu]sage:' msg
+'
+
+test_expect_success 'revert --nonsense' '
+
+	pos=$(git rev-parse HEAD) &&
+	git diff --exit-code HEAD &&
+	test_must_fail git revert --nonsense 2>msg &&
+	git diff --exit-code HEAD "$pos" &&
+	grep '[Uu]sage:' msg
+'
+
 test_expect_success 'cherry-pick after renaming branch' '
 
 	git checkout rename2 &&
-- 
1.7.1.246.g398e5.dirty

Re: [PATCH] revert: add --stdin option to read commits from stdin

[Johannes Sixt]

Am 6/14/2010 5:22, schrieb Christian Couder:
> +--stdin::
> +	In addition to the '<commit>' listed on the command
> +	line, read them from the standard input.

And the order in which --stdin and command line commits are processed is...?

-- Hannes

Re: [PATCH] revert: add --stdin option to read commits from stdin

[Christian Couder]

On Monday 14 June 2010 07:20:27 Jonathan Nieder wrote:
> Christian Couder wrote:
> > --- a/t/t3508-cherry-pick-many-commits.sh
> > +++ b/t/t3508-cherry-pick-many-commits.sh
> > @@ -92,4 +92,14 @@ test_expect_failure 'cherry-pick -3 fourth works' '
> >  	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify
> > fourth)" '
> >
> > +test_expect_success 'cherry-pick --stdin works' '
> > +	git checkout master &&
> > +	git reset --hard first &&
> 
> [...]
> 
> This test fails for me as written, since the previous test leaves some
> files in an unmerged state.  Patch 1 below works around that.

Ok.

> > --- a/builtin/revert.c
> > +++ b/builtin/revert.c
> > @@ -79,7 +80,7 @@ static void parse_args(int argc, const char **argv)
> >  	}
> >
> >  	commit_argc = parse_options(argc, argv, NULL, options, usage_str, 0);
> 
> [...]
> 
> > @@ -527,10 +528,12 @@ static void prepare_revs(struct rev_info *revs)
> >  {
> >  	int argc = 0;
> >  	int i;
> > -	const char **argv = xmalloc((commit_argc + 4) * sizeof(*argv));
> > +	const char **argv = xmalloc((commit_argc + 5) * sizeof(*argv));
> >
> >  	argv[argc++] = NULL;
> >  	argv[argc++] = "--no-walk";
> > +	if (read_stdin)
> > +		argv[argc++] = "--stdin";
> 
> Ah, I see the problem now.  But it would be even nicer to allow arbitrary
> rev-list options, so a person could ‘git cherry-pick --reverse a..b’,
> for example.

Yeah, I agree that is nicer.

> In other words, how about something like patch 2 below?
> 
> Patch 3 is a small cleanup, as a bonus.
> 
> Christian Couder (1):
>   revert: accept arbitrary rev-list options
> 
> Jonathan Nieder (2):
>   t3508 (cherry-pick): futureproof against unmerged files
>   revert: do not rebuild argv on heap

I get "indent with spaces" errors when I apply patches 2/3 and 3/3:

$ git am ../messages/\[PATCH\ 2_3\]\ revert_\ accept\ arbitrary\ rev-list\ 
options.mbox
Applying: revert: accept arbitrary rev-list options                                                                      
/home/christian/work/git/.git/rebase-apply/patch:35: indent with spaces.                                                 
                                    PARSE_OPT_KEEP_UNKNOWN);  

$ git am ../messages/\[PATCH\ 3_3\]\ revert_\ do\ not\ rebuild\ argv\ on\ 
heap.mbox
Applying: revert: do not rebuild argv on heap                                                                        
/home/christian/work/git/.git/rebase-apply/patch:33: indent with spaces.                                             
                                    PARSE_OPT_KEEP_ARGV0 |                                                           
warning: 1 line adds whitespace errors.

But otherwise it all looks very good to me.

Even something like "git cherry-pick -3 fourth" is now working after patch 
2/3, so all test cases now pass. This is because "--no-walk" does not always 
take over "-3" it looks like it depends on the order of the arguments.

For example I get:

$ git rev-list --no-walk -3 --reverse fourth
453a04748224b3f212580d1195b452334d346e75
e85abe28a2b8ef771f760575b325f4c41f9c815f
94d3184b3f0dcfebb393faf5a122dc429d775538

but:

$ git rev-list -3 --no-walk --reverse fourth
94d3184b3f0dcfebb393faf5a122dc429d775538

I will post an updated v2 series without the whitespace errors and with a few 
documentation and test updates in patch 2/3.

Thanks,
Christian.