[PATCH] ls-remote: default to 'origin' when no remote specified

[PATCH] ls-remote: default to 'origin' when no remote specified

[Tay Ray Chuan]

Instead of breaking execution when no remote (as specified in the
variable dest) is specified when git-ls-remote is invoked, continue on
and let remote_get() handle it.

That way, we are able to use the default remote (by default, "origin"),
as git-fetch, git-push, and others, do.

While we're at it, die with a more interesting message ("Where do you
want to..."), as git-fetch does, instead of the plain usage help.

Add several tests to check that git-ls-remote handles the
no-remote-specified situation.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin/ls-remote.c  |    5 ++---
 t/t5512-ls-remote.sh |   37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 70f5622..dfada83 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (!dest)
-		usage(ls_remote_usage);
-
 	if (argv[i]) {
 		int j;
 		pattern = xcalloc(sizeof(const char *), argc - i + 1);
@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = remote_get(dest);
+	if (!remote)
+		die("Where do you want to list from today?");
 	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, NULL);
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 1dd8eed..e19429b 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -49,4 +49,41 @@ test_expect_success 'ls-remote self' '
 
 '
 
+cat >exp <<EOF
+fatal: Where do you want to list from today?
+EOF
+test_expect_success 'dies with message when no remote specified and no default remote found' '
+
+	!(git ls-remote >actual 2>&1) &&
+	test_cmp exp actual
+
+'
+
+test_expect_success 'defaults to "origin" when no remote specified' '
+
+	git remote add origin "$(pwd)/.git"
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
+cat >exp <<EOF
+fatal: 'refs*master' does not appear to be a git repository
+fatal: The remote end hung up unexpectedly
+EOF
+test_expect_success 'confuses pattern as remote when no remote specified' '
+	#
+	# Although ugly, this behaviour is akin to the confusion of refspecs for
+	# remotes by git-fetch and git-push, eg:
+	#
+	#   $ git fetch branch
+	#
+
+	# We could just as easily have used "master"; the "*" emphasizes its
+	# role as a pattern.
+	!(git ls-remote refs*master >actual 2>&1) &&
+	test_cmp exp actual
+
+'
+
 test_done
-- 
1.7.0.97.g1372c

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Jeff King]

On Thu, Apr 08, 2010 at 11:58:03AM +0800, Tay Ray Chuan wrote:

> Instead of breaking execution when no remote (as specified in the
> variable dest) is specified when git-ls-remote is invoked, continue on
> and let remote_get() handle it.
> 
> That way, we are able to use the default remote (by default, "origin"),
> as git-fetch, git-push, and others, do.
> 
> While we're at it, die with a more interesting message ("Where do you
> want to..."), as git-fetch does, instead of the plain usage help.

I don't really see a problem with this. The current behavior produces an
error, so it is not as if we are breaking somebody's workflow, and the
only sensible default is the same one used by the other commands.

> +	if (!remote)
> +		die("Where do you want to list from today?");

Heh.

> +test_expect_success 'dies with message when no remote specified and no default remote found' '
> +
> +	!(git ls-remote >actual 2>&1) &&
> +	test_cmp exp actual

Use test_must_fail?

> +cat >exp <<EOF
> +fatal: 'refs*master' does not appear to be a git repository
> +fatal: The remote end hung up unexpectedly
> +EOF
> +test_expect_success 'confuses pattern as remote when no remote specified' '
> +	#
> +	# Although ugly, this behaviour is akin to the confusion of refspecs for
> +	# remotes by git-fetch and git-push, eg:
> +	#
> +	#   $ git fetch branch
> +	#
> +
> +	# We could just as easily have used "master"; the "*" emphasizes its
> +	# role as a pattern.
> +	!(git ls-remote refs*master >actual 2>&1) &&
> +	test_cmp exp actual
> +
> +'

This seems like a very odd thing to be testing. Should you not instead
test that "git ls-remote $foo" still treats $foo as a remote and lists
it, which is what we really care about?

-Peff

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Junio C Hamano]

Tay Ray Chuan <rctay89@gmail.com> writes:

> Instead of breaking execution when no remote (as specified in the
> variable dest) is specified when git-ls-remote is invoked, continue on
> and let remote_get() handle it.

Shouldn't it default to "branch.$current.remote", not "origin", though, as
ls-remote to inspect is most closely related to "git pull"?

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I don't really see a problem with this. The current behavior produces an
> error, so it is not as if we are breaking somebody's workflow, and the
> only sensible default is the same one used by the other commands.

