[PATCH] add ls-remote --get-push-url option

[PATCH] add ls-remote --get-push-url option

[Ben Boeckel]

Not sure if it would be better to make a new variable or to reuse the existing
one. I'm reusing it currently because it makes it easier to ensure they
are mutually exclusive.

Please keep me CC'd to the list; I'm not subscribed.

Thanks,

--Ben

Ben Boeckel (1):
  ls-remote: add --get-push-url option

 Documentation/git-ls-remote.txt |  7 ++++++-
 builtin/ls-remote.c             | 15 +++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.1.0

[PATCH] ls-remote: add --get-push-url option

[Ben Boeckel]

With pushInsteadOf and triangle workflows, a flag to have git fully
expand the push URL is also useful since it can be very different from
the --get-url output.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-ls-remote.txt |  7 ++++++-
 builtin/ls-remote.c             | 15 +++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 2e22915..66cda0e 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git ls-remote' [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
-	      [--exit-code] <repository> [<refs>...]
+	      [--exit-code] [--get-url | --get-push-url] <repository> [<refs>...]
 
 DESCRIPTION
 -----------
@@ -47,6 +47,11 @@ OPTIONS
 	"url.<base>.insteadOf" config setting (See linkgit:git-config[1]) and
 	exit without talking to the remote.
 
+--get-push-url::
+	Expand the URL of the given remote repository taking into account any
+	"url.<base>.pushInsteadOf" config setting (See linkgit:git-config[1])
+	and exit without talking to the remote.
+
 <repository>::
 	The "remote" repository to query.  This parameter can be
 	either a URL or the name of a remote (see the GIT URLS and
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 4554dbc..25c6f79 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,7 @@
 
 static const char ls_remote_usage[] =
 "git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]\n"
-"                     [-q | --quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]";
+"                     [-q | --quiet] [--exit-code] [--get-url | --get-push-url] [<repository> [<refs>...]]";
 
 /*
  * Is there one among the list of patterns that match the tail part
@@ -40,6 +40,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	const char **pattern = NULL;
 
 	struct remote *remote;
+	struct remote *pushremote;
 	struct transport *transport;
 	const struct ref *ref;
 
@@ -78,6 +79,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 				get_url = 1;
 				continue;
 			}
+			if (!strcmp("--get-push-url", arg)) {
+				get_url = 2;
+				continue;
+			}
 			if (!strcmp("--exit-code", arg)) {
 				/* return this code if no refs are reported */
 				status = 2;
@@ -109,9 +114,15 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (!remote->url_nr)
 		die("remote %s has no configured URL", dest);
 
-	if (get_url) {
+	if (get_url == 1) {
 		printf("%s\n", *remote->url);
 		return 0;
+	} else if (get_url == 2) {
+		pushremote = pushremote_get(dest);
+		if (!pushremote->url_nr)
+			die("remote %s has no configured push URL", dest);
+		printf("%s\n", *pushremote->url);
+		return 0;
 	}
 
 	transport = transport_get(remote, NULL);
-- 
2.1.0

Re: [PATCH] add ls-remote --get-push-url option

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> Not sure if it would be better to make a new variable or to reuse the existing
> one. I'm reusing it currently because it makes it easier to ensure they
> are mutually exclusive.
>
> Please keep me CC'd to the list; I'm not subscribed.
>
> Thanks,
>
> --Ben
>
> Ben Boeckel (1):
>   ls-remote: add --get-push-url option
>
>  Documentation/git-ls-remote.txt |  7 ++++++-
>  builtin/ls-remote.c             | 15 +++++++++++++--
>  2 files changed, 19 insertions(+), 3 deletions(-)

Probably get-url makes (some) sense because ls-remote is a "fetch
that does not actually fetch anything".  But "get-push-url" to
ls-remote makes _no_ sense whatsoever.  ls-remote and fetch do not
have to know or care about push-url; they do not even have to know
there exists a thing called "git push" ;-)

Wouldn't "git push -v -n" or something suit your needs already?

Re: [PATCH] add ls-remote --get-push-url option

[Ben Boeckel]

On Fri, Jul 31, 2015 at 11:40:12 -0700, Junio C Hamano wrote:
> Probably get-url makes (some) sense because ls-remote is a "fetch
> that does not actually fetch anything".  But "get-push-url" to
> ls-remote makes _no_ sense whatsoever.  ls-remote and fetch do not
> have to know or care about push-url; they do not even have to know
> there exists a thing called "git push" ;-)
> 
> Wouldn't "git push -v -n" or something suit your needs already?

With some sed, yes, but then so would `git remote show` just as useful
too (and in that case, "why does --get-url exist either?" comes to
mind).

Our use case is that we have some scripts which setup the project with
the right remotes and such. To do this, we detect if your remotes are
set up properly already and not ask if things are OK already. This is
currently done with git config --get remote.$x.pushurl, but
`pushInsteadOf` is not expanded and causes false positives.

