[PATCH v2 0/8] Makefile: optimize noop runs, add shared.mak

[PATCH v2 0/8] Makefile: optimize noop runs, add shared.mak

[Ævar Arnfjörð Bjarmason]

As the v1 notes (among other things):
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

    This speeds up noop runs of "make" by a lot. After a "make" running a
    "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
    as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
    but that's another matter).

This v2 re-roll addresses trivial commit message/comment grammar/typo
issues pointed out by Eric Sunshine, thanks Eric!

Ævar Arnfjörð Bjarmason (8):
  Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
  Makefile: disable GNU make built-in wildcard rules
  Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
  Makefile: move ".SUFFIXES" rule to shared.mak
  Makefile: move $(comma), $(empty) and $(space) to shared.mak
  Makefile: add "$(QUIET)" boilerplate to shared.mak
  Makefile: use $(wspfx) for $(QUIET...) in shared.mak
  Makefiles: add and use wildcard "mkdir -p" template

 Documentation/Makefile |  63 +++-------------------
 Makefile               | 118 +++++++++++++----------------------------
 config.mak.uname       |   1 -
 shared.mak             | 109 +++++++++++++++++++++++++++++++++++++
 t/Makefile             |   3 ++
 t/interop/Makefile     |   3 ++
 templates/Makefile     |   8 ++-
 7 files changed, 160 insertions(+), 145 deletions(-)
 create mode 100644 shared.mak

Range-diff against v1:
1:  f74b47662b7 = 1:  b652fc78fda Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
2:  b0c63abe091 = 2:  b0c9be581a6 Makefile: disable GNU make built-in wildcard rules
3:  c6c6f7cf8d8 = 3:  ed6fd1c0bd1 Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
4:  ed64cd1bd4a = 4:  4c6d8089fff Makefile: move ".SUFFIXES" rule to shared.mak
5:  1749085b929 = 5:  f1f02c71dbc Makefile: move $(comma), $(empty) and $(space) to shared.mak
6:  c25284b24cf = 6:  fb877060d6b Makefile: add "$(QUIET)" boilerplate to shared.mak
7:  3daef7672be = 7:  90d804ea9a0 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
8:  aca560ca410 ! 8:  59c1b7032db Makefiles: add and use wildcard "mkdir -p" template
    @@ Commit message
     
         But as it turns out we can use this neat trick of only doing a "mkdir
         -p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
    -    of a performance test similar to thatnoted downthread of [1] in [2]
    +    of a performance test similar to that noted downthread of [1] in [2]
         shows that this is faster, in addition to being less verbose and more
         reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):
     
    @@ shared.mak: ifndef V
     +## needed.
     +##
     +## Is racy, but in a good way; we might redundantly (and safely)
    -+## "mkdir -p" when running in parallel, but won't need to exhaustively
    ++## "mkdir -p" when running in parallel, but won't need to exhaustively create
     +## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
     +## "a/prefix/dir/file". This can instead be inserted at the start of
     +## the "a/prefix/dir/file" rule.
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 1/8] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

We have various behavior that's shared across our Makefiles, or that
really should be (e.g. via defined templates). Let's create a
top-level "shared.mak" to house those sorts of things, and start by
adding the ".DELETE_ON_ERROR" flag to it.

See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
.DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
".DELETE_ON_ERROR" flag.

This does have the potential downside that if e.g. templates/Makefile
would like to include this "shared.mak" in the future the semantics of
such a Makefile will change, but as noted in the above commits (and
GNU make's own documentation) any such change would be for the better,
so it's safe to do this.

This also doesn't introduce a bug by e.g. having this
".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
have no such scoping semantics.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile |  6 +++---
 Makefile               | 13 +++----------
 shared.mak             |  9 +++++++++
 t/Makefile             |  3 +++
 t/interop/Makefile     |  3 +++
 templates/Makefile     |  3 +++
 6 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 shared.mak

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ed656db2ae9..ba27456c86a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Guard against environment variables
 MAN1_TXT =
 MAN5_TXT =
@@ -524,7 +527,4 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
-# Delete the target file on error
-.DELETE_ON_ERROR:
-
 .PHONY: FORCE
diff --git a/Makefile b/Makefile
index 75ed168adbc..7dddea3caf3 100644
--- a/Makefile
+++ b/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include shared.mak
+
 # The default target of this Makefile is...
 all::
 
@@ -2169,16 +2172,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $^
 
-### Flags affecting all rules
-
-# A GNU make extension since gmake 3.72 (released in late 1994) to
-# remove the target of rules if commands in those rules fail. The
-# default is to only do that if make itself receives a signal. Affects
-# all targets, see:
-#
-#    info make --index-search=.DELETE_ON_ERROR
-.DELETE_ON_ERROR:
-
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
diff --git a/shared.mak b/shared.mak
new file mode 100644
index 00000000000..0170bb397ae
--- /dev/null
+++ b/shared.mak
@@ -0,0 +1,9 @@
+### Flags affecting all rules
+
+# A GNU make extension since gmake 3.72 (released in late 1994) to
+# remove the target of rules if commands in those rules fail. The
+# default is to only do that if make itself receives a signal. Affects
+# all targets, see:
+#
+#    info make --index-search=.DELETE_ON_ERROR
+.DELETE_ON_ERROR:
diff --git a/t/Makefile b/t/Makefile
index 46cd5fc5273..056ce55dcc9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Run tests
 #
 # Copyright (c) 2005 Junio C Hamano
diff --git a/t/interop/Makefile b/t/interop/Makefile
index 31a4bbc716a..6911c2915a7 100644
--- a/t/interop/Makefile
+++ b/t/interop/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..636cee52f51 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # make and install sample templates
 
 ifndef V
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 2/8] Makefile: disable GNU make built-in wildcard rules

[Ævar Arnfjörð Bjarmason]

Override built-in rules of GNU make that use a wildcard target. This
can speeds things up significantly as we don't need to stat() so many
files. GNU make does that by default to see if it can retrieve their
contents from RCS or SCCS. See [1] for an old mailing list discussion
about how to disable these.

The speed-up may wary. I've seen 1-10% depending on the speed of the
local disk, caches, -jN etc. Running:

    strace -f -c -S calls make -j1 NO_TCLTK=Y

Shows that we reduce the number of syscalls we make, mostly in "stat"
calls.

We could also invoke make with "-r" by setting "MAKEFLAGS = -r"
early. Doing so might make us a bit faster still. But doing so is a
much bigger hammer, since it will disable all built-in rules,
some (all?) of which can be seen with:

    make -f/dev/null -p | grep -v -e ^# -e ^$

We may have something that relies on them, so let's go for the more
isolated optimization here that gives us most or all of the wins.

1. https://lists.gnu.org/archive/html/help-make/2002-11/msg00063.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/shared.mak b/shared.mak
index 0170bb397ae..29f0e69ecb9 100644
--- a/shared.mak
+++ b/shared.mak
@@ -1,3 +1,14 @@
+### Remove GNU make implicit rules
+
+## This speeds things up since we don't need to look for and stat() a
+## "foo.c,v" every time a rule referring to "foo.c" is in play. See
+## "make -p -f/dev/null | grep ^%::'".
+%:: %,v
+%:: RCS/%,v
+%:: RCS/%
+%:: s.%
+%:: SCCS/s.%
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 3/8] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)

[Ævar Arnfjörð Bjarmason]

Combine the definitions of $(FIND_SOURCE_FILES) and $(LIB_H) to speed
up the Makefile, as these are the two main expensive $(shell) commands
that we execute unconditionally.

When see what was in $(FOUND_SOURCE_FILES) that wasn't in $(LIB_H) via
the ad-hoc test of:

    $(error $(filter-out $(LIB_H),$(filter %.h,$(ALL_SOURCE_FILES))))
    $(error $(filter-out $(ALL_SOURCE_FILES),$(filter %.h,$(LIB_H))))

We'll get, respectively:

    Makefile:850: *** t/helper/test-tool.h.  Stop.
    Makefile:850: *** .  Stop.

I.e. we only had a discrepancy when it came to
t/helper/test-tool.h. In terms of correctness this was broken before,
but now works:

    $ make t/helper/test-tool.hco
        HDR t/helper/test-tool.h

This speeds things up a lot:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make NO_TCLTK=Y' 'make -j1 NO_TCLTK=Y' --warmup 10 -M 10
    Benchmark 1: make -j1 NO_TCLTK=Y' in 'HEAD~1
      Time (mean ± σ):     159.9 ms ±   6.8 ms    [User: 137.2 ms, System: 28.0 ms]
      Range (min … max):   154.6 ms … 175.9 ms    10 runs

    Benchmark 2: make -j1 NO_TCLTK=Y' in 'HEAD~0
      Time (mean ± σ):     100.0 ms ±   1.3 ms    [User: 84.2 ms, System: 20.2 ms]
      Range (min … max):    98.8 ms … 102.8 ms    10 runs

    Summary
      'make -j1 NO_TCLTK=Y' in 'HEAD~0' ran
        1.60 ± 0.07 times faster than 'make -j1 NO_TCLTK=Y' in 'HEAD~1'

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 54 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index 7dddea3caf3..d3ce0d22199 100644
--- a/Makefile
+++ b/Makefile
@@ -826,12 +826,33 @@ GENERATED_H += hook-list.h
 .PHONY: generated-hdrs
 generated-hdrs: $(GENERATED_H)
 
-LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
+## Exhaustive lists of our source files, either dynamically generated,
+## or hardcoded.
+SOURCES_CMD = ( \
+	git ls-files \
+		'*.[hcS]' \
+		'*.sh' \
+		':!*[tp][0-9][0-9][0-9][0-9]*' \
+		':!contrib' \
+		2>/dev/null || \
 	$(FIND) . \
-	-name .git -prune -o \
-	-name t -prune -o \
-	-name Documentation -prune -o \
-	-name '*.h' -print)))
+		\( -name .git -type d -prune \) \
+		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
+		-o \( -name contrib -type d -prune \) \
+		-o \( -name build -type d -prune \) \
+		-o \( -name 'trash*' -type d -prune \) \
+		-o \( -name '*.[hcS]' -type f -print \) \
+		-o \( -name '*.sh' -type f -print \) \
+		| sed -e 's|^\./||' \
+	)
+FOUND_SOURCE_FILES := $(shell $(SOURCES_CMD))
+
+FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
+FOUND_H_SOURCES = $(filter %.h,$(FOUND_SOURCE_FILES))
+
+COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
+
+LIB_H = $(FOUND_H_SOURCES)
 
 LIB_OBJS += abspath.o
 LIB_OBJS += add-interactive.o
@@ -2758,26 +2779,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	pod2man $< $@
 
-FIND_SOURCE_FILES = ( \
-	git ls-files \
-		'*.[hcS]' \
-		'*.sh' \
-		':!*[tp][0-9][0-9][0-9][0-9]*' \
-		':!contrib' \
-		2>/dev/null || \
-	$(FIND) . \
-		\( -name .git -type d -prune \) \
-		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
-		-o \( -name contrib -type d -prune \) \
-		-o \( -name build -type d -prune \) \
-		-o \( -name 'trash*' -type d -prune \) \
-		-o \( -name '*.[hcS]' -type f -print \) \
-		-o \( -name '*.sh' -type f -print \) \
-		| sed -e 's|^\./||' \
-	)
-
-FOUND_SOURCE_FILES = $(shell $(FIND_SOURCE_FILES))
-
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
 	echo $(FOUND_SOURCE_FILES) | xargs etags -a -o $@+ && \
@@ -2987,9 +2988,6 @@ check: $(GENERATED_H)
 		exit 1; \
 	fi
 
-FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
-COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
-
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	$(QUIET_SPATCH) \
 	if test $(SPATCH_BATCH_SIZE) = 0; then \
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 4/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

This was added in 30248886ce8 (Makefile: disable default implicit
rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
apply to all our Makefiles.

This doesn't benefit the main Makefile at all, since it already had
the rule, but since we're including shared.mak in other Makefiles
starts to benefit them. E.g. running the 'man" target is now faster:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
    Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
      Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
      Range (min … max):   112.8 ms … 148.4 ms    26 runs

    Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
      Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
      Range (min … max):    89.8 ms … 111.8 ms    32 runs

    Summary
      'make -C Documentation -j1 man' in 'HEAD~0' ran
        1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 --
 shared.mak | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d3ce0d22199..e30093b83d3 100644
--- a/Makefile
+++ b/Makefile
@@ -2549,8 +2549,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
-.SUFFIXES:
-
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
diff --git a/shared.mak b/shared.mak
index 29f0e69ecb9..155ac84f867 100644
--- a/shared.mak
+++ b/shared.mak
@@ -9,6 +9,11 @@
 %:: s.%
 %:: SCCS/s.%
 
+## Likewise delete default $(SUFFIXES). See:
+##
+##     info make --index-search=.DELETE_ON_ERROR
+.SUFFIXES:
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.34.1.1215.g6e154b84c77

Re: [PATCH v2 4/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Taylor Blau]

On Fri, Dec 24, 2021 at 06:37:43PM +0100, Ævar Arnfjörð Bjarmason wrote:
>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
>
>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
>
>     Summary
>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

Nice speed-up. Though I am not sure I totally understand where it comes
from ;). Reading 30248886ce8 and the documentation on .SUFFIXES from
[1], I am still unclear. I guess removing the obsolete built-in suffix
rules gives make less work to do in general?

So long as we're not depending on any of these, this seems like a nice
little boost to me.

> diff --git a/shared.mak b/shared.mak
> index 29f0e69ecb9..155ac84f867 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -9,6 +9,11 @@
>  %:: s.%
>  %:: SCCS/s.%
>
> +## Likewise delete default $(SUFFIXES). See:
> +##
> +##     info make --index-search=.DELETE_ON_ERROR
> +.SUFFIXES:

Hmm. s/DELETE_ON_ERROR/SUFFIXES? Or perhaps I'm holding this whole thing
incorrectly:

    ~/s/git [nand] (ab/make-noop) $ info make --index-search=.DELETE_ON_ERROR
    no index entries found for '.DELETE_ON_ERROR'
    ~/s/git [nand] (ab/make-noop) $ info make --index-search=.SUFFIXES
    no index entries found for '.SUFFIXES'

Thanks,
Taylor

[1]: https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html

Re: [PATCH v2 4/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 21 2022, Taylor Blau wrote:

> On Fri, Dec 24, 2021 at 06:37:43PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
>>
>>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
>>
>>     Summary
>>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
>
> Nice speed-up. Though I am not sure I totally understand where it comes
> from ;). Reading 30248886ce8 and the documentation on .SUFFIXES from
> [1], I am still unclear. I guess removing the obsolete built-in suffix
> rules gives make less work to do in general?

I'll update the commit message, but basically the same applies as for
2/8 here (<patch-v2-2.8-b0c9be581a6-20211224T173558Z-avarab@gmail.com>),
or if you run "make" with "--debug=a". I.e. if before/after this change you do:

    git clean -dxf; make -C Documentation/ --debug=a git-status.1 >/tmp/b 2>&1
    git clean -dxf; make -C Documentation/ --debug=a git-status.1 >/tmp/a 2>&1

You'll get:
    
    $ wc -l /tmp/[ba]
       6515 /tmp/a
     144051 /tmp/b
     150566 total

Which e.g. for "git-status.txt" starts with (I cut a lot out, there's
way more than this just for that file):
    
          Considering target file 'git-status.txt'.
           Looking for an implicit rule for 'git-status.txt'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.o'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.c'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.cc'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.C'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.cpp'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.p'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.f'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.F'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.m'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.r'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.s'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.S'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.mod'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.sh'.
    -      Trying pattern rule with stem 'git-status.txt'.
    -      Trying implicit prerequisite 'git-status.txt.o'.
    [...]

I.e. given a foo.txt "make" by default will exhaustively consider foo
foo.txt.c, foo.txt.cpp etc. etc.

This is all ultimately a part of make's implicit rule
mechanism. I.e. even if you have no Makefile at all you can run "make
main" if you have a main.c in your directory, and it'll discover it and
compile it with implicit rules, unless you explicitl disable them,
e.g. with "-r":
    
    $ rm Makefile hello; make hello
    rm: cannot remove 'Makefile': No such file or directory
    rm: cannot remove 'hello': No such file or directory
    cc   hello.o   -o hello
    $ rm Makefile hello; make -r hello
    rm: cannot remove 'Makefile': No such file or directory
    make: *** No rule to make target 'hello'.  Stop.

> So long as we're not depending on any of these, this seems like a nice
> little boost to me.

Yes, definitely!

>> diff --git a/shared.mak b/shared.mak
>> index 29f0e69ecb9..155ac84f867 100644
>> --- a/shared.mak
>> +++ b/shared.mak
>> @@ -9,6 +9,11 @@
>>  %:: s.%
>>  %:: SCCS/s.%
>>
>> +## Likewise delete default $(SUFFIXES). See:
>> +##
>> +##     info make --index-search=.DELETE_ON_ERROR
>> +.SUFFIXES:
>
> Hmm. s/DELETE_ON_ERROR/SUFFIXES? Or perhaps I'm holding this whole thing
> incorrectly:
>
>     ~/s/git [nand] (ab/make-noop) $ info make --index-search=.DELETE_ON_ERROR
>     no index entries found for '.DELETE_ON_ERROR'
>     ~/s/git [nand] (ab/make-noop) $ info make --index-search=.SUFFIXES
>     no index entries found for '.SUFFIXES'

Yes that's a copy/paste error, it should be .SUFFIXES.

But both commands should work, or emit an error like:

    $ info doesnotexist --index-search=.DELETE_ON_ERROR
    info: No menu item 'doesnotexist' in node '(dir)Top'

A broken OS package? Self-built "make"?

In any case I could also link to
https://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html;
which is the same information online (but may not match your local
"make" version)>

[PATCH v2 5/8] Makefile: move $(comma), $(empty) and $(space) to shared.mak

[Ævar Arnfjörð Bjarmason]

Move these variables over to the shared.max, we'll make use of them in
a subsequent commit. There was no reason for these to be "simply
expanded variables", so let's use the normal lazy "=" assignment here.

See 425ca6710b2 (Makefile: allow combining UBSan with other
sanitizers, 2017-07-15) for the commit that introduced these.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 4 ----
 shared.mak | 8 ++++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index e30093b83d3..85f99e66b07 100644
--- a/Makefile
+++ b/Makefile
@@ -1279,10 +1279,6 @@ endif
 ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
-comma := ,
-empty :=
-space := $(empty) $(empty)
-
 ifdef SANITIZE
 SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))
 BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
diff --git a/shared.mak b/shared.mak
index 155ac84f867..b34fb948c0f 100644
--- a/shared.mak
+++ b/shared.mak
@@ -23,3 +23,11 @@
 #
 #    info make --index-search=.DELETE_ON_ERROR
 .DELETE_ON_ERROR:
+
+### Global variables
+
+## comma, empty, space: handy variables as these tokens are either
+## special or can be hard to spot among other Makefile syntax.
+comma = ,
+empty =
+space = $(empty) $(empty)
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 6/8] Makefile: add "$(QUIET)" boilerplate to shared.mak

[Ævar Arnfjörð Bjarmason]

The $(QUIET) variables we define are largely duplicated between our
various Makefiles, let's define them in the new "shared.mak" instead.

Since we're not using the environment to pass these around we don't
need to export the "QUIET_GEN" and "QUIET_BUILT_IN" variables
anymore. The "QUIET_GEN" variable is used in "git-gui/Makefile" and
"gitweb/Makefile", but they've got their own definition for those. The
"QUIET_BUILT_IN" variable is only used in the top-level "Makefile". We
still need to export the "V" variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 32 -------------------------
 Makefile               | 33 --------------------------
 config.mak.uname       |  1 -
 shared.mak             | 53 ++++++++++++++++++++++++++++++++++++++++++
 templates/Makefile     |  5 ----
 5 files changed, 53 insertions(+), 71 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..0f4ebdeda8a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -218,38 +218,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
-	QUIET		= @
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
-	QUIET_STDERR	= 2> /dev/null
-	QUIET_SUBDIR0	= +@subdir=
-	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			  $(MAKE) $(PRINT_DIR) -C $$subdir
-
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
-
-	export V
-endif
-endif
-
 all: html man
 
 html: $(DOC_HTML)
diff --git a/Makefile b/Makefile
index 85f99e66b07..d8051278519 100644
--- a/Makefile
+++ b/Makefile
@@ -1971,39 +1971,6 @@ ifndef PAGER_ENV
 PAGER_ENV = LESS=FRX LV=-c
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring w,$(MAKEFLAGS)),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-	export V
-	export QUIET_GEN
-	export QUIET_BUILT_IN
-endif
-endif
-
 ifdef NO_INSTALL_HARDLINKS
 	export NO_INSTALL_HARDLINKS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index a3a779327f8..cfc2416c935 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -720,7 +720,6 @@ vcxproj:
 	git diff-index --cached --quiet HEAD --
 
 	# Make .vcxproj files and add them
-	unset QUIET_GEN QUIET_BUILT_IN; \
 	perl contrib/buildsystems/generate -g Vcxproj
 	git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
 
diff --git a/shared.mak b/shared.mak
index b34fb948c0f..d6f70f3d6c9 100644
--- a/shared.mak
+++ b/shared.mak
@@ -31,3 +31,56 @@
 comma = ,
 empty =
 space = $(empty) $(empty)
+
+### Quieting
+## common
+QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
+QUIET_SUBDIR1  =
+
+ifneq ($(findstring w,$(MAKEFLAGS)),w)
+PRINT_DIR = --no-print-directory
+else # "make -w"
+NO_SUBDIR = :
+endif
+
+ifneq ($(findstring s,$(MAKEFLAGS)),s)
+ifndef V
+## common
+	QUIET_SUBDIR0  = +@subdir=
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+			 $(MAKE) $(PRINT_DIR) -C $$subdir
+
+	QUIET          = @
+	QUIET_GEN      = @echo '   ' GEN $@;
+
+## Used in "Makefile"
+	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_AR       = @echo '   ' AR $@;
+	QUIET_LINK     = @echo '   ' LINK $@;
+	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
+	QUIET_LNCP     = @echo '   ' LN/CP $@;
+	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
+	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
+	QUIET_GCOV     = @echo '   ' GCOV $@;
+	QUIET_SP       = @echo '   ' SP $<;
+	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
+	QUIET_RC       = @echo '   ' RC $@;
+	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+
+## Used in "Documentation/Makefile"
+	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
+	QUIET_XMLTO	= @echo '   ' XMLTO $@;
+	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
+	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
+	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
+	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_STDERR	= 2> /dev/null
+
+	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+
+	export V
+endif
+endif
diff --git a/templates/Makefile b/templates/Makefile
index 636cee52f51..367ad00c24c 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -2,11 +2,6 @@
 include ../shared.mak
 
 # make and install sample templates
-
-ifndef V
-	QUIET = @
-endif
-
 INSTALL ?= install
 TAR ?= tar
 RM ?= rm -f
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 7/8] Makefile: use $(wspfx) for $(QUIET...) in shared.mak

[Ævar Arnfjörð Bjarmason]

Change the mostly move-only change in the preceding commit to use the
$(wspfx) variable for defining the QUIET padding, to guarantee that
it's consistent with the "TRACK_template" template.

    $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
     -> GIT-CFLAGS PARAMETERS (changed)
     -> CC grep.o

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/shared.mak b/shared.mak
index d6f70f3d6c9..f8ebaf30569 100644
--- a/shared.mak
+++ b/shared.mak
@@ -32,6 +32,12 @@ comma = ,
 empty =
 space = $(empty) $(empty)
 
+## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
+## aligned output.
+wspfx = $(space)$(space)$(space)
+wspfx_SQ = '$(subst ','\'',$(wspfx))'
+# ' closing quote to appease Emacs make-mode.elxo
+
 ### Quieting
 ## common
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
@@ -47,39 +53,39 @@ ifneq ($(findstring s,$(MAKEFLAGS)),s)
 ifndef V
 ## common
 	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo $(wspfx_SQ) SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
 
 	QUIET          = @
-	QUIET_GEN      = @echo '   ' GEN $@;
+	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
 ## Used in "Makefile"
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
+	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
+	QUIET_LINK     = @echo $(wspfx_SQ) LINK $@;
+	QUIET_BUILT_IN = @echo $(wspfx_SQ) BUILTIN $@;
+	QUIET_LNCP     = @echo $(wspfx_SQ) LN/CP $@;
+	QUIET_XGETTEXT = @echo $(wspfx_SQ) XGETTEXT $@;
+	QUIET_MSGFMT   = @echo $(wspfx_SQ) MSGFMT $@;
+	QUIET_GCOV     = @echo $(wspfx_SQ) GCOV $@;
+	QUIET_SP       = @echo $(wspfx_SQ) SP $<;
+	QUIET_HDR      = @echo $(wspfx_SQ) HDR $(<:hcc=h);
+	QUIET_RC       = @echo $(wspfx_SQ) RC $@;
+	QUIET_SPATCH   = @echo $(wspfx_SQ) SPATCH $<;
 
 ## Used in "Documentation/Makefile"
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_ASCIIDOC	= @echo $(wspfx_SQ) ASCIIDOC $@;
+	QUIET_XMLTO	= @echo $(wspfx_SQ) XMLTO $@;
+	QUIET_DB2TEXI	= @echo $(wspfx_SQ) DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo $(wspfx_SQ) MAKEINFO $@;
+	QUIET_DBLATEX	= @echo $(wspfx_SQ) DBLATEX $@;
+	QUIET_XSLTPROC	= @echo $(wspfx_SQ) XSLTPROC $@;
+	QUIET_GEN	= @echo $(wspfx_SQ) GEN $@;
 	QUIET_STDERR	= 2> /dev/null
 
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+	QUIET_LINT_GITLINK	= @echo $(wspfx_SQ) LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo $(wspfx_SQ) LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo $(wspfx_SQ) LINT MAN END $<;
 
 	export V
 endif
-- 
2.34.1.1215.g6e154b84c77

[PATCH v2 8/8] Makefiles: add and use wildcard "mkdir -p" template

[Ævar Arnfjörð Bjarmason]

Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
for us, and use it for the "make lint-docs" targets I added in
8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

As seen in 4c64fb5aad9 (Documentation/Makefile: fix lint-docs mkdir
dependency, 2021-10-26) maintaining these manual lists of parent
directory dependencies is fragile, in addition to being obviously
verbose.

I used this pattern at the time because I couldn't find another method
than "order-only" prerequisites to avoid doing a "mkdir -p $(@D)" for
every file being created, which as noted in [1] would be significantly
slower.

But as it turns out we can use this neat trick of only doing a "mkdir
-p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
of a performance test similar to that noted downthread of [1] in [2]
shows that this is faster, in addition to being less verbose and more
reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation -j1 lint-docs'
    Benchmark 1: make -C Documentation -j1 lint-docs' in 'HEAD~1
      Time (mean ± σ):      2.914 s ±  0.062 s    [User: 2.449 s, System: 0.489 s]
      Range (min … max):    2.834 s …  3.020 s    10 runs

    Benchmark 2: make -C Documentation -j1 lint-docs' in 'HEAD~0
      Time (mean ± σ):      2.315 s ±  0.062 s    [User: 1.950 s, System: 0.386 s]
      Range (min … max):    2.229 s …  2.397 s    10 runs

    Summary
      'make -C Documentation -j1 lint-docs' in 'HEAD~0' ran
        1.26 ± 0.04 times faster than 'make -C Documentation -j1 lint-docs' in 'HEAD~1'

So let's use that pattern both for the "lint-docs" target, and a few
miscellaneous other targets.

This method of creating parent directories is explicitly racy in that
we don't know if we're going to say always create a "foo" followed by
a "foo/bar" under parallelism, or skip the "foo" because we created
"foo/bar" first. In this case it doesn't matter for anything except
that we aren't guaranteed to get the same number of rules firing when
running make in parallel.

1. https://lore.kernel.org/git/211028.861r45y3pt.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211028.86o879vvtp.gmgdl@evledraar.gmail.com/
3. https://gitlab.com/avar/git-hyperfine/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 25 +++----------------------
 Makefile               | 12 +++++++-----
 shared.mak             | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f4ebdeda8a..1eb9192dae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -434,25 +434,11 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
 ## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -463,23 +449,18 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
-lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
diff --git a/Makefile b/Makefile
index d8051278519..5474c27f2b1 100644
--- a/Makefile
+++ b/Makefile
@@ -2714,7 +2714,8 @@ all:: $(MOFILES)
 endif
 
 po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
-	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+	$(call mkdir_p_parent_template)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
@@ -2730,15 +2731,16 @@ NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN) \
 	sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
 	    -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \
 	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
 	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	pod2man $< $@
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)pod2man $< $@
 
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
@@ -2872,7 +2874,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
-	@mkdir -p bin-wrappers
+	$(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)))|' < $< > $@ && \
diff --git a/shared.mak b/shared.mak
index f8ebaf30569..7787f81856c 100644
--- a/shared.mak
+++ b/shared.mak
@@ -59,6 +59,8 @@ ifndef V
 	QUIET          = @
 	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
+	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
+
 ## Used in "Makefile"
 	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
 	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
@@ -90,3 +92,18 @@ ifndef V
 	export V
 endif
 endif
+
+### Templates
+
+## mkdir_p_parent: lazily "mkdir -p" the path needed for a $@
+## file. Uses $(wildcard) to avoid the "mkdir -p" if it's not
+## needed.
+##
+## Is racy, but in a good way; we might redundantly (and safely)
+## "mkdir -p" when running in parallel, but won't need to exhaustively create
+## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
+## "a/prefix/dir/file". This can instead be inserted at the start of
+## the "a/prefix/dir/file" rule.
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
-- 
2.34.1.1215.g6e154b84c77

Re: [PATCH v2 0/8] Makefile: optimize noop runs, add shared.mak

[Ævar Arnfjörð Bjarmason]

On Fri, Dec 24 2021, Ævar Arnfjörð Bjarmason wrote:

A gentle reminder about considering picking up this topic that got lost
around the holidays.

This one is independent of the other Makefile topic I've got outstanding
at
https://lore.kernel.org/git/220221.86r17w9dsq.gmgdl@evledraar.gmail.com/

This thread doesn't have any reviews, but I see that's because I screwed
up and omitted the In-Reply-To at the time. There was feeback on the v1
of this:
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

That v1 was in turn split off from a larger series, and as the
range-diff for that v1 shows the split-off version was was mostly
unchanged from that version, which had a few eyeballs on it.

> As the v1 notes (among other things):
> https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/
>
>     This speeds up noop runs of "make" by a lot. After a "make" running a
>     "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
>     as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
>     but that's another matter).
>
> This v2 re-roll addresses trivial commit message/comment grammar/typo
> issues pointed out by Eric Sunshine, thanks Eric!
>
> Ævar Arnfjörð Bjarmason (8):
>   Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
>   Makefile: disable GNU make built-in wildcard rules
>   Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
>   Makefile: move ".SUFFIXES" rule to shared.mak
>   Makefile: move $(comma), $(empty) and $(space) to shared.mak
>   Makefile: add "$(QUIET)" boilerplate to shared.mak
>   Makefile: use $(wspfx) for $(QUIET...) in shared.mak
>   Makefiles: add and use wildcard "mkdir -p" template
>
>  Documentation/Makefile |  63 +++-------------------
>  Makefile               | 118 +++++++++++++----------------------------
>  config.mak.uname       |   1 -
>  shared.mak             | 109 +++++++++++++++++++++++++++++++++++++
>  t/Makefile             |   3 ++
>  t/interop/Makefile     |   3 ++
>  templates/Makefile     |   8 ++-
>  7 files changed, 160 insertions(+), 145 deletions(-)
>  create mode 100644 shared.mak
>
> Range-diff against v1:
> 1:  f74b47662b7 = 1:  b652fc78fda Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
> 2:  b0c63abe091 = 2:  b0c9be581a6 Makefile: disable GNU make built-in wildcard rules
> 3:  c6c6f7cf8d8 = 3:  ed6fd1c0bd1 Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
> 4:  ed64cd1bd4a = 4:  4c6d8089fff Makefile: move ".SUFFIXES" rule to shared.mak
> 5:  1749085b929 = 5:  f1f02c71dbc Makefile: move $(comma), $(empty) and $(space) to shared.mak
> 6:  c25284b24cf = 6:  fb877060d6b Makefile: add "$(QUIET)" boilerplate to shared.mak
> 7:  3daef7672be = 7:  90d804ea9a0 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
> 8:  aca560ca410 ! 8:  59c1b7032db Makefiles: add and use wildcard "mkdir -p" template
>     @@ Commit message
>      
>          But as it turns out we can use this neat trick of only doing a "mkdir
>          -p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
>     -    of a performance test similar to thatnoted downthread of [1] in [2]
>     +    of a performance test similar to that noted downthread of [1] in [2]
>          shows that this is faster, in addition to being less verbose and more
>          reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):
>      
>     @@ shared.mak: ifndef V
>      +## needed.
>      +##
>      +## Is racy, but in a good way; we might redundantly (and safely)
>     -+## "mkdir -p" when running in parallel, but won't need to exhaustively
>     ++## "mkdir -p" when running in parallel, but won't need to exhaustively create
>      +## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
>      +## "a/prefix/dir/file". This can instead be inserted at the start of
>      +## the "a/prefix/dir/file" rule.

[PATCH v3 0/9] Makefile: optimize noop runs, add shared.mak

[Ævar Arnfjörð Bjarmason]

As the v1 notes (among other things):
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

    This speeds up noop runs of "make" by a lot. After a "make" running a
    "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
    as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
    but that's another matter).

This v3 re-roll:

 * Addresses a minor comment/documentation issue pointed out by Taylor
   Blau.

 * Adjusts the ".SUFFIXES" commit message to note the source of the
   speed-up from that change, in response to a question from Taylor.

 * Changes the contrib/scalar/Makefile to make use of the new
   shared.mak, which allows for deleting some copy/pasted code in it
   in favor of the same shared logic.

   The original version of these patches was written before that file
   landed in-tree.

For v2, see: https://lore.kernel.org/git/cover-v2-0.8-00000000000-20211224T173558Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (9):
  scalar Makefile: set the default target after the includes
  Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
  Makefile: disable GNU make built-in wildcard rules
  Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
  Makefile: move ".SUFFIXES" rule to shared.mak
  Makefile: move $(comma), $(empty) and $(space) to shared.mak
  Makefile: add "$(QUIET)" boilerplate to shared.mak
  Makefile: use $(wspfx) for $(QUIET...) in shared.mak
  Makefiles: add and use wildcard "mkdir -p" template

 Documentation/Makefile    |  63 ++------------------
 Makefile                  | 118 ++++++++++++--------------------------
 config.mak.uname          |   1 -
 contrib/scalar/Makefile   |  19 ++----
 contrib/scalar/t/Makefile |   3 +
 shared.mak                | 109 +++++++++++++++++++++++++++++++++++
 t/Makefile                |   3 +
 t/interop/Makefile        |   3 +
 t/perf/Makefile           |   3 +
 templates/Makefile        |   8 +--
 10 files changed, 170 insertions(+), 160 deletions(-)
 create mode 100644 shared.mak

Range-diff against v2:
 -:  ----------- >  1:  2404c4d8b96 scalar Makefile: set the default target after the includes
 1:  97dccacce20 !  2:  96a490bec54 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
    @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
      
      # The generic compilation pattern rule and automatically
     
    + ## contrib/scalar/Makefile ##
    +@@
    ++# Import tree-wide shared Makefile behavior and libraries
    ++include ../../shared.mak
    ++
    + QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
    + QUIET_SUBDIR1  =
    + 
    +
    + ## contrib/scalar/t/Makefile ##
    +@@
    ++# Import tree-wide shared Makefile behavior and libraries
    ++include ../../../shared.mak
    ++
    + # Run scalar tests
    + #
    + # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
    +
      ## shared.mak (new) ##
     @@
     +### Flags affecting all rules
    @@ t/interop/Makefile
      export GIT_TEST_OPTIONS
      
     
    + ## t/perf/Makefile ##
    +@@
    ++# Import tree-wide shared Makefile behavior and libraries
    ++include ../../shared.mak
    ++
    + -include ../../config.mak
    + export GIT_TEST_OPTIONS
    + 
    +
      ## templates/Makefile ##
     @@
     +# Import tree-wide shared Makefile behavior and libraries
 2:  b2bf32ab071 =  3:  9392e3c3e97 Makefile: disable GNU make built-in wildcard rules
 3:  275aba624fa =  4:  07cf9daa9d6 Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
 4:  7103c40de64 !  5:  16f2e3ff35b Makefile: move ".SUFFIXES" rule to shared.mak
    @@ Commit message
               'make -C Documentation -j1 man' in 'HEAD~0' ran
                 1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
     
    +    The reason for that can be seen when comparing that run with
    +    "--debug=a". Without this change making a target like "git-status.1"
    +    will cause "make" to consider not only "git-status.txt", but
    +    "git-status.txt.o", as well as numerous other implicit suffixes such
    +    as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
    +    example.
    +
    +    So this is causing us to omit a bunch of work we didn't need to
    +    do. For making "git-status.1" the "--debug=a" output is reduced from
    +    ~140k lines to ~6k.
    +
    +    1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Makefile ##
    @@ shared.mak
      
     +## Likewise delete default $(SUFFIXES). See:
     +##
    -+##     info make --index-search=.DELETE_ON_ERROR
    ++##     info make --index-search=.SUFFIXES
     +.SUFFIXES:
     +
      ### Flags affecting all rules
 5:  4a5c647b5e7 =  6:  1b6ecb27f02 Makefile: move $(comma), $(empty) and $(space) to shared.mak
 6:  940299d2a03 !  7:  471067deefc Makefile: add "$(QUIET)" boilerplate to shared.mak
    @@ config.mak.uname: vcxproj:
      	git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
      
     
    + ## contrib/scalar/Makefile ##
    +@@
    + # Import tree-wide shared Makefile behavior and libraries
    + include ../../shared.mak
    + 
    +-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
    +-QUIET_SUBDIR1  =
    +-
    +-ifneq ($(findstring s,$(MAKEFLAGS)),s)
    +-ifndef V
    +-	QUIET_GEN      = @echo '   ' GEN $@;
    +-	QUIET_SUBDIR0  = +@subdir=
    +-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
    +-			 $(MAKE) $(PRINT_DIR) -C $$subdir
    +-else
    +-	export V
    +-endif
    +-endif
    +-
    + include ../../config.mak.uname
    + -include ../../config.mak.autogen
    + -include ../../config.mak
    +
      ## shared.mak ##
     @@
      comma = ,
 7:  1e13fee526d =  8:  510306d2219 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
 8:  250b32540d9 =  9:  85bb74aa32f Makefiles: add and use wildcard "mkdir -p" template
-- 
2.35.1.1175.gf9e1b23ea35

[PATCH v3 1/9] scalar Makefile: set the default target after the includes

[Ævar Arnfjörð Bjarmason]

Make have the "contrib/scalar/Makefile" be stylistically consistent
with the top-level "Makefile" in including other makefiles before
setting the default target.

This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
executable, 2021-12-03), it's a style-only change, in a subsequent
commit the "QUIET" boilerplate at the beginning of this file will be
retrieved via an include, and having an "all:" between the two set of
"include"'s after that change would look odd.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/scalar/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 231b1ee1796..5b12a437426 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -12,12 +12,12 @@ else
 endif
 endif
 
-all:
-
 include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
 
+all:
+
 TARGETS = scalar$(X) scalar.o
 GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
 
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 1/9] scalar Makefile: set the default target after the includes

[Junio C Hamano]

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

> Make have the "contrib/scalar/Makefile" be stylistically consistent
> with the top-level "Makefile" in including other makefiles before
> setting the default target.

I think that goal makes sense, but the patch looks like doing a
different thing.  The top-level "Makefile" does it correctly, by
starting the file with

        # The default target of this Makefile is...
        all::

that declares that saying "make" is equivalent to "make all"
upfront.

The patch does not do that; it moves "all" even lower.

What am I missing?

> This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
> executable, 2021-12-03), it's a style-only change, in a subsequent
> commit the "QUIET" boilerplate at the beginning of this file will be
> retrieved via an include, and having an "all:" between the two set of
> "include"'s after that change would look odd.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  contrib/scalar/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
> index 231b1ee1796..5b12a437426 100644
> --- a/contrib/scalar/Makefile
> +++ b/contrib/scalar/Makefile
> @@ -12,12 +12,12 @@ else
>  endif
>  endif
>  
> -all:
> -
>  include ../../config.mak.uname
>  -include ../../config.mak.autogen
>  -include ../../config.mak
>  
> +all:
> +
>  TARGETS = scalar$(X) scalar.o
>  GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a

[PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

We have various behavior that's shared across our Makefiles, or that
really should be (e.g. via defined templates). Let's create a
top-level "shared.mak" to house those sorts of things, and start by
adding the ".DELETE_ON_ERROR" flag to it.

See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
.DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
".DELETE_ON_ERROR" flag.

This does have the potential downside that if e.g. templates/Makefile
would like to include this "shared.mak" in the future the semantics of
such a Makefile will change, but as noted in the above commits (and
GNU make's own documentation) any such change would be for the better,
so it's safe to do this.

This also doesn't introduce a bug by e.g. having this
".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
have no such scoping semantics.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile    |  6 +++---
 Makefile                  | 13 +++----------
 contrib/scalar/Makefile   |  3 +++
 contrib/scalar/t/Makefile |  3 +++
 shared.mak                |  9 +++++++++
 t/Makefile                |  3 +++
 t/interop/Makefile        |  3 +++
 t/perf/Makefile           |  3 +++
 templates/Makefile        |  3 +++
 9 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100644 shared.mak

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ed656db2ae9..ba27456c86a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Guard against environment variables
 MAN1_TXT =
 MAN5_TXT =
@@ -524,7 +527,4 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
-# Delete the target file on error
-.DELETE_ON_ERROR:
-
 .PHONY: FORCE
diff --git a/Makefile b/Makefile
index 6f0b4b775fe..d378ec22545 100644
--- a/Makefile
+++ b/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include shared.mak
+
 # The default target of this Makefile is...
 all::
 
@@ -2194,16 +2197,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $^
 
-### Flags affecting all rules
-
-# A GNU make extension since gmake 3.72 (released in late 1994) to
-# remove the target of rules if commands in those rules fail. The
-# default is to only do that if make itself receives a signal. Affects
-# all targets, see:
-#
-#    info make --index-search=.DELETE_ON_ERROR
-.DELETE_ON_ERROR:
-
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 5b12a437426..6fb5cc8b701 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
index 6170672bb37..01e82e56d15 100644
--- a/contrib/scalar/t/Makefile
+++ b/contrib/scalar/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../../shared.mak
+
 # Run scalar tests
 #
 # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
diff --git a/shared.mak b/shared.mak
new file mode 100644
index 00000000000..0170bb397ae
--- /dev/null
+++ b/shared.mak
@@ -0,0 +1,9 @@
+### Flags affecting all rules
+
+# A GNU make extension since gmake 3.72 (released in late 1994) to
+# remove the target of rules if commands in those rules fail. The
+# default is to only do that if make itself receives a signal. Affects
+# all targets, see:
+#
+#    info make --index-search=.DELETE_ON_ERROR
+.DELETE_ON_ERROR:
diff --git a/t/Makefile b/t/Makefile
index 46cd5fc5273..056ce55dcc9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Run tests
 #
 # Copyright (c) 2005 Junio C Hamano
diff --git a/t/interop/Makefile b/t/interop/Makefile
index 31a4bbc716a..6911c2915a7 100644
--- a/t/interop/Makefile
+++ b/t/interop/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/t/perf/Makefile b/t/perf/Makefile
index 2465770a782..e4808aebed0 100644
--- a/t/perf/Makefile
+++ b/t/perf/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..636cee52f51 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # make and install sample templates
 
 ifndef V
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Junio C Hamano]

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

> diff --git a/Makefile b/Makefile
> index 6f0b4b775fe..d378ec22545 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include shared.mak
> +
>  # The default target of this Makefile is...
>  all::

Continuing with the theme of [1/9], this change to Makefile gets my
firm NAK.  The first two lines MUST stay to be the first lines.

Otherwise, a mistaken patch that adds rules to shared.make will
make the first of these rules, not "all", as the default target.

Just move it below to the third line or so and you'd be OK.

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

On Fri, Feb 25 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> diff --git a/Makefile b/Makefile
>> index 6f0b4b775fe..d378ec22545 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include shared.mak
>> +
>>  # The default target of this Makefile is...
>>  all::
>
> Continuing with the theme of [1/9], this change to Makefile gets my
> firm NAK.  The first two lines MUST stay to be the first lines.
>
> Otherwise, a mistaken patch that adds rules to shared.make will
> make the first of these rules, not "all", as the default target.
>
> Just move it below to the third line or so and you'd be OK.

Understood, I can/will do that.

But wouldn't working this into this series be an even better option?

The ".DEFAULT_GOAL" variable is new in, 3.81 (01 Apr 2006), which I
think is old enough to hard depend on. We use $(or ...) added in the
same release in config.mak.dev.

We definitely have a hard dependency on "Version 3.80 (03 Oct 2002)"
$(eval ...) etc.

I checked and RHEL5 (which is otherwise the oldest OS we've supported)
has it, but RHEL4 doesn't (it has 3.80). So I think it's safe to have a
hard dependency on it and other things in 3.81 at this point.

diff --git a/Makefile b/Makefile
index 549ca6e7a5c..03321af7e3e 100644
--- a/Makefile
+++ b/Makefile
@@ -1,9 +1,6 @@
 # Import tree-wide shared Makefile behavior and libraries
 include shared.mak
 
-# The default target of this Makefile is...
-all::
-
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
diff --git a/shared.mak b/shared.mak
index 7ba04049c8a..1034dc758f1 100644
--- a/shared.mak
+++ b/shared.mak
@@ -1,3 +1,8 @@
+### By setting the default goal to "all" we override any implicit
+### setting once "make" sees our first target, which it'll ignore if
+### .DEFAULT_GOAL was explicitly set.
+.DEFAULT_GOAL = all
+
 ### Remove GNU make implicit rules
 
 ## This speeds things up since we don't need to look for and stat() a

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Junio C Hamano]

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

> But wouldn't working this into this series be an even better option?

No.

What our current Makefile does (i.e. to have the default goal at the
beginning, even though the recipe for it may live elsewhere) is a
very well understood and established pattern, and there is no reason
to start doing the same thing in a different way, even if it is safe
to assume that the "different way" may be available everywhere.  We
are not gaining anything from doing it differently.

Especially it is not something I want to see as "while at it"
change, which adds more to the review cycle without improving
anything.

> The ".DEFAULT_GOAL" variable is new in, 3.81 (01 Apr 2006), which I
> think is old enough to hard depend on. We use $(or ...) added in the
> same release in config.mak.dev.

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Phillip Wood]