I'd agree only if "the other commands" default to the same remote;
otherwise as a plumbing, ls-remote should insist that the user be more
explicit.

The only odd-man out that would worry me is "git pull", as I expect
everybody written in C would just use remote_get(NULL).

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Tay Ray Chuan]

Hi,

On Thu, Apr 8, 2010 at 1:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tay Ray Chuan <rctay89@gmail.com> writes:
>
>> Instead of breaking execution when no remote (as specified in the
>> variable dest) is specified when git-ls-remote is invoked, continue on
>> and let remote_get() handle it.
>
> Shouldn't it default to "branch.$current.remote", not "origin", though, as
> ls-remote to inspect is most closely related to "git pull"?

actually, it already does so (remote_get() does all the magic); it was
my commit message that was not accurate.

-- 
Cheers,
Ray Chuan

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Tay Ray Chuan]

Hi,

On Thu, Apr 8, 2010 at 12:45 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 08, 2010 at 11:58:03AM +0800, Tay Ray Chuan wrote:
>
>> Instead of breaking execution when no remote (as specified in the
>> variable dest) is specified when git-ls-remote is invoked, continue on
>> and let remote_get() handle it.
>>
>> That way, we are able to use the default remote (by default, "origin"),
>> as git-fetch, git-push, and others, do.
>>
>> While we're at it, die with a more interesting message ("Where do you
>> want to..."), as git-fetch does, instead of the plain usage help.
>
> I don't really see a problem with this. The current behavior produces an
> error, so it is not as if we are breaking somebody's workflow, and the
> only sensible default is the same one used by the other commands.

I'm trying to make it behave like the other commands that deal with
remotes, such as git-fetch and git-push; when they are run without any
arguments, they default to "origin" or branch.<name>.remote.

Assuming that you and I are talking about the same "other commands",
than the default isn't an issue; the rules used to determine the
default remote is done in remote_get(), so they are similar.

>> +     if (!remote)
>> +             die("Where do you want to list from today?");
>
> Heh.

Do you think this is too light-hearted for ls-remote's role? If so,
I'll just revert back exiting with the usage text.

>> +test_expect_success 'dies with message when no remote specified and no default remote found' '
>> +
>> +     !(git ls-remote >actual 2>&1) &&
>> +     test_cmp exp actual
>
> Use test_must_fail?

Noted.

>> +cat >exp <<EOF
>> +fatal: 'refs*master' does not appear to be a git repository
>> +fatal: The remote end hung up unexpectedly
>> +EOF
>> +test_expect_success 'confuses pattern as remote when no remote specified' '
>> +     #
>> +     # Although ugly, this behaviour is akin to the confusion of refspecs for
>> +     # remotes by git-fetch and git-push, eg:
>> +     #
>> +     #   $ git fetch branch
>> +     #
>> +
>> +     # We could just as easily have used "master"; the "*" emphasizes its
>> +     # role as a pattern.
>> +     !(git ls-remote refs*master >actual 2>&1) &&
>> +     test_cmp exp actual
>> +
>> +'
>
> This seems like a very odd thing to be testing. Should you not instead
> test that "git ls-remote $foo" still treats $foo as a remote and lists
> it, which is what we really care about?

There are already existing tests to test "git ls-remote $foo" (unless
you mean '$' to have a special significance, like '*' does).

The test was to let current and future git hackers/users know that the
usage of <pattern> as the remote by git-ls-remote ("<pattern> does not
appear to be a git repository") is indeed expected behaviour.

-- 
Cheers,
Ray Chuan

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Jeff King]

On Wed, Apr 07, 2010 at 10:35:40PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I don't really see a problem with this. The current behavior produces an
> > error, so it is not as if we are breaking somebody's workflow, and the
> > only sensible default is the same one used by the other commands.
> 
> I'd agree only if "the other commands" default to the same remote;
> otherwise as a plumbing, ls-remote should insist that the user be more
> explicit.
>
> The only odd-man out that would worry me is "git pull", as I expect
> everybody written in C would just use remote_get(NULL).

I agree, but I think they do all default to the same remote. Push and
fetch both use remote_get(NULL). Pull, AFAICT, just calls fetch and
picks out what to merge from FETCH_HEAD. Am I missing any others?

-Peff

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Jeff King]

On Thu, Apr 08, 2010 at 02:07:11PM +0800, Tay Ray Chuan wrote:

