Re: [RFC PATCH 08/10] pack-objects: introduce '--stdin-packs=follow-reachable'

[Taylor Blau <me@ttaylorr.com>]

On Fri, Jun 26, 2026 at 02:39:10PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > +/*
> > + * Flag bit set on commits that belong to an included pack during
> > + * '--stdin-packs=follow-reachable'. Used by the pre-walk to
> > + * identify which reachable commits should be tips for the main
> > + * object traversal.
> > + */
> > +#define IN_INCLUDED_PACK (1u<<11)
> > +
> > +static int mark_included_pack_tip(const struct object_id *oid,
> > +				  struct packed_git *p,
> > +				  uint32_t pos,
> > +				  void *data)
> > +{
> > +	struct rev_info *main_revs = data;
> > +	off_t ofs = nth_packed_object_offset(p, pos);
> > +	enum object_type type;
> > +	struct object_info oi = OBJECT_INFO_INIT;
> > +	struct object *obj;
> > +
> > +	oi.typep = &type;
> > +	if (packed_object_info(p, ofs, &oi) < 0)
> > +		return 0;
> > +	if (type != OBJ_COMMIT && type != OBJ_TAG)
> > +		return 0;
>
> We do not care about non commits, non tags.

This should not be an &&, but rather an ||. We only want to handle
objects which are either commits *or* tags via this function.

> > +	obj = parse_object(the_repository, oid);
> > +	if (!obj)
> > +		return 0;
> > +
> > +	obj->flags |= IN_INCLUDED_PACK;
> > +
> > +	if (type == OBJ_TAG && main_revs)
> > +		add_pending_object(main_revs, obj, "");
>
> Any tag object is added to the pending list of the second phase
> traversal here.  Doesn't this retain unreachable tags and
> (unreachable) objects that are only reachable from these unreachable
> tags found in the packfile?  Don't we want to limit this code to add
> only tags that actually are reachable from refs, or something?

Hmm. You're right, if we have an unreachable annotated tag object in one
of the STDIN_PACK_INCLUDE packs, then we'll pick it up and place it in
the non-cruft pack.

I'd have to think more about how to handle this properly to ensure that
*only* reachable objects are included. We probably want to re-order this
code so that we start a traversal from the reference tips, then mark
reachable objects as starting points based on whether or not they appear
in an included pack (and correspondingly do not appear in an excluded
one).

That adjusts how the traversal would work, I think, since we should be
able to pick up the resulting set of objects in a single pass. But I'd
have to think more about it...

Thanks,
Taylor