All of lore.kernel.org
 help / color / mirror / Atom feed
From: ZheNing Hu <adlternative@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH 1/2] ref-filter: hacky "streaming" mode
Date: Mon, 20 Sep 2021 15:42:17 +0800	[thread overview]
Message-ID: <CAOLTT8QxZ3rMwrgD3LkqAUb98gdyVC28=b4SQ5xKwGteFvwOFA@mail.gmail.com> (raw)
In-Reply-To: <YUO63qy2/5wibY4/@coredump.intra.peff.net>

Jeff King <peff@peff.net> 于2021年9月17日周五 上午5:45写道:
>
> On Wed, Sep 15, 2021 at 10:23:43PM +0800, ZheNing Hu wrote:
>
> > ZheNing Hu <adlternative@gmail.com> 于2021年9月15日周三 下午8:27写道:
> > >
> > > > So yes, it's complicated. And it must be explained to the user that
> > > > "%(refname)" behaves slightly differently with "git tag --verify", but
> > > > that is unavoidable if we do not want to break scripts (it _already_
> > > > behaves slightly differently, and we just never told anyone).
> > > >
> >
> > $ git tag --verify --format='verify: %(refname) %(symref)' annotated symref
> > verify: annotated
> > verify: symref
> > $ git tag --verify --format='verify: %(refname) %(symref)'
> > refs/tags/annotated refs/tags/symref
> > error: tag 'refs/tags/annotated' not found.
> > error: tag 'refs/tags/symref' not found.
>
> This is expected. When you provide a tag name on the command line of
> "git tag" it is assumed to be a non-qualified name in refs/tags/ (and
> ditto for git-branch and refs/heads/). It is tempting to try to be
> friendly and accept fully-qualified refs there, but it would create
> ambiguities (e.g., you could really have refs/tags/refs/tags/foo as a
> ref).
>

Yeah, maybe you are right, for git tag --verify, there may have ambiguities.
But for git verify-tag, if we have tags like "refs/tags/refs/tags/foo" and
"refs/tags/foo":

$ git verify-tag --format='verify: %(refname) %(symref)' refs/tags/foo
warning: refname 'refs/tags/foo' is ambiguous.

We see the ambiguities too here.

> I think we can ignore that for our purposes here, though. It's a
> question of input from the command-line, and we focus on just the output
> that we produce.
>

Yeah, but using different functions (read_ref_full(), get_oid()) will
affect what
kind of input we can provide.

> > $ git verify-tag --format='verify: %(refname) %(symref)' annotated
> > symref
> > verify: annotated
> > verify: symref
> > $ git verify-tag --format='verify: %(refname) %(symref)'
> > refs/tags/annotated refs/tags/symref
> > verify: refs/tags/annotated
> > verify: refs/tags/symref
> >
> > As we can see, there is a slight difference between git tag --verify and
> > git verify-tag: git tag --verify can not handle refs' fullname refs/tags/*
> > (because read_ref_full() | read_ref() can't handle them). So, as a standard,
> > which characteristics should we keep?
>
> Whereas are you notice here, verify-tag takes any name (which could be
> fully qualified), and uses it as-is. In fact, it might not even be a ref
> at all! You can say "git verify-tag c06b72d02" if you want to. And as a
> plumbing tool, we should make sure this continues to work. For example,
> careful scripts may resolve a ref into an object, and want to continue
> talking about that object without worrying about the ref being changed
> simultaneously.
>

Yes, this feature is very bad. %(refname) seems to do the %(objectname)
work.


> But it also creates a weirdness for "git verify-tag --format". We do not
> necessarily even have a ref to show. So IMHO the feature is somewhat
> mis-designed in the first place. But we should probably continue to
> support it as best we can.
>
> The best I can come up with is:
>
>   - when we resolve the name, if it was a ref, we should record that.
>     I think this is hard to do now. It would probably require
>     get_oid_with_context() learning to report on the results it got from
>     dwim_ref().
>
>   - if we have a refname, then feed it to pretty_print_ref() as a
>     fully-qualified name. And pass whatever "default lstrip=2" magic we
>     come up with for "git tag --verify". That would mean that "git
>     verify-tag --format=%(refname) v2.33.0" would behave the same before
>     and after.
>
>   - if we didn't get a refname, then...I guess continue to pass the name
>     the user gave us into pretty_print_ref()? That would keep "git
>     verify-tag --format=%(refname) c06b72d02" working as it does today.
>
> The alternative is to do none of those things, and just document that
> "verify-tag" is weird:
>
>   - its %(refname) reports whatever you gave it, whether it is a ref or
>     not
>
>   - some advanced format placeholders like %(symref) may not work if you
>     don't pass a fully-qualified ref
>
> -Peff

This is my solution according to your above suggection:
https://lore.kernel.org/git/pull.1042.git.1632123476.gitgitgadget@gmail.com/

Thanks.
--
ZheNing Hu

  reply	other threads:[~2021-09-20  7:42 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-04 12:40 [hacky PATCH 0/2] speeding up trivial for-each-ref invocations Jeff King
2021-09-04 12:41 ` [PATCH 1/2] ref-filter: hacky "streaming" mode Jeff King
2021-09-05  8:20   ` ZheNing Hu
2021-09-05 13:04     ` Jeff King
2021-09-07  5:28       ` ZheNing Hu
2021-09-07 18:01         ` Jeff King
2021-09-09 14:45           ` ZheNing Hu
2021-09-10 14:26             ` Jeff King
2021-09-15 12:27               ` ZheNing Hu
2021-09-15 14:23                 ` ZheNing Hu
2021-09-16 21:45                   ` Jeff King
2021-09-20  7:42                     ` ZheNing Hu [this message]
2021-09-16 21:31                 ` Jeff King
2021-09-05 13:15     ` Jeff King
2021-09-07  5:42       ` ZheNing Hu
2021-09-04 12:42 ` [PATCH 2/2] ref-filter: implement "quick" formats Jeff King
2021-09-05  8:20   ` ZheNing Hu
2021-09-05 13:07     ` Jeff King
2021-09-06 13:34       ` ZheNing Hu
2021-09-07 20:06       ` Junio C Hamano
2021-09-05  8:19 ` [hacky PATCH 0/2] speeding up trivial for-each-ref invocations ZheNing Hu
2021-09-05 12:49   ` Jeff King
2021-09-06 13:30     ` ZheNing Hu
2021-09-07 17:28       ` Jeff King
2021-09-09 13:20         ` ZheNing Hu
2021-09-06  6:54 ` Patrick Steinhardt

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

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

  git send-email \
    --in-reply-to='CAOLTT8QxZ3rMwrgD3LkqAUb98gdyVC28=b4SQ5xKwGteFvwOFA@mail.gmail.com' \
    --to=adlternative@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.