On Mon, Aug 17, 2020 at 4:43 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Jacob Keller <jacob.e.keller@xxxxxxxxx> writes:
>
> > Refspecs today are commutative, meaning that order doesn't expressly
> > matter. Rather than forcing an implied order, negative refspecs will
> > always be applied last. That is, in order to match, a ref must match at
> > least one positive refspec, and match none of the negative refspecs.
> > This is similar to how negative pathspecs work.
>
> Yes, enumerate what positive ones match and then exclude what
> negative ones match from the result is a time-tested pattern our
> users know how things work.
>
> > @@ -530,6 +530,9 @@ static struct ref *get_ref_map(struct remote *remote,
> > tail = &rm->next;
> > }
> >
> > + /* apply any negative refspecs now to prune the list of refs */
> > + ref_map = apply_negative_refspecs(ref_map, rs);
> > +
> > ref_map = ref_remove_duplicates(ref_map);
>
> How was the ordering here decided? Should it result the same set if
> negative ones are excluded after duplicates are removed?
>
Good question. This was what was done in peff's original patch. I need
to understand a bit more about what ref_remove_duplicates does to
really figure this out.
> > @@ -1441,6 +1445,8 @@ int match_push_refs(struct ref *src, struct ref **dst,
> > string_list_clear(&src_ref_index, 0);
> > }
> >
> > + *dst = apply_negative_refspecs(*dst, rs);
> > +
>
> The block of code whose tail is shown in the pre-context has
> prepared "delete these refs because we no longer have them" to the
> other side under MATCH_REFS_PRUNE but that was done based on the
> *dst list before we applied the negative refspec. Is the ordering
> of these two correct, or should we filter the dst list with negative
> ones and use the resulting one in pruning operation?
>
I think we need to swap the order here. I'll take a closer look.
> > + 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.
> > + */
>
> "a ref (or pattern of refs)" is clarified with the next sentence
> anyway, so let's not say it, e.g.
>
> ... only have a LHS, which indicates what to exclude from
> other matches.
>
Sure. There's also a slight bug here because in "fetch" mode,
standalone LHS-only refs cannot be globs, and I need to fix that too.
Thanks,
Jake
