Thread (159 messages) 159 messages, 9 authors, 2021-03-28

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

From: Jonathan Tan <hidden>
Date: 2021-02-17 20:06:07

quoted
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/87blcja2ha.fsf@evledraar.gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help