{"thread":{"id":"35502","subject":"[RFC/PATCH] rebase: use reflog to find common base with upstream","startedAt":"2013-12-08T20:06:42Z","lastAt":"2013-12-09T23:16:16Z","messageCount":5,"participants":["John Keeping","Martin von Zweigbergk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231766","messageId":"9e5fa57b027e1a5cd11a456c14f43b64f8f5386c.1386531376.git.john@keeping.me.uk","threadId":"35502","inReplyTo":null,"subject":"[RFC/PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-12-08T20:06:42Z","receivedAt":"2013-12-08T20:06:42Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Commit 15a147e (rebase: use @{upstream} if no upstream specified,\n2011-02-09) says:\n\n\tMake it default to 'git rebase @{upstream}'. That is also what\n\t'git pull [--rebase]' defaults to, so it only makes sense that\n\t'git rebase' defaults to the same thing.\n\nbut that isn't actually the case. Since commit d44e712 (pull: support\nrebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually\nchosen the most recent reflog entry which is an ancestor of the current\nbranch if it can find one.\n\nChange rebase so that it uses the same logic, but only when no upstream\nargument is given on the command line.\n\nSigned-off-by: John Keeping \n---\nLast time this came up [1], there was some discussion about moving the\nadded block of code to affect upstreams given on the command line as\nwell as when the upstream is discovered from the config. Having tried\nthat, it has some more fallout on the test suite than I like; this\npattern ends up dropping the tip commit of \"side\" because it's in the\nreflog of \"master\":\n\n\t# start on \"master\"\n\tgit branch side &&\n\tgit reset --hard HEAD^ &&\n\tgit checkout side &&\n\tgit rebase master\n\nI wonder if it would be better to add a --fork-point argument to\ngit-rebase and default it to true when no upstream is given on the\ncommand line.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/236252\n\n git-rebase.sh | 6 ++++++\n t/t3400-rebase.sh | 6 ++++--\n 2 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 226752f..17801ad 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -437,6 +437,12 @@ then\n \t\t\terror_on_missing_default_upstream \"rebase\" \"rebase\" \\\n \t\t\t\t\"against\" \"git rebase \"\n \t\tfi\n+\n+\t\tfork_point=$(git merge-base --fork-point \"$upstream_name\" HEAD)\n+\t\tif test -n \"$fork_point\" && test \"$fork_point\" != \"$upstream\"\n+\t\tthen\n+\t\t\tupstream=$fork_point\n+\t\tfi\n \t\t;;\n \t*)\tupstream_name=\"$1\"\n \t\tshift\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex ebf93b0..998503d 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -134,12 +134,14 @@ test_expect_success 'fail when upstream arg is missing and not configured' '\n \ttest_must_fail git rebase\n '\n \n-test_expect_success 'default to @{upstream} when upstream arg is missing' '\n+test_expect_success 'default to common base in @{upstream}s reflog if no upstream arg' '\n \tgit checkout -b default topic &&\n \tgit config branch.default.remote . &&\n \tgit config branch.default.merge refs/heads/master &&\n \tgit rebase &&\n-\ttest \"$(git rev-parse default~1)\" = \"$(git rev-parse master)\"\n+\tgit rev-parse --verify master >expect &&\n+\tgit rev-parse default~1 >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'rebase -q is quiet' '\n-- \n1.8.5.226.g0d60d77\n"},{"id":"231775","messageId":"CANiSa6hnHdC5Rk9gBZd=9Hr74wjnnjrsoagwsv=HLQb5CM0AEg@mail.gmail.com","threadId":"35502","inReplyTo":"9e5fa57b027e1a5cd11a456c14f43b64f8f5386c.1386531376.git.john@keeping.me.uk","subject":"Re: [RFC/PATCH] rebase: use reflog to find common base with upstream","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-12-08T23:34:58Z","receivedAt":"2013-12-08T23:34:58Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Sun, Dec 8, 2013 at 12:06 PM, John Keeping wrote:\n> Commit 15a147e (rebase: use @{upstream} if no upstream specified,\n> 2011-02-09) says:\n>\n> Make it default to 'git rebase @{upstream}'. That is also what\n> 'git pull [--rebase]' defaults to, so it only makes sense that\n> 'git rebase' defaults to the same thing.\n>\n> but that isn't actually the case. Since commit d44e712 (pull: support\n> rebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually\n> chosen the most recent reflog entry which is an ancestor of the current\n> branch if it can find one.\n\nIn my mind, 'git pull --rebase' does default to @{u}, it just started\ninterpreting it differently in d44e712, but maybe I'm just being\ndefensive :-).\n\nIn a similar way, I think your patch is about interpreting the\nupstream argument differently, not about changing the default upstream\nargument. This is why I think \"git rebase\" and \"git rebase\norigin/master\" should be the same (when origin/master is the\nconfigured upstream).\n"},{"id":"231805","messageId":"xmqq7gbdzsvt.fsf@gitster.dls.corp.google.com","threadId":"35502","inReplyTo":"9e5fa57b027e1a5cd11a456c14f43b64f8f5386c.1386531376.git.john@keeping.me.uk","subject":"Re: [RFC/PATCH] rebase: use reflog to find common base with upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-09T20:11:50Z","receivedAt":"2013-12-09T20:11:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> Last time this came up [1], there was some discussion about moving the\n> added block of code to affect upstreams given on the command line as\n> well as when the upstream is discovered from the config. Having tried\n> that, it has some more fallout on the test suite than I like; this\n> pattern ends up dropping the tip commit of \"side\" because it's in the\n> reflog of \"master\":\n>\n> \t# start on \"master\"\n> \tgit branch side &&\n> \tgit reset --hard HEAD^ &&\n> \tgit checkout side &&\n> \tgit rebase master\n\nWe shouldn't do anything funky using the reflog when an explicit\ncommit object name was given like in the last step above, I think.\nAutomation to help human end-users is good, but at some level there\nmust be a mechanism to reliably reproduce the same result given the\nsame precondition for those who implement such automation, and I do\nnot think it is a good idea to force scripts to say\n\n\tgit rebase --do-not-look-at-reflog master\n\nin order to do so.\n\n> I wonder if it would be better to add a --fork-point argument to\n> git-rebase and default it to true when no upstream is given on the\n> command line.\n\nI am not sure what you exactly mean by \"when no upstream is given\",\nthough. Do you mean\n\n\tgit rebase \n\nwhich we interpret as \"rebase the current branch on @{u}\", and it\nshould behave as if the command was run like so:\n\n\tgit rebase --fork-point @{u}\n\nIf that is what you suggest, I certainly can buy that. Those who\nwant to disable the automation can explicitly say\n\n\tgit rebase @{u}\n\nand rebase the current exactly on top of the named commit (e.g. the\ncurrent value of refs/remotes/origin/master or whatever remote-tracking\nbranch you forked from).\n"},{"id":"231809","messageId":"20131209204008.GF3163@serenity.lan","threadId":"35502","inReplyTo":"xmqq7gbdzsvt.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC/PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-12-09T20:40:08Z","receivedAt":"2013-12-09T20:40:08Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Dec 09, 2013 at 12:11:50PM -0800, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > Last time this came up [1], there was some discussion about moving the\n> > added block of code to affect upstreams given on the command line as\n> > well as when the upstream is discovered from the config. Having tried\n> > that, it has some more fallout on the test suite than I like; this\n> > pattern ends up dropping the tip commit of \"side\" because it's in the\n> > reflog of \"master\":\n> >\n> > \t# start on \"master\"\n> > \tgit branch side &&\n> > \tgit reset --hard HEAD^ &&\n> > \tgit checkout side &&\n> > \tgit rebase master\n> \n> We shouldn't do anything funky using the reflog when an explicit\n> commit object name was given like in the last step above, I think.\n> Automation to help human end-users is good, but at some level there\n> must be a mechanism to reliably reproduce the same result given the\n> same precondition for those who implement such automation, and I do\n> not think it is a good idea to force scripts to say\n> \n> \tgit rebase --do-not-look-at-reflog master\n> \n> in order to do so.\n> \n> > I wonder if it would be better to add a --fork-point argument to\n> > git-rebase and default it to true when no upstream is given on the\n> > command line.\n> \n> I am not sure what you exactly mean by \"when no upstream is given\",\n> though. Do you mean\n> \n> \tgit rebase \n> \n> which we interpret as \"rebase the current branch on @{u}\", and it\n> should behave as if the command was run like so:\n> \n> \tgit rebase --fork-point @{u}\n> \n> If that is what you suggest, I certainly can buy that. Those who\n> want to disable the automation can explicitly say\n> \n> \tgit rebase @{u}\n> \n> and rebase the current exactly on top of the named commit (e.g. the\n> current value of refs/remotes/origin/master or whatever remote-tracking\n> branch you forked from).\n\nYes, that's what I meant; the first non-option argument to \"git rebase\"\nis called \"upstream\" in the manpage (and throughout the code). So if\n\"\" means \"\" then that's\nexactly what I meant.\n"},{"id":"231837","messageId":"20131209231616.GG3163@serenity.lan","threadId":"35502","inReplyTo":"20131209204008.GF3163@serenity.lan","subject":"[PATCH v2] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-12-09T23:16:16Z","receivedAt":"2013-12-09T23:16:16Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"Commit 15a147e (rebase: use @{upstream} if no upstream specified,\n2011-02-09) says:\n\n\tMake it default to 'git rebase @{upstream}'. That is also what\n\t'git pull [--rebase]' defaults to, so it only makes sense that\n\t'git rebase' defaults to the same thing.\n\nbut that isn't actually the case. Since commit d44e712 (pull: support\nrebased upstream + fetch + pull --rebase, 2009-07-19), pull has actually\nchosen the most recent reflog entry which is an ancestor of the current\nbranch if it can find one.\n\nAdd a '--fork-point' argument to git-rebase that can be used to trigger\nthis behaviour. This option is turned on by default if no non-option\narguments are specified on the command line, otherwise we treat an\nupstream specified on the command-line literally.\n\nSigned-off-by: John Keeping \n---\nOn Mon, Dec 09, 2013 at 08:40:08PM +0000, John Keeping wrote:\n> On Mon, Dec 09, 2013 at 12:11:50PM -0800, Junio C Hamano wrote:\n> > Do you mean\n> > \n> > \tgit rebase \n> > \n> > which we interpret as \"rebase the current branch on @{u}\", and it\n> > should behave as if the command was run like so:\n> > \n> > \tgit rebase --fork-point @{u}\n> > \n> > If that is what you suggest, I certainly can buy that. Those who\n> > want to disable the automation can explicitly say\n> > \n> > \tgit rebase @{u}\n> > \n> > and rebase the current exactly on top of the named commit (e.g. the\n> > current value of refs/remotes/origin/master or whatever remote-tracking\n> > branch you forked from).\n> \n> Yes, that's what I meant; the first non-option argument to \"git rebase\"\n> is called \"upstream\" in the manpage (and throughout the code). So if\n> \"\" means \"\" then that's\n> exactly what I meant.\n\nHere's an updated patch that adds a --fork-point option to git-rebase,\nwith the behaviour described above.\n\n Documentation/git-rebase.txt | 10 ++++++++++\n git-rebase.sh | 19 +++++++++++++++++++\n t/t3400-rebase.sh | 6 ++++--\n 3 files changed, 33 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 94e07fd..2889be6 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -324,6 +324,16 @@ fresh commits so it can be remerged successfully without needing to \"revert\n the reversion\" (see the\n link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for details).\n \n+--fork-point::\n+--no-fork-point::\n+\tUse 'git merge-base --fork-point' to find a better common ancestor\n+\tbetween `upstream` and `branch` when calculating which commits have\n+\thave been introduced by `branch` (see linkgit:git-merge-base[1]).\n++\n+If no non-option arguments are given on the command line, then the default is\n+`--fork-point @{u}` otherwise the `upstream` argument is interpreted literally\n+unless the `--fork-point` option is specified.\n+\n --ignore-whitespace::\n --whitespace=