refs/reftable: fix consistency checks with worktrees

The ref consistency checks are driven via `cmd_refs_verify()`. That
function loops through all worktrees (including the main worktree) and
then checks the ref store for each of them individually. It follows that
the backend is expected to only verify refs that belong to the specified
worktree.

While the "files" backend handles this correctly, the "reftable" backend
doesn't. In fact, it completely ignores the passed worktree and instead
verifies refs of _all_ worktrees. The consequence is that we'll end up
every ref store N times, where N is the number of worktrees.

Or rather, that would be the case if we actually iterated through the
worktree reftable stacks correctly. But we use `strmap_for_each_entry()`
to iterate through the stacks, but the map is in fact not even properly
populated. So instead of checking stacks N^2 times, we actually only end
up checking the reftable stack of the main worktree.

Fix this bug by only verifying the stack of the passed-in worktree and
constructing the backends via `backend_for_worktree()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index dda961a32b532c6c3942da9e818bdf6f8b89646b..6361b270155e38d8c8b8373dad598d8c5d7b0f92 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -26,6 +26,7 @@
 #include "../setup.h"
 #include "../strmap.h"
 #include "../trace2.h"
+#include "../worktree.h"
 #include "../write-or-die.h"
 #include "refs-internal.h"
 
@@ -2762,25 +2763,23 @@ static int reftable_fsck_error_handler(struct reftable_fsck_info *info,
 }
 
 static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,
-                           struct worktree *wt UNUSED)
+                           struct worktree *wt)
 {
-       struct reftable_ref_store *refs;
-       struct strmap_entry *entry;
-       struct hashmap_iter iter;
-       int ret = 0;
-
-       refs = reftable_be_downcast(ref_store, REF_STORE_READ, "fsck");
-
-       ret |= reftable_fsck_check(refs->main_backend.stack, reftable_fsck_error_handler,
-                                  reftable_fsck_verbose_handler, o);
+       struct reftable_ref_store *refs =
+               reftable_be_downcast(ref_store, REF_STORE_READ, "fsck");
+       struct reftable_backend *backend;
 
-       strmap_for_each_entry(&refs->worktree_backends, &iter, entry) {
-               struct reftable_backend *b = (struct reftable_backend *)entry->value;
-               ret |= reftable_fsck_check(b->stack, reftable_fsck_error_handler,
-                                          reftable_fsck_verbose_handler, o);
+       if (is_main_worktree(wt)) {
+               backend = &refs->main_backend;
+       } else {
+               int ret = backend_for_worktree(&backend, refs, wt->id);
+               if (ret < 0)
+                       return error(_("reftable stack for worktree '%s' is broken"),
+                                    wt->id);
        }
 
-       return ret;
+       return reftable_fsck_check(backend->stack, reftable_fsck_error_handler,
+                                  reftable_fsck_verbose_handler, o);
 }
 
 struct ref_storage_be refs_be_reftable = {

diff --git a/t/t0614-reftable-fsck.sh b/t/t0614-reftable-fsck.sh
index 677eb9143c9de4e6b82011e022459e2d07be89fe..4757eb5931dd5af0defb276b32349901113daa5a 100755 (executable)
--- a/t/t0614-reftable-fsck.sh
+++ b/t/t0614-reftable-fsck.sh
@@ -55,4 +55,36 @@ for TABLE_NAME in "foo-bar-e4d12d59.ref" \
        '
 done
 
+test_expect_success 'worktree stacks can be verified' '
+       test_when_finished "rm -rf repo worktree" &&
+       git init repo &&
+       test_commit -C repo initial &&
+       git -C repo worktree add ../worktree &&
+
+       git -C worktree refs verify 2>err &&
+       test_must_be_empty err &&
+
+       REFTABLE_DIR=$(git -C worktree rev-parse --git-dir)/reftable &&
+       EXISTING_TABLE=$(head -n1 "$REFTABLE_DIR/tables.list") &&
+       mv "$REFTABLE_DIR/$EXISTING_TABLE" "$REFTABLE_DIR/broken.ref" &&
+
+       for d in repo worktree
+       do
+               echo "broken.ref" >"$REFTABLE_DIR/tables.list" &&
+               git -C "$d" refs verify 2>err &&
+               cat >expect <<-EOF &&
+               warning: broken.ref: badReftableTableName: invalid reftable table name
+               EOF
+               test_cmp expect err &&
+
+               echo garbage >"$REFTABLE_DIR/tables.list" &&
+               test_must_fail git -C "$d" refs verify 2>err &&
+               cat >expect <<-EOF &&
+               error: reftable stack for worktree ${SQ}worktree${SQ} is broken
+               EOF
+               test_cmp expect err || return 1
+
+       done
+'
+
 test_done