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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 24 Aug 2021 16:22:42 +0200

On Thu, Aug 19 2021, Emily Shaffer wrote:

> On Tue, Aug 03, 2021 at 09:38:28PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> 
>> 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?

I'll amend this. Yes it's being tested via the CI, and it's the CI that
failed before. I should have said "I have not tested this directly",
i.e. I don't have a local Windows machine, but pushing it to the CI
build that uses this works...

> Otherwise the diff looks straightforward.
>> 
>> 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 <[email-masked]>
> Reviewed-by: Emily Shaffer <[email-masked]>