threads / patch / 22935

v4, 7 partsadd --ff option to cherry-pick

Subject: [PATCH v4 0/7] add --ff option to cherry-pick

## tl;dr

12 messages between Mar 6, 2010 and Mar 7, 2010. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Christian Couder· Mar 6, 2010, 20:34 UTC · lore

The goal of this patch series is to make it possible for "git cherry-pick" to fast forward instead of creating a new commit if the cherry picked commit has the same parent as the one we are cherry-picking on.

The big change since previous version is that the first patches to implement the --ff option are based on what Junio suggested.

Christian Couder (4):
  cherry-pick: add tests for new --ff option
  Documentation: describe new cherry-pick --ff option
  cherry-pick: add a no-op --no-ff option to future proof scripts
  rebase -i: use new --ff cherry-pick option
Junio C Hamano (3):
  parse-options: add parse_options_concat() to concat options
  builtin/merge: make checkout_fast_forward() non static
  revert: add --ff option to allow fast forward when cherry-picking
 Documentation/git-cherry-pick.txt |   10 +++-
 builtin/merge.c                   |    2 +-
 builtin/revert.c                  |   47 +++++++++++++++-
 cache.h                           |    3 +
 git-rebase--interactive.sh        |   15 +----
 parse-options.c                   |   15 +++++
 parse-options.h                   |    1 +
 t/t3506-cherry-pick-ff.sh         |  106 +++++++++++++++++++++++++++++++++++++
 8 files changed, 182 insertions(+), 17 deletions(-)
 create mode 100755 t/t3506-cherry-pick-ff.sh
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 1/7] parse-options: add parse_options_concat() to concat options

From: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 parse-options.c |   15 +++++++++++++++
 parse-options.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)
Show changes to 2 files +16 −0

parse-options.c, parse-options.h

diff --git a/parse-options.c b/parse-options.c
index c83035d..8546d85 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -659,3 +659,18 @@ int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 	*target = unset ? 2 : 1;
 	return 0;
 }
+
+int parse_options_concat(struct option *dst, size_t dst_size, struct option *src)
+{
+	int i, j;
+
+	for (i = 0; i < dst_size; i++)
+		if (dst[i].type == OPTION_END)
+			break;
+	for (j = 0; i < dst_size; i++, j++) {
+		dst[i] = src[j];
+		if (src[j].type == OPTION_END)
+			return 0;
+	}
+	return -1;
+}
diff --git a/parse-options.h b/parse-options.h
index 9429f7e..7581e93 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -187,6 +187,7 @@ extern int parse_options_step(struct parse_opt_ctx_t *ctx,
 
 extern int parse_options_end(struct parse_opt_ctx_t *ctx);
 
+extern int parse_options_concat(struct option *dst, size_t, struct option *src);
 
 /*----- some often used options -----*/
 extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
-- 
1.7.0.1.307.g861f4
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 2/7] builtin/merge: make checkout_fast_forward() non static

From: Junio C Hamano <gitster@pobox.com>
and also export it in "cache.h".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/merge.c |    2 +-
 cache.h         |    3 +++
 2 files changed, 4 insertions(+), 1 deletions(-)
Show changes to 2 files +4 −1

builtin/merge.c, cache.h

diff --git a/builtin/merge.c b/builtin/merge.c
index 3aaec7b..c043066 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -667,7 +667,7 @@ static int count_unmerged_entries(void)
 	return ret;
 }
 
