Re: [GSoC][PATCH v13 04/10] fsck: add refs-related error report function
From: Patrick Steinhardt <hidden>
Date: 2024-07-30 08:31:31
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"?
quoted hunk ↗ jump to hunk
Mentored-by: Patrick Steinhardt [off-list ref] Mentored-by: Karthik Nayak [off-list ref] Signed-off-by: shejialuo <redacted> --- 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
Attachments
- signature.asc [application/pgp-signature] 833 bytes