git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Fully peel tags via for-each-ref?
@ 2019-08-19 23:22 Bryan Turner
  2019-08-20  5:50 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Bryan Turner @ 2019-08-19 23:22 UTC (permalink / raw)
  To: Git Users

When running "git for-each-ref", it's possible to use %(*objectname)
to get the SHA of the object a tag is tagging, and %(*objecttype) to
get the type. However, in the case of an annotated tag of an annotated
tag, this still isn't "fully" peeled--%(*objectname) is the tagged
tag's SHA.

Is there any way, with "git for-each-ref", to output the "fully"
peeled SHA of a tag's ultimate target, regardless of how many layers
must be traversed?

Best regards,
Bryan Turner

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fully peel tags via for-each-ref?
  2019-08-19 23:22 Fully peel tags via for-each-ref? Bryan Turner
@ 2019-08-20  5:50 ` Junio C Hamano
  2019-08-20 21:39   ` Bryan Turner
  2019-08-21 23:00   ` Jeff King
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2019-08-20  5:50 UTC (permalink / raw)
  To: Bryan Turner; +Cc: Git Users

Bryan Turner <bturner@atlassian.com> writes:

> Is there any way, with "git for-each-ref", to output the "fully"
> peeled SHA of a tag's ultimate target, regardless of how many layers
> must be traversed?

I do not think I wrote it to allow different degree of peeling, not
because I wanted to explicitly forbid a use case for tags that tag
another tag, but simply because I didn't think of anybody using it
and didn't see need to support such tags.

If %(*<stuff>) does not peel fully (I do not recall what I did
offhand), because all other things in Git (like $X~0, $X^{tree},
etc.) fully peel the outer object until they get to what they want,
it may even be OK to declare it a bug and "fix" the notation to
fully peel tags.  I dunno.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fully peel tags via for-each-ref?
  2019-08-20  5:50 ` Junio C Hamano
@ 2019-08-20 21:39   ` Bryan Turner
  2019-08-21 23:00   ` Jeff King
  1 sibling, 0 replies; 6+ messages in thread
From: Bryan Turner @ 2019-08-20 21:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Users

On Mon, Aug 19, 2019 at 10:50 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Bryan Turner <bturner@atlassian.com> writes:
>
> > Is there any way, with "git for-each-ref", to output the "fully"
> > peeled SHA of a tag's ultimate target, regardless of how many layers
> > must be traversed?
>
>
> If %(*<stuff>) does not peel fully (I do not recall what I did
> offhand), because all other things in Git (like $X~0, $X^{tree},
> etc.) fully peel the outer object until they get to what they want,
> it may even be OK to declare it a bug and "fix" the notation to
> fully peel tags.  I dunno.

%(*<stuff>) definitely doesn't fully peel.

$ git for-each-ref --format="%(refname) %(objectname) ->
%(*objectname) (%(*objecttype))" refs/tags/
refs/tags/backdated_annotated_tag
80aa5be5ecf39660f798858482254f7a2ae9110e ->
57150c54c38d6570b2fd5e6d6cfc19476de44e84 (commit)
refs/tags/retagged_signed_tag 9b4e781dea0769888fe270e06ad76675f73851b1
-> 12ebe2a58367347cd39f19f5a72f3cbec7b8f9a9 (tag)
refs/tags/signed_tag 12ebe2a58367347cd39f19f5a72f3cbec7b8f9a9 ->
0a943a29376f2336b78312d99e65da17048951db (commit)

"retagged_signed_tag" is tagging "signed_tag", and you can see that
%(*objectname) for it is "signed_tag"'s hash and %(*objecttype) is
tag.

$ git rev-parse retagged_signed_tag~0
0a943a29376f2336b78312d99e65da17048951db

I don't know that this use case is common enough for it to warrant
"fixing". I'm not even sure most would call it a "bug"; I hadn't
considered it that way, at least. I was just interested in whether
there might be an out-of-the-box approach I wasn't aware of for
getting fully peeled results.

Thanks for taking the time to answer, and to provide a bit of history!

Best regards,
Bryan Turner

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fully peel tags via for-each-ref?
  2019-08-20  5:50 ` Junio C Hamano
  2019-08-20 21:39   ` Bryan Turner
