Re: [PATCH v4 02/36] Makefile: stop hardcoding {command,config}-list.h

From: Emily Shaffer <emilyshaffer@google.com>
Date: Aug 20, 2021, 00:03 UTC
Message-ID: <YR7xRYWSDAHPH2NS@google.com>
In-Reply-To: <patch-v4-02.36-0f749530777-20210803T191505Z-avarab@gmail.com>

On Tue, Aug 03, 2021 at 09:38:28PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 15 quoted lines

> 
> Change various places that hardcode the names of these two files to
> refer to either $(GENERATED_H), or to a new generated-hdrs
> target. That target is consistent with the *-objs targets I recently
> added in 029bac01a8 (Makefile: add {program,xdiff,test,git,fuzz}-objs
> & objects targets, 2021-02-23).
> 
> A subsequent commit will add a new generated hook-list.h. By doing
> this refactoring we'll only need to add the new file to the
> GENERATED_H variable, not EXCEPT_HDRS, the vcbuild/README etc.
> 
> I have not tested the Windows-specific change in config.mak.uname
> being made here, but we use other variables from the Makefile in the
> same block, and the GENERATED_H is fully defined before we include
> config.mak.uname.

Is it not something you can get coverage for via, for example, the GitHub Actions CI suite? I wonder if that means we want some test to check that these generated lists came together correctly?

Otherwise the diff looks straightforward.

Show 8 quoted lines

> 
> Hardcoding command-list.h there seems to have been a case of
> copy/paste programming in 976aaedca0 (msvc: add a Makefile target to
> pre-generate the Visual Studio solution, 2019-07-29). The
> config-list.h was added later in 709df95b78 (help: move
> list_config_help to builtin/help, 2020-04-16).
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Reviewed-by: Emily Shaffer <emilyshaffer@google.com>