remote: simplify url/pushurl selection

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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 14, 2024 at 06:29 UTC b68118d2e85eef7aa993ef8e944e53b5be665160
4 files changed +31 -65
builtin/push.c
+5 -16
@@ -141,16 +141,6 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
141 free_refs(local_refs);
142 }
143
144 -static int push_url_of_remote(struct remote *remote, const char ***url_p)
145 -{
146 - if (remote->pushurl.nr) {
147 - *url_p = remote->pushurl.v;
148 - return remote->pushurl.nr;
149 - }
150 - *url_p = remote->url.v;
151 - return remote->url.nr;
152 -}
153 -
144 static NORETURN void die_push_simple(struct branch *branch,
145 struct remote *remote)
146 {
@@ -434,8 +424,7 @@ static int do_push(int flags,
424 struct remote *remote)
425 {
426 int i, errs;
437 - const char **url;
438 - int url_nr;
427 + struct strvec *url;
428 struct refspec *push_refspec = &rs;
429
430 if (push_options->nr)
@@ -448,11 +437,11 @@ static int do_push(int flags,
437 setup_default_push_refspecs(&flags, remote);
438 }
439 errs = 0;
451 - url_nr = push_url_of_remote(remote, &url);
452 - if (url_nr) {
453 - for (i = 0; i < url_nr; i++) {
440 + url = push_url_of_remote(remote);
441 + if (url->nr) {
442 + for (i = 0; i < url->nr; i++) {
443 struct transport *transport =
455 - transport_get(remote, url[i]);
444 + transport_get(remote, url->v[i]);
445 if (flags & TRANSPORT_PUSH_OPTIONS)
446 transport->push_options = push_options;
447 if (push_with_options(transport, push_refspec, flags))
builtin/remote.c
+20 -49
@@ -1213,8 +1213,8 @@ static int get_one_entry(struct remote *remote, void *priv)
1213 {
1214 struct string_list *list = priv;
1215 struct strbuf remote_info_buf = STRBUF_INIT;
1216 - const char **url;
1217 - int i, url_nr;
1216 + struct strvec *url;
1217 + int i;
1218
1219 if (remote->url.nr > 0) {
1220 struct strbuf promisor_config = STRBUF_INIT;
@@ -1230,16 +1230,10 @@ static int get_one_entry(struct remote *remote, void *priv)
1230 strbuf_detach(&remote_info_buf, NULL);
1231 } else
1232 string_list_append(list, remote->name)->util = NULL;
1233 - if (remote->pushurl.nr) {
1234 - url = remote->pushurl.v;
1235 - url_nr = remote->pushurl.nr;
1236 - } else {
1237 - url = remote->url.v;
1238 - url_nr = remote->url.nr;
1239 - }
1240 - for (i = 0; i < url_nr; i++)
1233 + url = push_url_of_remote(remote);
1234 + for (i = 0; i < url->nr; i++)
1235 {
1242 - strbuf_addf(&remote_info_buf, "%s (push)", url[i]);
1236 + strbuf_addf(&remote_info_buf, "%s (push)", url->v[i]);
1237 string_list_append(list, remote->name)->util =
1238 strbuf_detach(&remote_info_buf, NULL);
1239 }
@@ -1295,28 +1289,21 @@ static int show(int argc, const char **argv, const char *prefix)
1289
1290 for (; argc; argc--, argv++) {
1291 int i;
1298 - const char **url;
1299 - int url_nr;
1292 + struct strvec *url;
1293
1294 get_remote_ref_states(*argv, &info.states, query_flag);
1295
1296 printf_ln(_("* remote %s"), *argv);
1297 printf_ln(_(" Fetch URL: %s"), info.states.remote->url.nr > 0 ?
1298 info.states.remote->url.v[0] : _("(no URL)"));
1306 - if (info.states.remote->pushurl.nr) {
1307 - url = info.states.remote->pushurl.v;
1308 - url_nr = info.states.remote->pushurl.nr;
1309 - } else {
1310 - url = info.states.remote->url.v;
1311 - url_nr = info.states.remote->url.nr;
1312 - }
1313 - for (i = 0; i < url_nr; i++)
1299 + url = push_url_of_remote(info.states.remote);
1300 + for (i = 0; i < url->nr; i++)
1301 /*
1302 * TRANSLATORS: the colon ':' should align
1303 * with the one in " Fetch URL: %s"
1304 * translation.
1305 */
1319 - printf_ln(_(" Push URL: %s"), url[i]);
1306 + printf_ln(_(" Push URL: %s"), url->v[i]);
1307 if (!i)
1308 printf_ln(_(" Push URL: %s"), _("(no URL)"));
1309 if (no_query)
@@ -1622,8 +1609,7 @@ static int get_url(int argc, const char **argv, const char *prefix)
1609 int i, push_mode = 0, all_mode = 0;
1610 const char *remotename = NULL;
1611 struct remote *remote;
1625 - const char **url;
1626 - int url_nr;
1612 + struct strvec *url;
1613 struct option options[] = {
1614 OPT_BOOL('\0', "push", &push_mode,
1615 N_("query push URLs rather than fetch URLs")),
@@ -1645,27 +1631,15 @@ static int get_url(int argc, const char **argv, const char *prefix)
1631 exit(2);
1632 }
1633
1648 - url_nr = 0;
1649 - if (push_mode) {
1650 - url = remote->pushurl.v;
1651 - url_nr = remote->pushurl.nr;
1652 - }
1653 - /* else fetch mode */
1654 -
1655 - /* Use the fetch URL when no push URLs were found or requested. */
1656 - if (!url_nr) {
1657 - url = remote->url.v;
1658 - url_nr = remote->url.nr;
1659 - }
1660 -
1661 - if (!url_nr)
1634 + url = push_mode ? push_url_of_remote(remote) : &remote->url;
1635 + if (!url->nr)
1636 die(_("no URLs configured for remote '%s'"), remotename);
1637
1638 if (all_mode) {
1665 - for (i = 0; i < url_nr; i++)
1666 - printf_ln("%s", url[i]);
1639 + for (i = 0; i < url->nr; i++)
1640 + printf_ln("%s", url->v[i]);
1641 } else {
1668 - printf_ln("%s", *url);
1642 + printf_ln("%s", url->v[0]);
1643 }
1644
1645 return 0;
@@ -1680,8 +1654,7 @@ static int set_url(int argc, const char **argv, const char *prefix)
1654 const char *oldurl = NULL;
1655 struct remote *remote;
1656 regex_t old_regex;
1683 - const char **urlset;
1684 - int urlset_nr;
1657 + struct strvec *urlset;
1658 struct strbuf name_buf = STRBUF_INIT;
1659 struct option options[] = {
1660 OPT_BOOL('\0', "push", &push_mode,
@@ -1718,12 +1691,10 @@ static int set_url(int argc, const char **argv, const char *prefix)
1691
1692 if (push_mode) {
1693 strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
1721 - urlset = remote->pushurl.v;
1722 - urlset_nr = remote->pushurl.nr;
1694 + urlset = &remote->pushurl;
1695 } else {
1696 strbuf_addf(&name_buf, "remote.%s.url", remotename);
1725 - urlset = remote->url.v;
1726 - urlset_nr = remote->url.nr;
1697 + urlset = &remote->url;
1698 }
1699
1700 /* Special cases that add new entry. */
@@ -1740,8 +1711,8 @@ static int set_url(int argc, const char **argv, const char *prefix)
1711 if (regcomp(&old_regex, oldurl, REG_EXTENDED))
1712 die(_("Invalid old URL pattern: %s"), oldurl);
1713
1743 - for (i = 0; i < urlset_nr; i++)
1744 - if (!regexec(&old_regex, urlset[i], 0, NULL, 0))
1714 + for (i = 0; i < urlset->nr; i++)
1715 + if (!regexec(&old_regex, urlset->v[i], 0, NULL, 0))
1716 matches++;
1717 else
1718 negative_matches++;
remote.c
+5
@@ -827,6 +827,11 @@ int remote_has_url(struct remote *remote, const char *url)
827 return 0;
828 }
829
830 +struct strvec *push_url_of_remote(struct remote *remote)
831 +{
832 + return remote->pushurl.nr ? &remote->pushurl : &remote->url;
833 +}
834 +
835 static int match_name_with_pattern(const char *key, const char *name,
836 const char *value, char **result)
837 {
remote.h
+1
@@ -123,6 +123,7 @@ typedef int each_remote_fn(struct remote *remote, void *priv);
123 int for_each_remote(each_remote_fn fn, void *priv);
124
125 int remote_has_url(struct remote *remote, const char *url);
126 +struct strvec *push_url_of_remote(struct remote *remote);
127
128 struct ref_push_report {
129 const char *ref_name;