Where would a utility to have git expand its `pushInsteadOf` aliases
make more sense? Being right beside `insteadOf` expansion made sense to
me (certainly more than some locations for certain flags and actions,
but that boat sailed long ago :) ).

--Ben

Re: [PATCH] add ls-remote --get-push-url option

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> With some sed, yes, but then so would `git remote show` just as useful
> too (and in that case, "why does --get-url exist either?" comes to
> mind).

Either carelessness let it slip in, or it came before 'git remote show'.

Re: [PATCH] add ls-remote --get-push-url option

[Ben Boeckel]

On Fri, Jul 31, 2015 at 12:02:14 -0700, Junio C Hamano wrote:
> Ben Boeckel <mathstuf@gmail.com> writes:
> 
> > With some sed, yes, but then so would `git remote show` just as useful
> > too (and in that case, "why does --get-url exist either?" comes to
> > mind).
> 
> Either carelessness let it slip in, or it came before 'git remote show'.

Would adding `git remote show --url $remote` and `git remote show
--push-url $remote` be acceptable?

--Ben

Re: [PATCH] add ls-remote --get-push-url option

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> On Fri, Jul 31, 2015 at 12:02:14 -0700, Junio C Hamano wrote:
>> Ben Boeckel <mathstuf@gmail.com> writes:
>> 
>> > With some sed, yes, but then so would `git remote show` just as useful
>> > too (and in that case, "why does --get-url exist either?" comes to
>> > mind).
>> 
>> Either carelessness let it slip in, or it came before 'git remote show'.
>
> Would adding `git remote show --url $remote` and `git remote show
> --push-url $remote` be acceptable?

It is not just acceptable; I think "git remote" is a much better
place to have something like that.

Or even "git remote get url [$there]", "git remote get push-url [$there]".

Or to mirror the existing "ls-remote --get-url [$there]", which directly asks
"where does this request go if I run it without '--get-url' option?":

    $ git push --get-url [$there [$refspec...]]
    $ git push --get-refspec [$there [$refspec...]]

might be a better option.  The logic in "push" takes the current
branch and configurations like branch.*.remote and push.default into
account, so it is likely that you will get the exact information
without too much code.

I am not opposed to having a scriptable interface to obtain these
pieces of information.  I was only objecting to teach ls-remote
anything about push, which ls-remote does not have anything to do
with.

Re: [PATCH] add ls-remote --get-push-url option

[Ben Boeckel]

On Fri, Jul 31, 2015 at 12:16:46 -0700, Junio C Hamano wrote:
> Or even "git remote get url [$there]", "git remote get push-url [$there]".

Looking at `git remote`'s existing subcommands, consistency there would
be something like:

    git remote get-url $there
    git remote get-url --push $there

The new call could also show *all* remotes which will be pushed to, not
just the first. Looking at the structure, it also has fields for
multiple fetch urls. I would assume git would fetch the union of refs;
not sure about conflicts and such.

> Or to mirror the existing "ls-remote --get-url [$there]", which directly asks
> "where does this request go if I run it without '--get-url' option?":
> 
>     $ git push --get-url [$there [$refspec...]]
>     $ git push --get-refspec [$there [$refspec...]]
> 
> might be a better option.  The logic in "push" takes the current
> branch and configurations like branch.*.remote and push.default into
> account, so it is likely that you will get the exact information
> without too much code.

This is a little weird to me. `git push` is porcelain; `git ls-remote`
feels more like plumbing (command-list.txt seems to agree). Maybe it is
also useful since it takes these other configuration values into
account. But maybe pushing has enough extra factors? Might it make sense
in addition to a `git remote get-url`?

> I am not opposed to having a scriptable interface to obtain these
> pieces of information.  I was only objecting to teach ls-remote
> anything about push, which ls-remote does not have anything to do
> with.

OK. Thanks.

--Ben

[PATCH v2] add git-url subcommand

[Ben Boeckel]

Implements a get-url subcommand to git-remote which allows for querying the
URLs for a remote while expanding insteadOf and pushInsteadOf.

--Ben

Ben Boeckel (1):
  remote: add get-url subcommand

 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 55 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

-- 
2.1.0

[PATCH v2] remote: add get-url subcommand

[Ben Boeckel]

Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 55 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..c47b634 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
+'git remote get-url' [--push] [--all] <name>
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried instead of fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote <name> that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index f4a6ec9..9278a83 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
+	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
 	N_("git remote set-url --delete <name> <url>"),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
 	NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+	N_("git remote get-url [--push] [--all] <name>"),
