Thread (105 messages) flat view 105 messages, 10 authors, 2016-06-15

Re: [PATCH 31/31] rebase -i: remove unnecessary state rebase-root

From: Martin von Zweigbergk <hidden>
Date: 2016-06-15 22:50:18

On Tue, 28 Dec 2010, Thomas Rast wrote:
quoted hunk ↗ jump to hunk
Martin von Zweigbergk wrote:
quoted
@@ -168,11 +168,6 @@ pick_one () {
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
 	test -d "$REWRITTEN" &&
 		pick_one_preserving_merges "$@" && return
-	if test -n "$rebase_root"
-	then
-		output git cherry-pick "$@"
-		return
-	fi
 	output git cherry-pick $ff "$@"
 }
[...]
quoted
While factoring out the state writing code a few patches back, I went
through each of the pieces of state that was written. I was a bit
hesitant to include this patch since I'm not quite sure why the code
was introduced, but I thought I would include it anyway to hear what
you have to say.

There used to be bug when using --ff when rebasing a root commit. This
was fixed in 6355e50 (builtin/revert.c: don't dereference a NULL
pointer, 2010-09-27). Could this have been the reason for the check?
Thomas, do you remember?
I think this just ended up being such a strange test because of the
following hunk in 8e75abf (rebase -i: use new --ff cherry-pick option,
2010-03-06):
@@ -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 "$@"
 }
-- 
Yes, I saw that one as well, so it would probably have made more sense
to ask Christian instead (the author of 8e75abf). So do you remember,
Christian?

Anyway, thanks for your input, Thomas. That makes me feel a little
less worried about this patch.


/Martin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help