Hi Ævar

On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
> We have various behavior that's shared across our Makefiles, or that
> really should be (e.g. via defined templates). Let's create a
> top-level "shared.mak" to house those sorts of things, and start by
> adding the ".DELETE_ON_ERROR" flag to it.
> 
> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
> ".DELETE_ON_ERROR" flag.
> 
> This does have the potential downside that if e.g. templates/Makefile
> would like to include this "shared.mak" in the future the semantics of
> such a Makefile will change, but as noted in the above commits (and
> GNU make's own documentation) any such change would be for the better,
> so it's safe to do this.

I was confused about the mention of templates/Makefile in this 
paragraph, it seems to be saying that the behavior would change in the 
future if we included shared.mak in templates/Makefile but this patch 
does exactly that.

Also does this patch mean we're now using .DELETE_ON_ERROR in places 
where we were not previously using it?

Best Wishes

Phillip

> This also doesn't introduce a bug by e.g. having this
> ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
> have no such scoping semantics.
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   Documentation/Makefile    |  6 +++---
>   Makefile                  | 13 +++----------
>   contrib/scalar/Makefile   |  3 +++
>   contrib/scalar/t/Makefile |  3 +++
>   shared.mak                |  9 +++++++++
>   t/Makefile                |  3 +++
>   t/interop/Makefile        |  3 +++
>   t/perf/Makefile           |  3 +++
>   templates/Makefile        |  3 +++
>   9 files changed, 33 insertions(+), 13 deletions(-)
>   create mode 100644 shared.mak
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index ed656db2ae9..ba27456c86a 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # Guard against environment variables
>   MAN1_TXT =
>   MAN5_TXT =
> @@ -524,7 +527,4 @@ doc-l10n install-l10n::
>   	$(MAKE) -C po $@
>   endif
>   
> -# Delete the target file on error
> -.DELETE_ON_ERROR:
> -
>   .PHONY: FORCE
> diff --git a/Makefile b/Makefile
> index 6f0b4b775fe..d378ec22545 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include shared.mak
> +
>   # The default target of this Makefile is...
>   all::
>   
> @@ -2194,16 +2197,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>   strip: $(PROGRAMS) git$X
>   	$(STRIP) $(STRIP_OPTS) $^
>   
> -### Flags affecting all rules
> -
> -# A GNU make extension since gmake 3.72 (released in late 1994) to
> -# remove the target of rules if commands in those rules fail. The
> -# default is to only do that if make itself receives a signal. Affects
> -# all targets, see:
> -#
> -#    info make --index-search=.DELETE_ON_ERROR
> -.DELETE_ON_ERROR:
> -
>   ### Target-specific flags and dependencies
>   
>   # The generic compilation pattern rule and automatically
> diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
> index 5b12a437426..6fb5cc8b701 100644
> --- a/contrib/scalar/Makefile
> +++ b/contrib/scalar/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>   QUIET_SUBDIR1  =
>   
> diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
> index 6170672bb37..01e82e56d15 100644
> --- a/contrib/scalar/t/Makefile
> +++ b/contrib/scalar/t/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../../shared.mak
> +
>   # Run scalar tests
>   #
>   # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
> diff --git a/shared.mak b/shared.mak
> new file mode 100644
> index 00000000000..0170bb397ae
> --- /dev/null
> +++ b/shared.mak
> @@ -0,0 +1,9 @@
> +### Flags affecting all rules
> +
> +# A GNU make extension since gmake 3.72 (released in late 1994) to
> +# remove the target of rules if commands in those rules fail. The
> +# default is to only do that if make itself receives a signal. Affects
> +# all targets, see:
> +#
> +#    info make --index-search=.DELETE_ON_ERROR
> +.DELETE_ON_ERROR:
> diff --git a/t/Makefile b/t/Makefile
> index 46cd5fc5273..056ce55dcc9 100644
> --- a/t/Makefile
> +++ b/t/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # Run tests
>   #
>   # Copyright (c) 2005 Junio C Hamano
> diff --git a/t/interop/Makefile b/t/interop/Makefile
> index 31a4bbc716a..6911c2915a7 100644
> --- a/t/interop/Makefile
> +++ b/t/interop/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   -include ../../config.mak
>   export GIT_TEST_OPTIONS
>   
> diff --git a/t/perf/Makefile b/t/perf/Makefile
> index 2465770a782..e4808aebed0 100644
> --- a/t/perf/Makefile
> +++ b/t/perf/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../../shared.mak
> +
>   -include ../../config.mak
>   export GIT_TEST_OPTIONS
>   
> diff --git a/templates/Makefile b/templates/Makefile
> index d22a71a3999..636cee52f51 100644
> --- a/templates/Makefile
> +++ b/templates/Makefile
> @@ -1,3 +1,6 @@
> +# Import tree-wide shared Makefile behavior and libraries
> +include ../shared.mak
> +
>   # make and install sample templates
>   
>   ifndef V

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 28 2022, Phillip Wood wrote:

> Hi Ævar
>
> On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
>> We have various behavior that's shared across our Makefiles, or that
>> really should be (e.g. via defined templates). Let's create a
>> top-level "shared.mak" to house those sorts of things, and start by
>> adding the ".DELETE_ON_ERROR" flag to it.
>> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
>> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
>> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
>> ".DELETE_ON_ERROR" flag.
>> This does have the potential downside that if
>> e.g. templates/Makefile
>> would like to include this "shared.mak" in the future the semantics of
>> such a Makefile will change, but as noted in the above commits (and
>> GNU make's own documentation) any such change would be for the better,
>> so it's safe to do this.
>
> I was confused about the mention of templates/Makefile in this
> paragraph, it seems to be saying that the behavior would change in the 
> future if we included shared.mak in templates/Makefile but this patch
> does exactly that.

Yes, oops! It's a zombie comment that I forgot to adjust from an earlier
version of this where that wasn't the case. Will adjust & re-roll.

> Also does this patch mean we're now using .DELETE_ON_ERROR in places
> where we were not previously using it?

Yes, we'll now use it in those other Makefiles as well. The commits
referenced in the second paragrap of the commit message argue for this
being safe, and I've reviewed the logic myself & don't expect any
problems with it.

As the GNU make manual itself notes the cases where that would cause
problems are really obscure, but it's not the default out of an
abundance of backwards compatibility caution in GNU make.

>> This also doesn't introduce a bug by e.g. having this
>> ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
>> have no such scoping semantics.
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   Documentation/Makefile    |  6 +++---
>>   Makefile                  | 13 +++----------
>>   contrib/scalar/Makefile   |  3 +++
>>   contrib/scalar/t/Makefile |  3 +++
>>   shared.mak                |  9 +++++++++
>>   t/Makefile                |  3 +++
>>   t/interop/Makefile        |  3 +++
>>   t/perf/Makefile           |  3 +++
>>   templates/Makefile        |  3 +++
>>   9 files changed, 33 insertions(+), 13 deletions(-)
>>   create mode 100644 shared.mak
>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>> index ed656db2ae9..ba27456c86a 100644
>> --- a/Documentation/Makefile
>> +++ b/Documentation/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../shared.mak
>> +
>>   # Guard against environment variables
>>   MAN1_TXT =
>>   MAN5_TXT =
>> @@ -524,7 +527,4 @@ doc-l10n install-l10n::
>>   	$(MAKE) -C po $@
>>   endif
>>   -# Delete the target file on error
>> -.DELETE_ON_ERROR:
>> -
>>   .PHONY: FORCE
>> diff --git a/Makefile b/Makefile
>> index 6f0b4b775fe..d378ec22545 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include shared.mak
>> +
>>   # The default target of this Makefile is...
>>   all::
>>   @@ -2194,16 +2197,6 @@ shell_compatibility_test:
>> please_set_SHELL_PATH_to_a_more_modern_shell
>>   strip: $(PROGRAMS) git$X
>>   	$(STRIP) $(STRIP_OPTS) $^
>>   -### Flags affecting all rules
>> -
>> -# A GNU make extension since gmake 3.72 (released in late 1994) to
>> -# remove the target of rules if commands in those rules fail. The
>> -# default is to only do that if make itself receives a signal. Affects
>> -# all targets, see:
>> -#
>> -#    info make --index-search=.DELETE_ON_ERROR
>> -.DELETE_ON_ERROR:
>> -
>>   ### Target-specific flags and dependencies
>>     # The generic compilation pattern rule and automatically
>> diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
>> index 5b12a437426..6fb5cc8b701 100644
>> --- a/contrib/scalar/Makefile
>> +++ b/contrib/scalar/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../../shared.mak
>> +
>>   QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>>   QUIET_SUBDIR1  =
>>   diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
>> index 6170672bb37..01e82e56d15 100644
>> --- a/contrib/scalar/t/Makefile
>> +++ b/contrib/scalar/t/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../../../shared.mak
>> +
>>   # Run scalar tests
>>   #
>>   # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
>> diff --git a/shared.mak b/shared.mak
>> new file mode 100644
>> index 00000000000..0170bb397ae
>> --- /dev/null
>> +++ b/shared.mak
>> @@ -0,0 +1,9 @@
>> +### Flags affecting all rules
>> +
>> +# A GNU make extension since gmake 3.72 (released in late 1994) to
>> +# remove the target of rules if commands in those rules fail. The
>> +# default is to only do that if make itself receives a signal. Affects
>> +# all targets, see:
>> +#
>> +#    info make --index-search=.DELETE_ON_ERROR
>> +.DELETE_ON_ERROR:
>> diff --git a/t/Makefile b/t/Makefile
>> index 46cd5fc5273..056ce55dcc9 100644
>> --- a/t/Makefile
>> +++ b/t/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../shared.mak
>> +
>>   # Run tests
>>   #
>>   # Copyright (c) 2005 Junio C Hamano
>> diff --git a/t/interop/Makefile b/t/interop/Makefile
>> index 31a4bbc716a..6911c2915a7 100644
>> --- a/t/interop/Makefile
>> +++ b/t/interop/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../../shared.mak
>> +
>>   -include ../../config.mak
>>   export GIT_TEST_OPTIONS
>>   diff --git a/t/perf/Makefile b/t/perf/Makefile
>> index 2465770a782..e4808aebed0 100644
>> --- a/t/perf/Makefile
>> +++ b/t/perf/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../../shared.mak
>> +
>>   -include ../../config.mak
>>   export GIT_TEST_OPTIONS
>>   diff --git a/templates/Makefile b/templates/Makefile
>> index d22a71a3999..636cee52f51 100644
>> --- a/templates/Makefile
>> +++ b/templates/Makefile
>> @@ -1,3 +1,6 @@
>> +# Import tree-wide shared Makefile behavior and libraries
>> +include ../shared.mak
>> +
>>   # make and install sample templates
>>     ifndef V

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Phillip Wood]

On 28/02/2022 11:16, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Feb 28 2022, Phillip Wood wrote:
> 
>> Hi Ævar
>>
>> On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
>>> We have various behavior that's shared across our Makefiles, or that
>>> really should be (e.g. via defined templates). Let's create a
>>> top-level "shared.mak" to house those sorts of things, and start by
>>> adding the ".DELETE_ON_ERROR" flag to it.
>>> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
>>> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
>>> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
>>> ".DELETE_ON_ERROR" flag.
>>> This does have the potential downside that if
>>> e.g. templates/Makefile
>>> would like to include this "shared.mak" in the future the semantics of
>>> such a Makefile will change, but as noted in the above commits (and
>>> GNU make's own documentation) any such change would be for the better,
>>> so it's safe to do this.
>>
>> I was confused about the mention of templates/Makefile in this
>> paragraph, it seems to be saying that the behavior would change in the
>> future if we included shared.mak in templates/Makefile but this patch
>> does exactly that.
> 
> Yes, oops! It's a zombie comment that I forgot to adjust from an earlier
> version of this where that wasn't the case. Will adjust & re-roll.
> 
>> Also does this patch mean we're now using .DELETE_ON_ERROR in places
>> where we were not previously using it?
> 
> Yes, we'll now use it in those other Makefiles as well. The commits
> referenced in the second paragrap of the commit message argue for this
> being safe, and I've reviewed the logic myself & don't expect any
> problems with it.

Thanks for elaborating, maybe it is worth spelling explicitly in the 
commit message that this is turning on .DELETE_ON_ERROR in places we 
didn't previously use it. I had a look at the commit message you 
referenced and it seems to make a good case for using .DELETE_ON_ERROR. 
Having a shared makefile for common code makes sense and the speed ups 
from some of the other commits are nice.

Best Wishes

Phillip

> As the GNU make manual itself notes the cases where that would cause
> problems are really obscure, but it's not the default out of an
> abundance of backwards compatibility caution in GNU make.
> 
>>> This also doesn't introduce a bug by e.g. having this
>>> ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
>>> have no such scoping semantics.
>>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>>> ---
>>>    Documentation/Makefile    |  6 +++---
>>>    Makefile                  | 13 +++----------
>>>    contrib/scalar/Makefile   |  3 +++
>>>    contrib/scalar/t/Makefile |  3 +++
>>>    shared.mak                |  9 +++++++++
>>>    t/Makefile                |  3 +++
>>>    t/interop/Makefile        |  3 +++
>>>    t/perf/Makefile           |  3 +++
>>>    templates/Makefile        |  3 +++
>>>    9 files changed, 33 insertions(+), 13 deletions(-)
>>>    create mode 100644 shared.mak
>>> diff --git a/Documentation/Makefile b/Documentation/Makefile
>>> index ed656db2ae9..ba27456c86a 100644
>>> --- a/Documentation/Makefile
>>> +++ b/Documentation/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../shared.mak
>>> +
>>>    # Guard against environment variables
>>>    MAN1_TXT =
>>>    MAN5_TXT =
>>> @@ -524,7 +527,4 @@ doc-l10n install-l10n::
>>>    	$(MAKE) -C po $@
>>>    endif
>>>    -# Delete the target file on error
>>> -.DELETE_ON_ERROR:
>>> -
>>>    .PHONY: FORCE
>>> diff --git a/Makefile b/Makefile
>>> index 6f0b4b775fe..d378ec22545 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include shared.mak
>>> +
>>>    # The default target of this Makefile is...
>>>    all::
>>>    @@ -2194,16 +2197,6 @@ shell_compatibility_test:
>>> please_set_SHELL_PATH_to_a_more_modern_shell
>>>    strip: $(PROGRAMS) git$X
>>>    	$(STRIP) $(STRIP_OPTS) $^
>>>    -### Flags affecting all rules
>>> -
>>> -# A GNU make extension since gmake 3.72 (released in late 1994) to
>>> -# remove the target of rules if commands in those rules fail. The
>>> -# default is to only do that if make itself receives a signal. Affects
>>> -# all targets, see:
>>> -#
>>> -#    info make --index-search=.DELETE_ON_ERROR
>>> -.DELETE_ON_ERROR:
>>> -
>>>    ### Target-specific flags and dependencies
>>>      # The generic compilation pattern rule and automatically
>>> diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
>>> index 5b12a437426..6fb5cc8b701 100644
>>> --- a/contrib/scalar/Makefile
>>> +++ b/contrib/scalar/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../../shared.mak
>>> +
>>>    QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>>>    QUIET_SUBDIR1  =
>>>    diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
>>> index 6170672bb37..01e82e56d15 100644
>>> --- a/contrib/scalar/t/Makefile
>>> +++ b/contrib/scalar/t/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../../../shared.mak
>>> +
>>>    # Run scalar tests
>>>    #
>>>    # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
>>> diff --git a/shared.mak b/shared.mak
>>> new file mode 100644
>>> index 00000000000..0170bb397ae
>>> --- /dev/null
>>> +++ b/shared.mak
>>> @@ -0,0 +1,9 @@
>>> +### Flags affecting all rules
>>> +
>>> +# A GNU make extension since gmake 3.72 (released in late 1994) to
>>> +# remove the target of rules if commands in those rules fail. The
>>> +# default is to only do that if make itself receives a signal. Affects
>>> +# all targets, see:
>>> +#
>>> +#    info make --index-search=.DELETE_ON_ERROR
>>> +.DELETE_ON_ERROR:
>>> diff --git a/t/Makefile b/t/Makefile
>>> index 46cd5fc5273..056ce55dcc9 100644
>>> --- a/t/Makefile
>>> +++ b/t/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../shared.mak
>>> +
>>>    # Run tests
>>>    #
>>>    # Copyright (c) 2005 Junio C Hamano
>>> diff --git a/t/interop/Makefile b/t/interop/Makefile
>>> index 31a4bbc716a..6911c2915a7 100644
>>> --- a/t/interop/Makefile
>>> +++ b/t/interop/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../../shared.mak
>>> +
>>>    -include ../../config.mak
>>>    export GIT_TEST_OPTIONS
>>>    diff --git a/t/perf/Makefile b/t/perf/Makefile
>>> index 2465770a782..e4808aebed0 100644
>>> --- a/t/perf/Makefile
>>> +++ b/t/perf/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../../shared.mak
>>> +
>>>    -include ../../config.mak
>>>    export GIT_TEST_OPTIONS
>>>    diff --git a/templates/Makefile b/templates/Makefile
>>> index d22a71a3999..636cee52f51 100644
>>> --- a/templates/Makefile
>>> +++ b/templates/Makefile
>>> @@ -1,3 +1,6 @@
>>> +# Import tree-wide shared Makefile behavior and libraries
>>> +include ../shared.mak
>>> +
>>>    # make and install sample templates
>>>      ifndef V
> 

Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

On Mon, Feb 28 2022, Phillip Wood wrote:

> On 28/02/2022 11:16, Ævar Arnfjörð Bjarmason wrote:
>> On Mon, Feb 28 2022, Phillip Wood wrote:
>> 
>>> Hi Ævar
>>>
>>> On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
>>>> We have various behavior that's shared across our Makefiles, or that
>>>> really should be (e.g. via defined templates). Let's create a
>>>> top-level "shared.mak" to house those sorts of things, and start by
>>>> adding the ".DELETE_ON_ERROR" flag to it.
>>>> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
>>>> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
>>>> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
>>>> ".DELETE_ON_ERROR" flag.
>>>> This does have the potential downside that if
>>>> e.g. templates/Makefile
>>>> would like to include this "shared.mak" in the future the semantics of
>>>> such a Makefile will change, but as noted in the above commits (and
>>>> GNU make's own documentation) any such change would be for the better,
>>>> so it's safe to do this.
>>>
>>> I was confused about the mention of templates/Makefile in this
>>> paragraph, it seems to be saying that the behavior would change in the
>>> future if we included shared.mak in templates/Makefile but this patch
>>> does exactly that.
>> Yes, oops! It's a zombie comment that I forgot to adjust from an
>> earlier
>> version of this where that wasn't the case. Will adjust & re-roll.
>> 
>>> Also does this patch mean we're now using .DELETE_ON_ERROR in places
>>> where we were not previously using it?
>> Yes, we'll now use it in those other Makefiles as well. The commits
>> referenced in the second paragrap of the commit message argue for this
>> being safe, and I've reviewed the logic myself & don't expect any
>> problems with it.
>
> Thanks for elaborating, maybe it is worth spelling explicitly in the
> commit message that this is turning on .DELETE_ON_ERROR in places we 
> didn't previously use it. I had a look at the commit message you
> referenced and it seems to make a good case for using
> .DELETE_ON_ERROR. Having a shared makefile for common code makes sense
> and the speed ups from some of the other commits are nice.

Thanks, yes, willdo :) I have a re-roll of this queued up for
submission, after sitting on it for a bit longer to shake out any
potential last-minute issues...

[PATCH v3 3/9] Makefile: disable GNU make built-in wildcard rules

[Ævar Arnfjörð Bjarmason]

Override built-in rules of GNU make that use a wildcard target. This
can speeds things up significantly as we don't need to stat() so many
files. GNU make does that by default to see if it can retrieve their
contents from RCS or SCCS. See [1] for an old mailing list discussion
about how to disable these.

The speed-up may wary. I've seen 1-10% depending on the speed of the
local disk, caches, -jN etc. Running:

    strace -f -c -S calls make -j1 NO_TCLTK=Y

Shows that we reduce the number of syscalls we make, mostly in "stat"
calls.

We could also invoke make with "-r" by setting "MAKEFLAGS = -r"
early. Doing so might make us a bit faster still. But doing so is a
much bigger hammer, since it will disable all built-in rules,
some (all?) of which can be seen with:

    make -f/dev/null -p | grep -v -e ^# -e ^$

We may have something that relies on them, so let's go for the more
isolated optimization here that gives us most or all of the wins.

1. https://lists.gnu.org/archive/html/help-make/2002-11/msg00063.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/shared.mak b/shared.mak
index 0170bb397ae..29f0e69ecb9 100644
--- a/shared.mak
+++ b/shared.mak
@@ -1,3 +1,14 @@
+### Remove GNU make implicit rules
+
+## This speeds things up since we don't need to look for and stat() a
+## "foo.c,v" every time a rule referring to "foo.c" is in play. See
+## "make -p -f/dev/null | grep ^%::'".
+%:: %,v
+%:: RCS/%,v
+%:: RCS/%
+%:: s.%
+%:: SCCS/s.%
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 3/9] Makefile: disable GNU make built-in wildcard rules

[Junio C Hamano]

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

> Override built-in rules of GNU make that use a wildcard target. This
> can speeds things up significantly as we don't need to stat() so many
> files. GNU make does that by default to see if it can retrieve their
> contents from RCS or SCCS. See [1] for an old mailing list discussion
> about how to disable these.

;-)

> The speed-up may wary. I've seen 1-10% depending on the speed of the

vary, you mean?

> local disk, caches, -jN etc. Running:
>
>     strace -f -c -S calls make -j1 NO_TCLTK=Y
>
> Shows that we reduce the number of syscalls we make, mostly in "stat"
> calls.

Naturally understandable.

I do remember getting surprised after seeing output from "make -d"
and how many paths were considered for possible places that a source
file can come from.

> +### Remove GNU make implicit rules
> +
> +## This speeds things up since we don't need to look for and stat() a
> +## "foo.c,v" every time a rule referring to "foo.c" is in play. See
> +## "make -p -f/dev/null | grep ^%::'".
> +%:: %,v
> +%:: RCS/%,v
> +%:: RCS/%
> +%:: s.%
> +%:: SCCS/s.%
> +
>  ### Flags affecting all rules
>  
>  # A GNU make extension since gmake 3.72 (released in late 1994) to

[PATCH v3 4/9] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)

[Ævar Arnfjörð Bjarmason]

Combine the definitions of $(FIND_SOURCE_FILES) and $(LIB_H) to speed
up the Makefile, as these are the two main expensive $(shell) commands
that we execute unconditionally.

When see what was in $(FOUND_SOURCE_FILES) that wasn't in $(LIB_H) via
the ad-hoc test of:

    $(error $(filter-out $(LIB_H),$(filter %.h,$(ALL_SOURCE_FILES))))
    $(error $(filter-out $(ALL_SOURCE_FILES),$(filter %.h,$(LIB_H))))

We'll get, respectively:

    Makefile:850: *** t/helper/test-tool.h.  Stop.
    Makefile:850: *** .  Stop.