-static int checkout_fast_forward(unsigned char *head, unsigned char *remote)
+int checkout_fast_forward(const unsigned char *head, const unsigned char *remote)
 {
 	struct tree *trees[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
diff --git a/cache.h b/cache.h
index 89f6a40..0608b55 100644
--- a/cache.h
+++ b/cache.h
@@ -1054,4 +1054,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix);
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 
+/* builtin/merge.c */
+int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
+
 #endif /* CACHE_H */
-- 
1.7.0.1.307.g861f4
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking

From: Junio C Hamano <gitster@pobox.com>

As "git merge" fast forwards if possible, it seems sensible to have such a feature for "git cherry-pick" too, especially as it could be used in git-rebase--interactive.sh.

Maybe this option could be made the default in the long run, with another --no-ff option to disable this default behavior, but that could make some scripts backward incompatible and/or that would require testing if some GIT_AUTHOR_* environment variables are set. So we don't do that for now.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/revert.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)
Show changes to builtin/revert.c +40 −3
diff --git a/builtin/revert.c b/builtin/revert.c
index eff5268..476f41e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -13,6 +13,7 @@
 #include "revision.h"
 #include "rerere.h"
 #include "merge-recursive.h"
+#include "refs.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -35,7 +36,7 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff;
+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;
@@ -60,8 +61,19 @@ static void parse_args(int argc, const char **argv)
 		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
 		OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 		OPT_END(),
+		OPT_END(),
+		OPT_END(),
 	};
 
+	if (action == CHERRY_PICK) {
+		struct option cp_extra[] = {
+			OPT_BOOLEAN(0, "ff", &allow_ff, "allow fast-forward"),
+			OPT_END(),
+		};
+		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
+			die("program error");
+	}
+
 	if (parse_options(argc, argv, NULL, options, usage_str, 0) != 1)
 		usage_with_options(usage_str, options);
 
@@ -244,6 +256,17 @@ static NORETURN void die_dirty_index(const char *me)
 	}
 }
 
+static int fast_forward_to(const unsigned char *to, const unsigned char *from)
+{
+	struct ref_lock *ref_lock;
+
+	read_cache();
+	if (checkout_fast_forward(from, to))
+		exit(1); /* the callee should have complained already */
+	ref_lock = lock_any_ref_for_update("HEAD", from, 0);
+	return write_ref_sha1(ref_lock, to, "cherry-pick");
+}
+
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
@@ -265,6 +288,17 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	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");
+		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) {
@@ -284,8 +318,6 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	}
 	discard_cache();
 