+	NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
@@ -1497,6 +1503,53 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+	int i, push_mode = 0, all_mode = 0;
+	const char *remotename = NULL;
+	struct remote *remote;
+	const char **url;
+	int url_nr;
+	struct option options[] = {
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("query push URLs")),
+		OPT_BOOL('\0', "all", &all_mode,
+			 N_("return all URLs")),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage,
+			     PARSE_OPT_KEEP_ARGV0);
+
+	if (argc < 1 || argc > 2)
+		usage_with_options(builtin_remote_geturl_usage, options);
+
+	remotename = argv[1];
+
+	if (!remote_is_configured(remotename))
+		die(_("No such remote '%s'"), remotename);
+	remote = remote_get(remotename);
+
+	if (push_mode) {
+		url = remote->pushurl;
+		url_nr = remote->pushurl_nr;
+	} else {
+		url = remote->url;
+		url_nr = remote->url_nr;
+	}
+
+	if (!url_nr)
+		die(_("no URLs configured for remote '%s'"), remotename);
+
+	if (all_mode) {
+		for (i = 0; i < url_nr; i++)
+			printf_ln("%s", url[i]);
+	} else {
+		printf_ln("%s", *url);
+	}
+
+	return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
 	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1606,6 +1659,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "set-branches"))
 		result = set_branches(argc, argv);
+	else if (!strcmp(argv[0], "get-url"))
+		result = get_url(argc, argv);
 	else if (!strcmp(argv[0], "set-url"))
 		result = set_url(argc, argv);
 	else if (!strcmp(argv[0], "show"))
-- 
2.1.0

Re: [PATCH v2] remote: add get-url subcommand

[Eric Sunshine]

On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
> Expanding `insteadOf` is a part of ls-remote --url and there is no way
> to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
> to query both as well as a way to get all configured urls.
>
> Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
> ---
> diff --git a/builtin/remote.c b/builtin/remote.c
> index f4a6ec9..9278a83 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -1497,6 +1503,53 @@ static int set_branches(int argc, const char **argv)
> +static int get_url(int argc, const char **argv)
> +{
> +       int i, push_mode = 0, all_mode = 0;
> +       const char *remotename = NULL;
> +       struct remote *remote;
> +       const char **url;
> +       int url_nr;
> +       struct option options[] = {
> +               OPT_BOOL('\0', "push", &push_mode,
> +                        N_("query push URLs")),

A bit more explanatory:

    "query push URLs rather than fetch URLs"

> +               OPT_BOOL('\0', "all", &all_mode,
> +                        N_("return all URLs")),
> +               OPT_END()
> +       };
> +       argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage,
> +                            PARSE_OPT_KEEP_ARGV0);

What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?

> +       if (argc < 1 || argc > 2)
> +               usage_with_options(builtin_remote_geturl_usage, options);

So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).

> +       remotename = argv[1];

But here, argv[1] is accessed unconditionally, even though 'argc' may
have been 1, thus out of bounds.

> +       if (!remote_is_configured(remotename))
> +               die(_("No such remote '%s'"), remotename);
> +       remote = remote_get(remotename);
> +
> +       if (push_mode) {
> +               url = remote->pushurl;
> +               url_nr = remote->pushurl_nr;
> +       } else {
> +               url = remote->url;
> +               url_nr = remote->url_nr;
> +       }
> +
> +       if (!url_nr)
> +               die(_("no URLs configured for remote '%s'"), remotename);
> +
> +       if (all_mode) {
> +               for (i = 0; i < url_nr; i++)
> +                       printf_ln("%s", url[i]);
> +       } else {
> +               printf_ln("%s", *url);
> +       }
> +
> +       return 0;
> +}

Re: [PATCH v2] remote: add get-url subcommand

[Ben Boeckel]

On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
> > +               OPT_BOOL('\0', "push", &push_mode,
> > +                        N_("query push URLs")),
> 
> A bit more explanatory:
> 
>     "query push URLs rather than fetch URLs"

Fixed.

> > +               OPT_BOOL('\0', "all", &all_mode,
> > +                        N_("return all URLs")),
> > +               OPT_END()
> > +       };
> > +       argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage,
> > +                            PARSE_OPT_KEEP_ARGV0);
> 
> What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?

Copied from get-url; I presume for more natural argv[] usage within the
function.

> > +       if (argc < 1 || argc > 2)
> > +               usage_with_options(builtin_remote_geturl_usage, options);
> 
> So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).

…

> > +       remotename = argv[1];
> 
> But here, argv[1] is accessed unconditionally, even though 'argc' may
> have been 1, thus out of bounds.

Yep, should be (argc < 2 || argc > 2) (or 1 if PARSE_OPT_KEEP_ARGV0 is
removed). Off-by-one when converting from get-url.

I'll reroll tomorrow morning in case there are more comments until then
(particularly about PARSE_OPT_KEEP_ARGV0).

Thanks,

--Ben

Re: [PATCH v2] remote: add get-url subcommand

[Eric Sunshine]

On Mon, Aug 3, 2015 at 8:16 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
> On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
>> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
>> > +       argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage,
>> > +                            PARSE_OPT_KEEP_ARGV0);
>>
>> What is the reason for PARSE_OPT_KEEP_ARGV0 in this case?
>
> Copied from get-url; I presume for more natural argv[] usage within the
> function.
>
>> > +       if (argc < 1 || argc > 2)
>> > +               usage_with_options(builtin_remote_geturl_usage, options);
>>
>> So,  'argc' must be 1 or 2, which in 'argv' terms is argv[0] and argv[1]).
>>
>> > +       remotename = argv[1];
>>
>> But here, argv[1] is accessed unconditionally, even though 'argc' may
>> have been 1, thus out of bounds.
>
> Yep, should be (argc < 2 || argc > 2) (or 1 if PARSE_OPT_KEEP_ARGV0 is
> removed). Off-by-one when converting from get-url.

