threads / patch / 39982

patchadd ls-remote --get-push-url option

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

## tl;dr

22 messages between Jul 31, 2015 and Sep 17, 2015. Diffs are folded; open one to read it.

replies: 21people: 3as markdown or json

Ben Boeckel· Jul 31, 2015, 17:38 UTC · lore

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
Ben Boeckel· Jul 31, 2015, 17:38 UTC · re: Ben Boeckel · lore

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

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(-)
Show changes to 2 files +19 −3

Documentation/git-ls-remote.txt, builtin/ls-remote.c

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
Junio C Hamano· Jul 31, 2015, 18:40 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 16 quoted lines
> 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?
Ben Boeckel· Jul 31, 2015, 18:56 UTC · re: Junio C Hamano · lore

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

On Fri, Jul 31, 2015 at 11:40:12 -0700, Junio C Hamano wrote:
Show 7 quoted lines
> 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
Junio C Hamano· Jul 31, 2015, 19:02 UTC · re: Ben Boeckel · lore

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

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'.
Ben Boeckel· Jul 31, 2015, 19:04 UTC · re: Junio C Hamano · lore

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

On Fri, Jul 31, 2015 at 12:02:14 -0700, Junio C Hamano wrote:
Show 7 quoted lines
> 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
Junio C Hamano· Jul 31, 2015, 19:16 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 11 quoted lines
> 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.

Ben Boeckel· Jul 31, 2015, 20:51 UTC · re: Junio C Hamano · lore

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

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.

Show 10 quoted lines
> 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
Ben Boeckel· Aug 3, 2015, 21:00 UTC · re: Ben Boeckel · lore

[PATCH v2] add git-url subcommand

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
Ben Boeckel· Aug 3, 2015, 21:00 UTC · re: Ben Boeckel · lore

[PATCH v2] remote: add get-url subcommand

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(+)
Show changes to 2 files +65 −0

Documentation/git-remote.txt, builtin/remote.c

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
Eric Sunshine· Aug 3, 2015, 23:38 UTC · re: Ben Boeckel · lore

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

On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
Show 21 quoted lines
> 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"
Show 6 quoted lines
> +               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.

Show 24 quoted lines
> +       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;
> +}
Ben Boeckel· Aug 4, 2015, 00:16 UTC · re: Eric Sunshine · lore

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

On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
Show 7 quoted lines
> 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.
Show 8 quoted lines
> > +               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
Eric Sunshine· Aug 4, 2015, 00:45 UTC · re: Ben Boeckel · lore

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

On Mon, Aug 3, 2015 at 8:16 PM, Ben Boeckel <mathstuf@gmail.com> wrote:
Show 22 quoted lines
> 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.)

Ben Boeckel· Aug 4, 2015, 14:56 UTC · re: Ben Boeckel · lore

[PATCH v3] remote: add get-url subcommand

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(+)
Show changes to 2 files +64 −0

Documentation/git-remote.txt, builtin/remote.c

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
Junio C Hamano· Aug 5, 2015, 20:34 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 9 quoted lines
> 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.
Ben Boeckel· Aug 5, 2015, 21:33 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Aug 5, 2015, 21:39 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 28 quoted lines
> 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.

Ben Boeckel· Sep 4, 2015, 14:30 UTC · re: Ben Boeckel · lore

[PATCH v4] remote: add get-url subcommand

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(+)
Show changes to 3 files +121 −0

Documentation/git-remote.txt, builtin/remote.c, t/t5505-remote.sh

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
Junio C Hamano· Sep 4, 2015, 22:40 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 11 quoted lines
> +	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.

Show 19 quoted lines
> 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.

Show 21 quoted lines
> @@ -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.
Ben Boeckel· Sep 16, 2015, 01:53 UTC · re: Ben Boeckel · lore

[PATCH v5] remote: add get-url subcommand

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(+)
Show changes to 3 files +108 −0

Documentation/git-remote.txt, builtin/remote.c, t/t5505-remote.sh

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
Junio C Hamano· Sep 16, 2015, 22:51 UTC · re: Ben Boeckel · lore

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

Ben Boeckel <mathstuf@gmail.com> writes:
Show 7 quoted lines
> +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(-)
Show changes to t/t5505-remote.sh +2 −4
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' '
Ben Boeckel· Sep 17, 2015, 00:19 UTC · re: Ben Boeckel · lore

[PATCH v6] remote: add get-url subcommand

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(+)
Show changes to 3 files +106 −0

Documentation/git-remote.txt, builtin/remote.c, t/t5505-remote.sh

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

← back to recent threads