{"thread":{"id":"37683","subject":"Apparent bug in git rebase with a merge commit","startedAt":"2014-10-07T18:30:06Z","lastAt":"2014-10-13T18:43:51Z","messageCount":8,"participants":["David M. Lloyd","Fabian Ruch","Junio C Hamano","Derek Moore"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"250292","messageId":"5434312E.6040407@redhat.com","threadId":"37683","inReplyTo":null,"subject":"Apparent bug in git rebase with a merge commit","fromName":"David M. Lloyd","fromEmail":"david.lloyd@redhat.com","sentAt":"2014-10-07T18:30:06Z","receivedAt":"2014-10-07T18:30:06Z","isPatch":false,"sender":{"key":"david.lloyd@redhat.com","avatar":"https://gravatar.com/avatar/f1e98e7f01e943b78c4742d20263a2b3bf283775819308ca6e9aabccd2bd5dd8?d=mp&s=160"},"body":"If you have a git tree and you merge in another, independent git tree so \nthat they are the same, using a merge strategy like this:\n\n$ git merge importing/master -s recursive -Xours\n\nAnd if you later on want to rebase this merge commit on a newer upstream \nfor whatever reason, you get something like this:\n\n$ git rebase -s recursive -Xours\nFirst, rewinding head to replay your work on top of it...\nfatal: Could not parse object 'ca59931ee67fc01b4db4278600d3d92aece898f4^'\nUnknown exit code (128) from command: git-merge-recursive \nca59931ee67fc01b4db4278600d3d92aece898f4^ -- HEAD \nca59931ee67fc01b4db4278600d3d92aece898f4\n\nThe reason this occurs is that the first commit of the newly-merged-in \ncode obviously has no parent, so I guess the search for the common \nancestor is going to be doomed to fail.\n\nIt is possible that I'm misunderstanding the recursive merge strategy; \nhowever if this were the case I'd still expect a human-readable error \nmessage explaining my mistake rather than a 128 exit code.\n\nFor a workaround I'll just re-create the commit, but I thought I'd \nreport this behavior anyway.\n-- \n- DML\n"},{"id":"250435","messageId":"bf0e177fbaac91f8c55526729e580fade9f0f395.1412879523.git.bafain@gmail.com","threadId":"37683","inReplyTo":"5434312E.6040407@redhat.com","subject":"[PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-10-09T18:50:02Z","receivedAt":"2014-10-09T18:50:02Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"When the user specifies a merge strategy, `git-merge-$strategy` is\nused in non-interactive mode to replay the changes introduced by the\ncurrent branch relative to some upstream. Specifically, for each\ncommit `c` that is not in upstream the changes that led from `c^` to\n`c` are reapplied.\n\nIf the current has a different root than the upstream, either because\nthe history is disconnected or merged in a disconnected history, then\nthere will be a parentless commit `c` and `c^` will not refer to a\ncommit.\n\nIn order to cope with such a situation, check for every `c` whether\nits list of parents is empty. If it is empty, determine the\nintroduced changes by comparing the committed tree to the empty tree\ninstead. Otherwise, take the differences between `c^` and `c` as\nbefore.\n\nThe other git-rebase modes do not have similar problems because they\nuse git-cherry-pick to replay changes, even with strategy options. It\nseems that the non-interactive rebase with merge strategies was not\nimplemented using git-cherry-pick because it did not support them at\nthe time (`git rebase --merge` added in 58634dbf and `git cherry-pick\n--strategy` added in 91e52598). The idea of using the empty tree as\nreference tree for orphan commits is taken from the git-cherry-pick\nimplementation.\n\nRegarding the patch, we do not have to commit the empty tree before\nwe can pass it as a base argument to `git-merge-$strategy` because\ntree objects are recognized as such and implicitly committed by\n`git-merge-$strategy`.\n\nAdd a test. The test case rebases a single disconnected commit which\ncreates an isolated file on master and, therefore, does not require a\nspecific merge strategy. It is a mere sanity check.\n\nReported-by: David M. Lloyd <david.lloyd@redhat.com>\nSigned-off-by: Fabian Ruch <bafain@gmail.com>\n---\nHi David,\n\nI don't think you made a mistake at all. If I understand the --merge\nmode of git-rebase correctly there is no need to require a parent.\nThe error occurs when the script tries to determine the changes your\nmerge commit introduces, which includes the whole \"importing/master\"\nbranch. The strategy is not yet part of the picture then and will not\nbe until the changes are being replayed.\n\nThe test case tries to simplify your scenario because the relevant\ncharacteristic seems to be that a parentless commit gets rebased, the\nroot commit of \"importing/master\".\n\nRegards,\n   Fabian\n\n git-rebase--merge.sh          |  8 +++++++-\n t/t3400-rebase.sh             | 12 ++++++++++++\n t/t3402-rebase-merge.sh       | 12 ++++++++++++\n t/t3404-rebase-interactive.sh | 10 ++++++++++\n 4 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\nindex d3fb67d..3f754ae 100644\n--- a/git-rebase--merge.sh\n+++ b/git-rebase--merge.sh\n@@ -67,7 +67,13 @@ call_merge () {\n \t\tGIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY\n \tfi\n \ttest -z \"$strategy\" && strategy=recursive\n-\teval 'git-merge-$strategy' $strategy_opts '\"$cmt^\" -- \"$hd\" \"$cmt\"'\n+\tbase=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)\n+\tif test -z \"$base\"\n+\tthen\n+\t\t# the empty tree sha1\n+\t\tbase=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n+\tfi\n+\teval 'git-merge-$strategy' $strategy_opts '\"$base\" -- \"$hd\" \"$cmt\"'\n \trv=$?\n \tcase \"$rv\" in\n \t0)\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex 47b5682..9b0b57f 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -10,6 +10,8 @@ among other things.\n '\n . ./test-lib.sh\n \n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n GIT_AUTHOR_NAME=author@name\n GIT_AUTHOR_EMAIL=bogus@email@address\n export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n@@ -255,4 +257,14 @@ test_expect_success 'rebase commit with an ancient timestamp' '\n \tgrep \"author .* 34567 +0600$\" actual\n '\n \n+test_expect_success 'rebase disconnected' '\n+\ttest_when_finished reset_rebase &&\n+\tgit checkout --orphan test-rebase-disconnected &&\n+\tgit rm -rf . &&\n+\ttest_commit disconnected &&\n+\tgit rebase master &&\n+\ttest_path_is_file disconnected.t &&\n+\ttest_cmp_rev master HEAD^\n+'\n+\n test_done\ndiff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh\nindex 5a27ec9..1653540 100755\n--- a/t/t3402-rebase-merge.sh\n+++ b/t/t3402-rebase-merge.sh\n@@ -7,6 +7,8 @@ test_description='git rebase --merge test'\n \n . ./test-lib.sh\n \n+. \"$TEST_DIRECTORY\"/lib-rebase.sh\n+\n T=\"A quick brown fox\n jumps over the lazy dog.\"\n for i in 1 2 3 4 5 6 7 8 9 10\n@@ -153,4 +155,14 @@ test_expect_success 'rebase --skip works with two conflicts in a row' '\n \tgit rebase --skip\n '\n \n+test_expect_success 'rebase --merge disconnected' '\n+\ttest_when_finished reset_rebase &&\n+\tgit checkout --orphan test-rebase-disconnected &&\n+\tgit rm -rf . &&\n+\ttest_commit disconnected &&\n+\tgit rebase --merge master &&\n+\ttest_path_is_file disconnected.t &&\n+\ttest_cmp_rev master HEAD^\n+'\n+\n test_done\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 8197ed2..858c036 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1039,4 +1039,14 @@ test_expect_success 'short SHA-1 collide' '\n \t)\n '\n \n+test_expect_success 'rebase --interactive disconnected' '\n+\ttest_when_finished reset_rebase &&\n+\tgit checkout --orphan test-rebase-disconnected &&\n+\tgit rm -rf . &&\n+\ttest_commit disconnected &&\n+\tEDITOR=true git rebase --interactive master &&\n+\ttest_path_is_file disconnected.t &&\n+\ttest_cmp_rev master HEAD^\n+'\n+\n test_done\n-- \n2.1.1\n"},{"id":"250437","messageId":"xmqq1tqh6p3y.fsf@gitster.dls.corp.google.com","threadId":"37683","inReplyTo":"bf0e177fbaac91f8c55526729e580fade9f0f395.1412879523.git.bafain@gmail.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T19:05:05Z","receivedAt":"2014-10-09T19:05:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Ruch <bafain@gmail.com> writes:\n\n> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\n> index d3fb67d..3f754ae 100644\n> --- a/git-rebase--merge.sh\n> +++ b/git-rebase--merge.sh\n> @@ -67,7 +67,13 @@ call_merge () {\n>  \t\tGIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY\n>  \tfi\n>  \ttest -z \"$strategy\" && strategy=recursive\n> -\teval 'git-merge-$strategy' $strategy_opts '\"$cmt^\" -- \"$hd\" \"$cmt\"'\n> +\tbase=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)\n> +\tif test -z \"$base\"\n> +\tthen\n> +\t\t# the empty tree sha1\n> +\t\tbase=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n> +\tfi\n> +\teval 'git-merge-$strategy' $strategy_opts '\"$base\" -- \"$hd\" \"$cmt\"'\n\nThis looks wrong.\n\nThe interface to \"git-merge-$strategy\" is designed in such a way\nthat each strategy should be capable of taking _no_ base at all.\n\nSee how unquoted $common is given to git-merge-$strategy in\ncontrib/examples/git-merge.sh, i.e.\n\n    eval 'git-merge-$strategy '\"$xopt\"' $common -- \"$head_arg\" \"$@\"'\n\nwhere common comes from\n\n\tcommon=$(git merge-base ...)\n\nwhich would be empty when you are looking at disjoint histories.\n\nAlso rev-list piped to cut is too ugly to live in our codebase X-<.\n\nWouldn't it be sufficient to do something like this instead?\n\n\teval 'git-merge-$strategy' $strategy_opts \\\n        \t$(git rev-parse --quiet --verify \"$cmt^\") -- \"$hd\" \"$cmt\"\n"},{"id":"250438","messageId":"CAMsgyKbzNh3nx6m59JBnKjTmk9sFfqP95jtYH0uC3nW83SPiwA@mail.gmail.com","threadId":"37683","inReplyTo":"bf0e177fbaac91f8c55526729e580fade9f0f395.1412879523.git.bafain@gmail.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Derek Moore","fromEmail":"derek.p.moore@gmail.com","sentAt":"2014-10-09T19:06:33Z","receivedAt":"2014-10-09T19:06:33Z","isPatch":true,"sender":{"key":"derek.p.moore@gmail.com","avatar":"https://gravatar.com/avatar/4bf86633cdd04eb5d07180791de5ae0ece9f3d04b34f6e13fdb81d563fc62c23?d=mp&s=160"},"body":"Should perhaps you be using some symbolic method of referencing the\nempty tree instead of referencing a magic number?\n\nE.g., https://git.wiki.kernel.org/index.php/Aliases#Obtaining_the_Empty_Tree_SHA1\n\nOn Thu, Oct 9, 2014 at 1:50 PM, Fabian Ruch <bafain@gmail.com> wrote:\n> When the user specifies a merge strategy, `git-merge-$strategy` is\n> used in non-interactive mode to replay the changes introduced by the\n> current branch relative to some upstream. Specifically, for each\n> commit `c` that is not in upstream the changes that led from `c^` to\n> `c` are reapplied.\n>\n> If the current has a different root than the upstream, either because\n> the history is disconnected or merged in a disconnected history, then\n> there will be a parentless commit `c` and `c^` will not refer to a\n> commit.\n>\n> In order to cope with such a situation, check for every `c` whether\n> its list of parents is empty. If it is empty, determine the\n> introduced changes by comparing the committed tree to the empty tree\n> instead. Otherwise, take the differences between `c^` and `c` as\n> before.\n>\n> The other git-rebase modes do not have similar problems because they\n> use git-cherry-pick to replay changes, even with strategy options. It\n> seems that the non-interactive rebase with merge strategies was not\n> implemented using git-cherry-pick because it did not support them at\n> the time (`git rebase --merge` added in 58634dbf and `git cherry-pick\n> --strategy` added in 91e52598). The idea of using the empty tree as\n> reference tree for orphan commits is taken from the git-cherry-pick\n> implementation.\n>\n> Regarding the patch, we do not have to commit the empty tree before\n> we can pass it as a base argument to `git-merge-$strategy` because\n> tree objects are recognized as such and implicitly committed by\n> `git-merge-$strategy`.\n>\n> Add a test. The test case rebases a single disconnected commit which\n> creates an isolated file on master and, therefore, does not require a\n> specific merge strategy. It is a mere sanity check.\n>\n> Reported-by: David M. Lloyd <david.lloyd@redhat.com>\n> Signed-off-by: Fabian Ruch <bafain@gmail.com>\n> ---\n> Hi David,\n>\n> I don't think you made a mistake at all. If I understand the --merge\n> mode of git-rebase correctly there is no need to require a parent.\n> The error occurs when the script tries to determine the changes your\n> merge commit introduces, which includes the whole \"importing/master\"\n> branch. The strategy is not yet part of the picture then and will not\n> be until the changes are being replayed.\n>\n> The test case tries to simplify your scenario because the relevant\n> characteristic seems to be that a parentless commit gets rebased, the\n> root commit of \"importing/master\".\n>\n> Regards,\n>    Fabian\n>\n>  git-rebase--merge.sh          |  8 +++++++-\n>  t/t3400-rebase.sh             | 12 ++++++++++++\n>  t/t3402-rebase-merge.sh       | 12 ++++++++++++\n>  t/t3404-rebase-interactive.sh | 10 ++++++++++\n>  4 files changed, 41 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\n> index d3fb67d..3f754ae 100644\n> --- a/git-rebase--merge.sh\n> +++ b/git-rebase--merge.sh\n> @@ -67,7 +67,13 @@ call_merge () {\n>                 GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY\n>         fi\n>         test -z \"$strategy\" && strategy=recursive\n> -       eval 'git-merge-$strategy' $strategy_opts '\"$cmt^\" -- \"$hd\" \"$cmt\"'\n> +       base=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)\n> +       if test -z \"$base\"\n> +       then\n> +               # the empty tree sha1\n> +               base=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n> +       fi\n> +       eval 'git-merge-$strategy' $strategy_opts '\"$base\" -- \"$hd\" \"$cmt\"'\n>         rv=$?\n>         case \"$rv\" in\n>         0)\n> diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\n> index 47b5682..9b0b57f 100755\n> --- a/t/t3400-rebase.sh\n> +++ b/t/t3400-rebase.sh\n> @@ -10,6 +10,8 @@ among other things.\n>  '\n>  . ./test-lib.sh\n>\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n> +\n>  GIT_AUTHOR_NAME=author@name\n>  GIT_AUTHOR_EMAIL=bogus@email@address\n>  export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL\n> @@ -255,4 +257,14 @@ test_expect_success 'rebase commit with an ancient timestamp' '\n>         grep \"author .* 34567 +0600$\" actual\n>  '\n>\n> +test_expect_success 'rebase disconnected' '\n> +       test_when_finished reset_rebase &&\n> +       git checkout --orphan test-rebase-disconnected &&\n> +       git rm -rf . &&\n> +       test_commit disconnected &&\n> +       git rebase master &&\n> +       test_path_is_file disconnected.t &&\n> +       test_cmp_rev master HEAD^\n> +'\n> +\n>  test_done\n> diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh\n> index 5a27ec9..1653540 100755\n> --- a/t/t3402-rebase-merge.sh\n> +++ b/t/t3402-rebase-merge.sh\n> @@ -7,6 +7,8 @@ test_description='git rebase --merge test'\n>\n>  . ./test-lib.sh\n>\n> +. \"$TEST_DIRECTORY\"/lib-rebase.sh\n> +\n>  T=\"A quick brown fox\n>  jumps over the lazy dog.\"\n>  for i in 1 2 3 4 5 6 7 8 9 10\n> @@ -153,4 +155,14 @@ test_expect_success 'rebase --skip works with two conflicts in a row' '\n>         git rebase --skip\n>  '\n>\n> +test_expect_success 'rebase --merge disconnected' '\n> +       test_when_finished reset_rebase &&\n> +       git checkout --orphan test-rebase-disconnected &&\n> +       git rm -rf . &&\n> +       test_commit disconnected &&\n> +       git rebase --merge master &&\n> +       test_path_is_file disconnected.t &&\n> +       test_cmp_rev master HEAD^\n> +'\n> +\n>  test_done\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 8197ed2..858c036 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1039,4 +1039,14 @@ test_expect_success 'short SHA-1 collide' '\n>         )\n>  '\n>\n> +test_expect_success 'rebase --interactive disconnected' '\n> +       test_when_finished reset_rebase &&\n> +       git checkout --orphan test-rebase-disconnected &&\n> +       git rm -rf . &&\n> +       test_commit disconnected &&\n> +       EDITOR=true git rebase --interactive master &&\n> +       test_path_is_file disconnected.t &&\n> +       test_cmp_rev master HEAD^\n> +'\n> +\n>  test_done\n> --\n> 2.1.1\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"250441","messageId":"5436E004.20103@redhat.com","threadId":"37683","inReplyTo":"bf0e177fbaac91f8c55526729e580fade9f0f395.1412879523.git.bafain@gmail.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"David M. Lloyd","fromEmail":"david.lloyd@redhat.com","sentAt":"2014-10-09T19:20:36Z","receivedAt":"2014-10-09T19:20:36Z","isPatch":true,"sender":{"key":"david.lloyd@redhat.com","avatar":"https://gravatar.com/avatar/f1e98e7f01e943b78c4742d20263a2b3bf283775819308ca6e9aabccd2bd5dd8?d=mp&s=160"},"body":"On 10/09/2014 01:50 PM, Fabian Ruch wrote:\n> Hi David,\n>\n> I don't think you made a mistake at all. If I understand the --merge\n> mode of git-rebase correctly there is no need to require a parent.\n> The error occurs when the script tries to determine the changes your\n> merge commit introduces, which includes the whole \"importing/master\"\n> branch. The strategy is not yet part of the picture then and will not\n> be until the changes are being replayed.\n\nThank you for your prompt response, clear explanation, and patch even! \nI'm afraid my understanding of the git internals is next-to-nil.  But \nI'm glad I could contribute to helping to improve it in some small way.\n\n-- \n- DML\n"},{"id":"250452","messageId":"5436E83A.7070603@gmail.com","threadId":"37683","inReplyTo":"xmqq1tqh6p3y.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-10-09T19:55:38Z","receivedAt":"2014-10-09T19:55:38Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"Hi Junio,\n\nOn 10/09/2014 09:05 PM, Junio C Hamano wrote:\n> Fabian Ruch <bafain@gmail.com> writes:\n>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\n>> index d3fb67d..3f754ae 100644\n>> --- a/git-rebase--merge.sh\n>> +++ b/git-rebase--merge.sh\n>> @@ -67,7 +67,13 @@ call_merge () {\n>>  \t\tGIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY\n>>  \tfi\n>>  \ttest -z \"$strategy\" && strategy=recursive\n>> -\teval 'git-merge-$strategy' $strategy_opts '\"$cmt^\" -- \"$hd\" \"$cmt\"'\n>> +\tbase=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)\n>> +\tif test -z \"$base\"\n>> +\tthen\n>> +\t\t# the empty tree sha1\n>> +\t\tbase=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n>> +\tfi\n>> +\teval 'git-merge-$strategy' $strategy_opts '\"$base\" -- \"$hd\" \"$cmt\"'\n> \n> This looks wrong.\n\nOk.\n\n> The interface to \"git-merge-$strategy\" is designed in such a way\n> that each strategy should be capable of taking _no_ base at all.\n\nThe merge strategies \"resolve\" and \"octopus\" seem to refuse to run if no\nbase is specified. The former silently exits if no bases are given and\nthe latter dies saying \"Unable to find common commit\".\n\n> See how unquoted $common is given to git-merge-$strategy in\n> contrib/examples/git-merge.sh, i.e.\n> \n>     eval 'git-merge-$strategy '\"$xopt\"' $common -- \"$head_arg\" \"$@\"'\n> \n> where common comes from\n> \n> \tcommon=$(git merge-base ...)\n> \n> which would be empty when you are looking at disjoint histories.\n> \n> Also rev-list piped to cut is too ugly to live in our codebase X-<.\n\nIs there a better way to get the parents list from a shell script then?\nI stole the construct from git-rebase--interactive.sh which uses it to\ncheck for rewritten parents when preserving merges.\n\n> Wouldn't it be sufficient to do something like this instead?\n> \n> \teval 'git-merge-$strategy' $strategy_opts \\\n>         \t$(git rev-parse --quiet --verify \"$cmt^\") -- \"$hd\" \"$cmt\"\n\nYes, for the \"recursive\" strategies this seems to have the exact same\nbehaviour as it inserts the empty tree in case git-merge-base returns an\nempty list. Nice, we would get rid of both the magic number and the cut.\n\nRegards,\n   Fabian\n"},{"id":"250456","messageId":"xmqqoatl574t.fsf@gitster.dls.corp.google.com","threadId":"37683","inReplyTo":"5436E83A.7070603@gmail.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-09T20:18:42Z","receivedAt":"2014-10-09T20:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Fabian Ruch <bafain@gmail.com> writes:\n\n>> The interface to \"git-merge-$strategy\" is designed in such a way\n>> that each strategy should be capable of taking _no_ base at all.\n>\n> The merge strategies \"resolve\" and \"octopus\" seem to refuse to run if no\n> base is specified. The former silently exits if no bases are given and\n> the latter dies saying \"Unable to find common commit\".\n\nThat just means these two strategies are not prepared to do a merge\nwithout base (yet).  It does not automatically give license to the\ncaller to pass a random tree as if it is the merge base the user\nwanted to use.\n\nFor \"resolve\", I think it is OK for it to detect that the caller did\nnot give a common ancestor tree and use an empty tree when merging\n(which is what merge-recursive ends up doing internally).\n\nI am not offhand sure if the same is sensible for \"octopus\", though.\n"},{"id":"250563","messageId":"543C1D67.80501@gmail.com","threadId":"37683","inReplyTo":"xmqq1tqh6p3y.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v1] rebase -m: Use empty tree base for parentless commits","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-10-13T18:43:51Z","receivedAt":"2014-10-13T18:43:51Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"Hi,\n\nJunio C Hamano writes:\n> Fabian Ruch <bafain@gmail.com> writes:\n>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\n>> index d3fb67d..3f754ae 100644\n>> --- a/git-rebase--merge.sh\n>> +++ b/git-rebase--merge.sh\n>> @@ -67,7 +67,13 @@ call_merge () {\n>>  \t\tGIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY\n>>  \tfi\n>>  \ttest -z \"$strategy\" && strategy=recursive\n>> -\teval 'git-merge-$strategy' $strategy_opts '\"$cmt^\" -- \"$hd\" \"$cmt\"'\n>> +\tbase=$(git rev-list --parents -1 $cmt | cut -d ' ' -s -f 2 -)\n>> +\tif test -z \"$base\"\n>> +\tthen\n>> +\t\t# the empty tree sha1\n>> +\t\tbase=4b825dc642cb6eb9a060e54bf8d69288fbee4904\n>> +\tfi\n>> +\teval 'git-merge-$strategy' $strategy_opts '\"$base\" -- \"$hd\" \"$cmt\"'\n> \n> This looks wrong.\n> \n> The interface to \"git-merge-$strategy\" is designed in such a way\n> that each strategy should be capable of taking _no_ base at all.\n\nOk, but doesn't this use of the git-merge-$strategy interface (as shown\nin the example below) apply only to the case where one wants to merge\ntwo histories by creating a merge commit? When a merge commit is being\ncreated, the documentation states that git-merge abstracts from the\ncommit history considering the _total change_ since a merge base on each\nbranch.\n\nIn contrast, here (i.e., in the case of git-rebase--merge) we care about\nhow the changes introduced by the _individual commits_ are applied.\nTherefore, don't we want to be explicit about the \"base\" and tell\ngit-merge-$strategy exactly which changes it should merge into the\ncurrent head?\n\nThe codebase has always been doing this both for git-rebase--merge and\ngit-cherry-pick. What leads to the reported bug is that the latter\ncovers the case where the commit object has no parents but the former\ndoesn't. Root commits are handled by git-cherry-pick (and should be by\ngit-rebase--merge) using an explicit \"base\" for the same reason why\n$cmt^ is given.\n\n> See how unquoted $common is given to git-merge-$strategy in\n> contrib/examples/git-merge.sh, i.e.\n> \n>     eval 'git-merge-$strategy '\"$xopt\"' $common -- \"$head_arg\" \"$@\"'\n> \n> where common comes from\n> \n> \tcommon=$(git merge-base ...)\n> \n> which would be empty when you are looking at disjoint histories.\n\nIf there are still objections to the patch because of the magic number\nand the cut, it might be worth considering an implementation of\ngit-rebase--merge using git-cherry-pick's merge strategy option.\n\n   Fabian\n"}]}