{"thread":{"id":"41013","subject":"Odd rebase behavior","startedAt":"2015-12-16T03:17:30Z","lastAt":"2015-12-19T16:09:06Z","messageCount":6,"participants":["David A. Greene","John Keeping","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"274546","messageId":"877fkf9j5h.fsf@waller.obbligato.org","threadId":"41013","inReplyTo":null,"subject":"Odd rebase behavior","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2015-12-16T03:17:30Z","receivedAt":"2015-12-16T03:17:30Z","isPatch":false,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"Hi,\n\nThe attached tests do not do what I expected them to do.  I commented\nout the tests involving the new rebase empty commit behavior I just\nsent.  The uncommented tests show the strange behavior.\n\nAccording to the rebase man page, rebase gathers commits as in \"git log\n<upstream>..HEAD.\"  However, that is not what happens in the tests\nbelow.  Some of the commits disappear.\n\nThe test basically does this:\n\n- Setup a master project and a subproject, merged via a subtree-like\n  merge (this is how git-subtree does it).\n\n- Add some commits to the subproject directory after the subtree merge,\n  to create some history not in the original subproject.\n\n- filter-branch --subdirectory-filter to extract commits from the\n  subproject directory.\n\n- Rebase those commits back on to the original subproject repository.\n\nThe above loses all commits made after the subproject is merged into\nthe main project.\n\nNote that the rebase is a little wonky.  filter-branch creates a\ndisconnected graph and the rebase is invoked with <upstream>=master.\nI'm not sure how rebase is supposed to operate in this case (if it is\nsupported at all) but it definitely is not doing the master..HEAD thing.\n\nReplacing \"master\" with \"--root\" causes rebase to do the right thing.\n\nThis seems like a bug to me, even with the strange <upstream> on a\ndisconnected graph.  At the very least git should not silently lose\ncommits.\n\nI can think of two ways this could be resolved:\n\n- Forbid this kind of operation and error our with a message (when\n  <upstream> and HEAD do not share ancestry)\n\n- Make it work as if --root were specified\n\nThoughts?\n\n                         -David\n\n--->8---\n\n#!/bin/sh\n\ntest_description='git rebase tests for empty commits\n\nThis test runs git rebase and tests handling of empty commits.\n'\n. ./test-lib.sh\n\naddfile() {\n    name=$1\n    echo $(basename ${name}) > ${name}\n    ${git} add ${name}\n    ${git} commit -m \"Add $(basename ${name})\"\n}\n\ncheck_equal()\n{\n\ttest_debug 'echo'\n\ttest_debug \"echo \\\"check a:\\\" \\\"{$1}\\\"\"\n\ttest_debug \"echo \\\"      b:\\\" \\\"{$2}\\\"\"\n\tif [ \"$1\" = \"$2\" ]; then\n\t\treturn 0\n\telse\n\t\treturn 1\n\tfi\n}\n\nlast_commit_message()\n{\n\tgit log --pretty=format:%s -1\n}\n\ntest_expect_success 'setup' '\n\ttest_commit README &&\n\tmkdir files &&\n\tcd files &&\n\tgit init &&\n\ttest_commit master1 &&\n\ttest_commit master2 &&\n\ttest_commit master3 &&\n\tcd .. &&\n\ttest_debug \"echo Add project master to master\" &&\n\tgit fetch files master &&\n\tgit branch files-master FETCH_HEAD &&\n\ttest_debug \"echo Add subtree master to master via subtree\" &&\n\tgit read-tree --prefix=files_subtree files-master &&\n\tgit checkout -- files_subtree &&\n\ttree=$(git write-tree) &&\n\thead=$(git rev-parse HEAD) &&\n\trev=$(git rev-parse --verify files-master^0) &&\n\tcommit=$(git commit-tree -p ${head} -p ${rev} -m \"Add subproject master\" ${tree}) &&\n\tgit reset ${commit} &&\n\tcd files_subtree &&\n\ttest_commit master4 &&\n\tcd .. &&\n\ttest_commit files_subtree/master5\n'\n\n# Does not preserve master4 and master5.\ntest_expect_success 'Rebase default' '\n\tgit checkout -b rebase-default master &&\n\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n\tgit commit -m \"Empty commit\" --allow-empty &&\n\tgit rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n'\n\n# Does not preserve master4, master5 and empty.\ntest_expect_success 'Rebase --keep-empty' '\n\tgit checkout -b rebase-keep-empty master &&\n\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n\tgit commit -m \"Empty commit\" --allow-empty &&\n\tgit rebase -Xsubtree=files_subtree --keep-empty --preserve-merges --onto files-master master &&\n\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n'\n\n\n# Does not preserve master4 and master5.\n#test_expect_success 'Rebase --keep-redundant' '\n#\tgit checkout -b rebase-keep-redundant master &&\n#\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n#\tgit commit -m \"Empty commit\" --allow-empty &&\n#\tgit rebase -Xsubtree=files_subtree --keep-redundant --preserve-merges --onto files-master master &&\n#\tcheck_equal \"$(last_commit_message)\" \"files_subtree/master5\"\n#'\n\n\n# Does not preserve master4, master5 and empty.\n#test_expect_success 'Rebase --keep-empty --keep-redundant' '\n#\tgit checkout -b rebase-keep-empty-keep-redundant master &&\n#\tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n#\tgit commit -m \"Empty commit\" --allow-empty &&\n#\tgit rebase -Xsubtree=files_subtree --keep-empty --keep-redundant --preserve-merges --onto files-master master &&\n#\tcheck_equal \"$(last_commit_message)\" \"Empty commit\"\n#'\n\n\ntest_done\n"},{"id":"274630","messageId":"20151216221716.GD1581@serenity.lan","threadId":"41013","inReplyTo":"877fkf9j5h.fsf@waller.obbligato.org","subject":"Re: Odd rebase behavior","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2015-12-16T22:17:16Z","receivedAt":"2015-12-16T22:17:16Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Dec 15, 2015 at 09:17:30PM -0600, David A. Greene wrote:\n> According to the rebase man page, rebase gathers commits as in \"git log\n> <upstream>..HEAD.\"  However, that is not what happens in the tests\n> below.  Some of the commits disappear.\n> \n> The test basically does this:\n> \n> - Setup a master project and a subproject, merged via a subtree-like\n>   merge (this is how git-subtree does it).\n> \n> - Add some commits to the subproject directory after the subtree merge,\n>   to create some history not in the original subproject.\n> \n> - filter-branch --subdirectory-filter to extract commits from the\n>   subproject directory.\n> \n> - Rebase those commits back on to the original subproject repository.\n> \n> The above loses all commits made after the subproject is merged into\n> the main project.\n[snip]\n> # Does not preserve master4 and master5.\n> test_expect_success 'Rebase default' '\n> \tgit checkout -b rebase-default master &&\n> \tgit filter-branch --prune-empty -f --subdirectory-filter files_subtree &&\n> \tgit commit -m \"Empty commit\" --allow-empty &&\n> \tgit rebase -Xsubtree=files_subtree  --preserve-merges --onto files-master master &&\n\nIt seems that the problem is introduces by --preserve-merges (and\n-Xsubtree causes something interesting to happen as well).  I see the\nfollowing behaviour:\n\ngit rebase --onto files-master master\n\n\tWorks (master4 and master5 preserved).\n\ngit rebase --preserve-merges --onto files-master master\n\n\tBehaves as described above (master4 and master5 are lost).\n\ngit rebase -Xsubtree=files_subtree --onto files-master master\n\n\tfatal: Could not parse object 'b15c4133fc3146e1330c84159886f0f7a09fbf43^'\n\tUnknown exit code (128) from command: git-merge-recursive b15c4133fc3146e1330c84159886f0f7a09fbf43^ -- HEAD b15c4133fc3146e1330c84159886f0f7a09fbf43 \n\ngit rebase -Xsubtree=files_subtree --preserve-merges --onto files-master master\n\n\tSame as the version with only --preserve-merges.\n"},{"id":"274723","messageId":"nngmvt73b63.fsf@lnx-dag.us.cray.com","threadId":"41013","inReplyTo":"20151216221716.GD1581@serenity.lan","subject":"Re: Odd rebase behavior","fromName":"David A. Greene","fromEmail":"greened@obbligato.org","sentAt":"2015-12-18T17:43:16Z","receivedAt":"2015-12-18T17:43:16Z","isPatch":false,"sender":{"key":"greened@obbligato.org","avatar":"https://avatars.githubusercontent.com/u/5291869?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> It seems that the problem is introduces by --preserve-merges (and\n> -Xsubtree causes something interesting to happen as well).  I see the\n> following behaviour:\n\nThanks for narrowing this down!  Is it possible this is actually a\ncherry-pick problem since --preserve-merges forces rebase to use\ncherry-pick?\n\n> git rebase -Xsubtree=files_subtree --onto files-master master\n>\n> \tfatal: Could not parse object 'b15c4133fc3146e1330c84159886f0f7a09fbf43^'\n> \tUnknown exit code (128) from command: git-merge-recursive\n> b15c4133fc3146e1330c84159886f0f7a09fbf43^ -- HEAD\n> b15c4133fc3146e1330c84159886f0f7a09fbf43\n\nAh, good!  I had seen this behavior as well but couldn't remember what I\ndid to trigger it.\n\nI don't think I have the expertise to fix rebase and/or cherry-pick.\nWhat's the process for adding these tests to the testbase and marking\nthem so the appropriate person can fix them?  I see a lot of TODO tests.\nShould I mark these similarly and propose a patch to the testbase?\n\n                             -David\n"},{"id":"274724","messageId":"20151218180549.GA14056@serenity.lan","threadId":"41013","inReplyTo":"nngmvt73b63.fsf@lnx-dag.us.cray.com","subject":"Re: Odd rebase behavior","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2015-12-18T18:05:49Z","receivedAt":"2015-12-18T18:05:49Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Dec 18, 2015 at 11:43:16AM -0600, David A. Greene wrote:\n> John Keeping <john@keeping.me.uk> writes:\n> \n> > It seems that the problem is introduces by --preserve-merges (and\n> > -Xsubtree causes something interesting to happen as well).  I see the\n> > following behaviour:\n> \n> Thanks for narrowing this down!  Is it possible this is actually a\n> cherry-pick problem since --preserve-merges forces rebase to use\n> cherry-pick?\n\nI'm pretty sure this a result of the code in git-rebase--interactive.sh\njust below the comment \"Watch for commits that have been dropped by\ncherry-pick\", which filters out certain commits.  However, I'm not at\nall familiar with the --preserve-merges code in git-rebase so I could be\ncompletely wrong.\n\n> > git rebase -Xsubtree=files_subtree --onto files-master master\n> >\n> > \tfatal: Could not parse object 'b15c4133fc3146e1330c84159886f0f7a09fbf43^'\n> > \tUnknown exit code (128) from command: git-merge-recursive\n> > b15c4133fc3146e1330c84159886f0f7a09fbf43^ -- HEAD\n> > b15c4133fc3146e1330c84159886f0f7a09fbf43\n> \n> Ah, good!  I had seen this behavior as well but couldn't remember what I\n> did to trigger it.\n> \n> I don't think I have the expertise to fix rebase and/or cherry-pick.\n> What's the process for adding these tests to the testbase and marking\n> them so the appropriate person can fix them?  I see a lot of TODO tests.\n> Should I mark these similarly and propose a patch to the testbase?\n\nI think marking them with test_expect_failure (instead of\ntest_expect_success) is enough.\n"},{"id":"274728","messageId":"56747521.9090701@kdbg.org","threadId":"41013","inReplyTo":"20151218180549.GA14056@serenity.lan","subject":"Re: Odd rebase behavior","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-12-18T21:05:37Z","receivedAt":"2015-12-18T21:05:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 18.12.2015 um 19:05 schrieb John Keeping:\n> On Fri, Dec 18, 2015 at 11:43:16AM -0600, David A. Greene wrote:\n>> John Keeping <john@keeping.me.uk> writes:\n>>\n>>> It seems that the problem is introduces by --preserve-merges (and\n>>> -Xsubtree causes something interesting to happen as well).  I see the\n>>> following behaviour:\n>>\n>> Thanks for narrowing this down!  Is it possible this is actually a\n>> cherry-pick problem since --preserve-merges forces rebase to use\n>> cherry-pick?\n>\n> I'm pretty sure this a result of the code in git-rebase--interactive.sh\n> just below the comment \"Watch for commits that have been dropped by\n> cherry-pick\", which filters out certain commits.  However, I'm not at\n> all familiar with the --preserve-merges code in git-rebase so I could be\n> completely wrong.\n>\n>>> git rebase -Xsubtree=files_subtree --onto files-master master\n>>>\n>>> \tfatal: Could not parse object 'b15c4133fc3146e1330c84159886f0f7a09fbf43^'\n>>> \tUnknown exit code (128) from command: git-merge-recursive\n>>> b15c4133fc3146e1330c84159886f0f7a09fbf43^ -- HEAD\n>>> b15c4133fc3146e1330c84159886f0f7a09fbf43\n>>\n>> Ah, good!  I had seen this behavior as well but couldn't remember what I\n>> did to trigger it.\n>>\n>> I don't think I have the expertise to fix rebase and/or cherry-pick.\n>> What's the process for adding these tests to the testbase and marking\n>> them so the appropriate person can fix them?  I see a lot of TODO tests.\n>> Should I mark these similarly and propose a patch to the testbase?\n>\n> I think marking them with test_expect_failure (instead of\n> test_expect_success) is enough.\n>\n\nThere are a few known breakages recorded in \nt3512-cherry-pick-submodule.sh. Perhaps the one observed here is already \namong them?\n\n-- Hannes\n"},{"id":"274759","messageId":"20151219160906.GB14056@serenity.lan","threadId":"41013","inReplyTo":"20151218180549.GA14056@serenity.lan","subject":"Re: Odd rebase behavior","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2015-12-19T16:09:06Z","receivedAt":"2015-12-19T16:09:06Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Dec 18, 2015 at 06:05:49PM +0000, John Keeping wrote:\n> On Fri, Dec 18, 2015 at 11:43:16AM -0600, David A. Greene wrote:\n> > John Keeping <john@keeping.me.uk> writes:\n> > \n> > > It seems that the problem is introduces by --preserve-merges (and\n> > > -Xsubtree causes something interesting to happen as well).  I see the\n> > > following behaviour:\n> > \n> > Thanks for narrowing this down!  Is it possible this is actually a\n> > cherry-pick problem since --preserve-merges forces rebase to use\n> > cherry-pick?\n> \n> I'm pretty sure this a result of the code in git-rebase--interactive.sh\n> just below the comment \"Watch for commits that have been dropped by\n> cherry-pick\", which filters out certain commits.  However, I'm not at\n> all familiar with the --preserve-merges code in git-rebase so I could be\n> completely wrong.\n\nI've traced through git-rebase--interactive.sh and I can see what's\nhappening here now.  The problematic code is around the handling of the\n\"rewritten\" directory.\n\nIn --preserve-merges mode, we write the SHA1 of the \"onto\" commit into\nrewritten and then add any commits descended from it along the\nfirst-parent path that we have identified as candidates for being\nrebased.  This allows us to identify commits that have been merged in\nand remove them from the rebase instruction list.\n\nBecause the right-hand commit in this case is disjoint from \"onto\", we\nend up dropping everything at this point.  The --root option fixes this\nbecause instead of preserving just \"onto\", it adds all of the commits\ngiven by:\n\n\tgit merge-base --all $orig_head $upstream\n\nSince the disjoint history causes the root commit to be rewritten, I\nthink requiring --root for this case to work is reasonable.  However,\nI'm not sure we should add it automatically.  It may be better to detect\nthat there is no common history and die if --root has not been given.\n"}]}