{"thread":{"id":"34545","subject":"[PATCH] branch: make sure the upstream remote is configured","startedAt":"2013-07-26T17:39:37Z","lastAt":"2013-07-26T23:22:09Z","messageCount":6,"participants":["Carlos Martín Nieto","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"224133","messageId":"1374860377-17652-1-git-send-email-cmn@elego.de","threadId":"34545","inReplyTo":null,"subject":"[PATCH] branch: make sure the upstream remote is configured","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-07-26T17:39:37Z","receivedAt":"2013-07-26T17:39:37Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"A command of e.g.\n\n git push --set-upstream /tmp/t master\n\nwill call install_branch_config() with a remote name of \"/tmp/t\". This\nfunction will set the 'branch.master.remote' key to, which is\nnonsensical as there is no remote by that name.\n\nInstead, make sure that the remote given does exist when writing the\nconfiguration and warn if it does not. In order to distinguish named\nremotes, introduce REMOTE_NONE as the default origin for remotes,\nwhich the functions reading from the different sources will\noverwrite. Thus, an origin of REMOTE_NONE means it has been created at\nrun-time in order to push to it.\n\nSigned-off-by: Carlos Martín Nieto \n---\n\nIt's somewhat surprising that there didn't seem to be a way to\ndistinguish named remotes from those created from a command-line path,\nbut I guess nobody needed to.\n\n branch.c | 11 +++++++++++\n remote.h | 1 +\n t/t5523-push-upstream.sh | 5 +++++\n 3 files changed, 17 insertions(+)\n\ndiff --git a/branch.c b/branch.c\nindex c5c6984..cefb8f6 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -53,6 +53,7 @@ void install_branch_config(int flag, const char *local, const char *origin, cons\n \tint remote_is_branch = !prefixcmp(remote, \"refs/heads/\");\n \tstruct strbuf key = STRBUF_INIT;\n \tint rebasing = should_setup_rebase(origin);\n+\tstruct remote *r = remote_get(origin);\n \n \tif (remote_is_branch\n \t && !strcmp(local, shortname)\n@@ -62,6 +63,16 @@ void install_branch_config(int flag, const char *local, const char *origin, cons\n \t\treturn;\n \t}\n \n+\t/*\n+\t * Make sure that the remote passed is a configured remote, or\n+\t * we end up setting 'branch.foo.remote = /tmp/t' which is\n+\t * nonsensical.\n+\t */\n+\tif (origin && strcmp(origin, \".\") && r->origin == REMOTE_NONE) {\n+\t\twarning(_(\"there is no remote named '%s', no upstream configuration will be set.\"), origin);\n+\t\treturn;\n+\t}\n+\n \tstrbuf_addf(&key, \"branch.%s.remote\", local);\n \tgit_config_set(key.buf, origin ? origin : \".\");\n \ndiff --git a/remote.h b/remote.h\nindex cf56724..92f6e33 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -2,6 +2,7 @@\n #define REMOTE_H\n \n enum {\n+\tREMOTE_NONE,\n \tREMOTE_CONFIG,\n \tREMOTE_REMOTES,\n \tREMOTE_BRANCHES\ndiff --git a/t/t5523-push-upstream.sh b/t/t5523-push-upstream.sh\nindex 3683df1..e84c2f8 100755\n--- a/t/t5523-push-upstream.sh\n+++ b/t/t5523-push-upstream.sh\n@@ -71,6 +71,11 @@ test_expect_success 'push -u HEAD' '\n \tcheck_config headbranch upstream refs/heads/headbranch\n '\n \n+test_expect_success 'push -u ' '\n+ git push -u parent HEAD 2>err &&\n+ grep \"no upstream configuration will be set\" err\n+'\n+\n test_expect_success TTY 'progress messages go to tty' '\n \tensure_fresh_upstream &&\n \n-- \n1.8.3\n"},{"id":"224135","messageId":"20130726184311.GA29799@sigill.intra.peff.net","threadId":"34545","inReplyTo":"1374860377-17652-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH] branch: make sure the upstream remote is configured","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-26T18:43:11Z","receivedAt":"2013-07-26T18:43:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:\n\n> A command of e.g.\n> \n> git push --set-upstream /tmp/t master\n> \n> will call install_branch_config() with a remote name of \"/tmp/t\". This\n> function will set the 'branch.master.remote' key to, which is\n> nonsensical as there is no remote by that name.\n\nIs it nonsensical? It does not make sense for the @{upstream} magic\ntoken, because we will not have a branch in tracking branch refs/remotes\nto point to. But the configuration would still affect how \"git pull\"\nchooses a branch to fetch and merge.\n\nI.e., you can currently do:\n\n git push --set-upstream /tmp/t master\n git pull ;# pulls from /tmp/t master\n\n-Peff\n"},{"id":"224136","messageId":"20130726184815.GB29799@sigill.intra.peff.net","threadId":"34545","inReplyTo":"20130726184311.GA29799@sigill.intra.peff.net","subject":"Re: [PATCH] branch: make sure the upstream remote is configured","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-26T18:48:15Z","receivedAt":"2013-07-26T18:48:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 26, 2013 at 02:43:11PM -0400, Jeff King wrote:\n\n> On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:\n> \n> > A command of e.g.\n> > \n> > git push --set-upstream /tmp/t master\n> > \n> > will call install_branch_config() with a remote name of \"/tmp/t\". This\n> > function will set the 'branch.master.remote' key to, which is\n> > nonsensical as there is no remote by that name.\n> \n> Is it nonsensical? It does not make sense for the @{upstream} magic\n> token, because we will not have a branch in tracking branch refs/remotes\n\nEh, I am incapable of typing (and proofreading). That should be \"not\nhave a tracking branch in refs/remotes\".\n\n-Peff\n"},{"id":"224152","messageId":"1374877787.2670.6.camel@centaur.cmartin.tk","threadId":"34545","inReplyTo":"20130726184311.GA29799@sigill.intra.peff.net","subject":"Re: [PATCH] branch: make sure the upstream remote is configured","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-07-26T22:29:47Z","receivedAt":"2013-07-26T22:29:47Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Fri, 2013-07-26 at 14:43 -0400, Jeff King wrote:\n> On Fri, Jul 26, 2013 at 07:39:37PM +0200, Carlos Martín Nieto wrote:\n> \n> > A command of e.g.\n> > \n> > git push --set-upstream /tmp/t master\n> > \n> > will call install_branch_config() with a remote name of \"/tmp/t\". This\n> > function will set the 'branch.master.remote' key to, which is\n> > nonsensical as there is no remote by that name.\n> \n> Is it nonsensical? It does not make sense for the @{upstream} magic\n> token, because we will not have a branch in tracking branch refs/remotes\n\nThis was the main point, yes; the only time I've seen it used is by\nmistake/misunderstanding, and thinking that you wouldn't want to do\nsomething like what's below.\n\nYou are also unable to do this kind of thing through git-branch, and as\nit seemed to be an oversight, I wanted to tighten it up.\n\n> to point to. But the configuration would still affect how \"git pull\"\n> chooses a branch to fetch and merge.\n> \n> I.e., you can currently do:\n> \n> git push --set-upstream /tmp/t master\n> git pull ;# pulls from /tmp/t master\n\nInterestingly, this actually fetches the right branch from the remote. I\nwasn't expecting something like this to work at all.\n\nSomewhat doubtful that this usage is something you'd really want to do,\nI see that it does behave properly.\n\n cmn\n"},{"id":"224157","messageId":"20130726231211.GB12968@sigill.intra.peff.net","threadId":"34545","inReplyTo":"1374877787.2670.6.camel@centaur.cmartin.tk","subject":"Re: [PATCH] branch: make sure the upstream remote is configured","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-26T23:12:11Z","receivedAt":"2013-07-26T23:12:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 27, 2013 at 12:29:47AM +0200, Carlos Martín Nieto wrote:\n\n> > Is it nonsensical? It does not make sense for the @{upstream} magic\n> > token, because we will not have a branch in tracking branch refs/remotes\n> \n> This was the main point, yes; the only time I've seen it used is by\n> mistake/misunderstanding, and thinking that you wouldn't want to do\n> something like what's below.\n\nIf that is what you want to prevent, I do not think checking for a named\nremote is sufficient. You can also be pushing to a branch on a named\nremote that is not part of your fetch refspec, in which case you do not\nhave a tracking branch. I.e.:\n\n git clone $URL repo.git\n cd repo.git\n git push --set-upstream HEAD:refs/foo/whatever\n\nFor that matter, I wonder what \"--set-upstream\" would do if used with\n\"refs/tags/foo\". You would not do that in general, but what about:\n\n git push --set-upstream master:master master:v1.0\n\nI didn't test.\n\n> > to point to. But the configuration would still affect how \"git pull\"\n> > chooses a branch to fetch and merge.\n> > \n> > I.e., you can currently do:\n> > \n> > git push --set-upstream /tmp/t master\n> > git pull ;# pulls from /tmp/t master\n> \n> Interestingly, this actually fetches the right branch from the remote. I\n> wasn't expecting something like this to work at all.\n> \n> Somewhat doubtful that this usage is something you'd really want to do,\n> I see that it does behave properly.\n\nI do not claim to have used it myself. Tightening the \"--set-upstream\"\nbehavior would not hurt people who want to configure such a thing\nmanually, and it might catch errors from people doing it accidentally.\n\nSo even though the config it generates is not nonsensical, there is a\nreasonable chance it was an error, and tightening may make sense. But I\nthink you would not want the condition to be \"this is a named remote\",\nbut rather \"the generated configuration actually has an @{upstream}\".\n\n-Peff\n"},{"id":"224159","messageId":"20130726232208.GC12968@sigill.intra.peff.net","threadId":"34545","inReplyTo":"20130726231211.GB12968@sigill.intra.peff.net","subject":"Re: [PATCH] branch: make sure the upstream remote is configured","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-07-26T23:22:09Z","receivedAt":"2013-07-26T23:22:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 26, 2013 at 07:12:11PM -0400, Jeff King wrote:\n\n> If that is what you want to prevent, I do not think checking for a named\n> remote is sufficient. You can also be pushing to a branch on a named\n> remote that is not part of your fetch refspec, in which case you do not\n> have a tracking branch. I.e.:\n> \n> git clone $URL repo.git\n> cd repo.git\n> git push --set-upstream HEAD:refs/foo/whatever\n> \n> For that matter, I wonder what \"--set-upstream\" would do if used with\n> \"refs/tags/foo\". You would not do that in general, but what about:\n> \n> git push --set-upstream master:master master:v1.0\n> \n> I didn't test.\n\nAh, nevermind. We already catch the case of non-heads (on both the local\nand remote sides) and abort.\n\nSo that makes me more confident that your change is a reasonable one; we\nare already disallowing a subset of what's possible via \"--set-upstream\"\nin the name of preventing weird accidental configurations. This is just\nfixing another such loophole.\n\n-Peff\n"}]}