{"thread":{"id":"18747","subject":"[PATCH 3/3] git remote update: Fallback to remote if group does not exist","startedAt":"2009-04-06T13:40:59Z","lastAt":"2009-04-08T18:48:49Z","messageCount":10,"participants":["Finn Arne Gangstad","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"110577","messageId":"1239025262-16960-1-git-send-email-finnag@pvv.org","threadId":"18747","inReplyTo":null,"subject":"[PATCH 0/3] git remote update: Check args and fallback to remotes","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2009-04-06T13:40:59Z","receivedAt":"2009-04-06T13:40:59Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"This series is on top of next.\n\ngit remote update would previously silently do nothing.\nWith this patch series, it will (with 1/3) error out when non-existing groups\nare given, and with 2/3 & 3/3 it will use a remote if a group cannot be found.\n\nThis enables \"git remote update origin\" for example. All previous uses\nof \"git remote update \" that did something useful should still work\nexactly as before.\n\nThere seems to be no current way to check for the existence of a configured\nremote, so 2/3 adds a remote_is_configured() function which checks for a\nconfigured remote.\n\nFinn Arne Gangstad (3):\n git remote update: Report error for non-existing groups\n remote: New function remote_is_configured()\n git remote update: Fallback to remote if group does not exist\n\n Documentation/git-remote.txt | 2 +-\n builtin-remote.c | 17 ++++++++++++++---\n remote.c | 11 +++++++++++\n remote.h | 1 +\n 4 files changed, 27 insertions(+), 4 deletions(-)\n"},{"id":"110576","messageId":"1239025262-16960-2-git-send-email-finnag@pvv.org","threadId":"18747","inReplyTo":"1239025262-16960-1-git-send-email-finnag@pvv.org","subject":"[PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2009-04-06T13:41:00Z","receivedAt":"2009-04-06T13:41:00Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"Previosly, git remote update would just silently fail\nand do nothing. Now it will report an error saying that the group does\nnot exist.\n\nSigned-off-by: Finn Arne Gangstad \n---\n builtin-remote.c | 11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 3146eb4..51df99b 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -1188,16 +1188,18 @@ struct remote_group {\n \tstruct string_list *list;\n } remote_group;\n \n-static int get_remote_group(const char *key, const char *value, void *cb)\n+static int get_remote_group(const char *key, const char *value, void *num_hits)\n {\n \tif (!prefixcmp(key, \"remotes.\") &&\n \t\t\t!strcmp(key + 8, remote_group.name)) {\n \t\t/* split list by white space */\n \t\tint space = strcspn(value, \" \\t\\n\");\n \t\twhile (*value) {\n-\t\t\tif (space > 1)\n+\t\t\tif (space > 1) {\n \t\t\t\tstring_list_append(xstrndup(value, space),\n \t\t\t\t\t\tremote_group.list);\n+\t\t\t\t++*((int *)num_hits);\n+\t\t\t}\n \t\t\tvalue += space + (value[space] != '\\0');\n \t\t\tspace = strcspn(value, \" \\t\\n\");\n \t\t}\n@@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)\n \n \tremote_group.list = &list;\n \tfor (i = 1; i < argc; i++) {\n+\t\tint groups_found = 0;\n \t\tremote_group.name = argv[i];\n-\t\tresult = git_config(get_remote_group, NULL);\n+\t\tresult = git_config(get_remote_group, &groups_found);\n+\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\")))\n+\t\t\tdie(\"No such remote group: '%s'\", argv[i]);\n \t}\n \n \tif (!result && !list.nr && argc == 2 && !strcmp(argv[1], \"default\"))\n-- \n1.6.2.1.471.gdfdaa\n"},{"id":"110575","messageId":"1239025262-16960-3-git-send-email-finnag@pvv.org","threadId":"18747","inReplyTo":"1239025262-16960-1-git-send-email-finnag@pvv.org","subject":"[PATCH 2/3] remote: New function remote_is_configured()","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2009-04-06T13:41:01Z","receivedAt":"2009-04-06T13:41:01Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"Previously, there was no beautiful way to check for the existence of\na configured remote. remote_get for example would always create the remote\n\"on demand\".\n\nThis new function returns 1 if the remote is configured, 0 otherwise.\n\nSigned-off-by: Finn Arne Gangstad \n---\n remote.c | 11 +++++++++++\n remote.h | 1 +\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex d12140e..a06761a 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -667,6 +667,17 @@ struct remote *remote_get(const char *name)\n \treturn ret;\n }\n \n+int remote_is_configured(const char *name)\n+{\n+\tint i;\n+\tread_config();\n+\n+\tfor (i = 0; i < remotes_nr; i++)\n+\t\tif (!strcmp(name, remotes[i]->name))\n+\t\t\treturn 1;\n+\treturn 0;\n+}\n+\n int for_each_remote(each_remote_fn fn, void *priv)\n {\n \tint i, result = 0;\ndiff --git a/remote.h b/remote.h\nindex de3d21b..99706a8 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -45,6 +45,7 @@ struct remote {\n };\n \n struct remote *remote_get(const char *name);\n+int remote_is_configured(const char *name);\n \n typedef int each_remote_fn(struct remote *remote, void *priv);\n int for_each_remote(each_remote_fn fn, void *priv);\n-- \n1.6.2.1.471.gdfdaa\n"},{"id":"110574","messageId":"1239025262-16960-4-git-send-email-finnag@pvv.org","threadId":"18747","inReplyTo":"1239025262-16960-1-git-send-email-finnag@pvv.org","subject":"[PATCH 3/3] git remote update: Fallback to remote if group does not exist","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2009-04-06T13:41:02Z","receivedAt":"2009-04-06T13:41:02Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"Previously, git remote update would fail unless there was\na remote group configured with the same name as the remote.\ngit remote update will now fall back to using the remote if no matching\ngroup can be found.\n\nThis enables \"git remote update -p ...\" to fetch and prune one\nor more remotes, for example.\n\nSigned-off-by: Finn Arne Gangstad \n---\n Documentation/git-remote.txt | 2 +-\n builtin-remote.c | 10 ++++++++--\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex 0b6e67d..9e2b4ea 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n 'git remote set-head' [-a | -d | ]\n 'git remote show' [-n] \n 'git remote prune' [-n | --dry-run] \n-'git remote update' [-p | --prune] [group]\n+'git remote update' [-p | --prune] [group | remote]...\n \n DESCRIPTION\n -----------\ndiff --git a/builtin-remote.c b/builtin-remote.c\nindex 51df99b..ca7c639 100644\n--- a/builtin-remote.c\n+++ b/builtin-remote.c\n@@ -1232,8 +1232,14 @@ static int update(int argc, const char **argv)\n \t\tint groups_found = 0;\n \t\tremote_group.name = argv[i];\n \t\tresult = git_config(get_remote_group, &groups_found);\n-\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\")))\n-\t\t\tdie(\"No such remote group: '%s'\", argv[i]);\n+\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\"))) {\n+\t\t\tstruct remote *remote;\n+\t\t\tif (!remote_is_configured(argv[i]))\n+\t\t\t\tdie(\"No such remote or remote group: %s\",\n+\t\t\t\t argv[i]);\n+\t\t\tremote = remote_get(argv[i]);\n+\t\t\tstring_list_append(remote->name, remote_group.list);\n+\t\t}\n \t}\n \n \tif (!result && !list.nr && argc == 2 && !strcmp(argv[1], \"default\"))\n-- \n1.6.2.1.471.gdfdaa\n"},{"id":"110625","messageId":"20090406201823.GD28120@coredump.intra.peff.net","threadId":"18747","inReplyTo":"1239025262-16960-1-git-send-email-finnag@pvv.org","subject":"Re: [PATCH 0/3] git remote update: Check args and fallback to remotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-06T20:18:23Z","receivedAt":"2009-04-06T20:18:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 06, 2009 at 03:40:59PM +0200, Finn Arne Gangstad wrote:\n\n> This series is on top of next.\n> \n> git remote update would previously silently do nothing.\n> With this patch series, it will (with 1/3) error out when non-existing groups\n> are given, and with 2/3 & 3/3 it will use a remote if a group cannot be found.\n\nGreat, this was on my todo list so I am happy that procrastination saved\nme some work. :)\n\nThe patches look fine to me, except that there are no tests. The patch\nbelow adds a \"remote groups\" test script. There is a slight bit of\noverlap with the update tests from t5505, but I don't think it is a\nproblem.\n\nIt is intended to be applied before your series. Your 1/3 would switch\nt5506.3 from expect_failure to expect_success, and 3/3 would switch\nt5506.6 from failure to success.\n\n-- >8 --\nSubject: [PATCH] add tests for remote groups\n\nThis tries to systematically cover existing behavior, and\nalso mark some expect_failure cases for desired behavior.\n\nSigned-off-by: Jeff King \n---\n t/t5506-remote-groups.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 81 insertions(+), 0 deletions(-)\n create mode 100755 t/t5506-remote-groups.sh\n\ndiff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh\nnew file mode 100755\nindex 0000000..6653a9c\n--- /dev/null\n+++ b/t/t5506-remote-groups.sh\n@@ -0,0 +1,81 @@\n+#!/bin/sh\n+\n+test_description='git remote group handling'\n+. ./test-lib.sh\n+\n+mark() {\n+\techo \"$1\" >mark\n+}\n+\n+update_repo() {\n+\t(cd $1 &&\n+\techo content >>file &&\n+\tgit add file &&\n+\tgit commit -F ../mark)\n+}\n+\n+update_repos() {\n+\tupdate_repo one $1 &&\n+\tupdate_repo two $1\n+}\n+\n+repo_fetched() {\n+\tif test \"`git log -1 --pretty=format:%s $1 --`\" = \"`cat mark`\"; then\n+\t\techo >&2 \"repo was fetched: $1\"\n+\t\treturn 0\n+\tfi\n+\techo >&2 \"repo was not fetched: $1\"\n+\treturn 1\n+}\n+\n+test_expect_success 'setup' '\n+\tmkdir one && (cd one && git init) &&\n+\tmkdir two && (cd two && git init) &&\n+\tgit remote add -m master one one &&\n+\tgit remote add -m master two two\n+'\n+\n+test_expect_success 'no group updates all' '\n+\tmark update-all &&\n+\tupdate_repos &&\n+\tgit remote update &&\n+\trepo_fetched one &&\n+\trepo_fetched two\n+'\n+\n+test_expect_failure 'nonexistant group produces error' '\n+\tmark nonexistant &&\n+\tupdate_repos &&\n+\ttest_must_fail git remote update nonexistant &&\n+\t! repo_fetched one &&\n+\t! repo_fetched two\n+'\n+\n+test_expect_success 'updating group updates all members' '\n+\tmark group-all &&\n+\tupdate_repos &&\n+\tgit config --add remotes.all one &&\n+\tgit config --add remotes.all two &&\n+\tgit remote update all &&\n+\trepo_fetched one &&\n+\trepo_fetched two\n+'\n+\n+test_expect_success 'updating group does not update non-members' '\n+\tmark group-some &&\n+\tupdate_repos &&\n+\tgit config --add remotes.some one &&\n+\tgit remote update some &&\n+\trepo_fetched one &&\n+\t! repo_fetched two\n+'\n+\n+test_expect_failure 'updating remote name updates that remote' '\n+\tmark remote-name &&\n+\tupdate_repos &&\n+\tgit remote update one &&\n+\trepo_fetched one &&\n+\t! repo_fetched two\n+'\n+\n+test_done\n-- \n1.6.2.2.585.g1e067\n"},{"id":"110768","messageId":"7vprfnubyi.fsf@gitster.siamese.dyndns.org","threadId":"18747","inReplyTo":"1239025262-16960-2-git-send-email-finnag@pvv.org","subject":"Re: [PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T02:16:21Z","receivedAt":"2009-04-08T02:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Finn Arne Gangstad writes:\n\n> @@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)\n> \n> \tremote_group.list = &list;\n> \tfor (i = 1; i < argc; i++) {\n> +\t\tint groups_found = 0;\n> \t\tremote_group.name = argv[i];\n> -\t\tresult = git_config(get_remote_group, NULL);\n> +\t\tresult = git_config(get_remote_group, &groups_found);\n> +\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\")))\n> +\t\t\tdie(\"No such remote group: '%s'\", argv[i]);\n\nI think you are trying to be silent about the case where the caller feeds\nyou the default_argv[] array with this, but do we want to be more explicit\nabout this so that we do die when the end user explicitly says \"default\"\nfrom the command line?\n"},{"id":"110808","messageId":"20090408080738.GA24386@pvv.org","threadId":"18747","inReplyTo":"7vprfnubyi.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Finn Arne Gangstad","fromEmail":"finnag@pvv.org","sentAt":"2009-04-08T08:07:38Z","receivedAt":"2009-04-08T08:07:38Z","isPatch":true,"sender":{"key":"finnag@pvv.org","avatar":"https://gravatar.com/avatar/b421ddd58c3f0f93aa473e17b98bb8d53c221fef741746bc8cb59fae4ec6d95e?d=mp&s=160"},"body":"On Tue, Apr 07, 2009 at 07:16:21PM -0700, Junio C Hamano wrote:\n> Finn Arne Gangstad writes:\n> \n> > @@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)\n> > \n> > \tremote_group.list = &list;\n> > \tfor (i = 1; i < argc; i++) {\n> > +\t\tint groups_found = 0;\n> > \t\tremote_group.name = argv[i];\n> > -\t\tresult = git_config(get_remote_group, NULL);\n> > +\t\tresult = git_config(get_remote_group, &groups_found);\n> > +\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\")))\n> > +\t\t\tdie(\"No such remote group: '%s'\", argv[i]);\n> \n> I think you are trying to be silent about the case where the caller feeds\n> you the default_argv[] array with this, but do we want to be more explicit\n> about this so that we do die when the end user explicitly says \"default\"\n> from the command line?\n\nAre you thinking that \"git remote update default\" should only be allowed\nif you have configured a group named default?\n\nThe old code would allow \"git remote update default\" and actually do the\nsame as \"git remote update\", so I wanted to keep the (possibly unwanted?)\nbehaviour. If we want to disallow it, we can just do\nif (!groups_found && argv != default_argv) instead.\n\n- Finn Arne\n"},{"id":"110810","messageId":"7vy6ubo8tn.fsf@gitster.siamese.dyndns.org","threadId":"18747","inReplyTo":"20090408080738.GA24386@pvv.org","subject":"Re: [PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T08:20:36Z","receivedAt":"2009-04-08T08:20:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Finn Arne Gangstad writes:\n\n> On Tue, Apr 07, 2009 at 07:16:21PM -0700, Junio C Hamano wrote:\n>> Finn Arne Gangstad writes:\n>> \n>> > @@ -1227,8 +1229,11 @@ static int update(int argc, const char **argv)\n>> > \n>> > \tremote_group.list = &list;\n>> > \tfor (i = 1; i < argc; i++) {\n>> > +\t\tint groups_found = 0;\n>> > \t\tremote_group.name = argv[i];\n>> > -\t\tresult = git_config(get_remote_group, NULL);\n>> > +\t\tresult = git_config(get_remote_group, &groups_found);\n>> > +\t\tif (!groups_found && (i != 1 || strcmp(argv[1], \"default\")))\n>> > +\t\t\tdie(\"No such remote group: '%s'\", argv[i]);\n>> \n>> I think you are trying to be silent about the case where the caller feeds\n>> you the default_argv[] array with this, but do we want to be more explicit\n>> about this so that we do die when the end user explicitly says \"default\"\n>> from the command line?\n>\n> Are you thinking that \"git remote update default\" should only be allowed\n> if you have configured a group named default?\n\nI have no preference either way, and that is why I asked.\n\n\"git remote update\" without explicit \"default\" is obviously what your code\ntry not to say \"No such remote group\" to, and that probably is a sane\nthing to do.\n\nI don't know what users want to see when they say \"default\" explicitly\nwithout having an explicit configuration. Should it do the same thing as\n\"git remote update\"?\n"},{"id":"110846","messageId":"20090408170844.GB28069@coredump.intra.peff.net","threadId":"18747","inReplyTo":"7vy6ubo8tn.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-04-08T17:08:45Z","receivedAt":"2009-04-08T17:08:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 08, 2009 at 01:20:36AM -0700, Junio C Hamano wrote:\n\n> I don't know what users want to see when they say \"default\" explicitly\n> without having an explicit configuration. Should it do the same thing as\n> \"git remote update\"?\n\nI'm not sure we have a choice anymore; is it worth breaking\ncompatibility to \"fix\" something that doesn't actually seem to be\nharming anyone?\n\n-Peff\n"},{"id":"110858","messageId":"7v1vs3nfqm.fsf@gitster.siamese.dyndns.org","threadId":"18747","inReplyTo":"20090408170844.GB28069@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] git remote update: Report error for non-existing groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-08T18:48:49Z","receivedAt":"2009-04-08T18:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, Apr 08, 2009 at 01:20:36AM -0700, Junio C Hamano wrote:\n>\n>> I don't know what users want to see when they say \"default\" explicitly\n>> without having an explicit configuration. Should it do the same thing as\n>> \"git remote update\"?\n>\n> I'm not sure we have a choice anymore; is it worth breaking\n> compatibility to \"fix\" something that doesn't actually seem to be\n> harming anyone?\n\nNope. I do not use the \"remote update\" myself to begin with, and I\nsomehow suspect that the reason it does not seem to be harming anyone is\nbecause nobody sane uses these \"remote groups\".\n\nAnyway, I took the patch as-is already.\n"}]}