Re: [RFC v2 1/1] refspec: add support for negative refspecs

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

Jacob Keller <jacob.e.keller@intel.com> writes:

> @@ -66,6 +74,28 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
>  	item->src = xstrndup(lhs, llen);
>  	flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
>  
> +	if (item->negative) {
> +		struct object_id unused;
> +
> +		/*
> +		 * Negative refspecs only have a LHS, which indicates a ref
> +		 * (or pattern of refs) to exclude from other matches. This
> +		 * can either be a simple ref, a glob pattern, or even an
> +		 * exact sha1 match.
> +		 */
> +		if (!*item->src)
> +			return 0; /* negative refspecs must not be empty */
> +		else if (llen == the_hash_algo->hexsz && !get_oid_hex(item->src, &unused))
> +			item->exact_sha1 = 1; /* ok */
> +		else if (!check_refname_format(item->src, flags))
> +			; /* valid looking ref is ok */
> +		else
> +			return 0;
> +
> +		/* other rules for negative refspecs don't apply */

This comment confused me a bit; did you mean "other rules don't
apply to negative refspecs"?

> +		return 1;
> +	}
> +
>  	if (fetch) {
>  		struct object_id unused;


> diff --git a/remote.c b/remote.c
> index c5ed74f91c63..2f583d72c3f0 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1058,7 +1172,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
>  	const char *dst_value = rs->dst;
>  	char *dst_guess;
>  
> -	if (rs->pattern || rs->matching)
> +	if (rs->pattern || rs->matching || rs->negative)
>  		return 0;

OK.  These "special" ones do not participate in explicit matching.

> @@ -1134,6 +1248,10 @@ static char *get_ref_match(const struct refspec *rs, const struct ref *ref,
>  	int matching_refs = -1;
>  	for (i = 0; i < rs->nr; i++) {
>  		const struct refspec_item *item = &rs->items[i];
> +
> +		if (item->negative)
> +			continue;
> +

And a negative one does not decide if a ref being pushed will be
pushed out for real at this point.  This helper is only to enumerate
the candidate refs to be pushed out; the caller makes a separate
call to apply_negative_refspecs() to cull the candidate list later.

OK.

> @@ -1339,7 +1457,7 @@ int check_push_refs(struct ref *src, struct refspec *rs)
>  	for (i = 0; i < rs->nr; i++) {
>  		struct refspec_item *item = &rs->items[i];
>  
> -		if (item->pattern || item->matching)
> +		if (item->pattern || item->matching || item->negative)
>  			continue;
>  
>  		ret |= match_explicit_lhs(src, item, NULL, NULL);

match_explicit_lhs(), like match_explicit(), are for explicit
matching and should not be called for the "special" ones.  OK.

> @@ -1441,6 +1559,8 @@ int match_push_refs(struct ref *src, struct ref **dst,
>  		string_list_clear(&src_ref_index, 0);
>  	}
>  
> +	*dst = apply_negative_refspecs(*dst, rs);
> +
>  	if (errs)
>  		return -1;
>  	return 0;

And after grabbing all the candidate refs to be updated via this
push, we filter out the ones that match negative pattern.  Can it
also produce an error, or it can never fail (to udpate errs)?

> @@ -1810,6 +1930,9 @@ int get_fetch_map(const struct ref *remote_refs,
>  {
>  	struct ref *ref_map, **rmp;
>  
> +	if (refspec->negative)
> +		return 0;
> +

Again, the idea is to let the existing codepath to only deal with
the positive refspec elements to keep the same behaviour, and let
the caller filter the ones that match negative ones out of the
result.  So we return without anything here for negative one.

Nothing jumped out at me as being suspicious so far, other than that
the GNU "?<empty>:" thing needs to be fixed as pointed out by Dscho.

Thanks.