On Thu, Jun 09, 2022 at 04:44:20PM -0700, Junio C Hamano wrote:
> The resolve-undo extension was added to the index in cfc5789a
> (resolve-undo: record resolved conflicts in a new index extension
> section, 2009-12-25). This extension records the blob object names
> and their modes of conflicted paths when the path gets resolved
> (e.g. with "git add"), to allow "undoing" the resolution with
> "checkout -m path". These blob objects should be guarded from
> garbage-collection while we have the resolve-undo information in the
> index (otherwise unresolve operation may try to use a blob object
> that has already been pruned away).
>
> But the code called from mark_reachable_objects() for the index
> forgets to do so. Teach add_index_objects_to_pending() helper to
> also add objects referred to by the resolve-undo extension.
Nice find!
> Also make matching changes to "fsck", which has code that is fairly
> similar to the reachability stuff, but have parallel implementations
> for all these stuff, which may (or may not) someday want to be unified.
I wasn't sure what the change in fsck was when skimming the diffstat
before reading your patch message, but makes sense. I'm glad that you
included this, too.
> +static void add_resolve_undo_to_pending(struct index_state *istate, struct rev_info *revs)
> +{
> + struct string_list_item *item;
> + struct string_list *resolve_undo = istate->resolve_undo;
> +
> + if (!resolve_undo)
> + return;
> +
> + for_each_string_list_item(item, resolve_undo) {
> + const char *path = item->string;
> + struct resolve_undo_info *ru = item->util;
> + int i;
> +
> + if (!ru)
> + continue;
> + for (i = 0; i < 3; i++) {
> + struct blob *blob;
> +
> + if (!ru->mode[i] || !S_ISREG(ru->mode[i]))
> + continue;
> +
> + blob = lookup_blob(revs->repo, &ru->oid[i]);
> + if (!blob) {
> + warning(_("resolve-undo records `%s` which is missing"),
> + oid_to_hex(&ru->oid[i]));
> + continue;
> + }
> + add_pending_object_with_path(revs, &blob->object, "",
> + ru->mode[i], path);
> + }
> + }
> +}
This implementation looks good to my eyes.
> @@ -1718,6 +1752,8 @@ static void do_add_index_objects_to_pending(struct rev_info *revs,
> add_cache_tree(istate->cache_tree, revs, &path, flags);
> strbuf_release(&path);
> }
> +
> + add_resolve_undo_to_pending(istate, revs);
> }
Great; this fixes the bug for cruft packs, too, whose reachable pack is
generated with `--indexed-objects`, so the cruft pack will no longer
contain the resolve-undo objects.
> +test_expect_success 'resolve-undo keeps blobs from gc' '
Very thorough. Thanks!
Taylor