> > I don't really see a problem with this. The current behavior produces an
> > error, so it is not as if we are breaking somebody's workflow, and the
> > only sensible default is the same one used by the other commands.
> 
> I'm trying to make it behave like the other commands that deal with
> remotes, such as git-fetch and git-push; when they are run without any
> arguments, they default to "origin" or branch.<name>.remote.
> 
> Assuming that you and I are talking about the same "other commands",
> than the default isn't an issue; the rules used to determine the
> default remote is done in remote_get(), so they are similar.

The commands I meant were push, fetch, and pull. I couldn't think of any
others.

> >> +     if (!remote)
> >> +             die("Where do you want to list from today?");
> >
> > Heh.
> 
> Do you think this is too light-hearted for ls-remote's role? If so,
> I'll just revert back exiting with the usage text.

Perhaps. In general that is not a helpful message to show to the user,
but it is very unlikely to be shown at all. You would have to have no
configured remote for your current branch, _and_ you would have had to
delete the config for the "origin" remote.

Fetch's message is equally hard to trigger, I think, which is perhaps
why nobody has complained about it yet.

> > This seems like a very odd thing to be testing. Should you not instead
> > test that "git ls-remote $foo" still treats $foo as a remote and lists
> > it, which is what we really care about?
> 
> There are already existing tests to test "git ls-remote $foo" (unless
> you mean '$' to have a special significance, like '*' does).

No, I meant $foo as a variable, as you interpreted.

> The test was to let current and future git hackers/users know that the
> usage of <pattern> as the remote by git-ls-remote ("<pattern> does not
> appear to be a git repository") is indeed expected behaviour.

I think documenting that in the commit message would be fine, but I
don't have a strong opinion. It seems like the existing "git ls-remote
$foo" tests would cover this by correctly interpreting $foo as a remote.

If you do keep it, it also should use test_must_fail.

Also, I note that you are testing for:

  fatal: 'refs*master' does not appear to be a git repository
  fatal: The remote end hung up unexpectedly

which is coming from two separate processes, which means the output may
have a race condition. I suspect we're OK because the second message is
triggered by closing the fd which must happen after the first message is
printed.

-Peff

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Fetch's message is equally hard to trigger, I think, which is perhaps
> why nobody has complained about it yet.

See http://thread.gmane.org/gmane.comp.version-control.git/143229/focus=143404

;-)

Re: [PATCH] ls-remote: default to 'origin' when no remote specified

[Jeff King]

On Wed, Apr 07, 2010 at 11:44:08PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Fetch's message is equally hard to trigger, I think, which is perhaps
> > why nobody has complained about it yet.
> 
> See http://thread.gmane.org/gmane.comp.version-control.git/143229/focus=143404
> 
> ;-)

Fair enough. :)

If people are seeing it (both the new one and the old one), perhaps we
should lose the easter egg and just change it to "no default remote
defined" or somesuch. Boring, I know, but probably more helpful.

-Peff

[PATCH v2] ls-remote: fall-back to default remotes when no remote specified

[Tay Ray Chuan]

Instead of breaking execution when no remote (as specified in the
variable dest) is specified when git-ls-remote is invoked, continue on
and let remote_get() handle it; if no suitable remote was found, then
exit with the usage text, as we do previously.

That way, we are able to use the default remotes (eg. "origin",
branch.<name>.remote), as git-fetch, git-push, and other users of
remote_get(), do.

Add several tests to check that git-ls-remote handles the
no-remote-specified situation.

Also add a test that "git ls-remote <pattern>" does not work; we are
unable to guess the remote in that situation, as are git-fetch and
git-push.

In that test, we are testing for messages coming from two separate
processes, but we should be OK, because the second message is triggered
by closing the fd which must happen after the first message is printed.
(analysis by Jeff King.)

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin/ls-remote.c  |    5 ++---
 t/t5512-ls-remote.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 70f5622..efde78b 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (!dest)
-		usage(ls_remote_usage);
-
 	if (argv[i]) {
 		int j;
 		pattern = xcalloc(sizeof(const char *), argc - i + 1);
@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = remote_get(dest);
+	if (!remote)
+		usage(ls_remote_usage);
 	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, NULL);
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 1dd8eed..34dca0f 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -49,4 +49,36 @@ test_expect_success 'ls-remote self' '
 
 '
 
