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

Subsystems: documentation, the rest

STALE3736d

7 messages, 3 authors, 2016-06-15 · open the first message on its own page

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

From: Christian Couder <hidden>
Date: 2016-06-15 22:48:57

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 <redacted>
---
	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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:48:57

Christian Couder wrote:
quoted hunk
--- 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.
quoted hunk
--- 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);
[...]
quoted hunk
@@ -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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:48:57

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 <redacted>
---
 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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:48:57

From: Christian Couder <redacted>

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 <redacted>
Signed-off-by: Jonathan Nieder <redacted>
---
 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

From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:48:57

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 <redacted>
---
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

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:48:57

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

From: Christian Couder <hidden>
Date: 2016-06-15 22:48:58

On Monday 14 June 2010 07:20:27 Jonathan Nieder wrote:
Christian Couder wrote:
quoted
--- 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.
quoted
--- 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);
[...]
quoted
@@ -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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help