Or, expressed more naturally:

    if (argc != 1)
        usage_with_options(...);

assuming the unnecessary PARSE_OPT_KEEP_ARGV0 is dropped.

> I'll reroll tomorrow morning in case there are more comments until then
> (particularly about PARSE_OPT_KEEP_ARGV0).

This new code doesn't take advantage of it, and it's very rarely used
in Git itself, thus its use here is a potential source of confusion,
so it's probably best to drop it. (The same could be said for
set_url(), but that's a separate topic.)

[PATCH v3] remote: add get-url subcommand

[Ben Boeckel]

Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 54 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..3c9bf45 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
+'git remote get-url' [--push] [--all] <name>
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried rather than fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote <name> that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index f4a6ec9..904869a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
+	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
 	N_("git remote set-url --delete <name> <url>"),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
 	NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+	N_("git remote get-url [--push] [--all] <name>"),
+	NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
@@ -1497,6 +1503,52 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+	int i, push_mode = 0, all_mode = 0;
+	const char *remotename = NULL;
+	struct remote *remote;
+	const char **url;
+	int url_nr;
+	struct option options[] = {
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("query push URLs rather than fetch URLs")),
+		OPT_BOOL('\0', "all", &all_mode,
+			 N_("return all URLs")),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);
+
+	if (argc != 1)
+		usage_with_options(builtin_remote_geturl_usage, options);
+
+	remotename = argv[0];
+
+	if (!remote_is_configured(remotename))
+		die(_("No such remote '%s'"), remotename);
+	remote = remote_get(remotename);
+
+	if (push_mode) {
+		url = remote->pushurl;
+		url_nr = remote->pushurl_nr;
+	} else {
+		url = remote->url;
+		url_nr = remote->url_nr;
+	}
+
+	if (!url_nr)
+		die(_("no URLs configured for remote '%s'"), remotename);
+
+	if (all_mode) {
+		for (i = 0; i < url_nr; i++)
+			printf_ln("%s", url[i]);
+	} else {
+		printf_ln("%s", *url);
+	}
+
+	return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
 	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1606,6 +1658,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "set-branches"))
 		result = set_branches(argc, argv);
+	else if (!strcmp(argv[0], "get-url"))
+		result = get_url(argc, argv);
 	else if (!strcmp(argv[0], "set-url"))
 		result = set_url(argc, argv);
 	else if (!strcmp(argv[0], "show"))
-- 
2.1.0

Re: [PATCH v3] remote: add get-url subcommand

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> Expanding `insteadOf` is a part of ls-remote --url and there is no way
> to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
> to query both as well as a way to get all configured urls.
>
> Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
> ---
>  Documentation/git-remote.txt | 10 ++++++++
>  builtin/remote.c             | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)

Changes to these two files look reasonable.

Don't you want to protect this feature from future breakage by
others by adding a couple of tests, though, to t/t5505?

Thanks.

Re: [PATCH v3] remote: add get-url subcommand

[Ben Boeckel]

On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote:
> Changes to these two files look reasonable.
> 
> Don't you want to protect this feature from future breakage by
> others by adding a couple of tests, though, to t/t5505?

Thanks, I've done so locally. It actually brings up this case:

    $ git remote add someremote foo
    $ git remote get-url --push someremote
    fatal: no URLs configured for remote 'someremote'

Is it better to use:

    remote = remote_get(remotename);
    remote->pushurl;

    if (remote->pushurl_nr)
        remote->pushurl;
    else
        remote->url;

or:

    remote = pushremote_get(remotename);
    remote->pushurl;

? What is the actual difference between the two?

Thanks,

--Ben

Re: [PATCH v3] remote: add get-url subcommand

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote:
>> Changes to these two files look reasonable.
>> 
>> Don't you want to protect this feature from future breakage by
>> others by adding a couple of tests, though, to t/t5505?
>
> Thanks, I've done so locally. It actually brings up this case:
>
>     $ git remote add someremote foo
>     $ git remote get-url --push someremote
>     fatal: no URLs configured for remote 'someremote'
>
> Is it better to use:
>
>     remote = remote_get(remotename);
>     remote->pushurl;
>
>     if (remote->pushurl_nr)
>         remote->pushurl;
>     else
>         remote->url;
>
> or:
>
>     remote = pushremote_get(remotename);
>     remote->pushurl;
>
> ? What is the actual difference between the two?

You tell me ;-)

The default remote based on the current branch is computed
differently based on the direction of the transfer, I think.

        struct remote *remote_get(const char *name)
        {
                return remote_get_1(name, remote_for_branch);
        }

        struct remote *pushremote_get(const char *name)
        {
                return remote_get_1(name, pushremote_for_branch);
        }

