On 02/27/2014 10:00 AM, Carlos Martín Nieto wrote:
> From: Carlos Martín Nieto <c...@dwim.me>
>
> We need to consider that a remote-tracking branch may match more than
> one rhs of a fetch refspec. In such a case, it is not enough to stop at
> the first match but look at all of the matches in order to determine
> whether a head is stale.
>
> To this goal, introduce a variant of query_refspecs which returns all of
> the matching refspecs and loop over those answers to check for
> staleness.
>
> Signed-off-by: Carlos Martín Nieto <c...@elego.de>
> ---
>
> There is an unfortunate duplication of code here, as
> query_refspecs_multiple is mostly query_refspecs but we only care
> about the other side of matching refspecs and disregard the 'force'
> information which query_refspecs does want.
>
> I thought about putting both together via callbacks and having
> query_refspecs stop at the first one, but I'm not sure that it would
> make it easier to read or manage.
>
> remote.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
> t/t5510-fetch.sh | 2 +-
> 2 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 9f1a8aa..26140c7 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -821,6 +821,33 @@ static int match_name_with_pattern(const char *key,
> const char *name,
> return ret;
> }
>
> +static void query_refspecs_multiple(struct refspec *refs, int ref_count,
> struct refspec *query, struct string_list *results)
> +{
> + int i;
> + int find_src = !query->src;
> +
> + if (find_src && !query->dst)
> + error("query_refspecs_multiple: need either src or dst");
> +
> + for (i = 0; i < ref_count; i++) {
> + struct refspec *refspec = &refs[i];
> + const char *key = find_src ? refspec->dst : refspec->src;
> + const char *value = find_src ? refspec->src : refspec->dst;
> + const char *needle = find_src ? query->dst : query->src;
> + char **result = find_src ? &query->src : &query->dst;
> +
> + if (!refspec->dst)
> + continue;
> + if (refspec->pattern) {
> + if (match_name_with_pattern(key, needle, value,
> result)) {
> + string_list_append_nodup(results, *result);
> + }
> + } else if (!strcmp(needle, key)) {
> + string_list_append(results, value);
> + }
> + }
> +}
> +
> static int query_refspecs(struct refspec *refs, int ref_count, struct
> refspec *query)
> {
> int i;
> @@ -1954,25 +1981,40 @@ static int get_stale_heads_cb(const char *refname,
> const unsigned char *sha1, int flags, void *cb_data)
> {
> struct stale_heads_info *info = cb_data;
> + struct string_list matches = STRING_LIST_INIT_DUP;
> struct refspec query;
> + int i, stale = 1;
> memset(&query, 0, sizeof(struct refspec));
> query.dst = (char *)refname;
>
> - if (query_refspecs(info->refs, info->ref_count, &query))
> + query_refspecs_multiple(info->refs, info->ref_count, &query, &matches);
> + if (matches.nr == 0)
> return 0; /* No matches */
>
> /*
> * If we did find a suitable refspec and it's not a symref and
> * it's not in the list of refs that currently exist in that
> - * remote we consider it to be stale.
> + * remote we consider it to be stale. In order to deal with
> + * overlapping refspecs, we need to go over all of the
> + * matching refs.
> */
> - if (!((flags & REF_ISSYMREF) ||
> - string_list_has_string(info->ref_names, query.src))) {
> + if (flags & REF_ISSYMREF)
> + return 0;
> +
> + for (i = 0; i < matches.nr; i++) {
> + if (string_list_has_string(info->ref_names,
> matches.items[i].string)) {
> + stale = 0;
> + break;
> + }
> + }
> +
> + string_list_clear(&matches, 0);
> +
> + if (stale) {
> struct ref *ref = make_linked_ref(refname,
> &info->stale_refs_tail);
> hashcpy(ref->new_sha1, sha1);
> }
>
> - free(query.src);
> return 0;
> }
I didn't have time to review this fully, but I think you are missing
calls to string_list_clear(&matches) on a couple of code paths.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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
