[PATCH v3 06/10] builtin/fsck: move packfile verification into the packed source
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 11, 2026, 13:27 UTC
- Message-ID
- <20260911-pks-odb-source-fsck-v3-6-ef2fdc085e38@pks.im>
- In-Reply-To
- <20260911-pks-odb-source-fsck-v3-0-ef2fdc085e38@pks.im>
Move the packfile verification out of `cmd_fsck()` and into the "packed" source. While doing so, thread the progress meter and object callback through the newly introduced `struct odb_fsck_options` so that the caller's preferences are honoured without exposing those details at the "builtin/fsck.c" level.
Note that the old code reported failures when verifying packfiles with the `ERROR_PACK` bit, which gets returned to the caller via the exit code. This bit is neither exercised in our test suite nor is it documented anywhere in our codebase. Furthermore, this bit is highly specific to the object storage backend, which makes it a bad fit for the new pluggable infrastructure. So instead of retaining these semantics, we drop them and return the generic `ERROR_OBJECT` bit.
Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/fsck.c | 33 ++++----------------------------- odb.h | 7 +++++++ odb/source-packed.c | 46 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 54 insertions(+), 32 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c index adbe192e56..e504dae904 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -7,7 +7,6 @@ #include "blob.h" #include "tag.h" #include "refs.h" -#include "pack.h" #include "cache-tree.h" #include "fsck.h" #include "parse-options.h" @@ -49,7 +48,6 @@ static int show_dangling = 1; static timestamp_t now; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 -#define ERROR_PACK 04 #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 @@ -967,6 +965,8 @@ int cmd_fsck(int argc, { struct odb_fsck_options odb_fsck_opts = { .flags = ODB_FSCK_FULL, + .object_cb = fsck_obj_buffer, + .object_payload = repo, }; int keep_cache_objects = 0; int name_objects = 0; @@ -1019,6 +1019,8 @@ int cmd_fsck(int argc, show_progress = isatty(2); if (verbose) show_progress = 0; + if (show_progress) + odb_fsck_opts.flags |= ODB_FSCK_PROGRESS; if (write_lost_and_found) { odb_fsck_opts.flags |= ODB_FSCK_FULL; @@ -1056,33 +1058,6 @@ int cmd_fsck(int argc, if (odb_fsck(repo->objects, &odb_fsck_opts) < 0) errors_found |= ERROR_OBJECT; - if (odb_fsck_opts.flags & ODB_FSCK_FULL) { - struct packed_git *p; - uint32_t total = 0, count = 0; - struct progress *progress = NULL; - - if (show_progress) { - repo_for_each_pack(repo, p) { - if (open_pack_index(p)) - continue; - total += p->num_objects; - } - - progress = start_progress(repo, - _("Checking objects"), total); - } - - repo_for_each_pack(repo, p) { - /* verify gives error messages itself */ - if (verify_pack(repo, - p, fsck_obj_buffer, repo, - progress, count)) - errors_found |= ERROR_PACK; - count += p->num_objects; - } - stop_progress(&progress); - } - if (fsck_finish(&fsck_obj_options)) errors_found |= ERROR_OBJECT; } diff --git a/odb.h b/odb.h index 76c15e48f5..0bf6c8d7d2 100644 --- a/odb.h +++ b/odb.h @@ -215,11 +215,18 @@ enum odb_fsck_flags { * verification of their metadata. */ ODB_FSCK_FULL = (1 << 0), + + /* Display a progress meter, if sensible. */ + ODB_FSCK_PROGRESS = (1 << 1), }; /* Options that shall be passed to `odb_fsck()`. */ struct odb_fsck_options { enum odb_fsck_flags flags; + + int (*object_cb)(const struct object_id *oid, enum object_type type, + unsigned long size, void *buffer, int *eaten, void *cb_data); + void *object_payload; }; /* diff --git a/odb/source-packed.c b/odb/source-packed.c index 7aacf4bc45..0d3599f8fe 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -9,8 +9,10 @@ #include "midx.h" #include "odb/source-packed.h" #include "odb/streaming.h" +#include "pack.h" #include "packfile.h" #include "pack-bitmap.h" +#include "progress.h" static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, @@ -827,10 +829,48 @@ static void odb_source_packed_free(struct odb_source *source) free(packed); } -static int odb_source_packed_fsck(struct odb_source *source UNUSED, - struct odb_fsck_options *opts UNUSED) +static int verify_packs(struct odb_source_packed *source, + struct odb_fsck_options *opts) { - return 0; + struct progress *progress = NULL; + struct packfile_list_entry *e; + uint32_t total = 0, count = 0; + int ret = 0; + + if (opts->flags & ODB_FSCK_PROGRESS) { + for (e = packfile_store_get_packs(source); e; e = e->next) { + if (open_pack_index(e->pack)) + continue; + total += e->pack->num_objects; + } + + progress = start_progress(source->base.odb->repo, + _("Checking objects"), total); + } + + for (e = packfile_store_get_packs(source); e; e = e->next) { + /* verify gives error messages itself */ + if (verify_pack(source->base.odb->repo, e->pack, + opts->object_cb, opts->object_payload, + progress, count)) + ret = -1; + count += e->pack->num_objects; + } + stop_progress(&progress); + + return ret; +} + +static int odb_source_packed_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_packed *packed = odb_source_packed_downcast(source); + int ret = 0; + + if ((opts->flags & ODB_FSCK_FULL) && verify_packs(packed, opts) < 0) + ret = -1; + + return ret; } struct odb_source_packed *odb_source_packed_new(struct object_database *odb,
-- 2.55.0.1074.ge7621b4bad.dirty