* [PATCH v3 01/23] Makefile: don't invoke msgfmt with --statistics
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y Ævar Arnfjörð Bjarmason
` (22 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Remove the --statistics flag that I added in 5e9637c6297 (i18n: add
infrastructure for translating Git with gettext, 2011-11-18). Our
Makefile output is good about reducing verbosity by default, except in
this case:
$ rm -rf po/build/locale/e*; time make -j $(nproc) all
SUBDIR templates
MKDIR -p po/build/locale/el/LC_MESSAGES
MSGFMT po/build/locale/el/LC_MESSAGES/git.mo
MKDIR -p po/build/locale/es/LC_MESSAGES
MSGFMT po/build/locale/es/LC_MESSAGES/git.mo
1038 translated messages, 3325 untranslated messages.
5230 translated messages.
I didn't have any good reason for using --statistics at the time other
than ad-hoc eyeballing of the output. We don't need to spew out
exactly how many messages we've got translated every time. Now we'll
instead emit:
$ rm -rf po/build/locale/e*; time make -j $(nproc) all
SUBDIR templates
MKDIR -p po/build/locale/el/LC_MESSAGES
MSGFMT po/build/locale/el/LC_MESSAGES/git.mo
MKDIR -p po/build/locale/es/LC_MESSAGES
MSGFMT po/build/locale/es/LC_MESSAGES/git.mo
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d56c0e4aadc..11da97de233 100644
--- a/Makefile
+++ b/Makefile
@@ -1870,7 +1870,7 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
endif
ifndef NO_MSGFMT_EXTENDED_OPTIONS
- MSGFMT += --check --statistics
+ MSGFMT += --check
endif
ifdef HAVE_CLOCK_GETTIME
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 01/23] Makefile: don't invoke msgfmt with --statistics Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 03/23] Makefile: use "=" not ":=" for po/* and perl/* Ævar Arnfjörð Bjarmason
` (21 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Do not define LIB_{PERL,CPAN}{,_GEN} if NO_PERL is defined. This
changes no functionality, but makes it clear which of these rules are
needed under NO_PERL=Y. See 20d2a30f8ff (Makefile: replace
perl/Makefile.PL with simple make rules, 2017-12-10) for the initial
implementation.
We do for better or worse rely on "install-doc" calling
"install-man-perl" regardless of whether NO_PERL=Y is defined or not,
i.e. we'll always end up with that manual page, even if we don't have
any of the Perl code installed. Let's add a comment about that
adjacent to the rules that build perl/build.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 11da97de233..06405825a24 100644
--- a/Makefile
+++ b/Makefile
@@ -2684,19 +2684,12 @@ endif
po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+ifndef NO_PERL
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))
LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
-ifndef NO_PERL
-all:: $(LIB_PERL_GEN)
-ifndef NO_PERL_CPAN_FALLBACKS
-all:: $(LIB_CPAN_GEN)
-endif
-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 $@) && \
sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
@@ -2704,6 +2697,14 @@ perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
< $< > $@
+all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
+all:: $(LIB_CPAN_GEN)
+endif
+NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
+endif
+
+# install-man depends on Git.3pm even with NO_PERL=Y
perl/build/man/man3/Git.3pm: perl/Git.pm
$(QUIET_GEN)mkdir -p $(dir $@) && \
pod2man $< $@
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 03/23] Makefile: use "=" not ":=" for po/* and perl/*
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 01/23] Makefile: don't invoke msgfmt with --statistics Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 1:57 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 04/23] Makefile: clean perl/build/ even with NO_PERL=Y Ævar Arnfjörð Bjarmason
` (20 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Change these variable definitions from being simply-expanded to be
recursively expanded instead. I.e. they'll be lazily expanded when
used.
I added these in 5e9637c6297 (i18n: add infrastructure for translating
Git with gettext, 2011-11-18) and 20d2a30f8ff (Makefile: replace
perl/Makefile.PL with simple make rules, 2017-12-10), the reason for
using ":=" over "=" was that I didn't know the difference in 2011 (I
think I copied some POC code), and in 2017 I used the 2011 commit for
reference.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 06405825a24..6b77702e102 100644
--- a/Makefile
+++ b/Makefile
@@ -2672,11 +2672,11 @@ po/git.pot: $(GENERATED_H) FORCE
pot: po/git.pot
ifdef NO_GETTEXT
-POFILES :=
-MOFILES :=
+POFILES =
+MOFILES =
else
-POFILES := $(wildcard po/*.po)
-MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
+POFILES = $(wildcard po/*.po)
+MOFILES = $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
all:: $(MOFILES)
endif
@@ -2685,10 +2685,10 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
ifndef NO_PERL
-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))
-LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
-LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
+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))
+LIB_CPAN = $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN = $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
$(QUIET_GEN)mkdir -p $(dir $@) && \
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 03/23] Makefile: use "=" not ":=" for po/* and perl/*
2021-11-16 12:00 ` [PATCH v3 03/23] Makefile: use "=" not ":=" for po/* and perl/* Ævar Arnfjörð Bjarmason
@ 2021-11-17 1:57 ` Mike Hommey
0 siblings, 0 replies; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 1:57 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
On Tue, Nov 16, 2021 at 01:00:03PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Change these variable definitions from being simply-expanded to be
> recursively expanded instead. I.e. they'll be lazily expanded when
> used.
>
> I added these in 5e9637c6297 (i18n: add infrastructure for translating
> Git with gettext, 2011-11-18) and 20d2a30f8ff (Makefile: replace
> perl/Makefile.PL with simple make rules, 2017-12-10), the reason for
> using ":=" over "=" was that I didn't know the difference in 2011 (I
> think I copied some POC code), and in 2017 I used the 2011 commit for
> reference.
FWIW, with those variables being used as dependencies, I don't think this
makes a difference in practice.
Mike
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 04/23] Makefile: clean perl/build/ even with NO_PERL=Y
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 03/23] Makefile: use "=" not ":=" for po/* and perl/* Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR Ævar Arnfjörð Bjarmason
` (19 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Fix a regression in 499c29394ce (Makefile: allow building without
perl, 2009-04-03) where we'd stop cleaning the perl/* directory
because NO_PERL was defined, thus leaving behind litter if the flag at
"clean" time didn't match that of build time.
In 499c29394ce this was done to avoid relying on the perl/Makefile.PL,
but since my 20d2a30f8ff (Makefile: replace perl/Makefile.PL with
simple make rules, 2017-12-10) we can clean things in that directory
unconditionally.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 6b77702e102..a71fba15e30 100644
--- a/Makefile
+++ b/Makefile
@@ -3234,6 +3234,7 @@ clean: profile-clean coverage-clean cocciclean
$(RM) $(HCC)
$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json
$(RM) -r po/build/
+ $(RM) -r perl/build/
$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*
$(RM) -r .dist-tmp-dir .doc-tmp-dir
$(RM) $(GIT_TARNAME).tar.gz
@@ -3242,7 +3243,6 @@ clean: profile-clean coverage-clean cocciclean
$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
ifndef NO_PERL
$(MAKE) -C gitweb clean
- $(RM) -r perl/build/
endif
$(MAKE) -C templates/ clean
$(MAKE) -C t/ clean
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 04/23] Makefile: clean perl/build/ even with NO_PERL=Y Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 2:01 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 06/23] Makefile: guard Perl-only variable assignments Ævar Arnfjörð Bjarmason
` (18 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Since 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
2021-06-29) we don't need to guard the clobbering of $@ with this sort
of "mv $@+ $@" pattern in these cases where we're merely generating a
file that'll be used as a dependency for other files, as in this case
for GIT-PERL-HEADER.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a71fba15e30..284725099c9 100644
--- a/Makefile
+++ b/Makefile
@@ -2349,8 +2349,7 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
-e 's=@@PERLLIBDIR_REL@@=$(perllibdir_relative_SQ)=g' \
-e 's=@@GITEXECDIR_REL@@=$(gitexecdir_relative_SQ)=g' \
-e 's=@@LOCALEDIR_REL@@=$(localedir_relative_SQ)=g' \
- $< >$@+ && \
- mv $@+ $@
+ $< >$@
.PHONY: perllibdir
perllibdir:
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
2021-11-16 12:00 ` [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR Ævar Arnfjörð Bjarmason
@ 2021-11-17 2:01 ` Mike Hommey
2021-11-17 9:20 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 2:01 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Tue, Nov 16, 2021 at 01:00:05PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Since 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
> 2021-06-29) we don't need to guard the clobbering of $@ with this sort
> of "mv $@+ $@" pattern in these cases where we're merely generating a
> file that'll be used as a dependency for other files, as in this case
> for GIT-PERL-HEADER.
.DELETE_ON_ERROR is, as far as I know, a GNUism. Does building git require
the use of GNU make?
Mike
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
2021-11-17 2:01 ` Mike Hommey
@ 2021-11-17 9:20 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 9:20 UTC (permalink / raw)
To: Mike Hommey
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17 2021, Mike Hommey wrote:
> On Tue, Nov 16, 2021 at 01:00:05PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> Since 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
>> 2021-06-29) we don't need to guard the clobbering of $@ with this sort
>> of "mv $@+ $@" pattern in these cases where we're merely generating a
>> file that'll be used as a dependency for other files, as in this case
>> for GIT-PERL-HEADER.
>
> .DELETE_ON_ERROR is, as far as I know, a GNUism. Does building git require
> the use of GNU make?
Yes, we've had a hard dependency on GNU make for forever, and already
use .DELETE_ON_ERROR, this is just moving it around.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 06/23] Makefile: guard Perl-only variable assignments
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL" Ævar Arnfjörð Bjarmason
` (17 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Move the "ifndef NO_PERL" a few lines earlier to encompass the
"perl_localedir_SQ" variable. We'll only use it under !NO_PERL.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 284725099c9..0cb10f00ebb 100644
--- a/Makefile
+++ b/Makefile
@@ -2291,11 +2291,11 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
# This makes sure we depend on the NO_PERL setting itself.
$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
+ifndef NO_PERL
# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
# since the locale directory is injected.
perl_localedir_SQ = $(localedir_SQ)
-ifndef NO_PERL
PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
PERL_DEFINES =
PERL_DEFINES += $(PERL_PATH_SQ)
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL"
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 06/23] Makefile: guard Perl-only variable assignments Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 08/23] Makefile: adjust Perl-related comments & whitespace Ævar Arnfjörð Bjarmason
` (16 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Change the NO_PERL variable assignments so that they declare the much
smaller fallback condition first.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 0cb10f00ebb..288f4834db8 100644
--- a/Makefile
+++ b/Makefile
@@ -2291,7 +2291,15 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
# This makes sure we depend on the NO_PERL setting itself.
$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
-ifndef NO_PERL
+ifdef NO_PERL
+$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
+ $(QUIET_GEN) \
+ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+ -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+ unimplemented.sh >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
+else # NO_PERL
# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
# since the locale directory is injected.
perl_localedir_SQ = $(localedir_SQ)
@@ -2363,14 +2371,6 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
chmod +x $@+ && \
mv $@+ $@
-else # NO_PERL
-$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
- $(QUIET_GEN) \
- sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
- -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
- unimplemented.sh >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
endif # NO_PERL
# This makes sure we depend on the NO_PYTHON setting itself.
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 08/23] Makefile: adjust Perl-related comments & whitespace
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL" Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph Ævar Arnfjörð Bjarmason
` (15 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Folllow-up my 4070c9e09fc (Makefile: don't re-define PERL_DEFINES,
2021-05-05) and move the rest of the assignments to PERL_DEFINES to
one place.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 288f4834db8..ab78f8dd42e 100644
--- a/Makefile
+++ b/Makefile
@@ -2312,21 +2312,19 @@ PERL_DEFINES += $(perllibdir_SQ)
PERL_DEFINES += $(RUNTIME_PREFIX)
PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
PERL_DEFINES += $(NO_GETTEXT)
+PERL_DEFINES += $(gitexecdir)
+PERL_DEFINES += $(perllibdir)
+PERL_DEFINES += $(localedir)
+PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
-# Support Perl runtime prefix. In this mode, a different header is installed
-# into Perl scripts.
ifdef RUNTIME_PREFIX
-
PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
-# Don't export a fixed $(localedir) path; it will be resolved by the Perl header
-# at runtime.
+# The RUNTIME_PREFIX header defines $Git::I18N::TEXTDOMAINDIR, so
+# $(perl_localedir_SQ) won't be needed
perl_localedir_SQ =
-
endif
-PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)
-
$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
$(QUIET_GEN) \
sed -e '1{' \
@@ -2339,7 +2337,6 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
chmod +x $@+ && \
mv $@+ $@
-PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
GIT-PERL-DEFINES: FORCE
@FLAGS='$(PERL_DEFINES)'; \
if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (7 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 08/23] Makefile: adjust Perl-related comments & whitespace Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL" Ævar Arnfjörð Bjarmason
` (14 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Fix several small issues with the dependency graph of the generated
"GIT-PERL-DEFINES" and "GIT-PERL-HEADER" files:
1. Don't have "GIT-PERL-HEADER" depend on the "Makefile". That was a
lazy way to over-declare the dependencies added in
f6a0ad4be71 (Makefile: generate Perl header from template file,
2018-04-10). Let's correct our dependency graph instead.
2. Don't have $(SCRIPT_PERL_GEN) (such as "git-send-email") depend on
GIT-BUILD-OPTIONS. Let's instead use GIT-PERL-DEFINES.
The reason for depending on "GIT-BUILD-OPTIONS" was to trigger a
re-build if NO_PERL=Y was defined. We can instead add that variable
to "PERL_DEFINES", and have "GIT-PERL-DEFINES" created and updated
if "NO_PERL=Y" is defined.
3. Due to #2 we'll need to have GIT-PERL-DEFINES be generated even
under NO_PERL, since that variable will be used by the
"unimplemented.sh" script.
4. Don't depend on $(gitexecdir), $(perllibdir) and $(localedir),
instead depend on the $(*_relative_SQ) versions of those.
The latter is what we'll actually use, while it's unlikely to
matter in practice (we'd just skip re-building these under
RUNTIME_PREFIX if the relative path was the same) it makes the code
easier to read.
That's because this brings us to a 1=1 mapping of these variables
and what's subsequently used in the "GIT-PERL-DEFINES",
"GIT-PERL-HEADER" and "perl/build/lib/%.pm" rules below.
5. We don't need the substitution of " " for ":" added in
07d90eadb50 (Makefile: add Perl runtime prefix support,
2018-04-10), let's drop it. This doesn't matter for the correctness
of these files, because unlike GIT-BUILD-OPTIONS nothing is
consuming them except the Makefile itself.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index ab78f8dd42e..361abff2402 100644
--- a/Makefile
+++ b/Makefile
@@ -2288,10 +2288,14 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
$(shell echo $(GIT_VERSION) 0 0 0 0 | tr '.a-zA-Z-' ' '))) \
-DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" -i $< -o $@
-# This makes sure we depend on the NO_PERL setting itself.
-$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
+# Under NO_PERL=Y we'll still make GIT-PERL-DEFINES. We need to depend
+# on NO_PERL=Y itself for creating "unimplemented.sh" scripts.
+PERL_DEFINES =
+$(SCRIPT_PERL_GEN): GIT-PERL-DEFINES
ifdef NO_PERL
+PERL_DEFINES += $(NO_PERL)
+
$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
$(QUIET_GEN) \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2300,22 +2304,26 @@ $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
chmod +x $@+ && \
mv $@+ $@
else # NO_PERL
-# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
-# since the locale directory is injected.
+# The localedir is only used in Perl modules if !NO_GETTEXT
+ifndef NO_GETTEXT
perl_localedir_SQ = $(localedir_SQ)
+endif
PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
-PERL_DEFINES =
+
PERL_DEFINES += $(PERL_PATH_SQ)
PERL_DEFINES += $(PERLLIB_EXTRA_SQ)
PERL_DEFINES += $(perllibdir_SQ)
PERL_DEFINES += $(RUNTIME_PREFIX)
PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
PERL_DEFINES += $(NO_GETTEXT)
-PERL_DEFINES += $(gitexecdir)
-PERL_DEFINES += $(perllibdir)
-PERL_DEFINES += $(localedir)
-PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
+ifdef RUNTIME_PREFIX
+PERL_DEFINES += $(gitexecdir_relative_SQ)
+PERL_DEFINES += $(perllibdir_relative_SQ)
+PERL_DEFINES += $(localedir_relative_SQ)
+else
+PERL_DEFINES += $(perllocaledir_SQ)
+endif
ifdef RUNTIME_PREFIX
PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
@@ -2337,14 +2345,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
chmod +x $@+ && \
mv $@+ $@
-GIT-PERL-DEFINES: FORCE
- @FLAGS='$(PERL_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new perl-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
-
-GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES
$(QUIET_GEN) \
INSTLIBDIR='$(perllibdir_SQ)' && \
INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
@@ -2370,6 +2371,13 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
mv $@+ $@
endif # NO_PERL
+GIT-PERL-DEFINES: FORCE
+ @FLAGS='$(PERL_DEFINES)'; \
+ if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo >&2 " * new perl-specific parameters"; \
+ echo "$$FLAGS" >$@; \
+ fi
+
# This makes sure we depend on the NO_PYTHON setting itself.
$(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL"
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (8 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts Ævar Arnfjörð Bjarmason
` (13 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Create a new "GIT-PYTHON-DEFINES" file, and untangle the dependency
issues of the Python by copying over the patterns established for
building the adjacent Perl code in preceding commits.
As with Perl, there's no real reason to depend on GIT-BUILD-OPTIONS or
GIT-CFLAGS when building the Python code, nor did we need
GIT-PREFIX. Let's instead add those variables we care about to a
"GIT-PYTHON-DEFINES" and depend on that.
This changes code originally added in ca3bcabf118 (auto-detect changed
prefix and/or changed build flags, 2006-06-15), and adjusted in
96a4647fca5 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
The relevant code for the "Perl" targets was then added in
07981dce81e (Makefile: rebuild perl scripts when perl paths change,
2013-11-18), and has been adjusted in preceding commits.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
.gitignore | 2 +-
Makefile | 49 +++++++++++++++++++++++++------------------------
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/.gitignore b/.gitignore
index 054249b20a8..845e5d0c355 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,7 +8,7 @@
/GIT-PREFIX
/GIT-PERL-DEFINES
/GIT-PERL-HEADER
-/GIT-PYTHON-VARS
+/GIT-PYTHON-DEFINES
/GIT-SCRIPT-DEFINES
/GIT-USER-AGENT
/GIT-VERSION-FILE
diff --git a/Makefile b/Makefile
index 361abff2402..c698c5b058a 100644
--- a/Makefile
+++ b/Makefile
@@ -2378,18 +2378,15 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
-# This makes sure we depend on the NO_PYTHON setting itself.
-$(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS
+# As with NO_PERL=Y we'll still make GIT-PYTHON-DEFINES if "NO_PYTHON"
+# is defined, for creating the "unimplemented.sh" scripts.
+PYTHON_DEFINES =
+$(SCRIPT_PYTHON_GEN): GIT-PYTHON-DEFINES
+
+ifdef NO_PYTHON
+PYTHON_DEFINES += $(SHELL_PATH_SQ)
+PYTHON_DEFINES += $(NO_PYTHON)
-ifndef NO_PYTHON
-$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
-$(SCRIPT_PYTHON_GEN): % : %.py
- $(QUIET_GEN) \
- sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
- $< >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
-else # NO_PYTHON
$(SCRIPT_PYTHON_GEN): % : unimplemented.sh
$(QUIET_GEN) \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2397,8 +2394,24 @@ $(SCRIPT_PYTHON_GEN): % : unimplemented.sh
unimplemented.sh >$@+ && \
chmod +x $@+ && \
mv $@+ $@
+else # NO_PYTHON
+PYTHON_DEFINES += $(PYTHON_PATH_SQ)
+
+$(SCRIPT_PYTHON_GEN): % : %.py GIT-PYTHON-DEFINES
+ $(QUIET_GEN) \
+ sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
+ $< >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
endif # NO_PYTHON
+GIT-PYTHON-DEFINES: FORCE
+ @FLAGS='$(PYTHON_DEFINES)'; \
+ if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo >&2 " * new python-specific parameters"; \
+ echo "$$FLAGS" >$@; \
+ fi
+
CONFIGURE_RECIPE = sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
configure.ac >configure.ac+ && \
autoconf -o configure configure.ac+ && \
@@ -2848,18 +2861,6 @@ else
endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
-### Detect Python interpreter path changes
-ifndef NO_PYTHON
-TRACK_PYTHON = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
-
-GIT-PYTHON-VARS: FORCE
- @VARS='$(TRACK_PYTHON)'; \
- if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new Python interpreter location"; \
- echo "$$VARS" >$@; \
- fi
-endif
-
test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -3256,7 +3257,7 @@ ifndef NO_TCLTK
endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX
- $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
+ $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-DEFINES
ifdef MSVC
$(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS))
$(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS))
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (9 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL" Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
` (12 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Change the hardcoding of @@GIT_VERSION@@ in generated *.perl scripts
to instead shell out to "git version". This means that we can stop
re-building during development every time the HEAD changes.
These codepaths are not "hot", so shelling out to get the version
shouldn't matter to users, in the one case where it potentially would
in send-email (the loop for each E-Mail we send) we now cache the
value, so we'll only retrieve it once.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
git-cvsserver.perl | 6 +++---
git-send-email.perl | 7 ++-----
git-svn.perl | 2 +-
4 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index c698c5b058a..8205614c6ec 100644
--- a/Makefile
+++ b/Makefile
@@ -2333,7 +2333,7 @@ PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
perl_localedir_SQ =
endif
-$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
+$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER
$(QUIET_GEN) \
sed -e '1{' \
-e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 64319bed43f..76f0e8bbbef 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -26,8 +26,6 @@
use File::Basename;
use Getopt::Long qw(:config require_order no_ignore_case);
-my $VERSION = '@@GIT_VERSION@@';
-
my $log = GITCVS::log->new();
my $cfg;
@@ -126,7 +124,9 @@
or die $usage;
if ($state->{version}) {
- print "git-cvsserver version $VERSION\n";
+ my $version = qx[git version];
+ $version =~ s/^(git)\b/$1-cvsserver/;
+ print $version;
exit;
}
if ($state->{help}) {
diff --git a/git-send-email.perl b/git-send-email.perl
index 5262d88ee32..041cd2fb96d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1468,6 +1468,7 @@ sub file_name_is_absolute {
#
# If an error occurs sending the email, this just dies.
+my $gitversion;
sub send_message {
my @recipients = unique_email_list(@to);
@cc = (grep { my $cc = extract_valid_address_or_die($_);
@@ -1478,11 +1479,6 @@ sub send_message {
@recipients = unique_email_list(@recipients,@cc,@initial_bcc);
@recipients = (map { extract_valid_address_or_die($_) } @recipients);
my $date = format_2822_time($time++);
- my $gitversion = '@@GIT_VERSION@@';
- if ($gitversion =~ m/..GIT_VERSION../) {
- $gitversion = Git::version();
- }
-
my $cc = join(",\n\t", unique_email_list(@cc));
my $ccline = "";
if ($cc ne '') {
@@ -1497,6 +1493,7 @@ sub send_message {
Message-Id: $message_id
";
if ($use_xmailer) {
+ $gitversion ||= Git::version();
$header .= "X-Mailer: git-send-email $gitversion\n";
}
if ($in_reply_to) {
diff --git a/git-svn.perl b/git-svn.perl
index be987e316f9..727431412be 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -9,7 +9,6 @@
$_revision $_repository
$_q $_authors $_authors_prog %users/;
$AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
-$VERSION = '@@GIT_VERSION@@';
use Carp qw/croak/;
use File::Basename qw/dirname basename/;
@@ -47,6 +46,7 @@
command_close_bidi_pipe
get_record
);
+$VERSION = Git::version();
BEGIN {
Memoize::memoize 'Git::config';
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (10 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak Ævar Arnfjörð Bjarmason
` (11 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 +++++++++
3 files changed, 15 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 8205614c6ec..5ae7d012cfb 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::
@@ -2158,16 +2161,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:
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (11 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 14/23] Makefile: re-add and use the "shellquote" macros Ævar Arnfjörð Bjarmason
` (10 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 5ae7d012cfb..7130e32a251 100644
--- a/Makefile
+++ b/Makefile
@@ -1252,10 +1252,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 0170bb397ae..2d597ef7603 100644
--- a/shared.mak
+++ b/shared.mak
@@ -7,3 +7,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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 14/23] Makefile: re-add and use the "shellquote" macros
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (12 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
` (9 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Re-add and use, and expand on "shellquote" macros added in
4769948afe7 (Deal with $(bindir) and friends with whitespaces.,
2005-10-10).
We avoided using them due to the "$(call)" feature of GNU make being
relatively new at the time, but it isn't anymore. We hard depend on
GNU make versions that have it.
The use of "$(call)" was removed in 39c015c556f (Fixes for ancient
versions of GNU make, 2006-02-18) and 7ffe7098dca (Fix installation of
templates on ancient systems., 2006-07-29) due to those
incompatibilities with older GNU make versions, and we've used the
more verbose *_SQ pattern ever since.
The "$(call)" feature was introduced in GNU make version 3.78,
released on the 22nd of September, 1999. That release also introduced
"$(error)" and "$(warning)", which we've been making use of since
f2fabbf76e4 (Teach Makefile to check header dependencies, 2010-01-26).
This extends upon the macros added in 4769948afe7: We now have macros
for quoting a ' inside '', and a ' with no surrounding '' as before.
Additionally provide and use a "shelldquote" macro along with
"shellquote" for the common case of wanting to quote a C string we
pass to the compiler with a -D flag.
This doesn't get rid of all of our shell quoting. We've still got some
in the main Makefile, let's leave most of it to avoid in-flight
conflicts. I've fully converted "templates/Makefile" and "t/Makefile"
though.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 15 +++++----------
shared.mak | 14 ++++++++++++++
t/Makefile | 34 +++++++++++++++-------------------
templates/Makefile | 14 +++++---------
4 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/Makefile b/Makefile
index 7130e32a251..b458c24d95e 100644
--- a/Makefile
+++ b/Makefile
@@ -1996,11 +1996,7 @@ ifneq ("$(PROFILE)","")
endif
endif
-# Shell quote (do not use $(call) to accommodate ancient setups);
-
-ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
-ETC_GITATTRIBUTES_SQ = $(subst ','\'',$(ETC_GITATTRIBUTES))
-
+# Shell quote, should be changed to use $(call shellquote,...)
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
NO_GETTEXT_SQ = $(subst ','\'',$(NO_GETTEXT))
bindir_SQ = $(subst ','\'',$(bindir))
@@ -2535,11 +2531,11 @@ builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \
config.sp config.s config.o: GIT-PREFIX
config.sp config.s config.o: EXTRA_CPPFLAGS = \
- -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+ -DETC_GITCONFIG=$(call shelldquote,$(ETC_GITCONFIG))
attr.sp attr.s attr.o: GIT-PREFIX
attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
- -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
+ -DETC_GITATTRIBUTES=$(call shelldquote,$(ETC_GITATTRIBUTES))
gettext.sp gettext.s gettext.o: GIT-PREFIX
gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
@@ -2700,14 +2696,13 @@ perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
$(QUIET_GEN)mkdir -p $(dir $@) && \
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' \
+ -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(call shq,$(NO_PERL_CPAN_FALLBACKS))|g' \
< $< > $@
all:: $(LIB_PERL_GEN)
ifndef NO_PERL_CPAN_FALLBACKS
all:: $(LIB_CPAN_GEN)
endif
-NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
endif
# install-man depends on Git.3pm even with NO_PERL=Y
@@ -3021,7 +3016,7 @@ else
$(INSTALL) $(vcpkg_dbg_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
endif
endif
- $(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
+ $(MAKE) -C templates DESTDIR=$(call shellquote,$(DESTDIR)) install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
ifndef NO_GETTEXT
diff --git a/shared.mak b/shared.mak
index 2d597ef7603..ef03c2bc094 100644
--- a/shared.mak
+++ b/shared.mak
@@ -8,6 +8,20 @@
# info make --index-search=.DELETE_ON_ERROR
.DELETE_ON_ERROR:
+### Quoting helpers
+
+## Quote a ' inside a '': FOO='$(call shq,$(BAR))'
+shq = $(subst ','\'',$(1))
+
+## Quote a ' and provide a '': FOO=$(call shq,$(BAR))
+shellquote = '$(call shq,$(1))'
+
+## Quote a " inside a ""
+shdq = $(subst ",\",$(1))
+
+## Quote ' for the shell, and embedded " for C: -DFOO=$(call shelldquote,$(BAR))
+shelldquote = '"$(call shdq,$(call shq,$(1)))"'
+
### Global variables
## comma, empty, space: handy variables as these tokens are either
diff --git a/t/Makefile b/t/Makefile
index 882d26eee30..4168b5c6ce6 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
@@ -24,13 +27,6 @@ TEST_RESULTS_DIRECTORY = test-results
CHAINLINTTMP = chainlinttmp
endif
-# Shell quote;
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
-CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
-
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
@@ -44,38 +40,38 @@ test: pre-clean check-chainlint $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
failed:
- @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
+ @failed=$$(cd $(call shellquote,$(TEST_RESULTS_DIRECTORY)) && \
grep -l '^failed [1-9]' *.counts | \
sed -n 's/\.counts$$/.sh/p') && \
test -z "$$failed" || $(MAKE) $$failed
prove: pre-clean check-chainlint $(TEST_LINT)
- @echo "*** prove ***"; $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+ @echo "*** prove ***"; $(PROVE) --exec $(call shellquote,$(SHELL_PATH)) $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
$(T):
- @echo "*** $@ ***"; '$(TEST_SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+ @echo "*** $@ ***"; $(call shellquote,$(SHELL_PATH)) $@ $(GIT_TEST_OPTS)
pre-clean:
- $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
+ $(RM) -r $(call shellquote,$(TEST_RESULTS_DIRECTORY))
clean-except-prove-cache: clean-chainlint
- $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'
+ $(RM) -r 'trash directory'.* $(call shellquote,$(TEST_RESULTS_DIRECTORY))
$(RM) -r valgrind/bin
clean: clean-except-prove-cache
$(RM) .prove
clean-chainlint:
- $(RM) -r '$(CHAINLINTTMP_SQ)'
+ $(RM) -r $(call shellquote,$(CHAINLINTTMP))
check-chainlint:
- @mkdir -p '$(CHAINLINTTMP_SQ)' && \
+ @mkdir -p $(call shellquote,$(CHAINLINTTMP)) && \
err=0 && \
for i in $(CHAINLINTTESTS); do \
$(CHAINLINT) <chainlint/$$i.test | \
- sed -e '/^# LINT: /d' >'$(CHAINLINTTMP_SQ)'/$$i.actual && \
- diff -u chainlint/$$i.expect '$(CHAINLINTTMP_SQ)'/$$i.actual || err=1; \
+ sed -e '/^# LINT: /d' >$(call shellquote,$(CHAINLINTTMP))/$$i.actual && \
+ diff -u chainlint/$$i.expect $(call shellquote,$(CHAINLINTTMP))/$$i.actual || err=1; \
done && exit $$err
test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
@@ -92,7 +88,7 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
+ @$(call shellquote,$(PERL_PATH)) check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
test-lint-filenames:
@# We do *not* pass a glob to ls-files but use grep instead, to catch
@@ -107,9 +103,9 @@ aggregate-results-and-cleanup: $(T)
$(MAKE) clean
aggregate-results:
- for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
+ for f in $(call shellquote,$(TEST_RESULTS_DIRECTORY))/t*-*.counts; do \
echo "$$f"; \
- done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh
+ done | $(call shellquote,$(SHELL_PATH)) ./aggregate-results.sh
gitweb-test:
$(MAKE) $(TGITWEB)
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..c9251a96622 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
@@ -18,13 +21,6 @@ ifndef PERL_PATH
PERL_PATH = perl
endif
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-
-# Shell quote (do not use $(call) to accommodate ancient setups);
-DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
-template_instdir_SQ = $(subst ','\'',$(template_instdir))
-
all: boilerplates.made custom
# Put templates that can be copied straight from the source
@@ -61,6 +57,6 @@ clean:
$(RM) -r blt boilerplates.made
install: all
- $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
+ $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(template_instdir))
(cd blt && $(TAR) cf - .) | \
- (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xof -)
+ (cd $(call shellquote,$(DESTDIR)$(template_instdir)) && umask 022 && $(TAR) xof -)
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (13 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 14/23] Makefile: re-add and use the "shellquote" macros Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 2:19 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak Ævar Arnfjörð Bjarmason
` (8 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Move the mostly copy/pasted code in the "Makefile" and
"Documentation/Makefile" which FORCE-generates a file based on a
variable in play in the Makefile to use a template.
This will make it easier later on to move these to rules that aren't
FORCE-run using optional gmake 4.2+ features, but for now just getting
rid of the repetition is worth it.
The message for the new generated rule will say whether or not we're
generating the file for the first time, as opposed to the old messages
saying "new" whether we had flag modifications, or were building for
the first time.
Example output before:
$ make clean
[...]
$ make
GIT_VERSION = 2.34.0-rc1-dev
* new build flags
CC grep.o
$ make CFLAGS=-I$RANDOM grep.o
* new build flags
CC grep.o
After:
$ make clean
[...]
$ make grep.o
GIT_VERSION = 2.34.0-rc1-dev
GIT-CFLAGS PARAMETERS (new)
CC grep.o
$ make CFLAGS=-I$RANDOM grep.o
GIT-CFLAGS PARAMETERS (changed)
CC grep.o
Note: It's important that "@FLAGS" here be defined as '$$($(2))', and
not the eagerly expanded '$($(2))'. The latter will break if
e.g. curl-config isn't installed, since we'll end up recursively
expanding that part of the variable even if NO_CURL isn't defined,
which happens e.g. for the "check-docs" target in CI.
We're also introducing a $(wspfx) variable here to control the
whitespace prefixing. It matches the $(QUIET...) variables, who'll be
using these variables in a subsequent commit. Note that it's important
that we call the shell quote escaping macros inline (or equivalent),
because if we'd like variables to be overridable we need to support e.g.:
$ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
-> GIT-CFLAGS PARAMETERS (changed)
CC grep.o
If we simply quoted and used $(wspfx) then the user would need to
provide us with a quoted version, so there's still some use-cases for
these $(*_sq) variables. It could also be done inline, but that's a
lot more verbose.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Documentation/Makefile | 8 +------
Makefile | 51 ++++++------------------------------------
shared.mak | 22 ++++++++++++++++++
3 files changed, 30 insertions(+), 51 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..4a939cc2c25 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -343,13 +343,7 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
date >$@
TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
-
-GIT-ASCIIDOCFLAGS: FORCE
- @FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-ASCIIDOCFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new asciidoc flags"; \
- echo "$$FLAGS" >GIT-ASCIIDOCFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-ASCIIDOCFLAGS,TRACK_ASCIIDOCFLAGS))
clean:
$(RM) -rf .build/
diff --git a/Makefile b/Makefile
index b458c24d95e..c8a0a1586ca 100644
--- a/Makefile
+++ b/Makefile
@@ -2087,10 +2087,7 @@ endif
GIT_USER_AGENT_SQ = $(subst ','\'',$(GIT_USER_AGENT))
GIT_USER_AGENT_CQ = "$(subst ",\",$(subst \,\\,$(GIT_USER_AGENT)))"
GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ))
-GIT-USER-AGENT: FORCE
- @if test x'$(GIT_USER_AGENT_SQ)' != x"`cat GIT-USER-AGENT 2>/dev/null`"; then \
- echo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \
- fi
+$(eval $(call TRACK_template,GIT-USER-AGENT,GIT_USER_AGENT_SQ))
ifdef DEFAULT_HELP_FORMAT
BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='"$(DEFAULT_HELP_FORMAT)"'
@@ -2238,12 +2235,7 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
$(perllibdir_SQ)
-GIT-SCRIPT-DEFINES: FORCE
- @FLAGS='$(SCRIPT_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new script parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-SCRIPT-DEFINES,SCRIPT_DEFINES))
define cmd_munge_script
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2355,13 +2347,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
chmod +x $@+ && \
mv $@+ $@
endif # NO_PERL
-
-GIT-PERL-DEFINES: FORCE
- @FLAGS='$(PERL_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new perl-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-PERL-DEFINES,PERL_DEFINES))
# As with NO_PERL=Y we'll still make GIT-PYTHON-DEFINES if "NO_PYTHON"
# is defined, for creating the "unimplemented.sh" scripts.
@@ -2390,12 +2376,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py GIT-PYTHON-DEFINES
mv $@+ $@
endif # NO_PYTHON
-GIT-PYTHON-DEFINES: FORCE
- @FLAGS='$(PYTHON_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new python-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-PYTHON-DEFINES,PYTHON_DEFINES))
CONFIGURE_RECIPE = sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
configure.ac >configure.ac+ && \
@@ -2751,31 +2732,13 @@ cscope: cscope.out
### Detect prefix changes
TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\
$(localedir_SQ)
-
-GIT-PREFIX: FORCE
- @FLAGS='$(TRACK_PREFIX)'; \
- if test x"$$FLAGS" != x"`cat GIT-PREFIX 2>/dev/null`" ; then \
- echo >&2 " * new prefix flags"; \
- echo "$$FLAGS" >GIT-PREFIX; \
- fi
+$(eval $(call TRACK_template,GIT-PREFIX,TRACK_PREFIX))
TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)
-
-GIT-CFLAGS: FORCE
- @FLAGS='$(TRACK_CFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new build flags"; \
- echo "$$FLAGS" >GIT-CFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-CFLAGS,TRACK_CFLAGS))
TRACK_LDFLAGS = $(subst ','\'',$(ALL_LDFLAGS))
-
-GIT-LDFLAGS: FORCE
- @FLAGS='$(TRACK_LDFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-LDFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new link flags"; \
- echo "$$FLAGS" >GIT-LDFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-LDFLAGS,TRACK_LDFLAGS))
# We need to apply sq twice, once to protect from the shell
# that runs GIT-BUILD-OPTIONS, and then again to protect it
diff --git a/shared.mak b/shared.mak
index ef03c2bc094..3b4163e652a 100644
--- a/shared.mak
+++ b/shared.mak
@@ -29,3 +29,25 @@ shelldquote = '"$(call shdq,$(call shq,$(1)))"'
comma = ,
empty =
space = $(empty) $(empty)
+
+## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
+## aligned output.
+wspfx = $(space)$(space)$(space)
+wspfx_sq = $(call shellquote,$(wspfx))
+
+### Templates
+
+## Template for making a GIT-SOMETHING, which changes if a
+## TRACK_SOMETHING variable changes.
+define TRACK_template
+.PHONY: FORCE
+$(1): FORCE
+ @FLAGS='$$($(2))'; \
+ if ! test -f $(1) ; then \
+ echo $(wspfx_sq) "$(1) PARAMETERS (new)" $@; \
+ echo "$$$$FLAGS" >$(1); \
+ elif test x"$$$$FLAGS" != x"`cat $(1) 2>/dev/null`" ; then \
+ echo $(wspfx_sq) "$(1) PARAMETERS (changed)" $@; \
+ echo "$$$$FLAGS" >$(1); \
+ fi
+endef
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-16 12:00 ` [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
@ 2021-11-17 2:19 ` Mike Hommey
2021-11-17 9:24 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 2:19 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
On Tue, Nov 16, 2021 at 01:00:15PM +0100, Ævar Arnfjörð Bjarmason wrote:
> + echo $(wspfx_sq) "$(1) PARAMETERS (new)" $@; \
> + echo "$$$$FLAGS" >$(1); \
> + elif test x"$$$$FLAGS" != x"`cat $(1) 2>/dev/null`" ; then \
> + echo $(wspfx_sq) "$(1) PARAMETERS (changed)" $@; \
These $@ will expand to nothing, I think.
Mike
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-17 2:19 ` Mike Hommey
@ 2021-11-17 9:24 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 9:24 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
On Wed, Nov 17 2021, Mike Hommey wrote:
> On Tue, Nov 16, 2021 at 01:00:15PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> + echo $(wspfx_sq) "$(1) PARAMETERS (new)" $@; \
>> + echo "$$$$FLAGS" >$(1); \
>> + elif test x"$$$$FLAGS" != x"`cat $(1) 2>/dev/null`" ; then \
>> + echo $(wspfx_sq) "$(1) PARAMETERS (changed)" $@; \
>
> These $@ will expand to nothing, I think.
That's correct, oops, I had that left over from some previous
version. Will fix.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (14 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 4a939cc2c25..69a9af35397 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 c8a0a1586ca..c437aea9e4a 100644
--- a/Makefile
+++ b/Makefile
@@ -1939,39 +1939,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 d0701f9beb0..1a12d8c635f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -715,7 +715,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 3b4163e652a..80176f705fc 100644
--- a/shared.mak
+++ b/shared.mak
@@ -35,6 +35,59 @@ space = $(empty) $(empty)
wspfx = $(space)$(space)$(space)
wspfx_sq = $(call shellquote,$(wspfx))
+### 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
+
### Templates
## Template for making a GIT-SOMETHING, which changes if a
diff --git a/templates/Makefile b/templates/Makefile
index c9251a96622..b056e710b7e 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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (15 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 | 48 ++++++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/shared.mak b/shared.mak
index 80176f705fc..a1b62b272f8 100644
--- a/shared.mak
+++ b/shared.mak
@@ -50,39 +50,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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (16 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 2:51 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 19/23] Makefile: correct the dependency graph of hook-list.h Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
Range (min … max): 2.121 s … 2.158 s 10 runs
Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
Range (min … max): 2.657 s … 2.662 s 10 runs
Summary
'make -C Documentation lint-docs' in 'HEAD~0' ran
1.25 ± 0.01 times faster than 'make -C Documentation 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 | 7 +++++++
3 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 69a9af35397..d16b653394c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -428,25 +428,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) \
@@ -457,23 +443,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 c437aea9e4a..0a3f292bf82 100644
--- a/Makefile
+++ b/Makefile
@@ -2632,7 +2632,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 $@ $<
ifndef NO_PERL
LIB_PERL = $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
@@ -2641,7 +2642,8 @@ LIB_CPAN = $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
LIB_CPAN_GEN = $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
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@@|$(call shq,$(NO_PERL_CPAN_FALLBACKS))|g' \
@@ -2655,8 +2657,8 @@ endif
# install-man depends on Git.3pm even with NO_PERL=Y
perl/build/man/man3/Git.3pm: perl/Git.pm
- $(QUIET_GEN)mkdir -p $(dir $@) && \
- pod2man $< $@
+ $(call mkdir_p_parent_template)
+ $(QUIET_GEN)pod2man $< $@
FIND_SOURCE_FILES = ( \
git ls-files \
@@ -2780,7 +2782,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 a1b62b272f8..363138a5577 100644
--- a/shared.mak
+++ b/shared.mak
@@ -56,6 +56,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 $@;
@@ -88,6 +90,11 @@ ifndef V
endif
endif
+## Helpers
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
+
### Templates
## Template for making a GIT-SOMETHING, which changes if a
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-16 12:00 ` [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
@ 2021-11-17 2:51 ` Mike Hommey
2021-11-17 9:26 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 2:51 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
> Range (min … max): 2.121 s … 2.158 s 10 runs
>
> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
> Range (min … max): 2.657 s … 2.662 s 10 runs
>
> Summary
> 'make -C Documentation lint-docs' in 'HEAD~0' ran
> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
Something else that is racy is that $(wildcard) might be saying the
directory doesn't exist while there's another make subprocess that has
already started spawning `mkdir -p` for that directory.
That doesn't make a huge difference, but you can probably still end up
with multiple `mkdir -p` runs for the same directory.
I think something like the following could work while avoiding those
races:
define create_parent_dir_RULE
$(1): | $(dir $(1)).
ALL_DIRS += $(dir $(1))
endef
define create_parent_dir_TARGET
$(1)/.: $(dir $(1)).
echo mkdir $$(@D)
endef
$(eval $(call create_parent_dir_RULE, first/path/file))
$(eval $(call create_parent_dir_RULE, second/path/file))
# ...
$(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
Mike
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-17 2:51 ` Mike Hommey
@ 2021-11-17 9:26 ` Ævar Arnfjörð Bjarmason
2021-11-17 9:39 ` Mike Hommey
0 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 9:26 UTC (permalink / raw)
To: Mike Hommey
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17 2021, Mike Hommey wrote:
> On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
>> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
>> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
>> Range (min … max): 2.121 s … 2.158 s 10 runs
>>
>> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
>> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
>> Range (min … max): 2.657 s … 2.662 s 10 runs
>>
>> Summary
>> 'make -C Documentation lint-docs' in 'HEAD~0' ran
>> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
>
> Something else that is racy is that $(wildcard) might be saying the
> directory doesn't exist while there's another make subprocess that has
> already started spawning `mkdir -p` for that directory.
> That doesn't make a huge difference, but you can probably still end up
> with multiple `mkdir -p` runs for the same directory.
>
> I think something like the following could work while avoiding those
> races:
>
> define create_parent_dir_RULE
> $(1): | $(dir $(1)).
> ALL_DIRS += $(dir $(1))
> endef
>
> define create_parent_dir_TARGET
> $(1)/.: $(dir $(1)).
> echo mkdir $$(@D)
> endef
>
> $(eval $(call create_parent_dir_RULE, first/path/file))
> $(eval $(call create_parent_dir_RULE, second/path/file))
> # ...
>
> $(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
I think the "race" just isn't a problem, and makes managing this much
simpler.
I.e. we already rely on "mkdir -p" not failing on an existing directory,
so the case where we redundantly try to create a directory that just got
created by a concurrent process is OK, and as the quoted benchmark shows
is much faster than a similar (but not quite the same as) a
dependency-based implementaiton.
I haven't implemented your solution, but it seems to be inherently more
complex.
I.e. with the one I've got you just stick the "mkdir if needed"
one-liner in each rule, with yours you'll need to accumulate things in
ALL_DIRS, and have some foreach somewhere or dependency relationship to
create those beforehand if they're nested, no?
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-17 9:26 ` Ævar Arnfjörð Bjarmason
@ 2021-11-17 9:39 ` Mike Hommey
2021-11-17 11:52 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 9:39 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17, 2021 at 10:26:27AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Nov 17 2021, Mike Hommey wrote:
>
> > On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
> >> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
> >> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
> >> Range (min … max): 2.121 s … 2.158 s 10 runs
> >>
> >> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
> >> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
> >> Range (min … max): 2.657 s … 2.662 s 10 runs
> >>
> >> Summary
> >> 'make -C Documentation lint-docs' in 'HEAD~0' ran
> >> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
> >
> > Something else that is racy is that $(wildcard) might be saying the
> > directory doesn't exist while there's another make subprocess that has
> > already started spawning `mkdir -p` for that directory.
> > That doesn't make a huge difference, but you can probably still end up
> > with multiple `mkdir -p` runs for the same directory.
> >
> > I think something like the following could work while avoiding those
> > races:
> >
> > define create_parent_dir_RULE
> > $(1): | $(dir $(1)).
> > ALL_DIRS += $(dir $(1))
> > endef
> >
> > define create_parent_dir_TARGET
> > $(1)/.: $(dir $(1)).
> > echo mkdir $$(@D)
erf, s/echo //
> > endef
> >
> > $(eval $(call create_parent_dir_RULE, first/path/file))
> > $(eval $(call create_parent_dir_RULE, second/path/file))
> > # ...
> >
> > $(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
>
> I think the "race" just isn't a problem, and makes managing this much
> simpler.
>
> I.e. we already rely on "mkdir -p" not failing on an existing directory,
> so the case where we redundantly try to create a directory that just got
> created by a concurrent process is OK, and as the quoted benchmark shows
> is much faster than a similar (but not quite the same as) a
> dependency-based implementaiton.
>
> I haven't implemented your solution, but it seems to be inherently more
> complex.
>
> I.e. with the one I've got you just stick the "mkdir if needed"
> one-liner in each rule, with yours you'll need to accumulate things in
> ALL_DIRS, and have some foreach somewhere or dependency relationship to
> create those beforehand if they're nested, no?
For each rule, it would also be a oneliner to add above the rule. The rest
would be a prelude and a an epilogue to stick somewhere in the Makefile.
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-17 9:39 ` Mike Hommey
@ 2021-11-17 11:52 ` Ævar Arnfjörð Bjarmason
2021-11-18 0:00 ` Mike Hommey
0 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 11:52 UTC (permalink / raw)
To: Mike Hommey
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17 2021, Mike Hommey wrote:
> On Wed, Nov 17, 2021 at 10:26:27AM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Nov 17 2021, Mike Hommey wrote:
>>
>> > On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
>> >> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
>> >> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
>> >> Range (min … max): 2.121 s … 2.158 s 10 runs
>> >>
>> >> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
>> >> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
>> >> Range (min … max): 2.657 s … 2.662 s 10 runs
>> >>
>> >> Summary
>> >> 'make -C Documentation lint-docs' in 'HEAD~0' ran
>> >> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
>> >
>> > Something else that is racy is that $(wildcard) might be saying the
>> > directory doesn't exist while there's another make subprocess that has
>> > already started spawning `mkdir -p` for that directory.
>> > That doesn't make a huge difference, but you can probably still end up
>> > with multiple `mkdir -p` runs for the same directory.
>> >
>> > I think something like the following could work while avoiding those
>> > races:
>> >
>> > define create_parent_dir_RULE
>> > $(1): | $(dir $(1)).
>> > ALL_DIRS += $(dir $(1))
>> > endef
>> >
>> > define create_parent_dir_TARGET
>> > $(1)/.: $(dir $(1)).
>> > echo mkdir $$(@D)
>
> erf, s/echo //
>
>> > endef
>> >
>> > $(eval $(call create_parent_dir_RULE, first/path/file))
>> > $(eval $(call create_parent_dir_RULE, second/path/file))
>> > # ...
>> >
>> > $(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
>>
>> I think the "race" just isn't a problem, and makes managing this much
>> simpler.
>>
>> I.e. we already rely on "mkdir -p" not failing on an existing directory,
>> so the case where we redundantly try to create a directory that just got
>> created by a concurrent process is OK, and as the quoted benchmark shows
>> is much faster than a similar (but not quite the same as) a
>> dependency-based implementaiton.
>>
>> I haven't implemented your solution, but it seems to be inherently more
>> complex.
>>
>> I.e. with the one I've got you just stick the "mkdir if needed"
>> one-liner in each rule, with yours you'll need to accumulate things in
>> ALL_DIRS, and have some foreach somewhere or dependency relationship to
>> create those beforehand if they're nested, no?
>
> For each rule, it would also be a oneliner to add above the rule. The rest
> would be a prelude and a an epilogue to stick somewhere in the Makefile.
How would that epilogue know to handle cases where we're running "clean"
or whatever thing doesn't want to create the full set of directories
we've accumulated in ALL_DIRS while parsing the Makefile?
Won't that require extensive use of the same sort of MAKECMDGOALS
trickery I added for one target in 8cc804d0abf (doc build: speed up
"make lint-docs", 2021-10-15)?
I can imagine how we /could/ get that to work, but so far I just dont's
see the point. The wildcard trick I'm proposing here seems to be the
best of both worlds, it's both faster than anything else I've come up
with or seen, and requires zero management outside the rule itself.
Also, only the lint-docs rule I'm modifying here went to any trouble to
avoid redundant "mkdir" calls at all, the rest are all doing a "mkdir
parent-dir" every single time. So as far as any edge cases with these
rules tripping over one another are concerned I think we're already
handling that.
Well, I suppose *theoretically* not, because I'm assuming that we only
ever run the equivalent of "make all" or "make clean && make all" and
not "make -jN clean all".
But while it's possible to get a Makefile working that can clean and
build things in parallel in practice nobody does that, and ours
doesn't. If you try that it'll fail right away on some bad dependency
(which we both just generated and rm -rf'd).
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-17 11:52 ` Ævar Arnfjörð Bjarmason
@ 2021-11-18 0:00 ` Mike Hommey
2021-11-18 13:05 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-18 0:00 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17, 2021 at 12:52:11PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Nov 17 2021, Mike Hommey wrote:
>
> > On Wed, Nov 17, 2021 at 10:26:27AM +0100, Ævar Arnfjörð Bjarmason wrote:
> >>
> >> On Wed, Nov 17 2021, Mike Hommey wrote:
> >>
> >> > On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> >> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
> >> >> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
> >> >> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
> >> >> Range (min … max): 2.121 s … 2.158 s 10 runs
> >> >>
> >> >> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
> >> >> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
> >> >> Range (min … max): 2.657 s … 2.662 s 10 runs
> >> >>
> >> >> Summary
> >> >> 'make -C Documentation lint-docs' in 'HEAD~0' ran
> >> >> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
> >> >
> >> > Something else that is racy is that $(wildcard) might be saying the
> >> > directory doesn't exist while there's another make subprocess that has
> >> > already started spawning `mkdir -p` for that directory.
> >> > That doesn't make a huge difference, but you can probably still end up
> >> > with multiple `mkdir -p` runs for the same directory.
> >> >
> >> > I think something like the following could work while avoiding those
> >> > races:
> >> >
> >> > define create_parent_dir_RULE
> >> > $(1): | $(dir $(1)).
> >> > ALL_DIRS += $(dir $(1))
> >> > endef
> >> >
> >> > define create_parent_dir_TARGET
> >> > $(1)/.: $(dir $(1)).
> >> > echo mkdir $$(@D)
> >
> > erf, s/echo //
> >
> >> > endef
> >> >
> >> > $(eval $(call create_parent_dir_RULE, first/path/file))
> >> > $(eval $(call create_parent_dir_RULE, second/path/file))
> >> > # ...
> >> >
> >> > $(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
> >>
> >> I think the "race" just isn't a problem, and makes managing this much
> >> simpler.
> >>
> >> I.e. we already rely on "mkdir -p" not failing on an existing directory,
> >> so the case where we redundantly try to create a directory that just got
> >> created by a concurrent process is OK, and as the quoted benchmark shows
> >> is much faster than a similar (but not quite the same as) a
> >> dependency-based implementaiton.
> >>
> >> I haven't implemented your solution, but it seems to be inherently more
> >> complex.
> >>
> >> I.e. with the one I've got you just stick the "mkdir if needed"
> >> one-liner in each rule, with yours you'll need to accumulate things in
> >> ALL_DIRS, and have some foreach somewhere or dependency relationship to
> >> create those beforehand if they're nested, no?
> >
> > For each rule, it would also be a oneliner to add above the rule. The rest
> > would be a prelude and a an epilogue to stick somewhere in the Makefile.
>
> How would that epilogue know to handle cases where we're running "clean"
> or whatever thing doesn't want to create the full set of directories
> we've accumulated in ALL_DIRS while parsing the Makefile?
The epilogue only adds rules like:
dir/subdir/.: dir/.
mkdir $(@D)
As long as those "clean" or whatever rules don't depend on those,
nothing will happen.
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-18 0:00 ` Mike Hommey
@ 2021-11-18 13:05 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-18 13:05 UTC (permalink / raw)
To: Mike Hommey
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Thu, Nov 18 2021, Mike Hommey wrote:
> On Wed, Nov 17, 2021 at 12:52:11PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Nov 17 2021, Mike Hommey wrote:
>>
>> > On Wed, Nov 17, 2021 at 10:26:27AM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >>
>> >> On Wed, Nov 17 2021, Mike Hommey wrote:
>> >>
>> >> > On Tue, Nov 16, 2021 at 01:00:18PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >> >> 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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
>> >> >> Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
>> >> >> Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
>> >> >> Range (min … max): 2.121 s … 2.158 s 10 runs
>> >> >>
>> >> >> Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
>> >> >> Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
>> >> >> Range (min … max): 2.657 s … 2.662 s 10 runs
>> >> >>
>> >> >> Summary
>> >> >> 'make -C Documentation lint-docs' in 'HEAD~0' ran
>> >> >> 1.25 ± 0.01 times faster than 'make -C Documentation 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.
>> >> >
>> >> > Something else that is racy is that $(wildcard) might be saying the
>> >> > directory doesn't exist while there's another make subprocess that has
>> >> > already started spawning `mkdir -p` for that directory.
>> >> > That doesn't make a huge difference, but you can probably still end up
>> >> > with multiple `mkdir -p` runs for the same directory.
>> >> >
>> >> > I think something like the following could work while avoiding those
>> >> > races:
>> >> >
>> >> > define create_parent_dir_RULE
>> >> > $(1): | $(dir $(1)).
>> >> > ALL_DIRS += $(dir $(1))
>> >> > endef
>> >> >
>> >> > define create_parent_dir_TARGET
>> >> > $(1)/.: $(dir $(1)).
>> >> > echo mkdir $$(@D)
>> >
>> > erf, s/echo //
>> >
>> >> > endef
>> >> >
>> >> > $(eval $(call create_parent_dir_RULE, first/path/file))
>> >> > $(eval $(call create_parent_dir_RULE, second/path/file))
>> >> > # ...
>> >> >
>> >> > $(foreach dir,$(sort $(ALL_DIRS)),$(eval $(call create_parent_dir_TARGET,$(dir:%/=%))))
>> >>
>> >> I think the "race" just isn't a problem, and makes managing this much
>> >> simpler.
>> >>
>> >> I.e. we already rely on "mkdir -p" not failing on an existing directory,
>> >> so the case where we redundantly try to create a directory that just got
>> >> created by a concurrent process is OK, and as the quoted benchmark shows
>> >> is much faster than a similar (but not quite the same as) a
>> >> dependency-based implementaiton.
>> >>
>> >> I haven't implemented your solution, but it seems to be inherently more
>> >> complex.
>> >>
>> >> I.e. with the one I've got you just stick the "mkdir if needed"
>> >> one-liner in each rule, with yours you'll need to accumulate things in
>> >> ALL_DIRS, and have some foreach somewhere or dependency relationship to
>> >> create those beforehand if they're nested, no?
>> >
>> > For each rule, it would also be a oneliner to add above the rule. The rest
>> > would be a prelude and a an epilogue to stick somewhere in the Makefile.
>>
>> How would that epilogue know to handle cases where we're running "clean"
>> or whatever thing doesn't want to create the full set of directories
>> we've accumulated in ALL_DIRS while parsing the Makefile?
>
> The epilogue only adds rules like:
>
> dir/subdir/.: dir/.
> mkdir $(@D)
>
> As long as those "clean" or whatever rules don't depend on those,
> nothing will happen.
Ah, I see.
I don't see why why this pattern would be preferrable to the $(wildcard)
idiom I'm introducing, which doesn't require any boilerplate at all.
We've got that in snippet form on the one hand, and then my working
patch. I haven't tried to implement what you suggested, but don't see
how it wouldn't be the same thing speed-wise as the explicitly
enumerated dependiencies I replaced for the lint-docs target.
So unless there's something broken etc. about that approach I think we
should go forward with it.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 19/23] Makefile: correct the dependency graph of hook-list.h
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (17 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 18/23] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 2:52 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Fix an issue in my cfe853e66be (hook-list.h: add a generated list of
hooks, like config-list.h, 2021-09-26), the builtin/help.c was
inadvertently made to depend on hook-list.h, but it's used by
builtin/bugreport.c.
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 0a3f292bf82..bae20eb1b40 100644
--- a/Makefile
+++ b/Makefile
@@ -2162,8 +2162,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
help.sp help.s help.o: command-list.h
hook.sp hook.s hook.o: hook-list.h
+builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
-builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 19/23] Makefile: correct the dependency graph of hook-list.h
2021-11-16 12:00 ` [PATCH v3 19/23] Makefile: correct the dependency graph of hook-list.h Ævar Arnfjörð Bjarmason
@ 2021-11-17 2:52 ` Mike Hommey
0 siblings, 0 replies; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 2:52 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Tue, Nov 16, 2021 at 01:00:19PM +0100, Ævar Arnfjörð Bjarmason wrote:
> Fix an issue in my cfe853e66be (hook-list.h: add a generated list of
> hooks, like config-list.h, 2021-09-26), the builtin/help.c was
> inadvertently made to depend on hook-list.h, but it's used by
> builtin/bugreport.c.
>
> Reported-by: Mike Hommey <mh@glandium.org>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 0a3f292bf82..bae20eb1b40 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2162,8 +2162,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
>
> help.sp help.s help.o: command-list.h
> hook.sp hook.s hook.o: hook-list.h
You can remove this line too.
Mike
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (18 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 19/23] Makefile: correct the dependency graph of hook-list.h Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Provide an alternate implementation of the recently added
"TRACK_template" which uses GNU make 4.2 features. If the 'file'
function is available we don't need to shell out at all to check if
our tracked files change.
We need to use the intermediate TRACK_template calling a
TRACK_template_eval within the generated rule so that we don't eagerly
fleshen these when "make" reads the file.
This doesn't make the runtime faster on my system, but helps to cut
down on the noise of things we shell out for
unconditionally. I.e. running "make" with "SHELL_PATH='sh -x'" (twice,
so we pick up the setting) shows than a no-op run went from emitting:
$ SHELL_PATH='sh -x' make -j8 >/dev/null 2>&1; SHELL_PATH='sh -x' make 2>&1 |wc -l
124
To:
$ SHELL_PATH='sh -x' make -j8 >/dev/null 2>&1; SHELL_PATH='sh -x' make 2>&1 |wc -l
95
That 124 to 95 number is a rough approximation of how many times we
shell out. "strace -f -c" similarly shows that we went from 8798 to
8466 syscalls. So this brings us further along in the goal of making
"make" do as little as possible when it's got nothing to re-build (see
[2]).
1. https://lore.kernel.org/git/874kdn1j6i.fsf@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
shared.mak | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/shared.mak b/shared.mak
index 363138a5577..4ee0bb7a13d 100644
--- a/shared.mak
+++ b/shared.mak
@@ -8,6 +8,22 @@
# info make --index-search=.DELETE_ON_ERROR
.DELETE_ON_ERROR:
+### GNU Make version detection
+# We don't care about "release" versions like the "90" in "3.99.90"
+MAKE_VERSION_MAJOR = $(word 1,$(subst ., ,$(MAKE_VERSION)))
+MAKE_VERSION_MINOR = $(word 2,$(subst ., ,$(MAKE_VERSION)))
+
+# The oldest supported version of GNU make is 3-something. So "not v3"
+# is a future-proof way to ask "is it modern?"
+ifneq ($(MAKE_VERSION_MAJOR),3)
+# $(file >[...]) and $(file >>[...]) is in 4.0...
+MAKE_HAVE_FILE_WRITE = Need version 4.0 or later (released in late 2013)
+# .. but we need 4.2 for $(file <[...])
+ifneq ($(filter-out 0 1,$(MAKE_VERSION_MINOR)),)
+MAKE_HAVE_FILE_READ = Need version 4.2 or later (released in mid-2016)
+endif
+endif
+
### Quoting helpers
## Quote a ' inside a '': FOO='$(call shq,$(BAR))'
@@ -99,6 +115,10 @@ endef
## Template for making a GIT-SOMETHING, which changes if a
## TRACK_SOMETHING variable changes.
+##
+## This is the slower version used on GNU make <4.2.
+ifndef MAKE_HAVE_FILE_READ
+
define TRACK_template
.PHONY: FORCE
$(1): FORCE
@@ -111,3 +131,41 @@ $(1): FORCE
echo "$$$$FLAGS" >$(1); \
fi
endef
+
+endif # !MAKE_HAVE_FILE_READ
+
+## A TRACK_template template compatible with the one above. Uses
+## features of GNU make >=4.2 to avoid shelling out for this "hot"
+## "FORCE" logic.
+##
+## Since version >=4.2 can do both "I" and "O" in I/O with using
+## $(file <)/$(file >) we read the GIT-SOMETHING file into a variable
+## with the former, and if it's different from our expected value
+## write it out with the latter.
+ifdef MAKE_HAVE_FILE_READ
+
+define TRACK_template_eval
+$(1)_WRITE =
+$(1)_EXISTS = $(wildcard $(1))
+ifeq ($$($(1)_EXISTS),)
+$(1)_WRITE = new
+else
+$(1)_CONTENT = $(file <$(1))
+ifeq ($$($(1)_CONTENT),$($(2)))
+$(1)_WRITE = same
+else
+$(1)_WRITE = changed
+endif
+endif
+ifneq ($$($(1)_WRITE),same)
+$$(info $$(wspfx) $(1) parameters ($$($(1)_WRITE)))
+$$(file >$(1),$($(2)))
+endif
+endef # TRACK_template_eval
+
+define TRACK_template
+$(1):
+ $$(eval $$(call TRACK_template_eval,$(1),$(2)))
+endef
+
+endif # MAKE_HAVE_FILE_READ
--
2.34.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (19 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 3:00 ` Mike Hommey
2021-11-16 12:00 ` [PATCH v3 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
23 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Ævar Arnfjörð Bjarmason
Override built-in rules of GNU make that use a wildcard target. This
speeds things up significantly as we don't need to stat() so many
files just in case we'd be able to retrieve their contents from RCS or
SCCS. See [1] for an old mailing list discussion about how to disable
these.
This gives us a noticeable speedup on a no-op run:
$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make -j8 all NO_TCLTK=Y' 'make NO_TCLTK=Y' --warmup 10 -M 10
Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
Time (mean ± σ): 182.2 ms ± 4.1 ms [User: 146.8 ms, System: 49.5 ms]
Range (min … max): 179.6 ms … 193.4 ms 10 runs
Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
Time (mean ± σ): 167.9 ms ± 1.3 ms [User: 127.8 ms, System: 55.6 ms]
Range (min … max): 166.1 ms … 170.4 ms 10 runs
Summary
'make NO_TCLTK=Y' in 'HEAD~0' ran
1.09 ± 0.03 times faster than 'make NO_TCLTK=Y' in 'HEAD~1'
Running the same except with 'strace -c -S calls make' as the
benchmark command shows (under --show-output) that we went from ~7716
syscalls to ~7519, mostly a reduction in [l]stat().
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 4ee0bb7a13d..4862df1607a 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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules
2021-11-16 12:00 ` [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
@ 2021-11-17 3:00 ` Mike Hommey
2021-11-17 9:47 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Mike Hommey @ 2021-11-17 3:00 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Tue, Nov 16, 2021 at 01:00:21PM +0100, Ævar Arnfjörð Bjarmason wrote:
> +### 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.%
`MAKEFLAGS += -r` should have the same effect (and more)
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules
2021-11-17 3:00 ` Mike Hommey
@ 2021-11-17 9:47 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 9:47 UTC (permalink / raw)
To: Mike Hommey
Cc: git, Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques,
Eric Wong, Jonathan Nieder
On Wed, Nov 17 2021, Mike Hommey wrote:
> On Tue, Nov 16, 2021 at 01:00:21PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> +### 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.%
>
> `MAKEFLAGS += -r` should have the same effect (and more)
Yeah, I did try that. It's a much bigger hammer though, and throws out
all implicit rules, some of which we may use (I'm not sure). I've
updated the commit message to note this.
In terms of performance it doesn't seem to matter over the above more
narrow change.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v3 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (20 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 21/23] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-16 12:00 ` [PATCH v3 23/23] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 -j8 all NO_TCLTK=Y' 'make NO_TCLTK=Y' --war
mup 10 -M 10
Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
Time (mean ± σ): 99.5 ms ± 0.5 ms [User: 83.4 ms, System: 20.7 ms]
Range (min … max): 98.8 ms … 100.2 ms 10 runs
Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
Time (mean ± σ): 69.4 ms ± 0.5 ms [User: 58.8 ms, System: 14.2 ms]
Range (min … max): 68.9 ms … 70.3 ms 10 runs
Summary
'make NO_TCLTK=Y' in 'HEAD~0' ran
1.43 ± 0.01 times faster than 'make 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 bae20eb1b40..1041efb6e5e 100644
--- a/Makefile
+++ b/Makefile
@@ -821,12 +821,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
@@ -2661,26 +2682,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
$(call mkdir_p_parent_template)
$(QUIET_GEN)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 $@+ && \
@@ -2860,9 +2861,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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v3 23/23] Makefile: move ".SUFFIXES" rule to shared.mak
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (21 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
@ 2021-11-16 12:00 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
23 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-16 12:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Æ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 ~1.3x
faster:
$ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -j8 -C Documentation man' 'make -C Documentation man' --warmup 10 -M 10
Benchmark 1: make -C Documentation man' in 'HEAD~1
Time (mean ± σ): 87.2 ms ± 1.0 ms [User: 79.3 ms, System: 10.8 ms]
Range (min … max): 86.3 ms … 89.7 ms 10 runs
Benchmark 2: make -C Documentation man' in 'HEAD~0
Time (mean ± σ): 64.5 ms ± 0.6 ms [User: 54.5 ms, System: 13.0 ms]
Range (min … max): 63.8 ms … 65.7 ms 10 runs
Summary
'make -C Documentation man' in 'HEAD~0' ran
1.35 ± 0.02 times faster than 'make -C Documentation 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 1041efb6e5e..d2cc6865a4e 100644
--- a/Makefile
+++ b/Makefile
@@ -2460,8 +2460,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 4862df1607a..a20193b0c8f 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.0.795.g1e9501ab396
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster
2021-11-16 12:00 ` [PATCH v3 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (22 preceding siblings ...)
2021-11-16 12:00 ` [PATCH v3 23/23] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:19 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 01/23] Makefile: don't invoke msgfmt with --statistics Ævar Arnfjörð Bjarmason
` (22 more replies)
23 siblings, 23 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:19 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
As noted in v2[1] this series fixes various dependency issues in the
Makfile. See [2] for before/after benchmark numbers.
This v4 addresses small issues Mike Hommey noted, and clarifies
various questions that came up with updated commit messages.
There's also a fix-for-the-fix here in removing the "hook.c"
dependency on hook-list.h in 19/23.
1. https://lore.kernel.org/git/cover-v2-00.18-00000000000-20211112T214150Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-v3-00.23-00000000000-20211116T114334Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (23):
Makefile: don't invoke msgfmt with --statistics
Makefile: don't set up "perl/build" rules under NO_PERL=Y
Makefile: use "=" not ":=" for po/* and perl/*
Makefile: clean perl/build/ even with NO_PERL=Y
Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
Makefile: guard Perl-only variable assignments
Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL"
Makefile: adjust Perl-related comments & whitespace
Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph
Makefile: create a GIT-PYTHON-DEFINES, like "PERL"
Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts
Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
Makefile: move $(comma), $(empty) and $(space) to shared.mak
Makefile: re-add and use the "shellquote" macros
Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
Makefile: add "$(QUIET)" boilerplate to shared.mak
Makefile: use $(wspfx) for $(QUIET...) in shared.mak
Makefiles: add and use wildcard "mkdir -p" template
Makefile: correct the dependency graph of hook-list.h
Makefile: use $(file) I/O instead of "FORCE" when possible
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
.gitignore | 2 +-
Documentation/Makefile | 71 +---------
Makefile | 314 +++++++++++++++--------------------------
config.mak.uname | 1 -
git-cvsserver.perl | 6 +-
git-send-email.perl | 7 +-
git-svn.perl | 2 +-
shared.mak | 187 ++++++++++++++++++++++++
t/Makefile | 34 ++---
templates/Makefile | 19 +--
10 files changed, 337 insertions(+), 306 deletions(-)
create mode 100644 shared.mak
Range-diff against v3:
1: 1621ca72c1d = 1: 1621ca72c1d Makefile: don't invoke msgfmt with --statistics
2: b7c36c9fea0 = 2: b7c36c9fea0 Makefile: don't set up "perl/build" rules under NO_PERL=Y
3: 510499d18ba ! 3: 29b000eb0f1 Makefile: use "=" not ":=" for po/* and perl/*
@@ Commit message
think I copied some POC code), and in 2017 I used the 2011 commit for
reference.
+ This doesn't make much if any of a practical difference, doing this is
+ cheap either way, but as simply-expanded variables in our Makefile
+ generally indicate special behavior (e.g. making a copy now, and
+ modifying the RHS later) let's change these to show that nothing odd
+ is going on here).
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Makefile ##
4: 37f3591bcca = 4: daead5ec293 Makefile: clean perl/build/ even with NO_PERL=Y
5: e38c90ad0b6 ! 5: 3c987590740 Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
@@ Commit message
file that'll be used as a dependency for other files, as in this case
for GIT-PERL-HEADER.
+ We have had a hard dependency on .DELETE_ON_ERROR since
+ 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
+ 2021-06-29), so this is a pure cleanup as a follow-up to that
+ commit. Support for the ".DELETE_ON_ERROR" target itself is much older
+ than any GNU make version we support, it was added to GNU make in
+ 1994.
+
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Makefile ##
6: 98e14c7eba9 = 6: b57f582ccd3 Makefile: guard Perl-only variable assignments
7: 047a42b01cf = 7: fcdee92f64c Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL"
8: 0c0a3de390e = 8: 1e25b532ca2 Makefile: adjust Perl-related comments & whitespace
9: 1ece3160915 = 9: 77d9855bfcf Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph
10: e9b61cd0ba5 = 10: 6004cdcd8d9 Makefile: create a GIT-PYTHON-DEFINES, like "PERL"
11: b020f8e3257 = 11: 17b30e96057 Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts
12: 19539ce7d2d = 12: 30ddf7da2c8 Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
13: 6c9291c2c9f = 13: f378a7dc35e Makefile: move $(comma), $(empty) and $(space) to shared.mak
14: e811a907b08 = 14: 13cbb851d32 Makefile: re-add and use the "shellquote" macros
15: fac30fe8b56 ! 15: 337953e4994 Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
@@ shared.mak: shelldquote = '"$(call shdq,$(call shq,$(1)))"'
+$(1): FORCE
+ @FLAGS='$$($(2))'; \
+ if ! test -f $(1) ; then \
-+ echo $(wspfx_sq) "$(1) PARAMETERS (new)" $@; \
++ echo $(wspfx_sq) "$(1) PARAMETERS (new)"; \
+ echo "$$$$FLAGS" >$(1); \
+ elif test x"$$$$FLAGS" != x"`cat $(1) 2>/dev/null`" ; then \
-+ echo $(wspfx_sq) "$(1) PARAMETERS (changed)" $@; \
++ echo $(wspfx_sq) "$(1) PARAMETERS (changed)"; \
+ echo "$$$$FLAGS" >$(1); \
+ fi
+endef
16: a3e3acea82d = 16: 5bb597c1993 Makefile: add "$(QUIET)" boilerplate to shared.mak
17: 22264f431c8 = 17: 3c4d0589667 Makefile: use $(wspfx) for $(QUIET...) in shared.mak
18: d61e2b44f68 = 18: be5882b2c99 Makefiles: add and use wildcard "mkdir -p" template
19: 234b4eb613c ! 19: 2710f8af6cd Makefile: correct the dependency graph of hook-list.h
@@ Commit message
inadvertently made to depend on hook-list.h, but it's used by
builtin/bugreport.c.
+ The hook.c also does not depend on hook-list.h. It did in an earlier
+ version of the greater series cfe853e66be was extracted from, but not
+ anymore. We might end up needing that line again, but let's remove it
+ for now.
+
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Makefile ##
@@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
+ $(filter %.o,$^) $(LIBS)
help.sp help.s help.o: command-list.h
- hook.sp hook.s hook.o: hook-list.h
+-hook.sp hook.s hook.o: hook-list.h
+builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
-builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX
20: 567ad5c3ebc = 20: 59f22a0269a Makefile: use $(file) I/O instead of "FORCE" when possible
21: cb3ae5ce00b ! 21: dd569a59c74 Makefile: disable GNU make built-in wildcard rules
@@ Commit message
benchmark command shows (under --show-output) that we went from ~7716
syscalls to ~7519, mostly a reduction in [l]stat().
+ We could also invoke make with "-r" by setting "MAKEFLAGS = -r" early,
+ adding a "-r" variant to the above benchmark shows that it may be 1.01
+ or so faster (but in my tests, always with a much bigger error
+ bar). 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>
22: 88cfc946b37 = 22: 4168a7e3b30 Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
23: 276e226f0a8 = 23: 48a3927d972 Makefile: move ".SUFFIXES" rule to shared.mak
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v4 01/23] Makefile: don't invoke msgfmt with --statistics
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y Ævar Arnfjörð Bjarmason
` (21 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Remove the --statistics flag that I added in 5e9637c6297 (i18n: add
infrastructure for translating Git with gettext, 2011-11-18). Our
Makefile output is good about reducing verbosity by default, except in
this case:
$ rm -rf po/build/locale/e*; time make -j $(nproc) all
SUBDIR templates
MKDIR -p po/build/locale/el/LC_MESSAGES
MSGFMT po/build/locale/el/LC_MESSAGES/git.mo
MKDIR -p po/build/locale/es/LC_MESSAGES
MSGFMT po/build/locale/es/LC_MESSAGES/git.mo
1038 translated messages, 3325 untranslated messages.
5230 translated messages.
I didn't have any good reason for using --statistics at the time other
than ad-hoc eyeballing of the output. We don't need to spew out
exactly how many messages we've got translated every time. Now we'll
instead emit:
$ rm -rf po/build/locale/e*; time make -j $(nproc) all
SUBDIR templates
MKDIR -p po/build/locale/el/LC_MESSAGES
MSGFMT po/build/locale/el/LC_MESSAGES/git.mo
MKDIR -p po/build/locale/es/LC_MESSAGES
MSGFMT po/build/locale/es/LC_MESSAGES/git.mo
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d56c0e4aadc..11da97de233 100644
--- a/Makefile
+++ b/Makefile
@@ -1870,7 +1870,7 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
endif
ifndef NO_MSGFMT_EXTENDED_OPTIONS
- MSGFMT += --check --statistics
+ MSGFMT += --check
endif
ifdef HAVE_CLOCK_GETTIME
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 01/23] Makefile: don't invoke msgfmt with --statistics Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 03/23] Makefile: use "=" not ":=" for po/* and perl/* Ævar Arnfjörð Bjarmason
` (20 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Do not define LIB_{PERL,CPAN}{,_GEN} if NO_PERL is defined. This
changes no functionality, but makes it clear which of these rules are
needed under NO_PERL=Y. See 20d2a30f8ff (Makefile: replace
perl/Makefile.PL with simple make rules, 2017-12-10) for the initial
implementation.
We do for better or worse rely on "install-doc" calling
"install-man-perl" regardless of whether NO_PERL=Y is defined or not,
i.e. we'll always end up with that manual page, even if we don't have
any of the Perl code installed. Let's add a comment about that
adjacent to the rules that build perl/build.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 11da97de233..06405825a24 100644
--- a/Makefile
+++ b/Makefile
@@ -2684,19 +2684,12 @@ endif
po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
+ifndef NO_PERL
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))
LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
-ifndef NO_PERL
-all:: $(LIB_PERL_GEN)
-ifndef NO_PERL_CPAN_FALLBACKS
-all:: $(LIB_CPAN_GEN)
-endif
-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 $@) && \
sed -e 's|@@LOCALEDIR@@|$(perl_localedir_SQ)|g' \
@@ -2704,6 +2697,14 @@ perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
-e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
< $< > $@
+all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
+all:: $(LIB_CPAN_GEN)
+endif
+NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
+endif
+
+# install-man depends on Git.3pm even with NO_PERL=Y
perl/build/man/man3/Git.3pm: perl/Git.pm
$(QUIET_GEN)mkdir -p $(dir $@) && \
pod2man $< $@
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 03/23] Makefile: use "=" not ":=" for po/* and perl/*
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 01/23] Makefile: don't invoke msgfmt with --statistics Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 02/23] Makefile: don't set up "perl/build" rules under NO_PERL=Y Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 04/23] Makefile: clean perl/build/ even with NO_PERL=Y Ævar Arnfjörð Bjarmason
` (19 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Change these variable definitions from being simply-expanded to be
recursively expanded instead. I.e. they'll be lazily expanded when
used.
I added these in 5e9637c6297 (i18n: add infrastructure for translating
Git with gettext, 2011-11-18) and 20d2a30f8ff (Makefile: replace
perl/Makefile.PL with simple make rules, 2017-12-10), the reason for
using ":=" over "=" was that I didn't know the difference in 2011 (I
think I copied some POC code), and in 2017 I used the 2011 commit for
reference.
This doesn't make much if any of a practical difference, doing this is
cheap either way, but as simply-expanded variables in our Makefile
generally indicate special behavior (e.g. making a copy now, and
modifying the RHS later) let's change these to show that nothing odd
is going on here).
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 06405825a24..6b77702e102 100644
--- a/Makefile
+++ b/Makefile
@@ -2672,11 +2672,11 @@ po/git.pot: $(GENERATED_H) FORCE
pot: po/git.pot
ifdef NO_GETTEXT
-POFILES :=
-MOFILES :=
+POFILES =
+MOFILES =
else
-POFILES := $(wildcard po/*.po)
-MOFILES := $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
+POFILES = $(wildcard po/*.po)
+MOFILES = $(patsubst po/%.po,po/build/locale/%/LC_MESSAGES/git.mo,$(POFILES))
all:: $(MOFILES)
endif
@@ -2685,10 +2685,10 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
ifndef NO_PERL
-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))
-LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
-LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
+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))
+LIB_CPAN = $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN = $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
$(QUIET_GEN)mkdir -p $(dir $@) && \
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 04/23] Makefile: clean perl/build/ even with NO_PERL=Y
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 03/23] Makefile: use "=" not ":=" for po/* and perl/* Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR Ævar Arnfjörð Bjarmason
` (18 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Fix a regression in 499c29394ce (Makefile: allow building without
perl, 2009-04-03) where we'd stop cleaning the perl/* directory
because NO_PERL was defined, thus leaving behind litter if the flag at
"clean" time didn't match that of build time.
In 499c29394ce this was done to avoid relying on the perl/Makefile.PL,
but since my 20d2a30f8ff (Makefile: replace perl/Makefile.PL with
simple make rules, 2017-12-10) we can clean things in that directory
unconditionally.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 6b77702e102..a71fba15e30 100644
--- a/Makefile
+++ b/Makefile
@@ -3234,6 +3234,7 @@ clean: profile-clean coverage-clean cocciclean
$(RM) $(HCC)
$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json
$(RM) -r po/build/
+ $(RM) -r perl/build/
$(RM) *.pyc *.pyo */*.pyc */*.pyo $(GENERATED_H) $(ETAGS_TARGET) tags cscope*
$(RM) -r .dist-tmp-dir .doc-tmp-dir
$(RM) $(GIT_TARNAME).tar.gz
@@ -3242,7 +3243,6 @@ clean: profile-clean coverage-clean cocciclean
$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
ifndef NO_PERL
$(MAKE) -C gitweb clean
- $(RM) -r perl/build/
endif
$(MAKE) -C templates/ clean
$(MAKE) -C t/ clean
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 04/23] Makefile: clean perl/build/ even with NO_PERL=Y Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 06/23] Makefile: guard Perl-only variable assignments Ævar Arnfjörð Bjarmason
` (17 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Since 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
2021-06-29) we don't need to guard the clobbering of $@ with this sort
of "mv $@+ $@" pattern in these cases where we're merely generating a
file that'll be used as a dependency for other files, as in this case
for GIT-PERL-HEADER.
We have had a hard dependency on .DELETE_ON_ERROR since
7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag,
2021-06-29), so this is a pure cleanup as a follow-up to that
commit. Support for the ".DELETE_ON_ERROR" target itself is much older
than any GNU make version we support, it was added to GNU make in
1994.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a71fba15e30..284725099c9 100644
--- a/Makefile
+++ b/Makefile
@@ -2349,8 +2349,7 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
-e 's=@@PERLLIBDIR_REL@@=$(perllibdir_relative_SQ)=g' \
-e 's=@@GITEXECDIR_REL@@=$(gitexecdir_relative_SQ)=g' \
-e 's=@@LOCALEDIR_REL@@=$(localedir_relative_SQ)=g' \
- $< >$@+ && \
- mv $@+ $@
+ $< >$@
.PHONY: perllibdir
perllibdir:
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 06/23] Makefile: guard Perl-only variable assignments
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 05/23] Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL" Ævar Arnfjörð Bjarmason
` (16 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Move the "ifndef NO_PERL" a few lines earlier to encompass the
"perl_localedir_SQ" variable. We'll only use it under !NO_PERL.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 284725099c9..0cb10f00ebb 100644
--- a/Makefile
+++ b/Makefile
@@ -2291,11 +2291,11 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
# This makes sure we depend on the NO_PERL setting itself.
$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
+ifndef NO_PERL
# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
# since the locale directory is injected.
perl_localedir_SQ = $(localedir_SQ)
-ifndef NO_PERL
PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
PERL_DEFINES =
PERL_DEFINES += $(PERL_PATH_SQ)
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL"
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 06/23] Makefile: guard Perl-only variable assignments Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 08/23] Makefile: adjust Perl-related comments & whitespace Ævar Arnfjörð Bjarmason
` (15 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Change the NO_PERL variable assignments so that they declare the much
smaller fallback condition first.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 0cb10f00ebb..288f4834db8 100644
--- a/Makefile
+++ b/Makefile
@@ -2291,7 +2291,15 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
# This makes sure we depend on the NO_PERL setting itself.
$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
-ifndef NO_PERL
+ifdef NO_PERL
+$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
+ $(QUIET_GEN) \
+ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+ -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
+ unimplemented.sh >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
+else # NO_PERL
# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
# since the locale directory is injected.
perl_localedir_SQ = $(localedir_SQ)
@@ -2363,14 +2371,6 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
chmod +x $@+ && \
mv $@+ $@
-else # NO_PERL
-$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
- $(QUIET_GEN) \
- sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
- -e 's|@@REASON@@|NO_PERL=$(NO_PERL)|g' \
- unimplemented.sh >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
endif # NO_PERL
# This makes sure we depend on the NO_PYTHON setting itself.
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 08/23] Makefile: adjust Perl-related comments & whitespace
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 07/23] Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL" Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph Ævar Arnfjörð Bjarmason
` (14 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Folllow-up my 4070c9e09fc (Makefile: don't re-define PERL_DEFINES,
2021-05-05) and move the rest of the assignments to PERL_DEFINES to
one place.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 288f4834db8..ab78f8dd42e 100644
--- a/Makefile
+++ b/Makefile
@@ -2312,21 +2312,19 @@ PERL_DEFINES += $(perllibdir_SQ)
PERL_DEFINES += $(RUNTIME_PREFIX)
PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
PERL_DEFINES += $(NO_GETTEXT)
+PERL_DEFINES += $(gitexecdir)
+PERL_DEFINES += $(perllibdir)
+PERL_DEFINES += $(localedir)
+PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
-# Support Perl runtime prefix. In this mode, a different header is installed
-# into Perl scripts.
ifdef RUNTIME_PREFIX
-
PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
-# Don't export a fixed $(localedir) path; it will be resolved by the Perl header
-# at runtime.
+# The RUNTIME_PREFIX header defines $Git::I18N::TEXTDOMAINDIR, so
+# $(perl_localedir_SQ) won't be needed
perl_localedir_SQ =
-
endif
-PERL_DEFINES += $(gitexecdir) $(perllibdir) $(localedir)
-
$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
$(QUIET_GEN) \
sed -e '1{' \
@@ -2339,7 +2337,6 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
chmod +x $@+ && \
mv $@+ $@
-PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
GIT-PERL-DEFINES: FORCE
@FLAGS='$(PERL_DEFINES)'; \
if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (7 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 08/23] Makefile: adjust Perl-related comments & whitespace Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL" Ævar Arnfjörð Bjarmason
` (13 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Fix several small issues with the dependency graph of the generated
"GIT-PERL-DEFINES" and "GIT-PERL-HEADER" files:
1. Don't have "GIT-PERL-HEADER" depend on the "Makefile". That was a
lazy way to over-declare the dependencies added in
f6a0ad4be71 (Makefile: generate Perl header from template file,
2018-04-10). Let's correct our dependency graph instead.
2. Don't have $(SCRIPT_PERL_GEN) (such as "git-send-email") depend on
GIT-BUILD-OPTIONS. Let's instead use GIT-PERL-DEFINES.
The reason for depending on "GIT-BUILD-OPTIONS" was to trigger a
re-build if NO_PERL=Y was defined. We can instead add that variable
to "PERL_DEFINES", and have "GIT-PERL-DEFINES" created and updated
if "NO_PERL=Y" is defined.
3. Due to #2 we'll need to have GIT-PERL-DEFINES be generated even
under NO_PERL, since that variable will be used by the
"unimplemented.sh" script.
4. Don't depend on $(gitexecdir), $(perllibdir) and $(localedir),
instead depend on the $(*_relative_SQ) versions of those.
The latter is what we'll actually use, while it's unlikely to
matter in practice (we'd just skip re-building these under
RUNTIME_PREFIX if the relative path was the same) it makes the code
easier to read.
That's because this brings us to a 1=1 mapping of these variables
and what's subsequently used in the "GIT-PERL-DEFINES",
"GIT-PERL-HEADER" and "perl/build/lib/%.pm" rules below.
5. We don't need the substitution of " " for ":" added in
07d90eadb50 (Makefile: add Perl runtime prefix support,
2018-04-10), let's drop it. This doesn't matter for the correctness
of these files, because unlike GIT-BUILD-OPTIONS nothing is
consuming them except the Makefile itself.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 42 +++++++++++++++++++++++++-----------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index ab78f8dd42e..361abff2402 100644
--- a/Makefile
+++ b/Makefile
@@ -2288,10 +2288,14 @@ git.res: git.rc GIT-VERSION-FILE GIT-PREFIX
$(shell echo $(GIT_VERSION) 0 0 0 0 | tr '.a-zA-Z-' ' '))) \
-DGIT_VERSION="\\\"$(GIT_VERSION)\\\"" -i $< -o $@
-# This makes sure we depend on the NO_PERL setting itself.
-$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
+# Under NO_PERL=Y we'll still make GIT-PERL-DEFINES. We need to depend
+# on NO_PERL=Y itself for creating "unimplemented.sh" scripts.
+PERL_DEFINES =
+$(SCRIPT_PERL_GEN): GIT-PERL-DEFINES
ifdef NO_PERL
+PERL_DEFINES += $(NO_PERL)
+
$(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
$(QUIET_GEN) \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2300,22 +2304,26 @@ $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh
chmod +x $@+ && \
mv $@+ $@
else # NO_PERL
-# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX
-# since the locale directory is injected.
+# The localedir is only used in Perl modules if !NO_GETTEXT
+ifndef NO_GETTEXT
perl_localedir_SQ = $(localedir_SQ)
+endif
PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
-PERL_DEFINES =
+
PERL_DEFINES += $(PERL_PATH_SQ)
PERL_DEFINES += $(PERLLIB_EXTRA_SQ)
PERL_DEFINES += $(perllibdir_SQ)
PERL_DEFINES += $(RUNTIME_PREFIX)
PERL_DEFINES += $(NO_PERL_CPAN_FALLBACKS)
PERL_DEFINES += $(NO_GETTEXT)
-PERL_DEFINES += $(gitexecdir)
-PERL_DEFINES += $(perllibdir)
-PERL_DEFINES += $(localedir)
-PERL_DEFINES := $(subst $(space),:,$(PERL_DEFINES))
+ifdef RUNTIME_PREFIX
+PERL_DEFINES += $(gitexecdir_relative_SQ)
+PERL_DEFINES += $(perllibdir_relative_SQ)
+PERL_DEFINES += $(localedir_relative_SQ)
+else
+PERL_DEFINES += $(perllocaledir_SQ)
+endif
ifdef RUNTIME_PREFIX
PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
@@ -2337,14 +2345,7 @@ $(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
chmod +x $@+ && \
mv $@+ $@
-GIT-PERL-DEFINES: FORCE
- @FLAGS='$(PERL_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new perl-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
-
-GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES
$(QUIET_GEN) \
INSTLIBDIR='$(perllibdir_SQ)' && \
INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
@@ -2370,6 +2371,13 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
mv $@+ $@
endif # NO_PERL
+GIT-PERL-DEFINES: FORCE
+ @FLAGS='$(PERL_DEFINES)'; \
+ if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo >&2 " * new perl-specific parameters"; \
+ echo "$$FLAGS" >$@; \
+ fi
+
# This makes sure we depend on the NO_PYTHON setting itself.
$(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL"
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (8 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 09/23] Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts Ævar Arnfjörð Bjarmason
` (12 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Create a new "GIT-PYTHON-DEFINES" file, and untangle the dependency
issues of the Python by copying over the patterns established for
building the adjacent Perl code in preceding commits.
As with Perl, there's no real reason to depend on GIT-BUILD-OPTIONS or
GIT-CFLAGS when building the Python code, nor did we need
GIT-PREFIX. Let's instead add those variables we care about to a
"GIT-PYTHON-DEFINES" and depend on that.
This changes code originally added in ca3bcabf118 (auto-detect changed
prefix and/or changed build flags, 2006-06-15), and adjusted in
96a4647fca5 (Makefile: detect when PYTHON_PATH changes, 2012-12-18).
The relevant code for the "Perl" targets was then added in
07981dce81e (Makefile: rebuild perl scripts when perl paths change,
2013-11-18), and has been adjusted in preceding commits.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
.gitignore | 2 +-
Makefile | 49 +++++++++++++++++++++++++------------------------
2 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/.gitignore b/.gitignore
index 054249b20a8..845e5d0c355 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,7 +8,7 @@
/GIT-PREFIX
/GIT-PERL-DEFINES
/GIT-PERL-HEADER
-/GIT-PYTHON-VARS
+/GIT-PYTHON-DEFINES
/GIT-SCRIPT-DEFINES
/GIT-USER-AGENT
/GIT-VERSION-FILE
diff --git a/Makefile b/Makefile
index 361abff2402..c698c5b058a 100644
--- a/Makefile
+++ b/Makefile
@@ -2378,18 +2378,15 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
-# This makes sure we depend on the NO_PYTHON setting itself.
-$(SCRIPT_PYTHON_GEN): GIT-BUILD-OPTIONS
+# As with NO_PERL=Y we'll still make GIT-PYTHON-DEFINES if "NO_PYTHON"
+# is defined, for creating the "unimplemented.sh" scripts.
+PYTHON_DEFINES =
+$(SCRIPT_PYTHON_GEN): GIT-PYTHON-DEFINES
+
+ifdef NO_PYTHON
+PYTHON_DEFINES += $(SHELL_PATH_SQ)
+PYTHON_DEFINES += $(NO_PYTHON)
-ifndef NO_PYTHON
-$(SCRIPT_PYTHON_GEN): GIT-CFLAGS GIT-PREFIX GIT-PYTHON-VARS
-$(SCRIPT_PYTHON_GEN): % : %.py
- $(QUIET_GEN) \
- sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
- $< >$@+ && \
- chmod +x $@+ && \
- mv $@+ $@
-else # NO_PYTHON
$(SCRIPT_PYTHON_GEN): % : unimplemented.sh
$(QUIET_GEN) \
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2397,8 +2394,24 @@ $(SCRIPT_PYTHON_GEN): % : unimplemented.sh
unimplemented.sh >$@+ && \
chmod +x $@+ && \
mv $@+ $@
+else # NO_PYTHON
+PYTHON_DEFINES += $(PYTHON_PATH_SQ)
+
+$(SCRIPT_PYTHON_GEN): % : %.py GIT-PYTHON-DEFINES
+ $(QUIET_GEN) \
+ sed -e '1s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
+ $< >$@+ && \
+ chmod +x $@+ && \
+ mv $@+ $@
endif # NO_PYTHON
+GIT-PYTHON-DEFINES: FORCE
+ @FLAGS='$(PYTHON_DEFINES)'; \
+ if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
+ echo >&2 " * new python-specific parameters"; \
+ echo "$$FLAGS" >$@; \
+ fi
+
CONFIGURE_RECIPE = sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
configure.ac >configure.ac+ && \
autoconf -o configure configure.ac+ && \
@@ -2848,18 +2861,6 @@ else
endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
-### Detect Python interpreter path changes
-ifndef NO_PYTHON
-TRACK_PYTHON = $(subst ','\'',-DPYTHON_PATH='$(PYTHON_PATH_SQ)')
-
-GIT-PYTHON-VARS: FORCE
- @VARS='$(TRACK_PYTHON)'; \
- if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new Python interpreter location"; \
- echo "$$VARS" >$@; \
- fi
-endif
-
test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))
all:: $(TEST_PROGRAMS) $(test_bindir_programs)
@@ -3256,7 +3257,7 @@ ifndef NO_TCLTK
endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX
- $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
+ $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-DEFINES
ifdef MSVC
$(RM) $(patsubst %.o,%.o.pdb,$(OBJECTS))
$(RM) $(patsubst %.exe,%.pdb,$(OTHER_PROGRAMS))
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (9 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 10/23] Makefile: create a GIT-PYTHON-DEFINES, like "PERL" Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
` (11 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Change the hardcoding of @@GIT_VERSION@@ in generated *.perl scripts
to instead shell out to "git version". This means that we can stop
re-building during development every time the HEAD changes.
These codepaths are not "hot", so shelling out to get the version
shouldn't matter to users, in the one case where it potentially would
in send-email (the loop for each E-Mail we send) we now cache the
value, so we'll only retrieve it once.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 2 +-
git-cvsserver.perl | 6 +++---
git-send-email.perl | 7 ++-----
git-svn.perl | 2 +-
4 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index c698c5b058a..8205614c6ec 100644
--- a/Makefile
+++ b/Makefile
@@ -2333,7 +2333,7 @@ PERL_HEADER_TEMPLATE = perl/header_templates/runtime_prefix.template.pl
perl_localedir_SQ =
endif
-$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER GIT-VERSION-FILE
+$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER
$(QUIET_GEN) \
sed -e '1{' \
-e ' s|#!.*perl|#!$(PERL_PATH_SQ)|' \
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 64319bed43f..76f0e8bbbef 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -26,8 +26,6 @@
use File::Basename;
use Getopt::Long qw(:config require_order no_ignore_case);
-my $VERSION = '@@GIT_VERSION@@';
-
my $log = GITCVS::log->new();
my $cfg;
@@ -126,7 +124,9 @@
or die $usage;
if ($state->{version}) {
- print "git-cvsserver version $VERSION\n";
+ my $version = qx[git version];
+ $version =~ s/^(git)\b/$1-cvsserver/;
+ print $version;
exit;
}
if ($state->{help}) {
diff --git a/git-send-email.perl b/git-send-email.perl
index 5262d88ee32..041cd2fb96d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1468,6 +1468,7 @@ sub file_name_is_absolute {
#
# If an error occurs sending the email, this just dies.
+my $gitversion;
sub send_message {
my @recipients = unique_email_list(@to);
@cc = (grep { my $cc = extract_valid_address_or_die($_);
@@ -1478,11 +1479,6 @@ sub send_message {
@recipients = unique_email_list(@recipients,@cc,@initial_bcc);
@recipients = (map { extract_valid_address_or_die($_) } @recipients);
my $date = format_2822_time($time++);
- my $gitversion = '@@GIT_VERSION@@';
- if ($gitversion =~ m/..GIT_VERSION../) {
- $gitversion = Git::version();
- }
-
my $cc = join(",\n\t", unique_email_list(@cc));
my $ccline = "";
if ($cc ne '') {
@@ -1497,6 +1493,7 @@ sub send_message {
Message-Id: $message_id
";
if ($use_xmailer) {
+ $gitversion ||= Git::version();
$header .= "X-Mailer: git-send-email $gitversion\n";
}
if ($in_reply_to) {
diff --git a/git-svn.perl b/git-svn.perl
index be987e316f9..727431412be 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -9,7 +9,6 @@
$_revision $_repository
$_q $_authors $_authors_prog %users/;
$AUTHOR = 'Eric Wong <normalperson@yhbt.net>';
-$VERSION = '@@GIT_VERSION@@';
use Carp qw/croak/;
use File::Basename qw/dirname basename/;
@@ -47,6 +46,7 @@
command_close_bidi_pipe
get_record
);
+$VERSION = Git::version();
BEGIN {
Memoize::memoize 'Git::config';
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (10 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 11/23] Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak Ævar Arnfjörð Bjarmason
` (10 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 +++++++++
3 files changed, 15 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 8205614c6ec..5ae7d012cfb 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::
@@ -2158,16 +2161,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:
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (11 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 12/23] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros Ævar Arnfjörð Bjarmason
` (9 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 5ae7d012cfb..7130e32a251 100644
--- a/Makefile
+++ b/Makefile
@@ -1252,10 +1252,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 0170bb397ae..2d597ef7603 100644
--- a/shared.mak
+++ b/shared.mak
@@ -7,3 +7,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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (12 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 13/23] Makefile: move $(comma), $(empty) and $(space) to shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-12-13 23:09 ` Junio C Hamano
2021-11-17 10:20 ` [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
` (8 subsequent siblings)
22 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Re-add and use, and expand on "shellquote" macros added in
4769948afe7 (Deal with $(bindir) and friends with whitespaces.,
2005-10-10).
We avoided using them due to the "$(call)" feature of GNU make being
relatively new at the time, but it isn't anymore. We hard depend on
GNU make versions that have it.
The use of "$(call)" was removed in 39c015c556f (Fixes for ancient
versions of GNU make, 2006-02-18) and 7ffe7098dca (Fix installation of
templates on ancient systems., 2006-07-29) due to those
incompatibilities with older GNU make versions, and we've used the
more verbose *_SQ pattern ever since.
The "$(call)" feature was introduced in GNU make version 3.78,
released on the 22nd of September, 1999. That release also introduced
"$(error)" and "$(warning)", which we've been making use of since
f2fabbf76e4 (Teach Makefile to check header dependencies, 2010-01-26).
This extends upon the macros added in 4769948afe7: We now have macros
for quoting a ' inside '', and a ' with no surrounding '' as before.
Additionally provide and use a "shelldquote" macro along with
"shellquote" for the common case of wanting to quote a C string we
pass to the compiler with a -D flag.
This doesn't get rid of all of our shell quoting. We've still got some
in the main Makefile, let's leave most of it to avoid in-flight
conflicts. I've fully converted "templates/Makefile" and "t/Makefile"
though.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 15 +++++----------
shared.mak | 14 ++++++++++++++
t/Makefile | 34 +++++++++++++++-------------------
templates/Makefile | 14 +++++---------
4 files changed, 39 insertions(+), 38 deletions(-)
diff --git a/Makefile b/Makefile
index 7130e32a251..b458c24d95e 100644
--- a/Makefile
+++ b/Makefile
@@ -1996,11 +1996,7 @@ ifneq ("$(PROFILE)","")
endif
endif
-# Shell quote (do not use $(call) to accommodate ancient setups);
-
-ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
-ETC_GITATTRIBUTES_SQ = $(subst ','\'',$(ETC_GITATTRIBUTES))
-
+# Shell quote, should be changed to use $(call shellquote,...)
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
NO_GETTEXT_SQ = $(subst ','\'',$(NO_GETTEXT))
bindir_SQ = $(subst ','\'',$(bindir))
@@ -2535,11 +2531,11 @@ builtin/init-db.sp builtin/init-db.s builtin/init-db.o: EXTRA_CPPFLAGS = \
config.sp config.s config.o: GIT-PREFIX
config.sp config.s config.o: EXTRA_CPPFLAGS = \
- -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+ -DETC_GITCONFIG=$(call shelldquote,$(ETC_GITCONFIG))
attr.sp attr.s attr.o: GIT-PREFIX
attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \
- -DETC_GITATTRIBUTES='"$(ETC_GITATTRIBUTES_SQ)"'
+ -DETC_GITATTRIBUTES=$(call shelldquote,$(ETC_GITATTRIBUTES))
gettext.sp gettext.s gettext.o: GIT-PREFIX
gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
@@ -2700,14 +2696,13 @@ perl/build/lib/%.pm: perl/%.pm GIT-PERL-DEFINES
$(QUIET_GEN)mkdir -p $(dir $@) && \
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' \
+ -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(call shq,$(NO_PERL_CPAN_FALLBACKS))|g' \
< $< > $@
all:: $(LIB_PERL_GEN)
ifndef NO_PERL_CPAN_FALLBACKS
all:: $(LIB_CPAN_GEN)
endif
-NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
endif
# install-man depends on Git.3pm even with NO_PERL=Y
@@ -3021,7 +3016,7 @@ else
$(INSTALL) $(vcpkg_dbg_bin)/*.pdb '$(DESTDIR_SQ)$(bindir_SQ)'
endif
endif
- $(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
+ $(MAKE) -C templates DESTDIR=$(call shellquote,$(DESTDIR)) install
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'
ifndef NO_GETTEXT
diff --git a/shared.mak b/shared.mak
index 2d597ef7603..ef03c2bc094 100644
--- a/shared.mak
+++ b/shared.mak
@@ -8,6 +8,20 @@
# info make --index-search=.DELETE_ON_ERROR
.DELETE_ON_ERROR:
+### Quoting helpers
+
+## Quote a ' inside a '': FOO='$(call shq,$(BAR))'
+shq = $(subst ','\'',$(1))
+
+## Quote a ' and provide a '': FOO=$(call shq,$(BAR))
+shellquote = '$(call shq,$(1))'
+
+## Quote a " inside a ""
+shdq = $(subst ",\",$(1))
+
+## Quote ' for the shell, and embedded " for C: -DFOO=$(call shelldquote,$(BAR))
+shelldquote = '"$(call shdq,$(call shq,$(1)))"'
+
### Global variables
## comma, empty, space: handy variables as these tokens are either
diff --git a/t/Makefile b/t/Makefile
index 882d26eee30..4168b5c6ce6 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
@@ -24,13 +27,6 @@ TEST_RESULTS_DIRECTORY = test-results
CHAINLINTTMP = chainlinttmp
endif
-# Shell quote;
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
-CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
-
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
@@ -44,38 +40,38 @@ test: pre-clean check-chainlint $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
failed:
- @failed=$$(cd '$(TEST_RESULTS_DIRECTORY_SQ)' && \
+ @failed=$$(cd $(call shellquote,$(TEST_RESULTS_DIRECTORY)) && \
grep -l '^failed [1-9]' *.counts | \
sed -n 's/\.counts$$/.sh/p') && \
test -z "$$failed" || $(MAKE) $$failed
prove: pre-clean check-chainlint $(TEST_LINT)
- @echo "*** prove ***"; $(PROVE) --exec '$(TEST_SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+ @echo "*** prove ***"; $(PROVE) --exec $(call shellquote,$(SHELL_PATH)) $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
$(T):
- @echo "*** $@ ***"; '$(TEST_SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+ @echo "*** $@ ***"; $(call shellquote,$(SHELL_PATH)) $@ $(GIT_TEST_OPTS)
pre-clean:
- $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
+ $(RM) -r $(call shellquote,$(TEST_RESULTS_DIRECTORY))
clean-except-prove-cache: clean-chainlint
- $(RM) -r 'trash directory'.* '$(TEST_RESULTS_DIRECTORY_SQ)'
+ $(RM) -r 'trash directory'.* $(call shellquote,$(TEST_RESULTS_DIRECTORY))
$(RM) -r valgrind/bin
clean: clean-except-prove-cache
$(RM) .prove
clean-chainlint:
- $(RM) -r '$(CHAINLINTTMP_SQ)'
+ $(RM) -r $(call shellquote,$(CHAINLINTTMP))
check-chainlint:
- @mkdir -p '$(CHAINLINTTMP_SQ)' && \
+ @mkdir -p $(call shellquote,$(CHAINLINTTMP)) && \
err=0 && \
for i in $(CHAINLINTTESTS); do \
$(CHAINLINT) <chainlint/$$i.test | \
- sed -e '/^# LINT: /d' >'$(CHAINLINTTMP_SQ)'/$$i.actual && \
- diff -u chainlint/$$i.expect '$(CHAINLINTTMP_SQ)'/$$i.actual || err=1; \
+ sed -e '/^# LINT: /d' >$(call shellquote,$(CHAINLINTTMP))/$$i.actual && \
+ diff -u chainlint/$$i.expect $(call shellquote,$(CHAINLINTTMP))/$$i.actual || err=1; \
done && exit $$err
test-lint: test-lint-duplicates test-lint-executable test-lint-shell-syntax \
@@ -92,7 +88,7 @@ test-lint-executable:
echo >&2 "non-executable tests:" $$bad; exit 1; }
test-lint-shell-syntax:
- @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
+ @$(call shellquote,$(PERL_PATH)) check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
test-lint-filenames:
@# We do *not* pass a glob to ls-files but use grep instead, to catch
@@ -107,9 +103,9 @@ aggregate-results-and-cleanup: $(T)
$(MAKE) clean
aggregate-results:
- for f in '$(TEST_RESULTS_DIRECTORY_SQ)'/t*-*.counts; do \
+ for f in $(call shellquote,$(TEST_RESULTS_DIRECTORY))/t*-*.counts; do \
echo "$$f"; \
- done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh
+ done | $(call shellquote,$(SHELL_PATH)) ./aggregate-results.sh
gitweb-test:
$(MAKE) $(TGITWEB)
diff --git a/templates/Makefile b/templates/Makefile
index d22a71a3999..c9251a96622 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
@@ -18,13 +21,6 @@ ifndef PERL_PATH
PERL_PATH = perl
endif
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
-
-# Shell quote (do not use $(call) to accommodate ancient setups);
-DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
-template_instdir_SQ = $(subst ','\'',$(template_instdir))
-
all: boilerplates.made custom
# Put templates that can be copied straight from the source
@@ -61,6 +57,6 @@ clean:
$(RM) -r blt boilerplates.made
install: all
- $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(template_instdir_SQ)'
+ $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(template_instdir))
(cd blt && $(TAR) cf - .) | \
- (cd '$(DESTDIR_SQ)$(template_instdir_SQ)' && umask 022 && $(TAR) xof -)
+ (cd $(call shellquote,$(DESTDIR)$(template_instdir)) && umask 022 && $(TAR) xof -)
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros
2021-11-17 10:20 ` [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros Ævar Arnfjörð Bjarmason
@ 2021-12-13 23:09 ` Junio C Hamano
2021-12-14 0:31 ` Junio C Hamano
0 siblings, 1 reply; 107+ messages in thread
From: Junio C Hamano @ 2021-12-13 23:09 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Re-add and use, and expand on "shellquote" macros added in
> 4769948afe7 (Deal with $(bindir) and friends with whitespaces.,
> 2005-10-10).
>
> We avoided using them due to the "$(call)" feature of GNU make being
> relatively new at the time, but it isn't anymore. We hard depend on
> GNU make versions that have it.
>
> The use of "$(call)" was removed in 39c015c556f (Fixes for ancient
> versions of GNU make, 2006-02-18) and 7ffe7098dca (Fix installation of
> templates on ancient systems., 2006-07-29) due to those
> incompatibilities with older GNU make versions, and we've used the
> more verbose *_SQ pattern ever since.
>
> The "$(call)" feature was introduced in GNU make version 3.78,
> released on the 22nd of September, 1999. That release also introduced
> "$(error)" and "$(warning)", which we've been making use of since
> f2fabbf76e4 (Teach Makefile to check header dependencies, 2010-01-26).
>
> This extends upon the macros added in 4769948afe7: We now have macros
> for quoting a ' inside '', and a ' with no surrounding '' as before.
>
> Additionally provide and use a "shelldquote" macro along with
> "shellquote" for the common case of wanting to quote a C string we
> pass to the compiler with a -D flag.
>
> This doesn't get rid of all of our shell quoting. We've still got some
> in the main Makefile, let's leave most of it to avoid in-flight
> conflicts. I've fully converted "templates/Makefile" and "t/Makefile"
> though.
All of the above may very well explain why we decided not to use
$(call shellquote) in the past, and it may also explain why it is
safe to start using it again today, but it does not explain why we
want to do so in the first place.
Having to write
$(call shellquote,$(VAR))
in longhand is much more cumbersome to read, write and merge than
defining a prepackaged VAR_SQ just once and refer it as
'$(VAR_SQ)'
everywhere.
In fact, having to resolve merge conflicts in t/Makefile today,
which wouldn't have been necessary if we didn't have this change,
felt like a makework to me, and my time would have been better spent
elsewhere [*].
Is there a justification other than "this uses a feature supported
by newer GNUMake" we can give to future readers of the log message?
Why do we want to make this change?
It may be that this patch was particularly unlucky to collide with
something else to irritate me, and the above comment may apply to
other patches we saw recently on the list (and they are lucky that
they did not collide with anything not yet in 'master').
In any case, I am wondering if we see too much churn without much
real benefit X-<.
Here is a sample of the damage. Look how long the lines that use
_SQ twice have become.
clean-chainlint:
- $(RM) -r '$(CHAINLINTTMP_SQ)'
+ $(RM) -r $(call shellquote,$(CHAINLINTTMP))
check-chainlint:
- @mkdir -p '$(CHAINLINTTMP_SQ)' && \
- sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/tests && \
- sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >'$(CHAINLINTTMP_SQ)'/expect && \
- $(CHAINLINT) '$(CHAINLINTTMP_SQ)'/tests | grep -v '^[ ]*$$' >'$(CHAINLINTTMP_SQ)'/actual && \
- diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
+ @mkdir -p $(call shellquote,$(CHAINLINTTMP)) && \
+ sed -e '/^# LINT: /d' $(patsubst %,chainlint/%.test,$(CHAINLINTTESTS)) >$(call shellquote,$(CHAINLINTTMP))/tests && \
+ sed -e '/^[ ]*$$/d' $(patsubst %,chainlint/%.expect,$(CHAINLINTTESTS)) >$(call shellquote,$(CHAINLINTTMP))/expect && \
+ $(CHAINLINT) $(call shellquote,$(CHAINLINTTMP))/tests | grep -v '^[ ]*$$' >$(call shellquote,$(CHAINLINTTMP))/actual && \
+ diff -u $(call shellquote,$(CHAINLINTTMP))/expect $(call shellquote,$(CHAINLINTTMP))/actual
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros
2021-12-13 23:09 ` Junio C Hamano
@ 2021-12-14 0:31 ` Junio C Hamano
0 siblings, 0 replies; 107+ messages in thread
From: Junio C Hamano @ 2021-12-14 0:31 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh
Junio C Hamano <gitster@pobox.com> writes:
> All of the above may very well explain why we decided not to use
> $(call shellquote) in the past, and it may also explain why it is
> safe to start using it again today, but it does not explain why we
> want to do so in the first place.
>
> Having to write
>
> $(call shellquote,$(VAR))
>
> in longhand is much more cumbersome to read, write and merge than
> defining a prepackaged VAR_SQ just once and refer it as
>
> '$(VAR_SQ)'
>
> everywhere.
Having said all that, I would not have minded if this change were
more isolated and turned things like
CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
to use the $(call shellquote) mechanism, if all of us found it
easier to read than repeating the same line-noise substitutions for
many variables.
What was irritating was primarily that the patch went one step
further to eliminate the _SQ variant variables, making the resulting
sites that use them harder to read and understand.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (13 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 14/23] Makefile: re-add and use the "shellquote" macros Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-19 6:45 ` Junio C Hamano
2021-11-17 10:20 ` [PATCH v4 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
22 siblings, 1 reply; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Move the mostly copy/pasted code in the "Makefile" and
"Documentation/Makefile" which FORCE-generates a file based on a
variable in play in the Makefile to use a template.
This will make it easier later on to move these to rules that aren't
FORCE-run using optional gmake 4.2+ features, but for now just getting
rid of the repetition is worth it.
The message for the new generated rule will say whether or not we're
generating the file for the first time, as opposed to the old messages
saying "new" whether we had flag modifications, or were building for
the first time.
Example output before:
$ make clean
[...]
$ make
GIT_VERSION = 2.34.0-rc1-dev
* new build flags
CC grep.o
$ make CFLAGS=-I$RANDOM grep.o
* new build flags
CC grep.o
After:
$ make clean
[...]
$ make grep.o
GIT_VERSION = 2.34.0-rc1-dev
GIT-CFLAGS PARAMETERS (new)
CC grep.o
$ make CFLAGS=-I$RANDOM grep.o
GIT-CFLAGS PARAMETERS (changed)
CC grep.o
Note: It's important that "@FLAGS" here be defined as '$$($(2))', and
not the eagerly expanded '$($(2))'. The latter will break if
e.g. curl-config isn't installed, since we'll end up recursively
expanding that part of the variable even if NO_CURL isn't defined,
which happens e.g. for the "check-docs" target in CI.
We're also introducing a $(wspfx) variable here to control the
whitespace prefixing. It matches the $(QUIET...) variables, who'll be
using these variables in a subsequent commit. Note that it's important
that we call the shell quote escaping macros inline (or equivalent),
because if we'd like variables to be overridable we need to support e.g.:
$ make CFLAGS=-I$RANDOM grep.o wspfx='$(space)->'
-> GIT-CFLAGS PARAMETERS (changed)
CC grep.o
If we simply quoted and used $(wspfx) then the user would need to
provide us with a quoted version, so there's still some use-cases for
these $(*_sq) variables. It could also be done inline, but that's a
lot more verbose.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Documentation/Makefile | 8 +------
Makefile | 51 ++++++------------------------------------
shared.mak | 22 ++++++++++++++++++
3 files changed, 30 insertions(+), 51 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index ba27456c86a..4a939cc2c25 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -343,13 +343,7 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
date >$@
TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
-
-GIT-ASCIIDOCFLAGS: FORCE
- @FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-ASCIIDOCFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new asciidoc flags"; \
- echo "$$FLAGS" >GIT-ASCIIDOCFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-ASCIIDOCFLAGS,TRACK_ASCIIDOCFLAGS))
clean:
$(RM) -rf .build/
diff --git a/Makefile b/Makefile
index b458c24d95e..c8a0a1586ca 100644
--- a/Makefile
+++ b/Makefile
@@ -2087,10 +2087,7 @@ endif
GIT_USER_AGENT_SQ = $(subst ','\'',$(GIT_USER_AGENT))
GIT_USER_AGENT_CQ = "$(subst ",\",$(subst \,\\,$(GIT_USER_AGENT)))"
GIT_USER_AGENT_CQ_SQ = $(subst ','\'',$(GIT_USER_AGENT_CQ))
-GIT-USER-AGENT: FORCE
- @if test x'$(GIT_USER_AGENT_SQ)' != x"`cat GIT-USER-AGENT 2>/dev/null`"; then \
- echo '$(GIT_USER_AGENT_SQ)' >GIT-USER-AGENT; \
- fi
+$(eval $(call TRACK_template,GIT-USER-AGENT,GIT_USER_AGENT_SQ))
ifdef DEFAULT_HELP_FORMAT
BASIC_CFLAGS += -DDEFAULT_HELP_FORMAT='"$(DEFAULT_HELP_FORMAT)"'
@@ -2238,12 +2235,7 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
$(perllibdir_SQ)
-GIT-SCRIPT-DEFINES: FORCE
- @FLAGS='$(SCRIPT_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new script parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-SCRIPT-DEFINES,SCRIPT_DEFINES))
define cmd_munge_script
sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
@@ -2355,13 +2347,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
chmod +x $@+ && \
mv $@+ $@
endif # NO_PERL
-
-GIT-PERL-DEFINES: FORCE
- @FLAGS='$(PERL_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new perl-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-PERL-DEFINES,PERL_DEFINES))
# As with NO_PERL=Y we'll still make GIT-PYTHON-DEFINES if "NO_PYTHON"
# is defined, for creating the "unimplemented.sh" scripts.
@@ -2390,12 +2376,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py GIT-PYTHON-DEFINES
mv $@+ $@
endif # NO_PYTHON
-GIT-PYTHON-DEFINES: FORCE
- @FLAGS='$(PYTHON_DEFINES)'; \
- if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
- echo >&2 " * new python-specific parameters"; \
- echo "$$FLAGS" >$@; \
- fi
+$(eval $(call TRACK_template,GIT-PYTHON-DEFINES,PYTHON_DEFINES))
CONFIGURE_RECIPE = sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
configure.ac >configure.ac+ && \
@@ -2751,31 +2732,13 @@ cscope: cscope.out
### Detect prefix changes
TRACK_PREFIX = $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ):\
$(localedir_SQ)
-
-GIT-PREFIX: FORCE
- @FLAGS='$(TRACK_PREFIX)'; \
- if test x"$$FLAGS" != x"`cat GIT-PREFIX 2>/dev/null`" ; then \
- echo >&2 " * new prefix flags"; \
- echo "$$FLAGS" >GIT-PREFIX; \
- fi
+$(eval $(call TRACK_template,GIT-PREFIX,TRACK_PREFIX))
TRACK_CFLAGS = $(CC):$(subst ','\'',$(ALL_CFLAGS)):$(USE_GETTEXT_SCHEME)
-
-GIT-CFLAGS: FORCE
- @FLAGS='$(TRACK_CFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new build flags"; \
- echo "$$FLAGS" >GIT-CFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-CFLAGS,TRACK_CFLAGS))
TRACK_LDFLAGS = $(subst ','\'',$(ALL_LDFLAGS))
-
-GIT-LDFLAGS: FORCE
- @FLAGS='$(TRACK_LDFLAGS)'; \
- if test x"$$FLAGS" != x"`cat GIT-LDFLAGS 2>/dev/null`" ; then \
- echo >&2 " * new link flags"; \
- echo "$$FLAGS" >GIT-LDFLAGS; \
- fi
+$(eval $(call TRACK_template,GIT-LDFLAGS,TRACK_LDFLAGS))
# We need to apply sq twice, once to protect from the shell
# that runs GIT-BUILD-OPTIONS, and then again to protect it
diff --git a/shared.mak b/shared.mak
index ef03c2bc094..97c8903f22c 100644
--- a/shared.mak
+++ b/shared.mak
@@ -29,3 +29,25 @@ shelldquote = '"$(call shdq,$(call shq,$(1)))"'
comma = ,
empty =
space = $(empty) $(empty)
+
+## wspfx: the whitespace prefix padding for $(QUIET...) and similarly
+## aligned output.
+wspfx = $(space)$(space)$(space)
+wspfx_sq = $(call shellquote,$(wspfx))
+
+### Templates
+
+## Template for making a GIT-SOMETHING, which changes if a
+## TRACK_SOMETHING variable changes.
+define TRACK_template
+.PHONY: FORCE
+$(1): FORCE
+ @FLAGS='$$($(2))'; \
+ if ! test -f $(1) ; then \
+ echo $(wspfx_sq) "$(1) PARAMETERS (new)"; \
+ echo "$$$$FLAGS" >$(1); \
+ elif test x"$$$$FLAGS" != x"`cat $(1) 2>/dev/null`" ; then \
+ echo $(wspfx_sq) "$(1) PARAMETERS (changed)"; \
+ echo "$$$$FLAGS" >$(1); \
+ fi
+endef
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* Re: [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-17 10:20 ` [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
@ 2021-11-19 6:45 ` Junio C Hamano
2021-11-19 6:56 ` Junio C Hamano
0 siblings, 1 reply; 107+ messages in thread
From: Junio C Hamano @ 2021-11-19 6:45 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> @@ -2238,12 +2235,7 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
> $(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> $(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
> $(perllibdir_SQ)
> -GIT-SCRIPT-DEFINES: FORCE
> - @FLAGS='$(SCRIPT_DEFINES)'; \
> - if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
> - echo >&2 " * new script parameters"; \
> - echo "$$FLAGS" >$@; \
> - fi
> +$(eval $(call TRACK_template,GIT-SCRIPT-DEFINES,SCRIPT_DEFINES))
>
> define cmd_munge_script
> sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
Attempting to apply 1-14 on top of 'master' gets stuck here, as
"GIT-SCRIPT-DEFINES: FORCE" appears after "define cmd_munge_script".
Are you basing these patches on some unrelated work?
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-19 6:45 ` Junio C Hamano
@ 2021-11-19 6:56 ` Junio C Hamano
2021-11-19 7:30 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 107+ messages in thread
From: Junio C Hamano @ 2021-11-19 6:56 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh
Junio C Hamano <gitster@pobox.com> writes:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> @@ -2238,12 +2235,7 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
>> $(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
>> $(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
>> $(perllibdir_SQ)
>> -GIT-SCRIPT-DEFINES: FORCE
>> - @FLAGS='$(SCRIPT_DEFINES)'; \
>> - if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
>> - echo >&2 " * new script parameters"; \
>> - echo "$$FLAGS" >$@; \
>> - fi
>> +$(eval $(call TRACK_template,GIT-SCRIPT-DEFINES,SCRIPT_DEFINES))
>>
>> define cmd_munge_script
>> sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>
> Attempting to apply 1-14 on top of 'master' gets stuck here, as
> "GIT-SCRIPT-DEFINES: FORCE" appears after "define cmd_munge_script".
>
> Are you basing these patches on some unrelated work?
Ah, I figured it out. This wants to be applied after the
ab/sh-retire-helper-functions topic graduates to 'master'.
^ permalink raw reply [flat|nested] 107+ messages in thread
* Re: [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
2021-11-19 6:56 ` Junio C Hamano
@ 2021-11-19 7:30 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-19 7:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh
On Thu, Nov 18 2021, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> @@ -2238,12 +2235,7 @@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
>>> $(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
>>> $(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
>>> $(perllibdir_SQ)
>>> -GIT-SCRIPT-DEFINES: FORCE
>>> - @FLAGS='$(SCRIPT_DEFINES)'; \
>>> - if test x"$$FLAGS" != x"`cat $@ 2>/dev/null`" ; then \
>>> - echo >&2 " * new script parameters"; \
>>> - echo "$$FLAGS" >$@; \
>>> - fi
>>> +$(eval $(call TRACK_template,GIT-SCRIPT-DEFINES,SCRIPT_DEFINES))
>>>
>>> define cmd_munge_script
>>> sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
>>
>> Attempting to apply 1-14 on top of 'master' gets stuck here, as
>> "GIT-SCRIPT-DEFINES: FORCE" appears after "define cmd_munge_script".
>>
>> Are you basing these patches on some unrelated work?
>
> Ah, I figured it out. This wants to be applied after the
> ab/sh-retire-helper-functions topic graduates to 'master'.
It is noted in the v1..v3 cover letter of this series that it needs to
be on top of ab/sh-retire-helper-functions.
But I omitted it from the v4 by mistake. Sorry about wasting your time
on finding that out as a result, it really should have been noted there.
^ permalink raw reply [flat|nested] 107+ messages in thread
* [PATCH v4 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (14 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 15/23] Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...} Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 4a939cc2c25..69a9af35397 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 c8a0a1586ca..c437aea9e4a 100644
--- a/Makefile
+++ b/Makefile
@@ -1939,39 +1939,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 d0701f9beb0..1a12d8c635f 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -715,7 +715,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 97c8903f22c..1b666a1bc69 100644
--- a/shared.mak
+++ b/shared.mak
@@ -35,6 +35,59 @@ space = $(empty) $(empty)
wspfx = $(space)$(space)$(space)
wspfx_sq = $(call shellquote,$(wspfx))
+### 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
+
### Templates
## Template for making a GIT-SOMETHING, which changes if a
diff --git a/templates/Makefile b/templates/Makefile
index c9251a96622..b056e710b7e 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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (15 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 16/23] Makefile: add "$(QUIET)" boilerplate to shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 18/23] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 | 48 ++++++++++++++++++++++++------------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/shared.mak b/shared.mak
index 1b666a1bc69..36f00e88bb8 100644
--- a/shared.mak
+++ b/shared.mak
@@ -50,39 +50,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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 18/23] Makefiles: add and use wildcard "mkdir -p" template
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (16 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 17/23] Makefile: use $(wspfx) for $(QUIET...) in shared.mak Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 19/23] Makefile: correct the dependency graph of hook-list.h Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 thatnoted 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 hyperfine -L rev HEAD~0,HEAD~1 -b 'make -C Documentation lint-docs' -p 'rm -rf Documentation/.build' 'make -C Documentation lint-docs'
Benchmark 1: make -C Documentation lint-docs' in 'HEAD~0
Time (mean ± σ): 2.129 s ± 0.011 s [User: 1.840 s, System: 0.321 s]
Range (min … max): 2.121 s … 2.158 s 10 runs
Benchmark 2: make -C Documentation lint-docs' in 'HEAD~1
Time (mean ± σ): 2.659 s ± 0.002 s [User: 2.306 s, System: 0.397 s]
Range (min … max): 2.657 s … 2.662 s 10 runs
Summary
'make -C Documentation lint-docs' in 'HEAD~0' ran
1.25 ± 0.01 times faster than 'make -C Documentation 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 | 7 +++++++
3 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 69a9af35397..d16b653394c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -428,25 +428,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) \
@@ -457,23 +443,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 c437aea9e4a..0a3f292bf82 100644
--- a/Makefile
+++ b/Makefile
@@ -2632,7 +2632,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 $@ $<
ifndef NO_PERL
LIB_PERL = $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
@@ -2641,7 +2642,8 @@ LIB_CPAN = $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
LIB_CPAN_GEN = $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
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@@|$(call shq,$(NO_PERL_CPAN_FALLBACKS))|g' \
@@ -2655,8 +2657,8 @@ endif
# install-man depends on Git.3pm even with NO_PERL=Y
perl/build/man/man3/Git.3pm: perl/Git.pm
- $(QUIET_GEN)mkdir -p $(dir $@) && \
- pod2man $< $@
+ $(call mkdir_p_parent_template)
+ $(QUIET_GEN)pod2man $< $@
FIND_SOURCE_FILES = ( \
git ls-files \
@@ -2780,7 +2782,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 36f00e88bb8..8d3023c2782 100644
--- a/shared.mak
+++ b/shared.mak
@@ -56,6 +56,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 $@;
@@ -88,6 +90,11 @@ ifndef V
endif
endif
+## Helpers
+define mkdir_p_parent_template
+$(if $(wildcard $(@D)),,$(QUIET_MKDIR_P_PARENT)$(shell mkdir -p $(@D)))
+endef
+
### Templates
## Template for making a GIT-SOMETHING, which changes if a
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 19/23] Makefile: correct the dependency graph of hook-list.h
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (17 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 18/23] Makefiles: add and use wildcard "mkdir -p" template Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Fix an issue in my cfe853e66be (hook-list.h: add a generated list of
hooks, like config-list.h, 2021-09-26), the builtin/help.c was
inadvertently made to depend on hook-list.h, but it's used by
builtin/bugreport.c.
The hook.c also does not depend on hook-list.h. It did in an earlier
version of the greater series cfe853e66be was extracted from, but not
anymore. We might end up needing that line again, but let's remove it
for now.
Reported-by: Mike Hommey <mh@glandium.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 0a3f292bf82..a9fa9461613 100644
--- a/Makefile
+++ b/Makefile
@@ -2161,9 +2161,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
$(filter %.o,$^) $(LIBS)
help.sp help.s help.o: command-list.h
-hook.sp hook.s hook.o: hook-list.h
+builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
-builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX
+builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (18 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 19/23] Makefile: correct the dependency graph of hook-list.h Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 21/23] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Provide an alternate implementation of the recently added
"TRACK_template" which uses GNU make 4.2 features. If the 'file'
function is available we don't need to shell out at all to check if
our tracked files change.
We need to use the intermediate TRACK_template calling a
TRACK_template_eval within the generated rule so that we don't eagerly
fleshen these when "make" reads the file.
This doesn't make the runtime faster on my system, but helps to cut
down on the noise of things we shell out for
unconditionally. I.e. running "make" with "SHELL_PATH='sh -x'" (twice,
so we pick up the setting) shows than a no-op run went from emitting:
$ SHELL_PATH='sh -x' make -j8 >/dev/null 2>&1; SHELL_PATH='sh -x' make 2>&1 |wc -l
124
To:
$ SHELL_PATH='sh -x' make -j8 >/dev/null 2>&1; SHELL_PATH='sh -x' make 2>&1 |wc -l
95
That 124 to 95 number is a rough approximation of how many times we
shell out. "strace -f -c" similarly shows that we went from 8798 to
8466 syscalls. So this brings us further along in the goal of making
"make" do as little as possible when it's got nothing to re-build (see
[2]).
1. https://lore.kernel.org/git/874kdn1j6i.fsf@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
shared.mak | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/shared.mak b/shared.mak
index 8d3023c2782..2d5d5b2fb9b 100644
--- a/shared.mak
+++ b/shared.mak
@@ -8,6 +8,22 @@
# info make --index-search=.DELETE_ON_ERROR
.DELETE_ON_ERROR:
+### GNU Make version detection
+# We don't care about "release" versions like the "90" in "3.99.90"
+MAKE_VERSION_MAJOR = $(word 1,$(subst ., ,$(MAKE_VERSION)))
+MAKE_VERSION_MINOR = $(word 2,$(subst ., ,$(MAKE_VERSION)))
+
+# The oldest supported version of GNU make is 3-something. So "not v3"
+# is a future-proof way to ask "is it modern?"
+ifneq ($(MAKE_VERSION_MAJOR),3)
+# $(file >[...]) and $(file >>[...]) is in 4.0...
+MAKE_HAVE_FILE_WRITE = Need version 4.0 or later (released in late 2013)
+# .. but we need 4.2 for $(file <[...])
+ifneq ($(filter-out 0 1,$(MAKE_VERSION_MINOR)),)
+MAKE_HAVE_FILE_READ = Need version 4.2 or later (released in mid-2016)
+endif
+endif
+
### Quoting helpers
## Quote a ' inside a '': FOO='$(call shq,$(BAR))'
@@ -99,6 +115,10 @@ endef
## Template for making a GIT-SOMETHING, which changes if a
## TRACK_SOMETHING variable changes.
+##
+## This is the slower version used on GNU make <4.2.
+ifndef MAKE_HAVE_FILE_READ
+
define TRACK_template
.PHONY: FORCE
$(1): FORCE
@@ -111,3 +131,41 @@ $(1): FORCE
echo "$$$$FLAGS" >$(1); \
fi
endef
+
+endif # !MAKE_HAVE_FILE_READ
+
+## A TRACK_template template compatible with the one above. Uses
+## features of GNU make >=4.2 to avoid shelling out for this "hot"
+## "FORCE" logic.
+##
+## Since version >=4.2 can do both "I" and "O" in I/O with using
+## $(file <)/$(file >) we read the GIT-SOMETHING file into a variable
+## with the former, and if it's different from our expected value
+## write it out with the latter.
+ifdef MAKE_HAVE_FILE_READ
+
+define TRACK_template_eval
+$(1)_WRITE =
+$(1)_EXISTS = $(wildcard $(1))
+ifeq ($$($(1)_EXISTS),)
+$(1)_WRITE = new
+else
+$(1)_CONTENT = $(file <$(1))
+ifeq ($$($(1)_CONTENT),$($(2)))
+$(1)_WRITE = same
+else
+$(1)_WRITE = changed
+endif
+endif
+ifneq ($$($(1)_WRITE),same)
+$$(info $$(wspfx) $(1) parameters ($$($(1)_WRITE)))
+$$(file >$(1),$($(2)))
+endif
+endef # TRACK_template_eval
+
+define TRACK_template
+$(1):
+ $$(eval $$(call TRACK_template_eval,$(1),$(2)))
+endef
+
+endif # MAKE_HAVE_FILE_READ
--
2.34.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 21/23] Makefile: disable GNU make built-in wildcard rules
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (19 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 20/23] Makefile: use $(file) I/O instead of "FORCE" when possible Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 23/23] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Ævar Arnfjörð Bjarmason
Override built-in rules of GNU make that use a wildcard target. This
speeds things up significantly as we don't need to stat() so many
files just in case we'd be able to retrieve their contents from RCS or
SCCS. See [1] for an old mailing list discussion about how to disable
these.
This gives us a noticeable speedup on a no-op run:
$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make -j8 all NO_TCLTK=Y' 'make NO_TCLTK=Y' --warmup 10 -M 10
Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
Time (mean ± σ): 182.2 ms ± 4.1 ms [User: 146.8 ms, System: 49.5 ms]
Range (min … max): 179.6 ms … 193.4 ms 10 runs
Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
Time (mean ± σ): 167.9 ms ± 1.3 ms [User: 127.8 ms, System: 55.6 ms]
Range (min … max): 166.1 ms … 170.4 ms 10 runs
Summary
'make NO_TCLTK=Y' in 'HEAD~0' ran
1.09 ± 0.03 times faster than 'make NO_TCLTK=Y' in 'HEAD~1'
Running the same except with 'strace -c -S calls make' as the
benchmark command shows (under --show-output) that we went from ~7716
syscalls to ~7519, mostly a reduction in [l]stat().
We could also invoke make with "-r" by setting "MAKEFLAGS = -r" early,
adding a "-r" variant to the above benchmark shows that it may be 1.01
or so faster (but in my tests, always with a much bigger error
bar). 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 2d5d5b2fb9b..20109e6a90e 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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (20 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 21/23] Makefile: disable GNU make built-in wildcard rules Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
2021-11-17 10:20 ` [PATCH v4 23/23] Makefile: move ".SUFFIXES" rule to shared.mak Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 -j8 all NO_TCLTK=Y' 'make NO_TCLTK=Y' --war
mup 10 -M 10
Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
Time (mean ± σ): 99.5 ms ± 0.5 ms [User: 83.4 ms, System: 20.7 ms]
Range (min … max): 98.8 ms … 100.2 ms 10 runs
Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
Time (mean ± σ): 69.4 ms ± 0.5 ms [User: 58.8 ms, System: 14.2 ms]
Range (min … max): 68.9 ms … 70.3 ms 10 runs
Summary
'make NO_TCLTK=Y' in 'HEAD~0' ran
1.43 ± 0.01 times faster than 'make 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 a9fa9461613..eb1c944787b 100644
--- a/Makefile
+++ b/Makefile
@@ -821,12 +821,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
@@ -2660,26 +2681,6 @@ perl/build/man/man3/Git.3pm: perl/Git.pm
$(call mkdir_p_parent_template)
$(QUIET_GEN)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 $@+ && \
@@ -2859,9 +2860,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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread
* [PATCH v4 23/23] Makefile: move ".SUFFIXES" rule to shared.mak
2021-11-17 10:19 ` [PATCH v4 00/23] Makefile: dependency fixes, make noop runtime ~1.4x faster Ævar Arnfjörð Bjarmason
` (21 preceding siblings ...)
2021-11-17 10:20 ` [PATCH v4 22/23] Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES) Ævar Arnfjörð Bjarmason
@ 2021-11-17 10:20 ` Ævar Arnfjörð Bjarmason
22 siblings, 0 replies; 107+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-11-17 10:20 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, Jeff King, Dan Jacques, Eric Wong,
Jonathan Nieder, Mike Hommey,
Đoàn Trần Công Danh,
Æ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 ~1.3x
faster:
$ git -c hyperfine.hook.setup= hyperfine -L rev HEAD~1,HEAD~0 -s 'make -j8 -C Documentation man' 'make -C Documentation man' --warmup 10 -M 10
Benchmark 1: make -C Documentation man' in 'HEAD~1
Time (mean ± σ): 87.2 ms ± 1.0 ms [User: 79.3 ms, System: 10.8 ms]
Range (min … max): 86.3 ms … 89.7 ms 10 runs
Benchmark 2: make -C Documentation man' in 'HEAD~0
Time (mean ± σ): 64.5 ms ± 0.6 ms [User: 54.5 ms, System: 13.0 ms]
Range (min … max): 63.8 ms … 65.7 ms 10 runs
Summary
'make -C Documentation man' in 'HEAD~0' ran
1.35 ± 0.02 times faster than 'make -C Documentation 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 eb1c944787b..035573c8997 100644
--- a/Makefile
+++ b/Makefile
@@ -2459,8 +2459,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 20109e6a90e..d8453bb471f 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.0.796.g2c87ed6146a
^ permalink raw reply related [flat|nested] 107+ messages in thread