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

From: Jeff King <peff@peff.net>
Date: Oct 26, 2022, 05:34 UTC
Message-ID: <Y1jG8p1aARb9+CV+@coredump.intra.peff.net>
In-Reply-To: <221026.867d0ncncu.gmgdl@evledraar.gmail.com>

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

Show 13 quoted lines

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

Show 7 quoted lines

> +# 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