Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids

[Jeff King <peff@peff.net>]

On Wed, Oct 26, 2022 at 04:43:32AM +0200, Ævar Arnfjörð Bjarmason wrote:

> 
> On Tue, Oct 25 2022, Junio C Hamano wrote:
> 
> > During the initial development of the fsck-msgids.txt feature, it
> > has become apparent that it is very much error prone to make sure
> > the description in the documentation file are sorted and correctly
> > match what is in the fsck.h header file.
> 
> I have local fixes for the same issues in the list of advice in our
> docs, some of it's missing, wrong, out of date etc.
> 
> I tried to quickly adapt the generation script I had for that, which
> works nicely, and by line count much shorter than the lint :)

Yeah, my instinct here was to generate rather than lint. If you make a
mistake and the linter hits you over the head, that is better than
quietly letting your mistake go. But better still is making it
impossible to make in the first place.

The downside is added complexity to the build, but it doesn't seem too
bad in this case.

(I had a similar thought after getting hit on the head by the recent
t0450-txt-doc-vs-help.sh).

> Having to exhaustively list every *.c file that uses fsck.h is a bit of
> a bother, but we have the same with the other generated *.h's, so it
> shouldn't be too bad.

It feels like this could be made much shorter by having a separate
fsck-msgs.h and not including it from fsck.h. Only fsck.c and mktag.c
need the actual list. It would probably have to stop being an "enum",
though.

Another alternative is to generate the doc from the code, rather than
the other way around.

> +# Unfortunately our dependency management of generated headers used
> +# from within other headers suck, so we'll need to list every user of
> +# fsck.h here, but not too bad...
> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects
> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h
> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck
> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h

I don't understand the "used within other headers" part here. Computed
dependencies will get this right. It's only the initial build (before we
have any computed dependencies) that needs to know that the C files in
question depend on the generated file. But that is true whether they do
so directly or indirectly.

-Peff