Re: [PATCH] revision: mark blobs needed for resolve-undo as reachable
From: Taylor Blau <hidden>
Date: 2022-06-14 02:58:16
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.
quoted hunk ↗ jump to hunk
@@ -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