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

From: Jonathan Tan <[email-masked]>
Date: Wed, 17 Feb 2021 12:05:22 -0800

> > 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 can't strstr() because of false positives (if, e.g. there is a
submodule name that contains the string we're looking for), but looking
at report() in fsck.c, the message ID is the very first thing appended,
so I think we can use starts_with().

> We document & hard rely on that already, since it's also a
> config key.

Ah, good point.

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

I agree - I thought this wouldn't be possible, but like you said, we can
reliably make use of the string in report() (or pass the ID, like your
patch set [1] does) so we should do this.

What would be the best way to proceed, now that we have at least 2 patch
sets (mine and yours) in play? I was thinking that I should update my
one to use the string reported in report() (with starts_with()), so that
both our patch sets can be reviewed and merged in parallel, and after
that, update the fsck code to use the ID instead of the string.

[1] https://lore.kernel.org/git/[email-masked]/