[PATCH 0/5] Makefile: split up $(test_bindir_programs)

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Sep 01 2022, Johannes Schindelin wrote:

> [...]
> On Wed, 31 Aug 2022, Victoria Dye via GitGitGadget wrote:
> [...]
>> @@ -3062,7 +3067,7 @@ bin-wrappers/%: wrap-for-bin.sh
>>  	$(call mkdir_p_parent_template)
>>  	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>>  	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
>> -	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \
>> +	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))$(if $(filter-out $(BINDIR_PROGRAMS_NO_X),$(@F)),$(X),)|' < $< > $@ && \
>
> It took me a good while to wrap my head around this (and let me be clear:
> I consider none of this your fault, it's the fault of the design of the
> Makefile syntax).
>
> Let me untangle this, for posterity's benefit. We substitute the
> placeholder `@@PROG@@` with a concatenation of two strings that are both
> derived from `@F`, i.e. the basename of the to-be-wrapped command.

We could do this later, but the 3/5 here is my reply to the "fault of
the design of the Makfile syntax".

I really don't think that's the case, the problem here is something
you'd get any any language.

We have three lists which we'd like to treat differently, but for no
particularly good reason other than incrementally building on past
changes to end up with this we end up having to on-the-fly guess which
list a given item came from.

With this series the end result is instead to do:
	
	define bin_wrappers_template
	BW_$(1) = $$($(1):%=bin-wrappers/%)
	BIN_WRAPPERS += $$(BW_$(1))
	all:: $$(BW_$(1))
	$$(BW_$(1)): bin-wrappers/% : $(3)%$(4)
	$$(BW_$(1)): wrap-for-bin.sh
		$$(call mkdir_p_parent_template)
		$$(QUIET_GEN)$$(call cmd_munge_bin_wrappers_script,$(2),$(3),$(4))
	endef
	
	$(eval $(call bin_wrappers_template,BINDIR_PROGRAMS_NEED_X,'$$(@F)',,$$X))
	$(eval $(call bin_wrappers_template,BINDIR_PROGRAMS_NO_X,'$$(@F)'))
	$(eval $(call bin_wrappers_template,TEST_PROGRAMS_NEED_X,'$$(@F)',t/helper/,$$X))
	
	all:: $(BIN_WRAPPERS)

This obviously conflicts with Victoria's changes here, but if picked
up the conflict can be entirely solved in favor of this series, and
this "scalar" series will benefit.

I.e. the only reason this series needs to patch this one liner is
because the Makefile is losing track of where the item(s) came from.

Once we're not doing that we're perfectly capable of creating a
bin-wrappers/scalar, because we're no longer running into the logic
that uses git% as a heuristic to determine whether something is "not
from the $(TEST_PROGRAMS_NEED_X) variable".

A CI run, showing that this also works on Windows etc.:
https://github.com/avar/git/runs/8135048620

Ævar Arnfjörð Bjarmason (5):
  Makefile: factor sed-powered '#!/bin/sh' munging into a variable
  Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier
  Makefile: simplify $(test_bindir_programs) rule by splitting it up
  Makefile: define bin-wrappers/% rules with a template
  Makefile: fix "make clean && make bin-wrappers/$NAME" dependencies

 Makefile | 58 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 19 deletions(-)

-- 
2.37.3.1426.g360dd7cf8ca