'struct remote' stores fetch refspecs twice: once in their original
string form in remote->fetch_refspecs and once in their parsed form in
remote->fetch.
The main reason for this is that we want to read the configuration
only once, but we don't want to error out while doing so because of a
bogus refspec in a remote that we aren't accessing at all. Therefore,
when the configuration is read all refspecs are simply stored in
string arrays for each remote, and then refspecs of a particular
remote are parsed lazily, only when that remote is actually accessed.
However, storing refspecs in both forms has some drawbacks:
- The same information is stored twice, wasting memory.
- remote->fetch_refspecs, i.e. the string array is conveniently
ALLOC_GROW()-able with associated 'fetch_refspec_{nr,alloc}'
fields, but remote->fetch is not.
- Wherever remote->fetch are accessed, the number of parsed refspecs
in there is specified by remote->fetch_refspec_nr. This requires
us to keep the two arrays in sync and makes adding additional
refspecs cumbersome and error prone.
So eliminate remote->fetch_refspec and parse fetch refspecs right away
while they are being read from the configuration. However, to avoid
erroring out on bogus refspecs in "uninteresting" remotes, parse them
gently: instead of die()ing, store the problematic refspec in
remote->bogus_refspec, so it will be available later when that remote
is actually accessed and we can use it in the error message. Make
remote->fetch ALLOC_GROW()-able. Add a new add_fetch_refspec()
function to the remote API, replacing an old function with the same
name and add_and_parse_fetch_refspec(), to encompass memory
management of remote->fetch and parsing the new refspec (gently or
otherwise). Update all sites accessing fetch-refspec-related fields
to use the new field names.
Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
---
builtin/clone.c | 6 +++---
builtin/fetch.c | 20 ++++++++++----------
builtin/remote.c | 18 +++++++++---------
remote.c | 55 +++++++++++++++++++++++++++++--------------------------
remote.h | 23 ++++++++++++++++++-----
5 files changed, 69 insertions(+), 53 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5b72d853f..05cc57f79 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1054,7 +1054,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
remote = remote_get(option_origin);
strbuf_addf(&default_refspec, "+%s*:%s*", src_ref_prefix,
branch_top.buf);
- add_and_parse_fetch_refspec(remote, default_refspec.buf);
+ add_fetch_refspec(remote, default_refspec.buf, 0);
transport = transport_get(remote, remote->url[0]);
transport_set_verbosity(transport, option_verbosity, option_progress);
@@ -1106,8 +1106,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
refs = transport_get_remote_refs(transport);
if (refs) {
- mapped_refs = wanted_peer_refs(refs, remote->fetch,
- remote->fetch_refspec_nr);
+ mapped_refs = wanted_peer_refs(refs, remote->fetch.rs,
+ remote->fetch.nr);
/*
* transport_get_remote_refs() may return refs with null sha-1
* in mapped_refs (see struct transport->get_refs_list
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 100248c5a..0f791a12f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -360,8 +360,8 @@ static struct ref *get_ref_map(struct transport *transport,
fetch_refspec = parse_fetch_refspec(refmap_nr, refmap_array);
fetch_refspec_nr = refmap_nr;
} else {
- fetch_refspec = transport->remote->fetch;
- fetch_refspec_nr = transport->remote->fetch_refspec_nr;
+ fetch_refspec = transport->remote->fetch.rs;
+ fetch_refspec_nr = transport->remote->fetch.nr;
}
for (i = 0; i < fetch_refspec_nr; i++)
@@ -374,16 +374,16 @@ static struct ref *get_ref_map(struct transport *transport,
struct branch *branch = branch_get(NULL);
int has_merge = branch_has_merge_config(branch);
if (remote &&
- (remote->fetch_refspec_nr ||
+ (remote->fetch.nr ||
/* Note: has_merge implies non-NULL branch->remote_name */
(has_merge && !strcmp(branch->remote_name, remote->name)))) {
- for (i = 0; i < remote->fetch_refspec_nr; i++) {
- get_fetch_map(remote_refs, &remote->fetch[i], &tail, 0);
- if (remote->fetch[i].dst &&
- remote->fetch[i].dst[0])
+ for (i = 0; i < remote->fetch.nr; i++) {
+ get_fetch_map(remote_refs, &remote->fetch.rs[i], &tail, 0);
+ if (remote->fetch.rs[i].dst &&
+ remote->fetch.rs[i].dst[0])
*autotags = 1;
if (!i && !has_merge && ref_map &&
- !remote->fetch[0].pattern)
+ !remote->fetch.rs[0].pattern)
ref_map->fetch_head_status = FETCH_HEAD_MERGE;
}
/*
@@ -1117,8 +1117,8 @@ static int do_fetch(struct transport *transport,
if (ref_count) {
prune_refs(refs, ref_count, ref_map, transport->url);
} else {
- prune_refs(transport->remote->fetch,
- transport->remote->fetch_refspec_nr,
+ prune_refs(transport->remote->fetch.rs,
+ transport->remote->fetch.nr,
ref_map,
transport->url);
}
diff --git a/builtin/remote.c b/builtin/remote.c
index 7f0072fe5..d61daa5e8 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -331,10 +331,10 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
struct ref *ref, *stale_refs;
int i;
- for (i = 0; i < states->remote->fetch_refspec_nr; i++)
- if (get_fetch_map(remote_refs, states->remote->fetch + i, &tail, 1))
+ for (i = 0; i < states->remote->fetch.nr; i++)
+ if (get_fetch_map(remote_refs, states->remote->fetch.rs + i, &tail, 1))
die(_("Could not get fetch map for refspec %s"),
- refspec_to_string(&states->remote->fetch[i]));
+ refspec_to_string(&states->remote->fetch.rs[i]));
states->new.strdup_strings = 1;
states->tracked.strdup_strings = 1;
@@ -345,8 +345,8 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
else
string_list_append(&states->tracked, abbrev_branch(ref->name));
}
- stale_refs = get_stale_heads(states->remote->fetch,
- states->remote->fetch_refspec_nr, fetch_map);
+ stale_refs = get_stale_heads(states->remote->fetch.rs,
+ states->remote->fetch.nr, fetch_map);
for (ref = stale_refs; ref; ref = ref->next) {
struct string_list_item *item =
string_list_append(&states->stale, abbrev_branch(ref->name));
@@ -591,8 +591,8 @@ static int migrate_file(struct remote *remote)
}
strbuf_reset(&buf);
strbuf_addf(&buf, "remote.%s.fetch", remote->name);
- for (i = 0; i < remote->fetch_refspec_nr; i++) {
- strbuf_add_refspec(&refspec, &remote->fetch[i]);
+ for (i = 0; i < remote->fetch.nr; i++) {
+ strbuf_add_refspec(&refspec, &remote->fetch.rs[i]);
git_config_set_multivar(buf.buf, refspec.buf, "^$", 0);
strbuf_reset(&refspec);
}
@@ -651,11 +651,11 @@ static int mv(int argc, const char **argv)
strbuf_addf(&buf, "remote.%s.fetch", rename.new);
git_config_set_multivar(buf.buf, NULL, NULL, 1);
strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
- for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
+ for (i = 0; i < oldremote->fetch.nr; i++) {
char *ptr;
strbuf_reset(&buf2);
- strbuf_add_refspec(&buf2, &oldremote->fetch[i]);
+ strbuf_add_refspec(&buf2, &oldremote->fetch.rs[i]);
ptr = strstr(buf2.buf, old_remote_context.buf);
if (ptr) {
refspec_updated = 1;
diff --git a/remote.c b/remote.c
index fc1d3cf7a..952000b61 100644
--- a/remote.c
+++ b/remote.c
@@ -94,14 +94,6 @@ static void add_push_refspec(struct remote *remote, const char *ref)
remote->push_refspec[remote->push_refspec_nr++] = strdup(ref);
}
-static void add_fetch_refspec(struct remote *remote, const char *ref)
-{
- ALLOC_GROW(remote->fetch_refspec,
- remote->fetch_refspec_nr + 1,
- remote->fetch_refspec_alloc);
- remote->fetch_refspec[remote->fetch_refspec_nr++] = strdup(ref);
-}
-
static void add_url(struct remote *remote, const char *url)
{
ALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);
@@ -267,7 +259,7 @@ static void read_remotes_file(struct remote *remote)
else if (skip_prefix(buf.buf, "Push:", &v))
add_push_refspec(remote, skip_spaces(v));
else if (skip_prefix(buf.buf, "Pull:", &v))
- add_fetch_refspec(remote, skip_spaces(v));
+ add_fetch_refspec(remote, skip_spaces(v), 1);
}
strbuf_release(&buf);
fclose(f);
@@ -308,7 +300,7 @@ static void read_branches_file(struct remote *remote)
add_url_alias(remote, strbuf_detach(&buf, NULL));
strbuf_addf(&buf, "refs/heads/%s:refs/heads/%s", frag, remote->name);
- add_fetch_refspec(remote, buf.buf);
+ add_fetch_refspec(remote, buf.buf, 1);
strbuf_reset(&buf);
/*
@@ -408,7 +400,7 @@ static int handle_config(const char *key, const char *value, void *cb)
const char *v;
if (git_config_string(&v, key, value))
return -1;
- add_fetch_refspec(remote, v);
+ add_fetch_refspec(remote, v, 1);
free((char*)v);
} else if (!strcmp(subkey, "receivepack")) {
const char *v;
@@ -630,17 +622,28 @@ struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec)
return parse_refspec_internal(nr_refspec, refspec, 1, 0);
}
-void add_and_parse_fetch_refspec(struct remote *remote, const char *refspec)
+static int add_refspec(struct remote *remote, const char *refspec,
+ int fetch, int gently)
{
- struct refspec *rs;
+ struct refspec_array *rsa = fetch ? &remote->fetch : NULL;
- add_fetch_refspec(remote, refspec);
- rs = parse_fetch_refspec(1, &refspec);
- REALLOC_ARRAY(remote->fetch, remote->fetch_refspec_nr);
- remote->fetch[remote->fetch_refspec_nr - 1] = *rs;
+ ALLOC_GROW(rsa->rs, rsa->nr + 1, rsa->alloc);
- /* Not free_refspecs(), as we copied its pointers above */
- free(rs);
+ if (parse_one_refspec(&rsa->rs[rsa->nr], refspec, fetch, gently) < 0) {
+ if (gently) {
+ if (!remote->bogus_refspec)
+ remote->bogus_refspec = strdup(refspec);
+ return -1;
+ } else
+ die("Invalid refspec: '%s'", refspec);
+ }
+ rsa->nr++;
+ return 0;
+}
+
+int add_fetch_refspec(struct remote *remote, const char *refspec, int gently)
+{
+ return add_refspec(remote, refspec, 1, gently);
}
struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
@@ -725,7 +728,8 @@ static struct remote *remote_get_1(const char *name,
add_url_alias(ret, name);
if (!valid_remote(ret))
return NULL;
- ret->fetch = parse_fetch_refspec(ret->fetch_refspec_nr, ret->fetch_refspec);
+ if (ret->bogus_refspec)
+ die("Invalid refspec '%s'", ret->bogus_refspec);
ret->push = parse_push_refspec(ret->push_refspec_nr, ret->push_refspec);
return ret;
}
@@ -757,9 +761,8 @@ int for_each_remote(each_remote_fn fn, void *priv)
struct remote *r = remotes[i];
if (!r)
continue;
- if (!r->fetch)
- r->fetch = parse_fetch_refspec(r->fetch_refspec_nr,
- r->fetch_refspec);
+ if (r->bogus_refspec)
+ die("Invalid refspec '%s'", r->bogus_refspec);
if (!r->push)
r->push = parse_push_refspec(r->push_refspec_nr,
r->push_refspec);
@@ -961,7 +964,7 @@ char *refspec_to_string(const struct refspec *refspec)
int remote_find_tracking(struct remote *remote, struct refspec *refspec)
{
- return query_refspecs(remote->fetch, remote->fetch_refspec_nr, refspec);
+ return query_refspecs(remote->fetch.rs, remote->fetch.nr, refspec);
}
static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
@@ -1756,7 +1759,7 @@ static const char *tracking_for_push_dest(struct remote *remote,
{
char *ret;
- ret = apply_refspecs(remote->fetch, remote->fetch_refspec_nr, refname);
+ ret = apply_refspecs(remote->fetch.rs, remote->fetch.nr, refname);
if (!ret)
return error_buf(err,
_("push destination '%s' on remote '%s' has no local tracking branch"),
@@ -2373,7 +2376,7 @@ static int remote_tracking(struct remote *remote, const char *refname,
{
char *dst;
- dst = apply_refspecs(remote->fetch, remote->fetch_refspec_nr, refname);
+ dst = apply_refspecs(remote->fetch.rs, remote->fetch.nr, refname);
if (!dst)
return -1; /* no tracking ref for refname at remote */
if (read_ref(dst, oid->hash))
diff --git a/remote.h b/remote.h
index ee6c432d0..416a08501 100644
--- a/remote.h
+++ b/remote.h
@@ -11,6 +11,11 @@ enum {
REMOTE_BRANCHES
};
+struct refspec_array {
+ struct refspec *rs;
+ int nr, alloc;
+};
+
struct remote {
struct hashmap_entry ent; /* must be first */
@@ -32,10 +37,10 @@ struct remote {
int push_refspec_nr;
int push_refspec_alloc;
- const char **fetch_refspec;
- struct refspec *fetch;
- int fetch_refspec_nr;
- int fetch_refspec_alloc;
+ struct refspec_array fetch;
+
+ /* Copy of the first bogus fetch refspec we couldn't parse */
+ const char *bogus_refspec;
/*
* -1 to never fetch tags
@@ -169,7 +174,15 @@ struct ref *ref_remove_duplicates(struct ref *ref_map);
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
-void add_and_parse_fetch_refspec(struct remote *remote, const char *refspec);
+/*
+ * Add a fetch refspec to a remote.
+ * If the refspec cannot be parsed successfully and
+ * - gently=0: die().
+ * - gently=1: store the refspec in remote->bogus_refspec and return with -1.
+ * If there are more than one bogus refspecs in the same remote,
+ * then only the first one will be stored.
+ */
+int add_fetch_refspec(struct remote *remote, const char *refspec, int gently);
extern struct refspec *parse_push_refspec(int nr_refspec, const char **refspec);
void free_refspec(int nr_refspec, struct refspec *refspec);
--
2.13.1.505.g7cc9fcafb