Re: [PATCH 14/35] remote: convert fetch refspecs to struct refspec

[Brandon Williams <bmwill@google.com>]

On 05/15, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, May 14 2018, Brandon Williams wrote:
> 
> >  void add_prune_tags_to_fetch_refspec(struct remote *remote)
> >  {
> > -	int nr = remote->fetch_refspec_nr;
> > -	int bufsize = nr  + 1;
> > -	int size = sizeof(struct refspec_item);
> > -
> > -	remote->fetch = xrealloc(remote->fetch, size  * bufsize);
> > -	memcpy(&remote->fetch[nr], tag_refspec, size);
> > -	add_fetch_refspec(remote, xstrdup(TAG_REFSPEC));
> > +	refspec_append(&remote->fetch, TAG_REFSPEC);
> >  }
> 
> Thanks for fixing the hack I needed to put in place in 97716d217c
> ("fetch: add a --prune-tags option and fetch.pruneTags config",
> 2018-02-09).
> 
> I'm not sure where it belongs in this series, but I think this makes
> sense on top of the whole thing:

This actually would work well immediately after this patch, so I'll add
it here :)

Thanks!

> 
>     diff --git a/builtin/fetch.c b/builtin/fetch.c
>     index af7064dce3..9a523249f5 100644
>     --- a/builtin/fetch.c
>     +++ b/builtin/fetch.c
>     @@ -1383,7 +1383,8 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, int pru
> 
>             maybe_prune_tags = prune_tags_ok && prune_tags;
>             if (maybe_prune_tags && remote_via_config)
>     -               add_prune_tags_to_fetch_refspec(remote);
>     +               refspec_append(&remote->fetch, TAG_REFSPEC);
>     +
> 
>             if (maybe_prune_tags && (argc || !remote_via_config))
>                     refspec_append(&rs, TAG_REFSPEC);
>     diff --git a/remote.c b/remote.c
>     index 8e6522f4d0..946b95d18d 100644
>     --- a/remote.c
>     +++ b/remote.c
>     @@ -87,11 +87,6 @@ static void add_fetch_refspec(struct remote *remote, const char *ref)
>             refspec_append(&remote->fetch, ref);
>      }
> 
>     -void add_prune_tags_to_fetch_refspec(struct remote *remote)
>     -{
>     -       refspec_append(&remote->fetch, TAG_REFSPEC);
>     -}
>     -
>      static void add_url(struct remote *remote, const char *url)
>      {
>             ALLOC_GROW(remote->url, remote->url_nr + 1, remote->url_alloc);
>     diff --git a/remote.h b/remote.h
>     index 9014f707f0..62a6566594 100644
>     --- a/remote.h
>     +++ b/remote.h
>     @@ -289,6 +289,4 @@ extern int parseopt_push_cas_option(const struct option *, const char *arg, int
>      extern int is_empty_cas(const struct push_cas_option *);
>      void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);
> 
>     -void add_prune_tags_to_fetch_refspec(struct remote *remote);
>     -
>      #endif
> 
> I.e. the whole reason we have this function is because of my above
> commit where I had to very carefully hack around the fact that we didn't
> have something which could ALLOW_GROW() the structure after it had been
> created.
> 
> So I added the add_prune_tags_to_fetch_refspec() function to very
> carefully do *only* that so others wouldn't be tempted to use this hack
> more generally.
> 
> But now we have a nice API for it, so we can just throw away the
> wrapper, and use the same API everywhere. You already did the other half
> of that in your e69b54f53a ("fetch: convert fetch_one to use struct
> refspec", 2018-05-11).

-- 
Brandon Williams