I.e. we only had a discrepancy when it came to
t/helper/test-tool.h. In terms of correctness this was broken before,
but now works:

    $ make t/helper/test-tool.hco
        HDR t/helper/test-tool.h

This speeds things up a lot:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make NO_TCLTK=Y' 'make -j1 NO_TCLTK=Y' --warmup 10 -M 10
    Benchmark 1: make -j1 NO_TCLTK=Y' in 'HEAD~1
      Time (mean ± σ):     159.9 ms ±   6.8 ms    [User: 137.2 ms, System: 28.0 ms]
      Range (min … max):   154.6 ms … 175.9 ms    10 runs

    Benchmark 2: make -j1 NO_TCLTK=Y' in 'HEAD~0
      Time (mean ± σ):     100.0 ms ±   1.3 ms    [User: 84.2 ms, System: 20.2 ms]
      Range (min … max):    98.8 ms … 102.8 ms    10 runs

    Summary
      'make -j1 NO_TCLTK=Y' in 'HEAD~0' ran
        1.60 ± 0.07 times faster than 'make -j1 NO_TCLTK=Y' in 'HEAD~1'

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 54 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index d378ec22545..185bfa71df3 100644
--- a/Makefile
+++ b/Makefile
@@ -833,12 +833,33 @@ GENERATED_H += hook-list.h
 .PHONY: generated-hdrs
 generated-hdrs: $(GENERATED_H)
 
-LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
+## Exhaustive lists of our source files, either dynamically generated,
+## or hardcoded.
+SOURCES_CMD = ( \
+	git ls-files \
+		'*.[hcS]' \
+		'*.sh' \
+		':!*[tp][0-9][0-9][0-9][0-9]*' \
+		':!contrib' \
+		2>/dev/null || \
 	$(FIND) . \
-	-name .git -prune -o \
-	-name t -prune -o \
-	-name Documentation -prune -o \
-	-name '*.h' -print)))
+		\( -name .git -type d -prune \) \
+		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
+		-o \( -name contrib -type d -prune \) \
+		-o \( -name build -type d -prune \) \
+		-o \( -name 'trash*' -type d -prune \) \
+		-o \( -name '*.[hcS]' -type f -print \) \
+		-o \( -name '*.sh' -type f -print \) \
+		| sed -e 's|^\./||' \
+	)
+FOUND_SOURCE_FILES := $(shell $(SOURCES_CMD))
+
+FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
+FOUND_H_SOURCES = $(filter %.h,$(FOUND_SOURCE_FILES))
+
+COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
+
+LIB_H = $(FOUND_H_SOURCES)
 
 LIB_OBJS += abspath.o
 LIB_OBJS += add-interactive.o
@@ -2789,26 +2810,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	pod2man $< $@
 
-FIND_SOURCE_FILES = ( \
-	git ls-files \
-		'*.[hcS]' \
-		'*.sh' \
-		':!*[tp][0-9][0-9][0-9][0-9]*' \
-		':!contrib' \
-		2>/dev/null || \
-	$(FIND) . \
-		\( -name .git -type d -prune \) \
-		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
-		-o \( -name contrib -type d -prune \) \
-		-o \( -name build -type d -prune \) \
-		-o \( -name 'trash*' -type d -prune \) \
-		-o \( -name '*.[hcS]' -type f -print \) \
-		-o \( -name '*.sh' -type f -print \) \
-		| sed -e 's|^\./||' \
-	)
-
-FOUND_SOURCE_FILES = $(shell $(FIND_SOURCE_FILES))
-
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
 	echo $(FOUND_SOURCE_FILES) | xargs etags -a -o $@+ && \
@@ -3018,9 +3019,6 @@ check: $(GENERATED_H)
 		exit 1; \
 	fi
 
-FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
-COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
-
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	$(QUIET_SPATCH) \
 	if test $(SPATCH_BATCH_SIZE) = 0; then \
-- 
2.35.1.1175.gf9e1b23ea35

[PATCH v3 5/9] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

This was added in 30248886ce8 (Makefile: disable default implicit
rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
apply to all our Makefiles.

This doesn't benefit the main Makefile at all, since it already had
the rule, but since we're including shared.mak in other Makefiles
starts to benefit them. E.g. running the 'man" target is now faster:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
    Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
      Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
      Range (min … max):   112.8 ms … 148.4 ms    26 runs

    Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
      Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
      Range (min … max):    89.8 ms … 111.8 ms    32 runs

    Summary
      'make -C Documentation -j1 man' in 'HEAD~0' ran
        1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

The reason for that can be seen when comparing that run with
"--debug=a". Without this change making a target like "git-status.1"
will cause "make" to consider not only "git-status.txt", but
"git-status.txt.o", as well as numerous other implicit suffixes such
as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
example.

So this is causing us to omit a bunch of work we didn't need to
do. For making "git-status.1" the "--debug=a" output is reduced from
~140k lines to ~6k.

1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 --
 shared.mak | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 185bfa71df3..1462016a3f4 100644
--- a/Makefile
+++ b/Makefile
@@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
-.SUFFIXES:
-
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
diff --git a/shared.mak b/shared.mak
index 29f0e69ecb9..1dda948df09 100644
--- a/shared.mak
+++ b/shared.mak
@@ -9,6 +9,11 @@
 %:: s.%
 %:: SCCS/s.%
 
+## Likewise delete default $(SUFFIXES). See:
+##
+##     info make --index-search=.SUFFIXES
+.SUFFIXES:
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 5/9] Makefile: move ".SUFFIXES" rule to shared.mak

[Junio C Hamano]

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

> This was added in 30248886ce8 (Makefile: disable default implicit
> rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
> apply to all our Makefiles.
>
> This doesn't benefit the main Makefile at all, since it already had
> the rule, but since we're including shared.mak in other Makefiles
> starts to benefit them. E.g. running the 'man" target is now faster:

Nice.

[PATCH v3 6/9] Makefile: move $(comma), $(empty) and $(space) to shared.mak

[Ævar Arnfjörð Bjarmason]

Move these variables over to the shared.max, we'll make use of them in
a subsequent commit. There was no reason for these to be "simply
expanded variables", so let's use the normal lazy "=" assignment here.

See 425ca6710b2 (Makefile: allow combining UBSan with other
sanitizers, 2017-07-15) for the commit that introduced these.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 4 ----
 shared.mak | 8 ++++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 1462016a3f4..81b2eaa0355 100644
--- a/Makefile
+++ b/Makefile
@@ -1289,10 +1289,6 @@ endif
 ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
-comma := ,
-empty :=
-space := $(empty) $(empty)
-
 ifdef SANITIZE
 SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))
 BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
diff --git a/shared.mak b/shared.mak
index 1dda948df09..f6b589ffd8f 100644
--- a/shared.mak
+++ b/shared.mak
@@ -23,3 +23,11 @@
 #
 #    info make --index-search=.DELETE_ON_ERROR
 .DELETE_ON_ERROR:
+
+### Global variables
+
+## comma, empty, space: handy variables as these tokens are either
+## special or can be hard to spot among other Makefile syntax.
+comma = ,
+empty =
+space = $(empty) $(empty)
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 6/9] Makefile: move $(comma), $(empty) and $(space) to shared.mak

[Junio C Hamano]

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

> Move these variables over to the shared.max, we'll make use of them in

max?

> a subsequent commit. There was no reason for these to be "simply
> expanded variables", so let's use the normal lazy "=" assignment here.

Seems to do unrelated things "while at it" which is a common pattern
that slows the reviews down.

> diff --git a/shared.mak b/shared.mak
> index 1dda948df09..f6b589ffd8f 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -23,3 +23,11 @@
>  #
>  #    info make --index-search=.DELETE_ON_ERROR
>  .DELETE_ON_ERROR:
> +
> +### Global variables
> +
> +## comma, empty, space: handy variables as these tokens are either
> +## special or can be hard to spot among other Makefile syntax.
> +comma = ,
> +empty =
> +space = $(empty) $(empty)

In any case, wanting to have a $(space) is not specific to the
top-level Makefile, and it is a good idea to move them to the shared
file.

[PATCH v3 7/9] Makefile: add "$(QUIET)" boilerplate to shared.mak

[Ævar Arnfjörð Bjarmason]

The $(QUIET) variables we define are largely duplicated between our
various Makefiles, let's define them in the new "shared.mak" instead.

Since we're not using the environment to pass these around we don't
need to export the "QUIET_GEN" and "QUIET_BUILT_IN" variables
anymore. The "QUIET_GEN" variable is used in "git-gui/Makefile" and
"gitweb/Makefile", but they've got their own definition for those. The
"QUIET_BUILT_IN" variable is only used in the top-level "Makefile". We
still need to export the "V" variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile  | 32 -------------------------
 Makefile                | 33 -------------------------
 config.mak.uname        |  1 -
 contrib/scalar/Makefile | 14 -----------
 shared.mak              | 53 +++++++++++++++++++++++++++++++++++++++++
 templates/Makefile      |  5 ----
 6 files changed, 53 insertions(+), 85 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..0f4ebdeda8a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -218,38 +218,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
-	QUIET		= @
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
-	QUIET_STDERR	= 2> /dev/null
-	QUIET_SUBDIR0	= +@subdir=
-	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			  $(MAKE) $(PRINT_DIR) -C $$subdir
-
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
-
-	export V
-endif
-endif
-
 all: html man
 
 html: $(DOC_HTML)
diff --git a/Makefile b/Makefile
index 81b2eaa0355..9f93ee1532c 100644
--- a/Makefile
+++ b/Makefile
@@ -2001,39 +2001,6 @@ ifndef PAGER_ENV
 PAGER_ENV = LESS=FRX LV=-c
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring w,$(MAKEFLAGS)),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-	export V
-	export QUIET_GEN
-	export QUIET_BUILT_IN
-endif
-endif
-
 ifdef NO_INSTALL_HARDLINKS
 	export NO_INSTALL_HARDLINKS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 4352ea39e9b..7727b707b74 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -727,7 +727,6 @@ vcxproj:
 	git diff-index --cached --quiet HEAD --
 
 	# Make .vcxproj files and add them
-	unset QUIET_GEN QUIET_BUILT_IN; \
 	perl contrib/buildsystems/generate -g Vcxproj
 	git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
 
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 6fb5cc8b701..139b2a2c8c8 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,20 +1,6 @@
 # Import tree-wide shared Makefile behavior and libraries
 include ../../shared.mak
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-else
-	export V
-endif
-endif
-
 include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
diff --git a/shared.mak b/shared.mak
index f6b589ffd8f..20be9ddf5a6 100644
--- a/shared.mak
+++ b/shared.mak
@@ -31,3 +31,56 @@
 comma = ,
 empty =
 space = $(empty) $(empty)
+
+### Quieting
+## common
+QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
+QUIET_SUBDIR1  =
+
+ifneq ($(findstring w,$(MAKEFLAGS)),w)
+PRINT_DIR = --no-print-directory
+else # "make -w"
+NO_SUBDIR = :
+endif
+
+ifneq ($(findstring s,$(MAKEFLAGS)),s)
+ifndef V
+## common
+	QUIET_SUBDIR0  = +@subdir=
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+			 $(MAKE) $(PRINT_DIR) -C $$subdir
+
+	QUIET          = @
+	QUIET_GEN      = @echo '   ' GEN $@;
+
+## Used in "Makefile"
+	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_AR       = @echo '   ' AR $@;
+	QUIET_LINK     = @echo '   ' LINK $@;
+	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
+	QUIET_LNCP     = @echo '   ' LN/CP $@;
+	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
+	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
+	QUIET_GCOV     = @echo '   ' GCOV $@;
+	QUIET_SP       = @echo '   ' SP $<;
+	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
+	QUIET_RC       = @echo '   ' RC $@;
+	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+
+## Used in "Documentation/Makefile"
+	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
+	QUIET_XMLTO	= @echo '   ' XMLTO $@;
+	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
+	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
+	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
+	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_STDERR	= 2> /dev/null
+
+	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+
+	export V
+endif
+endif
diff --git a/templates/Makefile b/templates/Makefile
index 636cee52f51..367ad00c24c 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -2,11 +2,6 @@
 include ../shared.mak
 
 # make and install sample templates
-
-ifndef V
-	QUIET = @
-endif
-
 INSTALL ?= install
 TAR ?= tar
 RM ?= rm -f
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 7/9] Makefile: add "$(QUIET)" boilerplate to shared.mak

[Junio C Hamano]

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

> The $(QUIET) variables we define are largely duplicated between our
> various Makefiles, let's define them in the new "shared.mak" instead.

Thank you thank you thank you.

It has been disturbing me a lot to see these copied-and-pasted
duplication across Makefiles.

[PATCH v3 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak

[Ævar Arnfjörð Bjarmason]

Change the mostly move-only change in the preceding commit to use the
$(wspfx) variable for defining the QUIET padding, to guarantee that
it's consistent with the "TRACK_template" template.

    $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
     -> GIT-CFLAGS PARAMETERS (changed)
     -> CC grep.o

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/shared.mak b/shared.mak
index 20be9ddf5a6..6e0f103b65f 100644
--- a/shared.mak
+++ b/shared.mak
@@ -32,6 +32,12 @@ comma = ,
 empty =
 space = $(empty) $(empty)
 
+## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
+## aligned output.
+wspfx = $(space)$(space)$(space)
+wspfx_SQ = '$(subst ','\'',$(wspfx))'
+# ' closing quote to appease Emacs make-mode.elxo
+
 ### Quieting
 ## common
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
@@ -47,39 +53,39 @@ ifneq ($(findstring s,$(MAKEFLAGS)),s)
 ifndef V
 ## common
 	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo $(wspfx_SQ) SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
 
 	QUIET          = @
-	QUIET_GEN      = @echo '   ' GEN $@;
+	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
 ## Used in "Makefile"
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
+	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
+	QUIET_LINK     = @echo $(wspfx_SQ) LINK $@;
+	QUIET_BUILT_IN = @echo $(wspfx_SQ) BUILTIN $@;
+	QUIET_LNCP     = @echo $(wspfx_SQ) LN/CP $@;
+	QUIET_XGETTEXT = @echo $(wspfx_SQ) XGETTEXT $@;
+	QUIET_MSGFMT   = @echo $(wspfx_SQ) MSGFMT $@;
+	QUIET_GCOV     = @echo $(wspfx_SQ) GCOV $@;
+	QUIET_SP       = @echo $(wspfx_SQ) SP $<;
+	QUIET_HDR      = @echo $(wspfx_SQ) HDR $(<:hcc=h);
+	QUIET_RC       = @echo $(wspfx_SQ) RC $@;
+	QUIET_SPATCH   = @echo $(wspfx_SQ) SPATCH $<;
 
 ## Used in "Documentation/Makefile"
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_ASCIIDOC	= @echo $(wspfx_SQ) ASCIIDOC $@;
+	QUIET_XMLTO	= @echo $(wspfx_SQ) XMLTO $@;
+	QUIET_DB2TEXI	= @echo $(wspfx_SQ) DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo $(wspfx_SQ) MAKEINFO $@;
+	QUIET_DBLATEX	= @echo $(wspfx_SQ) DBLATEX $@;
+	QUIET_XSLTPROC	= @echo $(wspfx_SQ) XSLTPROC $@;
+	QUIET_GEN	= @echo $(wspfx_SQ) GEN $@;
 	QUIET_STDERR	= 2> /dev/null
 
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+	QUIET_LINT_GITLINK	= @echo $(wspfx_SQ) LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo $(wspfx_SQ) LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo $(wspfx_SQ) LINT MAN END $<;
 
 	export V
 endif
-- 
2.35.1.1175.gf9e1b23ea35

Re: [PATCH v3 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak

[Junio C Hamano]

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

> Change the mostly move-only change in the preceding commit to use the
> $(wspfx) variable for defining the QUIET padding, to guarantee that
> it's consistent with the "TRACK_template" template.
>
>     $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
>      -> GIT-CFLAGS PARAMETERS (changed)
>      -> CC grep.o

What is a TRACK_template?  Is this some "rebase -i" gone bad?

[PATCH v3 9/9] Makefiles: add and use wildcard "mkdir -p" template

[Ævar Arnfjörð Bjarmason]

Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
for us, and use it for the "make lint-docs" targets I added in
8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

As seen in 4c64fb5aad9 (Documentation/Makefile: fix lint-docs mkdir
dependency, 2021-10-26) maintaining these manual lists of parent
directory dependencies is fragile, in addition to being obviously
verbose.

I used this pattern at the time because I couldn't find another method
than "order-only" prerequisites to avoid doing a "mkdir -p $(@D)" for
every file being created, which as noted in [1] would be significantly
slower.

But as it turns out we can use this neat trick of only doing a "mkdir
-p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
of a performance test similar to that noted downthread of [1] in [2]
shows that this is faster, in addition to being less verbose and more
reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation -j1 lint-docs'
    Benchmark 1: make -C Documentation -j1 lint-docs' in 'HEAD~1
      Time (mean ± σ):      2.914 s ±  0.062 s    [User: 2.449 s, System: 0.489 s]
      Range (min … max):    2.834 s …  3.020 s    10 runs

    Benchmark 2: make -C Documentation -j1 lint-docs' in 'HEAD~0
      Time (mean ± σ):      2.315 s ±  0.062 s    [User: 1.950 s, System: 0.386 s]
      Range (min … max):    2.229 s …  2.397 s    10 runs

    Summary
      'make -C Documentation -j1 lint-docs' in 'HEAD~0' ran
        1.26 ± 0.04 times faster than 'make -C Documentation -j1 lint-docs' in 'HEAD~1'

So let's use that pattern both for the "lint-docs" target, and a few
miscellaneous other targets.

This method of creating parent directories is explicitly racy in that
we don't know if we're going to say always create a "foo" followed by
a "foo/bar" under parallelism, or skip the "foo" because we created
"foo/bar" first. In this case it doesn't matter for anything except
that we aren't guaranteed to get the same number of rules firing when
running make in parallel.

1. https://lore.kernel.org/git/211028.861r45y3pt.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211028.86o879vvtp.gmgdl@evledraar.gmail.com/
3. https://gitlab.com/avar/git-hyperfine/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 25 +++----------------------
 Makefile               | 12 +++++++-----
 shared.mak             | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f4ebdeda8a..1eb9192dae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -434,25 +434,11 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
 ## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -463,23 +449,18 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
-lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
diff --git a/Makefile b/Makefile
index 9f93ee1532c..549ca6e7a5c 100644
--- a/Makefile
+++ b/Makefile
@@ -2745,7 +2745,8 @@ all:: $(MOFILES)
 endif
 
 po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
-	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+	$(call mkdir_p_parent_template)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
@@ -2761,15 +2762,16 @@ NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN) \
 	sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
 	    -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \
 	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
 	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	pod2man $< $@
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)pod2man $< $@
 
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
@@ -2903,7 +2905,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
-	@mkdir -p bin-wrappers
+	$(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)))|' < $< > $@ && \
diff --git a/shared.mak b/shared.mak
index 6e0f103b65f..7ba04049c8a 100644
--- a/shared.mak
+++ b/shared.mak
@@ -59,6 +59,8 @@ ifndef V
 	QUIET          = @
 	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
+	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
+
 ## Used in "Makefile"
 	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
 	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
@@ -90,3 +92,18 @@ ifndef V
 	export V
 endif
 endif
+
+### Templates
+
+## mkdir_p_parent: lazily "mkdir -p" the path needed for a $@
+## file. Uses $(wildcard) to avoid the "mkdir -p" if it's not
+## needed.
+##
+## Is racy, but in a good way; we might redundantly (and safely)
+## "mkdir -p" when running in parallel, but won't need to exhaustively create
+## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
+## "a/prefix/dir/file". This can instead be inserted at the start of
+## the "a/prefix/dir/file" rule.
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
-- 
2.35.1.1175.gf9e1b23ea35

[PATCH v4 0/9] Makefile: optimize noop runs, add shared.mak

[Ævar Arnfjörð Bjarmason]

As the v1 notes (among other things):
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

    This speeds up noop runs of "make" by a lot. After a "make" running a
    "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
    as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
    but that's another matter).

