Re: [PATCH v6 8/9] pack-objects: introduce '--stdin-packs=follow'

[Taylor Blau <me@ttaylorr.com>]

On Mon, Jun 23, 2025 at 04:35:44PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> >  static void show_object_pack_hint(struct object *object, const char *name,
> > -				  void *data UNUSED)
> > +				  void *data)
> >  {
> > -	struct object_entry *oe = packlist_find(&to_pack, &object->oid);
> > -	if (!oe)
> > -		return;
> > +	enum stdin_packs_mode mode = *(enum stdin_packs_mode *)data;
> > +	if (mode == STDIN_PACKS_MODE_FOLLOW) {
> > +		if (object->type == OBJ_BLOB &&
> > +		    !has_object(the_repository, &object->oid, 0))
> > +			return;
>
> So, --stdin-packs opened a pack and is feeding the objects contained
> in it to this machinery.  show_commit_pack_hint() calls this
> function in the `follow` mode.  How would such an object be missing?
> Ah, lazy clones.  OK.

That is one such place, but another would be that the object is part of
some unreachable portion of the repository and points at another
unreachable object that is missing. We take care to accommodate those
holes in the unreachable object graph when generating cruft packs, and
AFAIK in general tolerate broken links and/or missing objects provided
they are unreachable.

> > +		add_object_entry(&object->oid, object->type, name, 0);
> > +	} else {
>
> And only up to this point is the new code.  The "else" clause is
> just the original indented one-level deeper.

Right.

> > +static void show_commit_pack_hint(struct commit *commit, void *data)
> >  {
> > +	enum stdin_packs_mode mode = *(enum stdin_packs_mode *)data;
> > +
> > +	if (mode == STDIN_PACKS_MODE_FOLLOW) {
> > +		show_object_pack_hint((struct object *)commit, "", data);
> > +		return;
> > +	}
> > +
> >  	/* nothing to do; commits don't have a namehash */
> > +
> >  }
>
> What is this new blank line doing here?

Weird, this one evaded my proof-reading as well. Sorry about that -- I
can send a new round with this and the other spot fixed up if you want
one.

Thanks,
Taylor