[PATCH 00/13] refs: improvements and fixes for peeling tags

[Patrick Steinhardt <ps@pks.im>]

Hi,

originally, all I wanted to do was the last patch: a small performance
optimization that stops parsing objects in git-for-each-ref(1) unless we
really need to parse them. But that fix cause one specific test to fail,
and only with the reftable backend. So this led me down the rabbit hole
of tag peeling, ending up with this patch series.

The series is structured like follows:

  - Patches 1 to 7 refactor our codebase so that we don't have the
    `peel_iterated_object()` hack anymore. I just found it hard to
    follow and thought it shouldn't be too hard to get rid of it.

  - Patches 8 and 9 remove infrastructure that we don't need anymore
    after the first couple of patches.

  - Patches 10 to 12 fix a couple of issues with peeled tags that I
    found. The underlying issue is that tags store both the tagged
    object and their type, but this information may not match. We never
    verify the actual object type though when allocating the tagged
    object, so this only blows up much later.

  - Patch 13 was my original motivation, a small performance
    optimization.

I'm not particularly fond of the patches 10 to 12. It feels more like
playing whack-a-mole, and I very much assume that there still are edge
cases where we should properly verify the tagged object type. But
changing it in `parse_tag_buffer()` itself causes a bunch of tests to
fail where we intentionally create such corrupted tags. So I didn't
really dare to touch that part, to be honest.

If anybody has suggestions for an alternative approach I'd be very open
to it.

The topic is built on top of 45547b60ac (Merge branch 'master' of
https://github.com/j6t/gitk, 2025-10-05). There is a merge conflict with
tb/incremental-midx-part-3.1, which moves code from "builtin/repack.c"
into "repack-*.c".

The conflict can be solved by accepting "builtin/repack.c" from
tb/incremental-midx-part-3.1 and adding the below patch to
"repack-midx.c". I can also rebase on top of that series, but given that
it is rather huge it may take a while before it lands.

Thanks!

Patrick

diff --cc builtin/repack.c
index 873e21c35d,ad60c4290d..0000000000
--- a/builtin/repack.c
+++ b/builtin/repack.c
diff --git a/repack-midx.c b/repack-midx.c
index 6f6202c5bc..74bdfa3a6e 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -16,25 +16,23 @@ struct midx_snapshot_ref_data {
 	int preferred;
 };
 
-static int midx_snapshot_ref_one(const char *refname UNUSED,
-				 const char *referent UNUSED,
-				 const struct object_id *oid,
-				 int flag UNUSED, void *_data)
+static int midx_snapshot_ref_one(const struct reference *ref, void *_data)
 {
 	struct midx_snapshot_ref_data *data = _data;
+	const struct object_id *maybe_peeled = ref->oid;
 	struct object_id peeled;
 
-	if (!peel_iterated_oid(data->repo, oid, &peeled))
-		oid = &peeled;
+	if (!reference_get_peeled_oid(data->repo, ref, &peeled))
+		maybe_peeled = &peeled;
 
-	if (oidset_insert(&data->seen, oid))
+	if (oidset_insert(&data->seen, maybe_peeled))
 		return 0; /* already seen */
 
-	if (odb_read_object_info(data->repo->objects, oid, NULL) != OBJ_COMMIT)
+	if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) != OBJ_COMMIT)
 		return 0;
 
 	fprintf(data->f->fp, "%s%s\n", data->preferred ? "+" : "",
-		oid_to_hex(oid));
+		oid_to_hex(maybe_peeled));
 
 	return 0;
 }

---
Patrick Steinhardt (13):
      refs: introduce wrapper struct for `each_ref_fn`
      refs: introduce `.ref` field for the base iterator
      refs: refactor reference status flags
      refs: expose peeled object ID via the iterator
      upload-pack: convert to use `reference_get_peeled_oid()`
      ref-filter: propagate peeled object ID
      builtin/show-ref: convert to use `reference_get_peeled_oid()`
      refs: drop `current_ref_iter` hack
      refs: drop infrastructure to peel via iterators
      object: add flag to `peel_object()` to verify object type
      refs: don't store peeled object IDs for invalid tags
      ref-filter: detect broken tags when dereferencing them
      ref-filter: parse objects on demand

 bisect.c                    |  24 ++---
 builtin/bisect.c            |  17 +---
 builtin/checkout.c          |   6 +-
 builtin/describe.c          |  18 ++--
 builtin/fetch.c             |  13 +--
 builtin/fsck.c              |  33 +++---
 builtin/gc.c                |  15 ++-
 builtin/ls-remote.c         |   2 +-
 builtin/name-rev.c          |  17 ++--
 builtin/pack-objects.c      |  28 +++---
 builtin/receive-pack.c      |  13 ++-
 builtin/remote.c            |  44 ++++----
 builtin/repack.c            |  16 ++-
 builtin/replace.c           |  21 ++--
 builtin/rev-parse.c         |  12 +--
 builtin/show-branch.c       |  35 +++----
 builtin/show-ref.c          |  50 ++++-----
 builtin/submodule--helper.c |  10 +-
 builtin/tag.c               |   2 +-
 builtin/verify-tag.c        |   2 +-
 builtin/worktree.c          |   6 +-
 commit-graph.c              |  14 ++-
 delta-islands.c             |   9 +-
 fetch-pack.c                |  16 +--
 help.c                      |  10 +-
 http-backend.c              |  20 ++--
 log-tree.c                  |  24 ++---
 ls-refs.c                   |  36 ++++---
 midx-write.c                |  17 ++--
 negotiator/default.c        |   7 +-
 negotiator/skipping.c       |   7 +-
 notes.c                     |   8 +-
 object-name.c               |  10 +-
 object.c                    |  20 +++-
 object.h                    |  15 ++-
 pseudo-merge.c              |  21 ++--
 reachable.c                 |   9 +-
 ref-filter.c                | 239 ++++++++++++++++++++++++++++++--------------
 ref-filter.h                |   5 +-
 reflog.c                    |   9 +-
 refs.c                      |  85 +++++++++-------
 refs.h                      |  84 ++++++++++------
 refs/debug.c                |  17 +---
 refs/files-backend.c        |  71 +++++--------
 refs/iterator.c             |  73 +++-----------
 refs/packed-backend.c       |  72 +++++--------
 refs/ref-cache.c            |  18 +---
 refs/refs-internal.h        |  25 +----
 refs/reftable-backend.c     |  48 +++------
 remote.c                    |  27 +++--
 replace-object.c            |  16 ++-
 revision.c                  |  12 +--
 server-info.c               |  12 +--
 shallow.c                   |  16 +--
 submodule.c                 |  12 +--
 t/for-each-ref-tests.sh     |   4 +-
 t/helper/test-reach.c       |   2 +-
 t/helper/test-ref-store.c   |   5 +-
 t/pack-refs-tests.sh        |  32 ++++++
 t/t0610-reftable-basics.sh  |  28 ++++++
 tag.c                       |  12 ---
 tag.h                       |   1 -
 upload-pack.c               |  49 ++++-----
 walker.c                    |   8 +-
 worktree.c                  |  11 +-
 65 files changed, 791 insertions(+), 829 deletions(-)


---
base-commit: 45547b60aca32b45d2f1ef93462cf9df28637c13
change-id: 20250918-b4-pks-ref-filter-skip-parsing-objects-f0d1f6af4a9f