On Mon, Oct 31 2022, Taylor Blau wrote:
> On Mon, Oct 31, 2022 at 11:28:05PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> Ævar Arnfjörð Bjarmason (4):
>> Makefile: factor sed-powered '#!/bin/sh' munging into a variable
>> Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier
>> Makefile: rename "test_bindir_programs" variable, pre-declare
>> Makefile: simplify $(test_bindir_programs) rule by splitting it up
>>
>> Makefile | 70 +++++++++++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 51 insertions(+), 19 deletions(-)
>
> Thanks. I replaced the earlier round with this one and pushed the result
> out to ttaylorr/git.
>
> But having read the topic over again, I have to say that I also find the
> pre-existing behavior to be as expected. "make bin-wrappers/git"
> produces its target as expected, but the target is useless because the
> relationship it has with "git" is a runtime dependency, not a Make-time
> dependency.
>
> So I'm not inclined to pick this one up, honestly. Perhaps other
> reviewers feel differently.
I'm fine with changing that. The part where it makes:
make clean &&
make bin-wrappers/git &&
./git ...
work is a 1-line change to remove, i.e.:
diff --git a/Makefile b/Makefile
index d7ab68e3db8..d3c0a66b4b4 100644
--- a/Makefile
+++ b/Makefile
@@ -3089,7 +3089,6 @@ define bin_wrappers_template
# 4='$(4)'
BW_$(1) = $$($(1):%=bin-wrappers/%)
BIN_WRAPPERS += $$(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))
This topic is mainly about untangling the dense $(filter) mess in the
existing rule, which last came up as we need do to special-case
"scalar", and we'd need to special-case any future binaries.
But I see it's my own fault for making that clear, the "as seen in the
range-diff below" in v2's CL [1] was meant to refer to "here's what's
new in v2", not the topic's reason for existing.
So, while I'm happy to re-roll it and remove that one line (and the
associated part of the commit message) I'm surprised that there's such a
hang-up about this aspect of it, to the point where I think I'm missing
something.
Nothing depends on "bin-wrappers/%" now, so having this harms nothing,
but just seems to me to nicely tie off a loose end.
I'm aware that e.g. git-submodule which depends on git-submodule.sh
doesn't depend on e.g. "submodule--helper" being built, so in that sense
this is inconsistent with the rest of our shellscripts.
But in the case of wrap-for-bin.sh its generation is driven by the
Makefile, and it's *only* there to to exec() the target binary (at least
some of those other *.sh's can serve up -h on their own).
And if we don't include that line, then (like on "master"):
$ make clean >/dev/null; make bin-wrappers/git; ./git; echo $?
GIT_VERSION = 2.38.GIT-dev
MKDIR -p bin-wrappers
GEN bin-wrappers/git
bash: ./git: No such file or directory
127
But *shrug*, I just don't see how it's useful, when it's so easy to make
it do something useful.
1. https://lore.kernel.org/git/cover-v2-0.3-00000000000-20221026T143533Z-avarab@xxxxxxxxx/