-	index_fd = hold_locked_index(&index_lock, 1);
-
 	if (!commit->parents) {
 		if (action == REVERT)
 			die ("Cannot revert a root commit");
@@ -314,6 +346,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	else
 		parent = commit->parents->item;
 
+	if (allow_ff && !hashcmp(parent->object.sha1, head))
+		return fast_forward_to(commit->object.sha1, head);
+
 	if (!(message = commit->buffer))
 		die ("Cannot get commit message for %s",
 				sha1_to_hex(commit->object.sha1));
@@ -343,6 +378,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	oneline = get_oneline(message);
 
+	index_fd = hold_locked_index(&index_lock, 1);
+
 	if (action == REVERT) {
 		char *oneline_body = strchr(oneline, ' ');
 
-- 
1.7.0.1.307.g861f4
Junio C Hamano· Mar 7, 2010, 03:55 UTC · re: Christian Couder · lore

Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking

Christian Couder <chriscool@tuxfamily.org> writes:
Show 7 quoted lines
> From: Junio C Hamano <gitster@pobox.com>
>
> As "git merge" fast forwards if possible, it seems sensible to
> have such a feature for "git cherry-pick" too, especially as it
> could be used in git-rebase--interactive.sh.
>
> Maybe this option could be made the default in the long run,
I never said that.

For scripted use, like we can see in your [PATCH 7/7], "cherry-pick --ff" could be a good ingredient (but even then the calling Porcelain script may already know that what is being picked is a direct descendent (rebase -i certainly should, as the last commit it processed from the todo file is at the HEAD), but for manual use case it just feels silly to assume that the end user is so stupid that s/he doesn't know the commit being picked might be a direct descendant, which would be the only reason why making --ff the default might make sense.

And adding --no-ff and sprinkling that to scripts, only to support that default change as a future possibility, feels doubly silly.

What was the real motive/use case of "cherry-pick --ff"?
Paolo Bonzini· Mar 7, 2010, 08:34 UTC · re: Junio C Hamano · lore

Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking

On 03/07/2010 04:55 AM, Junio C Hamano wrote:
> What was the real motive/use case of "cherry-pick --ff"?

Avoiding pointless separation of histories. It's true that nowadays git-patch-id will make a good job of reconciling them, but they do look ugly in gitk.

Paolo
Junio C Hamano· Mar 7, 2010, 09:05 UTC · re: Paolo Bonzini · lore

Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking

Paolo Bonzini <bonzini@gnu.org> writes:
Show 6 quoted lines
> On 03/07/2010 04:55 AM, Junio C Hamano wrote:
>> What was the real motive/use case of "cherry-pick --ff"?
>
> Avoiding pointless separation of histories.  It's true that nowadays
> git-patch-id will make a good job of reconciling them, but they do
> look ugly in gitk.
Sorry, but that is a not-very-useful XY answer.

Why were _you_ using "cherry-pick" on a commit that is an immediate child of the current commit? What were you trying to achieve by blindly using cherry-pick even in such a case?

I am sort-of guessing that "blindly" is coming from a script that doesn't bother to check if the commit you are cherry-picking is a direct child, and I do not think it is such a bad thing to allow scripts to blindly call cherry-pick that way and at the same time avoid making cherry-picked commits that are unnecessary. So in that sense I am not opposed to having an option to allow "--ff".

But if that is the real motivation, then making --ff default is a wrong thing to do, as existing scripts knew and trusted cherry-pick will _not_ fast-forward, and the ones that do want fast-forward have already checked just like "rebase -i" does. Changing the default like Chriscool suggested would not help anybody.

That is what I wanted to find out.
Christian Couder· Mar 7, 2010, 20:39 UTC · re: Junio C Hamano · lore

Re: [PATCH v4 3/7] revert: add --ff option to allow fast forward when cherry-picking

On Sunday 07 March 2010 10:05:04 Junio C Hamano wrote:
Show 26 quoted lines
> Paolo Bonzini <bonzini@gnu.org> writes:
> > On 03/07/2010 04:55 AM, Junio C Hamano wrote:
> >> What was the real motive/use case of "cherry-pick --ff"?
> >
> > Avoiding pointless separation of histories.  It's true that nowadays
> > git-patch-id will make a good job of reconciling them, but they do
> > look ugly in gitk.
> 
> Sorry, but that is a not-very-useful XY answer.
> 
> Why were _you_ using "cherry-pick" on a commit that is an immediate child
> of the current commit?  What were you trying to achieve by blindly using
> cherry-pick even in such a case?
> 
> I am sort-of guessing that "blindly" is coming from a script that doesn't
> bother to check if the commit you are cherry-picking is a direct child,
> and I do not think it is such a bad thing to allow scripts to blindly call
> cherry-pick that way and at the same time avoid making cherry-picked
> commits that are unnecessary.  So in that sense I am not opposed to having
> an option to allow "--ff".
> 
> But if that is the real motivation, then making --ff default is a wrong
> thing to do, as existing scripts knew and trusted cherry-pick will _not_
> fast-forward, and the ones that do want fast-forward have already checked
> just like "rebase -i" does.  Changing the default like Chriscool suggested
> would not help anybody.

The wording is: "Maybe this option could be made the default in the long run, ..."

I didn't wrote something like: "This option should be made the default in the long run, ..."

I really didn't know if it should or should not be made the default now or in the long run, but I thought I had to talk about it.

Why? Because Paolo said that he would like it to become the default, and also because if we implement "git cherry-pick A..B" and people start to use it a lot, they may find it boring to have to always add --ff to avoid creating lots of spurious commit (instead of reusing existing ones).

Now if you want me to remove this part of the commit message, I will do it. But my opinion is that it is interesting to document that we thought about making it the default now or in the future.

Do you prefer something like:

"Making this option the default was discussed on the mailing list but we decided that it is best left as non default for now. Though we recognized that it is best to have a --no-ff option too to future proof scripts in case the decision to make fast forwarding the default is made in the future."

?

Best regards, Christian.

Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 4/7] cherry-pick: add tests for new --ff option

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t3506-cherry-pick-ff.sh |   98 +++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100755 t/t3506-cherry-pick-ff.sh
Show changes to t/t3506-cherry-pick-ff.sh +98 −0
diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
new file mode 100755
index 0000000..e17ae71
--- /dev/null
+++ b/t/t3506-cherry-pick-ff.sh
@@ -0,0 +1,98 @@
+#!/bin/sh
+
+test_description='test cherry-picking with --ff option'
+
+. ./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 &&
+	echo second >> file1 &&
+	git add file1 &&
+	test_tick &&
+	git commit -m "second" &&
+	git tag second
+'
+
+test_expect_success 'cherry-pick using --ff fast forwards' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick --ff second &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify second)"
+'
+
+test_expect_success 'cherry-pick not using --ff does not fast forwards' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick second &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify second)"
+'
+
+#
+# We setup the following graph:
+#
+#	      B---C
+#	     /   /
+#	first---A
+#
+# (This has been taken from t3502-cherry-pick-merge.sh)
+#
+test_expect_success 'merge setup' '
+	git checkout master &&
+	git reset --hard first &&
+	echo new line >A &&
+	git add A &&
+	test_tick &&
+	git commit -m "add line to A" A &&
+	git tag A &&
+	git checkout -b side first &&
+	echo new line >B &&
+	git add B &&
+	test_tick &&
+	git commit -m "add line to B" B &&
+	git tag B &&
+	git checkout master &&
+	git merge side &&
+	git tag C &&
+	git checkout -b new A
+'
+
+test_expect_success 'cherry-pick a non-merge with --ff and -m should fail' '
+	git reset --hard A -- &&
+	test_must_fail git cherry-pick --ff -m 1 B &&
+	git diff --exit-code A --
+'
+
+test_expect_success 'cherry pick a merge with --ff but without -m should fail' '
+	git reset --hard A -- &&
+	test_must_fail git cherry-pick --ff C &&
+	git diff --exit-code A --
+'
+
+test_expect_success 'cherry pick with --ff a merge (1)' '
+	git reset --hard A -- &&
+	git cherry-pick --ff -m 1 C &&
+	git diff --exit-code C &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify C)"
+'
+
+test_expect_success 'cherry pick with --ff a merge (2)' '
+	git reset --hard B -- &&
+	git cherry-pick --ff -m 2 C &&
+	git diff --exit-code C &&
+	test "$(git rev-parse --verify HEAD)" = "$(git rev-parse --verify C)"
+'
+
+test_expect_success 'cherry pick a merge relative to nonexistent parent with --ff should fail' '
+	git reset --hard B -- &&
+	test_must_fail git cherry-pick --ff -m 3 C
+'
+
+test_done
-- 
1.7.0.1.307.g861f4
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 5/7] Documentation: describe new cherry-pick --ff option

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-cherry-pick.txt |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
Show changes to Documentation/git-cherry-pick.txt +5 −1
diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index 78f4714..d71607a 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit
 
 SYNOPSIS
 --------
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] <commit>
+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>
 
 DESCRIPTION
 -----------
