Re: [PATCH 4/4] fetch-pack: print and use dangling .gitmodules

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jan 28, 2021, 01:03 UTC
Message-ID: <20210128010320.3880242-1-jonathantanmy@google.com>
In-Reply-To: <87ft2q7ck4.fsf@evledraar.gmail.com>

Show 11 quoted lines

> 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.

Show 5 quoted lines

>         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/

Show 8 quoted lines

> 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.