On Wed, Sep 03, 2025 at 10:07:13AM +0200, Patrick Steinhardt wrote:
[snip]
> > +static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,
> > struct worktree *wt UNUSED)
> > {
> > - return 0;
> > + 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");
> > +
> > + if (o->verbose)
> > + fprintf_ln(stderr, _("Checking references consistency"));
>
> This line is duplicate across both backends, right? Maybe it's something
> that we can do in the generic logic?
>
That's right, it is duplicate. If we want to remove this, we need to do
this in the "builtin/refs.c". But I wonder whether we should do this in
the first place. Should we rather add more detailed information just
like the following code for packed backend?
if (o->verbose)
fprintf_ln(stderr, "Checking packed-refs file %s", refs->path);
Instead of just using
Checking references consistency
Could we use
Checking reftable references consistency
However, I also feel strange about above, :)
[snip]
> > +/* Represents an individual error encountered during the FSCK checks. */
> > +struct reftable_fsck_info {
> > + enum reftable_fsck_error error;
> > + const char *msg;
> > + const char *path;
> > +};
>
> I wonder whether it should be the reftable library that decides on the
> severity of each generated finding.
>
That's an interesting question. Let's inspect how Git handles the
severity. When defining the fsck message id, we need to specify its
severity like the following shows, this happens at compile time:
FUNC(BAD_REFERENT_NAME, ERROR)
And we could set the configuration "fsck.[message id]=" to change the
fsck message severity.
Then let's think if reftable library decides the severity. It means that
we need to use the API from reftable library to update
"fsck_option->msg_type" at the runtime. And it is bad because the fsck
infrastructure would be highly coupled with the reftable library.
So, I don't think it's a good idea for reftable library to choose the
severity. Instead, reftable library should just provide users with error
types and let the users decide the severity.
Thanks,
Jialuo
