Re: [PATCH v3 05/27] refspec: remove global tag refspec structure

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> We have a global tag refspec structure that is used by both git-clone(1)
> and git-fetch(1). Initialization fo the structure will break once we

"fo" -> "of".

> enable `-Wwrite-strings`, even though the breakage is harmless. While we
> could just add casts, the structure isn't really required in the first
> place as we can simply initialize the structures at the respective
> callsites.
>
> Refactor the code accordingly.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/clone.c |  8 ++++++--
>  builtin/fetch.c | 11 ++++++++---
>  refspec.c       | 13 -------------
>  refspec.h       |  1 -
>  4 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 92ab7d7165..bde1d284a2 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -523,6 +523,9 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
>  	struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
>  	struct ref *local_refs = head;
>  	struct ref **tail = head ? &head->next : &local_refs;
> +	struct refspec_item tag_refspec;
> +
> +	refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
>  
>  	if (option_single_branch) {
>  		struct ref *remote_head = NULL;
> @@ -545,7 +548,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
>  					      &tail, 0);
>  
>  			/* if --branch=tag, pull the requested tag explicitly */
> -			get_fetch_map(remote_head, tag_refspec, &tail, 0);
> +			get_fetch_map(remote_head, &tag_refspec, &tail, 0);
>  		}
>  		free_refs(remote_head);
>  	} else {
> @@ -555,8 +558,9 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
>  	}
>  
>  	if (!option_mirror && !option_single_branch && !option_no_tags)
> -		get_fetch_map(refs, tag_refspec, &tail, 0);
> +		get_fetch_map(refs, &tag_refspec, &tail, 0);
>  
> +	refspec_item_clear(&tag_refspec);
>  	return local_refs;
>  }
>  
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 75255dc600..06b60867f5 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -582,11 +582,16 @@ static struct ref *get_ref_map(struct remote *remote,
>  		}
>  	}
>  
> -	if (tags == TAGS_SET)
> +	if (tags == TAGS_SET) {
> +		struct refspec_item tag_refspec;
> +
>  		/* also fetch all tags */
> -		get_fetch_map(remote_refs, tag_refspec, &tail, 0);
> -	else if (tags == TAGS_DEFAULT && *autotags)
> +		refspec_item_init(&tag_refspec, TAG_REFSPEC, 0);
> +		get_fetch_map(remote_refs, &tag_refspec, &tail, 0);
> +		refspec_item_clear(&tag_refspec);
> +	} else if (tags == TAGS_DEFAULT && *autotags) {
>  		find_non_local_tags(remote_refs, NULL, &ref_map, &tail);
> +	}
>  
>  	/* Now append any refs to be updated opportunistically: */
>  	*tail = orefs;
> diff --git a/refspec.c b/refspec.c
> index d60932f4de..1df5de6c2f 100644
> --- a/refspec.c
> +++ b/refspec.c
> @@ -7,19 +7,6 @@
>  #include "refspec.h"
>  #include "strbuf.h"
>  
> -static struct refspec_item s_tag_refspec = {
> -	.force = 0,
> -	.pattern = 1,
> -	.matching = 0,
> -	.exact_sha1 = 0,
> -	.negative = 0,
> -	.src = "refs/tags/*",
> -	.dst = "refs/tags/*",
> -};
> -
> -/* See TAG_REFSPEC for the string version */
> -const struct refspec_item *tag_refspec = &s_tag_refspec;
> -
>  /*
>   * Parses the provided refspec 'refspec' and populates the refspec_item 'item'.
>   * Returns 1 if successful and 0 if the refspec is invalid.
> diff --git a/refspec.h b/refspec.h
> index 8c0c446993..754be45cee 100644
> --- a/refspec.h
> +++ b/refspec.h
> @@ -2,7 +2,6 @@
>  #define REFSPEC_H
>  
>  #define TAG_REFSPEC "refs/tags/*:refs/tags/*"
> -extern const struct refspec_item *tag_refspec;
>  
>  /**
>   * A struct refspec_item holds the parsed interpretation of a refspec.  If it