On Mon, Jul 29, 2024 at 09:26:51PM +0800, shejialuo wrote: > Add refs-related options to the "fsck_options", create refs-specific > "error_func" callback "fsck_refs_error_function". We should have an explanation _why_ we are adding these functions in the commit message. > "fsck_refs_error_function" will use the "oid" parameter. When the caller > passes the oid, it will use "oid_to_hex" to get the corresponding hex > value to report to the caller. > > Last, add "FSCK_REFS_OPTIONS_DEFAULT" and "FSCK_REFS_OPTIONS_STRICT" > macros to create refs options easily. It is a bit unclear to me what you mean with "create refs options easily". Do you mean to say that `git refs check` (or whatever this will be called) will have flags like "--strict"? > Mentored-by: Patrick Steinhardt <ps@xxxxxx> > Mentored-by: Karthik Nayak <karthik.188@xxxxxxxxx> > Signed-off-by: shejialuo <shejialuo@xxxxxxxxx> > --- > fsck.c | 25 +++++++++++++++++++++++++ > fsck.h | 14 ++++++++++++++ > 2 files changed, 39 insertions(+) > > diff --git a/fsck.c b/fsck.c > index af61fa90ba..56de29b4c0 100644 > --- a/fsck.c > +++ b/fsck.c > @@ -1251,6 +1251,31 @@ int fsck_objects_error_function(struct fsck_options *o, > return 1; > } > > +int fsck_refs_error_function(struct fsck_options *options, > + const struct object_id *oid, > + enum object_type object_type UNUSED, > + const struct fsck_refs_info *refs_info, > + enum fsck_msg_type msg_type, > + enum fsck_msg_id msg_id UNUSED, > + const char *message) > +{ > + struct strbuf sb = STRBUF_INIT; > + int ret = 0; > + > + strbuf_addstr(&sb, refs_info->path); > + > + if (oid) > + strbuf_addf(&sb, " -> (%s)", oid_to_hex(oid)); Okay, so we do end up printing the object ID indeed. But wouldn't we want to potentially do the same with symbolic refs? Also, would it make more sense to put the `oid` (and potentially the `referent` when we also handle symbolic refs) into `struct fsck_refs_info`? Like this, the whole state would be self-contained in that structure, which would also make my proposal from a preceding commit more feasible where the subsystem-specific error functions only get a void pointer to this structure. It would require another refactoring on top to move the object type and OID into a `struct fsck_objects_info`, too, but that shouldn't be too involved, I guess. Patrick
Attachment:
signature.asc
Description: PGP signature