On Thu, May 30, 2024 at 08:27:52PM +0800, shejialuo wrote:
> Add a new interface `fsck_refs_fn` for the `ref_storage_be`. Implement
> dummy method for files and reftable backends.
>
> Mentored-by: Patrick Steinhardt <ps@xxxxxx>
> Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> Signed-off-by: shejialuo <shejialuo@xxxxxxxxx>
> ---
> builtin/fsck.c | 5 +++++
> refs.c | 5 +++++
> refs.h | 5 +++++
> refs/files-backend.c | 9 ++++++++-
> refs/packed-backend.c | 7 +++++++
> refs/refs-internal.h | 4 ++++
> refs/reftable-backend.c | 7 +++++++
> 7 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/fsck.c b/builtin/fsck.c
> index d13a226c2e..65a26e2d1b 100644
> --- a/builtin/fsck.c
> +++ b/builtin/fsck.c
> @@ -1065,6 +1065,11 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
>
> check_connectivity();
>
> + if (refs_fsck(get_main_ref_store(the_repository))) {
> + error("ref database is corrupt");
> + errors_found |= ERROR_REFS;
> + }
One thing I was wondering is whether we want to make the infrastructure
part of the new git-refs(1) command, which is about to land via my
series that introduces ref backend migrations. We already make it a
habit that we spawn new processes here, as you can see in two lines
below for example. It would also make it trivial to execute ref checks
standalone, by saying e.g. `git refs verify`.
> if (the_repository->settings.core_commit_graph) {
> struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
>
[snip]
> diff --git a/refs.h b/refs.h
> index 34568ee1fb..2799820c40 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -544,6 +544,11 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat
> */
> int check_refname_format(const char *refname, int flags);
>
> +/*
> + * Return 0 iff all refs in filesystem are consistent.
> +*/
> +int refs_fsck(struct ref_store *refs);
We should probably also mention that errors will be written to `stderr`?
Also, I noticed that you are missing the implementation for the debug
backend in "refs/debug.c".
> /*
> * Apply the rules from check_refname_format, but mutate the result until it
> * is acceptable, and place the result in "out".
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 5f3089d947..b6147c588b 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3299,6 +3299,11 @@ static int files_init_db(struct ref_store *ref_store,
> return 0;
> }
>
> +static int files_fsck(struct ref_store *ref_store)
> +{
> + return 0;
> +}
We can already wire up the call to the embedded packed-refs ref store
here. We always want to call that from the files backend.
Patrick
Attachment:
signature.asc
Description: PGP signature
