From: Karthik Nayak <karthik.188@gmail.com>
Date: Mon, 31 Aug 2026 09:26:57 GMT
Subject: Re: [PATCH v2 00/10] odb: make consistency checks pluggable
Message-ID: <CAOLa=ZSi1TiTZ=i=SQp+pmjTOm2_wY-NiCotx66+M6VDKx=ZXg@mail.gmail.com>
In-Reply-To: <20260831-pks-odb-source-fsck-v2-0-f9b16ef4957b@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> Hi,
>
> this patch series makes object database consistency checks pluggable.
>
> This series is built on top of 2c3adbb2c4 (The 18th batch, 2026-08-24)
> with the following two dependencsie merged into it:
>
>   - ps/odb-eagerly-load-alternates at 0076dc9f81 (odb: drop
>     `alternates_db` field, 2026-08-17)
>
>   - ps/odb-pluggable-pack-generation at 5176dd3d05 (bundle: generate
>     packfiles via the object database, 2026-08-21)
>
> Changes in v2:
>   - Some commit message improvements.
>   - Link to v1: https://patch.msgid.link/20260825-pks-odb-source-fsck-v1-0-b756de0bf24f@pks.im
>
> Thanks!
>
> Patrick
>
> ---
> Patrick Steinhardt (10):
>       builtin/fsck: use `fsck_obj_buffer()` when checking loose objects
>       builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()`
>       builtin/fsck: de-globalize option handling
>       builtin/fsck: don't check alternates with "--no-full"
>       odb: provide infrastructure for pluggable fsck checks
>       builtin/fsck: move packfile verification into the packed source
>       builtin/fsck: move reverse index verification into the packed source
>       builtin/fsck: move bitmap verification into the packed source
>       builtin/fsck: move multi-pack index verification into the packed source
>       builtin/fsck: move loose object verification into the loose source
>
>  builtin/fsck.c                | 296 ++++++++----------------------------------
>  odb.c                         |   9 ++
>  odb.h                         |  33 +++++
>  odb/source-files.c            |  13 ++
>  odb/source-inmemory.c         |   8 ++
>  odb/source-loose.c            |  92 +++++++++++++
>  odb/source-packed.c           | 117 +++++++++++++++++
>  odb/source.h                  |  21 +++
>  pack-bitmap.c                 |  26 ++--
>  pack-bitmap.h                 |   2 +-
>  t/t1450-fsck.sh               |   5 +
>  t/t5319-multi-pack-index.sh   |  13 ++
>  t/t5325-reverse-index.sh      |   8 ++
>  t/t5326-multi-pack-bitmaps.sh |  10 +-
>  14 files changed, 394 insertions(+), 259 deletions(-)
>
> Range-diff versus v1:
>
>  1:  cf49376600 !  1:  1aec903546 builtin/fsck: use `fsck_obj_buffer()` when checking loose objects
>     @@ Commit message
>
>          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()`.
>     -    Almost the exact same steps are also performed by `fsck_obj_buffer()`.
>     +    The exact same steps are also performed by `fsck_obj_buffer()`.
>
>          Stop open-coding this logic and call `fsck_obj_buffer()` instead.
>
>  2:  da2ca27041 !  2:  3804f0339e builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()`
>     @@ Commit message
>          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 in
>     -    `fsck_obj()` in case `SEEN` is set as the only caller unconditionally
>     -    clears that bit before calling it anyway.
>     +    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 <ps@pks.im>
>
>  3:  a24506f55e =  3:  b2cb9032cf builtin/fsck: de-globalize option handling
>  4:  f6a407efd0 =  4:  10ee3b8baf builtin/fsck: don't check alternates with "--no-full"
>  5:  31841a1f05 =  5:  1e65eec60e odb: provide infrastructure for pluggable fsck checks
>  6:  2cd6d71983 =  6:  0b8cf751aa builtin/fsck: move packfile verification into the packed source
>  7:  c0559f1820 =  7:  3a38a75549 builtin/fsck: move reverse index verification into the packed source
>  8:  96ae1ce3c6 !  8:  dd3a4c6cea builtin/fsck: move bitmap verification into the packed source
>     @@ Commit message
>          instead use the generic `ERROR_OBJECT` bit.
>
>          Note that this change also adapts `verify_bitmap_files()` to be
>     -    focussed on a single "packed" source instead of verifying bitmaps from
>     +    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.
>
>  9:  4721f4b4ba =  9:  90ada56b7f builtin/fsck: move multi-pack index verification into the packed source
> 10:  0b36829fd9 = 10:  b0f6fccae8 builtin/fsck: move loose object verification into the loose source
>
> ---
> base-commit: 6b08999fb1b3ad0bad04d492dc206ad42839e274
> change-id: 20260810-pks-odb-source-fsck-e64772c7ee5f

The changes here look good to me, thanks!