Skip to content

Commit

Permalink
remote: simplify url/pushurl selection
Browse files Browse the repository at this point in the history 
When we want to know the push urls for a remote, there is some simple
logic:

  - if the user configured any remote.*.pushurl keys, then those make
    the complete set of push urls

  - otherwise we push to all urls in remote.*.url

Many spots implement this with a level of indirection, assigning to a
local url/url_nr pair. But since both arrays are now strvecs, we can
just use a pointer to select the appropriate strvec, shortening the code
a bit.

Even though this is now a one-liner, since it is application logic that
is present in so many places, it's worth abstracting a helper function.
In fact, we already have such a function, but it's local to
builtin/push.c. So we'll just make it available everywhere via remote.h.

There are two spots to pay special attention to here:

  1. in builtin/remote.c's get_url(), we are selecting first based on
     push_mode and then falling back to "url" when we're in push_mode
     but no pushurl is defined. The updated code makes that much more
     clear, compared to the original which had an "else" fall-through.

  2. likewise in that file's set_url(), we _only_ respect push_mode,
     sine the point is that we are adding to pushurl in that case
     (whether it is empty or not). And thus it does not use our helper
     function.

Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
@peff @gitster
peff authored and gitster committed Jun 14, 2024
1 parent 8e80441 commit b68118d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 65 deletions.
21 changes: 5 additions & 16 deletions builtin/push.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,16 +141,6 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
free_refs(local_refs);
}

static int push_url_of_remote(struct remote *remote, const char ***url_p)
{
if (remote->pushurl.nr) {
*url_p = remote->pushurl.v;
return remote->pushurl.nr;
}
*url_p = remote->url.v;
return remote->url.nr;
}