This v4 re-roll (for v3, see
https://lore.kernel.org/git/cover-v3-0.9-00000000000-20220225T090127Z-avarab@gmail.com/):

 * The "all" boilerplate goes first now, before "include"
 * Elaborated on DELETE_ON_ERROR ina commit message.
 * No longer change a ":=" to "=" while moving code.
 * Rephrased other commit messages (one of which referred to a
   function from another future series)
 * Typo fix in commit message.

Ævar Arnfjörð Bjarmason (9):
  scalar Makefile: use "The default target of..." pattern
  Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
  Makefile: disable GNU make built-in wildcard rules
  Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
  Makefile: move ".SUFFIXES" rule to shared.mak
  Makefile: move $(comma), $(empty) and $(space) to shared.mak
  Makefile: add "$(QUIET)" boilerplate to shared.mak
  Makefile: use $(wspfx) for $(QUIET...) in shared.mak
  Makefiles: add and use wildcard "mkdir -p" template

 Documentation/Makefile    |  63 ++------------------
 Makefile                  | 118 ++++++++++++--------------------------
 config.mak.uname          |   1 -
 contrib/scalar/Makefile   |  20 ++-----
 contrib/scalar/t/Makefile |   3 +
 shared.mak                | 109 +++++++++++++++++++++++++++++++++++
 t/Makefile                |   3 +
 t/interop/Makefile        |   3 +
 t/perf/Makefile           |   3 +
 templates/Makefile        |   8 +--
 10 files changed, 171 insertions(+), 160 deletions(-)
 create mode 100644 shared.mak

Range-diff against v3:
 1:  2404c4d8b96 <  -:  ----------- scalar Makefile: set the default target after the includes
 -:  ----------- >  1:  26c6bb897cf scalar Makefile: use "The default target of..." pattern
 2:  96a490bec54 !  2:  74692458b70 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
    @@ Commit message
         .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
         ".DELETE_ON_ERROR" flag.
     
    -    This does have the potential downside that if e.g. templates/Makefile
    -    would like to include this "shared.mak" in the future the semantics of
    -    such a Makefile will change, but as noted in the above commits (and
    -    GNU make's own documentation) any such change would be for the better,
    -    so it's safe to do this.
    +    I.e. this changes the behavior of existing rules in the altered
    +    Makefiles (except "Makefile" & "Documentation/Makefile"). I'm
    +    confident that this is safe having read the relevant rules in those
    +    Makfiles, and as the GNU make manual notes that it isn't the default
    +    behavior is out of an abundance of backwards compatibility
    +    caution. From edition 0.75 of its manual, covering GNU make 4.3:
     
    -    This also doesn't introduce a bug by e.g. having this
    +        [Enabling '.DELETE_ON_ERROR' is] almost always what you want
    +        'make' to do, but it is not historical practice; so for
    +        compatibility, you must explicitly request it.
    +
    +    This doesn't introduce a bug by e.g. having this
         ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
         have no such scoping semantics.
     
    +    It does increase the danger that any Makefile without an explicit "The
    +    default target of this Makefile is..." snippet to define the default
    +    target as "all" could have its default rule changed if our new
    +    shared.mak ever defines a "real" rule. In subsequent commits we'll be
    +    careful not to do that, and such breakage would be obvious e.g. in the
    +    case of "make -C t".
    +
    +    We might want to make that less fragile still (e.g. by using
    +    ".DEFAULT_GOAL" as noted in the preceding commit), but for now let's
    +    simply include "shared.mak" without adding that boilerplate to all the
    +    Makefiles that don't have it already. Most of those are already
    +    exposed to that potential caveat e.g. due to including "config.mak*".
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Documentation/Makefile ##
    @@ Documentation/Makefile: doc-l10n install-l10n::
     
      ## Makefile ##
     @@
    -+# Import tree-wide shared Makefile behavior and libraries
    -+include shared.mak
    -+
      # The default target of this Makefile is...
      all::
      
    ++# Import tree-wide shared Makefile behavior and libraries
    ++include shared.mak
    ++
    + # Define V=1 to have a more verbose compile.
    + #
    + # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
     @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
      strip: $(PROGRAMS) git$X
      	$(STRIP) $(STRIP_OPTS) $^
    @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
     
      ## contrib/scalar/Makefile ##
     @@
    + # The default target of this Makefile is...
    + all::
    + 
     +# Import tree-wide shared Makefile behavior and libraries
     +include ../../shared.mak
     +
    - QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
    - QUIET_SUBDIR1  =
    - 
    + include ../../config.mak.uname
    + -include ../../config.mak.autogen
    + -include ../../config.mak
     
      ## contrib/scalar/t/Makefile ##
     @@
 3:  9392e3c3e97 !  3:  0fbdeeffc7b Makefile: disable GNU make built-in wildcard rules
    @@ Commit message
         contents from RCS or SCCS. See [1] for an old mailing list discussion
         about how to disable these.
     
    -    The speed-up may wary. I've seen 1-10% depending on the speed of the
    +    The speed-up may vary. I've seen 1-10% depending on the speed of the
         local disk, caches, -jN etc. Running:
     
             strace -f -c -S calls make -j1 NO_TCLTK=Y
 4:  07cf9daa9d6 =  4:  ea6b835308a Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
 5:  16f2e3ff35b =  5:  c2339694cf7 Makefile: move ".SUFFIXES" rule to shared.mak
 6:  1b6ecb27f02 !  6:  741fdfd48e2 Makefile: move $(comma), $(empty) and $(space) to shared.mak
    @@ Metadata
      ## Commit message ##
         Makefile: move $(comma), $(empty) and $(space) to shared.mak
     
    -    Move these variables over to the shared.max, we'll make use of them in
    -    a subsequent commit. There was no reason for these to be "simply
    -    expanded variables", so let's use the normal lazy "=" assignment here.
    +    Move these variables over to the shared.mak, we'll make use of them in
    +    a subsequent commit.
    +
    +    Note that there's reason for these to be "simply expanded variables",
    +    i.e. to use ":=" assignments instead of lazily expanded "="
    +    assignments. We could use "=", but let's leave this as-is for now for
    +    ease of review.
     
         See 425ca6710b2 (Makefile: allow combining UBSan with other
         sanitizers, 2017-07-15) for the commit that introduced these.
    @@ shared.mak
     +
     +## comma, empty, space: handy variables as these tokens are either
     +## special or can be hard to spot among other Makefile syntax.
    -+comma = ,
    -+empty =
    -+space = $(empty) $(empty)
    ++comma := ,
    ++empty :=
    ++space := $(empty) $(empty)
 7:  471067deefc !  7:  a723cbce270 Makefile: add "$(QUIET)" boilerplate to shared.mak
    @@ config.mak.uname: vcxproj:
      
     
      ## contrib/scalar/Makefile ##
    -@@
    - # Import tree-wide shared Makefile behavior and libraries
    - include ../../shared.mak
    +@@ contrib/scalar/Makefile: include ../../config.mak.uname
    + -include ../../config.mak.autogen
    + -include ../../config.mak
      
     -QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
     -QUIET_SUBDIR1  =
    @@ contrib/scalar/Makefile
     -endif
     -endif
     -
    - include ../../config.mak.uname
    - -include ../../config.mak.autogen
    - -include ../../config.mak
    + TARGETS = scalar$(X) scalar.o
    + GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
    + 
     
      ## shared.mak ##
     @@
    - comma = ,
    - empty =
    - space = $(empty) $(empty)
    + comma := ,
    + empty :=
    + space := $(empty) $(empty)
     +
     +### Quieting
     +## common
 8:  510306d2219 !  8:  3733b0c8df1 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
    @@ Commit message
         Makefile: use $(wspfx) for $(QUIET...) in shared.mak
     
         Change the mostly move-only change in the preceding commit to use the
    -    $(wspfx) variable for defining the QUIET padding, to guarantee that
    -    it's consistent with the "TRACK_template" template.
    +    $(wspfx) variable for defining the QUIET padding. This refactoring
    +    will make it easier to emit that exact amount of padding in functions
    +    that we might add to shared.mak in the future.
     
    -        $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
    -         -> GIT-CFLAGS PARAMETERS (changed)
    -         -> CC grep.o
    +    Such a function is not part of this patch series, but a
    +    "TRACK_template" that I'd like to add as a follow-up to it makes use
    +    of this. Let's make this change now while modifying these QUIET rules
    +    is fresh in our minds.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## shared.mak ##
    -@@ shared.mak: comma = ,
    - empty =
    - space = $(empty) $(empty)
    +@@ shared.mak: comma := ,
    + empty :=
    + space := $(empty) $(empty)
      
     +## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
     +## aligned output.
 9:  85bb74aa32f =  9:  4cc4aeabb20 Makefiles: add and use wildcard "mkdir -p" template
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 1/9] scalar Makefile: use "The default target of..." pattern

[Ævar Arnfjörð Bjarmason]

Make have the "contrib/scalar/Makefile" be stylistically consistent
with the top-level "Makefile" in first declaring "all" to be the
default rule, follwed by including other Makefile snippets.

This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
executable, 2021-12-03), it's a style-only change, in a subsequent
commit the "QUIET" boilerplate at the beginning of this file will be
retrieved via an include, and having an "all:" between the two set of
"include"'s after that change would look odd.

As noted in [1] using ".DEFAULT_GOAL = all" is another way to do this
in more modern GNU make versions, which we already have a hard
dependency on, but let's leave any such change for a future
improvement and go with using our established pattern consistently for
now.

1. https://lore.kernel.org/git/220226.861qzq7d2r.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/scalar/Makefile | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 231b1ee1796..1e9b969f3df 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,3 +1,10 @@
+# The default target of this Makefile is...
+all::
+
+include ../../config.mak.uname
+-include ../../config.mak.autogen
+-include ../../config.mak
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -12,12 +19,6 @@ else
 endif
 endif
 
-all:
-
-include ../../config.mak.uname
--include ../../config.mak.autogen
--include ../../config.mak
-
 TARGETS = scalar$(X) scalar.o
 GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
 
-- 
2.35.1.1228.g56895c6ee86

Re: [PATCH v4 1/9] scalar Makefile: use "The default target of..." pattern

[Junio C Hamano]

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

> Make have the "contrib/scalar/Makefile" be stylistically consistent
> with the top-level "Makefile" in first declaring "all" to be the
> default rule, follwed by including other Makefile snippets.

Sorry, but I cannot quite parse it.  Perhaps remove "have"?

> This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
> executable, 2021-12-03), it's a style-only change, in a subsequent
> commit the "QUIET" boilerplate at the beginning of this file will be
> retrieved via an include, and having an "all:" between the two set of
> "include"'s after that change would look odd.

In other words, raising these includes to the top is not style-only
but is more significant change.  I think these two are logically
distinct changes, but I am fine to see both in the same commit, as
long as they are explained as such.


>
> As noted in [1] using ".DEFAULT_GOAL = all" is another way to do this
> in more modern GNU make versions, which we already have a hard
> dependency on, but let's leave any such change for a future
> improvement and go with using our established pattern consistently for
> now.

I would suggest dropping this paragraph.  In the discussion, we
didn't even establish that such a change would be an "improvement",
and in fact, not everything new is an improvement.  If we know
.DEFAULT_GOAL is not used, readers of Makefile do not have to run
around and look for it to figure out what happens when "make" is run
without argument.

[PATCH v4 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

We have various behavior that's shared across our Makefiles, or that
really should be (e.g. via defined templates). Let's create a
top-level "shared.mak" to house those sorts of things, and start by
adding the ".DELETE_ON_ERROR" flag to it.

See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
.DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
".DELETE_ON_ERROR" flag.

I.e. this changes the behavior of existing rules in the altered
Makefiles (except "Makefile" & "Documentation/Makefile"). I'm
confident that this is safe having read the relevant rules in those
Makfiles, and as the GNU make manual notes that it isn't the default
behavior is out of an abundance of backwards compatibility
caution. From edition 0.75 of its manual, covering GNU make 4.3:

    [Enabling '.DELETE_ON_ERROR' is] almost always what you want
    'make' to do, but it is not historical practice; so for
    compatibility, you must explicitly request it.

This doesn't introduce a bug by e.g. having this
".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
have no such scoping semantics.

It does increase the danger that any Makefile without an explicit "The
default target of this Makefile is..." snippet to define the default
target as "all" could have its default rule changed if our new
shared.mak ever defines a "real" rule. In subsequent commits we'll be
careful not to do that, and such breakage would be obvious e.g. in the
case of "make -C t".

We might want to make that less fragile still (e.g. by using
".DEFAULT_GOAL" as noted in the preceding commit), but for now let's
simply include "shared.mak" without adding that boilerplate to all the
Makefiles that don't have it already. Most of those are already
exposed to that potential caveat e.g. due to including "config.mak*".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile    |  6 +++---
 Makefile                  | 13 +++----------
 contrib/scalar/Makefile   |  3 +++
 contrib/scalar/t/Makefile |  3 +++
 shared.mak                |  9 +++++++++
 t/Makefile                |  3 +++
 t/interop/Makefile        |  3 +++
 t/perf/Makefile           |  3 +++
 templates/Makefile        |  3 +++
 9 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100644 shared.mak

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ed656db2ae9..ba27456c86a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Guard against environment variables
 MAN1_TXT =
 MAN5_TXT =
@@ -524,7 +527,4 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
-# Delete the target file on error
-.DELETE_ON_ERROR:
-
 .PHONY: FORCE
diff --git a/Makefile b/Makefile
index 6f0b4b775fe..aa5ff001b6e 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,9 @@
 # The default target of this Makefile is...
 all::
 
+# Import tree-wide shared Makefile behavior and libraries
+include shared.mak
+
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
@@ -2194,16 +2197,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $^
 
-### Flags affecting all rules
-
-# A GNU make extension since gmake 3.72 (released in late 1994) to
-# remove the target of rules if commands in those rules fail. The
-# default is to only do that if make itself receives a signal. Affects
-# all targets, see:
-#
-#    info make --index-search=.DELETE_ON_ERROR
-.DELETE_ON_ERROR:
-
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 1e9b969f3df..d585b319edc 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,6 +1,9 @@
 # The default target of this Makefile is...
 all::
 
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
index 6170672bb37..01e82e56d15 100644
--- a/contrib/scalar/t/Makefile
+++ b/contrib/scalar/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../../shared.mak
+
 # Run scalar tests
 #
 # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
diff --git a/shared.mak b/shared.mak
new file mode 100644
index 00000000000..0170bb397ae
--- /dev/null
+++ b/shared.mak
@@ -0,0 +1,9 @@
+### Flags affecting all rules
+
+# A GNU make extension since gmake 3.72 (released in late 1994) to
+# remove the target of rules if commands in those rules fail. The
+# default is to only do that if make itself receives a signal. Affects
+# all targets, see:
+#
+#    info make --index-search=.DELETE_ON_ERROR
+.DELETE_ON_ERROR:
diff --git a/t/Makefile b/t/Makefile
index 46cd5fc5273..056ce55dcc9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Run tests
 #
 # Copyright (c) 2005 Junio C Hamano
diff --git a/t/interop/Makefile b/t/interop/Makefile
index 31a4bbc716a..6911c2915a7 100644
--- a/t/interop/Makefile
+++ b/t/interop/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/t/perf/Makefile b/t/perf/Makefile
index 2465770a782..e4808aebed0 100644
--- a/t/perf/Makefile
+++ b/t/perf/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..636cee52f51 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # make and install sample templates
 
 ifndef V
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 3/9] Makefile: disable GNU make built-in wildcard rules

[Ævar Arnfjörð Bjarmason]

Override built-in rules of GNU make that use a wildcard target. This
can speeds things up significantly as we don't need to stat() so many
files. GNU make does that by default to see if it can retrieve their
contents from RCS or SCCS. See [1] for an old mailing list discussion
about how to disable these.

The speed-up may vary. I've seen 1-10% depending on the speed of the
local disk, caches, -jN etc. Running:

    strace -f -c -S calls make -j1 NO_TCLTK=Y

Shows that we reduce the number of syscalls we make, mostly in "stat"
calls.

We could also invoke make with "-r" by setting "MAKEFLAGS = -r"
early. Doing so might make us a bit faster still. But doing so is a
much bigger hammer, since it will disable all built-in rules,
some (all?) of which can be seen with:

    make -f/dev/null -p | grep -v -e ^# -e ^$

We may have something that relies on them, so let's go for the more
isolated optimization here that gives us most or all of the wins.

1. https://lists.gnu.org/archive/html/help-make/2002-11/msg00063.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/shared.mak b/shared.mak
index 0170bb397ae..29f0e69ecb9 100644
--- a/shared.mak
+++ b/shared.mak
@@ -1,3 +1,14 @@
+### Remove GNU make implicit rules
+
+## This speeds things up since we don't need to look for and stat() a
+## "foo.c,v" every time a rule referring to "foo.c" is in play. See
+## "make -p -f/dev/null | grep ^%::'".
+%:: %,v
+%:: RCS/%,v
+%:: RCS/%
+%:: s.%
+%:: SCCS/s.%
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 4/9] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)

[Ævar Arnfjörð Bjarmason]

Combine the definitions of $(FIND_SOURCE_FILES) and $(LIB_H) to speed
up the Makefile, as these are the two main expensive $(shell) commands
that we execute unconditionally.

When see what was in $(FOUND_SOURCE_FILES) that wasn't in $(LIB_H) via
the ad-hoc test of:

    $(error $(filter-out $(LIB_H),$(filter %.h,$(ALL_SOURCE_FILES))))
    $(error $(filter-out $(ALL_SOURCE_FILES),$(filter %.h,$(LIB_H))))

We'll get, respectively:

    Makefile:850: *** t/helper/test-tool.h.  Stop.
    Makefile:850: *** .  Stop.

I.e. we only had a discrepancy when it came to
t/helper/test-tool.h. In terms of correctness this was broken before,
but now works:

    $ make t/helper/test-tool.hco
        HDR t/helper/test-tool.h

This speeds things up a lot:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make NO_TCLTK=Y' 'make -j1 NO_TCLTK=Y' --warmup 10 -M 10
    Benchmark 1: make -j1 NO_TCLTK=Y' in 'HEAD~1
      Time (mean ± σ):     159.9 ms ±   6.8 ms    [User: 137.2 ms, System: 28.0 ms]
      Range (min … max):   154.6 ms … 175.9 ms    10 runs

    Benchmark 2: make -j1 NO_TCLTK=Y' in 'HEAD~0
      Time (mean ± σ):     100.0 ms ±   1.3 ms    [User: 84.2 ms, System: 20.2 ms]
      Range (min … max):    98.8 ms … 102.8 ms    10 runs

    Summary
      'make -j1 NO_TCLTK=Y' in 'HEAD~0' ran
        1.60 ± 0.07 times faster than 'make -j1 NO_TCLTK=Y' in 'HEAD~1'

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 54 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index aa5ff001b6e..1ac924bd844 100644
--- a/Makefile
+++ b/Makefile
@@ -833,12 +833,33 @@ GENERATED_H += hook-list.h
 .PHONY: generated-hdrs
 generated-hdrs: $(GENERATED_H)
 
-LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
+## Exhaustive lists of our source files, either dynamically generated,
+## or hardcoded.
+SOURCES_CMD = ( \
+	git ls-files \
+		'*.[hcS]' \
+		'*.sh' \
+		':!*[tp][0-9][0-9][0-9][0-9]*' \
+		':!contrib' \
+		2>/dev/null || \
 	$(FIND) . \
-	-name .git -prune -o \
-	-name t -prune -o \
-	-name Documentation -prune -o \
-	-name '*.h' -print)))
+		\( -name .git -type d -prune \) \
+		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
+		-o \( -name contrib -type d -prune \) \
+		-o \( -name build -type d -prune \) \
+		-o \( -name 'trash*' -type d -prune \) \
+		-o \( -name '*.[hcS]' -type f -print \) \
+		-o \( -name '*.sh' -type f -print \) \
+		| sed -e 's|^\./||' \
+	)
+FOUND_SOURCE_FILES := $(shell $(SOURCES_CMD))
+
+FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
+FOUND_H_SOURCES = $(filter %.h,$(FOUND_SOURCE_FILES))
+
+COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
+
+LIB_H = $(FOUND_H_SOURCES)
 
 LIB_OBJS += abspath.o
 LIB_OBJS += add-interactive.o
@@ -2789,26 +2810,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	pod2man $< $@
 
-FIND_SOURCE_FILES = ( \
-	git ls-files \
-		'*.[hcS]' \
-		'*.sh' \
-		':!*[tp][0-9][0-9][0-9][0-9]*' \
-		':!contrib' \
-		2>/dev/null || \
-	$(FIND) . \
-		\( -name .git -type d -prune \) \
-		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
-		-o \( -name contrib -type d -prune \) \
-		-o \( -name build -type d -prune \) \
-		-o \( -name 'trash*' -type d -prune \) \
-		-o \( -name '*.[hcS]' -type f -print \) \
-		-o \( -name '*.sh' -type f -print \) \
-		| sed -e 's|^\./||' \
-	)
-
-FOUND_SOURCE_FILES = $(shell $(FIND_SOURCE_FILES))
-
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
 	echo $(FOUND_SOURCE_FILES) | xargs etags -a -o $@+ && \
@@ -3018,9 +3019,6 @@ check: $(GENERATED_H)
 		exit 1; \
 	fi
 
-FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
-COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
-
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	$(QUIET_SPATCH) \
 	if test $(SPATCH_BATCH_SIZE) = 0; then \
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 5/9] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

This was added in 30248886ce8 (Makefile: disable default implicit
rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
apply to all our Makefiles.

This doesn't benefit the main Makefile at all, since it already had
the rule, but since we're including shared.mak in other Makefiles
starts to benefit them. E.g. running the 'man" target is now faster:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
    Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
      Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
      Range (min … max):   112.8 ms … 148.4 ms    26 runs

    Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
      Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
      Range (min … max):    89.8 ms … 111.8 ms    32 runs

    Summary
      'make -C Documentation -j1 man' in 'HEAD~0' ran
        1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

The reason for that can be seen when comparing that run with
"--debug=a". Without this change making a target like "git-status.1"
will cause "make" to consider not only "git-status.txt", but
"git-status.txt.o", as well as numerous other implicit suffixes such
as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
example.

So this is causing us to omit a bunch of work we didn't need to
do. For making "git-status.1" the "--debug=a" output is reduced from
~140k lines to ~6k.

1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 --
 shared.mak | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1ac924bd844..ce362720947 100644
--- a/Makefile
+++ b/Makefile
@@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
-.SUFFIXES:
-
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
diff --git a/shared.mak b/shared.mak
index 29f0e69ecb9..1dda948df09 100644
--- a/shared.mak
+++ b/shared.mak
@@ -9,6 +9,11 @@
 %:: s.%
 %:: SCCS/s.%
 
+## Likewise delete default $(SUFFIXES). See:
+##
+##     info make --index-search=.SUFFIXES
+.SUFFIXES:
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 6/9] Makefile: move $(comma), $(empty) and $(space) to shared.mak

[Ævar Arnfjörð Bjarmason]

Move these variables over to the shared.mak, we'll make use of them in
a subsequent commit.

Note that there's reason for these to be "simply expanded variables",
i.e. to use ":=" assignments instead of lazily expanded "="
assignments. We could use "=", but let's leave this as-is for now for
ease of review.

See 425ca6710b2 (Makefile: allow combining UBSan with other
sanitizers, 2017-07-15) for the commit that introduced these.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 4 ----
 shared.mak | 8 ++++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index ce362720947..1e7a1277ce9 100644
--- a/Makefile
+++ b/Makefile
@@ -1289,10 +1289,6 @@ endif
 ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
-comma := ,
-empty :=
-space := $(empty) $(empty)
-
 ifdef SANITIZE
 SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))
 BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
diff --git a/shared.mak b/shared.mak
index 1dda948df09..934bf428936 100644
--- a/shared.mak
+++ b/shared.mak
@@ -23,3 +23,11 @@
 #
 #    info make --index-search=.DELETE_ON_ERROR
 .DELETE_ON_ERROR:
+
+### Global variables
+
+## comma, empty, space: handy variables as these tokens are either
+## special or can be hard to spot among other Makefile syntax.
+comma := ,
+empty :=
+space := $(empty) $(empty)
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 7/9] Makefile: add "$(QUIET)" boilerplate to shared.mak

[Ævar Arnfjörð Bjarmason]

The $(QUIET) variables we define are largely duplicated between our
various Makefiles, let's define them in the new "shared.mak" instead.

Since we're not using the environment to pass these around we don't
need to export the "QUIET_GEN" and "QUIET_BUILT_IN" variables
anymore. The "QUIET_GEN" variable is used in "git-gui/Makefile" and
"gitweb/Makefile", but they've got their own definition for those. The
"QUIET_BUILT_IN" variable is only used in the top-level "Makefile". We
still need to export the "V" variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile  | 32 -------------------------
 Makefile                | 33 -------------------------
 config.mak.uname        |  1 -
 contrib/scalar/Makefile | 14 -----------
 shared.mak              | 53 +++++++++++++++++++++++++++++++++++++++++
 templates/Makefile      |  5 ----
 6 files changed, 53 insertions(+), 85 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..0f4ebdeda8a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -218,38 +218,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
-	QUIET		= @
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
-	QUIET_STDERR	= 2> /dev/null
-	QUIET_SUBDIR0	= +@subdir=
-	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			  $(MAKE) $(PRINT_DIR) -C $$subdir
-
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
-
-	export V
-endif
-endif
-
 all: html man
 
 html: $(DOC_HTML)
diff --git a/Makefile b/Makefile
index 1e7a1277ce9..d4c0f4ca7b8 100644
--- a/Makefile
+++ b/Makefile
@@ -2001,39 +2001,6 @@ ifndef PAGER_ENV
 PAGER_ENV = LESS=FRX LV=-c
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring w,$(MAKEFLAGS)),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-	export V
-	export QUIET_GEN
-	export QUIET_BUILT_IN
-endif
-endif
-
 ifdef NO_INSTALL_HARDLINKS
 	export NO_INSTALL_HARDLINKS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 4352ea39e9b..7727b707b74 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -727,7 +727,6 @@ vcxproj:
 	git diff-index --cached --quiet HEAD --
 
 	# Make .vcxproj files and add them
-	unset QUIET_GEN QUIET_BUILT_IN; \
 	perl contrib/buildsystems/generate -g Vcxproj
 	git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
 
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index d585b319edc..5e86d78e19b 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -8,20 +8,6 @@ include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-else
-	export V
-endif
-endif
-
 TARGETS = scalar$(X) scalar.o
 GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
 
diff --git a/shared.mak b/shared.mak
index 934bf428936..c45b2812eb6 100644
--- a/shared.mak
+++ b/shared.mak
@@ -31,3 +31,56 @@
 comma := ,
 empty :=
 space := $(empty) $(empty)
