git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jacob Keller <jacob.keller@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	Git mailing list <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [RFC 3/3] refspec: add support for negative refspecs
Date: Mon, 17 Aug 2020 17:04:00 -0700	[thread overview]
Message-ID: <CA+P7+xpcm51cLPDDW+F1J-XZ2VvwNDWjnZqm54f3DKXxDfBF5Q@mail.gmail.com> (raw)
In-Reply-To: <xmqq5z9gzvmb.fsf@gitster.c.googlers.com>

On Mon, Aug 17, 2020 at 4:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.e.keller@intel.com> 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

  reply	other threads:[~2020-08-18  0:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-15  0:25 [RFC 1/3] refspec: fix documentation referring to refspec_item Jacob Keller
2020-08-15  0:25 ` [RFC 2/3] refspec: make sure stack refspec_item variables are zeroed Jacob Keller
2020-08-17 16:33   ` Junio C Hamano
2020-08-17 16:49     ` Jacob Keller
2020-08-15  0:25 ` [RFC 3/3] refspec: add support for negative refspecs Jacob Keller
2020-08-17 18:02   ` Jacob Keller
2020-08-17 23:43   ` Junio C Hamano
2020-08-18  0:04     ` Jacob Keller [this message]
2020-08-18 17:41       ` Jeff King
2020-08-20 23:59         ` Jacob Keller
2020-08-21  2:33           ` Jeff King
2020-08-21 16:19             ` Junio C Hamano
2020-08-21 16:28               ` Jacob Keller
2020-08-21 17:16         ` Jacob Keller
2020-08-21 17:26           ` Jacob Keller
2020-08-21 18:21             ` Jacob Keller
2020-08-21 18:59               ` Jeff King
2020-08-17 16:18 ` [RFC 1/3] refspec: fix documentation referring to refspec_item Junio C Hamano
2020-08-21 21:17   ` Jacob Keller
2020-08-21 21:41     ` Junio C Hamano

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=CA+P7+xpcm51cLPDDW+F1J-XZ2VvwNDWjnZqm54f3DKXxDfBF5Q@mail.gmail.com \
    --to=jacob.keller@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.e.keller@intel.com \
    --cc=peff@peff.net \
    --subject='Re: [RFC 3/3] 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).