When you are not giving name explicitly, the second parameter to _1 
function is used to determine the name.

[PATCH v4] remote: add get-url subcommand

[Ben Boeckel]

Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 58 ++++++++++++++++++++++++++++++++++++++++++++
 t/t5505-remote.sh            | 53 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 121 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..3c9bf45 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
+'git remote get-url' [--push] [--all] <name>
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried rather than fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote <name> that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index 181668d..192b9cb 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
+	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
 	N_("git remote set-url --delete <name> <url>"),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
 	NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+	N_("git remote get-url [--push] [--all] <name>"),
+	NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
@@ -1467,6 +1473,56 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+	int i, push_mode = 0, all_mode = 0;
+	const char *remotename = NULL;
+	struct remote *remote;
+	const char **url;
+	int url_nr;
+	struct option options[] = {
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("query push URLs rather than fetch URLs")),
+		OPT_BOOL('\0', "all", &all_mode,
+			 N_("return all URLs")),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);
+
+	if (argc != 1)
+		usage_with_options(builtin_remote_geturl_usage, options);
+
+	remotename = argv[0];
+
+	if (!remote_is_configured(remotename))
+		die(_("No such remote '%s'"), remotename);
+	remote = remote_get(remotename);
+
+	url_nr = 0;
+	if (push_mode) {
+		url = remote->pushurl;
+		url_nr = remote->pushurl_nr;
+	}
+
+	/* Fall back to the fetch URL if no push URLs are set. */
+	if (!url_nr) {
+		url = remote->url;
+		url_nr = remote->url_nr;
+	}
+
+	if (!url_nr)
+		die(_("no URLs configured for remote '%s'"), remotename);
+
+	if (all_mode) {
+		for (i = 0; i < url_nr; i++)
+			printf_ln("%s", url[i]);
+	} else {
+		printf_ln("%s", *url);
+	}
+
+	return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
 	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1576,6 +1632,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "set-branches"))
 		result = set_branches(argc, argv);
