{"thread":{"id":"23380","subject":"[PATCH] ls-remote: default to 'origin' when no remote specified","startedAt":"2010-04-08T03:58:03Z","lastAt":"2010-04-09T09:15:51Z","messageCount":18,"participants":["Tay Ray Chuan","Jeff King","Junio C Hamano","Peter Kjellerstedt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"138916","messageId":"1270699083-5424-1-git-send-email-rctay89@gmail.com","threadId":"23380","inReplyTo":null,"subject":"[PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T03:58:03Z","receivedAt":"2010-04-08T03:58:03Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Instead of breaking execution when no remote (as specified in the\nvariable dest) is specified when git-ls-remote is invoked, continue on\nand let remote_get() handle it.\n\nThat way, we are able to use the default remote (by default, \"origin\"),\nas git-fetch, git-push, and others, do.\n\nWhile we're at it, die with a more interesting message (\"Where do you\nwant to...\"), as git-fetch does, instead of the plain usage help.\n\nAdd several tests to check that git-ls-remote handles the\nno-remote-specified situation.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/ls-remote.c  |    5 ++---\n t/t5512-ls-remote.sh |   37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 39 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 70f5622..dfada83 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n-\tif (!dest)\n-\t\tusage(ls_remote_usage);\n-\n \tif (argv[i]) {\n \t\tint j;\n \t\tpattern = xcalloc(sizeof(const char *), argc - i + 1);\n@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \tremote = remote_get(dest);\n+\tif (!remote)\n+\t\tdie(\"Where do you want to list from today?\");\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n \ttransport = transport_get(remote, NULL);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 1dd8eed..e19429b 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -49,4 +49,41 @@ test_expect_success 'ls-remote self' '\n \n '\n \n+cat >exp <<EOF\n+fatal: Where do you want to list from today?\n+EOF\n+test_expect_success 'dies with message when no remote specified and no default remote found' '\n+\n+\t!(git ls-remote >actual 2>&1) &&\n+\ttest_cmp exp actual\n+\n+'\n+\n+test_expect_success 'defaults to \"origin\" when no remote specified' '\n+\n+\tgit remote add origin \"$(pwd)/.git\"\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n+cat >exp <<EOF\n+fatal: 'refs*master' does not appear to be a git repository\n+fatal: The remote end hung up unexpectedly\n+EOF\n+test_expect_success 'confuses pattern as remote when no remote specified' '\n+\t#\n+\t# Although ugly, this behaviour is akin to the confusion of refspecs for\n+\t# remotes by git-fetch and git-push, eg:\n+\t#\n+\t#   $ git fetch branch\n+\t#\n+\n+\t# We could just as easily have used \"master\"; the \"*\" emphasizes its\n+\t# role as a pattern.\n+\t!(git ls-remote refs*master >actual 2>&1) &&\n+\ttest_cmp exp actual\n+\n+'\n+\n test_done\n-- \n1.7.0.97.g1372c\n"},{"id":"138919","messageId":"20100408044552.GA30473@coredump.intra.peff.net","threadId":"23380","inReplyTo":"1270699083-5424-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T04:45:52Z","receivedAt":"2010-04-08T04:45:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 08, 2010 at 11:58:03AM +0800, Tay Ray Chuan wrote:\n\n> Instead of breaking execution when no remote (as specified in the\n> variable dest) is specified when git-ls-remote is invoked, continue on\n> and let remote_get() handle it.\n> \n> That way, we are able to use the default remote (by default, \"origin\"),\n> as git-fetch, git-push, and others, do.\n> \n> While we're at it, die with a more interesting message (\"Where do you\n> want to...\"), as git-fetch does, instead of the plain usage help.\n\nI don't really see a problem with this. The current behavior produces an\nerror, so it is not as if we are breaking somebody's workflow, and the\nonly sensible default is the same one used by the other commands.\n\n> +\tif (!remote)\n> +\t\tdie(\"Where do you want to list from today?\");\n\nHeh.\n\n> +test_expect_success 'dies with message when no remote specified and no default remote found' '\n> +\n> +\t!(git ls-remote >actual 2>&1) &&\n> +\ttest_cmp exp actual\n\nUse test_must_fail?\n\n> +cat >exp <<EOF\n> +fatal: 'refs*master' does not appear to be a git repository\n> +fatal: The remote end hung up unexpectedly\n> +EOF\n> +test_expect_success 'confuses pattern as remote when no remote specified' '\n> +\t#\n> +\t# Although ugly, this behaviour is akin to the confusion of refspecs for\n> +\t# remotes by git-fetch and git-push, eg:\n> +\t#\n> +\t#   $ git fetch branch\n> +\t#\n> +\n> +\t# We could just as easily have used \"master\"; the \"*\" emphasizes its\n> +\t# role as a pattern.\n> +\t!(git ls-remote refs*master >actual 2>&1) &&\n> +\ttest_cmp exp actual\n> +\n> +'\n\nThis seems like a very odd thing to be testing. Should you not instead\ntest that \"git ls-remote $foo\" still treats $foo as a remote and lists\nit, which is what we really care about?\n\n-Peff\n"},{"id":"138922","messageId":"7vochu8rg7.fsf@alter.siamese.dyndns.org","threadId":"23380","inReplyTo":"1270699083-5424-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-08T05:05:12Z","receivedAt":"2010-04-08T05:05:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tay Ray Chuan <rctay89@gmail.com> writes:\n\n> Instead of breaking execution when no remote (as specified in the\n> variable dest) is specified when git-ls-remote is invoked, continue on\n> and let remote_get() handle it.\n\nShouldn't it default to \"branch.$current.remote\", not \"origin\", though, as\nls-remote to inspect is most closely related to \"git pull\"?\n"},{"id":"138925","messageId":"7v8w8y8q1f.fsf@alter.siamese.dyndns.org","threadId":"23380","inReplyTo":"20100408044552.GA30473@coredump.intra.peff.net","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-08T05:35:40Z","receivedAt":"2010-04-08T05:35:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I don't really see a problem with this. The current behavior produces an\n> error, so it is not as if we are breaking somebody's workflow, and the\n> only sensible default is the same one used by the other commands.\n\nI'd agree only if \"the other commands\" default to the same remote;\notherwise as a plumbing, ls-remote should insist that the user be more\nexplicit.\n\nThe only odd-man out that would worry me is \"git pull\", as I expect\neverybody written in C would just use remote_get(NULL).\n"},{"id":"138927","messageId":"o2qbe6fef0d1004072258h3cab0156o9f63d6bfa732eb87@mail.gmail.com","threadId":"23380","inReplyTo":"7vochu8rg7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T05:58:03Z","receivedAt":"2010-04-08T05:58:03Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Apr 8, 2010 at 1:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Tay Ray Chuan <rctay89@gmail.com> writes:\n>\n>> Instead of breaking execution when no remote (as specified in the\n>> variable dest) is specified when git-ls-remote is invoked, continue on\n>> and let remote_get() handle it.\n>\n> Shouldn't it default to \"branch.$current.remote\", not \"origin\", though, as\n> ls-remote to inspect is most closely related to \"git pull\"?\n\nactually, it already does so (remote_get() does all the magic); it was\nmy commit message that was not accurate.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"138929","messageId":"l2xbe6fef0d1004072307ma8dff5c2ic5dce170b28e5957@mail.gmail.com","threadId":"23380","inReplyTo":"20100408044552.GA30473@coredump.intra.peff.net","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T06:07:11Z","receivedAt":"2010-04-08T06:07:11Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Apr 8, 2010 at 12:45 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Apr 08, 2010 at 11:58:03AM +0800, Tay Ray Chuan wrote:\n>\n>> Instead of breaking execution when no remote (as specified in the\n>> variable dest) is specified when git-ls-remote is invoked, continue on\n>> and let remote_get() handle it.\n>>\n>> That way, we are able to use the default remote (by default, \"origin\"),\n>> as git-fetch, git-push, and others, do.\n>>\n>> While we're at it, die with a more interesting message (\"Where do you\n>> want to...\"), as git-fetch does, instead of the plain usage help.\n>\n> I don't really see a problem with this. The current behavior produces an\n> error, so it is not as if we are breaking somebody's workflow, and the\n> only sensible default is the same one used by the other commands.\n\nI'm trying to make it behave like the other commands that deal with\nremotes, such as git-fetch and git-push; when they are run without any\narguments, they default to \"origin\" or branch.<name>.remote.\n\nAssuming that you and I are talking about the same \"other commands\",\nthan the default isn't an issue; the rules used to determine the\ndefault remote is done in remote_get(), so they are similar.\n\n>> +     if (!remote)\n>> +             die(\"Where do you want to list from today?\");\n>\n> Heh.\n\nDo you think this is too light-hearted for ls-remote's role? If so,\nI'll just revert back exiting with the usage text.\n\n>> +test_expect_success 'dies with message when no remote specified and no default remote found' '\n>> +\n>> +     !(git ls-remote >actual 2>&1) &&\n>> +     test_cmp exp actual\n>\n> Use test_must_fail?\n\nNoted.\n\n>> +cat >exp <<EOF\n>> +fatal: 'refs*master' does not appear to be a git repository\n>> +fatal: The remote end hung up unexpectedly\n>> +EOF\n>> +test_expect_success 'confuses pattern as remote when no remote specified' '\n>> +     #\n>> +     # Although ugly, this behaviour is akin to the confusion of refspecs for\n>> +     # remotes by git-fetch and git-push, eg:\n>> +     #\n>> +     #   $ git fetch branch\n>> +     #\n>> +\n>> +     # We could just as easily have used \"master\"; the \"*\" emphasizes its\n>> +     # role as a pattern.\n>> +     !(git ls-remote refs*master >actual 2>&1) &&\n>> +     test_cmp exp actual\n>> +\n>> +'\n>\n> This seems like a very odd thing to be testing. Should you not instead\n> test that \"git ls-remote $foo\" still treats $foo as a remote and lists\n> it, which is what we really care about?\n\nThere are already existing tests to test \"git ls-remote $foo\" (unless\nyou mean '$' to have a special significance, like '*' does).\n\nThe test was to let current and future git hackers/users know that the\nusage of <pattern> as the remote by git-ls-remote (\"<pattern> does not\nappear to be a git repository\") is indeed expected behaviour.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"138931","messageId":"20100408062539.GC30473@coredump.intra.peff.net","threadId":"23380","inReplyTo":"7v8w8y8q1f.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T06:25:39Z","receivedAt":"2010-04-08T06:25:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 07, 2010 at 10:35:40PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I don't really see a problem with this. The current behavior produces an\n> > error, so it is not as if we are breaking somebody's workflow, and the\n> > only sensible default is the same one used by the other commands.\n> \n> I'd agree only if \"the other commands\" default to the same remote;\n> otherwise as a plumbing, ls-remote should insist that the user be more\n> explicit.\n>\n> The only odd-man out that would worry me is \"git pull\", as I expect\n> everybody written in C would just use remote_get(NULL).\n\nI agree, but I think they do all default to the same remote. Push and\nfetch both use remote_get(NULL). Pull, AFAICT, just calls fetch and\npicks out what to merge from FETCH_HEAD. Am I missing any others?\n\n-Peff\n"},{"id":"138933","messageId":"20100408063422.GD30473@coredump.intra.peff.net","threadId":"23380","inReplyTo":"l2xbe6fef0d1004072307ma8dff5c2ic5dce170b28e5957@mail.gmail.com","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T06:34:22Z","receivedAt":"2010-04-08T06:34:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 08, 2010 at 02:07:11PM +0800, Tay Ray Chuan wrote:\n\n> > I don't really see a problem with this. The current behavior produces an\n> > error, so it is not as if we are breaking somebody's workflow, and the\n> > only sensible default is the same one used by the other commands.\n> \n> I'm trying to make it behave like the other commands that deal with\n> remotes, such as git-fetch and git-push; when they are run without any\n> arguments, they default to \"origin\" or branch.<name>.remote.\n> \n> Assuming that you and I are talking about the same \"other commands\",\n> than the default isn't an issue; the rules used to determine the\n> default remote is done in remote_get(), so they are similar.\n\nThe commands I meant were push, fetch, and pull. I couldn't think of any\nothers.\n\n> >> +     if (!remote)\n> >> +             die(\"Where do you want to list from today?\");\n> >\n> > Heh.\n> \n> Do you think this is too light-hearted for ls-remote's role? If so,\n> I'll just revert back exiting with the usage text.\n\nPerhaps. In general that is not a helpful message to show to the user,\nbut it is very unlikely to be shown at all. You would have to have no\nconfigured remote for your current branch, _and_ you would have had to\ndelete the config for the \"origin\" remote.\n\nFetch's message is equally hard to trigger, I think, which is perhaps\nwhy nobody has complained about it yet.\n\n> > This seems like a very odd thing to be testing. Should you not instead\n> > test that \"git ls-remote $foo\" still treats $foo as a remote and lists\n> > it, which is what we really care about?\n> \n> There are already existing tests to test \"git ls-remote $foo\" (unless\n> you mean '$' to have a special significance, like '*' does).\n\nNo, I meant $foo as a variable, as you interpreted.\n\n> The test was to let current and future git hackers/users know that the\n> usage of <pattern> as the remote by git-ls-remote (\"<pattern> does not\n> appear to be a git repository\") is indeed expected behaviour.\n\nI think documenting that in the commit message would be fine, but I\ndon't have a strong opinion. It seems like the existing \"git ls-remote\n$foo\" tests would cover this by correctly interpreting $foo as a remote.\n\nIf you do keep it, it also should use test_must_fail.\n\nAlso, I note that you are testing for:\n\n  fatal: 'refs*master' does not appear to be a git repository\n  fatal: The remote end hung up unexpectedly\n\nwhich is coming from two separate processes, which means the output may\nhave a race condition. I suspect we're OK because the second message is\ntriggered by closing the fd which must happen after the first message is\nprinted.\n\n-Peff\n"},{"id":"138934","messageId":"7vsk7678av.fsf@alter.siamese.dyndns.org","threadId":"23380","inReplyTo":"20100408063422.GD30473@coredump.intra.peff.net","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-08T06:44:08Z","receivedAt":"2010-04-08T06:44:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Fetch's message is equally hard to trigger, I think, which is perhaps\n> why nobody has complained about it yet.\n\nSee http://thread.gmane.org/gmane.comp.version-control.git/143229/focus=143404\n\n;-)\n"},{"id":"138936","messageId":"20100408064728.GA12941@coredump.intra.peff.net","threadId":"23380","inReplyTo":"7vsk7678av.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] ls-remote: default to 'origin' when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T06:47:28Z","receivedAt":"2010-04-08T06:47:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 07, 2010 at 11:44:08PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Fetch's message is equally hard to trigger, I think, which is perhaps\n> > why nobody has complained about it yet.\n> \n> See http://thread.gmane.org/gmane.comp.version-control.git/143229/focus=143404\n> \n> ;-)\n\nFair enough. :)\n\nIf people are seeing it (both the new one and the old one), perhaps we\nshould lose the easter egg and just change it to \"no default remote\ndefined\" or somesuch. Boring, I know, but probably more helpful.\n\n-Peff\n"},{"id":"138942","messageId":"1270710303-7828-1-git-send-email-rctay89@gmail.com","threadId":"23380","inReplyTo":"1270699083-5424-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2] ls-remote: fall-back to default remotes when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T07:05:03Z","receivedAt":"2010-04-08T07:05:03Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Instead of breaking execution when no remote (as specified in the\nvariable dest) is specified when git-ls-remote is invoked, continue on\nand let remote_get() handle it; if no suitable remote was found, then\nexit with the usage text, as we do previously.\n\nThat way, we are able to use the default remotes (eg. \"origin\",\nbranch.<name>.remote), as git-fetch, git-push, and other users of\nremote_get(), do.\n\nAdd several tests to check that git-ls-remote handles the\nno-remote-specified situation.\n\nAlso add a test that \"git ls-remote <pattern>\" does not work; we are\nunable to guess the remote in that situation, as are git-fetch and\ngit-push.\n\nIn that test, we are testing for messages coming from two separate\nprocesses, but we should be OK, because the second message is triggered\nby closing the fd which must happen after the first message is printed.\n(analysis by Jeff King.)\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/ls-remote.c  |    5 ++---\n t/t5512-ls-remote.sh |   32 ++++++++++++++++++++++++++++++++\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 70f5622..efde78b 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n-\tif (!dest)\n-\t\tusage(ls_remote_usage);\n-\n \tif (argv[i]) {\n \t\tint j;\n \t\tpattern = xcalloc(sizeof(const char *), argc - i + 1);\n@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \tremote = remote_get(dest);\n+\tif (!remote)\n+\t\tusage(ls_remote_usage);\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n \ttransport = transport_get(remote, NULL);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 1dd8eed..34dca0f 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -49,4 +49,36 @@ test_expect_success 'ls-remote self' '\n \n '\n \n+cat >exp <<EOF\n+fatal: Where do you want to list from today?\n+EOF\n+test_expect_success 'dies with message when no remote specified and no default remote found' '\n+\n+\ttest_must_fail git ls-remote >actual 2>&1 &&\n+\ttest_cmp exp actual\n+\n+'\n+\n+test_expect_success 'use \"origin\" when no remote specified' '\n+\n+\tgit remote add origin \"$(pwd)/.git\" &&\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n+test_expect_success 'use branch.<name>.remote if possible' '\n+\n+\t# Remove \"origin\" so that we know that ls-remote is not using it.\n+\t#\n+\t# Ideally, we should test that branch.<name>.remote takes precedence\n+\t# over \"origin\".\n+\t#\n+\tgit remote rm origin &&\n+\tgit config branch.master.remote self &&\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n test_done\n-- \n1.7.0.97.g1372c\n"},{"id":"138944","messageId":"1270710427-6680-1-git-send-email-rctay89@gmail.com","threadId":"23380","inReplyTo":"1270699083-5424-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T07:07:07Z","receivedAt":"2010-04-08T07:07:07Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Instead of breaking execution when no remote (as specified in the\nvariable dest) is specified when git-ls-remote is invoked, continue on\nand let remote_get() handle it; if no suitable remote was found, then\nexit with the usage text, as we do previously.\n\nThat way, we are able to use the default remotes (eg. \"origin\",\nbranch.<name>.remote), as git-fetch, git-push, and other users of\nremote_get(), do.\n\nAdd several tests to check that git-ls-remote handles the\nno-remote-specified situation.\n\nAlso add a test that \"git ls-remote <pattern>\" does not work; we are\nunable to guess the remote in that situation, as are git-fetch and\ngit-push.\n\nIn that test, we are testing for messages coming from two separate\nprocesses, but we should be OK, because the second message is triggered\nby closing the fd which must happen after the first message is printed.\n(analysis by Jeff King.)\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/ls-remote.c  |    5 ++---\n t/t5512-ls-remote.sh |   49 +++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 51 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 70f5622..efde78b 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n-\tif (!dest)\n-\t\tusage(ls_remote_usage);\n-\n \tif (argv[i]) {\n \t\tint j;\n \t\tpattern = xcalloc(sizeof(const char *), argc - i + 1);\n@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \tremote = remote_get(dest);\n+\tif (!remote)\n+\t\tusage(ls_remote_usage);\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n \ttransport = transport_get(remote, NULL);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 1dd8eed..341c79b 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -49,4 +49,53 @@ test_expect_success 'ls-remote self' '\n \n '\n \n+test_expect_success 'dies when no remote specified and no default remotes found' '\n+\n+\ttest_must_fail git ls-remote\n+\n+'\n+\n+test_expect_success 'use \"origin\" when no remote specified' '\n+\n+\tgit remote add origin \"$(pwd)/.git\" &&\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n+test_expect_success 'use branch.<name>.remote if possible' '\n+\n+\t# Remove \"origin\" so that we know that ls-remote is not using it.\n+\t#\n+\t# Ideally, we should test that branch.<name>.remote takes precedence\n+\t# over \"origin\", but that is another matter altogether.\n+\t#\n+\tgit remote rm origin &&\n+\tgit config branch.master.remote self &&\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n+cat >exp <<EOF\n+fatal: 'refs*master' does not appear to be a git repository\n+fatal: The remote end hung up unexpectedly\n+EOF\n+test_expect_success 'confuses pattern as remote when no remote specified' '\n+\t#\n+\t# Do not expect \"git ls-remote <pattern>\" to work; ls-remote, correctly,\n+\t# confuses <pattern> for <remote>. Although ugly, this behaviour is akin\n+\t# to the confusion of refspecs for remotes by git-fetch and git-push,\n+\t# eg:\n+\t#\n+\t#   $ git fetch branch\n+\t#\n+\n+\t# We could just as easily have used \"master\"; the \"*\" emphasizes its\n+\t# role as a pattern.\n+\ttest_must_fail git ls-remote refs*master >actual 2>&1 &&\n+\ttest_cmp exp actual\n+\n+'\n+\n test_done\n-- \n1.7.0.97.g1372c\n"},{"id":"138947","messageId":"20100408071639.GJ30473@coredump.intra.peff.net","threadId":"23380","inReplyTo":"1270710427-6680-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T07:16:39Z","receivedAt":"2010-04-08T07:16:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 08, 2010 at 03:07:07PM +0800, Tay Ray Chuan wrote:\n\n>  \tremote = remote_get(dest);\n> +\tif (!remote)\n> +\t\tusage(ls_remote_usage);\n\nI don't see an update to ls_remote_usage, but shouldn't it now be:\n\n  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n    [repository] [<refs>...]\n\nor something now (while we're at it, maybe we can wrap it better as it's\nlarger than 80 characters).\n\nBut once that is done, shouldn't the (!remote) case say something like\n\"you don't have a default remote\". The user didn't invoke ls-remote\nincorrectly (as the usage message shows), but rather there was a\nconfiguration problem.\n\n> +test_expect_success 'use branch.<name>.remote if possible' '\n> +\n> +\t# Remove \"origin\" so that we know that ls-remote is not using it.\n> +\t#\n> +\t# Ideally, we should test that branch.<name>.remote takes precedence\n> +\t# over \"origin\", but that is another matter altogether.\n> +\t#\n> +\tgit remote rm origin &&\n> +\tgit config branch.master.remote self &&\n> +\tgit ls-remote >actual &&\n> +\ttest_cmp expected.all actual\n> +\n> +'\n\nWouldn't your \"ideally\" just be:\n\n  git clone . other-remote &&\n  git push other-remote HEAD:unique-ref &&\n  git config branch.master.remote other-remote &&\n  ...\n\nand check for \"unique-ref\" in the output?\n\n-Peff\n"},{"id":"138970","messageId":"t2mbe6fef0d1004081010sb6dfd0e1l2e73a36ef1af3870@mail.gmail.com","threadId":"23380","inReplyTo":"20100408071639.GJ30473@coredump.intra.peff.net","subject":"Re: [PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T17:10:09Z","receivedAt":"2010-04-08T17:10:09Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Apr 8, 2010 at 3:16 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Apr 08, 2010 at 03:07:07PM +0800, Tay Ray Chuan wrote:\n>\n>>       remote = remote_get(dest);\n>> +     if (!remote)\n>> +             usage(ls_remote_usage);\n>\n> I don't see an update to ls_remote_usage, but shouldn't it now be:\n>\n>  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n>    [repository] [<refs>...]\n>\n> or something now (while we're at it, maybe we can wrap it better as it's\n> larger than 80 characters).\n\nyes. I think\n\n  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n                [<repository> [<refs>...]]\n\nwould be more accurate.\n\n> But once that is done, shouldn't the (!remote) case say something like\n> \"you don't have a default remote\". The user didn't invoke ls-remote\n> incorrectly (as the usage message shows), but rather there was a\n> configuration problem.\n\nNoted.\n\n>> +test_expect_success 'use branch.<name>.remote if possible' '\n>> +\n>> +     # Remove \"origin\" so that we know that ls-remote is not using it.\n>> +     #\n>> +     # Ideally, we should test that branch.<name>.remote takes precedence\n>> +     # over \"origin\", but that is another matter altogether.\n>> +     #\n>> +     git remote rm origin &&\n>> +     git config branch.master.remote self &&\n>> +     git ls-remote >actual &&\n>> +     test_cmp expected.all actual\n>> +\n>> +'\n>\n> Wouldn't your \"ideally\" just be:\n>\n>  git clone . other-remote &&\n>  git push other-remote HEAD:unique-ref &&\n>  git config branch.master.remote other-remote &&\n>  ...\n>\n> and check for \"unique-ref\" in the output?\n\nhmm, upon clone, tracking branches are already created, so we can just\ntreat them as the \"unique refs\", without having to create a unique\nref.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"138972","messageId":"1270747273-1504-1-git-send-email-rctay89@gmail.com","threadId":"23380","inReplyTo":"1270710427-6680-1-git-send-email-rctay89@gmail.com","subject":"[PATCH v3] ls-remote: fall-back to default remotes when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-08T17:21:13Z","receivedAt":"2010-04-08T17:21:13Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Instead of breaking execution when no remote (as specified in the\nvariable dest) is specified when git-ls-remote is invoked, continue on\nand let remote_get() handle it.\n\nThis way, we are able to use the default remotes (eg. \"origin\",\nbranch.<name>.remote), as git-fetch, git-push, and other users of\nremote_get(), do.\n\nIf no suitable remote is found, exit with a message describing the\nissue, instead of just the usage text, as we do previously.\n\nAdd several tests to check that git-ls-remote handles the\nno-remote-specified situation.\n\nAlso add a test that \"git ls-remote <pattern>\" does not work; we are\nunable to guess the remote in that situation, as are git-fetch and\ngit-push.\n\nIn that test, we are testing for messages coming from two separate\nprocesses, but we should be OK, because the second message is triggered\nby closing the fd which must happen after the first message is printed.\n(analysis by Jeff King.)\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n builtin/ls-remote.c  |   11 ++++++---\n t/t5512-ls-remote.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 65 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 70f5622..6c738c9 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -4,7 +4,8 @@\n #include \"remote.h\"\n \n static const char ls_remote_usage[] =\n-\"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...\";\n+       \"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\\n\"\n+\"                     [<repository> [<refs>...]]\";\n \n /*\n  * Is there one among the list of patterns that match the tail part\n@@ -73,9 +74,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \t}\n \n-\tif (!dest)\n-\t\tusage(ls_remote_usage);\n-\n \tif (argv[i]) {\n \t\tint j;\n \t\tpattern = xcalloc(sizeof(const char *), argc - i + 1);\n@@ -87,6 +85,11 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \tremote = remote_get(dest);\n+\tif (!remote) {\n+\t\tif (dest)\n+\t\t\tdie(\"bad repository '%s'\", dest);\n+\t\tdie(\"No remote configured to list refs from.\");\n+\t}\n \tif (!remote->url_nr)\n \t\tdie(\"remote %s has no configured URL\", dest);\n \ttransport = transport_get(remote, NULL);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 1dd8eed..3cf1b3d 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -49,4 +49,62 @@ test_expect_success 'ls-remote self' '\n \n '\n \n+test_expect_success 'dies when no remote specified and no default remotes found' '\n+\n+\ttest_must_fail git ls-remote\n+\n+'\n+\n+test_expect_success 'use \"origin\" when no remote specified' '\n+\n+\tgit remote add origin \"$(pwd)/.git\" &&\n+\tgit ls-remote >actual &&\n+\ttest_cmp expected.all actual\n+\n+'\n+\n+test_expect_success 'use branch.<name>.remote if possible' '\n+\n+\t#\n+\t# Test that we are indeed using branch.<name>.remote, not \"origin\", even\n+\t# though the \"origin\" remote has been set.\n+\t#\n+\n+\t# setup a new remote to differentiate from \"origin\"\n+\tgit clone . other.git &&\n+\t(\n+\t\tcd other.git &&\n+\t\techo \"$(git rev-parse HEAD)\tHEAD\"\n+\t\tgit show-ref\t| sed -e \"s/ /\t/\"\n+\t) >exp &&\n+\n+\tgit remote add other other.git &&\n+\tgit config branch.master.remote other &&\n+\n+\tgit ls-remote >actual &&\n+\ttest_cmp exp actual\n+\n+'\n+\n+cat >exp <<EOF\n+fatal: 'refs*master' does not appear to be a git repository\n+fatal: The remote end hung up unexpectedly\n+EOF\n+test_expect_success 'confuses pattern as remote when no remote specified' '\n+\t#\n+\t# Do not expect \"git ls-remote <pattern>\" to work; ls-remote, correctly,\n+\t# confuses <pattern> for <remote>. Although ugly, this behaviour is akin\n+\t# to the confusion of refspecs for remotes by git-fetch and git-push,\n+\t# eg:\n+\t#\n+\t#   $ git fetch branch\n+\t#\n+\n+\t# We could just as easily have used \"master\"; the \"*\" emphasizes its\n+\t# role as a pattern.\n+\ttest_must_fail git ls-remote refs*master >actual 2>&1 &&\n+\ttest_cmp exp actual\n+\n+'\n+\n test_done\n-- \n1.7.0.97.g1372c\n"},{"id":"138978","messageId":"20100408191905.GB14066@sigill.intra.peff.net","threadId":"23380","inReplyTo":"1270747273-1504-1-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-08T19:19:06Z","receivedAt":"2010-04-08T19:19:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2010 at 01:21:13AM +0800, Tay Ray Chuan wrote:\n\n> Instead of breaking execution when no remote (as specified in the\n> variable dest) is specified when git-ls-remote is invoked, continue on\n> and let remote_get() handle it.\n\nThis version looks OK to me. Thanks.\n\n-Peff\n"},{"id":"139024","messageId":"A612847CFE53224C91B23E3A5B48BAC7448206860D@xmail3.se.axis.com","threadId":"23380","inReplyTo":"1270747273-1504-1-git-send-email-rctay89@gmail.com","subject":"RE: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified","fromName":"Peter Kjellerstedt","fromEmail":"peter.kjellerstedt@axis.com","sentAt":"2010-04-09T08:49:13Z","receivedAt":"2010-04-09T08:49:13Z","isPatch":true,"sender":{"key":"peter.kjellerstedt@axis.com","avatar":"https://gravatar.com/avatar/6d5a0182283c8eccd7b134a54dbfd5f30038f3ad4d38b96f424884b614a61ca2?d=mp&s=160"},"body":"> -----Original Message-----\n> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n> Behalf Of Tay Ray Chuan\n> Sent: den 8 april 2010 19:21\n> To: Git Mailing List\n> Cc: Junio C Hamano; Jeff King\n> Subject: [PATCH v3] ls-remote: fall-back to default remotes when no\n> remote specified\n> \n> Instead of breaking execution when no remote (as specified in the\n> variable dest) is specified when git-ls-remote is invoked, continue on\n> and let remote_get() handle it.\n> \n> This way, we are able to use the default remotes (eg. \"origin\",\n> branch.<name>.remote), as git-fetch, git-push, and other users of\n> remote_get(), do.\n> \n> If no suitable remote is found, exit with a message describing the\n> issue, instead of just the usage text, as we do previously.\n> \n> Add several tests to check that git-ls-remote handles the\n> no-remote-specified situation.\n> \n> Also add a test that \"git ls-remote <pattern>\" does not work; we are\n> unable to guess the remote in that situation, as are git-fetch and\n> git-push.\n> \n> In that test, we are testing for messages coming from two separate\n> processes, but we should be OK, because the second message is triggered\n> by closing the fd which must happen after the first message is printed.\n> (analysis by Jeff King.)\n> \n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>  builtin/ls-remote.c  |   11 ++++++---\n>  t/t5512-ls-remote.sh |   58\n> ++++++++++++++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 65 insertions(+), 4 deletions(-)\n> \n> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\n> index 70f5622..6c738c9 100644\n> --- a/builtin/ls-remote.c\n> +++ b/builtin/ls-remote.c\n> @@ -4,7 +4,8 @@\n>  #include \"remote.h\"\n> \n>  static const char ls_remote_usage[] =\n> -\"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...\";\n> +       \"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\\n\"\n> +\"                     [<repository> [<refs>...]]\";\n\nThat should be:\n\nstatic const char ls_remote_usage[] =\n\t\"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\\n\"\n\t\"              [<repository> [<refs>...]]\";\n\nor the second line of the usage will not be indented as intended.\n\n//Peter\n"},{"id":"139025","messageId":"g2ube6fef0d1004090215uefd96052w64de7233c26002ae@mail.gmail.com","threadId":"23380","inReplyTo":"A612847CFE53224C91B23E3A5B48BAC7448206860D@xmail3.se.axis.com","subject":"Re: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-04-09T09:15:51Z","receivedAt":"2010-04-09T09:15:51Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Fri, Apr 9, 2010 at 4:49 PM, Peter Kjellerstedt\n<peter.kjellerstedt@axis.com> wrote:\n>> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On\n>> Behalf Of Tay Ray Chuan\n>>  static const char ls_remote_usage[] =\n>> -\"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...\";\n>> +       \"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\\n\"\n>> +\"                     [<repository> [<refs>...]]\";\n>\n> That should be:\n>\n> static const char ls_remote_usage[] =\n>        \"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\\n\"\n>        \"              [<repository> [<refs>...]]\";\n>\n> or the second line of the usage will not be indented as intended.\n\nNope, you are not right - you didn't take into account the \"usage: \"\nstring that is prepended. (The non-printed whitespace in front of \"git\nls-remote...\" is equivalent to that string.)\n\n-- \nCheers,\nRay Chuan\n"}]}