Re: [PATCH v2 2/8] pack-objects: ensure tree/tag closure with '--stdin-packs=follow'

On Fri, Oct 02, 2026 at 07:13:36PM -0400, Jeff King wrote:

So it would have made more sense to me to comment it there. Of course
that is hard when there are two such places.

I dunno.

Yeah, me either. I'm happy to change things around if you feel strongly.

quoted

+	oidset_iter_init(&ctx.extra_roots, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		struct object *obj = lookup_object(repo, oid);
+
+		if (!obj || !(obj->flags & SEEN))
+			add_pending_oid(&revs, NULL, oid, 0);
+	}
+	if (revs.pending.nr) {
+		if (prepare_revision_walk(&revs))
+			die(_("revision walk setup failed"));
+		traverse_commit_list(&revs,
+				     show_commit_pack_hint,
+				     show_object_pack_hint,
+				     &mode);
+	}
+	oidset_clear(&ctx.extra_roots);
+
 	release_revisions(&revs);

BTW, is it safe to prepare_revision_walk() twice on the same rev_info? I
could believe it works, but I could also believe that there are hidden
corner cases, as I don't think it was ever really intended to work this
way.

Maybe OK for the vanilla set of options we are using here (as opposed to
taking arbitrary options from the user). The rev_info is created locally
in this function, though, so I guess if we wanted to be double-plus sure
we could release and reinit the struct.

It seems to work in practice. From reading through and thinking about it
I couldn't find any obvious issues.

Just as well, there are a couple of spots that I was able to find that
already call `prepare_revision_walk()` more than once:

  * In builtin/pack-objects.c::get_object_list() (with the exception of
    '--path-walk') we call `prepare_revision_walk()` twice when
    exploding unreachable objects as loose.

  * In reachable.c::mark_reachable_objects(), we also call the
    `prepare_revision_walk()` function twice when given a timestamp via
    `mark_recent`.

This all works since `revs.pending` is emptied by the first revwalk. But
it is under-documented, so callers relying on this behavior may be
surprised if/when it changes. Probably good #leftoverbits.

Thanks,
Taylor