Re: [PATCH v2] remote: add get-url subcommand
On Mon, Aug 3, 2015 at 8:16 PM, Ben Boeckel 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 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.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] remote: add get-url subcommand
On Mon, Aug 03, 2015 at 19:38:15 -0400, Eric Sunshine wrote:
> On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] remote: add get-url subcommand
On Mon, Aug 3, 2015 at 5:00 PM, Ben Boeckel 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
> ---
> 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;
> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[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
---
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'
'git remote set-head' (-a | --auto | -d | --delete | )
'git remote set-branches' [--add] ...
+'git remote get-url' [--push] [--all]
'git remote set-url' [--push] []
'git remote set-url --add' [--push]
'git remote set-url --delete' [--push]
@@ -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 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] "),
N_("git remote [-v | --verbose] update [-p | --prune] [( |
)...]"),
N_("git remote set-branches [--add] ..."),
+ N_("git remote get-url [--push] [--all] "),
N_("git remote set-url [--push] []"),
N_("git remote set-url --add "),
N_("git remote set-url --delete "),
@@ -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] "),
+ NULL
+};
+
static const char * const builtin_remote_seturl_usage[] = {
N_("git remote set-url [--push] []"),
N_("git remote set-url --add "),
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