@@ -70,6 +70,10 @@ effect to your index in a row.
 --signoff::
 	Add Signed-off-by line at the end of the commit message.
 
+--ff::
+	If the current HEAD is the same as the parent of the
+	cherry-pick'ed commit, then a fast forward to this commit will
+	be performed.
 
 Author
 ------
-- 
1.7.0.1.307.g861f4
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 6/7] cherry-pick: add a no-op --no-ff option to future proof scripts

A --ff option to allow "git cherry-pick" to fast forward if possible was added in a previous patch, and this behavior may become the default one in the future.

So to future proof scripts that may rely on the current behavior it is safer to add a --no-ff option to make sure that the current behavior will be used.

Requested-by: Paolo Bonzini <bonzini@gnu.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-cherry-pick.txt |    6 +++++-
 builtin/revert.c                  |    6 +++++-
 t/t3506-cherry-pick-ff.sh         |    8 ++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)
Show changes to 3 files +18 −2

Documentation/git-cherry-pick.txt, builtin/revert.c, t/t3506-cherry-pick-ff.sh

diff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt
index d71607a..dfc8243 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -7,7 +7,7 @@ git-cherry-pick - Apply the change introduced by an existing commit
 
 SYNOPSIS
 --------
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] <commit>
+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--[no-]ff] <commit>
 
 DESCRIPTION
 -----------