static NORETURN void die_push_simple(struct branch *branch,
struct remote *remote)
{
Expand Down Expand Up @@ -434,8 +424,7 @@ static int do_push(int flags,
struct remote *remote)
{
int i, errs;
const char **url;
int url_nr;
struct strvec *url;
struct refspec *push_refspec = &rs;

if (push_options->nr)
Expand All @@ -448,11 +437,11 @@ static int do_push(int flags,
setup_default_push_refspecs(&flags, remote);
}
errs = 0;
url_nr = push_url_of_remote(remote, &url);
if (url_nr) {
for (i = 0; i < url_nr; i++) {
url = push_url_of_remote(remote);
if (url->nr) {
for (i = 0; i < url->nr; i++) {
struct transport *transport =
transport_get(remote, url[i]);
transport_get(remote, url->v[i]);
if (flags & TRANSPORT_PUSH_OPTIONS)
transport->push_options = push_options;
if (push_with_options(transport, push_refspec, flags))
Expand Down
69 changes: 20 additions & 49 deletions builtin/remote.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1213,8 +1213,8 @@ static int get_one_entry(struct remote *remote, void *priv)
{
struct string_list *list = priv;
struct strbuf remote_info_buf = STRBUF_INIT;
const char **url;
int i, url_nr;
struct strvec *url;
int i;

if (remote->url.nr > 0) {
struct strbuf promisor_config = STRBUF_INIT;
Expand All @@ -1230,16 +1230,10 @@ static int get_one_entry(struct remote *remote, void *priv)
strbuf_detach(&remote_info_buf, NULL);
} else
string_list_append(list, remote->name)->util = NULL;
if (remote->pushurl.nr) {
url = remote->pushurl.v;
url_nr = remote->pushurl.nr;
} else {
url = remote->url.v;
url_nr = remote->url.nr;
}
for (i = 0; i < url_nr; i++)
url = push_url_of_remote(remote);
for (i = 0; i < url->nr; i++)
{
strbuf_addf(&remote_info_buf, "%s (push)", url[i]);
strbuf_addf(&remote_info_buf, "%s (push)", url->v[i]);
string_list_append(list, remote->name)->util =
strbuf_detach(&remote_info_buf, NULL);
}
Expand Down Expand Up @@ -1295,28 +1289,21 @@ static int show(int argc, const char **argv, const char *prefix)

for (; argc; argc--, argv++) {
int i;
const char **url;
int url_nr;
struct strvec *url;

get_remote_ref_states(*argv, &info.states, query_flag);

printf_ln(_("* remote %s"), *argv);
printf_ln(_(" Fetch URL: %s"), info.states.remote->url.nr > 0 ?
info.states.remote->url.v[0] : _("(no URL)"));
if (info.states.remote->pushurl.nr) {
url = info.states.remote->pushurl.v;
url_nr = info.states.remote->pushurl.nr;
} else {
url = info.states.remote->url.v;
url_nr = info.states.remote->url.nr;
}
for (i = 0; i < url_nr; i++)
url = push_url_of_remote(info.states.remote);
for (i = 0; i < url->nr; i++)
/*
* TRANSLATORS: the colon ':' should align
* with the one in " Fetch URL: %s"
* translation.
*/
printf_ln(_(" Push URL: %s"), url[i]);
printf_ln(_(" Push URL: %s"), url->v[i]);
if (!i)
printf_ln(_(" Push URL: %s"), _("(no URL)"));
if (no_query)
Expand Down Expand Up @@ -1622,8 +1609,7 @@ static int get_url(int argc, const char **argv, const char *prefix)
int i, push_mode = 0, all_mode = 0;
const char *remotename = NULL;
struct remote *remote;
const char **url;
int url_nr;
struct strvec *url;
struct option options[] = {
OPT_BOOL('\0', "push", &push_mode,
N_("query push URLs rather than fetch URLs")),
Expand All @@ -1645,27 +1631,15 @@ static int get_url(int argc, const char **argv, const char *prefix)
exit(2);
}

url_nr = 0;
if (push_mode) {
url = remote->pushurl.v;
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.v;
url_nr = remote->url.nr;
}

if (!url_nr)
url = push_mode ? push_url_of_remote(remote) : &remote->url;
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]);
for (i = 0; i < url->nr; i++)
printf_ln("%s", url->v[i]);
} else {
printf_ln("%s", *url);
printf_ln("%s", url->v[0]);
}

return 0;
Expand All @@ -1680,8 +1654,7 @@ static int set_url(int argc, const char **argv, const char *prefix)
const char *oldurl = NULL;
struct remote *remote;
regex_t old_regex;
const char **urlset;
int urlset_nr;
struct strvec *urlset;
struct strbuf name_buf = STRBUF_INIT;
struct option options[] = {
OPT_BOOL('\0', "push", &push_mode,
Expand Down Expand Up @@ -1718,12 +1691,10 @@ static int set_url(int argc, const char **argv, const char *prefix)

if (push_mode) {
strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
urlset = remote->pushurl.v;
urlset_nr = remote->pushurl.nr;
urlset = &remote->pushurl;
} else {
strbuf_addf(&name_buf, "remote.%s.url", remotename);
urlset = remote->url.v;
urlset_nr = remote->url.nr;
urlset = &remote->url;
}

/* Special cases that add new entry. */
Expand All @@ -1740,8 +1711,8 @@ static int set_url(int argc, const char **argv, const char *prefix)
if (regcomp(&old_regex, oldurl, REG_EXTENDED))
die(_("Invalid old URL pattern: %s"), oldurl);

for (i = 0; i < urlset_nr; i++)
if (!regexec(&old_regex, urlset[i], 0, NULL, 0))
for (i = 0; i < urlset->nr; i++)
if (!regexec(&old_regex, urlset->v[i], 0, NULL, 0))
matches++;
else
negative_matches++;
Expand Down
5 changes: 5 additions & 0 deletions remote.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,11 @@ int remote_has_url(struct remote *remote, const char *url)
return 0;
}

struct strvec *push_url_of_remote(struct remote *remote)
{
return remote->pushurl.nr ? &remote->pushurl : &remote->url;
}

static int match_name_with_pattern(const char *key, const char *name,
const char *value, char **result)
{
Expand Down
1 change: 1 addition & 0 deletions remote.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ typedef int each_remote_fn(struct remote *remote, void *priv);
int for_each_remote(each_remote_fn fn, void *priv);

int remote_has_url(struct remote *remote, const char *url);
struct strvec *push_url_of_remote(struct remote *remote);

struct ref_push_report {
const char *ref_name;
Expand Down

0 comments on commit b68118d

Please sign in to comment.