{"thread":{"id":"18262","subject":"[PATCH] Give error when no remote is configured","startedAt":"2009-03-11T05:47:20Z","lastAt":"2009-03-17T16:06:42Z","messageCount":7,"participants":["Daniel Barkalow","Bernie Innocenti","Junio C Hamano","Jay Soffian","Giovanni Bajo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107646","messageId":"alpine.LNX.1.00.0903110139450.19665@iabervon.org","threadId":"18262","inReplyTo":null,"subject":"[PATCH] Give error when no remote is configured","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-11T05:47:20Z","receivedAt":"2009-03-11T05:47:20Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"When there's no explicitly-named remote, we use the remote specified\nfor the current branch, which in turn defaults to \"origin\". But it\nthis case should require the remote to actually be configured, and not\nfall back to the path \"origin\".\n\nPossibly, the config file's \"remote = something\" should require the\nsomething to be a configured remote instead of a bare repository URL,\nbut we actually test with a bare repository URL.\n\nIn fetch, we were giving the sensible error message when coming up\nwith a URL failed, but this wasn't actually reachable, so move that\nerror up and use it when appropriate.\n\nIn push, we need a new error message, because the old one (formerly\nunreachable without a lot of help) used the repo name, which was NULL.\n\nSigned-off-by: Daniel Barkalow <barkalow@iabervon.org>\n---\nI think the main way to reach this in actual usage would be to use git-svn \nto create a repository and then forget that you used it and therefore \ndon't have an origin.\n\n builtin-fetch.c |    6 +++---\n builtin-push.c  |    7 +++++--\n remote.c        |   30 +++++++++++++++++++++++++++---\n 3 files changed, 35 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 1e4a3d9..7fb35fc 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -636,6 +636,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \telse\n \t\tremote = remote_get(argv[0]);\n \n+\tif (!remote)\n+\t\tdie(\"Where do you want to fetch from today?\");\n+\n \ttransport = transport_get(remote, remote->url[0]);\n \tif (verbosity >= 2)\n \t\ttransport->verbose = 1;\n@@ -648,9 +651,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (depth)\n \t\tset_option(TRANS_OPT_DEPTH, depth);\n \n-\tif (!transport->url)\n-\t\tdie(\"Where do you want to fetch from today?\");\n-\n \tif (argc > 1) {\n \t\tint j = 0;\n \t\trefs = xcalloc(argc + 1, sizeof(const char *));\ndiff --git a/builtin-push.c b/builtin-push.c\nindex 122fdcf..ca36fb1 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -53,8 +53,11 @@ static int do_push(const char *repo, int flags)\n \tint i, errs;\n \tstruct remote *remote = remote_get(repo);\n \n-\tif (!remote)\n-\t\tdie(\"bad repository '%s'\", repo);\n+\tif (!remote) {\n+\t\tif (repo)\n+\t\t\tdie(\"bad repository '%s'\", repo);\n+\t\tdie(\"No destination configured to push to.\");\n+\t}\n \n \tif (remote->mirror)\n \t\tflags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);\ndiff --git a/remote.c b/remote.c\nindex d7079c6..199830e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -38,6 +38,7 @@ static int branches_nr;\n \n static struct branch *current_branch;\n static const char *default_remote_name;\n+static int explicit_default_remote_name;\n \n static struct rewrite **rewrite;\n static int rewrite_alloc;\n@@ -104,6 +105,16 @@ static void add_url_alias(struct remote *remote, const char *url)\n \tadd_url(remote, alias_url(url));\n }\n \n+static struct remote *get_remote_by_name(const char *name)\n+{\n+\tint i;\n+\tfor (i = 0; i < remotes_nr; i++) {\n+\t\tif (!strcmp(name, remotes[i]->name))\n+\t\t\treturn remotes[i];\n+\t}\n+\treturn NULL;\n+}\n+\n static struct remote *make_remote(const char *name, int len)\n {\n \tstruct remote *ret;\n@@ -330,8 +341,10 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n \t\t\tbranch->remote_name = xstrdup(value);\n-\t\t\tif (branch == current_branch)\n+\t\t\tif (branch == current_branch) {\n \t\t\t\tdefault_remote_name = branch->remote_name;\n+\t\t\t\texplicit_default_remote_name = 1;\n+\t\t\t}\n \t\t} else if (!strcmp(subkey, \".merge\")) {\n \t\t\tif (!value)\n \t\t\t\treturn config_error_nonbool(key);\n@@ -643,11 +656,22 @@ static int valid_remote_nick(const char *name)\n struct remote *remote_get(const char *name)\n {\n \tstruct remote *ret;\n+\tint name_given = 0;\n \n \tread_config();\n-\tif (!name)\n+\tif (name)\n+\t\tname_given = 1;\n+\telse {\n \t\tname = default_remote_name;\n-\tret = make_remote(name, 0);\n+\t\tname_given = explicit_default_remote_name;\n+\t}\n+\tif (name_given)\n+\t\tret = make_remote(name, 0);\n+\telse {\n+\t\tret = get_remote_by_name(name);\n+\t\tif (!ret)\n+\t\t\treturn NULL;\n+\t}\n \tif (valid_remote_nick(name)) {\n \t\tif (!ret->url)\n \t\t\tread_remotes_file(ret);\n-- \n1.6.2.104.g7aeb2.dirty\n"},{"id":"107648","messageId":"49B757E3.8000807@codewiz.org","threadId":"18262","inReplyTo":"alpine.LNX.1.00.0903110139450.19665@iabervon.org","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Bernie Innocenti","fromEmail":"bernie@codewiz.org","sentAt":"2009-03-11T06:19:15Z","receivedAt":"2009-03-11T06:19:15Z","isPatch":true,"sender":{"key":"bernie@codewiz.org","avatar":"https://gravatar.com/avatar/42735a3728f3ff3f989a12e9d6931e6e742865fedc6f5eca63cdb327eb820720?d=mp&s=160"},"body":"Daniel Barkalow wrote:\n> When there's no explicitly-named remote, we use the remote specified\n> for the current branch, which in turn defaults to \"origin\". But it\n> this case should require the remote to actually be configured, and not\n> fall back to the path \"origin\".\n> [...]\n\nThanks, works like a charm!\n\n-- \n   // Bernie Innocenti - http://www.codewiz.org/\n \\X/  Sugar Labs       - http://www.sugarlabs.org/\n"},{"id":"108069","messageId":"7vocw2x7ob.fsf@gitster.siamese.dyndns.org","threadId":"18262","inReplyTo":"alpine.LNX.1.00.0903110139450.19665@iabervon.org","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-16T07:12:36Z","receivedAt":"2009-03-16T07:12:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> When there's no explicitly-named remote, we use the remote specified\n> for the current branch, which in turn defaults to \"origin\". But it\n> this case should require the remote to actually be configured, and not\n> fall back to the path \"origin\".\n\nThis is seriously broken.\n\n> @@ -643,11 +656,22 @@ static int valid_remote_nick(const char *name)\n>  struct remote *remote_get(const char *name)\n>  {\n>  \tstruct remote *ret;\n> +\tint name_given = 0;\n>  \n>  \tread_config();\n> -\tif (!name)\n> +\tif (name)\n> +\t\tname_given = 1;\n> +\telse {\n>  \t\tname = default_remote_name;\n> -\tret = make_remote(name, 0);\n> +\t\tname_given = explicit_default_remote_name;\n> +\t}\n> +\tif (name_given)\n> +\t\tret = make_remote(name, 0);\n> +\telse {\n> +\t\tret = get_remote_by_name(name);\n> +\t\tif (!ret)\n> +\t\t\treturn NULL;\n> +\t}\n\nWhen you do not have any config entry to name your remotes but have been\nusing .git/remotes/origin happily, you may have read config already at\nthis point, but when you call get_remote_by_name() you haven't read\nanything from .git/remotes/* (nor .git/branches/* for that matter).  The\ncaller will get NULL in such a case.  This happens for both fetch and\npush.\n\nBecause you did not have any test to protect whatever you wanted to \"fix\"\nwith your patch, I have no way knowing if I am breaking something else you\nwanted to do with your patch, but the patch below at least fixes the\nregression for me when running \"git pull\" in a repository I initialized\nlong time ago that does not use the .git/config file to specify where my\nremote repositories are.\n\nIt applies on top of fa685bd (Give error when no remote is configured,\n2009-03-11)\n\n-- >8 --\nSubject: Remove total confusion from git-fetch and git-push\n\nThe config file is not the only place remotes are defined, and without\nconsulting .git/remotes and .git/branches, you won't know if \"origin\" is\nconfigured by the user.  Don't give up too early and insult the user with\na wisecrack \"Where do you want to fetch from today?\"\n\nInsulting is ok, but I personally get really pissed off if a tool is both\nconfused and insulting.  At least be _correct_ and insulting.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n remote.c |   21 ++++-----------------\n 1 files changed, 4 insertions(+), 17 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 199830e..9f07dbc 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -105,16 +105,6 @@ static void add_url_alias(struct remote *remote, const char *url)\n \tadd_url(remote, alias_url(url));\n }\n \n-static struct remote *get_remote_by_name(const char *name)\n-{\n-\tint i;\n-\tfor (i = 0; i < remotes_nr; i++) {\n-\t\tif (!strcmp(name, remotes[i]->name))\n-\t\t\treturn remotes[i];\n-\t}\n-\treturn NULL;\n-}\n-\n static struct remote *make_remote(const char *name, int len)\n {\n \tstruct remote *ret;\n@@ -665,19 +655,16 @@ struct remote *remote_get(const char *name)\n \t\tname = default_remote_name;\n \t\tname_given = explicit_default_remote_name;\n \t}\n-\tif (name_given)\n-\t\tret = make_remote(name, 0);\n-\telse {\n-\t\tret = get_remote_by_name(name);\n-\t\tif (!ret)\n-\t\t\treturn NULL;\n-\t}\n+\n+\tret = make_remote(name, 0);\n \tif (valid_remote_nick(name)) {\n \t\tif (!ret->url)\n \t\t\tread_remotes_file(ret);\n \t\tif (!ret->url)\n \t\t\tread_branches_file(ret);\n \t}\n+\tif (!name_given && !ret->url)\n+\t\treturn NULL;\n \tif (!ret->url)\n \t\tadd_url_alias(ret, name);\n \tif (!ret->url)\n"},{"id":"108099","messageId":"alpine.LNX.1.00.0903161204240.19665@iabervon.org","threadId":"18262","inReplyTo":"7vocw2x7ob.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-16T16:55:26Z","receivedAt":"2009-03-16T16:55:26Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 16 Mar 2009, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> > When there's no explicitly-named remote, we use the remote specified\n> > for the current branch, which in turn defaults to \"origin\". But it\n> > this case should require the remote to actually be configured, and not\n> > fall back to the path \"origin\".\n> \n> This is seriously broken.\n> \n> > @@ -643,11 +656,22 @@ static int valid_remote_nick(const char *name)\n> >  struct remote *remote_get(const char *name)\n> >  {\n> >  \tstruct remote *ret;\n> > +\tint name_given = 0;\n> >  \n> >  \tread_config();\n> > -\tif (!name)\n> > +\tif (name)\n> > +\t\tname_given = 1;\n> > +\telse {\n> >  \t\tname = default_remote_name;\n> > -\tret = make_remote(name, 0);\n> > +\t\tname_given = explicit_default_remote_name;\n> > +\t}\n> > +\tif (name_given)\n> > +\t\tret = make_remote(name, 0);\n> > +\telse {\n> > +\t\tret = get_remote_by_name(name);\n> > +\t\tif (!ret)\n> > +\t\t\treturn NULL;\n> > +\t}\n> \n> When you do not have any config entry to name your remotes but have been\n> using .git/remotes/origin happily, you may have read config already at\n> this point, but when you call get_remote_by_name() you haven't read\n> anything from .git/remotes/* (nor .git/branches/* for that matter).  The\n> caller will get NULL in such a case.  This happens for both fetch and\n> push.\n\nThat's actually a simple bug; the block that's just after what you quoted \nshould be just before it. I thought we had a test for having \"origin\" \ndefined by one of the old methods, but I guess not. Your version is \nbetter, though; I'd forgotten that using the name as the URL was in \nremote_get() and not make_remote().\n\n> Because you did not have any test to protect whatever you wanted to \"fix\"\n> with your patch, I have no way knowing if I am breaking something else you\n> wanted to do with your patch,\n\n$ git init\n$ git fetch\n\nShouldn't try fetching from ./origin/.git; I suppose the best test would \nbe to do something like:\n\n$ mkdir origin\n$ (cd origin; git init; touch a; git add a; git commit -m \"initial\")\n$ git init\n$ git fetch\n\nWith test_must_fail. (But I'm more going for having it not give weird \nerrors in an error situation, which is kind of fluffy to try to test.)\n\n> but the patch below at least fixes the\n> regression for me when running \"git pull\" in a repository I initialized\n> long time ago that does not use the .git/config file to specify where my\n> remote repositories are.\n> \n> It applies on top of fa685bd (Give error when no remote is configured,\n> 2009-03-11)\n> \n> -- >8 --\n> Subject: Remove total confusion from git-fetch and git-push\n> \n> The config file is not the only place remotes are defined, and without\n> consulting .git/remotes and .git/branches, you won't know if \"origin\" is\n> configured by the user.  Don't give up too early and insult the user with\n> a wisecrack \"Where do you want to fetch from today?\"\n\nYou actually wrote that message, in 853a3697. I think a better message \nwould probably be something like:\n\nNo default remote is configured for your current branch, and the default \nremote \"origin\" is not configured either.\n\nI think the message missed being made user-friendly in earlier passes due \nto being inaccessible at the time.\n\n> Insulting is ok, but I personally get really pissed off if a tool is both\n> confused and insulting.  At least be _correct_ and insulting.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  remote.c |   21 ++++-----------------\n>  1 files changed, 4 insertions(+), 17 deletions(-)\n> \n> diff --git a/remote.c b/remote.c\n> index 199830e..9f07dbc 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -105,16 +105,6 @@ static void add_url_alias(struct remote *remote, const char *url)\n>  \tadd_url(remote, alias_url(url));\n>  }\n>  \n> -static struct remote *get_remote_by_name(const char *name)\n> -{\n> -\tint i;\n> -\tfor (i = 0; i < remotes_nr; i++) {\n> -\t\tif (!strcmp(name, remotes[i]->name))\n> -\t\t\treturn remotes[i];\n> -\t}\n> -\treturn NULL;\n> -}\n> -\n>  static struct remote *make_remote(const char *name, int len)\n>  {\n>  \tstruct remote *ret;\n> @@ -665,19 +655,16 @@ struct remote *remote_get(const char *name)\n>  \t\tname = default_remote_name;\n>  \t\tname_given = explicit_default_remote_name;\n>  \t}\n> -\tif (name_given)\n> -\t\tret = make_remote(name, 0);\n> -\telse {\n> -\t\tret = get_remote_by_name(name);\n> -\t\tif (!ret)\n> -\t\t\treturn NULL;\n> -\t}\n> +\n> +\tret = make_remote(name, 0);\n>  \tif (valid_remote_nick(name)) {\n>  \t\tif (!ret->url)\n>  \t\t\tread_remotes_file(ret);\n>  \t\tif (!ret->url)\n>  \t\t\tread_branches_file(ret);\n>  \t}\n> +\tif (!name_given && !ret->url)\n> +\t\treturn NULL;\n>  \tif (!ret->url)\n>  \t\tadd_url_alias(ret, name);\n>  \tif (!ret->url)\n> \n"},{"id":"108114","messageId":"76718490903161301qca2214cta87411bad1b0b8b5@mail.gmail.com","threadId":"18262","inReplyTo":"alpine.LNX.1.00.0903161204240.19665@iabervon.org","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-16T20:01:07Z","receivedAt":"2009-03-16T20:01:07Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 16, 2009 at 12:55 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n> No default remote is configured for your current branch, and the default\n> remote \"origin\" is not configured either.\n\nThe use of \"default\" twice is slightly confusing. Perhaps:\n\nNo remote is configured for the current branch, and the default\nremote \"origin\" is not configured either.\n\nj.\n"},{"id":"108214","messageId":"49BEEAF4.40403@develer.com","threadId":"18262","inReplyTo":"76718490903161301qca2214cta87411bad1b0b8b5@mail.gmail.com","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Giovanni Bajo","fromEmail":"rasky@develer.com","sentAt":"2009-03-17T00:12:36Z","receivedAt":"2009-03-17T00:12:36Z","isPatch":true,"sender":{"key":"rasky@develer.com","avatar":"https://gravatar.com/avatar/852c310d41ec49adc82a6e2937c16aa0dc87bad4da1be1d686da48aa255b875e?d=mp&s=160"},"body":"On 3/16/2009 9:01 PM, Jay Soffian wrote:\n> On Mon, Mar 16, 2009 at 12:55 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:\n>> No default remote is configured for your current branch, and the default\n>> remote \"origin\" is not configured either.\n> \n> The use of \"default\" twice is slightly confusing. Perhaps:\n> \n> No remote is configured for the current branch, and the default\n> remote \"origin\" is not configured either.\n\nI'm a total newbie with git. I must say that the above sentence means \nabsolutely nothing to me (in either version) because of the confusing \nusage of the word \"remote\" (twice, one as a substantive, one as an \nadjective) and the word \"origin\" which is git jargon which I don't \nmaster yet.\n\nMy suggestion is that you should at least add a sentence that points to \na likely solution. Something like:\n\n   (use \"git remote add\" to configure a remote URL)\n\nNote that I don't have any clue if this sentece is correct and/or is the \ncorrect solution. The above is just an example of a helpful error message.\n-- \nGiovanni Bajo\nDeveler S.r.l.\nhttp://www.develer.com\n"},{"id":"108245","messageId":"alpine.LNX.1.00.0903171121260.19665@iabervon.org","threadId":"18262","inReplyTo":"49BEEAF4.40403@develer.com","subject":"Re: [PATCH] Give error when no remote is configured","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2009-03-17T16:06:42Z","receivedAt":"2009-03-17T16:06:42Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 17 Mar 2009, Giovanni Bajo wrote:\n\n> On 3/16/2009 9:01 PM, Jay Soffian wrote:\n> > On Mon, Mar 16, 2009 at 12:55 PM, Daniel Barkalow <barkalow@iabervon.org>\n> > wrote:\n> > > No default remote is configured for your current branch, and the default\n> > > remote \"origin\" is not configured either.\n> > \n> > The use of \"default\" twice is slightly confusing. Perhaps:\n> > \n> > No remote is configured for the current branch, and the default\n> > remote \"origin\" is not configured either.\n> \n> I'm a total newbie with git. I must say that the above sentence means\n> absolutely nothing to me (in either version) because of the confusing usage of\n> the word \"remote\" (twice, one as a substantive, one as an adjective) and the\n> word \"origin\" which is git jargon which I don't master yet.\n\nActually, it's not that complicated a sentence. In order to \nfetch/pull/push, you need to specify some attributes of the other side; at \na minimum, this is the location, but you can also specify other things \n(HTTP gateways if you're using HTTP, for example). You can have multiple \nof these sets of configuration stored under different names; these are \nremotes.\n\nWhen you run fetch, you can specify the remote on the command line. If you \ndon't specify, there are two levels of defaults: the first is a setting in \nthe configuration for whichever branch is current; the second is the \nconstant \"origin\".\n\nThe message is trying to say that it fell back to looking for a configured \nremote named \"origin\", but there wasn't one configured.\n\n> My suggestion is that you should at least add a sentence that points to a\n> likely solution. Something like:\n> \n>   (use \"git remote add\" to configure a remote URL)\n> \n> Note that I don't have any clue if this sentece is correct and/or is the\n> correct solution. The above is just an example of a helpful error message.\n\nThe problem is that there are a number of different sorts of configuration \nyou might want, and we have no way of knowing which it is.\n\nYou might mean to explicitly specify URLs every time to fetch or push in \nthis repository; you might mean to have a different URL for each branch \nyou work on; or you might mean to have a general default. It's also \npossible that you actually meant \"git svn fetch\", which uses different \nconfiguration info.\n\nSo I think we can only give pointers to the various things it looked for, \nand leave it up to the documentation to explain which you want, if any, \nand how to get it.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}