+cat >exp <<EOF
+fatal: Where do you want to list from today?
+EOF
+test_expect_success 'dies with message when no remote specified and no default remote found' '
+
+	test_must_fail git ls-remote >actual 2>&1 &&
+	test_cmp exp actual
+
+'
+
+test_expect_success 'use "origin" when no remote specified' '
+
+	git remote add origin "$(pwd)/.git" &&
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
+test_expect_success 'use branch.<name>.remote if possible' '
+
+	# Remove "origin" so that we know that ls-remote is not using it.
+	#
+	# Ideally, we should test that branch.<name>.remote takes precedence
+	# over "origin".
+	#
+	git remote rm origin &&
+	git config branch.master.remote self &&
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
 test_done
-- 
1.7.0.97.g1372c

[PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified

[Tay Ray Chuan]

Instead of breaking execution when no remote (as specified in the
variable dest) is specified when git-ls-remote is invoked, continue on
and let remote_get() handle it; if no suitable remote was found, then
exit with the usage text, as we do previously.

That way, we are able to use the default remotes (eg. "origin",
branch.<name>.remote), as git-fetch, git-push, and other users of
remote_get(), do.

Add several tests to check that git-ls-remote handles the
no-remote-specified situation.

Also add a test that "git ls-remote <pattern>" does not work; we are
unable to guess the remote in that situation, as are git-fetch and
git-push.

In that test, we are testing for messages coming from two separate
processes, but we should be OK, because the second message is triggered
by closing the fd which must happen after the first message is printed.
(analysis by Jeff King.)

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin/ls-remote.c  |    5 ++---
 t/t5512-ls-remote.sh |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 70f5622..efde78b 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -73,9 +73,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (!dest)
-		usage(ls_remote_usage);
-
 	if (argv[i]) {
 		int j;
 		pattern = xcalloc(sizeof(const char *), argc - i + 1);
@@ -87,6 +84,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = remote_get(dest);
+	if (!remote)
+		usage(ls_remote_usage);
 	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, NULL);
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 1dd8eed..341c79b 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -49,4 +49,53 @@ test_expect_success 'ls-remote self' '
 
 '
 
+test_expect_success 'dies when no remote specified and no default remotes found' '
+
+	test_must_fail git ls-remote
+
+'
+
+test_expect_success 'use "origin" when no remote specified' '
+
+	git remote add origin "$(pwd)/.git" &&
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
+test_expect_success 'use branch.<name>.remote if possible' '
+
+	# Remove "origin" so that we know that ls-remote is not using it.
+	#
+	# Ideally, we should test that branch.<name>.remote takes precedence
+	# over "origin", but that is another matter altogether.
+	#
+	git remote rm origin &&
+	git config branch.master.remote self &&
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
+cat >exp <<EOF
+fatal: 'refs*master' does not appear to be a git repository
+fatal: The remote end hung up unexpectedly
+EOF
+test_expect_success 'confuses pattern as remote when no remote specified' '
+	#
+	# Do not expect "git ls-remote <pattern>" to work; ls-remote, correctly,
+	# confuses <pattern> for <remote>. Although ugly, this behaviour is akin
+	# to the confusion of refspecs for remotes by git-fetch and git-push,
+	# eg:
+	#
+	#   $ git fetch branch
+	#
+
+	# We could just as easily have used "master"; the "*" emphasizes its
+	# role as a pattern.
+	test_must_fail git ls-remote refs*master >actual 2>&1 &&
+	test_cmp exp actual
+
+'
+
 test_done
-- 
1.7.0.97.g1372c

Re: [PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified

[Jeff King]

On Thu, Apr 08, 2010 at 03:07:07PM +0800, Tay Ray Chuan wrote:

>  	remote = remote_get(dest);
> +	if (!remote)
> +		usage(ls_remote_usage);

I don't see an update to ls_remote_usage, but shouldn't it now be:

  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
    [repository] [<refs>...]

or something now (while we're at it, maybe we can wrap it better as it's
larger than 80 characters).

But once that is done, shouldn't the (!remote) case say something like
"you don't have a default remote". The user didn't invoke ls-remote
incorrectly (as the usage message shows), but rather there was a
configuration problem.

> +test_expect_success 'use branch.<name>.remote if possible' '
> +
> +	# Remove "origin" so that we know that ls-remote is not using it.
> +	#
> +	# Ideally, we should test that branch.<name>.remote takes precedence
> +	# over "origin", but that is another matter altogether.
> +	#
> +	git remote rm origin &&
> +	git config branch.master.remote self &&
> +	git ls-remote >actual &&
> +	test_cmp expected.all actual
> +
> +'

Wouldn't your "ideally" just be:

  git clone . other-remote &&
  git push other-remote HEAD:unique-ref &&
  git config branch.master.remote other-remote &&
  ...

and check for "unique-ref" in the output?

-Peff

Re: [PATCH v2 (resend)] ls-remote: fall-back to default remotes when no remote specified

[Tay Ray Chuan]

Hi,

On Thu, Apr 8, 2010 at 3:16 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Apr 08, 2010 at 03:07:07PM +0800, Tay Ray Chuan wrote:
>
>>       remote = remote_get(dest);
>> +     if (!remote)
>> +             usage(ls_remote_usage);
>
> I don't see an update to ls_remote_usage, but shouldn't it now be:
>
>  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
>    [repository] [<refs>...]
>
> or something now (while we're at it, maybe we can wrap it better as it's
> larger than 80 characters).

yes. I think

  git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
                [<repository> [<refs>...]]

would be more accurate.

> But once that is done, shouldn't the (!remote) case say something like
> "you don't have a default remote". The user didn't invoke ls-remote
> incorrectly (as the usage message shows), but rather there was a
> configuration problem.

Noted.

>> +test_expect_success 'use branch.<name>.remote if possible' '
>> +
>> +     # Remove "origin" so that we know that ls-remote is not using it.
>> +     #
>> +     # Ideally, we should test that branch.<name>.remote takes precedence
>> +     # over "origin", but that is another matter altogether.
>> +     #
>> +     git remote rm origin &&
>> +     git config branch.master.remote self &&
>> +     git ls-remote >actual &&
>> +     test_cmp expected.all actual
>> +
>> +'
>
> Wouldn't your "ideally" just be:
>
>  git clone . other-remote &&
>  git push other-remote HEAD:unique-ref &&
>  git config branch.master.remote other-remote &&
>  ...
>
> and check for "unique-ref" in the output?

hmm, upon clone, tracking branches are already created, so we can just
treat them as the "unique refs", without having to create a unique
ref.

-- 
Cheers,
Ray Chuan

[PATCH v3] ls-remote: fall-back to default remotes when no remote specified

[Tay Ray Chuan]

Instead of breaking execution when no remote (as specified in the
variable dest) is specified when git-ls-remote is invoked, continue on
and let remote_get() handle it.

This way, we are able to use the default remotes (eg. "origin",
branch.<name>.remote), as git-fetch, git-push, and other users of
remote_get(), do.

If no suitable remote is found, exit with a message describing the
issue, instead of just the usage text, as we do previously.

Add several tests to check that git-ls-remote handles the
no-remote-specified situation.

Also add a test that "git ls-remote <pattern>" does not work; we are
unable to guess the remote in that situation, as are git-fetch and
git-push.

In that test, we are testing for messages coming from two separate
processes, but we should be OK, because the second message is triggered
by closing the fd which must happen after the first message is printed.
(analysis by Jeff King.)

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
 builtin/ls-remote.c  |   11 ++++++---
 t/t5512-ls-remote.sh |   58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 70f5622..6c738c9 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -4,7 +4,8 @@
 #include "remote.h"
 
 static const char ls_remote_usage[] =
-"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...";
+       "git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
+"                     [<repository> [<refs>...]]";
 
 /*
  * Is there one among the list of patterns that match the tail part
@@ -73,9 +74,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		break;
 	}
 
-	if (!dest)
-		usage(ls_remote_usage);
-
 	if (argv[i]) {
 		int j;
 		pattern = xcalloc(sizeof(const char *), argc - i + 1);
@@ -87,6 +85,11 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		}
 	}
 	remote = remote_get(dest);
+	if (!remote) {
+		if (dest)
+			die("bad repository '%s'", dest);
+		die("No remote configured to list refs from.");
+	}
 	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
 	transport = transport_get(remote, NULL);
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
index 1dd8eed..3cf1b3d 100755
--- a/t/t5512-ls-remote.sh
+++ b/t/t5512-ls-remote.sh
@@ -49,4 +49,62 @@ test_expect_success 'ls-remote self' '
 
 '
 
+test_expect_success 'dies when no remote specified and no default remotes found' '
+
+	test_must_fail git ls-remote
+
+'
+
+test_expect_success 'use "origin" when no remote specified' '
+
+	git remote add origin "$(pwd)/.git" &&
+	git ls-remote >actual &&
+	test_cmp expected.all actual
+
+'
+
+test_expect_success 'use branch.<name>.remote if possible' '
+
+	#
+	# Test that we are indeed using branch.<name>.remote, not "origin", even
+	# though the "origin" remote has been set.
+	#
+
+	# setup a new remote to differentiate from "origin"
+	git clone . other.git &&
+	(
+		cd other.git &&
+		echo "$(git rev-parse HEAD)	HEAD"
+		git show-ref	| sed -e "s/ /	/"
+	) >exp &&
+
+	git remote add other other.git &&
+	git config branch.master.remote other &&
+
+	git ls-remote >actual &&
+	test_cmp exp actual
+
+'
+
+cat >exp <<EOF
+fatal: 'refs*master' does not appear to be a git repository
+fatal: The remote end hung up unexpectedly
+EOF
+test_expect_success 'confuses pattern as remote when no remote specified' '
+	#
+	# Do not expect "git ls-remote <pattern>" to work; ls-remote, correctly,
+	# confuses <pattern> for <remote>. Although ugly, this behaviour is akin
+	# to the confusion of refspecs for remotes by git-fetch and git-push,
+	# eg:
+	#
+	#   $ git fetch branch
+	#
+
+	# We could just as easily have used "master"; the "*" emphasizes its
+	# role as a pattern.
+	test_must_fail git ls-remote refs*master >actual 2>&1 &&
+	test_cmp exp actual
+
+'
+
 test_done
-- 
1.7.0.97.g1372c

Re: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified

[Jeff King]

On Fri, Apr 09, 2010 at 01:21:13AM +0800, Tay Ray Chuan wrote:

> Instead of breaking execution when no remote (as specified in the
> variable dest) is specified when git-ls-remote is invoked, continue on
> and let remote_get() handle it.

This version looks OK to me. Thanks.

-Peff

RE: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified

[Peter Kjellerstedt]

> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of Tay Ray Chuan
> Sent: den 8 april 2010 19:21
> To: Git Mailing List
> Cc: Junio C Hamano; Jeff King
> Subject: [PATCH v3] ls-remote: fall-back to default remotes when no
> remote specified
> 
> Instead of breaking execution when no remote (as specified in the
> variable dest) is specified when git-ls-remote is invoked, continue on
> and let remote_get() handle it.
> 
> This way, we are able to use the default remotes (eg. "origin",
> branch.<name>.remote), as git-fetch, git-push, and other users of
> remote_get(), do.
> 
> If no suitable remote is found, exit with a message describing the
> issue, instead of just the usage text, as we do previously.
> 
> Add several tests to check that git-ls-remote handles the
> no-remote-specified situation.
> 
> Also add a test that "git ls-remote <pattern>" does not work; we are
> unable to guess the remote in that situation, as are git-fetch and
> git-push.
> 
> In that test, we are testing for messages coming from two separate
> processes, but we should be OK, because the second message is triggered
> by closing the fd which must happen after the first message is printed.
> (analysis by Jeff King.)
> 
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>  builtin/ls-remote.c  |   11 ++++++---
>  t/t5512-ls-remote.sh |   58
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 70f5622..6c738c9 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -4,7 +4,8 @@
>  #include "remote.h"
> 
>  static const char ls_remote_usage[] =
> -"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...";
> +       "git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
> +"                     [<repository> [<refs>...]]";

That should be:

static const char ls_remote_usage[] =
	"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
	"              [<repository> [<refs>...]]";

or the second line of the usage will not be indented as intended.

//Peter

Re: [PATCH v3] ls-remote: fall-back to default remotes when no remote specified

[Tay Ray Chuan]

Hi,

On Fri, Apr 9, 2010 at 4:49 PM, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
>> Behalf Of Tay Ray Chuan
>>  static const char ls_remote_usage[] =
>> -"git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>] <repository> <refs>...";
>> +       "git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
>> +"                     [<repository> [<refs>...]]";
>
> That should be:
>
> static const char ls_remote_usage[] =
>        "git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
>        "              [<repository> [<refs>...]]";
>
> or the second line of the usage will not be indented as intended.

Nope, you are not right - you didn't take into account the "usage: "
string that is prepended. (The non-printed whitespace in front of "git
ls-remote..." is equivalent to that string.)

-- 
Cheers,
Ray Chuan