+
+### Quieting
+## common
+QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
+QUIET_SUBDIR1  =
+
+ifneq ($(findstring w,$(MAKEFLAGS)),w)
+PRINT_DIR = --no-print-directory
+else # "make -w"
+NO_SUBDIR = :
+endif
+
+ifneq ($(findstring s,$(MAKEFLAGS)),s)
+ifndef V
+## common
+	QUIET_SUBDIR0  = +@subdir=
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+			 $(MAKE) $(PRINT_DIR) -C $$subdir
+
+	QUIET          = @
+	QUIET_GEN      = @echo '   ' GEN $@;
+
+## Used in "Makefile"
+	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_AR       = @echo '   ' AR $@;
+	QUIET_LINK     = @echo '   ' LINK $@;
+	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
+	QUIET_LNCP     = @echo '   ' LN/CP $@;
+	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
+	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
+	QUIET_GCOV     = @echo '   ' GCOV $@;
+	QUIET_SP       = @echo '   ' SP $<;
+	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
+	QUIET_RC       = @echo '   ' RC $@;
+	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+
+## Used in "Documentation/Makefile"
+	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
+	QUIET_XMLTO	= @echo '   ' XMLTO $@;
+	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
+	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
+	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
+	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_STDERR	= 2> /dev/null
+
+	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+
+	export V
+endif
+endif
diff --git a/templates/Makefile b/templates/Makefile
index 636cee52f51..367ad00c24c 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -2,11 +2,6 @@
 include ../shared.mak
 
 # make and install sample templates
-
-ifndef V
-	QUIET = @
-endif
-
 INSTALL ?= install
 TAR ?= tar
 RM ?= rm -f
-- 
2.35.1.1228.g56895c6ee86

[PATCH v4 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak

[Ævar Arnfjörð Bjarmason]

Change the mostly move-only change in the preceding commit to use the
$(wspfx) variable for defining the QUIET padding. This refactoring
will make it easier to emit that exact amount of padding in functions
that we might add to shared.mak in the future.

Such a function is not part of this patch series, but a
"TRACK_template" that I'd like to add as a follow-up to it makes use
of this. Let's make this change now while modifying these QUIET rules
is fresh in our minds.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/shared.mak b/shared.mak
index c45b2812eb6..bb910273538 100644
--- a/shared.mak
+++ b/shared.mak
@@ -32,6 +32,12 @@ comma := ,
 empty :=
 space := $(empty) $(empty)
 
+## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
+## aligned output.
+wspfx = $(space)$(space)$(space)
+wspfx_SQ = '$(subst ','\'',$(wspfx))'
+# ' closing quote to appease Emacs make-mode.elxo
+
 ### Quieting
 ## common
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
@@ -47,39 +53,39 @@ ifneq ($(findstring s,$(MAKEFLAGS)),s)
 ifndef V
 ## common
 	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo $(wspfx_SQ) SUBDIR $$subdir; \
 			 $(MAKE) $(PRINT_DIR) -C $$subdir
 
 	QUIET          = @
-	QUIET_GEN      = @echo '   ' GEN $@;
+	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
 ## Used in "Makefile"
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
+	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
+	QUIET_LINK     = @echo $(wspfx_SQ) LINK $@;
+	QUIET_BUILT_IN = @echo $(wspfx_SQ) BUILTIN $@;
+	QUIET_LNCP     = @echo $(wspfx_SQ) LN/CP $@;
+	QUIET_XGETTEXT = @echo $(wspfx_SQ) XGETTEXT $@;
+	QUIET_MSGFMT   = @echo $(wspfx_SQ) MSGFMT $@;
+	QUIET_GCOV     = @echo $(wspfx_SQ) GCOV $@;
+	QUIET_SP       = @echo $(wspfx_SQ) SP $<;
+	QUIET_HDR      = @echo $(wspfx_SQ) HDR $(<:hcc=h);
+	QUIET_RC       = @echo $(wspfx_SQ) RC $@;
+	QUIET_SPATCH   = @echo $(wspfx_SQ) SPATCH $<;
 
 ## Used in "Documentation/Makefile"
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_ASCIIDOC	= @echo $(wspfx_SQ) ASCIIDOC $@;
+	QUIET_XMLTO	= @echo $(wspfx_SQ) XMLTO $@;
+	QUIET_DB2TEXI	= @echo $(wspfx_SQ) DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo $(wspfx_SQ) MAKEINFO $@;
+	QUIET_DBLATEX	= @echo $(wspfx_SQ) DBLATEX $@;
+	QUIET_XSLTPROC	= @echo $(wspfx_SQ) XSLTPROC $@;
+	QUIET_GEN	= @echo $(wspfx_SQ) GEN $@;
 	QUIET_STDERR	= 2> /dev/null
 
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+	QUIET_LINT_GITLINK	= @echo $(wspfx_SQ) LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo $(wspfx_SQ) LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo $(wspfx_SQ) LINT MAN END $<;
 
 	export V
 endif
-- 
2.35.1.1228.g56895c6ee86

Re: [PATCH v4 8/9] Makefile: use $(wspfx) for $(QUIET...) in shared.mak

[Junio C Hamano]

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

> Change the mostly move-only change in the preceding commit to use the
> $(wspfx) variable for defining the QUIET padding. This refactoring
> will make it easier to emit that exact amount of padding in functions
> that we might add to shared.mak in the future.
>
> Such a function is not part of this patch series, but a
> "TRACK_template" that I'd like to add as a follow-up to it makes use
> of this. Let's make this change now while modifying these QUIET rules
> is fresh in our minds.

I do not see why this change is still part of this series, then.

>  ## Used in "Makefile"
> -	QUIET_CC       = @echo '   ' CC $@;
> -	QUIET_AR       = @echo '   ' AR $@;
> ...
> +	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
> +	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
> ...

Look how much uglier these lines got.

Without the benefit TRACK_template brings us, whatever that is, this
change looks more of an unnecessary churn.  Let's leave it to the
later series that introduces the user that justifies this change.

[PATCH v4 9/9] Makefiles: add and use wildcard "mkdir -p" template

[Ævar Arnfjörð Bjarmason]

Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
for us, and use it for the "make lint-docs" targets I added in
8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

As seen in 4c64fb5aad9 (Documentation/Makefile: fix lint-docs mkdir
dependency, 2021-10-26) maintaining these manual lists of parent
directory dependencies is fragile, in addition to being obviously
verbose.

I used this pattern at the time because I couldn't find another method
than "order-only" prerequisites to avoid doing a "mkdir -p $(@D)" for
every file being created, which as noted in [1] would be significantly
slower.

But as it turns out we can use this neat trick of only doing a "mkdir
-p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
of a performance test similar to that noted downthread of [1] in [2]
shows that this is faster, in addition to being less verbose and more
reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation -j1 lint-docs'
    Benchmark 1: make -C Documentation -j1 lint-docs' in 'HEAD~1
      Time (mean ± σ):      2.914 s ±  0.062 s    [User: 2.449 s, System: 0.489 s]
      Range (min … max):    2.834 s …  3.020 s    10 runs

    Benchmark 2: make -C Documentation -j1 lint-docs' in 'HEAD~0
      Time (mean ± σ):      2.315 s ±  0.062 s    [User: 1.950 s, System: 0.386 s]
      Range (min … max):    2.229 s …  2.397 s    10 runs

    Summary
      'make -C Documentation -j1 lint-docs' in 'HEAD~0' ran
        1.26 ± 0.04 times faster than 'make -C Documentation -j1 lint-docs' in 'HEAD~1'

So let's use that pattern both for the "lint-docs" target, and a few
miscellaneous other targets.

This method of creating parent directories is explicitly racy in that
we don't know if we're going to say always create a "foo" followed by
a "foo/bar" under parallelism, or skip the "foo" because we created
"foo/bar" first. In this case it doesn't matter for anything except
that we aren't guaranteed to get the same number of rules firing when
running make in parallel.

1. https://lore.kernel.org/git/211028.861r45y3pt.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211028.86o879vvtp.gmgdl@evledraar.gmail.com/
3. https://gitlab.com/avar/git-hyperfine/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 25 +++----------------------
 Makefile               | 12 +++++++-----
 shared.mak             | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f4ebdeda8a..1eb9192dae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -434,25 +434,11 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
 ## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -463,23 +449,18 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
-lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
diff --git a/Makefile b/Makefile
index d4c0f4ca7b8..12f08411d2d 100644
--- a/Makefile
+++ b/Makefile
@@ -2745,7 +2745,8 @@ all:: $(MOFILES)
 endif
 
 po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
-	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+	$(call mkdir_p_parent_template)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
@@ -2761,15 +2762,16 @@ NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN) \
 	sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
 	    -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \
 	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
 	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	pod2man $< $@
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)pod2man $< $@
 
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
@@ -2903,7 +2905,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
-	@mkdir -p bin-wrappers
+	$(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)))|' < $< > $@ && \
diff --git a/shared.mak b/shared.mak
index bb910273538..83c22d5e7b3 100644
--- a/shared.mak
+++ b/shared.mak
@@ -59,6 +59,8 @@ ifndef V
 	QUIET          = @
 	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
 
+	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
+
 ## Used in "Makefile"
 	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
 	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
@@ -90,3 +92,18 @@ ifndef V
 	export V
 endif
 endif
+
+### Templates
+
+## mkdir_p_parent: lazily "mkdir -p" the path needed for a $@
+## file. Uses $(wildcard) to avoid the "mkdir -p" if it's not
+## needed.
+##
+## Is racy, but in a good way; we might redundantly (and safely)
+## "mkdir -p" when running in parallel, but won't need to exhaustively create
+## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
+## "a/prefix/dir/file". This can instead be inserted at the start of
+## the "a/prefix/dir/file" rule.
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
-- 
2.35.1.1228.g56895c6ee86

Re: [PATCH v4 9/9] Makefiles: add and use wildcard "mkdir -p" template

[Junio C Hamano]

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

> Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
> for us, and use it for the "make lint-docs" targets I added in
> 8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

Sounds good.

> But as it turns out we can use this neat trick of only doing a "mkdir
> -p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
> of a performance test similar to that noted downthread of [1] in [2]
> shows that this is faster, in addition to being less verbose and more
> reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

Cute.

> diff --git a/shared.mak b/shared.mak
> index bb910273538..83c22d5e7b3 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -59,6 +59,8 @@ ifndef V
>  	QUIET          = @
>  	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
>  
> +	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
> +
>  ## Used in "Makefile"
>  	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
>  	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
> @@ -90,3 +92,18 @@ ifndef V
>  	export V
>  endif
>  endif

It is unfortunate that this came after [8/9] and needs manual
resolution to reorder to leave 8/9 for later series.

Re: [PATCH v4 0/9] Makefile: optimize noop runs, add shared.mak

[Junio C Hamano]

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

> As the v1 notes (among other things):
> https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/
>
>     This speeds up noop runs of "make" by a lot. After a "make" running a
>     "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
>     as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
>     but that's another matter).

Other than a few things I noticed and commented on, 1-7 and 9 looked
all sensible.

Thanks.

Re: [PATCH v4 0/9] Makefile: optimize noop runs, add shared.mak

[Phillip Wood]

On 02/03/2022 12:49, Ævar Arnfjörð Bjarmason wrote:
> As the v1 notes (among other things):
> https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/
> 
>      This speeds up noop runs of "make" by a lot. After a "make" running a
>      "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
>      as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
>      but that's another matter).
> 
> This v4 re-roll (for v3, see
> https://lore.kernel.org/git/cover-v3-0.9-00000000000-20220225T090127Z-avarab@gmail.com/):
> 
>   * The "all" boilerplate goes first now, before "include"
>   * Elaborated on DELETE_ON_ERROR ina commit message.
>   * No longer change a ":=" to "=" while moving code.
>   * Rephrased other commit messages (one of which referred to a
>     function from another future series)
>   * Typo fix in commit message.
> 
> Ævar Arnfjörð Bjarmason (9):
>    scalar Makefile: use "The default target of..." pattern
>    Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
>    Makefile: disable GNU make built-in wildcard rules
>    Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
>    Makefile: move ".SUFFIXES" rule to shared.mak
>    Makefile: move $(comma), $(empty) and $(space) to shared.mak
>    Makefile: add "$(QUIET)" boilerplate to shared.mak
>    Makefile: use $(wspfx) for $(QUIET...) in shared.mak
>    Makefiles: add and use wildcard "mkdir -p" template
> 
>   Documentation/Makefile    |  63 ++------------------
>   Makefile                  | 118 ++++++++++++--------------------------
>   config.mak.uname          |   1 -
>   contrib/scalar/Makefile   |  20 ++-----
>   contrib/scalar/t/Makefile |   3 +
>   shared.mak                | 109 +++++++++++++++++++++++++++++++++++
>   t/Makefile                |   3 +
>   t/interop/Makefile        |   3 +
>   t/perf/Makefile           |   3 +
>   templates/Makefile        |   8 +--
>   10 files changed, 171 insertions(+), 160 deletions(-)
>   create mode 100644 shared.mak
> 
> Range-diff against v3:
>   1:  2404c4d8b96 <  -:  ----------- scalar Makefile: set the default target after the includes
>   -:  ----------- >  1:  26c6bb897cf scalar Makefile: use "The default target of..." pattern
>   2:  96a490bec54 !  2:  74692458b70 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
>      @@ Commit message
>           .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
>           ".DELETE_ON_ERROR" flag.
>       
>      -    This does have the potential downside that if e.g. templates/Makefile
>      -    would like to include this "shared.mak" in the future the semantics of
>      -    such a Makefile will change, but as noted in the above commits (and
>      -    GNU make's own documentation) any such change would be for the better,
>      -    so it's safe to do this.
>      +    I.e. this changes the behavior of existing rules in the altered
>      +    Makefiles (except "Makefile" & "Documentation/Makefile"). I'm
>      +    confident that this is safe having read the relevant rules in those
>      +    Makfiles, and as the GNU make manual notes that it isn't the default
>      +    behavior is out of an abundance of backwards compatibility
>      +    caution. From edition 0.75 of its manual, covering GNU make 4.3:
>       
>      -    This also doesn't introduce a bug by e.g. having this
>      +        [Enabling '.DELETE_ON_ERROR' is] almost always what you want
>      +        'make' to do, but it is not historical practice; so for
>      +        compatibility, you must explicitly request it.
>      +
>      +    This doesn't introduce a bug by e.g. having this
>           ".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
>           have no such scoping semantics.

I think this is much clearer now,

Best Wishes

Phillip

>      +    It does increase the danger that any Makefile without an explicit "The
>      +    default target of this Makefile is..." snippet to define the default
>      +    target as "all" could have its default rule changed if our new
>      +    shared.mak ever defines a "real" rule. In subsequent commits we'll be
>      +    careful not to do that, and such breakage would be obvious e.g. in the
>      +    case of "make -C t".
>      +
>      +    We might want to make that less fragile still (e.g. by using
>      +    ".DEFAULT_GOAL" as noted in the preceding commit), but for now let's
>      +    simply include "shared.mak" without adding that boilerplate to all the
>      +    Makefiles that don't have it already. Most of those are already
>      +    exposed to that potential caveat e.g. due to including "config.mak*".
>      +
>           Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>       
>        ## Documentation/Makefile ##
>      @@ Documentation/Makefile: doc-l10n install-l10n::
>       
>        ## Makefile ##
>       @@
>      -+# Import tree-wide shared Makefile behavior and libraries
>      -+include shared.mak
>      -+
>        # The default target of this Makefile is...
>        all::
>        
>      ++# Import tree-wide shared Makefile behavior and libraries
>      ++include shared.mak
>      ++
>      + # Define V=1 to have a more verbose compile.
>      + #
>      + # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
>       @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>        strip: $(PROGRAMS) git$X
>        	$(STRIP) $(STRIP_OPTS) $^
>      @@ Makefile: shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
>       
>        ## contrib/scalar/Makefile ##
>       @@
>      + # The default target of this Makefile is...
>      + all::
>      +
>       +# Import tree-wide shared Makefile behavior and libraries
>       +include ../../shared.mak
>       +
>      - QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>      - QUIET_SUBDIR1  =
>      -
>      + include ../../config.mak.uname
>      + -include ../../config.mak.autogen
>      + -include ../../config.mak
>       
>        ## contrib/scalar/t/Makefile ##
>       @@
>   3:  9392e3c3e97 !  3:  0fbdeeffc7b Makefile: disable GNU make built-in wildcard rules
>      @@ Commit message
>           contents from RCS or SCCS. See [1] for an old mailing list discussion
>           about how to disable these.
>       
>      -    The speed-up may wary. I've seen 1-10% depending on the speed of the
>      +    The speed-up may vary. I've seen 1-10% depending on the speed of the
>           local disk, caches, -jN etc. Running:
>       
>               strace -f -c -S calls make -j1 NO_TCLTK=Y
>   4:  07cf9daa9d6 =  4:  ea6b835308a Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
>   5:  16f2e3ff35b =  5:  c2339694cf7 Makefile: move ".SUFFIXES" rule to shared.mak
>   6:  1b6ecb27f02 !  6:  741fdfd48e2 Makefile: move $(comma), $(empty) and $(space) to shared.mak
>      @@ Metadata
>        ## Commit message ##
>           Makefile: move $(comma), $(empty) and $(space) to shared.mak
>       
>      -    Move these variables over to the shared.max, we'll make use of them in
>      -    a subsequent commit. There was no reason for these to be "simply
>      -    expanded variables", so let's use the normal lazy "=" assignment here.
>      +    Move these variables over to the shared.mak, we'll make use of them in
>      +    a subsequent commit.
>      +
>      +    Note that there's reason for these to be "simply expanded variables",
>      +    i.e. to use ":=" assignments instead of lazily expanded "="
>      +    assignments. We could use "=", but let's leave this as-is for now for
>      +    ease of review.
>       
>           See 425ca6710b2 (Makefile: allow combining UBSan with other
>           sanitizers, 2017-07-15) for the commit that introduced these.
>      @@ shared.mak
>       +
>       +## comma, empty, space: handy variables as these tokens are either
>       +## special or can be hard to spot among other Makefile syntax.
>      -+comma = ,
>      -+empty =
>      -+space = $(empty) $(empty)
>      ++comma := ,
>      ++empty :=
>      ++space := $(empty) $(empty)
>   7:  471067deefc !  7:  a723cbce270 Makefile: add "$(QUIET)" boilerplate to shared.mak
>      @@ config.mak.uname: vcxproj:
>        
>       
>        ## contrib/scalar/Makefile ##
>      -@@
>      - # Import tree-wide shared Makefile behavior and libraries
>      - include ../../shared.mak
>      +@@ contrib/scalar/Makefile: include ../../config.mak.uname
>      + -include ../../config.mak.autogen
>      + -include ../../config.mak
>        
>       -QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
>       -QUIET_SUBDIR1  =
>      @@ contrib/scalar/Makefile
>       -endif
>       -endif
>       -
>      - include ../../config.mak.uname
>      - -include ../../config.mak.autogen
>      - -include ../../config.mak
>      + TARGETS = scalar$(X) scalar.o
>      + GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
>      +
>       
>        ## shared.mak ##
>       @@
>      - comma = ,
>      - empty =
>      - space = $(empty) $(empty)
>      + comma := ,
>      + empty :=
>      + space := $(empty) $(empty)
>       +
>       +### Quieting
>       +## common
>   8:  510306d2219 !  8:  3733b0c8df1 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
>      @@ Commit message
>           Makefile: use $(wspfx) for $(QUIET...) in shared.mak
>       
>           Change the mostly move-only change in the preceding commit to use the
>      -    $(wspfx) variable for defining the QUIET padding, to guarantee that
>      -    it's consistent with the "TRACK_template" template.
>      +    $(wspfx) variable for defining the QUIET padding. This refactoring
>      +    will make it easier to emit that exact amount of padding in functions
>      +    that we might add to shared.mak in the future.
>       
>      -        $ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
>      -         -> GIT-CFLAGS PARAMETERS (changed)
>      -         -> CC grep.o
>      +    Such a function is not part of this patch series, but a
>      +    "TRACK_template" that I'd like to add as a follow-up to it makes use
>      +    of this. Let's make this change now while modifying these QUIET rules
>      +    is fresh in our minds.
>       
>           Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>       
>        ## shared.mak ##
>      -@@ shared.mak: comma = ,
>      - empty =
>      - space = $(empty) $(empty)
>      +@@ shared.mak: comma := ,
>      + empty :=
>      + space := $(empty) $(empty)
>        
>       +## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
>       +## aligned output.
>   9:  85bb74aa32f =  9:  4cc4aeabb20 Makefiles: add and use wildcard "mkdir -p" template

[PATCH v5 0/8] Makefile: optimize noop runs, add shared.mak

[Ævar Arnfjörð Bjarmason]

As the v1 notes (among other things):
https://lore.kernel.org/git/cover-0.8-00000000000-20211217T012902Z-avarab@gmail.com/

    This speeds up noop runs of "make" by a lot. After a "make" running a
    "make -j1" with this is ~1.5 faster than on "master"[2], and around 3x
    as fast with "make -j1 NO_TCLTK=Y" (the TCL part takes a lot of time,
    but that's another matter).

This v5 re-roll (for v3, see:
https://lore.kernel.org/git/cover-v4-0.9-00000000000-20220302T124320Z-avarab@gmail.com/):

 * Drops the "wspfx" patch.
 * Rephrases the 1/8 commit message, dropping the mention of .DEFAULT_GOAL etc.

I think this should be ready to advance now per Junio's comment on v4
(https://lore.kernel.org/git/xmqqpmn4jdhy.fsf@gitster.g/):

    Other than a few things I noticed and commented on, 1-7 and 9
    looked all sensible.

Ævar Arnfjörð Bjarmason (8):
  scalar Makefile: use "The default target of..." pattern
  Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
  Makefile: disable GNU make built-in wildcard rules
  Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
  Makefile: move ".SUFFIXES" rule to shared.mak
  Makefile: move $(comma), $(empty) and $(space) to shared.mak
  Makefile: add "$(QUIET)" boilerplate to shared.mak
  Makefiles: add and use wildcard "mkdir -p" template

 Documentation/Makefile    |  63 ++------------------
 Makefile                  | 118 ++++++++++++--------------------------
 config.mak.uname          |   1 -
 contrib/scalar/Makefile   |  20 ++-----
 contrib/scalar/t/Makefile |   3 +
 shared.mak                | 103 +++++++++++++++++++++++++++++++++
 t/Makefile                |   3 +
 t/interop/Makefile        |   3 +
 t/perf/Makefile           |   3 +
 templates/Makefile        |   8 +--
 10 files changed, 165 insertions(+), 160 deletions(-)
 create mode 100644 shared.mak

Range-diff against v4:
 1:  26c6bb897cf !  1:  7547bf3e481 scalar Makefile: use "The default target of..." pattern
    @@ Metadata
      ## Commit message ##
         scalar Makefile: use "The default target of..." pattern
     
    -    Make have the "contrib/scalar/Makefile" be stylistically consistent
    -    with the top-level "Makefile" in first declaring "all" to be the
    -    default rule, follwed by including other Makefile snippets.
    +    Make the "contrib/scalar/Makefile" be stylistically consistent with
    +    the top-level "Makefile" in first declaring "all" to be the default
    +    rule, followed by including other Makefile snippets.
     
         This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
    -    executable, 2021-12-03), it's a style-only change, in a subsequent
    -    commit the "QUIET" boilerplate at the beginning of this file will be
    -    retrieved via an include, and having an "all:" between the two set of
    -    "include"'s after that change would look odd.
    -
    -    As noted in [1] using ".DEFAULT_GOAL = all" is another way to do this
    -    in more modern GNU make versions, which we already have a hard
    -    dependency on, but let's leave any such change for a future
    -    improvement and go with using our established pattern consistently for
    -    now.
    -
    -    1. https://lore.kernel.org/git/220226.861qzq7d2r.gmgdl@evledraar.gmail.com/
    +    executable, 2021-12-03), it further ensures that when we add another
    +    "include" file in a subsequent commit that the included file won't be
    +    the one to define our default target.
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
 2:  74692458b70 =  2:  91795eccc32 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
 3:  0fbdeeffc7b =  3:  9f42f40b518 Makefile: disable GNU make built-in wildcard rules
 4:  ea6b835308a =  4:  034779ff7fb Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
 5:  c2339694cf7 =  5:  18e0a6985f1 Makefile: move ".SUFFIXES" rule to shared.mak
 6:  741fdfd48e2 =  6:  422dee02ae9 Makefile: move $(comma), $(empty) and $(space) to shared.mak
 7:  a723cbce270 =  7:  21bf1e6e01c Makefile: add "$(QUIET)" boilerplate to shared.mak
 8:  3733b0c8df1 <  -:  ----------- Makefile: use $(wspfx) for $(QUIET...) in shared.mak
 9:  4cc4aeabb20 !  8:  e6a93cae81d Makefiles: add and use wildcard "mkdir -p" template
    @@ Makefile: test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_
      ## shared.mak ##
     @@ shared.mak: ifndef V
      	QUIET          = @
    - 	QUIET_GEN      = @echo $(wspfx_SQ) GEN $@;
    + 	QUIET_GEN      = @echo '   ' GEN $@;
      
     +	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
     +
      ## Used in "Makefile"
    - 	QUIET_CC       = @echo $(wspfx_SQ) CC $@;
    - 	QUIET_AR       = @echo $(wspfx_SQ) AR $@;
    + 	QUIET_CC       = @echo '   ' CC $@;
    + 	QUIET_AR       = @echo '   ' AR $@;
     @@ shared.mak: ifndef V
      	export V
      endif
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 1/8] scalar Makefile: use "The default target of..." pattern

[Ævar Arnfjörð Bjarmason]

Make the "contrib/scalar/Makefile" be stylistically consistent with
the top-level "Makefile" in first declaring "all" to be the default
rule, followed by including other Makefile snippets.

This adjusts code added in 0a43fb22026 (scalar: create a rudimentary
executable, 2021-12-03), it further ensures that when we add another
"include" file in a subsequent commit that the included file won't be
the one to define our default target.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 contrib/scalar/Makefile | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 231b1ee1796..1e9b969f3df 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,3 +1,10 @@
+# The default target of this Makefile is...
+all::
+
+include ../../config.mak.uname
+-include ../../config.mak.autogen
+-include ../../config.mak
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
@@ -12,12 +19,6 @@ else
 endif
 endif
 
-all:
-
-include ../../config.mak.uname
--include ../../config.mak.autogen
--include ../../config.mak
-
 TARGETS = scalar$(X) scalar.o
 GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
 
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 2/8] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it

[Ævar Arnfjörð Bjarmason]

We have various behavior that's shared across our Makefiles, or that
really should be (e.g. via defined templates). Let's create a
top-level "shared.mak" to house those sorts of things, and start by
adding the ".DELETE_ON_ERROR" flag to it.

See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
.DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
".DELETE_ON_ERROR" flag.

I.e. this changes the behavior of existing rules in the altered
Makefiles (except "Makefile" & "Documentation/Makefile"). I'm
confident that this is safe having read the relevant rules in those
Makfiles, and as the GNU make manual notes that it isn't the default
behavior is out of an abundance of backwards compatibility
caution. From edition 0.75 of its manual, covering GNU make 4.3:

    [Enabling '.DELETE_ON_ERROR' is] almost always what you want
    'make' to do, but it is not historical practice; so for
    compatibility, you must explicitly request it.

This doesn't introduce a bug by e.g. having this
".DELETE_ON_ERROR" flag only apply to this new shared.mak, Makefiles
have no such scoping semantics.

It does increase the danger that any Makefile without an explicit "The
default target of this Makefile is..." snippet to define the default
target as "all" could have its default rule changed if our new
shared.mak ever defines a "real" rule. In subsequent commits we'll be
careful not to do that, and such breakage would be obvious e.g. in the
case of "make -C t".

We might want to make that less fragile still (e.g. by using
".DEFAULT_GOAL" as noted in the preceding commit), but for now let's
simply include "shared.mak" without adding that boilerplate to all the
Makefiles that don't have it already. Most of those are already
exposed to that potential caveat e.g. due to including "config.mak*".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile    |  6 +++---
 Makefile                  | 13 +++----------
 contrib/scalar/Makefile   |  3 +++
 contrib/scalar/t/Makefile |  3 +++
 shared.mak                |  9 +++++++++
 t/Makefile                |  3 +++
 t/interop/Makefile        |  3 +++
 t/perf/Makefile           |  3 +++
 templates/Makefile        |  3 +++
 9 files changed, 33 insertions(+), 13 deletions(-)
 create mode 100644 shared.mak

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ed656db2ae9..ba27456c86a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Guard against environment variables
 MAN1_TXT =
 MAN5_TXT =
@@ -524,7 +527,4 @@ doc-l10n install-l10n::
 	$(MAKE) -C po $@
 endif
 
-# Delete the target file on error
-.DELETE_ON_ERROR:
-
 .PHONY: FORCE
diff --git a/Makefile b/Makefile
index 6f0b4b775fe..aa5ff001b6e 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,9 @@
 # The default target of this Makefile is...
 all::
 
+# Import tree-wide shared Makefile behavior and libraries
+include shared.mak
+
 # Define V=1 to have a more verbose compile.
 #
 # Define SHELL_PATH to a POSIX shell if your /bin/sh is broken.
@@ -2194,16 +2197,6 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $^
 
-### Flags affecting all rules
-
-# A GNU make extension since gmake 3.72 (released in late 1994) to
-# remove the target of rules if commands in those rules fail. The
-# default is to only do that if make itself receives a signal. Affects
-# all targets, see:
-#
-#    info make --index-search=.DELETE_ON_ERROR
-.DELETE_ON_ERROR:
-
 ### Target-specific flags and dependencies
 
 # The generic compilation pattern rule and automatically
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index 1e9b969f3df..d585b319edc 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -1,6 +1,9 @@
 # The default target of this Makefile is...
 all::
 
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
diff --git a/contrib/scalar/t/Makefile b/contrib/scalar/t/Makefile
index 6170672bb37..01e82e56d15 100644
--- a/contrib/scalar/t/Makefile
+++ b/contrib/scalar/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../../shared.mak
+
 # Run scalar tests
 #
 # Copyright (c) 2005,2021 Junio C Hamano, Johannes Schindelin
diff --git a/shared.mak b/shared.mak
new file mode 100644
index 00000000000..0170bb397ae
--- /dev/null
+++ b/shared.mak
@@ -0,0 +1,9 @@
+### Flags affecting all rules
+
+# A GNU make extension since gmake 3.72 (released in late 1994) to
+# remove the target of rules if commands in those rules fail. The
+# default is to only do that if make itself receives a signal. Affects
+# all targets, see:
+#
+#    info make --index-search=.DELETE_ON_ERROR
+.DELETE_ON_ERROR:
diff --git a/t/Makefile b/t/Makefile
index 46cd5fc5273..056ce55dcc9 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # Run tests
 #
 # Copyright (c) 2005 Junio C Hamano
diff --git a/t/interop/Makefile b/t/interop/Makefile
index 31a4bbc716a..6911c2915a7 100644
--- a/t/interop/Makefile
+++ b/t/interop/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/t/perf/Makefile b/t/perf/Makefile
index 2465770a782..e4808aebed0 100644
--- a/t/perf/Makefile
+++ b/t/perf/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../../shared.mak
+
 -include ../../config.mak
 export GIT_TEST_OPTIONS
 
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..636cee52f51 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -1,3 +1,6 @@
+# Import tree-wide shared Makefile behavior and libraries
+include ../shared.mak
+
 # make and install sample templates
 
 ifndef V
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 3/8] Makefile: disable GNU make built-in wildcard rules

