Re: [PATCH v2 2/3] Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Define the variables that make up TEST_OBJS earlier, and don't go back
> & forth in their definition. Before we'd first append $X to
> $(TEST_PROGRAMS), and then substitute $X back out of it to define
> $(TEST_OBJS). Let's instead add a new $(TEST_PROGRAM_OBJS) variable,
> which avoids this needless back & forth substitution.

Makes sense, I guess.  So TEST_OBJS is no longer used?

>  TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))
> +all:: $(TEST_PROGRAMS)

This change is not necessary to achieve the stated goal of this
step, though.  It is one of those "while at it" distraction that
consumes our already constrained reviewer bandwidth, no?

Having said that, "all::" being able to be built up with independent
pieces shine here in this split from the original.  It probably is
easier to reason about while seeing this isolated area of Makefile
what is being done to TEST_PROGRAMS.

The rest of the patch is quite straight-forward renaming of
TEST_OBJS to TEST_PROGRAM_OBJS and an improvement of how the
elements on the list are computed from the source-of-truth list that
is TEST_PROGRAMS_NEED_X that looks correct.

> +TEST_PROGRAM_OBJS += $(patsubst %,t/helper/%.o,$(TEST_PROGRAMS_NEED_X))
> +.PRECIOUS: $(TEST_PROGRAM_OBJS)

>  # List built-in command $C whose implementation cmd_$C() is not in
>  # builtin/$C.o but is linked in as part of some other command.
> @@ -2543,10 +2548,8 @@ REFTABLE_TEST_OBJS += reftable/stack_test.o
>  REFTABLE_TEST_OBJS += reftable/test_framework.o
>  REFTABLE_TEST_OBJS += reftable/tree_test.o
>  
> -TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
> -
>  .PHONY: test-objs
> -test-objs: $(TEST_OBJS)
> +test-objs: $(TEST_PROGRAM_OBJS)
>  
>  GIT_OBJS += $(LIB_OBJS)
>  GIT_OBJS += $(BUILTIN_OBJS)
> @@ -2562,7 +2565,7 @@ scalar-objs: $(SCALAR_OBJS)
>  OBJECTS += $(GIT_OBJS)
>  OBJECTS += $(SCALAR_OBJS)
>  OBJECTS += $(PROGRAM_OBJS)
> -OBJECTS += $(TEST_OBJS)
> +OBJECTS += $(TEST_PROGRAM_OBJS)
>  OBJECTS += $(XDIFF_OBJS)
>  OBJECTS += $(FUZZ_OBJS)
>  OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
> @@ -3061,7 +3064,7 @@ endif
>  
>  test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
>  
> -all:: $(TEST_PROGRAMS) $(test_bindir_programs)
> +all:: $(test_bindir_programs)
>  
>  bin-wrappers/%: wrap-for-bin.sh
>  	$(call mkdir_p_parent_template)
> @@ -3087,8 +3090,6 @@ perf: all
>  
>  .PHONY: test perf
>  
> -.PRECIOUS: $(TEST_OBJS)
> -
>  t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
>  
>  t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) $(REFTABLE_TEST_LIB)