{"thread":{"id":"35152","subject":"[PATCH] rebase: use reflog to find common base with upstream","startedAt":"2013-10-16T18:53:53Z","lastAt":"2013-10-29T20:11:25Z","messageCount":35,"participants":["John Keeping","Jonathan Nieder","Martin von Zweigbergk","Junio C Hamano","Eric Sunshine","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"229077","messageId":"d8e9f102609ee4502f579cb4ce872e0a40756204.1381949622.git.john@keeping.me.uk","threadId":"35152","inReplyTo":null,"subject":"[PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-16T18:53:53Z","receivedAt":"2013-10-16T18:53:53Z","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.\n\nSigned-off-by: John Keeping \n---\n git-rebase.sh | 8 ++++++++\n t/t3400-rebase.sh | 6 ++++--\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 226752f..fd36cf7 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -437,6 +437,14 @@ then\n \t\t\terror_on_missing_default_upstream \"rebase\" \"rebase\" \\\n \t\t\t\t\"against\" \"git rebase \"\n \t\tfi\n+\t\tfor reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n+\t\tdo\n+\t\t\tif test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n+\t\t\tthen\n+\t\t\t\tupstream_name=$reflog\n+\t\t\t\tbreak\n+\t\t\tfi\n+\t\tdone\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..4f45f7e 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 merge-base topic 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.4.566.g73d370b\n"},{"id":"229078","messageId":"20131016192412.GK9464@google.com","threadId":"35152","inReplyTo":"d8e9f102609ee4502f579cb4ce872e0a40756204.1381949622.git.john@keeping.me.uk","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-10-16T19:24:13Z","receivedAt":"2013-10-16T19:24:13Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJohn Keeping wrote:\n\n> 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>\n> Change rebase so that it uses the same logic.\n\nNice idea.\n\nCould pull be made to rely on rebase for this as a follow-up?\n\n[...]\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -437,6 +437,14 @@ then\n> \t\t\terror_on_missing_default_upstream \"rebase\" \"rebase\" \\\n> \t\t\t\t\"against\" \"git rebase \"\n> \t\tfi\n> +\t\tfor reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n> +\t\tdo\n> +\t\t\tif test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n\n\"git merge-base --is-ancestor\" is faster.\n\nWhat should happen if HEAD is not a valid commit? (Tested with:\n\n\t$ git checkout --orphan foo\n\t$ cat >>.git/config <&1 | wc -l\n\t83\n\n).\n\ndiff --git i/git-rebase.sh w/git-rebase.sh\nindex fd36cf7..d2e2c2e 100755\n--- i/git-rebase.sh\n+++ w/git-rebase.sh\n@@ -439,7 +439,7 @@ then\n \t\tfi\n \t\tfor reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n \t\tdo\n-\t\t\tif test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n+\t\t\tif git merge-base --is-ancestor \"$reflog\" HEAD\n \t\t\tthen\n \t\t\t\tupstream_name=$reflog\n \t\t\t\tbreak\n"},{"id":"229081","messageId":"20131016194444.GQ27238@serenity.lan","threadId":"35152","inReplyTo":"20131016192412.GK9464@google.com","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-16T19:44:44Z","receivedAt":"2013-10-16T19:44:44Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Wed, Oct 16, 2013 at 12:24:13PM -0700, Jonathan Nieder wrote:\n> John Keeping wrote:\n> \n> > 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> >\n> > Change rebase so that it uses the same logic.\n> \n> Nice idea.\n> \n> Could pull be made to rely on rebase for this as a follow-up?\n\nIt's not an obvious change to do that (at least to me) - pull does the\ndifferent steps of figuring out the base and then rebasing at different\npoints in the script.\n\n> [...]\n> > --- a/git-rebase.sh\n> > +++ b/git-rebase.sh\n> > @@ -437,6 +437,14 @@ then\n> > \t\t\terror_on_missing_default_upstream \"rebase\" \"rebase\" \\\n> > \t\t\t\t\"against\" \"git rebase \"\n> > \t\tfi\n> > +\t\tfor reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n> > +\t\tdo\n> > +\t\t\tif test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n> \n> \"git merge-base --is-ancestor\" is faster.\n\nWe should probably change git-pull to use that as well.\n\n> What should happen if HEAD is not a valid commit? (Tested with:\n> \n> \t$ git checkout --orphan foo\n> \t$ cat >>.git/config < \t[branch \"foo\"]\n> \t\tremote = origin\n> \t\tmerge = refs/heads/master\n> \tEOF\n> \t$ bin-wrappers/git rebase 2>&1 | wc -l\n> \t83\n> \n> ).\n\nAdding \"2>/dev/null\" to the merge-base line silences most of that, all\nwe're left with is \"fatal: Needed a single revision\" which is the same\nas I get from master's git-rebase. I think silencing stderr is the\nright thing to do here - in the worst case we just use the merge-base of\nthe two branches instead of the appropriate reflog entry.\n\n> diff --git i/git-rebase.sh w/git-rebase.sh\n> index fd36cf7..d2e2c2e 100755\n> --- i/git-rebase.sh\n> +++ w/git-rebase.sh\n> @@ -439,7 +439,7 @@ then\n> \t\tfi\n> \t\tfor reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n> \t\tdo\n> -\t\t\tif test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n> +\t\t\tif git merge-base --is-ancestor \"$reflog\" HEAD\n> \t\t\tthen\n> \t\t\t\tupstream_name=$reflog\n> \t\t\t\tbreak\n"},{"id":"229242","messageId":"CANiSa6gqGKAyLwwPVoZ_gzN85_06aTCfkdRRscNNZYs7g1rL0A@mail.gmail.com","threadId":"35152","inReplyTo":"d8e9f102609ee4502f579cb4ce872e0a40756204.1381949622.git.john@keeping.me.uk","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-10-21T05:03:29Z","receivedAt":"2013-10-21T05:03:29Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Oct 16, 2013 at 11:53 AM, 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\nIt is exactly this inconsistency between \"git rebase\" and \"git pull\n--rebase\" that confused me enough to make me send my first email to\nthis list almost 4 years ago [1], so thanks for working on this! I\nfinished that thread with:\n\n Would it make sense to teach \"git rebase\" the same tricks as \"git\npull --rebase\"?\n\nThen it took me a year before I sent a patch not unlike this one [2].\nTo summarize, the patch did not get accepted then because it makes\nrebase a little slower (or a lot slower in some cases). \"git pull\n--rebase\" is of course at least as slow in the same cases, but because\nit often involves connecting to a remote host, people would probably\nblame the connection rather than git itself even in those rare (?)\ncases.\n\nI think\n\n git merge-base HEAD $(git rev-list -g \"$upstream_name\")\n\nis roughly correct and hopefully fast enough. That can lead to too\nlong a command line, so I was planning on teaching merge-base a\n--stdin option, but never got around to it.\n\nMartin\n\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/136339\n[2] http://thread.gmane.org/gmane.comp.version-control.git/166710\n"},{"id":"229245","messageId":"20131021112408.GA24317@serenity.lan","threadId":"35152","inReplyTo":"CANiSa6gqGKAyLwwPVoZ_gzN85_06aTCfkdRRscNNZYs7g1rL0A@mail.gmail.com","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-21T11:24:08Z","receivedAt":"2013-10-21T11:24:08Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Sun, Oct 20, 2013 at 10:03:29PM -0700, Martin von Zweigbergk wrote:\n> On Wed, Oct 16, 2013 at 11:53 AM, 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> \n> It is exactly this inconsistency between \"git rebase\" and \"git pull\n> --rebase\" that confused me enough to make me send my first email to\n> this list almost 4 years ago [1], so thanks for working on this! I\n> finished that thread with:\n> \n> Would it make sense to teach \"git rebase\" the same tricks as \"git\n> pull --rebase\"?\n> \n> Then it took me a year before I sent a patch not unlike this one [2].\n> To summarize, the patch did not get accepted then because it makes\n> rebase a little slower (or a lot slower in some cases). \"git pull\n> --rebase\" is of course at least as slow in the same cases, but because\n> it often involves connecting to a remote host, people would probably\n> blame the connection rather than git itself even in those rare (?)\n> cases.\n> \n> I think\n> \n> git merge-base HEAD $(git rev-list -g \"$upstream_name\")\n> \n> is roughly correct and hopefully fast enough. That can lead to too\n> long a command line, so I was planning on teaching merge-base a\n> --stdin option, but never got around to it.\n\nI'm not sure we should worry about the additional overhead here. In the\ncommon case, we should hit a common ancestor within the first couple of\nreflog entries; and in the case that will be slow, it's likely that\nthere are a lot of differences between the branches so the cherry\ncomparison phase will take a while anyway.\n"},{"id":"229271","messageId":"CANiSa6gNJiUGb47Sw38V3oeJ-=0c1oZ0wO-GhNQminN3+SvgaA@mail.gmail.com","threadId":"35152","inReplyTo":"d8e9f102609ee4502f579cb4ce872e0a40756204.1381949622.git.john@keeping.me.uk","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-10-22T05:40:22Z","receivedAt":"2013-10-22T05:40:22Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, Oct 16, 2013 at 11:53 AM, 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>\n> Change rebase so that it uses the same logic.\n>\n> Signed-off-by: John Keeping \n> ---\n> git-rebase.sh | 8 ++++++++\n> t/t3400-rebase.sh | 6 ++++--\n> 2 files changed, 12 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 226752f..fd36cf7 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -437,6 +437,14 @@ then\n> error_on_missing_default_upstream \"rebase\" \"rebase\" \\\n> \"against\" \"git rebase \"\n> fi\n> + for reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n> + do\n> + if test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n> + then\n> + upstream_name=$reflog\n> + break\n> + fi\n> + done\n> ;;\n> *) upstream_name=\"$1\"\n> shift\n\nA little later, \"onto_name\" gets assigned like so:\n\n onto_name=${onto-\"$upstream_name\"}\n\nSo if upstream_name was set above, then onto would get the same value,\nwhich is not what we want, right? It seems like this block of code\nshould come a bit later.\n\nI also think it not be run only when rebase was run without a given\nupstream. If the configured upstream is \"origin/master\", it seems like\nit would be surprising to get different behavior from \"git rebase\" and\n\"git rebase origin/master\".\n"},{"id":"229276","messageId":"CANiSa6jSVyKmF4Z5uRFSEZbciq+AqU3ybhrDmf-2gyCj0GD6Eg@mail.gmail.com","threadId":"35152","inReplyTo":"20131021112408.GA24317@serenity.lan","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-10-22T06:24:14Z","receivedAt":"2013-10-22T06:24:14Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, Oct 21, 2013 at 4:24 AM, John Keeping wrote:\n> On Sun, Oct 20, 2013 at 10:03:29PM -0700, Martin von Zweigbergk wrote:\n>> On Wed, Oct 16, 2013 at 11:53 AM, 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>>\n>> It is exactly this inconsistency between \"git rebase\" and \"git pull\n>> --rebase\" that confused me enough to make me send my first email to\n>> this list almost 4 years ago [1], so thanks for working on this! I\n>> finished that thread with:\n>>\n>> Would it make sense to teach \"git rebase\" the same tricks as \"git\n>> pull --rebase\"?\n>>\n>> Then it took me a year before I sent a patch not unlike this one [2].\n>> To summarize, the patch did not get accepted then because it makes\n>> rebase a little slower (or a lot slower in some cases). \"git pull\n>> --rebase\" is of course at least as slow in the same cases, but because\n>> it often involves connecting to a remote host, people would probably\n>> blame the connection rather than git itself even in those rare (?)\n>> cases.\n>>\n>> I think\n>>\n>> git merge-base HEAD $(git rev-list -g \"$upstream_name\")\n>>\n>> is roughly correct and hopefully fast enough. That can lead to too\n>> long a command line, so I was planning on teaching merge-base a\n>> --stdin option, but never got around to it.\n>\n> I'm not sure we should worry about the additional overhead here. In the\n> common case, we should hit a common ancestor within the first couple of\n> reflog entries; and in the case that will be slow, it's likely that\n> there are a lot of differences between the branches so the cherry\n> comparison phase will take a while anyway.\n\nPerhaps true. I created a simple commit based on my origin/master@{1}\nin git.git, which happened to be 136 commits behind origin/master.\nBefore (a modified version of) your patch, it took 0.756s to rebase it\n(best of 5) and afterwards it took 0.720s.\n\nAnd in a worse case: The same test with one commit off my\norigin/master@{13}, 2910 behind origin/master, shows an increase from\n2.75s to 4.04s.\n\nAnd a degenerate case: I created a test branch (called u) with\n1000-entry reflog from the output of \"git rev-list --first-parent\norigin/master | head -1000 | tac\" and created the same simple commit\nas before off of the end of this reflog (u@{999}). This ended up 3769\ncommits behind u@{0} (aka origin/master). In this case it went from\n3.43s to 3m32s. Obviously, this was a degenerate case designed to be\nslow, but I think it's still worth noting that one can get such O(n^2)\nbehavior e.g. if one lets a branch get out of sync with an upstream\nthat's very frequently fetches (I've heard of people running\nshort-interval cron jobs that fetch from a remote).\n\nI do like the feature, but I'm still concerned about this last case.\n"},{"id":"229422","messageId":"xmqqhac6o5hj.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"CANiSa6gqGKAyLwwPVoZ_gzN85_06aTCfkdRRscNNZYs7g1rL0A@mail.gmail.com","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T19:04:24Z","receivedAt":"2013-10-24T19:04:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> I think\n>\n> git merge-base HEAD $(git rev-list -g \"$upstream_name\")\n>\n> is roughly correct and hopefully fast enough. That can lead to too\n> long a command line, so I was planning on teaching merge-base a\n> --stdin option, but never got around to it.\n\nSorry for coming in late.\n\nI think the above with s/HEAD/$curr_branch/ is a good way to compute\nwhat the whole \"for reflog in $(git rev-list -g $remoteref\" loop\ncomputes when one of the historic tips recorded in the reflog was\nwhere $curr_branch forked from, i.e. the loop actually finds at\nleast one ancestor in the reflog and breaks out after setting\noldremoteref. But it would give a completely different commit if\nnone of the reflog entries is a fork point.\n\nA two patch series forthcoming.\n"},{"id":"229423","messageId":"1382641884-14756-1-git-send-email-gitster@pobox.com","threadId":"35152","inReplyTo":"xmqqhac6o5hj.fsf@gitster.dls.corp.google.com","subject":"[PATCH 0/2] finding the fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T19:11:22Z","receivedAt":"2013-10-24T19:11:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The first one is a clean-up of the code to parse command line\noptions to \"git merge-base\". Options such as \"--independent\",\n\"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\nbetter expressed in terms of the recently introduced OPT_CMDMODE.\n\nThe second one implements the entire logic of the for loop we see in\n\"git pull --rebase\" directly using get_merge_bases_many() and\npostprocessing the result.\n\nJunio C Hamano (2):\n merge-base: use OPT_CMDMODE and clarify the command line parsing\n merge-base: \"--reflog\" mode finds fork point from reflog entries\n\n builtin/merge-base.c | 115 +++++++++++++++++++++++++++++++++++++++++++-------\n t/t6010-merge-base.sh | 27 ++++++++++++\n 2 files changed, 126 insertions(+), 16 deletions(-)\n\n-- \n1.8.4.1-799-g1c32b8d\n"},{"id":"229424","messageId":"1382641884-14756-2-git-send-email-gitster@pobox.com","threadId":"35152","inReplyTo":"1382641884-14756-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/2] merge-base: use OPT_CMDMODE and clarify the command line parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T19:11:23Z","receivedAt":"2013-10-24T19:11:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The --octopus, --independent and --is-ancestor are mutually\nexclusive command modes (in addition to not giving any of these\noptions), so represent them as such using the recent OPT_CMDMODE\nfacility available since 11588263 (parse-options: add OPT_CMDMODE(),\n2013-07-30), which is in v1.8.4-82-g366b80b. --all is compatible\nonly with plain vanilla mode and --octopus mode, and the minimum\nnumber of arguments the command takes depends on the command modes,\nso these are now separately checked in each command mode.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/merge-base.c | 38 ++++++++++++++++++++++----------------\n 1 file changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex e88eb93..d39c910 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -90,32 +90,38 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tstruct commit **rev;\n \tint rev_nr = 0;\n \tint show_all = 0;\n-\tint octopus = 0;\n-\tint reduce = 0;\n-\tint is_ancestor = 0;\n+\tint cmdmode = 0;\n \n \tstruct option options[] = {\n \t\tOPT_BOOL('a', \"all\", &show_all, N_(\"output all common ancestors\")),\n-\t\tOPT_BOOL(0, \"octopus\", &octopus, N_(\"find ancestors for a single n-way merge\")),\n-\t\tOPT_BOOL(0, \"independent\", &reduce, N_(\"list revs not reachable from others\")),\n-\t\tOPT_BOOL(0, \"is-ancestor\", &is_ancestor,\n-\t\t\t N_(\"is the first one ancestor of the other?\")),\n+\t\tOPT_CMDMODE(0, \"octopus\", &cmdmode,\n+\t\t\t N_(\"find ancestors for a single n-way merge\"), 'o'),\n+\t\tOPT_CMDMODE(0, \"independent\", &cmdmode,\n+\t\t\t N_(\"list revs not reachable from others\"), 'r'),\n+\t\tOPT_CMDMODE(0, \"is-ancestor\", &cmdmode,\n+\t\t\t N_(\"is the first one ancestor of the other?\"), 'a'),\n \t\tOPT_END()\n \t};\n \n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, merge_base_usage, 0);\n-\tif (!octopus && !reduce && argc < 2)\n-\t\tusage_with_options(merge_base_usage, options);\n-\tif (is_ancestor && (show_all || octopus || reduce))\n-\t\tdie(\"--is-ancestor cannot be used with other options\");\n-\tif (is_ancestor)\n+\n+\tif (cmdmode == 'a') {\n+\t\tif (argc < 2)\n+\t\t\tusage_with_options(merge_base_usage, options);\n+\t\tif (show_all)\n+\t\t\tdie(\"--is-ancestor cannot be used with --all\");\n \t\treturn handle_is_ancestor(argc, argv);\n-\tif (reduce && (show_all || octopus))\n-\t\tdie(\"--independent cannot be used with other options\");\n+\t}\n \n-\tif (octopus || reduce)\n-\t\treturn handle_octopus(argc, argv, reduce, show_all);\n+\tif (cmdmode == 'r' && show_all)\n+\t\tdie(\"--independent cannot be used with --all\");\n+\n+\tif (cmdmode == 'r' || cmdmode == 'o')\n+\t\treturn handle_octopus(argc, argv, cmdmode == 'r', show_all);\n+\n+\tif (argc < 2)\n+\t\tusage_with_options(merge_base_usage, options);\n \n \trev = xmalloc(argc * sizeof(*rev));\n \twhile (argc-- > 0)\n-- \n1.8.4.1-799-g1c32b8d\n"},{"id":"229425","messageId":"1382641884-14756-3-git-send-email-gitster@pobox.com","threadId":"35152","inReplyTo":"1382641884-14756-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T19:11:24Z","receivedAt":"2013-10-24T19:11:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"git pull --rebase\" command computes the fork point of the\nbranch being rebased using the reflog entries of the \"base\" branch\n(typically a remote-tracking branch) the branch's work was based on,\nin order to cope with the case in which the \"base\" branch has been\nrewound and rebuilt. For example, if the history looked like this:\n\n o---B1\n /\n ---o---o---B2--o---o---o---Base\n \\\n\t B3\n\t \\\n\t Derived\n\nwhere the current tip of the \"base\" branch is at Base, but earlier\nfetch observed that its tip used to be B3 and then B2 and then B1\nbefore getting to the current commit, and the branch being rebased\non top of the latest \"base\" is based on commit B3, it tries to find\nB3 by going through the output of \"git rev-list --reflog base\" (i.e.\nBase, B1, B2, B3) until it finds a commit that is an ancestor of the\ncurrent tip \"Derived\".\n\nInternally, we have get_merge_bases_many() that can compute this\nwith one-go. We would want a merge-base between Derived and a\nfictitious merge commit that would result by merging all the\nhistorical tips of \"base\". When such a commit exist, we should get\na single result, which exactly match one of the reflog entries of\n\"base\".\n\nTeach \"git merge-base\" a new mode, \"--reflog\", to compute exactly\nthat.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/merge-base.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++\n t/t6010-merge-base.sh | 27 ++++++++++++++++++\n 2 files changed, 104 insertions(+)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex d39c910..7b9bc15 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -1,6 +1,7 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"commit.h\"\n+#include \"refs.h\"\n #include \"parse-options.h\"\n \n static int show_merge_base(struct commit **rev, int rev_nr, int show_all)\n@@ -27,6 +28,7 @@ static const char * const merge_base_usage[] = {\n \tN_(\"git merge-base [-a|--all] --octopus ...\"),\n \tN_(\"git merge-base --independent ...\"),\n \tN_(\"git merge-base --is-ancestor \"),\n+\tN_(\"git merge-base --reflog []\"),\n \tNULL\n };\n \n@@ -85,6 +87,73 @@ static int handle_is_ancestor(int argc, const char **argv)\n \t\treturn 1;\n }\n \n+struct rev_collect {\n+\tstruct commit **commit;\n+\tint nr;\n+\tint alloc;\n+};\n+\n+static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t\t const char *ident, unsigned long timestamp,\n+\t\t\t\t int tz, const char *message, void *cbdata_)\n+{\n+\tstruct rev_collect *revs = cbdata_;\n+\tstruct commit *commit = lookup_commit(nsha1);\n+\tif (commit) {\n+\t\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n+\t\trevs->commit[revs->nr++] = commit;\n+\t}\n+\treturn 0;\n+}\n+\n+static int handle_reflog(int argc, const char **argv)\n+{\n+\tunsigned char sha1[20];\n+\tchar *refname;\n+\tconst char *commitname;\n+\tstruct rev_collect revs;\n+\tstruct commit *derived;\n+\tstruct commit_list *bases;\n+\tint i;\n+\n+\tswitch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {\n+\tcase 0:\n+\t\tdie(\"No such ref: '%s'\", argv[0]);\n+\tcase 1:\n+\t\tbreak; /* good */\n+\tdefault:\n+\t\tdie(\"Ambiguous refname: '%s'\", argv[0]);\n+\t}\n+\n+\tcommitname = (argc == 2) ? argv[1] : \"HEAD\";\n+\tif (get_sha1(commitname, sha1))\n+\t\tdie(\"Not a valid object name: '%s'\", commitname);\n+\n+\tderived = lookup_commit_reference(sha1);\n+\tmemset(&revs, 0, sizeof(revs));\n+\tfor_each_reflog_ent(refname, collect_one_reflog_ent, &revs);\n+\n+\tbases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n+\n+\t/*\n+\t * There should be one and only one merge base, when we found\n+\t * a common ancestor among reflog entries.\n+\t */\n+\tif (!bases || bases->next)\n+\t\treturn 1;\n+\n+\t/* And the found one must be one of the reflog entries */\n+\tfor (i = 0; i < revs.nr; i++)\n+\t\tif (&bases->item->object == &revs.commit[i]->object)\n+\t\t\tbreak; /* found */\n+\tif (revs.nr <= i)\n+\t\treturn 1; /* not found */\n+\n+\tprintf(\"%s\\n\", sha1_to_hex(bases->item->object.sha1));\n+\tfree_commit_list(bases);\n+\treturn 0;\n+}\n+\n int cmd_merge_base(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit **rev;\n@@ -100,6 +169,8 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"list revs not reachable from others\"), 'r'),\n \t\tOPT_CMDMODE(0, \"is-ancestor\", &cmdmode,\n \t\t\t N_(\"is the first one ancestor of the other?\"), 'a'),\n+\t\tOPT_CMDMODE(0, \"reflog\", &cmdmode,\n+\t\t\t N_(\"find where forked from reflog of \"), 'g'),\n \t\tOPT_END()\n \t};\n \n@@ -120,6 +191,12 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'r' || cmdmode == 'o')\n \t\treturn handle_octopus(argc, argv, cmdmode == 'r', show_all);\n \n+\tif (cmdmode == 'g') {\n+\t\tif (argc < 1 || 2 < argc)\n+\t\t\tusage_with_options(merge_base_usage, options);\n+\t\treturn handle_reflog(argc, argv);\n+\t}\n+\n \tif (argc < 2)\n \t\tusage_with_options(merge_base_usage, options);\n \ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex f80bba8..3a1abee 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n \ttest_cmp expected.sorted actual.sorted\n '\n \n+test_expect_success 'using reflog to find the fork point' '\n+\tgit reset --hard &&\n+\tgit checkout -b base $E &&\n+\t(\n+\t\tfor count in 1 2 3 4 5\n+\t\tdo\n+\t\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n+\t\t\tgit rev-parse HEAD >expect$count &&\n+\t\t\tgit checkout -B derived &&\n+\t\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n+\t\t\tgit rev-parse HEAD >derived$count &&\n+\t\t\tgit checkout base &&\n+\t\t\tcount=$(( $count + 1 )) || exit 1\n+\t\tdone\n+\n+\t\tfor count in 1 2 3 4 5\n+\t\tdo\n+\t\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n+\t\t\ttest_cmp expect$count actual || exit 1\n+\t\tdone\n+\n+\t\t# check defaulting to HEAD\n+\t\tgit merge-base --reflog base >actual &&\n+\t\ttest_cmp expect5 actual\n+\t)\n+'\n+\n test_done\n-- \n1.8.4.1-799-g1c32b8d\n"},{"id":"229432","messageId":"20131024202635.GB10779@serenity.lan","threadId":"35152","inReplyTo":"CANiSa6gNJiUGb47Sw38V3oeJ-=0c1oZ0wO-GhNQminN3+SvgaA@mail.gmail.com","subject":"Re: [PATCH] rebase: use reflog to find common base with upstream","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-24T20:26:35Z","receivedAt":"2013-10-24T20:26:35Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Oct 21, 2013 at 10:40:22PM -0700, Martin von Zweigbergk wrote:\n> On Wed, Oct 16, 2013 at 11:53 AM, 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> >\n> > Change rebase so that it uses the same logic.\n> >\n> > Signed-off-by: John Keeping \n> > ---\n> > git-rebase.sh | 8 ++++++++\n> > t/t3400-rebase.sh | 6 ++++--\n> > 2 files changed, 12 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/git-rebase.sh b/git-rebase.sh\n> > index 226752f..fd36cf7 100755\n> > --- a/git-rebase.sh\n> > +++ b/git-rebase.sh\n> > @@ -437,6 +437,14 @@ then\n> > error_on_missing_default_upstream \"rebase\" \"rebase\" \\\n> > \"against\" \"git rebase \"\n> > fi\n> > + for reflog in $(git rev-list -g \"$upstream_name\" 2>/dev/null)\n> > + do\n> > + if test \"$reflog\" = \"$(git merge-base \"$reflog\" HEAD)\"\n> > + then\n> > + upstream_name=$reflog\n> > + break\n> > + fi\n> > + done\n> > ;;\n> > *) upstream_name=\"$1\"\n> > shift\n> \n> A little later, \"onto_name\" gets assigned like so:\n> \n> onto_name=${onto-\"$upstream_name\"}\n> \n> So if upstream_name was set above, then onto would get the same value,\n> which is not what we want, right? It seems like this block of code\n> should come a bit later.\n> \n> I also think it not be run only when rebase was run without a given\n> upstream. If the configured upstream is \"origin/master\", it seems like\n> it would be surprising to get different behavior from \"git rebase\" and\n> \"git rebase origin/master\".\n\nHmm... I think you're right. I originally didn't want to change the\nbehaviour when the user specifies a branch, but in this case we want to\nlook for a common ancestor in the reflog of the upstream regardless of\nwhere the ref came from.\n"},{"id":"229434","messageId":"20131024205434.GC10779@serenity.lan","threadId":"35152","inReplyTo":"1382641884-14756-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-24T20:54:34Z","receivedAt":"2013-10-24T20:54:34Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Oct 24, 2013 at 12:11:22PM -0700, Junio C Hamano wrote:\n> The first one is a clean-up of the code to parse command line\n> options to \"git merge-base\". Options such as \"--independent\",\n> \"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\n> better expressed in terms of the recently introduced OPT_CMDMODE.\n> \n> The second one implements the entire logic of the for loop we see in\n> \"git pull --rebase\" directly using get_merge_bases_many() and\n> postprocessing the result.\n\nNice! I tried this in the case where the target commit happens to be\nthe 63rd reflog entry:\n\n$ time sh -c 'for rev in $(git rev-list -g origin/master 2>/dev/null)\ndo\n git merge-base --is-ancestor $rev b2edae0 && break\ndone\n'\n\nreal 0m3.772s\nuser 0m3.338s\nsys 0m0.440s\n\n$ time git merge-base --reflog origin/master b2edae0\n\nreal 0m0.156s\nuser 0m0.138s\nsys 0m0.018s\n\n\n> Junio C Hamano (2):\n> merge-base: use OPT_CMDMODE and clarify the command line parsing\n> merge-base: \"--reflog\" mode finds fork point from reflog entries\n> \n> builtin/merge-base.c | 115 +++++++++++++++++++++++++++++++++++++++++++-------\n> t/t6010-merge-base.sh | 27 ++++++++++++\n> 2 files changed, 126 insertions(+), 16 deletions(-)\n\nShould there be a change to Documentation/git-merge-base.txt in here?\n"},{"id":"229436","messageId":"CAPig+cQrBMMqSmOk0GSZJ9PTHNt-t+vuOG2Aq=7VTR1EZSeLsw@mail.gmail.com","threadId":"35152","inReplyTo":"1382641884-14756-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-10-24T21:01:56Z","receivedAt":"2013-10-24T21:01:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Oct 24, 2013 at 3:11 PM, Junio C Hamano wrote:\n> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n> index f80bba8..3a1abee 100755\n> --- a/t/t6010-merge-base.sh\n> +++ b/t/t6010-merge-base.sh\n> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n> test_cmp expected.sorted actual.sorted\n> '\n>\n> +test_expect_success 'using reflog to find the fork point' '\n> + git reset --hard &&\n> + git checkout -b base $E &&\n> + (\n> + for count in 1 2 3 4 5\n> + do\n> + git commit --allow-empty -m \"Base commit #$count\" &&\n> + git rev-parse HEAD >expect$count &&\n> + git checkout -B derived &&\n> + git commit --allow-empty -m \"Derived #$count\" &&\n> + git rev-parse HEAD >derived$count &&\n> + git checkout base &&\n> + count=$(( $count + 1 )) || exit 1\n> + done\n\nDid you want && here?\n\n> +\n> + for count in 1 2 3 4 5\n> + do\n> + git merge-base --reflog base $(cat derived$count) >actual &&\n> + test_cmp expect$count actual || exit 1\n> + done\n\nAnd here?\n\n> +\n> + # check defaulting to HEAD\n> + git merge-base --reflog base >actual &&\n> + test_cmp expect5 actual\n> + )\n> +'\n> +\n> test_done\n"},{"id":"229439","messageId":"xmqqa9hymkma.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"20131024205434.GC10779@serenity.lan","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T21:20:29Z","receivedAt":"2013-10-24T21:20:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> On Thu, Oct 24, 2013 at 12:11:22PM -0700, Junio C Hamano wrote:\n>> The first one is a clean-up of the code to parse command line\n>> options to \"git merge-base\". Options such as \"--independent\",\n>> \"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\n>> better expressed in terms of the recently introduced OPT_CMDMODE.\n>> \n>> The second one implements the entire logic of the for loop we see in\n>> \"git pull --rebase\" directly using get_merge_bases_many() and\n>> postprocessing the result.\n>\n> Nice! I tried this in the case where the target commit happens to be\n> the 63rd reflog entry:\n>\n> $ time sh -c 'for rev in $(git rev-list -g origin/master 2>/dev/null)\n> do\n> git merge-base --is-ancestor $rev b2edae0 && break\n> done\n> '\n>\n> real 0m3.772s\n> user 0m3.338s\n> sys 0m0.440s\n>\n> $ time git merge-base --reflog origin/master b2edae0\n>\n> real 0m0.156s\n> user 0m0.138s\n> sys 0m0.018s\n\nThe real question is if the C code computes the same as the shell\nloop.\n"},{"id":"229440","messageId":"xmqq61smmkc0.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"CAPig+cQrBMMqSmOk0GSZJ9PTHNt-t+vuOG2Aq=7VTR1EZSeLsw@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T21:26:39Z","receivedAt":"2013-10-24T21:26:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Thu, Oct 24, 2013 at 3:11 PM, Junio C Hamano wrote:\n>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n>> index f80bba8..3a1abee 100755\n>> --- a/t/t6010-merge-base.sh\n>> +++ b/t/t6010-merge-base.sh\n>> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n>> test_cmp expected.sorted actual.sorted\n>> '\n>>\n>> +test_expect_success 'using reflog to find the fork point' '\n>> + git reset --hard &&\n>> + git checkout -b base $E &&\n>> + (\n>> + for count in 1 2 3 4 5\n>> + do\n>> + git commit --allow-empty -m \"Base commit #$count\" &&\n>> + git rev-parse HEAD >expect$count &&\n>> + git checkout -B derived &&\n>> + git commit --allow-empty -m \"Derived #$count\" &&\n>> + git rev-parse HEAD >derived$count &&\n>> + git checkout base &&\n>> + count=$(( $count + 1 )) || exit 1\n>> + done\n>\n> Did you want && here?\n\nNo, I did not. Can't you tell from the fact that I didn't put one\nthere ;-)?\n\nIt does not hurt to have one there, but it is not necessary.\n\nBecause the loop itself does not &&-cascade from anything else, the\nonly case anything after \"done &&\" would be skipped and making the\nwhole thing fail would be when anything inside the loop fails, but\nwe already \"exit 1\" to terminate the whole subprocess in that case,\nso we will not continue past the loop.\n\n>> +\n>> + for count in 1 2 3 4 5\n>> + do\n>> + git merge-base --reflog base $(cat derived$count) >actual &&\n>> + test_cmp expect$count actual || exit 1\n>> + done\n>\n> And here?\n\nLikewise.\n\nThanks.\n\n>> +\n>> + # check defaulting to HEAD\n>> + git merge-base --reflog base >actual &&\n>> + test_cmp expect5 actual\n>> + )\n>> +'\n>> +\n>> test_done\n"},{"id":"229441","messageId":"20131024213134.GD10779@serenity.lan","threadId":"35152","inReplyTo":"xmqqa9hymkma.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-24T21:31:35Z","receivedAt":"2013-10-24T21:31:35Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Oct 24, 2013 at 02:20:29PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > On Thu, Oct 24, 2013 at 12:11:22PM -0700, Junio C Hamano wrote:\n> >> The first one is a clean-up of the code to parse command line\n> >> options to \"git merge-base\". Options such as \"--independent\",\n> >> \"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\n> >> better expressed in terms of the recently introduced OPT_CMDMODE.\n> >> \n> >> The second one implements the entire logic of the for loop we see in\n> >> \"git pull --rebase\" directly using get_merge_bases_many() and\n> >> postprocessing the result.\n> >\n> > Nice! I tried this in the case where the target commit happens to be\n> > the 63rd reflog entry:\n> >\n> > $ time sh -c 'for rev in $(git rev-list -g origin/master 2>/dev/null)\n> > do\n> > git merge-base --is-ancestor $rev b2edae0 && break\n> > done\n> > '\n> >\n> > real 0m3.772s\n> > user 0m3.338s\n> > sys 0m0.440s\n> >\n> > $ time git merge-base --reflog origin/master b2edae0\n> >\n> > real 0m0.156s\n> > user 0m0.138s\n> > sys 0m0.018s\n> \n> The real question is if the C code computes the same as the shell\n> loop.\n\nAnd in fact it doesn't - if you replace the \"break\" with \"echo $rev\" the\nshell version prints b2edae0... but the C version prints nothing (and\nexists with status 1).\n"},{"id":"229443","messageId":"20131024214007.GE10779@serenity.lan","threadId":"35152","inReplyTo":"20131024213134.GD10779@serenity.lan","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-24T21:40:07Z","receivedAt":"2013-10-24T21:40:07Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Oct 24, 2013 at 10:31:35PM +0100, John Keeping wrote:\n> On Thu, Oct 24, 2013 at 02:20:29PM -0700, Junio C Hamano wrote:\n> > John Keeping writes:\n> > \n> > > On Thu, Oct 24, 2013 at 12:11:22PM -0700, Junio C Hamano wrote:\n> > >> The first one is a clean-up of the code to parse command line\n> > >> options to \"git merge-base\". Options such as \"--independent\",\n> > >> \"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\n> > >> better expressed in terms of the recently introduced OPT_CMDMODE.\n> > >> \n> > >> The second one implements the entire logic of the for loop we see in\n> > >> \"git pull --rebase\" directly using get_merge_bases_many() and\n> > >> postprocessing the result.\n> > >\n> > > Nice! I tried this in the case where the target commit happens to be\n> > > the 63rd reflog entry:\n> > >\n> > > $ time sh -c 'for rev in $(git rev-list -g origin/master 2>/dev/null)\n> > > do\n> > > git merge-base --is-ancestor $rev b2edae0 && break\n> > > done\n> > > '\n> > >\n> > > real 0m3.772s\n> > > user 0m3.338s\n> > > sys 0m0.440s\n> > >\n> > > $ time git merge-base --reflog origin/master b2edae0\n> > >\n> > > real 0m0.156s\n> > > user 0m0.138s\n> > > sys 0m0.018s\n> > \n> > The real question is if the C code computes the same as the shell\n> > loop.\n> \n> And in fact it doesn't - if you replace the \"break\" with \"echo $rev\" the\n> shell version prints b2edae0... but the C version prints nothing (and\n> exists with status 1).\n\nTo clarify: the particular commit in the calls above happens to be the\noldest entry in the reflog, if I pick a newer entry then it works.\n\nIt seems that for_each_reflog_ent isn't returning the oldest entry;\nrevs.nr is 62 whereas \"git rev-list -g origin/master | wc -l\" gives 63.\n"},{"id":"229445","messageId":"CAPig+cQ2tWFXX-RYnUrHEZCaqaPV6ZwgoPfiNPv9P1jFNTGEYg@mail.gmail.com","threadId":"35152","inReplyTo":"xmqq61smmkc0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-10-24T21:43:16Z","receivedAt":"2013-10-24T21:43:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Oct 24, 2013 at 5:26 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>\n>> On Thu, Oct 24, 2013 at 3:11 PM, Junio C Hamano wrote:\n>>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n>>> index f80bba8..3a1abee 100755\n>>> --- a/t/t6010-merge-base.sh\n>>> +++ b/t/t6010-merge-base.sh\n>>> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n>>> test_cmp expected.sorted actual.sorted\n>>> '\n>>>\n>>> +test_expect_success 'using reflog to find the fork point' '\n>>> + git reset --hard &&\n>>> + git checkout -b base $E &&\n>>> + (\n>>> + for count in 1 2 3 4 5\n>>> + do\n>>> + git commit --allow-empty -m \"Base commit #$count\" &&\n>>> + git rev-parse HEAD >expect$count &&\n>>> + git checkout -B derived &&\n>>> + git commit --allow-empty -m \"Derived #$count\" &&\n>>> + git rev-parse HEAD >derived$count &&\n>>> + git checkout base &&\n>>> + count=$(( $count + 1 )) || exit 1\n>>> + done\n>>\n>> Did you want && here?\n>\n> No, I did not. Can't you tell from the fact that I didn't put one\n> there ;-)?\n>\n> It does not hurt to have one there, but it is not necessary.\n>\n> Because the loop itself does not &&-cascade from anything else, the\n> only case anything after \"done &&\" would be skipped and making the\n> whole thing fail would be when anything inside the loop fails, but\n> we already \"exit 1\" to terminate the whole subprocess in that case,\n> so we will not continue past the loop.\n\nI didn't read inside the loop closely enough to see that. Sorry for the noise.\n\n>\n>>> +\n>>> + for count in 1 2 3 4 5\n>>> + do\n>>> + git merge-base --reflog base $(cat derived$count) >actual &&\n>>> + test_cmp expect$count actual || exit 1\n>>> + done\n>>\n>> And here?\n>\n> Likewise.\n>\n> Thanks.\n>\n>>> +\n>>> + # check defaulting to HEAD\n>>> + git merge-base --reflog base >actual &&\n>>> + test_cmp expect5 actual\n>>> + )\n>>> +'\n>>> +\n>>> test_done\n"},{"id":"229446","messageId":"20131024215053.GF10779@serenity.lan","threadId":"35152","inReplyTo":"20131024214007.GE10779@serenity.lan","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-24T21:50:53Z","receivedAt":"2013-10-24T21:50:53Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Thu, Oct 24, 2013 at 10:40:07PM +0100, John Keeping wrote:\n> On Thu, Oct 24, 2013 at 10:31:35PM +0100, John Keeping wrote:\n> > On Thu, Oct 24, 2013 at 02:20:29PM -0700, Junio C Hamano wrote:\n> > > John Keeping writes:\n> > > \n> > > > On Thu, Oct 24, 2013 at 12:11:22PM -0700, Junio C Hamano wrote:\n> > > >> The first one is a clean-up of the code to parse command line\n> > > >> options to \"git merge-base\". Options such as \"--independent\",\n> > > >> \"--is-ancestor\" and \"--octopus\" are mutually exclusive and it is\n> > > >> better expressed in terms of the recently introduced OPT_CMDMODE.\n> > > >> \n> > > >> The second one implements the entire logic of the for loop we see in\n> > > >> \"git pull --rebase\" directly using get_merge_bases_many() and\n> > > >> postprocessing the result.\n> > > >\n> > > > Nice! I tried this in the case where the target commit happens to be\n> > > > the 63rd reflog entry:\n> > > >\n> > > > $ time sh -c 'for rev in $(git rev-list -g origin/master 2>/dev/null)\n> > > > do\n> > > > git merge-base --is-ancestor $rev b2edae0 && break\n> > > > done\n> > > > '\n> > > >\n> > > > real 0m3.772s\n> > > > user 0m3.338s\n> > > > sys 0m0.440s\n> > > >\n> > > > $ time git merge-base --reflog origin/master b2edae0\n> > > >\n> > > > real 0m0.156s\n> > > > user 0m0.138s\n> > > > sys 0m0.018s\n> > > \n> > > The real question is if the C code computes the same as the shell\n> > > loop.\n> > \n> > And in fact it doesn't - if you replace the \"break\" with \"echo $rev\" the\n> > shell version prints b2edae0... but the C version prints nothing (and\n> > exists with status 1).\n> \n> To clarify: the particular commit in the calls above happens to be the\n> oldest entry in the reflog, if I pick a newer entry then it works.\n> \n> It seems that for_each_reflog_ent isn't returning the oldest entry;\n> revs.nr is 62 whereas \"git rev-list -g origin/master | wc -l\" gives 63.\n\nThe following patch on top fixes it, but I'm sure it can be done in a\nneater way.\n\n-- >8 --\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 7b9bc15..f6f1f14 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -98,7 +98,17 @@ static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\t\t int tz, const char *message, void *cbdata_)\n {\n \tstruct rev_collect *revs = cbdata_;\n-\tstruct commit *commit = lookup_commit(nsha1);\n+\tstruct commit *commit;\n+\n+\tif (!revs->nr) {\n+\t\tcommit = lookup_commit(osha1);\n+\t\tif (commit) {\n+\t\t\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n+\t\t\trevs->commit[revs->nr++] = commit;\n+\t\t}\n+\t}\n+\n+\tcommit = lookup_commit(nsha1);\n \tif (commit) {\n \t\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n \t\trevs->commit[revs->nr++] = commit;\n"},{"id":"229447","messageId":"xmqqwql2l3ln.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"CAPig+cQ2tWFXX-RYnUrHEZCaqaPV6ZwgoPfiNPv9P1jFNTGEYg@mail.gmail.com","subject":"Re: [PATCH 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T22:13:24Z","receivedAt":"2013-10-24T22:13:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Thu, Oct 24, 2013 at 5:26 PM, Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>>\n>>> On Thu, Oct 24, 2013 at 3:11 PM, Junio C Hamano wrote:\n>>>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n>>>> index f80bba8..3a1abee 100755\n>>>> --- a/t/t6010-merge-base.sh\n>>>> +++ b/t/t6010-merge-base.sh\n>>>> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n>>>> test_cmp expected.sorted actual.sorted\n>>>> '\n>>>>\n>>>> +test_expect_success 'using reflog to find the fork point' '\n>>>> + git reset --hard &&\n>>>> + git checkout -b base $E &&\n>>>> + (\n>>>> + for count in 1 2 3 4 5\n>>>> + do\n>>>> + git commit --allow-empty -m \"Base commit #$count\" &&\n>>>> + git rev-parse HEAD >expect$count &&\n>>>> + git checkout -B derived &&\n>>>> + git commit --allow-empty -m \"Derived #$count\" &&\n>>>> + git rev-parse HEAD >derived$count &&\n>>>> + git checkout base &&\n>>>> + count=$(( $count + 1 )) || exit 1\n>>>> + done\n>>>\n>>> Did you want && here?\n>>\n>> No, I did not. Can't you tell from the fact that I didn't put one\n>> there ;-)?\n>>\n>> It does not hurt to have one there, but it is not necessary.\n>>\n>> Because the loop itself does not &&-cascade from anything else, the\n>> only case anything after \"done &&\" would be skipped and making the\n>> whole thing fail would be when anything inside the loop fails, but\n>> we already \"exit 1\" to terminate the whole subprocess in that case,\n>> so we will not continue past the loop.\n>\n> I didn't read inside the loop closely enough to see that. Sorry\n> for the noise.\n\nHeh, you helped me realize that the above was suboptimal, and it\nwasn't a noise ;-)\n\nWe could do it this way without the subshell and the exit, I would\nthink.\n\ntest_expect_success 'using reflog to find the fork point' '\n\tgit reset --hard &&\n\tgit checkout -b base $E &&\n\tfor count in 1 2 3 4 5\n\tdo\n\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n\t\tgit rev-parse HEAD >expect$count &&\n\t\tgit checkout -B derived &&\n\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n\t\tgit rev-parse HEAD >derived$count &&\n\t\tgit checkout base &&\n\t\tcount=$(( $count + 1 )) || break\n\tdone &&\n\n\tfor count in 1 2 3 4 5\n\tdo\n\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n\t\ttest_cmp expect$count actual || break\n\tdone &&\n\n\t# check defaulting to HEAD\n\tgit merge-base --reflog base >actual &&\n\ttest_cmp expect5 actual\n'\n\nTo the earlier code, somebody could add\n\n (\n + more setup code &&\n + yet more setup code &&\n for count in 1 2 3 4 5\n\ninside the subshell and we would fail to notice fairlure from the\nnew setup code if we didn't have && after the first \"done\". There\nis much less risk of that kind of breakage if we did the loop\nwithout subshell and exit and instead with the usual &&-cascade.\n\nThanks.\n"},{"id":"229448","messageId":"xmqqsivql37i.fsf_-_@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"xmqqwql2l3ln.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-24T22:21:53Z","receivedAt":"2013-10-24T22:21:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"git pull --rebase\" command computes the fork point of the\nbranch being rebased using the reflog entries of the \"base\" branch\n(typically a remote-tracking branch) the branch's work was based on,\nin order to cope with the case in which the \"base\" branch has been\nrewound and rebuilt. For example, if the history looked like this:\n\n o---B1\n /\n ---o---o---B2--o---o---o---Base\n \\\n\t B3\n\t \\\n\t Derived\n\nwhere the current tip of the \"base\" branch is at Base, but earlier\nfetch observed that its tip used to be B3 and then B2 and then B1\nbefore getting to the current commit, and the branch being rebased\non top of the latest \"base\" is based on commit B3, it tries to find\nB3 by going through the output of \"git rev-list --reflog base\" (i.e.\nBase, B1, B2, B3) until it finds a commit that is an ancestor of the\ncurrent tip \"Derived\".\n\nInternally, we have get_merge_bases_many() that can compute this\nwith one-go. We would want a merge-base between Derived and a\nfictitious merge commit that would result by merging all the\nhistorical tips of \"base\". When such a commit exist, we should get\na single result, which exactly match one of the reflog entries of\n\"base\".\n\nTeach \"git merge-base\" a new mode, \"--reflog\", to compute exactly\nthat.\n\nHelped-by: Eric Sunshine \nSigned-off-by: Junio C Hamano \n---\n\n * With updated tests, based on conversation with Eric Sunshine\n\n builtin/merge-base.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++\n t/t6010-merge-base.sh | 25 +++++++++++++++++\n 2 files changed, 102 insertions(+)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex d39c910..7b9bc15 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -1,6 +1,7 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"commit.h\"\n+#include \"refs.h\"\n #include \"parse-options.h\"\n \n static int show_merge_base(struct commit **rev, int rev_nr, int show_all)\n@@ -27,6 +28,7 @@ static const char * const merge_base_usage[] = {\n \tN_(\"git merge-base [-a|--all] --octopus ...\"),\n \tN_(\"git merge-base --independent ...\"),\n \tN_(\"git merge-base --is-ancestor \"),\n+\tN_(\"git merge-base --reflog []\"),\n \tNULL\n };\n \n@@ -85,6 +87,73 @@ static int handle_is_ancestor(int argc, const char **argv)\n \t\treturn 1;\n }\n \n+struct rev_collect {\n+\tstruct commit **commit;\n+\tint nr;\n+\tint alloc;\n+};\n+\n+static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t\t const char *ident, unsigned long timestamp,\n+\t\t\t\t int tz, const char *message, void *cbdata_)\n+{\n+\tstruct rev_collect *revs = cbdata_;\n+\tstruct commit *commit = lookup_commit(nsha1);\n+\tif (commit) {\n+\t\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n+\t\trevs->commit[revs->nr++] = commit;\n+\t}\n+\treturn 0;\n+}\n+\n+static int handle_reflog(int argc, const char **argv)\n+{\n+\tunsigned char sha1[20];\n+\tchar *refname;\n+\tconst char *commitname;\n+\tstruct rev_collect revs;\n+\tstruct commit *derived;\n+\tstruct commit_list *bases;\n+\tint i;\n+\n+\tswitch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {\n+\tcase 0:\n+\t\tdie(\"No such ref: '%s'\", argv[0]);\n+\tcase 1:\n+\t\tbreak; /* good */\n+\tdefault:\n+\t\tdie(\"Ambiguous refname: '%s'\", argv[0]);\n+\t}\n+\n+\tcommitname = (argc == 2) ? argv[1] : \"HEAD\";\n+\tif (get_sha1(commitname, sha1))\n+\t\tdie(\"Not a valid object name: '%s'\", commitname);\n+\n+\tderived = lookup_commit_reference(sha1);\n+\tmemset(&revs, 0, sizeof(revs));\n+\tfor_each_reflog_ent(refname, collect_one_reflog_ent, &revs);\n+\n+\tbases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n+\n+\t/*\n+\t * There should be one and only one merge base, when we found\n+\t * a common ancestor among reflog entries.\n+\t */\n+\tif (!bases || bases->next)\n+\t\treturn 1;\n+\n+\t/* And the found one must be one of the reflog entries */\n+\tfor (i = 0; i < revs.nr; i++)\n+\t\tif (&bases->item->object == &revs.commit[i]->object)\n+\t\t\tbreak; /* found */\n+\tif (revs.nr <= i)\n+\t\treturn 1; /* not found */\n+\n+\tprintf(\"%s\\n\", sha1_to_hex(bases->item->object.sha1));\n+\tfree_commit_list(bases);\n+\treturn 0;\n+}\n+\n int cmd_merge_base(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit **rev;\n@@ -100,6 +169,8 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"list revs not reachable from others\"), 'r'),\n \t\tOPT_CMDMODE(0, \"is-ancestor\", &cmdmode,\n \t\t\t N_(\"is the first one ancestor of the other?\"), 'a'),\n+\t\tOPT_CMDMODE(0, \"reflog\", &cmdmode,\n+\t\t\t N_(\"find where forked from reflog of \"), 'g'),\n \t\tOPT_END()\n \t};\n \n@@ -120,6 +191,12 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'r' || cmdmode == 'o')\n \t\treturn handle_octopus(argc, argv, cmdmode == 'r', show_all);\n \n+\tif (cmdmode == 'g') {\n+\t\tif (argc < 1 || 2 < argc)\n+\t\t\tusage_with_options(merge_base_usage, options);\n+\t\treturn handle_reflog(argc, argv);\n+\t}\n+\n \tif (argc < 2)\n \t\tusage_with_options(merge_base_usage, options);\n \ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex f80bba8..d5e76a6 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -230,4 +230,29 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n \ttest_cmp expected.sorted actual.sorted\n '\n \n+test_expect_success 'using reflog to find the fork point' '\n+\tgit reset --hard &&\n+\tgit checkout -b base $E &&\n+\n+\tfor count in 1 2 3 4 5\n+\tdo\n+\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n+\t\tgit rev-parse HEAD >expect$count &&\n+\t\tgit checkout -B derived &&\n+\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n+\t\tgit rev-parse HEAD >derived$count &&\n+\t\tgit checkout base || break\n+\tdone &&\n+\n+\tfor count in 1 2 3 4 5\n+\tdo\n+\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n+\t\ttest_cmp expect$count actual || break\n+\tdone &&\n+\n+\t# check defaulting to HEAD\n+\tgit merge-base --reflog base >actual &&\n+\ttest_cmp expect5 actual\n+'\n+\n test_done\n-- \n1.8.4.1-799-g1c32b8d\n"},{"id":"229457","messageId":"xmqqeh7akqxq.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"20131024214007.GE10779@serenity.lan","subject":"Re: [PATCH 0/2] finding the fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T02:46:57Z","receivedAt":"2013-10-25T02:46:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> To clarify: the particular commit in the calls above happens to be the\n> oldest entry in the reflog, if I pick a newer entry then it works.\n>\n> It seems that for_each_reflog_ent isn't returning the oldest entry;\n> revs.nr is 62 whereas \"git rev-list -g origin/master | wc -l\" gives 63.\n\nPerhaps this on top.\n\nOne thing I somehow feel dirty about this change is that we have to\ninclude diff.h only to include revision.h only to get the definition\nof TMP_MARK. Perhaps object.h should be the header that defines the\nbit assignment for objects.flags bit-field.\n\n builtin/merge-base.c | 28 ++++++++++++++++++++++++----\n 1 file changed, 24 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex 7b9bc15..7fdc3de 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -2,6 +2,8 @@\n #include \"cache.h\"\n #include \"commit.h\"\n #include \"refs.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n #include \"parse-options.h\"\n \n static int show_merge_base(struct commit **rev, int rev_nr, int show_all)\n@@ -91,18 +93,32 @@ struct rev_collect {\n \tstruct commit **commit;\n \tint nr;\n \tint alloc;\n+\tunsigned int initial : 1;\n };\n \n+static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)\n+{\n+\tstruct commit *commit = lookup_commit(sha1);\n+\n+\tif (!commit || (commit->object.flags & TMP_MARK))\n+\t\treturn;\n+\n+\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n+\trevs->commit[revs->nr++] = commit;\n+\tcommit->object.flags |= TMP_MARK;\n+}\n+\n static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n \t\t\t\t const char *ident, unsigned long timestamp,\n \t\t\t\t int tz, const char *message, void *cbdata_)\n {\n \tstruct rev_collect *revs = cbdata_;\n-\tstruct commit *commit = lookup_commit(nsha1);\n-\tif (commit) {\n-\t\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n-\t\trevs->commit[revs->nr++] = commit;\n+\n+\tif (revs->initial) {\n+\t\tadd_one_commit(osha1, revs);\n+\t\trevs->initial = 0;\n \t}\n+\tadd_one_commit(nsha1, revs);\n \treturn 0;\n }\n \n@@ -131,8 +147,12 @@ static int handle_reflog(int argc, const char **argv)\n \n \tderived = lookup_commit_reference(sha1);\n \tmemset(&revs, 0, sizeof(revs));\n+\trevs.initial = 1;\n \tfor_each_reflog_ent(refname, collect_one_reflog_ent, &revs);\n \n+\tfor (i = 0; i < revs.nr; i++)\n+\t\trevs.commit[i]->object.flags &= ~TMP_MARK;\n+\n \tbases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n \n \t/*\n"},{"id":"229503","messageId":"526A19CA.9020609@viscovery.net","threadId":"35152","inReplyTo":"xmqqsivql37i.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-10-25T07:12:10Z","receivedAt":"2013-10-25T07:12:10Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/25/2013 0:21, schrieb Junio C Hamano:\n> +test_expect_success 'using reflog to find the fork point' '\n> +\tgit reset --hard &&\n> +\tgit checkout -b base $E &&\n> +\n> +\tfor count in 1 2 3 4 5\n> +\tdo\n> +\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n> +\t\tgit rev-parse HEAD >expect$count &&\n> +\t\tgit checkout -B derived &&\n> +\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n> +\t\tgit rev-parse HEAD >derived$count &&\n> +\t\tgit checkout base || break\n> +\tdone &&\n> +\n> +\tfor count in 1 2 3 4 5\n> +\tdo\n> +\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n> +\t\ttest_cmp expect$count actual || break\n> +\tdone &&\n\nThis does not work as intended because the exit code of 'break' is always\nzero. Unlike 'exit' and 'return', it does *not* pick up the exit code of\nthe last command that was executed.\n\nThat's annoying, but makes some sense because 'break 2' does not mean to\napply exit code 2 to the command, either, but to break out of 2 levels of\nloops.\n\nYou could put the loops into a function from which you 'return', but that\nis obscure in this case. The first iteration was better, IMO.\n\n> +\n> +\t# check defaulting to HEAD\n> +\tgit merge-base --reflog base >actual &&\n> +\ttest_cmp expect5 actual\n> +'\n> +\n> test_done\n\n-- Hannes\n"},{"id":"229509","messageId":"20131025080923.GG10779@serenity.lan","threadId":"35152","inReplyTo":"526A19CA.9020609@viscovery.net","subject":"Re: [PATCH v2 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-25T08:09:24Z","receivedAt":"2013-10-25T08:09:24Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Oct 25, 2013 at 09:12:10AM +0200, Johannes Sixt wrote:\n> Am 10/25/2013 0:21, schrieb Junio C Hamano:\n> > +test_expect_success 'using reflog to find the fork point' '\n> > +\tgit reset --hard &&\n> > +\tgit checkout -b base $E &&\n> > +\n> > +\tfor count in 1 2 3 4 5\n> > +\tdo\n> > +\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n> > +\t\tgit rev-parse HEAD >expect$count &&\n> > +\t\tgit checkout -B derived &&\n> > +\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n> > +\t\tgit rev-parse HEAD >derived$count &&\n> > +\t\tgit checkout base || break\n> > +\tdone &&\n> > +\n> > +\tfor count in 1 2 3 4 5\n> > +\tdo\n> > +\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n> > +\t\ttest_cmp expect$count actual || break\n> > +\tdone &&\n> \n> This does not work as intended because the exit code of 'break' is always\n> zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of\n> the last command that was executed.\n> \n> That's annoying, but makes some sense because 'break 2' does not mean to\n> apply exit code 2 to the command, either, but to break out of 2 levels of\n> loops.\n> \n> You could put the loops into a function from which you 'return', but that\n> is obscure in this case. The first iteration was better, IMO.\n\nWouldn't it be simpler to just return from the test? That is, replace\nthe \"break\" in the above patch with \"return 1\".\n\nThere's code in several other test cases (e.g. t3311) which handles the\nsame problem like that.\n"},{"id":"229510","messageId":"526A28FF.4000207@viscovery.net","threadId":"35152","inReplyTo":"20131025080923.GG10779@serenity.lan","subject":"Re: [PATCH v2 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-10-25T08:17:03Z","receivedAt":"2013-10-25T08:17:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10/25/2013 10:09, schrieb John Keeping:\n> On Fri, Oct 25, 2013 at 09:12:10AM +0200, Johannes Sixt wrote:\n>> You could put the loops into a function from which you 'return', but that\n>> is obscure in this case. The first iteration was better, IMO.\n> \n> Wouldn't it be simpler to just return from the test? That is, replace\n> the \"break\" in the above patch with \"return 1\".\n\nGood catch! We explicitly have\n\ntest_eval_ () {\n\t# This is a separate function because some tests use\n\t# \"return\" to end a test_expect_success block early.\n\teval &3 2>&4 \"$*\"\n}\n\nto protect this use of return.\n\n-- Hannes\n"},{"id":"229521","messageId":"xmqqmwlxjnq6.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"526A19CA.9020609@viscovery.net","subject":"Re: [PATCH v2 2/2] merge-base: \"--reflog\" mode finds fork point from reflog entries","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T16:53:53Z","receivedAt":"2013-10-25T16:53:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> +\tfor count in 1 2 3 4 5\n>> +\tdo\n>> +\t\tgit merge-base --reflog base $(cat derived$count) >actual &&\n>> +\t\ttest_cmp expect$count actual || break\n>> +\tdone &&\n>\n> This does not work as intended because the exit code of 'break' is always\n> zero. Unlike 'exit' and 'return', it does *not* pick up the exit code of\n> the last command that was executed.\n\nYou are right. I obviously was not thinking straight.\n\n> You could put the loops into a function from which you 'return',\n> but that is obscure in this case. The first iteration was better,\n> IMO.\n\nI do not think using \"return 1\" is a good thing to do, either.\n\nWe saw breakages with different shells around the use of \"return\"\nand we know the original \"exit 1 inside subshell\" works reliably\neverywher.\n\nI'll send out a revamped version later today, updating not just the\ntest but the implementation.\n\nThank for a dose of sanity.\n"},{"id":"229528","messageId":"xmqq61sljakf.fsf_-_@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"xmqqmwlxjnq6.fsf@gitster.dls.corp.google.com","subject":"[PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-25T21:38:08Z","receivedAt":"2013-10-25T21:38:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"git pull --rebase\" command computes the fork point of the\nbranch being rebased using the reflog entries of the \"base\" branch\n(typically a remote-tracking branch) the branch's work was based on,\nin order to cope with the case in which the \"base\" branch has been\nrewound and rebuilt. For example, if the history looked like this:\n\n o---B1\n /\n ---o---o---B2--o---o---o---Base\n \\\n B3\n \\\n Derived\n\nwhere the current tip of the \"base\" branch is at Base, but earlier\nfetch observed that its tip used to be B3 and then B2 and then B1\nbefore getting to the current commit, and the branch being rebased\non top of the latest \"base\" is based on commit B3, it tries to find\nB3 by going through the output of \"git rev-list --reflog base\" (i.e.\nBase, B1, B2, B3) until it finds a commit that is an ancestor of the\ncurrent tip \"Derived\".\n\nInternally, we have get_merge_bases_many() that can compute this\nwith one-go. We would want a merge-base between Derived and a\nfictitious merge commit that would result by merging all the\nhistorical tips of \"base\". When such a commit exist, we should get\na single result, which exactly match one of the reflog entries of\n\"base\".\n\nTeach \"git merge-base\" a new mode, \"--fork-point\", to compute\nexactly that.\n\nSigned-off-by: Junio C Hamano \n---\n\n Junio C Hamano writes:\n\n > I'll send out a revamped version later today, updating not just the\n > test but the implementation.\n\n And here is the reroll. The test is back to use \"exit 1 from a subshell\"\n pattern to notice a breakage in the loop. I also rolled the \"we can\n pick one more old commit from the reflog after it got expired\"\n change John noticed into the logic.\n\n It also comes with a documentation update. The option is not called\n --reflog but --fork-point; naming a feature after what it does\n (i.e. it finds the fork point) is a lot more sensible than naming\n it after how it happens to do what it does (i.e. it does so by\n peeking into the reflog).\n\n Documentation/git-merge-base.txt | 35 ++++++++++++-\n builtin/merge-base.c | 103 +++++++++++++++++++++++++++++++++++++++\n t/t6010-merge-base.sh | 27 ++++++++++\n 3 files changed, 163 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt\nindex 87842e3..b383766 100644\n--- a/Documentation/git-merge-base.txt\n+++ b/Documentation/git-merge-base.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n 'git merge-base' [-a|--all] --octopus ...\n 'git merge-base' --is-ancestor \n 'git merge-base' --independent ...\n+'git merge-base' --fork-point []\n \n DESCRIPTION\n -----------\n@@ -24,8 +25,8 @@ that does not have any better common ancestor is a 'best common\n ancestor', i.e. a 'merge base'. Note that there can be more than one\n merge base for a pair of commits.\n \n-OPERATION MODE\n---------------\n+OPERATION MODES\n+---------------\n \n As the most common special case, specifying only two commits on the\n command line means computing the merge base between the given two commits.\n@@ -56,6 +57,11 @@ from linkgit:git-show-branch[1] when used with the `--merge-base` option.\n \tand exit with status 0 if true, or with status 1 if not.\n \tErrors are signaled by a non-zero status that is not 1.\n \n+--fork-point::\n+\tGiven a commit that is derived from possibly an earlier\n+\tincarnation of a ref, find an appropriate fork-point of the\n+\tderived history to rebase it on top of an updated base\n+\thistory (see discussion on this mode below).\n \n OPTIONS\n -------\n@@ -137,6 +143,31 @@ In modern git, you can say this in a more direct way:\n \n instead.\n \n+Discussion on fork-point mode\n+-----------------------------\n+\n+After working on the `topic` branch created with `git checkout -b\n+topic origin/master`, the history of remote-tracking branch\n+`origin/master` may have been rewound and rebuilt, leading to a\n+history of this shape:\n+\n+\t\t\t o---B1\n+\t\t\t/\n+\t---o---o---B2--o---o---o---B (origin/master)\n+\t\t\\\n+\t\t B3\n+\t\t \\\n+\t\t Derived (topic)\n+\n+where `origin/master` used to point at commits B3, B2, B1 and now it\n+points at B, and your `topic` branch was stated on top of it back\n+when `origin/master` was at B3. This mode uses the reflog of\n+`origin/master` to finds B3 as the fork point, so that the `topic`\n+can be rebased on top of the updated `origin/master` by:\n+\n+ $ fork_point=$(git merge-base --fork-point origin/master topic)\n+ $ git rebase --onto origin/master $fork_point topic\n+\n \n See also\n --------\ndiff --git a/builtin/merge-base.c b/builtin/merge-base.c\nindex d39c910..6a674e7 100644\n--- a/builtin/merge-base.c\n+++ b/builtin/merge-base.c\n@@ -1,6 +1,9 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"commit.h\"\n+#include \"refs.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n #include \"parse-options.h\"\n \n static int show_merge_base(struct commit **rev, int rev_nr, int show_all)\n@@ -27,6 +30,7 @@ static const char * const merge_base_usage[] = {\n \tN_(\"git merge-base [-a|--all] --octopus ...\"),\n \tN_(\"git merge-base --independent ...\"),\n \tN_(\"git merge-base --is-ancestor \"),\n+\tN_(\"git merge-base --fork-point []\"),\n \tNULL\n };\n \n@@ -85,6 +89,97 @@ static int handle_is_ancestor(int argc, const char **argv)\n \t\treturn 1;\n }\n \n+struct rev_collect {\n+\tstruct commit **commit;\n+\tint nr;\n+\tint alloc;\n+\tunsigned int initial : 1;\n+};\n+\n+static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)\n+{\n+\tstruct commit *commit;\n+\n+\tif (is_null_sha1(sha1))\n+\t\treturn;\n+\n+\tcommit = lookup_commit(sha1);\n+\tif (!commit ||\n+\t (commit->object.flags & TMP_MARK) ||\n+\t parse_commit(commit))\n+\t\treturn;\n+\n+\tALLOC_GROW(revs->commit, revs->nr + 1, revs->alloc);\n+\trevs->commit[revs->nr++] = commit;\n+\tcommit->object.flags |= TMP_MARK;\n+}\n+\n+static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,\n+\t\t\t\t const char *ident, unsigned long timestamp,\n+\t\t\t\t int tz, const char *message, void *cbdata)\n+{\n+\tstruct rev_collect *revs = cbdata;\n+\n+\tif (revs->initial) {\n+\t\trevs->initial = 0;\n+\t\tadd_one_commit(osha1, revs);\n+\t}\n+\tadd_one_commit(nsha1, revs);\n+\treturn 0;\n+}\n+\n+static int handle_fork_point(int argc, const char **argv)\n+{\n+\tunsigned char sha1[20];\n+\tchar *refname;\n+\tconst char *commitname;\n+\tstruct rev_collect revs;\n+\tstruct commit *derived;\n+\tstruct commit_list *bases;\n+\tint i;\n+\n+\tswitch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {\n+\tcase 0:\n+\t\tdie(\"No such ref: '%s'\", argv[0]);\n+\tcase 1:\n+\t\tbreak; /* good */\n+\tdefault:\n+\t\tdie(\"Ambiguous refname: '%s'\", argv[0]);\n+\t}\n+\n+\tcommitname = (argc == 2) ? argv[1] : \"HEAD\";\n+\tif (get_sha1(commitname, sha1))\n+\t\tdie(\"Not a valid object name: '%s'\", commitname);\n+\n+\tderived = lookup_commit_reference(sha1);\n+\tmemset(&revs, 0, sizeof(revs));\n+\trevs.initial = 1;\n+\tfor_each_reflog_ent(refname, collect_one_reflog_ent, &revs);\n+\n+\tfor (i = 0; i < revs.nr; i++)\n+\t\trevs.commit[i]->object.flags &= ~TMP_MARK;\n+\n+\tbases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n+\n+\t/*\n+\t * There should be one and only one merge base, when we found\n+\t * a common ancestor among reflog entries.\n+\t */\n+\tif (!bases || bases->next)\n+\t\treturn 1;\n+\n+\t/* And the found one must be one of the reflog entries */\n+\tfor (i = 0; i < revs.nr; i++)\n+\t\tif (&bases->item->object == &revs.commit[i]->object)\n+\t\t\tbreak; /* found */\n+\tif (revs.nr <= i)\n+\t\treturn 1; /* not found */\n+\n+\tprintf(\"%s\\n\", sha1_to_hex(bases->item->object.sha1));\n+\tfree_commit_list(bases);\n+\treturn 0;\n+}\n+\n int cmd_merge_base(int argc, const char **argv, const char *prefix)\n {\n \tstruct commit **rev;\n@@ -100,6 +195,8 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"list revs not reachable from others\"), 'r'),\n \t\tOPT_CMDMODE(0, \"is-ancestor\", &cmdmode,\n \t\t\t N_(\"is the first one ancestor of the other?\"), 'a'),\n+\t\tOPT_CMDMODE(0, \"fork-point\", &cmdmode,\n+\t\t\t N_(\"find where forked from reflog of \"), 'f'),\n \t\tOPT_END()\n \t};\n \n@@ -120,6 +217,12 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)\n \tif (cmdmode == 'r' || cmdmode == 'o')\n \t\treturn handle_octopus(argc, argv, cmdmode == 'r', show_all);\n \n+\tif (cmdmode == 'f') {\n+\t\tif (argc < 1 || 2 < argc)\n+\t\t\tusage_with_options(merge_base_usage, options);\n+\t\treturn handle_fork_point(argc, argv);\n+\t}\n+\n \tif (argc < 2)\n \t\tusage_with_options(merge_base_usage, options);\n \ndiff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\nindex f80bba8..4f09db0 100755\n--- a/t/t6010-merge-base.sh\n+++ b/t/t6010-merge-base.sh\n@@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n \ttest_cmp expected.sorted actual.sorted\n '\n \n+test_expect_success 'using reflog to find the fork point' '\n+\tgit reset --hard &&\n+\tgit checkout -b base $E &&\n+\n+\t(\n+\t\tfor count in 1 2 3 4 5\n+\t\tdo\n+\t\t\tgit commit --allow-empty -m \"Base commit #$count\" &&\n+\t\t\tgit rev-parse HEAD >expect$count &&\n+\t\t\tgit checkout -B derived &&\n+\t\t\tgit commit --allow-empty -m \"Derived #$count\" &&\n+\t\t\tgit rev-parse HEAD >derived$count &&\n+\t\t\tgit checkout base || exit 1\n+\t\tdone\n+\n+\t\tfor count in 1 2 3 4 5\n+\t\tdo\n+\t\t\tgit merge-base --fork-point base $(cat derived$count) >actual &&\n+\t\t\ttest_cmp expect$count actual || exit 1\n+\t\tdone\n+\n+\t) &&\n+\t# check that we correctly default to HEAD\n+\tgit merge-base --fork-point base >actual &&\n+\ttest_cmp expect5 actual\n+'\n+\n test_done\n-- \n1.8.4.1-797-gda615de\n"},{"id":"229529","messageId":"CAPig+cTYKVsS6t5XAfNDryfUgcQaizd4W780AVjgKrW6Zzs-Vw@mail.gmail.com","threadId":"35152","inReplyTo":"xmqq61sljakf.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-10-25T21:56:10Z","receivedAt":"2013-10-25T21:56:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 25, 2013 at 5:38 PM, Junio C Hamano wrote:\n> diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt\n> index 87842e3..b383766 100644\n> --- a/Documentation/git-merge-base.txt\n> +++ b/Documentation/git-merge-base.txt\n> @@ -137,6 +143,31 @@ In modern git, you can say this in a more direct way:\n>\n> instead.\n>\n> +Discussion on fork-point mode\n> +-----------------------------\n> +\n> +After working on the `topic` branch created with `git checkout -b\n> +topic origin/master`, the history of remote-tracking branch\n> +`origin/master` may have been rewound and rebuilt, leading to a\n> +history of this shape:\n> +\n> + o---B1\n> + /\n> + ---o---o---B2--o---o---o---B (origin/master)\n> + \\\n> + B3\n> + \\\n> + Derived (topic)\n> +\n> +where `origin/master` used to point at commits B3, B2, B1 and now it\n> +points at B, and your `topic` branch was stated on top of it back\n> +when `origin/master` was at B3. This mode uses the reflog of\n> +`origin/master` to finds B3 as the fork point, so that the `topic`\n\ns/finds/find/\n\n> +can be rebased on top of the updated `origin/master` by:\n> +\n> + $ fork_point=$(git merge-base --fork-point origin/master topic)\n> + $ git rebase --onto origin/master $fork_point topic\n> +\n>\n> See also\n> --------\n"},{"id":"229545","messageId":"CANiSa6h7x=CcwwAqvoaOm4-y+6MCugV5BE0OXnfGHvf+iJ66MA@mail.gmail.com","threadId":"35152","inReplyTo":"xmqq61sljakf.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-10-26T05:15:41Z","receivedAt":"2013-10-26T05:15:41Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Thanks for taking care of this! Maybe John or I can finally get the\nchanges to rebase done after this.\n\nA few comments below. Sorry I didn't find time to review the earlier revisions.\n\nOn Fri, Oct 25, 2013 at 2:38 PM, Junio C Hamano wrote:\n> +\n> +where `origin/master` used to point at commits B3, B2, B1 and now it\n> +points at B, and your `topic` branch was stated on top of it back\n\ns/stated/started/\n\n> + bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n> +\n> + /*\n> + * There should be one and only one merge base, when we found\n> + * a common ancestor among reflog entries.\n> + */\n> + if (!bases || bases->next)\n> + return 1;\n> +\n> + /* And the found one must be one of the reflog entries */\n> + for (i = 0; i < revs.nr; i++)\n> + if (&bases->item->object == &revs.commit[i]->object)\n> + break; /* found */\n> + if (revs.nr <= i)\n> + return 1; /* not found */\n> +\n> + printf(\"%s\\n\", sha1_to_hex(bases->item->object.sha1));\n> + free_commit_list(bases);\n> + return 0;\n\nShould free_commit_list also be called in the two \"return 1\" cases\nabove? I suppose the process will exit soon after this, but that seems\nto be true for all three cases.\n\n> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n> index f80bba8..4f09db0 100755\n> --- a/t/t6010-merge-base.sh\n> +++ b/t/t6010-merge-base.sh\n> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n> test_cmp expected.sorted actual.sorted\n> '\n>\n> +test_expect_success 'using reflog to find the fork point' '\n> + git reset --hard &&\n> + git checkout -b base $E &&\n> +\n> + (\n> + for count in 1 2 3 4 5\n> + do\n> + git commit --allow-empty -m \"Base commit #$count\" &&\n> + git rev-parse HEAD >expect$count &&\n> + git checkout -B derived &&\n> + git commit --allow-empty -m \"Derived #$count\" &&\n> + git rev-parse HEAD >derived$count &&\n> + git checkout base || exit 1\n\nI think this creates a history like\n\n---E---B1--B2--B3--B4--B5 (base)\n \\ \\ \\ \\ \\\n D1 D2 D3 D4 D5 (derived)\n\nSo I think the following test would pass even if you drop the\n--fork-point. Did you mean to create a fan-shaped history by resetting\nbase to $E on every iteration above?\n\n> + git merge-base --fork-point base $(cat derived$count) >actual &&\n> + test_cmp expect$count actual || exit 1\n"},{"id":"229555","messageId":"20131026090034.GH10779@serenity.lan","threadId":"35152","inReplyTo":"xmqq61sljakf.fsf_-_@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-26T09:00:35Z","receivedAt":"2013-10-26T09:00:35Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Fri, Oct 25, 2013 at 02:38:08PM -0700, Junio C Hamano wrote:\n> It also comes with a documentation update. The option is not called\n> --reflog but --fork-point; naming a feature after what it does\n> (i.e. it finds the fork point) is a lot more sensible than naming\n> it after how it happens to do what it does (i.e. it does so by\n> peeking into the reflog).\n\nI think the new name is likely to confuse normal users - when talking\nabout a branch, you can talk about where it forked from and in that case\nit normally means the merge-base of that branch and master.\n\nThe --reflog name has the advantage that it makes clear that this is\nlooking at something more than the commit graph and I don't think\n--fork-point does imply that.\n"},{"id":"229640","messageId":"xmqqwqkxh2q5.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"CANiSa6h7x=CcwwAqvoaOm4-y+6MCugV5BE0OXnfGHvf+iJ66MA@mail.gmail.com","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-28T14:47:14Z","receivedAt":"2013-10-28T14:47:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n>> + bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);\n>> + ...\n>> + if (revs.nr <= i)\n>> + return 1; /* not found */\n>> +\n>> + printf(\"%s\\n\", sha1_to_hex(bases->item->object.sha1));\n>> + free_commit_list(bases);\n>> + return 0;\n>\n> Should free_commit_list also be called in the two \"return 1\" cases\n> above? I suppose the process will exit soon after this, but that seems\n> to be true for all three cases.\n\nYou are right that the above is inconsistent. Because the code\nintends to be called only once in the lifetime of the program,\nit calls get_merge_bases_many() with cleanup set to 0, so in that\nsense, not freeing them anywhere may make it even more clear that\nthis function expects to be shortly followed by a process exit.\n\n>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh\n>> index f80bba8..4f09db0 100755\n>> --- a/t/t6010-merge-base.sh\n>> +++ b/t/t6010-merge-base.sh\n>> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for octopus-step' '\n>> test_cmp expected.sorted actual.sorted\n>> '\n>>\n>> +test_expect_success 'using reflog to find the fork point' '\n>> + git reset --hard &&\n>> + git checkout -b base $E &&\n>> +\n>> + (\n>> + for count in 1 2 3 4 5\n>> + do\n>> + git commit --allow-empty -m \"Base commit #$count\" &&\n>> + git rev-parse HEAD >expect$count &&\n>> + git checkout -B derived &&\n>> + git commit --allow-empty -m \"Derived #$count\" &&\n>> + git rev-parse HEAD >derived$count &&\n>> + git checkout base || exit 1\n>\n> I think this creates a history like\n>\n> ---E---B1--B2--B3--B4--B5 (base)\n> \\ \\ \\ \\ \\\n> D1 D2 D3 D4 D5 (derived)\n>\n> So I think the following test would pass even if you drop the\n> --fork-point. Did you mean to create a fan-shaped history by resetting\n> base to $E on every iteration above?\n\nJust showing that I didn't think things deeply ;-) I do agree that a\nfan-shaped history would show what we want to do a lot better.\n\nThanks.\n\n>> + git merge-base --fork-point base $(cat derived$count) >actual &&\n>> + test_cmp expect$count actual || exit 1\n"},{"id":"229658","messageId":"xmqq38nlfbu5.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"20131026090034.GH10779@serenity.lan","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-28T19:13:22Z","receivedAt":"2013-10-28T19:13:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> The --reflog name has the advantage that it makes clear that this is\n> looking at something more than the commit graph and I don't think\n> --fork-point does imply that.\n\nI think I understand what you are saying, but that \"more than the\ncommit graph\" part in your reasoning is exactly one of the two\nreasons why I do not think that it is a good idea to call it with\n\"reflog\" in its name. The next round of update to the feature may\nfind a better way to find the fork point than looking at the reflog.\nWhat the feature is meant to do, i.e. \"find the fork point\" can stay\nthe same (i.e. people can use it in their script), while the way how\nthe implementation achieves it (i.e. by looking at reflog) can\nevolve over time, and by not hardcoding \"how\" in the name, the users\nwill benefit from the updated behaviour, without having to ask for a\nbetter heuristics by using a different option by updating all of\ntheir scripts.\n\nThe other reason (of my two reasons) is because I do not think\n\"finding the fork-point\" will stay to be the _only_ feature that\nuses reflog in merge-base. When a next cool feature to compute\nsomething completely different from fork-point happens to be\nimplemented by taking advantage of reflog data, what would we call\nit?\n"},{"id":"229714","messageId":"20131029085111.GA24023@serenity.lan","threadId":"35152","inReplyTo":"xmqq38nlfbu5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2013-10-29T08:51:11Z","receivedAt":"2013-10-29T08:51:11Z","isPatch":true,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Mon, Oct 28, 2013 at 12:13:22PM -0700, Junio C Hamano wrote:\n> John Keeping writes:\n> \n> > The --reflog name has the advantage that it makes clear that this is\n> > looking at something more than the commit graph and I don't think\n> > --fork-point does imply that.\n> \n> I think I understand what you are saying, but that \"more than the\n> commit graph\" part in your reasoning is exactly one of the two\n> reasons why I do not think that it is a good idea to call it with\n> \"reflog\" in its name. The next round of update to the feature may\n> find a better way to find the fork point than looking at the reflog.\n> What the feature is meant to do, i.e. \"find the fork point\" can stay\n> the same (i.e. people can use it in their script), while the way how\n> the implementation achieves it (i.e. by looking at reflog) can\n> evolve over time, and by not hardcoding \"how\" in the name, the users\n> will benefit from the updated behaviour, without having to ask for a\n> better heuristics by using a different option by updating all of\n> their scripts.\n\nOK - given this reasoning I agree that --fork-point makes sense.\n\nI think this would have been clearer if the short description said\nsomething like:\n\n Find the point at which a branch forked from another branch; this\n does not just look for the common ancestor of the two commits but\n also takes into account the reflog of to see if the branch\n forked from an earlier incarnation.\n"},{"id":"229745","messageId":"xmqq61sfet1u.fsf@gitster.dls.corp.google.com","threadId":"35152","inReplyTo":"20131029085111.GA24023@serenity.lan","subject":"Re: [PATCH v3 2/2] merge-base: teach \"--fork-point\" mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-29T20:11:25Z","receivedAt":"2013-10-29T20:11:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping writes:\n\n> OK - given this reasoning I agree that --fork-point makes sense.\n>\n> I think this would have been clearer if the short description said\n> something like:\n>\n> Find the point at which a branch forked from another branch; this\n> does not just look for the common ancestor of the two commits but\n> also takes into account the reflog of to see if the branch\n> forked from an earlier incarnation.\n\nThat's much easier to read. Will squash the following in (I do want\nto make sure that it is clear that does not have to be at\nthe tip, and also does not have to be a branch---it could be\nany ref).\n\nThanks.\n\n --fork-point::\n-\tGiven a commit that is derived from possibly an earlier\n-\tincarnation of a ref, find an appropriate fork-point of the\n-\tderived history to rebase it on top of an updated base\n-\thistory (see discussion on this mode below).\n+\tFind the point at which a branch (or any history that leads\n+\tto ) forked from another branch (or any reference)\n+\t. This does not just look for the common ancestor of\n+\tthe two commits, but also takes into account the reflog of\n+\t to see if the history leading to forked from\n+\tan earlier incarnation of the branch (see discussion\n+\ton this mode below).\n \n OPTIONS\n -------\n"}]}