{"thread":{"id":"35801","subject":"[Bug] branch.*.merge interpreted too strictly by tracking logic","startedAt":"2014-02-04T22:49:16Z","lastAt":"2014-10-14T22:14:28Z","messageCount":6,"participants":["Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"234257","messageId":"xmqqa9e67atv.fsf@gitster.dls.corp.google.com","threadId":"35801","inReplyTo":null,"subject":"[Bug] branch.*.merge interpreted too strictly by tracking logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-04T22:49:16Z","receivedAt":"2014-02-04T22:49:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Start from an empty repository like so:\n\n    : gitster track; git init\n    Initialized empty Git repository in /var/tmp/x/track/.git/\n    : gitster track/master; git commit --allow-empty -m initial\n    [master (root-commit) abdcd1c] initial\n    : gitster track/master; git branch foo\n    : gitster track/master; git branch bar\n    : gitster track/master; git commit --allow-empty -m second\n    [master 78e28f4] second\n\nNow, 'master' has two commits, while 'foo' and 'bar' both are one\ncommit behind, pointing at 'master^1'.\n\nLet's tell these branches that they are both supposed to be building\non top of 'master'.\n\n    : gitster track/master; git config branch.foo.remote .\n    : gitster track/master; git config branch.foo.merge refs/heads/master\n    : gitster track/master; git config branch.bar.remote .\n    : gitster track/master; git config branch.bar.merge master\n\nThe difference between the two is that 'foo' spells the @{upstream}\nbranch in full (which is the recommended practice), while 'bar' is\nloose and asks for 'master'.\n\nLet's see what happens on these two branches.  First 'foo':\n\n    : gitster track/master; git checkout foo\n    Switched to branch 'foo'\n    Your branch is behind 'master' by 1 commit, and can be\n    fast-forwarded.\n      (use \"git pull\" to update your local branch)\n    : gitster track/foo; git pull\n    From .\n     * branch            master     -> FETCH_HEAD\n    Updating abdcd1c..78e28f4\n    Fast-forward\n\nThe 'checkout' correctly reports that 'foo' is building on (local)\n'master'.  'pull' works as expected, of course.\n\nNow, here is the bug part.  The same thing on 'bar':\n\n    : gitster track/foo; git checkout bar\n    Switched to branch 'bar'\n    Your branch is based on 'master', but the upstream is gone.\n      (use \"git branch --unset-upstream\" to fixup)\n\nIt knows about 'master', but what is \"the upstream is gone\"?  It is\nour local repository and it definitely is not gone.\n\nBut 'pull' of course works as expected (this behaviour is older and\nstable for a long time since even before @{upstream} was invented).\n\n    : gitster track/bar; git pull\n    From .\n     * branch            master     -> FETCH_HEAD\n    Updating abdcd1c..78e28f4\n    Fast-forward\n\nI suspect that the real culprit is somewhere in wt-status.c\n\n    : gitster track/bar; git status\n    On branch bar\n    Your branch is based on 'master', but the upstream is gone.\n      (use \"git branch --unset-upstream\" to fixup)\n\n    nothing to commit, working directory clean\n\nResolving @{upstream} works just fine for both.\n\n    : gitster track/bar; git rev-parse --symbolic-full-name foo@{upstream}\n    refs/heads/master\n    : gitster track/bar; git rev-parse --symbolic-full-name bar@{upstream}\n    refs/heads/master\n\nThanks.\n"},{"id":"234332","messageId":"20140205205030.GA16394@sigill.intra.peff.net","threadId":"35801","inReplyTo":"xmqqa9e67atv.fsf@gitster.dls.corp.google.com","subject":"Re: [Bug] branch.*.merge interpreted too strictly by tracking logic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T20:50:30Z","receivedAt":"2014-02-05T20:50:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 04, 2014 at 02:49:16PM -0800, Junio C Hamano wrote:\n\n> Let's tell these branches that they are both supposed to be building\n> on top of 'master'.\n> \n>     : gitster track/master; git config branch.foo.remote .\n>     : gitster track/master; git config branch.foo.merge refs/heads/master\n>     : gitster track/master; git config branch.bar.remote .\n>     : gitster track/master; git config branch.bar.merge master\n> \n> The difference between the two is that 'foo' spells the @{upstream}\n> branch in full (which is the recommended practice), while 'bar' is\n> loose and asks for 'master'.\n\nIs it legal to put an unqualified ref there? A wise man once said[1]:\n\n  > Actually, it is broken in a lot of places. for-each-ref relies on\n  > the same code as \"git status\", \"git checkout\", etc, which will all\n  > fail to display tracking info. I believe the same code is also used\n  > for updating tracking branches on push. So I'm not sure if it was\n  > ever intended to be a valid setting.\n\n  It wasn't.  Some places may accept them gracefully by either being\n  extra nice or by accident.\n\nI don't recall us ever doing anything after that. I don't have a problem\nwith making it work, of course, but I am not sure if it is really a bug.\n\n-Peff\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/121671\n"},{"id":"234337","messageId":"xmqqbnyl2run.fsf@gitster.dls.corp.google.com","threadId":"35801","inReplyTo":"20140205205030.GA16394@sigill.intra.peff.net","subject":"Re: [Bug] branch.*.merge interpreted too strictly by tracking logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T21:05:04Z","receivedAt":"2014-02-05T21:05:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Is it legal to put an unqualified ref there? A wise man once said[1]:\n>\n>   > Actually, it is broken in a lot of places. for-each-ref relies on\n>   > the same code as \"git status\", \"git checkout\", etc, which will all\n>   > fail to display tracking info. I believe the same code is also used\n>   > for updating tracking branches on push. So I'm not sure if it was\n>   > ever intended to be a valid setting.\n>\n>   It wasn't.  Some places may accept them gracefully by either being\n>   extra nice or by accident.\n>\n> I don't recall us ever doing anything after that. I don't have a problem\n> with making it work, of course, but I am not sure if it is really a bug.\n\nOnce people get used to us being extra nice in some places, other\nless nice places start looking more and more like bugs. It is an\nunfortunate fact of life, but fixing them up is a good thing for\nusers.  As long as we can make those less nice places nicer\nuniformly without bending backwards or introducing unnecessary\nambiguities, that is, and I think this one can be done without\nsuch downsides.\n"},{"id":"234338","messageId":"20140205210831.GB16753@sigill.intra.peff.net","threadId":"35801","inReplyTo":"xmqqbnyl2run.fsf@gitster.dls.corp.google.com","subject":"Re: [Bug] branch.*.merge interpreted too strictly by tracking logic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-05T21:08:31Z","receivedAt":"2014-02-05T21:08:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 05, 2014 at 01:05:04PM -0800, Junio C Hamano wrote:\n\n> > I don't recall us ever doing anything after that. I don't have a problem\n> > with making it work, of course, but I am not sure if it is really a bug.\n> \n> Once people get used to us being extra nice in some places, other\n> less nice places start looking more and more like bugs. It is an\n> unfortunate fact of life, but fixing them up is a good thing for\n> users.  As long as we can make those less nice places nicer\n> uniformly without bending backwards or introducing unnecessary\n> ambiguities, that is, and I think this one can be done without\n> such downsides.\n\nOh, absolutely, and I do not think we are breaking anything to start\nhandling it better (my \"I don't have a problem...\" above). But I guess I\nam doubting that people are actually doing this at all now. I'd expect\nmost people to have the config set automatically by \"branch\" or\n\"checkout\", or to use \"branch --set-upstream-to\". Did your report come\nout of a real case, or was it just something you noticed?\n\n-Peff\n"},{"id":"234341","messageId":"xmqq38jx2rl9.fsf@gitster.dls.corp.google.com","threadId":"35801","inReplyTo":"20140205210831.GB16753@sigill.intra.peff.net","subject":"Re: [Bug] branch.*.merge interpreted too strictly by tracking logic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-05T21:10:42Z","receivedAt":"2014-02-05T21:10:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> .... Did your report come\n> out of a real case, or was it just something you noticed?\n\nSome git-wrappers (like \"repo\") are reported to muck with the\nconfiguration files.\n"},{"id":"250629","messageId":"xmqqbnpexprv.fsf@gitster.dls.corp.google.com","threadId":"35801","inReplyTo":"xmqqa9e67atv.fsf@gitster.dls.corp.google.com","subject":"[PATCH] checkout: report upstream correctly even with loosely defined branch.*.merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-14T22:14:28Z","receivedAt":"2014-10-14T22:14:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When checking out a branch that is set to build on top of another\nbranch (often, a remote-tracking branch), \"git checkout\" reports how\nyour work relates to the other branch, e.g.\n\n    Your branch is behind 'origin/master', and can be fast-forwarded.\n\nBack when this feature was introduced, this was only done for\nbranches that build on remote-tracking branches, but 5e6e2b48 (Make\nlocal branches behave like remote branches when --tracked,\n2009-04-01) added support to give the same report for branches that\nbuild on other local branches (i.e. branches whose branch.*.remote\nvariables are set to '.').  Unlike the support for the branches\nbuilding on remote-tracking branches, however, this did not take\ninto account the fact that branch.*.merge configuration is allowed\nto record a shortened branch name.\n\nWhen branch.*.merge is set to 'master' (not 'refs/heads/master'),\ni.e. \"my branch builds on the local 'master' branch\", this caused\n\"git checkout\" to report:\n\n    Your branch is based on 'master', but the upstream is gone.\n\nThe upstream is our repository and is definitely not gone, so this\noutput is nonsense.\n\nThe fix is fairly obvious; just like the branch name is DWIMed when\n\"git pull\" merges from the 'master' branch without complaint on such\na branch, the name of the branch the current branch builds upon\nneeds to be DWIMed the same way.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n remote.c                 | 34 +++++++++++++++++++++++-----------\n t/t2024-checkout-dwim.sh | 18 ++++++++++++++++++\n 2 files changed, 41 insertions(+), 11 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 0e9459c..ecbe363 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1611,6 +1611,27 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,\n \t}\n }\n \n+static void set_merge(struct branch *ret)\n+{\n+\tchar *ref;\n+\tunsigned char sha1[20];\n+\tint i;\n+\n+\tret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));\n+\tfor (i = 0; i < ret->merge_nr; i++) {\n+\t\tret->merge[i] = xcalloc(1, sizeof(**ret->merge));\n+\t\tret->merge[i]->src = xstrdup(ret->merge_name[i]);\n+\t\tif (!remote_find_tracking(ret->remote, ret->merge[i]) ||\n+\t\t    strcmp(ret->remote_name, \".\"))\n+\t\t\tcontinue;\n+\t\tif (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),\n+\t\t\t     sha1, &ref) == 1)\n+\t\t\tret->merge[i]->dst = ref;\n+\t\telse\n+\t\t\tret->merge[i]->dst = xstrdup(ret->merge_name[i]);\n+\t}\n+}\n+\n struct branch *branch_get(const char *name)\n {\n \tstruct branch *ret;\n@@ -1622,17 +1643,8 @@ struct branch *branch_get(const char *name)\n \t\tret = make_branch(name, 0);\n \tif (ret && ret->remote_name) {\n \t\tret->remote = remote_get(ret->remote_name);\n-\t\tif (ret->merge_nr) {\n-\t\t\tint i;\n-\t\t\tret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));\n-\t\t\tfor (i = 0; i < ret->merge_nr; i++) {\n-\t\t\t\tret->merge[i] = xcalloc(1, sizeof(**ret->merge));\n-\t\t\t\tret->merge[i]->src = xstrdup(ret->merge_name[i]);\n-\t\t\t\tif (remote_find_tracking(ret->remote, ret->merge[i])\n-\t\t\t\t    && !strcmp(ret->remote_name, \".\"))\n-\t\t\t\t\tret->merge[i]->dst = xstrdup(ret->merge_name[i]);\n-\t\t\t}\n-\t\t}\n+\t\tif (ret->merge_nr)\n+\t\t\tset_merge(ret);\n \t}\n \treturn ret;\n }\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex 6ecb559..468a000 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -185,4 +185,22 @@ test_expect_success 'checkout <branch> -- succeeds, even if a file with the same\n \ttest_branch_upstream spam repo_c spam\n '\n \n+test_expect_success 'loosely defined local base branch is reported correctly' '\n+\n+\tgit checkout master &&\n+\tgit branch strict &&\n+\tgit branch loose &&\n+\tgit commit --allow-empty -m \"a bit more\" &&\n+\n+\ttest_config branch.strict.remote . &&\n+\ttest_config branch.loose.remote . &&\n+\ttest_config branch.strict.merge refs/heads/master &&\n+\ttest_config branch.loose.merge master &&\n+\n+\tgit checkout strict | sed -e \"s/strict/BRANCHNAME/g\" >expect &&\n+\tgit checkout loose | sed -e \"s/loose/BRANCHNAME/g\" >actual &&\n+\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.1.2-492-gf8d07d7\n"}]}