Re: [RFC v2 1/1] refspec: add support for negative refspecs
From: Junio C Hamano <hidden>
Date: 2020-09-18 00:02:03
Jacob Keller [off-list ref] writes:
quoted hunk ↗ jump to hunk
@@ -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;quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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.
quoted hunk ↗ jump to hunk
@@ -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)?
quoted hunk ↗ jump to hunk
@@ -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.