On Thu, Jan 28 2021, Jonathan Tan wrote:

Sorry I managed to miss this at the time. Hopefully a late reply is
better than never.

quoted

On Sun, Jan 24 2021, Jonathan Tan wrote:

quoted

+void register_found_gitmodules(const struct object_id *oid)
+{
+	oidset_insert(&gitmodules_found, oid);
+}
+

In fsck.c we only use this variable to insert into it, or in fsck_blob()
to do the actual check, but then we either abort early if we've found
it, or right after that:

By "this variable", do you mean gitmodules_found? fsck_finish() consumes
it.

Yes, consumes it to emit errors with report(), no?

quoted

        if (object_on_skiplist(options, oid))
                return 0;

So (along with comments I have below...) you could just use the existing
"skiplist" option instead, no?

I don't understand this part (in particular, the part you quoted). About
"skiplist", I'll reply to your other email [1] which has more details.

[1] https://lore.kernel.org/git/87czxu7c15.fsf@evledraar.gmail.com/ (local)

*nod*

quoted

This whole thing seems just like the bad path I took in earlier rounds
of my in-flight mktag series. You don't need this new custom API. You
just setup an error handler for your fsck which ignores / prints / logs
/ whatever the OIDs you want if you get a FSCK_MSG_GITMODULES_MISSING
error, which you then "return 0" on.

If you don't have FSCK_MSG_GITMODULES_MISSING punt and call
fsck_error_function().

I tried that first, and the issue is that IDs like
FSCK_MSG_GITMODULES_MISSING are internal to fsck.c. As for whether we
should start exposing the IDs publicly, I think we should wait until a
few new cases like this come up, so that we more fully understand the
requirements first.

The requirement is that you want the objects ids we'd otherwise error
about in fsck_finish(). Yeah we don't pass the "fsck_msg_id" down in the
"report()" function, but you can reliably strstr() it out of the
message. We document & hard rely on that already, since it's also a
config key.

But yeah, we could just change the report function to pass down the id
and move the relevant macros from fsck.c to fsck.h. I think that would
be a smaller change conceptually than a special-case flag in
fsck_options for something we could otherwise do with the error
reporting.