| Message ID | patch-v4-02.36-0f749530777-20210803T191505Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Run hooks via "git run hook" & hook library | expand |
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? 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 <avarab@gmail.com> Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
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 <avarab@gmail.com> > Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
diff --git a/Makefile b/Makefile index 2ff038069e8..89bf0dd7332 100644 --- a/Makefile +++ b/Makefile @@ -823,6 +823,8 @@ XDIFF_LIB = xdiff/lib.a GENERATED_H += command-list.h GENERATED_H += config-list.h +.PHONY: generated-hdrs +generated-hdrs: $(GENERATED_H) LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ $(FIND) . \ @@ -2909,7 +2911,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE .PHONY: sparse $(SP_OBJ) sparse: $(SP_OBJ) -EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/% +EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% ifndef GCRYPT_SHA256 EXCEPT_HDRS += sha256/gcrypt.h endif @@ -2932,7 +2934,7 @@ style: git clang-format --style file --diff --extensions c,h .PHONY: check -check: config-list.h command-list.h +check: $(GENERATED_H) @if sparse; \ then \ echo >&2 "Use 'make sparse' instead"; \ diff --git a/compat/vcbuild/README b/compat/vcbuild/README index 51fb083dbbe..29ec1d0f104 100644 --- a/compat/vcbuild/README +++ b/compat/vcbuild/README @@ -92,7 +92,7 @@ The Steps of Build Git with VS2008 the git operations. 3. Inside Git's directory run the command: - make command-list.h config-list.h + make generated-hdrs to generate the header file needed to compile git. 4. Then either build Git with the GNU Make Makefile in the Git projects diff --git a/config.mak.uname b/config.mak.uname index 69413fb3dc0..9988378160b 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -732,9 +732,9 @@ vcxproj: echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets - # Add command-list.h and config-list.h - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h - git add -f config-list.h command-list.h + # Add generated headers + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(GENERATED_H) + git add -f $(GENERATED_H) # Add scripts rm -f perl/perl.mak
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. 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> --- Makefile | 6 ++++-- compat/vcbuild/README | 2 +- config.mak.uname | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-)