git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jacob Keller <jacob.e.keller@intel.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: Re: [RFC v2 1/1] refspec: add support for negative refspecs
Date: Thu, 17 Sep 2020 17:01:58 -0700	[thread overview]
Message-ID: <xmqqzh5onea1.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200821215247.758978-2-jacob.e.keller@intel.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.

  parent reply	other threads:[~2020-09-18  0:02 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-21 21:52 [RFC v2 0/1] implement " Jacob Keller
2020-08-21 21:52 ` [RFC v2 1/1] refspec: add " Jacob Keller
2020-08-22 13:29   ` Johannes Schindelin
2020-08-24 15:47     ` Jacob Keller
2020-08-24 17:55     ` Junio C Hamano
2020-08-24 19:26       ` Jacob Keller
2020-09-17 20:21   ` Junio C Hamano
2020-09-18  0:01   ` Junio C Hamano [this message]
2020-09-24 23:33     ` Jacob Keller
2020-09-24 23:42     ` Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzh5onea1.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jacob.keller@gmail.com \
    --cc=peff@peff.net \
    --subject='Re: [RFC v2 1/1] refspec: add support for negative refspecs' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).