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

> 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@redhat.com>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html