+	else if (!strcmp(argv[0], "get-url"))
+		result = get_url(argc, argv);
 	else if (!strcmp(argv[0], "set-url"))
 		result = set_url(argc, argv);
 	else if (!strcmp(argv[0], "show"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 7a8499c..2cfd3cb 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -919,6 +919,18 @@ test_expect_success 'new remote' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on new remote' '
+	echo foo >expect &&
+	git remote get-url someremote >actual &&
+	cmp expect actual &&
+	git remote get-url --all someremote >actual &&
+	cmp expect actual &&
+	git remote get-url --push someremote >actual &&
+	cmp expect actual &&
+	git remote get-url --push --all someremote >actual &&
+	cmp expect actual
+'
+
 test_expect_success 'remote set-url bar' '
 	git remote set-url someremote bar &&
 	echo bar >expect &&
@@ -961,6 +973,24 @@ test_expect_success 'remote set-url --push zot' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url with different urls' '
+	echo baz >expect &&
+	echo "YYY" >>expect &&
+	echo baz >>expect &&
+	echo "YYY" >>expect &&
+	echo zot >>expect &&
+	echo "YYY" >>expect &&
+	echo zot >>expect &&
+	git remote get-url someremote >actual &&
+	echo "YYY" >>actual &&
+	git remote get-url --all someremote >>actual &&
+	echo "YYY" >>actual &&
+	git remote get-url --push someremote >>actual &&
+	echo "YYY" >>actual &&
+	git remote get-url --push --all someremote >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'remote set-url --push qux zot' '
 	git remote set-url --push someremote qux zot &&
 	echo qux >expect &&
@@ -995,6 +1025,17 @@ test_expect_success 'remote set-url --push --add aaa' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi push remote' '
+	echo foo >expect &&
+	echo "YYY" >>expect &&
+	echo foo >>expect &&
+	echo aaa >>expect &&
+	git remote get-url --push someremote >actual &&
+	echo "YYY" >>actual &&
+	git remote get-url --push --all someremote >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'remote set-url --push bar aaa' '
 	git remote set-url --push someremote bar aaa &&
 	echo foo >expect &&
@@ -1039,6 +1080,17 @@ test_expect_success 'remote set-url --add bbb' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi fetch remote' '
+	echo baz >expect &&
+	echo "YYY" >>expect &&
+	echo baz >>expect &&
+	echo bbb >>expect &&
+	git remote get-url someremote >actual &&
+	echo "YYY" >>actual &&
+	git remote get-url --all someremote >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'remote set-url --delete .*' '
 	test_must_fail git remote set-url --delete someremote .\* &&
 	echo "YYY" >expect &&
@@ -1108,6 +1160,7 @@ test_extra_arg rename origin newname
 test_extra_arg remove origin
 test_extra_arg set-head origin master
 # set-branches takes any number of args
+test_extra_arg get-url origin newurl
 test_extra_arg set-url origin newurl oldurl
 # show takes any number of args
 # prune takes any number of args
-- 
2.5.0.1.g397396c.dirty

Re: [PATCH v4] remote: add get-url subcommand

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> +	url_nr = 0;
> +	if (push_mode) {
> +		url = remote->pushurl;
> +		url_nr = remote->pushurl_nr;
> +	}
> +
> +	/* Fall back to the fetch URL if no push URLs are set. */
> +	if (!url_nr) {
> +		url = remote->url;
> +		url_nr = remote->url_nr;
> +	}

While the code does the right thing, the comment and the logic
looked somewhat on the side of funny than cute.  url_nr would be
zero when we are asking for fetch URL, and it would be zero also
when we are asking for push URL but there is no push URL defined.
So this statement covers both cases, but "Fall back to" talks only
about the case where the user asked for push URL.

> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 7a8499c..2cfd3cb 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -919,6 +919,18 @@ test_expect_success 'new remote' '
>  	cmp expect actual
>  '
>  
> +test_expect_success 'get-url on new remote' '
> +	echo foo >expect &&
> +	git remote get-url someremote >actual &&
> +	cmp expect actual &&
> +	git remote get-url --all someremote >actual &&
> +	cmp expect actual &&
> +	git remote get-url --push someremote >actual &&
> +	cmp expect actual &&
> +	git remote get-url --push --all someremote >actual &&
> +	cmp expect actual
> +'

In the pre-context of this hunk, I can see that you inherited this
habit from existing tests, but breakage can be made easier to see
if you used test_cmp instead of cmp.

> @@ -961,6 +973,24 @@ test_expect_success 'remote set-url --push zot' '
>  	cmp expect actual
>  '
>  
> +test_expect_success 'get-url with different urls' '
> +	echo baz >expect &&
> +	echo "YYY" >>expect &&
> +	echo baz >>expect &&
> +	echo "YYY" >>expect &&
> +	echo zot >>expect &&
> +	echo "YYY" >>expect &&
> +	echo zot >>expect &&
> +	git remote get-url someremote >actual &&
> +	echo "YYY" >>actual &&
> +	git remote get-url --all someremote >>actual &&
> +	echo "YYY" >>actual &&
> +	git remote get-url --push someremote >>actual &&
> +	echo "YYY" >>actual &&
> +	git remote get-url --push --all someremote >>actual &&
> +	cmp expect actual
> +'

I am not sure what these YYY are about. Is this an attempt to make
it easier to see which of the output from four logically separate
tests are broken?

I am wondering if something along this line might be easier to read
and maintain:

	get_url_test () {
		cat >expect &&
		test_expect_success "get-url $*" '
	                git remote get-url $* >actual &&
	                test_cmp expect actual
		'
	}

	echo baz | get_url_test someremote
	echo baz | get_url_test --all someremote

Then later when you have more than one pushURL to someremote, you
would do something like:

	get_url_test --all --push someremote <<\-EOF
	foo
        aaa
        EOF

This comment applies to the remainder of this patch that has YYY
sprinkled all over it.

Thanks.

[PATCH v5] remote: add get-url subcommand

[Ben Boeckel]

Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 59 ++++++++++++++++++++++++++++++++++++++++++++
 t/t5505-remote.sh            | 39 +++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..3c9bf45 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
+'git remote get-url' [--push] [--all] <name>
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried rather than fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote <name> that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index 181668d..e4c3ea1 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
+	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
 	N_("git remote set-url --delete <name> <url>"),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
 	NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+	N_("git remote get-url [--push] [--all] <name>"),
+	NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
@@ -1467,6 +1473,57 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+	int i, push_mode = 0, all_mode = 0;
+	const char *remotename = NULL;
+	struct remote *remote;
+	const char **url;
+	int url_nr;
+	struct option options[] = {
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("query push URLs rather than fetch URLs")),
+		OPT_BOOL('\0', "all", &all_mode,
+			 N_("return all URLs")),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);
+
+	if (argc != 1)
+		usage_with_options(builtin_remote_geturl_usage, options);
+
+	remotename = argv[0];
+
+	if (!remote_is_configured(remotename))
+		die(_("No such remote '%s'"), remotename);
+	remote = remote_get(remotename);
+
+	url_nr = 0;
+	if (push_mode) {
+		url = remote->pushurl;
+		url_nr = remote->pushurl_nr;
+	}
+	/* else fetch mode */
+
+	/* Use the fetch URL when no push URLs were found or requested. */
+	if (!url_nr) {
+		url = remote->url;
+		url_nr = remote->url_nr;
+	}
+
+	if (!url_nr)
+		die(_("no URLs configured for remote '%s'"), remotename);
+
+	if (all_mode) {
+		for (i = 0; i < url_nr; i++)
+			printf_ln("%s", url[i]);
+	} else {
+		printf_ln("%s", *url);
+	}
+
+	return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
 	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1576,6 +1633,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "set-branches"))
 		result = set_branches(argc, argv);
