> On Sun, Jan 24 2021, Jonathan Tan wrote: > > > +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. > 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@xxxxxxxxxxxxxxxxxxx/ > 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.