@ 2019-08-21 23:00   ` Jeff King
  2019-08-22  3:07     ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff King @ 2019-08-21 23:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bryan Turner, Git Users

On Mon, Aug 19, 2019 at 10:50:24PM -0700, Junio C Hamano wrote:

> Bryan Turner <bturner@atlassian.com> writes:
> 
> > Is there any way, with "git for-each-ref", to output the "fully"
> > peeled SHA of a tag's ultimate target, regardless of how many layers
> > must be traversed?
> 
> I do not think I wrote it to allow different degree of peeling, not
> because I wanted to explicitly forbid a use case for tags that tag
> another tag, but simply because I didn't think of anybody using it
> and didn't see need to support such tags.
> 
> If %(*<stuff>) does not peel fully (I do not recall what I did
> offhand), because all other things in Git (like $X~0, $X^{tree},
> etc.) fully peel the outer object until they get to what they want,
> it may even be OK to declare it a bug and "fix" the notation to
> fully peel tags.  I dunno.

Yeah, my first thought on reading Bryan's email was that this is so
inconsistent with the rest of Git as to be considered a bug.

There's this gem in ref-filter.c, which blames back to your 9f613ddd21
(Add git-for-each-ref: helper for language bindings, 2006-09-15):

          /*
           * NEEDSWORK: This derefs tag only once, which
           * is good to deal with chains of trust, but
           * is not consistent with what deref_tag() does
           * which peels the onion to the core.
           */
          return get_object(ref, 1, &obj, &oi_deref, err);

Which isn't to say it isn't useful to be able to do a single-layer peel,
but I can't think of another part of the system which does so (unless
you've asked to peel to a specific type, of course). I'm thinking
especially of the way upload-pack does its advertisement, with two
interesting implications:

  1. We store full peels in the packed-refs file[1], so we can show them
     in the advertisement without having to access the object. Doing:

       git upload-pack . </dev/null >/dev/null

     is much cheaper than:

       git for-each-ref \
         --format='%(objectname) %(refname)%0a%(*objectname) %(refname)^{}' \
	 >/dev/null

  2. When we switched for_each_alternate_ref to using peeled values, we
     stopped advertising peeled values for .push haves entirely. There's
     some discussion in a10a17877b (for_each_alternate_ref: replace
     transport code with for-each-ref, 2017-02-08), but if we _did_ want
     to show them, we'd probably want the fully peeled value to match
     the rest of the advertisement.

Given that it's not how the rest of Git works, I'd be surprised for
anybody to be relying on the shallow-peeling behavior of for-each-ref.
But if we did want to retain it, it might be worth adding a separate
syntax for a full peel. "%(**objectname)" or something?

-Peff

[1] Storing peeled values in the packed-refs file is itself a pretty
    horrible hack. It's really a property of the object itself, and
    we're just piggy-backing on the ref storage to make this cached
    metadata easily accessible. It would be more appropriate in an
    object metadata storage format similar to the commit-graph file.
    And then of course we could store both full or partial peels,
    depending on the format.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fully peel tags via for-each-ref?
  2019-08-21 23:00   ` Jeff King
@ 2019-08-22  3:07     ` Junio C Hamano
  2019-08-22  4:24       ` Jeff King
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2019-08-22  3:07 UTC (permalink / raw)
  To: Jeff King; +Cc: Bryan Turner, Git Users

Jeff King <peff@peff.net> writes:

> There's this gem in ref-filter.c, which blames back to your 9f613ddd21
> (Add git-for-each-ref: helper for language bindings, 2006-09-15):
>
>           /*
>            * NEEDSWORK: This derefs tag only once, which
>            * is good to deal with chains of trust, but
>            * is not consistent with what deref_tag() does
>            * which peels the onion to the core.
>            */
>           return get_object(ref, 1, &obj, &oi_deref, err);
>
> Which isn't to say it isn't useful to be able to do a single-layer peel,
> but I can't think of another part of the system which does so (unless
> you've asked to peel to a specific type, of course).

Quite honestly, I think the "only once" behaviour outlived its
usefulness, without other "features" that may help make it more
useful.  To help a script that wants to do "chains of trust", it may
first appear to be useful to peel only one layer, revealing that the
tagged object is another tag, and that was the thinking behind the
NEEDSWORK comment.

But after we learn that a ref "refs/tags/foo" points at a tag object
that points at another tag object, what can the script do?  It
cannot feed the other tag found that way back into --format=%(*<thing>)
machinery of for-each-ref, as the command and its variants, the "--list"
mode of "branch" and "tag" commands, only work on the object at the
tip of refs.  The script must manually peel the tag one layer at a time.

And that "manually" has to be really manual.  No ^{<type>} suffix
allows scripts to peel just one layer, so inside a loop, the script
has to say "cat-file tag <object>" and parse the "object" header
from the output.

The only thing that gets affected if we changed %(*<thing>) to fully
peel tags is a tag that points at another tag, and the traditional
behaviour to peel only one layer, while it _might_ have been done as
a good first step to add more support for chain of trust, is not all
that useful for such a tag, I am not sure if the current behaviour
is defensible.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Fully peel tags via for-each-ref?
  2019-08-22  3:07     ` Junio C Hamano
@ 2019-08-22  4:24       ` Jeff King
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff King @ 2019-08-22  4:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Bryan Turner, Git Users

On Wed, Aug 21, 2019 at 08:07:29PM -0700, Junio C Hamano wrote:

> But after we learn that a ref "refs/tags/foo" points at a tag object
> that points at another tag object, what can the script do?  It
> cannot feed the other tag found that way back into --format=%(*<thing>)
> machinery of for-each-ref, as the command and its variants, the "--list"
> mode of "branch" and "tag" commands, only work on the object at the
> tip of refs.  The script must manually peel the tag one layer at a time.
> 
> And that "manually" has to be really manual.  No ^{<type>} suffix
> allows scripts to peel just one layer, so inside a loop, the script
> has to say "cat-file tag <object>" and parse the "object" header
> from the output.
> 
> The only thing that gets affected if we changed %(*<thing>) to fully
> peel tags is a tag that points at another tag, and the traditional
> behaviour to peel only one layer, while it _might_ have been done as
> a good first step to add more support for chain of trust, is not all
> that useful for such a tag, I am not sure if the current behaviour
> is defensible.

Yeah, well said, and I think this is the heart of the matter. There
isn't good tooling for this case, but even if there was, for-each-ref is
definitely not the place for it.

I took a brief look at making a patch, and it's unfortunately
non-trivial because of the way the deref logic in ref-filter is
implemented. So it may not be worth digging too far into for now if
nobody cares much either way.

I still have dreams of writing a more efficient and more reusable
version of ref-filter, and it would probably make sense to use
peel_ref() as part of that rewrite.

-Peff

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-08-22  4:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19 23:22 Fully peel tags via for-each-ref? Bryan Turner
2019-08-20  5:50 ` Junio C Hamano
2019-08-20 21:39   ` Bryan Turner
2019-08-21 23:00   ` Jeff King
2019-08-22  3:07     ` Junio C Hamano
2019-08-22  4:24       ` Jeff King

Code repositories for project(s) associated with this public 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).