git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/2] ref-filter: hacky "streaming" mode

From
Jeff King <peff@peff.net>
Date
Sep 10, 2021, 14:26 UTC
Message-ID
<YTtrF8C0mmT6kBJT@coredump.intra.peff.net>
In-Reply-To
<CAOLTT8Ru-Zhmo5j=jNjWexrahT0qAO5zEMW09XT00-TCca-SkA@mail.gmail.com>
On Thu, Sep 09, 2021 at 10:45:15PM +0800, ZheNing Hu wrote:
Show 9 quoted lines
> @@ -2735,6 +2723,7 @@ void pretty_print_ref(const char *name, const
> struct object_id *oid,
> 
>         ref_item = new_ref_array_item(name, oid);
>         ref_item->kind = ref_kind_from_refname(name);
> +       read_ref_full(name, 0, NULL, &ref_item->flag);
>         if (format_ref_array_item(ref_item, format, &output, &err))
>                 die("%s", err.buf);
>         fwrite(output.buf, 1, output.len, stdout);

IMHO this is the wrong place to do it, since the caller may already have the flags (and looking up the ref again is a non-trivial cost).

The caller in builtin/tag.c should switch to using read_ref_full() and pass in the flags.

The caller in builtin/verify-tag.c _probably_ should resolve the ref in the same way and pass in that full refname and flags. I do worry that this may be a compatibility problem, but the current behavior seems so broken to me.

Show 23 quoted lines
> >   - I suspect people may be relying on the current behavior. The
> >     original was added to be able to compare the internal tagname to the
> >     refname. I.e., that:
> >
> >       git tag -v --format='%(refname) %(tag)' foo
> >
> >     would show "foo foo". Now that _should_ be "%(refname:strip=2)", I
> >     think, but we'd probably be breaking scripts. OTOH, it really feels
> >     like _not_ handing over a real, fully-qualified refname to the
> >     ref-filter code will mean other things are broken (e.g.,
> >     ATOM_UPSTREAM is assuming we have a fully-qualified ref).
> >
> 
> This is indeed a sad thing: A bug becomes a feature.
> 
> >     I think a backwards-compatible way of fixing it would be to have
> >     this call hand over the full refname to the ref-filter code, but
> >     tell it that %(refname) should default to strip=2. And then anybody
> >     who wants the full name can use %(refname:strip=0).
> >
> 
> Doesn't this make things more complicated? Those callers of git for-each-ref,
> wouldn't our changes like this destroy them?

My proposal was that we'd have a specific flag in ref-filter to say "default refname:strip to this value". And then _only_ "tag --verify" would set that (to "2"), leaving for-each-ref, etc unaffected.

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).

The other option is to declare the current behavior a bug and fix it. I am quite tempted by that route, given the inconsistency with other formatters, including even "git tag --list --format=%(refname)"!

-Peff
Previous: ZheNing HuNext: ZheNing Hu
Message 8 of 26 in “speeding up trivial for-each-ref invocations”
  1. 0/2 speeding up trivial for-each-ref invocationsJeff King, Sep 4, 2021
  2. 1/2 ref-filter: hacky "streaming" modeJeff King, Sep 4, 2021
  3. ZheNing HuSep 5, 2021
  4. Jeff KingSep 5, 2021
  5. ZheNing HuSep 7, 2021
  6. Jeff KingSep 7, 2021
  7. ZheNing HuSep 9, 2021
  8. Jeff KingSep 10, 2021
  9. ZheNing HuSep 15, 2021
  10. ZheNing HuSep 15, 2021
  11. Jeff KingSep 16, 2021
  12. ZheNing HuSep 20, 2021
  13. Jeff KingSep 16, 2021
  14. Jeff KingSep 5, 2021
  15. ZheNing HuSep 7, 2021
  16. 2/2 ref-filter: implement "quick" formatsJeff King, Sep 4, 2021
  17. ZheNing HuSep 5, 2021
  18. Jeff KingSep 5, 2021
  19. ZheNing HuSep 6, 2021
  20. Junio C HamanoSep 7, 2021
  21. ZheNing HuSep 5, 2021
  22. Jeff KingSep 5, 2021
  23. ZheNing HuSep 6, 2021
  24. Jeff KingSep 7, 2021
  25. ZheNing HuSep 9, 2021
  26. Patrick SteinhardtSep 6, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.