[Ævar Arnfjörð Bjarmason]

Override built-in rules of GNU make that use a wildcard target. This
can speeds things up significantly as we don't need to stat() so many
files. GNU make does that by default to see if it can retrieve their
contents from RCS or SCCS. See [1] for an old mailing list discussion
about how to disable these.

The speed-up may vary. I've seen 1-10% depending on the speed of the
local disk, caches, -jN etc. Running:

    strace -f -c -S calls make -j1 NO_TCLTK=Y

Shows that we reduce the number of syscalls we make, mostly in "stat"
calls.

We could also invoke make with "-r" by setting "MAKEFLAGS = -r"
early. Doing so might make us a bit faster still. But doing so is a
much bigger hammer, since it will disable all built-in rules,
some (all?) of which can be seen with:

    make -f/dev/null -p | grep -v -e ^# -e ^$

We may have something that relies on them, so let's go for the more
isolated optimization here that gives us most or all of the wins.

1. https://lists.gnu.org/archive/html/help-make/2002-11/msg00063.html

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 shared.mak | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/shared.mak b/shared.mak
index 0170bb397ae..29f0e69ecb9 100644
--- a/shared.mak
+++ b/shared.mak
@@ -1,3 +1,14 @@
+### Remove GNU make implicit rules
+
+## This speeds things up since we don't need to look for and stat() a
+## "foo.c,v" every time a rule referring to "foo.c" is in play. See
+## "make -p -f/dev/null | grep ^%::'".
+%:: %,v
+%:: RCS/%,v
+%:: RCS/%
+%:: s.%
+%:: SCCS/s.%
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1230.ga6e6579e98c

Re: [PATCH v5 3/8] Makefile: disable GNU make built-in wildcard rules

[Rene Kita]

On Thu,  Mar 03, 2022 at 05:04:14PM +0100, Ævar Arnfjörð Bjarmason wrote:

[...]

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  shared.mak | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/shared.mak b/shared.mak
> index 0170bb397ae..29f0e69ecb9 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -1,3 +1,14 @@
> +### Remove GNU make implicit rules
> +
> +## This speeds things up since we don't need to look for and stat() a
> +## "foo.c,v" every time a rule referring to "foo.c" is in play. See
> +## "make -p -f/dev/null | grep ^%::'".
                                  ^
A bit late to the party, but are you missing an opening single quote
there?

> +%:: %,v
> +%:: RCS/%,v
> +%:: RCS/%
> +%:: s.%
> +%:: SCCS/s.%
> +
>  ### Flags affecting all rules
>  
>  # A GNU make extension since gmake 3.72 (released in late 1994) to
> -- 
> 2.35.1.1230.ga6e6579e98c
> 

Re: [PATCH v5 3/8] Makefile: disable GNU make built-in wildcard rules

[Ævar Arnfjörð Bjarmason]

On Mon, Apr 11 2022, Rene Kita wrote:

> On Thu,  Mar 03, 2022 at 05:04:14PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> [...]
>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  shared.mak | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/shared.mak b/shared.mak
>> index 0170bb397ae..29f0e69ecb9 100644
>> --- a/shared.mak
>> +++ b/shared.mak
>> @@ -1,3 +1,14 @@
>> +### Remove GNU make implicit rules
>> +
>> +## This speeds things up since we don't need to look for and stat() a
>> +## "foo.c,v" every time a rule referring to "foo.c" is in play. See
>> +## "make -p -f/dev/null | grep ^%::'".
>                                   ^
> A bit late to the party, but are you missing an opening single quote
> there?

Yes, that's a typo. Oops!

[PATCH v5 4/8] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)

[Ævar Arnfjörð Bjarmason]

Combine the definitions of $(FIND_SOURCE_FILES) and $(LIB_H) to speed
up the Makefile, as these are the two main expensive $(shell) commands
that we execute unconditionally.

When see what was in $(FOUND_SOURCE_FILES) that wasn't in $(LIB_H) via
the ad-hoc test of:

    $(error $(filter-out $(LIB_H),$(filter %.h,$(ALL_SOURCE_FILES))))
    $(error $(filter-out $(ALL_SOURCE_FILES),$(filter %.h,$(LIB_H))))

We'll get, respectively:

    Makefile:850: *** t/helper/test-tool.h.  Stop.
    Makefile:850: *** .  Stop.

I.e. we only had a discrepancy when it came to
t/helper/test-tool.h. In terms of correctness this was broken before,
but now works:

    $ make t/helper/test-tool.hco
        HDR t/helper/test-tool.h

This speeds things up a lot:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make NO_TCLTK=Y' 'make -j1 NO_TCLTK=Y' --warmup 10 -M 10
    Benchmark 1: make -j1 NO_TCLTK=Y' in 'HEAD~1
      Time (mean ± σ):     159.9 ms ±   6.8 ms    [User: 137.2 ms, System: 28.0 ms]
      Range (min … max):   154.6 ms … 175.9 ms    10 runs

    Benchmark 2: make -j1 NO_TCLTK=Y' in 'HEAD~0
      Time (mean ± σ):     100.0 ms ±   1.3 ms    [User: 84.2 ms, System: 20.2 ms]
      Range (min … max):    98.8 ms … 102.8 ms    10 runs

    Summary
      'make -j1 NO_TCLTK=Y' in 'HEAD~0' ran
        1.60 ± 0.07 times faster than 'make -j1 NO_TCLTK=Y' in 'HEAD~1'

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile | 54 ++++++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index aa5ff001b6e..1ac924bd844 100644
--- a/Makefile
+++ b/Makefile
@@ -833,12 +833,33 @@ GENERATED_H += hook-list.h
 .PHONY: generated-hdrs
 generated-hdrs: $(GENERATED_H)
 
-LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
+## Exhaustive lists of our source files, either dynamically generated,
+## or hardcoded.
+SOURCES_CMD = ( \
+	git ls-files \
+		'*.[hcS]' \
+		'*.sh' \
+		':!*[tp][0-9][0-9][0-9][0-9]*' \
+		':!contrib' \
+		2>/dev/null || \
 	$(FIND) . \
-	-name .git -prune -o \
-	-name t -prune -o \
-	-name Documentation -prune -o \
-	-name '*.h' -print)))
+		\( -name .git -type d -prune \) \
+		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
+		-o \( -name contrib -type d -prune \) \
+		-o \( -name build -type d -prune \) \
+		-o \( -name 'trash*' -type d -prune \) \
+		-o \( -name '*.[hcS]' -type f -print \) \
+		-o \( -name '*.sh' -type f -print \) \
+		| sed -e 's|^\./||' \
+	)
+FOUND_SOURCE_FILES := $(shell $(SOURCES_CMD))
+
+FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
+FOUND_H_SOURCES = $(filter %.h,$(FOUND_SOURCE_FILES))
+
+COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
+
+LIB_H = $(FOUND_H_SOURCES)
 
 LIB_OBJS += abspath.o
 LIB_OBJS += add-interactive.o
@@ -2789,26 +2810,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	pod2man $< $@
 
-FIND_SOURCE_FILES = ( \
-	git ls-files \
-		'*.[hcS]' \
-		'*.sh' \
-		':!*[tp][0-9][0-9][0-9][0-9]*' \
-		':!contrib' \
-		2>/dev/null || \
-	$(FIND) . \
-		\( -name .git -type d -prune \) \
-		-o \( -name '[tp][0-9][0-9][0-9][0-9]*' -prune \) \
-		-o \( -name contrib -type d -prune \) \
-		-o \( -name build -type d -prune \) \
-		-o \( -name 'trash*' -type d -prune \) \
-		-o \( -name '*.[hcS]' -type f -print \) \
-		-o \( -name '*.sh' -type f -print \) \
-		| sed -e 's|^\./||' \
-	)
-
-FOUND_SOURCE_FILES = $(shell $(FIND_SOURCE_FILES))
-
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
 	echo $(FOUND_SOURCE_FILES) | xargs etags -a -o $@+ && \
@@ -3018,9 +3019,6 @@ check: $(GENERATED_H)
 		exit 1; \
 	fi
 
-FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
-COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
-
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	$(QUIET_SPATCH) \
 	if test $(SPATCH_BATCH_SIZE) = 0; then \
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 5/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

