From 68c869769e1632f2de73776c81bc8f6f7c3d08b4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:15 +0200 Subject: [PATCH 01/10] builtin/fsck: use `fsck_obj_buffer()` when checking loose objects When checking loose objects we manually parse the object buffer we have read from the on-disk file, mark the object and then call `fsck_obj()`. The exact same steps are also performed by `fsck_obj_buffer()`. Stop open-coding this logic and call `fsck_obj_buffer()` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 892c5661d9..3c4127f4d8 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -722,7 +722,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *cb_data) { struct for_each_loose_cb *data = cb_data; - struct object *obj; enum object_type type = OBJ_NONE; size_t size; void *contents = NULL; @@ -751,21 +750,7 @@ static int fsck_loose(const struct object_id *oid, const char *path, if (!contents && type != OBJ_BLOB) BUG("read_loose_object streamed a non-blob"); - obj = parse_object_buffer(data->repo, oid, type, size, - contents, &eaten); - - if (!obj) { - errors_found |= ERROR_OBJECT; - error(_("%s: object could not be parsed: %s"), - oid_to_hex(oid), path); - if (!eaten) - free(contents); - return 0; /* keep checking other objects */ - } - - obj->flags &= ~(REACHABLE | SEEN); - obj->flags |= HAS_OBJ; - if (fsck_obj(data->repo, obj, contents, size)) + if (fsck_obj_buffer(oid, type, size, contents, &eaten, data->repo)) errors_found |= ERROR_OBJECT; if (!eaten) From c711a2cfa56ffb5d01e683e3356c8d39659d8cbd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:16 +0200 Subject: [PATCH 02/10] builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()` The interfaces of the functions `fsck_obj()` and `fsck_obj_buffer()` are somewhat similar to one another. The only difference between those two is that `fsck_obj()` takes an already-parsed object as input, whereas `fsck_obj_buffer()` parses the buffer and then calls `fsck_obj()`. Furthermore, `fsck_obj()` has no callers other than `fsck_obj_buffer()`. Refactor the code by merging those two functions. This makes it obvious which function does what, and it allows us to get rid of the early return in `fsck_obj()` in case `SEEN` is set as the only caller unconditionally clears that bit before calling it anyway. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 47 ++++++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 3c4127f4d8..bed8481893 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -401,14 +401,27 @@ static void check_connectivity(struct repository *repo) } } -static int fsck_obj(struct repository *repo, - struct object *obj, void *buffer, unsigned long size) +static int fsck_obj_buffer(const struct object_id *oid, enum object_type type, + unsigned long size, void *buffer, int *eaten, void *cb_data) { + struct repository *repo = cb_data; + struct object *obj; int err; - if (obj->flags & SEEN) - return 0; - obj->flags |= SEEN; + /* + * Note, buffer may be NULL if type is OBJ_BLOB. See + * verify_packfile(), data_valid variable for details. + */ + obj = parse_object_buffer(repo, oid, type, size, buffer, eaten); + if (!obj) { + errors_found |= ERROR_OBJECT; + err = error(_("%s: object corrupt or missing"), + oid_to_hex(oid)); + goto out; + } + + obj->flags &= ~REACHABLE; + obj->flags |= HAS_OBJ | SEEN; if (verbose) fprintf_ln(stderr, _("Checking %s %s"), @@ -417,6 +430,7 @@ static int fsck_obj(struct repository *repo, if (fsck_walk(obj, NULL, &fsck_obj_options)) objerror(repo, obj, _("broken links")); + err = fsck_object(obj, buffer, size, &fsck_obj_options); if (err) goto out; @@ -442,32 +456,11 @@ static int fsck_obj(struct repository *repo, } out: - if (obj->type == OBJ_TREE) + if (obj && obj->type == OBJ_TREE) free_tree_buffer((struct tree *)obj); return err; } -static int fsck_obj_buffer(const struct object_id *oid, enum object_type type, - unsigned long size, void *buffer, int *eaten, void *cb_data) -{ - struct repository *repo = cb_data; - struct object *obj; - - /* - * Note, buffer may be NULL if type is OBJ_BLOB. See - * verify_packfile(), data_valid variable for details. - */ - obj = parse_object_buffer(repo, oid, type, size, buffer, eaten); - if (!obj) { - errors_found |= ERROR_OBJECT; - return error(_("%s: object corrupt or missing"), - oid_to_hex(oid)); - } - obj->flags &= ~(REACHABLE | SEEN); - obj->flags |= HAS_OBJ; - return fsck_obj(repo, obj, buffer, size); -} - static int default_refs; static void fsck_handle_reflog_oid(struct repository *repo, From bb9d79e1d0b372eb743bddc5b6c5ba31d6e737cd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:17 +0200 Subject: [PATCH 03/10] builtin/fsck: de-globalize option handling In subsequent commits we're about to rework some of the option handling in git-fsck(1) a bit. It is currently a bit of a mess though due to lots of global state that makes it hard to see which flags are used where exactly. Refactor the code by moving the fsck options into `cmd_fsck()`. This allows us to convert some of the options into function-local variables. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index bed8481893..5132ff0f15 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -37,10 +37,8 @@ static int show_root; static int show_tags; static int show_unreachable; static int include_reflogs = 1; -static int check_full = 1; static int connectivity_only; static int check_strict; -static int keep_cache_objects; static struct fsck_options fsck_walk_options; static struct fsck_options fsck_obj_options; static int errors_found; @@ -48,8 +46,6 @@ static int write_lost_and_found; static int verbose; static int show_progress = -1; static int show_dangling = 1; -static int name_objects; -static int check_references = 1; static timestamp_t now; #define ERROR_OBJECT 01 #define ERROR_REACHABLE 02 @@ -964,30 +960,33 @@ static char const * const fsck_usage[] = { NULL }; -static struct option fsck_opts[] = { - OPT__VERBOSE(&verbose, N_("be verbose")), - OPT_BOOL(0, "unreachable", &show_unreachable, N_("show unreachable objects")), - OPT_BOOL(0, "dangling", &show_dangling, N_("show dangling objects")), - OPT_BOOL(0, "tags", &show_tags, N_("report tags")), - OPT_BOOL(0, "root", &show_root, N_("report root nodes")), - OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), - OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), - OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), - OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), - OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), - OPT_BOOL(0, "lost-found", &write_lost_and_found, - N_("write dangling objects in .git/lost-found")), - OPT_BOOL(0, "progress", &show_progress, N_("show progress")), - OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), - OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), - OPT_END(), -}; - int cmd_fsck(int argc, const char **argv, const char *prefix, struct repository *repo) { + int check_full = 1; + int keep_cache_objects = 0; + int name_objects = 0; + int check_references = 1; + struct option fsck_opts[] = { + OPT__VERBOSE(&verbose, N_("be verbose")), + OPT_BOOL(0, "unreachable", &show_unreachable, N_("show unreachable objects")), + OPT_BOOL(0, "dangling", &show_dangling, N_("show dangling objects")), + OPT_BOOL(0, "tags", &show_tags, N_("report tags")), + OPT_BOOL(0, "root", &show_root, N_("report root nodes")), + OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), + OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), + OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), + OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), + OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), + OPT_BOOL(0, "lost-found", &write_lost_and_found, + N_("write dangling objects in .git/lost-found")), + OPT_BOOL(0, "progress", &show_progress, N_("show progress")), + OPT_BOOL(0, "name-objects", &name_objects, N_("show verbose names for reachable objects")), + OPT_BOOL(0, "references", &check_references, N_("check reference database consistency")), + OPT_END(), + }; struct odb_source *source; struct snapshot snap = { .nr = 0, From 819a54e43dbbafa41e505a3a941a481bd8d8e2c2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:18 +0200 Subject: [PATCH 04/10] builtin/fsck: don't check alternates with "--no-full" According to git-fsck(1), the "--full" option behaves in the following way: Check not just objects in GIT_OBJECT_DIRECTORY ($GIT_DIR/objects), but also the ones found in alternate object pools listed in GIT_ALTERNATE_OBJECT_DIRECTORIES or $GIT_DIR/objects/info/alternates, and in packed Git archives found in $GIT_DIR/objects/pack and corresponding pack subdirectories in alternate object pools. So ultimately, it is supposed to control two things: (1) whether we only check the main object directory, and (2) whether we check packfiles. In its current state though, the flag only controls whether we check packfiles or not, and if so we verify packfiles of all attached sources. But we also have checks for loose objects in git-fsck(1), and here we unconditionally check them in all sources. The flag is arguably conflating two unrelated concerns with one another, and it really should be split up into two flags: one that controls how thorough we want to check individual sources, and one that controls which sources we want to check in the first place. So ideally, we would have: - "--include-alternates": check all sources, not only the local one. - "--include-optimized-objects": check not only loose objects, but also those that have been packed. Note that we explicitly don't say "--include-packed-objects" here to be more backend-agnostic. - "--full": implies both of the above flags. This feels out of scope for this series though. So for now, simply fix the code by honoring locality of the sources for loose objects. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 3 ++- t/t1450-fsck.sh | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 5132ff0f15..3f6056535f 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -1047,7 +1047,8 @@ int cmd_fsck(int argc, mark_object_for_connectivity, repo, 0); } else { for (source = repo->objects->sources; source; source = source->next) - fsck_source(repo, source); + if (check_full || source->local) + fsck_source(repo, source); if (check_full) { struct packed_git *p; diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 77cd96de78..1b4074304c 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -844,6 +844,11 @@ test_expect_success 'alternate objects are correctly blamed' ' echo "../../alt.git/objects" >.git/objects/info/alternates && mkdir alt.git/objects/$(dirname $path) && >alt.git/objects/$(dirname $path)/$(basename $path) && + + # Without "--full", only the local object source is checked. + git fsck --no-full >out 2>&1 && + test_must_be_empty out && + test_must_fail git fsck >out 2>&1 && test_grep alt.git out ' From e3e0b2183f9bd65c6a1b8d90ac37832080c3bfa6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:19 +0200 Subject: [PATCH 05/10] odb: provide infrastructure for pluggable fsck checks The on-disk consistency checks in git-fsck(1) are conceptually backend-specific: while connectivity checks and object-level parsing checks are generic, verifying the physical integrity of packfiles and loose objects is meaningful only to backends that use these formats: Having these checks live in "builtin/fsck.c" violates that layering, because it forces the command to reach directly into format-specific internals. Provide new infrastructure to make these format-specific checks pluggable and implement stubs for the different source types we already have. In subsequent commits we'll move functionality over piece by piece. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 16 +++++++++++----- odb.c | 9 +++++++++ odb.h | 23 +++++++++++++++++++++++ odb/source-files.c | 13 +++++++++++++ odb/source-inmemory.c | 8 ++++++++ odb/source-loose.c | 7 +++++++ odb/source-packed.c | 8 ++++++++ odb/source.h | 21 +++++++++++++++++++++ 8 files changed, 100 insertions(+), 5 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 3f6056535f..adbe192e56 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -965,7 +965,9 @@ int cmd_fsck(int argc, const char *prefix, struct repository *repo) { - int check_full = 1; + struct odb_fsck_options odb_fsck_opts = { + .flags = ODB_FSCK_FULL, + }; int keep_cache_objects = 0; int name_objects = 0; int check_references = 1; @@ -977,7 +979,8 @@ int cmd_fsck(int argc, OPT_BOOL(0, "root", &show_root, N_("report root nodes")), OPT_BOOL(0, "cache", &keep_cache_objects, N_("make index objects head nodes")), OPT_BOOL(0, "reflogs", &include_reflogs, N_("make reflogs head nodes (default)")), - OPT_BOOL(0, "full", &check_full, N_("also consider packs and alternate objects")), + OPT_BIT(0, "full", &odb_fsck_opts.flags, + N_("also consider packs and alternate objects"), ODB_FSCK_FULL), OPT_BOOL(0, "connectivity-only", &connectivity_only, N_("check only connectivity")), OPT_BOOL(0, "strict", &check_strict, N_("enable more strict checking")), OPT_BOOL(0, "lost-found", &write_lost_and_found, @@ -1018,7 +1021,7 @@ int cmd_fsck(int argc, show_progress = 0; if (write_lost_and_found) { - check_full = 1; + odb_fsck_opts.flags |= ODB_FSCK_FULL; include_reflogs = 0; } @@ -1047,10 +1050,13 @@ int cmd_fsck(int argc, mark_object_for_connectivity, repo, 0); } else { for (source = repo->objects->sources; source; source = source->next) - if (check_full || source->local) + if ((odb_fsck_opts.flags & ODB_FSCK_FULL) || source->local) fsck_source(repo, source); - if (check_full) { + 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; diff --git a/odb.c b/odb.c index 1fe20808eb..766043b685 100644 --- a/odb.c +++ b/odb.c @@ -1177,3 +1177,12 @@ void odb_reprepare(struct object_database *o) { odb_prepare(o, ODB_PREPARE_FLUSH_CACHES); } + +int odb_fsck(struct object_database *odb, struct odb_fsck_options *options) +{ + int ret = 0; + for (struct odb_source *source = odb->sources; source; source = source->next) + if ((options->flags & ODB_FSCK_FULL) || source->local) + ret |= odb_source_fsck(source, options); + return ret; +} diff --git a/odb.h b/odb.h index e60174070f..76c15e48f5 100644 --- a/odb.h +++ b/odb.h @@ -206,6 +206,29 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags); /* Equivalent to `odb_prepare(o, ODB_PREPARE_FLUSH_CACHES)`. */ void odb_reprepare(struct object_database *o); +enum odb_fsck_flags { + /* + * If set, perform a full consistency check for the full object + * database, including all of its sources and the contents of their + * optimized formats. Otherwise, only check the local source, and + * restrict checks of its optimized formats to cheap structural + * verification of their metadata. + */ + ODB_FSCK_FULL = (1 << 0), +}; + +/* Options that shall be passed to `odb_fsck()`. */ +struct odb_fsck_options { + enum odb_fsck_flags flags; +}; + +/* + * Run backend-specific integrity checks on all object sources. Each source + * performs the checks appropriate to its type. Returns 0 on success, a + * negative error code otherwise. + */ +int odb_fsck(struct object_database *odb, struct odb_fsck_options *opts); + /* * Find source by its object directory path. Returns a `NULL` pointer in case * the source could not be found. diff --git a/odb/source-files.c b/odb/source-files.c index bd4fdf3a6c..f6fb560d2e 100644 --- a/odb/source-files.c +++ b/odb/source-files.c @@ -893,6 +893,18 @@ static int odb_source_files_generate_pack(struct odb_source *source UNUSED, return 0; } +static int odb_source_files_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_files *files = odb_source_files_downcast(source); + int ret = 0; + + ret |= odb_source_fsck(&files->loose->base, opts); + ret |= odb_source_fsck(&files->packed->base, opts); + + return ret; +} + struct odb_source_files *odb_source_files_new(struct object_database *odb, const char *path, bool local) @@ -908,6 +920,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb, files->base.close = odb_source_files_close; files->base.create_on_disk = odb_source_files_create_on_disk; files->base.prepare = odb_source_files_prepare; + files->base.fsck = odb_source_files_fsck; files->base.read_object_info = odb_source_files_read_object_info; files->base.read_object_stream = odb_source_files_read_object_stream; files->base.for_each_object = odb_source_files_for_each_object; diff --git a/odb/source-inmemory.c b/odb/source-inmemory.c index 795672adf2..ba0f86da26 100644 --- a/odb/source-inmemory.c +++ b/odb/source-inmemory.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "object-file.h" #include "odb.h" +#include "fsck.h" #include "odb/source-inmemory.h" #include "odb/streaming.h" #include "oidtree.h" @@ -368,6 +369,12 @@ static void odb_source_inmemory_free(struct odb_source *source) free(inmemory); } +static int odb_source_inmemory_fsck(struct odb_source *source UNUSED, + struct odb_fsck_options *opts UNUSED) +{ + return 0; +} + struct odb_source_inmemory *odb_source_inmemory_new(struct object_database *odb) { struct odb_source_inmemory *source; @@ -378,6 +385,7 @@ struct odb_source_inmemory *odb_source_inmemory_new(struct object_database *odb) source->base.free = odb_source_inmemory_free; source->base.close = odb_source_inmemory_close; source->base.prepare = odb_source_inmemory_prepare; + source->base.fsck = odb_source_inmemory_fsck; source->base.read_object_info = odb_source_inmemory_read_object_info; source->base.read_object_stream = odb_source_inmemory_read_object_stream; source->base.for_each_object = odb_source_inmemory_for_each_object; diff --git a/odb/source-loose.c b/odb/source-loose.c index bb3455dfbd..f68d3c4d6c 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -1031,6 +1031,12 @@ static void odb_source_loose_free(struct odb_source *source) free(loose); } +static int odb_source_loose_fsck(struct odb_source *source UNUSED, + struct odb_fsck_options *opts UNUSED) +{ + return 0; +} + struct odb_source_loose *odb_source_loose_new(struct object_database *odb, const char *path, bool local) @@ -1043,6 +1049,7 @@ struct odb_source_loose *odb_source_loose_new(struct object_database *odb, loose->base.free = odb_source_loose_free; loose->base.close = odb_source_loose_close; loose->base.prepare = odb_source_loose_prepare; + loose->base.fsck = odb_source_loose_fsck; loose->base.read_object_info = odb_source_loose_read_object_info; loose->base.read_object_stream = odb_source_loose_read_object_stream; loose->base.for_each_object = odb_source_loose_for_each_object; diff --git a/odb/source-packed.c b/odb/source-packed.c index 630d955585..7aacf4bc45 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -2,6 +2,7 @@ #include "abspath.h" #include "chdir-notify.h" #include "dir.h" +#include "fsck.h" #include "git-zlib.h" #include "list-objects-filter-options.h" #include "mergesort.h" @@ -826,6 +827,12 @@ 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) +{ + return 0; +} + struct odb_source_packed *odb_source_packed_new(struct object_database *odb, const char *path, bool local) @@ -839,6 +846,7 @@ struct odb_source_packed *odb_source_packed_new(struct object_database *odb, packed->base.free = odb_source_packed_free; packed->base.close = odb_source_packed_close; packed->base.prepare = odb_source_packed_prepare; + packed->base.fsck = odb_source_packed_fsck; packed->base.read_object_info = odb_source_packed_read_object_info; packed->base.read_object_stream = odb_source_packed_read_object_stream; packed->base.for_each_object = odb_source_packed_for_each_object; diff --git a/odb/source.h b/odb/source.h index 559e2ea2e9..10a5dd5194 100644 --- a/odb/source.h +++ b/odb/source.h @@ -320,6 +320,17 @@ struct odb_source { int (*generate_pack)(struct odb_source *source, struct odb_pack_generator **out, const struct odb_generate_pack_options *opts); + + /* + * This callback is expected to check the integrity of the object source + * and report any errors found via the fsck options. The checks performed + * are backend-specific. + * + * The callback is expected to return 0 on success, a negative error + * code otherwise. + */ + int (*fsck)(struct odb_source *source, + struct odb_fsck_options *options); }; /* @@ -588,4 +599,14 @@ static inline int odb_source_generate_pack(struct odb_source *source, return source->generate_pack(source, out, opts); } +/* + * Check the integrity of the object database source. The checks performed + * are backend-specific. Returns 0 on success, a negative error code otherwise. + */ +static inline int odb_source_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + return source->fsck(source, opts); +} + #endif From af061ff199381e83bbfd2ef5a7402a0063a2af0c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:20 +0200 Subject: [PATCH 06/10] builtin/fsck: move packfile verification into the packed source 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 Signed-off-by: Junio C Hamano --- 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, From 552de3d95ad000140bebb0507a65f16f731c06e6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:21 +0200 Subject: [PATCH 07/10] builtin/fsck: move reverse index verification into the packed source The checks for reverse indexes live in `check_pack_rev_indexes()`, which is hosted in "builtin/fsck.c". These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in the preceding commit, drop the dedicated `ERROR_PACK_REV_INDEX` bit and instead use the generic `ERROR_OBJECT` bit. Note that this changes behaviour in two ways: - The checks are now skipped when "--connectivity-only" was passed. This is because we don't even run `odb_fsck()` at all when that flag has been passed by the user, and not verifying data structures of the object database matches the documented intent of that flag, which is to only check the connectivity of reachable objects. - The checks are now skipped for non-local sources when "--no-full" was passed. This is, again, in line with the documented intent of that flag. Add a test to cast these semantics into stone. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 37 ------------------------------------- odb/source-packed.c | 39 +++++++++++++++++++++++++++++++++++++++ t/t5325-reverse-index.sh | 8 ++++++++ 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index e504dae904..06e72877f3 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -23,7 +23,6 @@ #include "run-command.h" #include "sparse-index.h" #include "worktree.h" -#include "pack-revindex.h" #include "pack-bitmap.h" #define REACHABLE 0x0001 @@ -51,7 +50,6 @@ static timestamp_t now; #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 -#define ERROR_PACK_REV_INDEX 0100 #define ERROR_BITMAP 0200 static const char *describe_object(const struct object_id *oid) @@ -890,40 +888,6 @@ static int mark_object_for_connectivity(const struct object_id *oid, return 0; } -static int check_pack_rev_indexes(struct repository *r, int show_progress) -{ - struct progress *progress = NULL; - struct packed_git *p; - uint32_t pack_count = 0; - int res = 0; - - if (show_progress) { - repo_for_each_pack(r, p) - pack_count++; - progress = start_delayed_progress(r, - "Verifying reverse pack-indexes", pack_count); - pack_count = 0; - } - - repo_for_each_pack(r, p) { - int load_error = load_pack_revindex_from_disk(p); - - if (load_error < 0) { - error(_("unable to load rev-index for pack '%s'"), p->pack_name); - res = ERROR_PACK_REV_INDEX; - } else if (!load_error && - !load_pack_revindex(r, p) && - verify_pack_revindex(p)) { - error(_("invalid rev-index for pack '%s'"), p->pack_name); - res = ERROR_PACK_REV_INDEX; - } - display_progress(progress, ++pack_count); - } - stop_progress(&progress); - - return res; -} - static void fsck_refs(struct repository *r) { struct child_process refs_verify = CHILD_PROCESS_INIT; @@ -1104,7 +1068,6 @@ int cmd_fsck(int argc, free_worktrees(worktrees); } - errors_found |= check_pack_rev_indexes(repo, show_progress); if (verify_bitmap_files(repo)) errors_found |= ERROR_BITMAP; diff --git a/odb/source-packed.c b/odb/source-packed.c index 0d3599f8fe..e5e69636dd 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -10,6 +10,7 @@ #include "odb/source-packed.h" #include "odb/streaming.h" #include "pack.h" +#include "pack-revindex.h" #include "packfile.h" #include "pack-bitmap.h" #include "progress.h" @@ -861,6 +862,41 @@ static int verify_packs(struct odb_source_packed *source, return ret; } +static int verify_reverse_indices(struct odb_source_packed *source, + struct odb_fsck_options *opts) +{ + struct progress *progress = NULL; + struct packfile_list_entry *e; + uint32_t pack_count = 0; + int res = 0; + + if (opts->flags & ODB_FSCK_PROGRESS) { + for (e = packfile_store_get_packs(source); e; e = e->next) + pack_count++; + progress = start_delayed_progress(source->base.odb->repo, + "Verifying reverse pack-indexes", pack_count); + pack_count = 0; + } + + for (e = packfile_store_get_packs(source); e; e = e->next) { + int load_error = load_pack_revindex_from_disk(e->pack); + + if (load_error < 0) { + error(_("unable to load rev-index for pack '%s'"), e->pack->pack_name); + res = -1; + } else if (!load_error && + !load_pack_revindex(source->base.odb->repo, e->pack) && + verify_pack_revindex(e->pack)) { + error(_("invalid rev-index for pack '%s'"), e->pack->pack_name); + res = -1; + } + display_progress(progress, ++pack_count); + } + stop_progress(&progress); + + return res; +} + static int odb_source_packed_fsck(struct odb_source *source, struct odb_fsck_options *opts) { @@ -870,6 +906,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if ((opts->flags & ODB_FSCK_FULL) && verify_packs(packed, opts) < 0) ret = -1; + if (verify_reverse_indices(packed, opts) < 0) + ret = -1; + return ret; } diff --git a/t/t5325-reverse-index.sh b/t/t5325-reverse-index.sh index 5493791938..6b81abf663 100755 --- a/t/t5325-reverse-index.sh +++ b/t/t5325-reverse-index.sh @@ -204,4 +204,12 @@ test_expect_success 'fsck catches invalid header: hash function' ' "reverse-index file .* has unsupported hash id" ' +test_expect_success 'fsck --no-full checks rev-index, --connectivity-only does not' ' + test_must_fail git -C corrupt fsck --no-full 2>err && + test_grep "has unsupported hash id" err && + + git -C corrupt fsck --connectivity-only 2>err && + test_grep ! "has unsupported hash id" err +' + test_done From b424879085f316b6c9b96d4591339f33c8371737 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:22 +0200 Subject: [PATCH 08/10] builtin/fsck: move bitmap verification into the packed source The checks for bitmaps live in `verify_bitmap_files()`, which is called by "builtin/fsck.c". These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_BITMAP` bit and instead use the generic `ERROR_OBJECT` bit. Note that this change also adapts `verify_bitmap_files()` to be focused on a single "packed" source instead of verifying bitmaps from all sources. This change is required as we already know to loop around the sources in `odb_fsck()` itself. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 5 ----- odb/source-packed.c | 3 +++ pack-bitmap.c | 26 ++++++++++---------------- pack-bitmap.h | 2 +- t/t5326-multi-pack-bitmaps.sh | 10 +++++++++- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 06e72877f3..2f7d29aa56 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -23,7 +23,6 @@ #include "run-command.h" #include "sparse-index.h" #include "worktree.h" -#include "pack-bitmap.h" #define REACHABLE 0x0001 #define SEEN 0x0002 @@ -50,7 +49,6 @@ static timestamp_t now; #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 -#define ERROR_BITMAP 0200 static const char *describe_object(const struct object_id *oid) { @@ -1068,9 +1066,6 @@ int cmd_fsck(int argc, free_worktrees(worktrees); } - if (verify_bitmap_files(repo)) - errors_found |= ERROR_BITMAP; - check_connectivity(repo); if (repo->settings.core_commit_graph) { diff --git a/odb/source-packed.c b/odb/source-packed.c index e5e69636dd..2b5dc502f5 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -909,6 +909,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if (verify_reverse_indices(packed, opts) < 0) ret = -1; + if (verify_bitmap_files(packed)) + ret = -1; + return ret; } diff --git a/pack-bitmap.c b/pack-bitmap.c index e0fb57d332..3de8e9590c 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -3410,28 +3410,22 @@ static int verify_bitmap_file(const struct git_hash_algo *algop, return res; } -int verify_bitmap_files(struct repository *r) +int verify_bitmap_files(struct odb_source_packed *source) { - struct odb_source *source; - struct packed_git *p; + struct packfile_list_entry *e; + struct multi_pack_index *m; int res = 0; - for (source = r->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - struct multi_pack_index *m = get_multi_pack_index(files->packed); - char *midx_bitmap_name; - - if (!m) - continue; - - midx_bitmap_name = midx_bitmap_filename(m); - res |= verify_bitmap_file(r->hash_algo, midx_bitmap_name); + m = get_multi_pack_index(source); + if (m) { + char *midx_bitmap_name = midx_bitmap_filename(m); + res |= verify_bitmap_file(source->base.odb->repo->hash_algo, midx_bitmap_name); free(midx_bitmap_name); } - repo_for_each_pack(r, p) { - char *pack_bitmap_name = pack_bitmap_filename(p); - res |= verify_bitmap_file(r->hash_algo, pack_bitmap_name); + for (e = packfile_store_get_packs(source); e; e = e->next) { + char *pack_bitmap_name = pack_bitmap_filename(e->pack); + res |= verify_bitmap_file(source->base.odb->repo->hash_algo, pack_bitmap_name); free(pack_bitmap_name); } diff --git a/pack-bitmap.h b/pack-bitmap.h index 1385027c1f..847ad4762d 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -205,7 +205,7 @@ int bitmap_is_midx(struct bitmap_index *bitmap_git); int bitmap_is_preferred_refname(struct repository *r, const char *refname); -int verify_bitmap_files(struct repository *r); +int verify_bitmap_files(struct odb_source_packed *source); struct ewah_bitmap *read_bitmap(const unsigned char *map, size_t map_size, size_t *map_pos); diff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh index 86beab1dae..8047459b00 100755 --- a/t/t5326-multi-pack-bitmaps.sh +++ b/t/t5326-multi-pack-bitmaps.sh @@ -498,7 +498,15 @@ test_expect_success 'git fsck correctly identifies good and bad bitmaps' ' corrupt_file "$packbitmap" && test_must_fail git fsck 2>err && test_grep "bitmap file '\''$midxbitmap'\'' has invalid checksum" err && - test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err + test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err && + + # The bitmap checks are performed with "--no-full", but not with + # "--connectivity-only". + test_must_fail git fsck --no-full 2>err && + test_grep "bitmap file '\''$midxbitmap'\'' has invalid checksum" err && + test_grep "bitmap file '\''$packbitmap'\'' has invalid checksum" err && + git fsck --connectivity-only 2>err && + test_grep ! "invalid checksum" err ' test_expect_success 'corrupt MIDX with bitmap causes fallback' ' From 31d10704d4d458e89de371e5a7f13932f54beb7d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:23 +0200 Subject: [PATCH 09/10] builtin/fsck: move multi-pack index verification into the packed source The checks for multi-pack indexes are hosted in `cmd_fsck()` directly. These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_MULTI_PACK_INDEX` bit and instead use the generic `ERROR_OBJECT` bit. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 18 ------------------ odb/source-packed.c | 27 +++++++++++++++++++++++++++ t/t5319-multi-pack-index.sh | 13 +++++++++++++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 2f7d29aa56..7eaea340b0 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -48,7 +48,6 @@ static timestamp_t now; #define ERROR_REACHABLE 02 #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 -#define ERROR_MULTI_PACK_INDEX 040 static const char *describe_object(const struct object_id *oid) { @@ -1085,23 +1084,6 @@ int cmd_fsck(int argc, } } - if (repo->settings.core_multi_pack_index) { - struct child_process midx_verify = CHILD_PROCESS_INIT; - - for (source = repo->objects->sources; source; source = source->next) { - child_process_init(&midx_verify); - midx_verify.git_cmd = 1; - strvec_pushl(&midx_verify.args, "multi-pack-index", - "verify", "--object-dir", source->path, NULL); - if (show_progress) - strvec_push(&midx_verify.args, "--progress"); - else - strvec_push(&midx_verify.args, "--no-progress"); - if (run_command(&midx_verify)) - errors_found |= ERROR_MULTI_PACK_INDEX; - } - } - free_snapshot_refs(&snap); return errors_found; } diff --git a/odb/source-packed.c b/odb/source-packed.c index 2b5dc502f5..9f42552377 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -14,6 +14,7 @@ #include "packfile.h" #include "pack-bitmap.h" #include "progress.h" +#include "run-command.h" static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, @@ -897,6 +898,29 @@ static int verify_reverse_indices(struct odb_source_packed *source, return res; } +static int verify_midx(struct odb_source_packed *source, + struct odb_fsck_options *opts) +{ + struct child_process midx_verify = CHILD_PROCESS_INIT; + int ret = 0; + + if (!source->base.odb->repo->settings.core_multi_pack_index) + return 0; + + child_process_init(&midx_verify); + midx_verify.git_cmd = 1; + strvec_pushl(&midx_verify.args, "multi-pack-index", + "verify", "--object-dir", source->base.path, NULL); + if (opts->flags & ODB_FSCK_PROGRESS) + strvec_push(&midx_verify.args, "--progress"); + else + strvec_push(&midx_verify.args, "--no-progress"); + if (run_command(&midx_verify)) + ret = -1; + + return ret; +} + static int odb_source_packed_fsck(struct odb_source *source, struct odb_fsck_options *opts) { @@ -912,6 +936,9 @@ static int odb_source_packed_fsck(struct odb_source *source, if (verify_bitmap_files(packed)) ret = -1; + if (verify_midx(packed, opts) < 0) + ret = -1; + return ret; } diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b7..20b010c33b 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -573,6 +573,19 @@ test_expect_success 'verify incorrect checksum' ' $objdir "incorrect checksum" ' +test_expect_success 'git fsck --no-full checks multi-pack-index, --connectivity-only does not' ' + pos=$(($(wc -c <$objdir/pack/multi-pack-index) - 10)) && + corrupt_midx_and_verify $pos \ + "\377\377\377\377\377\377\377\377\377\377" \ + $objdir "incorrect checksum" && + + test_must_fail git fsck --no-full 2>err && + test_grep "incorrect checksum" err && + + git fsck --connectivity-only 2>err && + test_grep ! "incorrect checksum" err +' + test_expect_success 'setup for v1-specific fsck tests' ' git -c midx.version=1 multi-pack-index write ' From 6cb1fd749e1155eb34a6eba11c5f95b660a82a07 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 31 Aug 2026 08:46:24 +0200 Subject: [PATCH 10/10] builtin/fsck: move loose object verification into the loose source The consistency checks for loose objects are hosted by "builtin/fsck.c". These checks are obviously specific to the "loose" backend. Move the logic into `odb_source_loose_fsck()`. Introduce a new "verbose" flag so that we can properly retain semantics around whether or not we want to print some status messages. Note that this fixes a bug as a side effect: the progress meter was captured in the callback data before `start_progress()` was even called, so the per-subdirectory progress updates always operated on a NULL pointer and the meter jumped straight from 0 to 256 upon completion. The new code only sets up the callback data's progress meter after it has been created, so the progress display now advances incrementally again. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fsck.c | 91 ++-------------------------------------------- odb.h | 3 ++ odb/source-loose.c | 89 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 90 deletions(-) diff --git a/builtin/fsck.c b/builtin/fsck.c index 7eaea340b0..4af1d874cc 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -12,7 +12,6 @@ #include "parse-options.h" #include "progress.h" #include "packfile.h" -#include "object-file.h" #include "object-name.h" #include "odb.h" #include "odb/streaming.h" @@ -695,88 +694,6 @@ static void process_refs(struct repository *repo, struct snapshot *snap) } } -struct for_each_loose_cb { - struct repository *repo; - struct progress *progress; -}; - -static int fsck_loose(const struct object_id *oid, const char *path, - void *cb_data) -{ - struct for_each_loose_cb *data = cb_data; - enum object_type type = OBJ_NONE; - size_t size; - void *contents = NULL; - int eaten; - struct object_info oi = OBJECT_INFO_INIT; - struct object_id real_oid = *null_oid(data->repo->hash_algo); - int err = 0; - - oi.sizep = &size; - oi.typep = &type; - - if (read_loose_object(data->repo, path, oid, &real_oid, &contents, &oi) < 0) { - if (contents && !oideq(&real_oid, oid)) - err = error(_("%s: hash-path mismatch, found at: %s"), - oid_to_hex(&real_oid), path); - else - err = error(_("%s: object corrupt or missing: %s"), - oid_to_hex(oid), path); - } - if (err < 0) { - errors_found |= ERROR_OBJECT; - free(contents); - return 0; /* keep checking other objects */ - } - - if (!contents && type != OBJ_BLOB) - BUG("read_loose_object streamed a non-blob"); - - if (fsck_obj_buffer(oid, type, size, contents, &eaten, data->repo)) - errors_found |= ERROR_OBJECT; - - if (!eaten) - free(contents); - return 0; /* keep checking other objects, even if we saw an error */ -} - -static int fsck_cruft(const char *basename, const char *path, - void *data UNUSED) -{ - if (!starts_with(basename, "tmp_obj_")) - fprintf_ln(stderr, _("bad sha1 file: %s"), path); - return 0; -} - -static int fsck_subdir(unsigned int nr, const char *path UNUSED, void *data) -{ - struct for_each_loose_cb *cb_data = data; - struct progress *progress = cb_data->progress; - display_progress(progress, nr + 1); - return 0; -} - -static void fsck_source(struct repository *repo, struct odb_source *source) -{ - struct progress *progress = NULL; - struct for_each_loose_cb cb_data = { - .repo = source->odb->repo, - .progress = progress, - }; - - if (verbose) - fprintf_ln(stderr, _("Checking object directory")); - - if (show_progress) - progress = start_progress(repo, - _("Checking object directories"), 256); - - for_each_loose_file_in_source(source, fsck_loose, - fsck_cruft, fsck_subdir, &cb_data); - display_progress(progress, 256); - stop_progress(&progress); -} - static int fsck_cache_tree(struct repository *repo, struct cache_tree *it, const char *index_path) { int i; @@ -978,8 +895,10 @@ int cmd_fsck(int argc, if (show_progress == -1) show_progress = isatty(2); - if (verbose) + if (verbose) { show_progress = 0; + odb_fsck_opts.flags |= ODB_FSCK_VERBOSE; + } if (show_progress) odb_fsck_opts.flags |= ODB_FSCK_PROGRESS; @@ -1012,10 +931,6 @@ int cmd_fsck(int argc, odb_for_each_object(repo->objects, NULL, mark_object_for_connectivity, repo, 0); } else { - for (source = repo->objects->sources; source; source = source->next) - if ((odb_fsck_opts.flags & ODB_FSCK_FULL) || source->local) - fsck_source(repo, source); - if (odb_fsck(repo->objects, &odb_fsck_opts) < 0) errors_found |= ERROR_OBJECT; diff --git a/odb.h b/odb.h index 0bf6c8d7d2..b87f281cbd 100644 --- a/odb.h +++ b/odb.h @@ -218,6 +218,9 @@ enum odb_fsck_flags { /* Display a progress meter, if sensible. */ ODB_FSCK_PROGRESS = (1 << 1), + + /* Be extra verbose when checking the database. */ + ODB_FSCK_VERBOSE = (1 << 2), }; /* Options that shall be passed to `odb_fsck()`. */ diff --git a/odb/source-loose.c b/odb/source-loose.c index f68d3c4d6c..efef9ca61f 100644 --- a/odb/source-loose.c +++ b/odb/source-loose.c @@ -12,6 +12,7 @@ #include "odb/streaming.h" #include "oidtree.h" #include "path.h" +#include "progress.h" #include "repository.h" #include "strbuf.h" #include "tempfile.h" @@ -1031,12 +1032,96 @@ static void odb_source_loose_free(struct odb_source *source) free(loose); } -static int odb_source_loose_fsck(struct odb_source *source UNUSED, - struct odb_fsck_options *opts UNUSED) +struct fsck_loose_data { + struct odb_source_loose *source; + struct odb_fsck_options *opts; + struct progress *progress; + bool error_found; +}; + +static int fsck_loose(const struct object_id *oid, const char *path, + void *cb_data) { + struct fsck_loose_data *data = cb_data; + enum object_type type = OBJ_NONE; + size_t size; + void *contents = NULL; + int eaten = 0; + struct object_info oi = OBJECT_INFO_INIT; + struct object_id real_oid = *null_oid(data->source->base.odb->repo->hash_algo); + int err = 0; + + oi.sizep = &size; + oi.typep = &type; + + if (read_loose_object(data->source->base.odb->repo, + path, oid, &real_oid, &contents, &oi) < 0) { + if (contents && !oideq(&real_oid, oid)) + err = error(_("%s: hash-path mismatch, found at: %s"), + oid_to_hex(&real_oid), path); + else + err = error(_("%s: object corrupt or missing: %s"), + oid_to_hex(oid), path); + } + if (err < 0) + goto out; + + if (!contents && type != OBJ_BLOB) + BUG("read_loose_object streamed a non-blob"); + + if (data->opts->object_cb(oid, type, size, contents, &eaten, + data->opts->object_payload)) { + err = -1; + goto out; + } + +out: + if (err) + data->error_found = true; + if (!eaten) + free(contents); + return 0; /* keep checking other objects, even if we saw an error */ +} + +static int fsck_cruft(const char *basename, const char *path, + void *data UNUSED) +{ + if (!starts_with(basename, "tmp_obj_")) + fprintf_ln(stderr, _("bad sha1 file: %s"), path); return 0; } +static int fsck_subdir(unsigned int nr, const char *path UNUSED, void *cb_data) +{ + struct fsck_loose_data *data = cb_data; + display_progress(data->progress, nr + 1); + return 0; +} + +static int odb_source_loose_fsck(struct odb_source *source, + struct odb_fsck_options *opts) +{ + struct odb_source_loose *loose = odb_source_loose_downcast(source); + struct fsck_loose_data data = { + .source = loose, + .opts = opts, + }; + + if (opts->flags & ODB_FSCK_VERBOSE) + fprintf_ln(stderr, _("Checking object directory")); + + if (opts->flags & ODB_FSCK_PROGRESS) + data.progress = start_progress(source->odb->repo, + _("Checking object directories"), 256); + + for_each_loose_file_in_source(source, fsck_loose, + fsck_cruft, fsck_subdir, &data); + display_progress(data.progress, 256); + stop_progress(&data.progress); + + return data.error_found ? -1 : 0; +} + struct odb_source_loose *odb_source_loose_new(struct object_database *odb, const char *path, bool local)