[PATCH v1] rebase -m: Use empty tree base for parentless commits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When the user specifies a merge strategy, `git-merge-$strategy` is
used in non-interactive mode to replay the changes introduced by the
current branch relative to some upstream. Specifically, for each
commit `c` that is not in upstream the changes that led from `c^` to
`c` are reapplied.

If the current has a different root than the upstream, either because
the history is disconnected or merged in a disconnected history, then
there will be a parentless commit `c` and `c^` will not refer to a
commit.

In order to cope with such a situation, check for every `c` whether
its list of parents is empty. If it is empty, determine the
introduced changes by comparing the committed tree to the empty tree
instead. Otherwise, take the differences between `c^` and `c` as
before.

The other git-rebase modes do not have similar problems because they
use git-cherry-pick to replay changes, even with strategy options. It
seems that the non-interactive rebase with merge strategies was not
implemented using git-cherry-pick because it did not support them at
the time (`git rebase --merge` added in 58634dbf and `git cherry-pick
--strategy` added in 91e52598). The idea of using the empty tree as
reference tree for orphan commits is taken from the git-cherry-pick
implementation.

Regarding the patch, we do not have to commit the empty tree before
we can pass it as a base argument to `git-merge-$strategy` because
tree objects are recognized as such and implicitly committed by
`git-merge-$strategy`.

Add a test. The test case rebases a single disconnected commit which
creates an isolated file on master and, therefore, does not require a
specific merge strategy. It is a mere sanity check.

Reported-by: David M. Lloyd <david.lloyd@xxxxxxxxxx>
Signed-off-by: Fabian Ruch <bafain@xxxxxxxxx>
---
Hi David,

I don't think you made a mistake at all. If I understand the --merge
mode of git-rebase correctly there is no need to require a parent.
The error occurs when the script tries to determine the changes your
merge commit introduces, which includes the whole "importing/master"
branch. The strategy is not yet part of the picture then and will not
be until the changes are being replayed.

The test case tries to simplify your scenario because the relevant
characteristic seems to be that a parentless commit gets rebased, the
root commit of "importing/master".

Regards,
   Fabian

 git-rebase--merge.sh          |  8 +++++++-
 t/t3400-rebase.sh             | 12 ++++++++++++
 t/t3402-rebase-merge.sh       | 12 ++++++++++++
 t/t3404-rebase-interactive.sh | 10 ++++++++++
 4 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index d3fb67d..3f754ae 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -67,7 +67,13 @@ call_merge () {
 		GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
 	fi
 	test -z "$strategy" && strategy=recursive
-	eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+	base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)
+	if test -z "$base"
+	then
+		# the empty tree sha1
+		base=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+	fi
+	eval 'git-merge-$strategy' $strategy_opts '"$base" -- "$hd" "$cmt"'
 	rv=$?
 	case "$rv" in
 	0)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 47b5682..9b0b57f 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -10,6 +10,8 @@ among other things.
 '
 . ./test-lib.sh
 
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
 GIT_AUTHOR_NAME=author@name
 GIT_AUTHOR_EMAIL=bogus@email@address
 export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL
@@ -255,4 +257,14 @@ test_expect_success 'rebase commit with an ancient timestamp' '
 	grep "author .* 34567 +0600$" actual
 '
 
+test_expect_success 'rebase disconnected' '
+	test_when_finished reset_rebase &&
+	git checkout --orphan test-rebase-disconnected &&
+	git rm -rf . &&
+	test_commit disconnected &&
+	git rebase master &&
+	test_path_is_file disconnected.t &&
+	test_cmp_rev master HEAD^
+'
+
 test_done
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 5a27ec9..1653540 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -7,6 +7,8 @@ test_description='git rebase --merge test'
 
 . ./test-lib.sh
 
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
 T="A quick brown fox
 jumps over the lazy dog."
 for i in 1 2 3 4 5 6 7 8 9 10
@@ -153,4 +155,14 @@ test_expect_success 'rebase --skip works with two conflicts in a row' '
 	git rebase --skip
 '
 
+test_expect_success 'rebase --merge disconnected' '
+	test_when_finished reset_rebase &&
+	git checkout --orphan test-rebase-disconnected &&
+	git rm -rf . &&
+	test_commit disconnected &&
+	git rebase --merge master &&
+	test_path_is_file disconnected.t &&
+	test_cmp_rev master HEAD^
+'
+
 test_done
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8197ed2..858c036 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1039,4 +1039,14 @@ test_expect_success 'short SHA-1 collide' '
 	)
 '
 
+test_expect_success 'rebase --interactive disconnected' '
+	test_when_finished reset_rebase &&
+	git checkout --orphan test-rebase-disconnected &&
+	git rm -rf . &&
+	test_commit disconnected &&
+	EDITOR=true git rebase --interactive master &&
+	test_path_is_file disconnected.t &&
+	test_cmp_rev master HEAD^
+'
+
 test_done
-- 
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]