This was added in 30248886ce8 (Makefile: disable default implicit
rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
apply to all our Makefiles.

This doesn't benefit the main Makefile at all, since it already had
the rule, but since we're including shared.mak in other Makefiles
starts to benefit them. E.g. running the 'man" target is now faster:

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
    Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
      Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
      Range (min … max):   112.8 ms … 148.4 ms    26 runs

    Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
      Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
      Range (min … max):    89.8 ms … 111.8 ms    32 runs

    Summary
      'make -C Documentation -j1 man' in 'HEAD~0' ran
        1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'

The reason for that can be seen when comparing that run with
"--debug=a". Without this change making a target like "git-status.1"
will cause "make" to consider not only "git-status.txt", but
"git-status.txt.o", as well as numerous other implicit suffixes such
as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
example.

So this is causing us to omit a bunch of work we didn't need to
do. For making "git-status.1" the "--debug=a" output is reduced from
~140k lines to ~6k.

1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 2 --
 shared.mak | 5 +++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1ac924bd844..ce362720947 100644
--- a/Makefile
+++ b/Makefile
@@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
 ASM_OBJ := $(ASM_SRC:S=o)
 C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
 
-.SUFFIXES:
-
 $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
 	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
 $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
diff --git a/shared.mak b/shared.mak
index 29f0e69ecb9..1dda948df09 100644
--- a/shared.mak
+++ b/shared.mak
@@ -9,6 +9,11 @@
 %:: s.%
 %:: SCCS/s.%
 
+## Likewise delete default $(SUFFIXES). See:
+##
+##     info make --index-search=.SUFFIXES
+.SUFFIXES:
+
 ### Flags affecting all rules
 
 # A GNU make extension since gmake 3.72 (released in late 1994) to
-- 
2.35.1.1230.ga6e6579e98c

Re: [PATCH v5 5/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Adam Dinwoodie]

On Thu, Mar 03, 2022 at 05:04:16PM +0100, Ævar Arnfjörð Bjarmason wrote:
> This was added in 30248886ce8 (Makefile: disable default implicit
> rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
> apply to all our Makefiles.
> 
> This doesn't benefit the main Makefile at all, since it already had
> the rule, but since we're including shared.mak in other Makefiles
> starts to benefit them. E.g. running the 'man" target is now faster:
> 
>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
> 
>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
> 
>     Summary
>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
> 
> The reason for that can be seen when comparing that run with
> "--debug=a". Without this change making a target like "git-status.1"
> will cause "make" to consider not only "git-status.txt", but
> "git-status.txt.o", as well as numerous other implicit suffixes such
> as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
> example.
> 
> So this is causing us to omit a bunch of work we didn't need to
> do. For making "git-status.1" the "--debug=a" output is reduced from
> ~140k lines to ~6k.
> 
> 1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Makefile   | 2 --
>  shared.mak | 5 +++++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1ac924bd844..ce362720947 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
>  ASM_OBJ := $(ASM_SRC:S=o)
>  C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
>  
> -.SUFFIXES:
> -
>  $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>  	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>  $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
> diff --git a/shared.mak b/shared.mak
> index 29f0e69ecb9..1dda948df09 100644
> --- a/shared.mak
> +++ b/shared.mak
> @@ -9,6 +9,11 @@
>  %:: s.%
>  %:: SCCS/s.%
>  
> +## Likewise delete default $(SUFFIXES). See:
> +##
> +##     info make --index-search=.SUFFIXES
> +.SUFFIXES:
> +
>  ### Flags affecting all rules
>  
>  # A GNU make extension since gmake 3.72 (released in late 1994) to

I confess I really don't understand why, but as part of testing
v2.36.0-rc0 on Cygwin, I've started getting errors building the info
pages, and bisect points to this commit as the culprit.

Specifically, I've been running

    git clean -dffx && make configure && ./configure && make -j4 info

Without this commit, that gets me a successful build; there's a bunch of
noisy warnings that have been hanging around for a long time, and I think
are fundamentally due to the slightly mismatched documentation libraries
that Cygwin has.  With this commit, I get the same noisy warnings, but I
also get the error "could not open .texi: No such file or directory".

I have to confess, I don't really understand this aspect of GNU Make, so
I'm not sure if this is a problem with Cygwin having a bad toolset or
there being something about my environment that means this doesn't work,
but regardless, it's currently causing the Cygwin Git builds to fail.

Re: [PATCH v5 5/8] Makefile: move ".SUFFIXES" rule to shared.mak

[Ævar Arnfjörð Bjarmason]

On Tue, Apr 05 2022, Adam Dinwoodie wrote:

> On Thu, Mar 03, 2022 at 05:04:16PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> This was added in 30248886ce8 (Makefile: disable default implicit
>> rules, 2010-01-26), let's move it to the top of "shared.mak" so it'll
>> apply to all our Makefiles.
>> 
>> This doesn't benefit the main Makefile at all, since it already had
>> the rule, but since we're including shared.mak in other Makefiles
>> starts to benefit them. E.g. running the 'man" target is now faster:
>> 
>>     $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation man' 'make -C Documentation -j1 man'
>>     Benchmark 1: make -C Documentation -j1 man' in 'HEAD~1
>>       Time (mean ± σ):     121.7 ms ±   8.8 ms    [User: 105.8 ms, System: 18.6 ms]
>>       Range (min … max):   112.8 ms … 148.4 ms    26 runs
>> 
>>     Benchmark 2: make -C Documentation -j1 man' in 'HEAD~0
>>       Time (mean ± σ):      97.5 ms ±   8.0 ms    [User: 80.1 ms, System: 20.1 ms]
>>       Range (min … max):    89.8 ms … 111.8 ms    32 runs
>> 
>>     Summary
>>       'make -C Documentation -j1 man' in 'HEAD~0' ran
>>         1.25 ± 0.14 times faster than 'make -C Documentation -j1 man' in 'HEAD~1'
>> 
>> The reason for that can be seen when comparing that run with
>> "--debug=a". Without this change making a target like "git-status.1"
>> will cause "make" to consider not only "git-status.txt", but
>> "git-status.txt.o", as well as numerous other implicit suffixes such
>> as ".c", ".cc", ".cpp" etc. See [1] for a more detailed before/after
>> example.
>> 
>> So this is causing us to omit a bunch of work we didn't need to
>> do. For making "git-status.1" the "--debug=a" output is reduced from
>> ~140k lines to ~6k.
>> 
>> 1. https://lore.kernel.org/git/220222.86bkyz875k.gmgdl@evledraar.gmail.com/
>> 
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  Makefile   | 2 --
>>  shared.mak | 5 +++++
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 1ac924bd844..ce362720947 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -2580,8 +2580,6 @@ ASM_SRC := $(wildcard $(OBJECTS:o=S))
>>  ASM_OBJ := $(ASM_SRC:S=o)
>>  C_OBJ := $(filter-out $(ASM_OBJ),$(OBJECTS))
>>  
>> -.SUFFIXES:
>> -
>>  $(C_OBJ): %.o: %.c GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>>  	$(QUIET_CC)$(CC) -o $*.o -c $(dep_args) $(compdb_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $<
>>  $(ASM_OBJ): %.o: %.S GIT-CFLAGS $(missing_dep_dirs) $(missing_compdb_dir)
>> diff --git a/shared.mak b/shared.mak
>> index 29f0e69ecb9..1dda948df09 100644
>> --- a/shared.mak
>> +++ b/shared.mak
>> @@ -9,6 +9,11 @@
>>  %:: s.%
>>  %:: SCCS/s.%
>>  
>> +## Likewise delete default $(SUFFIXES). See:
>> +##
>> +##     info make --index-search=.SUFFIXES
>> +.SUFFIXES:
>> +
>>  ### Flags affecting all rules
>>  
>>  # A GNU make extension since gmake 3.72 (released in late 1994) to
>
> I confess I really don't understand why, but as part of testing
> v2.36.0-rc0 on Cygwin, I've started getting errors building the info
> pages, and bisect points to this commit as the culprit.
>
> Specifically, I've been running
>
>     git clean -dffx && make configure && ./configure && make -j4 info
>
> Without this commit, that gets me a successful build; there's a bunch of
> noisy warnings that have been hanging around for a long time, and I think
> are fundamentally due to the slightly mismatched documentation libraries
> that Cygwin has.  With this commit, I get the same noisy warnings, but I
> also get the error "could not open .texi: No such file or directory".
>
> I have to confess, I don't really understand this aspect of GNU Make, so
> I'm not sure if this is a problem with Cygwin having a bad toolset or
> there being something about my environment that means this doesn't work,
> but regardless, it's currently causing the Cygwin Git builds to fail.

Hi. I can reproduce this locally, will look at it and fix it, sorry.

[PATCH] Documentation/Makefile: fix "make info" regression in dad9cd7d518

[Ævar Arnfjörð Bjarmason]

Fix a regression in my dad9cd7d518 (Makefile: move ".SUFFIXES" rule to
shared.mak, 2022-03-03). As explained in the GNU make documentation
for the $* variable, available at:

	info make --index-search='$*'

This rule relied on ".texi" being in the default list of suffixes, as
seen at:

	make -f/dev/null -p | grep -v -e ^# -e ^$|grep -F .SUFFIXES

The documentation explains what was going on here:

	In an explicit rule, there is no stem; so '$*' cannot be determined
	in that way.  Instead, if the target name ends with a recognized
	suffix (*note Old-Fashioned Suffix Rules: Suffix Rules.), '$*' is
	set to the target name minus the suffix.  For example, if the
	target name is 'foo.c', then '$*' is set to 'foo', since '.c' is a
	suffix.  GNU 'make' does this bizarre thing only for compatibility
	with other implementations of 'make'.  You should generally avoid
	using '$*' except in implicit rules or static pattern rules.

	If the target name in an explicit rule does not end with a
	recognized suffix, '$*' is set to the empty string for that rule.

I.e. this rule added back in 5cefc33bffd (Documentation: add
gitman.info target, 2007-12-10) was resolving gitman.texi from
gitman.info. We can instead just use the more obvious $< variable
referring to the prerequisite.

This was the only use of $* in our Makefiles in an explicit rule, the
three remaining ones are all implicit rules, and therefore didn't
depend on the ".SUFFIXES" list.

Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

On Tue, Apr 05 2022, Adam Dinwoodie wrote:

> With this commit, I get the same noisy warnings, but I also get the
> error "could not open .texi: No such file or directory".

Sorry about the regression. This fixes it, and as noted above I'm
pretty sure this was the only fallout of the change.

(I didn't have building the full extended documentation as part of my
local build, but I'll be adding it now).

 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 1eb9192dae8..44c080e3e5b 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -390,7 +390,7 @@ gitman.texi: $(MAN_XML) cat-texi.perl texi.xsl
 	$(RM) $@+
 
 gitman.info: gitman.texi
-	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $*.texi
+	$(QUIET_MAKEINFO)$(MAKEINFO) --no-split --no-validate $<
 
 $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml
 	$(QUIET_DB2TEXI)$(DOCBOOK2X_TEXI) --to-stdout $*.xml >$@
-- 
2.35.1.1604.g35dc6517170

Re: [PATCH] Documentation/Makefile: fix "make info" regression in dad9cd7d518

[Adam Dinwoodie]

On Tue, Apr 05, 2022 at 09:56:20PM +0200, Ævar Arnfjörð Bjarmason wrote:
> Fix a regression in my dad9cd7d518 (Makefile: move ".SUFFIXES" rule to
> shared.mak, 2022-03-03). As explained in the GNU make documentation
> for the $* variable, available at:
> 
> 	info make --index-search='$*'
> 
> This rule relied on ".texi" being in the default list of suffixes, as
> seen at:
> 
> 	make -f/dev/null -p | grep -v -e ^# -e ^$|grep -F .SUFFIXES
> 
> The documentation explains what was going on here:
> 
> 	In an explicit rule, there is no stem; so '$*' cannot be determined
> 	in that way.  Instead, if the target name ends with a recognized
> 	suffix (*note Old-Fashioned Suffix Rules: Suffix Rules.), '$*' is
> 	set to the target name minus the suffix.  For example, if the
> 	target name is 'foo.c', then '$*' is set to 'foo', since '.c' is a
> 	suffix.  GNU 'make' does this bizarre thing only for compatibility
> 	with other implementations of 'make'.  You should generally avoid
> 	using '$*' except in implicit rules or static pattern rules.
> 
> 	If the target name in an explicit rule does not end with a
> 	recognized suffix, '$*' is set to the empty string for that rule.
> 
> I.e. this rule added back in 5cefc33bffd (Documentation: add
> gitman.info target, 2007-12-10) was resolving gitman.texi from
> gitman.info. We can instead just use the more obvious $< variable
> referring to the prerequisite.
> 
> This was the only use of $* in our Makefiles in an explicit rule, the
> three remaining ones are all implicit rules, and therefore didn't
> depend on the ".SUFFIXES" list.
> 
> Reported-by: Adam Dinwoodie <adam@dinwoodie.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> 
> On Tue, Apr 05 2022, Adam Dinwoodie wrote:
> 
> > With this commit, I get the same noisy warnings, but I also get the
> > error "could not open .texi: No such file or directory".
> 
> Sorry about the regression. This fixes it, and as noted above I'm
> pretty sure this was the only fallout of the change.
> 
> (I didn't have building the full extended documentation as part of my
> local build, but I'll be adding it now).

Confirmed this patch fixes things for me.  Thanks for the quick fix!

Tested-by: Adam Dinwoodie <adam@dinwoodie.org>

Re: [PATCH] Documentation/Makefile: fix "make info" regression in dad9cd7d518

[Junio C Hamano]

Adam Dinwoodie <adam@dinwoodie.org> writes:

> Confirmed this patch fixes things for me.  Thanks for the quick fix!
>
> Tested-by: Adam Dinwoodie <adam@dinwoodie.org>

Thanks, both.

Re: [PATCH] Documentation/Makefile: fix "make info" regression in dad9cd7d518

[Taylor Blau]

On Wed, Apr 06, 2022 at 09:43:42AM -0700, Junio C Hamano wrote:
> Adam Dinwoodie <adam@dinwoodie.org> writes:
>
> > Confirmed this patch fixes things for me.  Thanks for the quick fix!
> >
> > Tested-by: Adam Dinwoodie <adam@dinwoodie.org>
>
> Thanks, both.

Indeed; thanks for the fix, Ævar. This sub-thread was a fun read for me
to get to learn about a new "make" quirk. You learn something new every
day ;-).

Thanks,
Taylor

[PATCH v5 6/8] Makefile: move $(comma), $(empty) and $(space) to shared.mak

[Ævar Arnfjörð Bjarmason]

Move these variables over to the shared.mak, we'll make use of them in
a subsequent commit.

Note that there's reason for these to be "simply expanded variables",
i.e. to use ":=" assignments instead of lazily expanded "="
assignments. We could use "=", but let's leave this as-is for now for
ease of review.

See 425ca6710b2 (Makefile: allow combining UBSan with other
sanitizers, 2017-07-15) for the commit that introduced these.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile   | 4 ----
 shared.mak | 8 ++++++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index ce362720947..1e7a1277ce9 100644
--- a/Makefile
+++ b/Makefile
@@ -1289,10 +1289,6 @@ endif
 ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
-comma := ,
-empty :=
-space := $(empty) $(empty)
-
 ifdef SANITIZE
 SANITIZERS := $(foreach flag,$(subst $(comma),$(space),$(SANITIZE)),$(flag))
 BASIC_CFLAGS += -fsanitize=$(SANITIZE) -fno-sanitize-recover=$(SANITIZE)
diff --git a/shared.mak b/shared.mak
index 1dda948df09..934bf428936 100644
--- a/shared.mak
+++ b/shared.mak
@@ -23,3 +23,11 @@
 #
 #    info make --index-search=.DELETE_ON_ERROR
 .DELETE_ON_ERROR:
+
+### Global variables
+
+## comma, empty, space: handy variables as these tokens are either
+## special or can be hard to spot among other Makefile syntax.
+comma := ,
+empty :=
+space := $(empty) $(empty)
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 7/8] Makefile: add "$(QUIET)" boilerplate to shared.mak

[Ævar Arnfjörð Bjarmason]

The $(QUIET) variables we define are largely duplicated between our
various Makefiles, let's define them in the new "shared.mak" instead.

Since we're not using the environment to pass these around we don't
need to export the "QUIET_GEN" and "QUIET_BUILT_IN" variables
anymore. The "QUIET_GEN" variable is used in "git-gui/Makefile" and
"gitweb/Makefile", but they've got their own definition for those. The
"QUIET_BUILT_IN" variable is only used in the top-level "Makefile". We
still need to export the "V" variable.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile  | 32 -------------------------
 Makefile                | 33 -------------------------
 config.mak.uname        |  1 -
 contrib/scalar/Makefile | 14 -----------
 shared.mak              | 53 +++++++++++++++++++++++++++++++++++++++++
 templates/Makefile      |  5 ----
 6 files changed, 53 insertions(+), 85 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..0f4ebdeda8a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -218,38 +218,6 @@ DEFAULT_EDITOR_SQ = $(subst ','\'',$(DEFAULT_EDITOR))
 ASCIIDOC_EXTRA += -a 'git-default-editor=$(DEFAULT_EDITOR_SQ)'
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
-	QUIET		= @
-	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
-	QUIET_XMLTO	= @echo '   ' XMLTO $@;
-	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
-	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
-	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
-	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
-	QUIET_GEN	= @echo '   ' GEN $@;
-	QUIET_STDERR	= 2> /dev/null
-	QUIET_SUBDIR0	= +@subdir=
-	QUIET_SUBDIR1	= ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			  $(MAKE) $(PRINT_DIR) -C $$subdir
-
-	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
-	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
-	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
-
-	export V
-endif
-endif
-
 all: html man
 
 html: $(DOC_HTML)
diff --git a/Makefile b/Makefile
index 1e7a1277ce9..d4c0f4ca7b8 100644
--- a/Makefile
+++ b/Makefile
@@ -2001,39 +2001,6 @@ ifndef PAGER_ENV
 PAGER_ENV = LESS=FRX LV=-c
 endif
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring w,$(MAKEFLAGS)),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_CC       = @echo '   ' CC $@;
-	QUIET_AR       = @echo '   ' AR $@;
-	QUIET_LINK     = @echo '   ' LINK $@;
-	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_LNCP     = @echo '   ' LN/CP $@;
-	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
-	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
-	QUIET_GCOV     = @echo '   ' GCOV $@;
-	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
-	QUIET_RC       = @echo '   ' RC $@;
-	QUIET_SPATCH   = @echo '   ' SPATCH $<;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-	export V
-	export QUIET_GEN
-	export QUIET_BUILT_IN
-endif
-endif
-
 ifdef NO_INSTALL_HARDLINKS
 	export NO_INSTALL_HARDLINKS
 endif
diff --git a/config.mak.uname b/config.mak.uname
index 4352ea39e9b..7727b707b74 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -727,7 +727,6 @@ vcxproj:
 	git diff-index --cached --quiet HEAD --
 
 	# Make .vcxproj files and add them
-	unset QUIET_GEN QUIET_BUILT_IN; \
 	perl contrib/buildsystems/generate -g Vcxproj
 	git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj
 
diff --git a/contrib/scalar/Makefile b/contrib/scalar/Makefile
index d585b319edc..5e86d78e19b 100644
--- a/contrib/scalar/Makefile
+++ b/contrib/scalar/Makefile
@@ -8,20 +8,6 @@ include ../../config.mak.uname
 -include ../../config.mak.autogen
 -include ../../config.mak
 
-QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1  =
-
-ifneq ($(findstring s,$(MAKEFLAGS)),s)
-ifndef V
-	QUIET_GEN      = @echo '   ' GEN $@;
-	QUIET_SUBDIR0  = +@subdir=
-	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
-			 $(MAKE) $(PRINT_DIR) -C $$subdir
-else
-	export V
-endif
-endif
-
 TARGETS = scalar$(X) scalar.o
 GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a
 
diff --git a/shared.mak b/shared.mak
index 934bf428936..c45b2812eb6 100644
--- a/shared.mak
+++ b/shared.mak
@@ -31,3 +31,56 @@
 comma := ,
 empty :=
 space := $(empty) $(empty)
+
+### Quieting
+## common
+QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
+QUIET_SUBDIR1  =
+
+ifneq ($(findstring w,$(MAKEFLAGS)),w)
+PRINT_DIR = --no-print-directory
+else # "make -w"
+NO_SUBDIR = :
+endif
+
+ifneq ($(findstring s,$(MAKEFLAGS)),s)
+ifndef V
+## common
+	QUIET_SUBDIR0  = +@subdir=
+	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
+			 $(MAKE) $(PRINT_DIR) -C $$subdir
+
+	QUIET          = @
+	QUIET_GEN      = @echo '   ' GEN $@;
+
+## Used in "Makefile"
+	QUIET_CC       = @echo '   ' CC $@;
+	QUIET_AR       = @echo '   ' AR $@;
+	QUIET_LINK     = @echo '   ' LINK $@;
+	QUIET_BUILT_IN = @echo '   ' BUILTIN $@;
+	QUIET_LNCP     = @echo '   ' LN/CP $@;
+	QUIET_XGETTEXT = @echo '   ' XGETTEXT $@;
+	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
+	QUIET_GCOV     = @echo '   ' GCOV $@;
+	QUIET_SP       = @echo '   ' SP $<;
+	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
+	QUIET_RC       = @echo '   ' RC $@;
+	QUIET_SPATCH   = @echo '   ' SPATCH $<;
+
+## Used in "Documentation/Makefile"
+	QUIET_ASCIIDOC	= @echo '   ' ASCIIDOC $@;
+	QUIET_XMLTO	= @echo '   ' XMLTO $@;
+	QUIET_DB2TEXI	= @echo '   ' DB2TEXI $@;
+	QUIET_MAKEINFO	= @echo '   ' MAKEINFO $@;
+	QUIET_DBLATEX	= @echo '   ' DBLATEX $@;
+	QUIET_XSLTPROC	= @echo '   ' XSLTPROC $@;
+	QUIET_GEN	= @echo '   ' GEN $@;
+	QUIET_STDERR	= 2> /dev/null
+
+	QUIET_LINT_GITLINK	= @echo '   ' LINT GITLINK $<;
+	QUIET_LINT_MANSEC	= @echo '   ' LINT MAN SEC $<;
+	QUIET_LINT_MANEND	= @echo '   ' LINT MAN END $<;
+
+	export V
+endif
+endif
diff --git a/templates/Makefile b/templates/Makefile
index 636cee52f51..367ad00c24c 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -2,11 +2,6 @@
 include ../shared.mak
 
 # make and install sample templates
-
-ifndef V
-	QUIET = @
-endif
-
 INSTALL ?= install
 TAR ?= tar
 RM ?= rm -f
-- 
2.35.1.1230.ga6e6579e98c

[PATCH v5 8/8] Makefiles: add and use wildcard "mkdir -p" template

[Ævar Arnfjörð Bjarmason]

Add a template to do the "mkdir -p" of $(@D) (the parent dir of $@)
for us, and use it for the "make lint-docs" targets I added in
8650c6298c1 (doc lint: make "lint-docs" non-.PHONY, 2021-10-15).

As seen in 4c64fb5aad9 (Documentation/Makefile: fix lint-docs mkdir
dependency, 2021-10-26) maintaining these manual lists of parent
directory dependencies is fragile, in addition to being obviously
verbose.

I used this pattern at the time because I couldn't find another method
than "order-only" prerequisites to avoid doing a "mkdir -p $(@D)" for
every file being created, which as noted in [1] would be significantly
slower.

But as it turns out we can use this neat trick of only doing a "mkdir
-p" if the $(wildcard) macro tells us the path doesn't exist. A re-run
of a performance test similar to that noted downthread of [1] in [2]
shows that this is faster, in addition to being less verbose and more
reliable (this uses my "git-hyperfine" thin wrapper for "hyperfine"[3]):

    $ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation -j1 lint-docs'
    Benchmark 1: make -C Documentation -j1 lint-docs' in 'HEAD~1
      Time (mean ± σ):      2.914 s ±  0.062 s    [User: 2.449 s, System: 0.489 s]
      Range (min … max):    2.834 s …  3.020 s    10 runs

    Benchmark 2: make -C Documentation -j1 lint-docs' in 'HEAD~0
      Time (mean ± σ):      2.315 s ±  0.062 s    [User: 1.950 s, System: 0.386 s]
      Range (min … max):    2.229 s …  2.397 s    10 runs

    Summary
      'make -C Documentation -j1 lint-docs' in 'HEAD~0' ran
        1.26 ± 0.04 times faster than 'make -C Documentation -j1 lint-docs' in 'HEAD~1'

So let's use that pattern both for the "lint-docs" target, and a few
miscellaneous other targets.

This method of creating parent directories is explicitly racy in that
we don't know if we're going to say always create a "foo" followed by
a "foo/bar" under parallelism, or skip the "foo" because we created
"foo/bar" first. In this case it doesn't matter for anything except
that we aren't guaranteed to get the same number of rules firing when
running make in parallel.

1. https://lore.kernel.org/git/211028.861r45y3pt.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/211028.86o879vvtp.gmgdl@evledraar.gmail.com/
3. https://gitlab.com/avar/git-hyperfine/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/Makefile | 25 +++----------------------
 Makefile               | 12 +++++++-----
 shared.mak             | 17 +++++++++++++++++
 3 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0f4ebdeda8a..1eb9192dae8 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -434,25 +434,11 @@ quick-install-html: require-htmlrepo
 print-man1:
 	@for i in $(MAN1_TXT); do echo $$i; done
 
-## Lint: Common
-.build:
-	$(QUIET)mkdir $@
-.build/lint-docs: | .build
-	$(QUIET)mkdir $@
-
 ## Lint: gitlink
-.build/lint-docs/gitlink: | .build/lint-docs
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/howto: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
-.build/lint-docs/gitlink/config: | .build/lint-docs/gitlink
-	$(QUIET)mkdir $@
 LINT_DOCS_GITLINK = $(patsubst %.txt,.build/lint-docs/gitlink/%.ok,$(HOWTO_TXT) $(DOC_DEP_TXT))
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/howto
-$(LINT_DOCS_GITLINK): | .build/lint-docs/gitlink/config
 $(LINT_DOCS_GITLINK): lint-gitlink.perl
 $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_GITLINK)$(PERL_PATH) lint-gitlink.perl \
 		$< \
 		$(HOWTO_TXT) $(DOC_DEP_TXT) \
@@ -463,23 +449,18 @@ $(LINT_DOCS_GITLINK): .build/lint-docs/gitlink/%.ok: %.txt
 lint-docs-gitlink: $(LINT_DOCS_GITLINK)
 
 ## Lint: man-end-blurb
-.build/lint-docs/man-end-blurb: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_END_BLURB = $(patsubst %.txt,.build/lint-docs/man-end-blurb/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_END_BLURB): | .build/lint-docs/man-end-blurb
 $(LINT_DOCS_MAN_END_BLURB): lint-man-end-blurb.perl
 $(LINT_DOCS_MAN_END_BLURB): .build/lint-docs/man-end-blurb/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANEND)$(PERL_PATH) lint-man-end-blurb.perl $< >$@
 .PHONY: lint-docs-man-end-blurb
-lint-docs-man-end-blurb: $(LINT_DOCS_MAN_END_BLURB)
 
 ## Lint: man-section-order
-.build/lint-docs/man-section-order: | .build/lint-docs
-	$(QUIET)mkdir $@
 LINT_DOCS_MAN_SECTION_ORDER = $(patsubst %.txt,.build/lint-docs/man-section-order/%.ok,$(MAN_TXT))
-$(LINT_DOCS_MAN_SECTION_ORDER): | .build/lint-docs/man-section-order
 $(LINT_DOCS_MAN_SECTION_ORDER): lint-man-section-order.perl
 $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt
+	$(call mkdir_p_parent_template)
 	$(QUIET_LINT_MANSEC)$(PERL_PATH) lint-man-section-order.perl $< >$@
 .PHONY: lint-docs-man-section-order
 lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)
diff --git a/Makefile b/Makefile
index d4c0f4ca7b8..12f08411d2d 100644
--- a/Makefile
+++ b/Makefile
@@ -2745,7 +2745,8 @@ all:: $(MOFILES)
 endif
 
 po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
-	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+	$(call mkdir_p_parent_template)
+	$(QUIET_MSGFMT)$(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
@@ -2761,15 +2762,16 @@ NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN) \
 	sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
 	    -e 's|@@NO_GETTEXT@@|$(NO_GETTEXT_SQ)|g' \
 	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
 	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
-	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	pod2man $< $@
+	$(call mkdir_p_parent_template)
+	$(QUIET_GEN)pod2man $< $@
 
 $(ETAGS_TARGET): $(FOUND_SOURCE_FILES)
 	$(QUIET_GEN)$(RM) $@+ && \
@@ -2903,7 +2905,7 @@ test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(
 all:: $(TEST_PROGRAMS) $(test_bindir_programs)
 
 bin-wrappers/%: wrap-for-bin.sh
-	@mkdir -p bin-wrappers
+	$(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)))|' < $< > $@ && \
diff --git a/shared.mak b/shared.mak
index c45b2812eb6..4e1b62ee99e 100644
--- a/shared.mak
+++ b/shared.mak
@@ -53,6 +53,8 @@ ifndef V
 	QUIET          = @
 	QUIET_GEN      = @echo '   ' GEN $@;
 
+	QUIET_MKDIR_P_PARENT  = @echo $(wspfx_SQ) MKDIR -p $(@D);
+
 ## Used in "Makefile"
 	QUIET_CC       = @echo '   ' CC $@;
 	QUIET_AR       = @echo '   ' AR $@;
@@ -84,3 +86,18 @@ ifndef V
 	export V
 endif
 endif
+
+### Templates
+
+## mkdir_p_parent: lazily "mkdir -p" the path needed for a $@
+## file. Uses $(wildcard) to avoid the "mkdir -p" if it's not
+## needed.
+##
+## Is racy, but in a good way; we might redundantly (and safely)
+## "mkdir -p" when running in parallel, but won't need to exhaustively create
+## individual rules for "a" -> "prefix" -> "dir" -> "file" if given a
+## "a/prefix/dir/file". This can instead be inserted at the start of
+## the "a/prefix/dir/file" rule.
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
-- 
2.35.1.1230.ga6e6579e98c