+	else if (!strcmp(argv[0], "get-url"))
+		result = get_url(argc, argv);
 	else if (!strcmp(argv[0], "set-url"))
 		result = set_url(argc, argv);
 	else if (!strcmp(argv[0], "show"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 7a8499c..f03ba4c 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -919,6 +919,21 @@ test_expect_success 'new remote' '
 	cmp expect actual
 '
 
+get_url_test () {
+	cat >expect &&
+	test_expect_success "get-url $*" "
+		git remote get-url $* >actual &&
+		test_cmp expect actual
+	"
+}
+
+test_expect_success 'get-url on new remote' '
+	echo foo | get_url_test someremote &&
+	echo foo | get_url_test --all someremote &&
+	echo foo | get_url_test --push someremote &&
+	echo foo | get_url_test --push --all someremote
+'
+
 test_expect_success 'remote set-url bar' '
 	git remote set-url someremote bar &&
 	echo bar >expect &&
@@ -961,6 +976,13 @@ test_expect_success 'remote set-url --push zot' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url with different urls' '
+	echo baz | get_url_test someremote &&
+	echo baz | get_url_test --all someremote &&
+	echo zot | get_url_test --push someremote &&
+	echo zot | get_url_test --push --all someremote
+'
+
 test_expect_success 'remote set-url --push qux zot' '
 	git remote set-url --push someremote qux zot &&
 	echo qux >expect &&
@@ -995,6 +1017,14 @@ test_expect_success 'remote set-url --push --add aaa' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi push remote' '
+	echo foo | get_url_test --push someremote &&
+	get_url_test --push --all someremote <<-\EOF
+	foo
+	aaa
+	EOF
+'
+
 test_expect_success 'remote set-url --push bar aaa' '
 	git remote set-url --push someremote bar aaa &&
 	echo foo >expect &&
@@ -1039,6 +1069,14 @@ test_expect_success 'remote set-url --add bbb' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi fetch remote' '
+	echo baz | get_url_test someremote &&
+	get_url_test --all someremote <<-\EOF
+	baz
+	bbb
+	EOF
+'
+
 test_expect_success 'remote set-url --delete .*' '
 	test_must_fail git remote set-url --delete someremote .\* &&
 	echo "YYY" >expect &&
@@ -1108,6 +1146,7 @@ test_extra_arg rename origin newname
 test_extra_arg remove origin
 test_extra_arg set-head origin master
 # set-branches takes any number of args
+test_extra_arg get-url origin newurl
 test_extra_arg set-url origin newurl oldurl
 # show takes any number of args
 # prune takes any number of args
-- 
2.5.2

Re: [PATCH v5] remote: add get-url subcommand

[Junio C Hamano]

Ben Boeckel <mathstuf@gmail.com> writes:

> +get_url_test () {
> +	cat >expect &&
> +	test_expect_success "get-url $*" "
> +		git remote get-url $* >actual &&
> +		test_cmp expect actual
> +	"
> +}

This makes any use of get_url_test inside test_expect_success wrong,
I suspect.  Try running the tests under prove.  I think it will
complain that the test numbers do not match or something.

Minimally, this would probably fix the breakage.

 t/t5505-remote.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index f03ba4c..dfaf9d9 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -921,10 +921,8 @@ test_expect_success 'new remote' '
 
 get_url_test () {
 	cat >expect &&
-	test_expect_success "get-url $*" "
-		git remote get-url $* >actual &&
-		test_cmp expect actual
-	"
+	git remote get-url "$@" >actual &&
+	test_cmp expect actual
 }
 
 test_expect_success 'get-url on new remote' '

[PATCH v6] remote: add get-url subcommand

[Ben Boeckel]

Expanding `insteadOf` is a part of ls-remote --url and there is no way
to expand `pushInsteadOf` as well. Add a get-url subcommand to be able
to query both as well as a way to get all configured urls.

Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
---
 Documentation/git-remote.txt | 10 ++++++++
 builtin/remote.c             | 59 ++++++++++++++++++++++++++++++++++++++++++++
 t/t5505-remote.sh            | 37 +++++++++++++++++++++++++++
 3 files changed, 106 insertions(+)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 4c6d6de..3c9bf45 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git remote remove' <name>
 'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
 'git remote set-branches' [--add] <name> <branch>...
+'git remote get-url' [--push] [--all] <name>
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
@@ -131,6 +132,15 @@ The named branches will be interpreted as if specified with the
 With `--add`, instead of replacing the list of currently tracked
 branches, adds to that list.
 
+'get-url'::
+
+Retrieves the URLs for a remote. Configurations for `insteadOf` and
+`pushInsteadOf` are expanded here. By default, only the first URL is listed.
++
+With '--push', push URLs are queried rather than fetch URLs.
++
+With '--all', all URLs for the remote will be listed.
+
 'set-url'::
 
 Changes URLs for the remote. Sets first URL for remote <name> that matches
diff --git a/builtin/remote.c b/builtin/remote.c
index 181668d..e4c3ea1 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,6 +18,7 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote prune [-n | --dry-run] <name>"),
 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
 	N_("git remote set-branches [--add] <name> <branch>..."),
+	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
 	N_("git remote set-url --delete <name> <url>"),
@@ -65,6 +66,11 @@ static const char * const builtin_remote_update_usage[] = {
 	NULL
 };
 
+static const char * const builtin_remote_geturl_usage[] = {
+	N_("git remote get-url [--push] [--all] <name>"),
+	NULL
+};
+
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
 	N_("git remote set-url --add <name> <newurl>"),
@@ -1467,6 +1473,57 @@ static int set_branches(int argc, const char **argv)
 	return set_remote_branches(argv[0], argv + 1, add_mode);
 }
 
+static int get_url(int argc, const char **argv)
+{
+	int i, push_mode = 0, all_mode = 0;
+	const char *remotename = NULL;
+	struct remote *remote;
+	const char **url;
+	int url_nr;
+	struct option options[] = {
+		OPT_BOOL('\0', "push", &push_mode,
+			 N_("query push URLs rather than fetch URLs")),
+		OPT_BOOL('\0', "all", &all_mode,
+			 N_("return all URLs")),
+		OPT_END()
+	};
+	argc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);
+
+	if (argc != 1)
+		usage_with_options(builtin_remote_geturl_usage, options);
+
+	remotename = argv[0];
+
+	if (!remote_is_configured(remotename))
+		die(_("No such remote '%s'"), remotename);
+	remote = remote_get(remotename);
+
+	url_nr = 0;
+	if (push_mode) {
+		url = remote->pushurl;
+		url_nr = remote->pushurl_nr;
+	}
+	/* else fetch mode */
+
+	/* Use the fetch URL when no push URLs were found or requested. */
+	if (!url_nr) {
+		url = remote->url;
+		url_nr = remote->url_nr;
+	}
+
+	if (!url_nr)
+		die(_("no URLs configured for remote '%s'"), remotename);
+
+	if (all_mode) {
+		for (i = 0; i < url_nr; i++)
+			printf_ln("%s", url[i]);
+	} else {
+		printf_ln("%s", *url);
+	}
+
+	return 0;
+}
+
 static int set_url(int argc, const char **argv)
 {
 	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
@@ -1576,6 +1633,8 @@ int cmd_remote(int argc, const char **argv, const char *prefix)
 		result = set_head(argc, argv);
 	else if (!strcmp(argv[0], "set-branches"))
 		result = set_branches(argc, argv);
+	else if (!strcmp(argv[0], "get-url"))
+		result = get_url(argc, argv);
 	else if (!strcmp(argv[0], "set-url"))
 		result = set_url(argc, argv);
 	else if (!strcmp(argv[0], "show"))
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 7a8499c..9a55389 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -919,6 +919,19 @@ test_expect_success 'new remote' '
 	cmp expect actual
 '
 
+get_url_test () {
+	cat >expect &&
+	git remote get-url $* >actual &&
+	test_cmp expect actual
+}
+
+test_expect_success 'get-url on new remote' '
+	echo foo | get_url_test someremote &&
+	echo foo | get_url_test --all someremote &&
+	echo foo | get_url_test --push someremote &&
+	echo foo | get_url_test --push --all someremote
+'
+
 test_expect_success 'remote set-url bar' '
 	git remote set-url someremote bar &&
 	echo bar >expect &&
@@ -961,6 +974,13 @@ test_expect_success 'remote set-url --push zot' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url with different urls' '
+	echo baz | get_url_test someremote &&
+	echo baz | get_url_test --all someremote &&
+	echo zot | get_url_test --push someremote &&
+	echo zot | get_url_test --push --all someremote
+'
+
 test_expect_success 'remote set-url --push qux zot' '
 	git remote set-url --push someremote qux zot &&
 	echo qux >expect &&
@@ -995,6 +1015,14 @@ test_expect_success 'remote set-url --push --add aaa' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi push remote' '
+	echo foo | get_url_test --push someremote &&
+	get_url_test --push --all someremote <<-\EOF
+	foo
+	aaa
+	EOF
+'
+
 test_expect_success 'remote set-url --push bar aaa' '
 	git remote set-url --push someremote bar aaa &&
 	echo foo >expect &&
@@ -1039,6 +1067,14 @@ test_expect_success 'remote set-url --add bbb' '
 	cmp expect actual
 '
 
+test_expect_success 'get-url on multi fetch remote' '
+	echo baz | get_url_test someremote &&
+	get_url_test --all someremote <<-\EOF
+	baz
+	bbb
+	EOF
+'
+
 test_expect_success 'remote set-url --delete .*' '
 	test_must_fail git remote set-url --delete someremote .\* &&
 	echo "YYY" >expect &&
@@ -1108,6 +1144,7 @@ test_extra_arg rename origin newname
 test_extra_arg remove origin
 test_extra_arg set-head origin master
 # set-branches takes any number of args
+test_extra_arg get-url origin newurl
 test_extra_arg set-url origin newurl oldurl
 # show takes any number of args
 # prune takes any number of args
-- 
2.5.2