@@ -75,6 +75,10 @@ effect to your index in a row.
 	cherry-pick'ed commit, then a fast forward to this commit will
 	be performed.
 
+--no-ff::
+	Does nothing right now, but in the future this may disallow
+	fast forward if it becomes the default behavior.
+
 Author
 ------
 Written by Junio C Hamano <gitster@pobox.com>
diff --git a/builtin/revert.c b/builtin/revert.c
index 476f41e..0651e34 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -36,7 +36,7 @@ static const char * const cherry_pick_usage[] = {
 	NULL
 };
 
-static int edit, no_replay, no_commit, mainline, signoff, allow_ff;
+static int edit, no_replay, no_commit, mainline, signoff, allow_ff, no_ff;
 static enum { REVERT, CHERRY_PICK } action;
 static struct commit *commit;
 static const char *commit_name;
@@ -63,11 +63,13 @@ static void parse_args(int argc, const char **argv)
 		OPT_END(),
 		OPT_END(),
 		OPT_END(),
+		OPT_END(),
 	};
 
 	if (action == CHERRY_PICK) {
 		struct option cp_extra[] = {
 			OPT_BOOLEAN(0, "ff", &allow_ff, "allow fast-forward"),
+			OPT_BOOLEAN(0, "no-ff", &no_ff, "disallow fast forward"),
 			OPT_END(),
 		};
 		if (parse_options_concat(options, ARRAY_SIZE(options), cp_extra))
@@ -297,6 +299,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 			die("cherry-pick --ff cannot be used with -x");
 		if (edit)
 			die("cherry-pick --ff cannot be used with --edit");
+		if (no_ff)
+			die("cherry-pick --ff cannot be used with --no-ff");
 	}
 
 	if (read_cache() < 0)
diff --git a/t/t3506-cherry-pick-ff.sh b/t/t3506-cherry-pick-ff.sh
index e17ae71..2d7e532 100755
--- a/t/t3506-cherry-pick-ff.sh
+++ b/t/t3506-cherry-pick-ff.sh
@@ -35,6 +35,14 @@ test_expect_success 'cherry-pick not using --ff does not fast forwards' '
 	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify second)"
 '
 
+test_expect_success 'cherry-pick using --no-ff does not fast forwards' '
+	git checkout master &&
+	git reset --hard first &&
+	test_tick &&
+	git cherry-pick --no-ff second &&
+	test "$(git rev-parse --verify HEAD)" != "$(git rev-parse --verify second)"
+'
+
 #
 # We setup the following graph:
 #
-- 
1.7.0.1.307.g861f4
Christian Couder· Mar 6, 2010, 20:34 UTC · re: Christian Couder · lore

[PATCH v4 7/7] rebase -i: use new --ff cherry-pick option

This simplifies rebase -i a little bit.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-rebase--interactive.sh |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)
Show changes to git-rebase--interactive.sh +3 −12
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3e4fd14..92d19f5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -222,8 +222,8 @@ do_with_author () {
 }
 
 pick_one () {
-	no_ff=
-	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
+	ff=--ff
+	case "$1" in -n) sha1=$2; ff= ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
 		pick_one_preserving_merges "$@" && return
@@ -232,16 +232,7 @@ pick_one () {
 		output git cherry-pick "$@"
 		return
 	fi
-	parent_sha1=$(git rev-parse --verify $sha1^) ||
-		die "Could not get the parent of $sha1"
-	current_sha1=$(git rev-parse --verify HEAD)
-	if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
-	then
-		output git reset --hard $sha1
-		output warn Fast-forward to $(git rev-parse --short $sha1)
-	else
-		output git cherry-pick "$@"
-	fi
+	output git cherry-pick $ff "$@"
 }
 
 pick_one_preserving_merges () {
-- 
1.7.0.1.307.g861f4

← back to recent threads