* [PATCH 0/4] Makefile: micro-optimize light non-test builds
@ 2021-01-26 16:07 Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason
` (4 more replies)
0 siblings, 5 replies; 39+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason
This small series speeds up builds where you just want to get to a
working "git" binary, but don't care about running git's own tests, or
about making/installing fallbacks for "git svn" et al (which we do
even with NO_PERL).
Ævar Arnfjörð Bjarmason (4):
Makefile: refactor assignment for subsequent change
Makefile: refactor for subsequent change
Makefile: add a NO_TEST_TOOLS flag
Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target
Makefile | 48 +++++++++++++++++++++++++++++++++++++++++++-----
t/test-lib.sh | 5 +++++
2 files changed, 48 insertions(+), 5 deletions(-)
--
2.29.2.222.g5d2a92d10f8
^ permalink raw reply [flat|nested] 39+ messages in thread* [PATCH 1/4] Makefile: refactor assignment for subsequent change 2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 ` Ævar Arnfjörð Bjarmason 2021-01-27 1:29 ` Junio C Hamano 2021-01-26 16:07 ` [PATCH 2/4] Makefile: refactor " Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 4 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Refactor a multi-line assignment into a form that'll lend itself better to having "ifdef" split it up in a follow-up commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 4edfda3e009..36c7b8fa08b 100644 --- a/Makefile +++ b/Makefile @@ -656,10 +656,10 @@ clean-perl-script: clean-python-script: $(RM) $(SCRIPT_PYTHON_GEN) -SCRIPTS = $(SCRIPT_SH_GEN) \ - $(SCRIPT_PERL_GEN) \ - $(SCRIPT_PYTHON_GEN) \ - git-instaweb +SCRIPTS = $(SCRIPT_SH_GEN) +SCRIPTS += $(SCRIPT_PERL_GEN) +SCRIPTS += $(SCRIPT_PYTHON_GEN) +SCRIPTS += git-instaweb ETAGS_TARGET = TAGS -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/4] Makefile: refactor assignment for subsequent change 2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason @ 2021-01-27 1:29 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-01-27 1:29 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Refactor a multi-line assignment into a form that'll lend itself > better to having "ifdef" split it up in a follow-up commit. Hmph, it would do even better to start with an empty definition without treating the first one specially, i.e. ... > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > Makefile | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Makefile b/Makefile > index 4edfda3e009..36c7b8fa08b 100644 > --- a/Makefile > +++ b/Makefile > @@ -656,10 +656,10 @@ clean-perl-script: > clean-python-script: > $(RM) $(SCRIPT_PYTHON_GEN) > > -SCRIPTS = $(SCRIPT_SH_GEN) \ > - $(SCRIPT_PERL_GEN) \ > - $(SCRIPT_PYTHON_GEN) \ > - git-instaweb > +SCRIPTS = $(SCRIPT_SH_GEN) ... if this becomes ... SCRIPTS = SCRIPTS += $(SCRIPT_SH_GEN) ... instead, no? > +SCRIPTS += $(SCRIPT_PERL_GEN) > +SCRIPTS += $(SCRIPT_PYTHON_GEN) > +SCRIPTS += git-instaweb > > ETAGS_TARGET = TAGS ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/4] Makefile: refactor for subsequent change 2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason 2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 ` Ævar Arnfjörð Bjarmason 2021-01-26 16:07 ` [PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 4 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Refactor the list of test programs into a handy variable. This does not matter now, but will make a subsequent change smaller. 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 36c7b8fa08b..4031fb1b22f 100644 --- a/Makefile +++ b/Makefile @@ -2788,8 +2788,9 @@ GIT-PYTHON-VARS: FORCE endif test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X)) +TEST_TOOLS = $(TEST_PROGRAMS) $(test_bindir_programs) -all:: $(TEST_PROGRAMS) $(test_bindir_programs) +all:: $(TEST_TOOLS) bin-wrappers/%: wrap-for-bin.sh @mkdir -p bin-wrappers -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag 2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason 2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason 2021-01-26 16:07 ` [PATCH 2/4] Makefile: refactor " Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 ` Ævar Arnfjörð Bjarmason 2021-01-26 16:07 ` [PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target Ævar Arnfjörð Bjarmason 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King 4 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Add a NO_TEST_TOOLS flag to build an installable git, but one that can't run "make test". This is useful e.g. in CI environments where you'd like to run external tests against a built git, but have no desire to run git's own tests. On my 8 core machine this saves me around 1 second out of an otherwise 11-12 second build time. So it doesn't make all the difference, but when you're wanting to run tests against a lot of git versions it adds up. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 6 ++++++ t/test-lib.sh | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/Makefile b/Makefile index 4031fb1b22f..cfa7bc58edf 100644 --- a/Makefile +++ b/Makefile @@ -309,6 +309,9 @@ all:: # # Define NO_TCLTK if you do not want Tcl/Tk GUI. # +# Define NO_TEST_TOOLS if you'd like to skip building the assets +# required to run the tests. +# # Define SANE_TEXT_GREP to "-a" if you use recent versions of GNU grep # and egrep that are pickier when their input contains non-ASCII data. # @@ -2732,6 +2735,7 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+ @echo NO_PTHREADS=\''$(subst ','\'',$(subst ','\'',$(NO_PTHREADS)))'\' >>$@+ @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+ + @echo NO_TEST_TOOLS=\''$(subst ','\'',$(subst ','\'',$(NO_TEST_TOOLS)))'\' >>$@+ @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+ @echo DC_SHA1=\''$(subst ','\'',$(subst ','\'',$(DC_SHA1)))'\' >>$@+ @@ -2787,8 +2791,10 @@ GIT-PYTHON-VARS: FORCE fi endif +ifndef NO_TEST_TOOLS test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X)) TEST_TOOLS = $(TEST_PROGRAMS) $(test_bindir_programs) +endif all:: $(TEST_TOOLS) diff --git a/t/test-lib.sh b/t/test-lib.sh index 03c1c0836f1..4029cd18031 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -55,6 +55,11 @@ then exit 1 fi . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS +if test -n "$NO_TEST_TOOLS" +then + echo >&2 'error: NO_TEST_TOOLS=$NO_TEST_TOOLS set in GIT-BUILD-OPTIONS, cannot run tests!.' + exit 1 +fi export PERL_PATH SHELL_PATH # Disallow the use of abbreviated options in the test suite by default -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target 2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2021-01-26 16:07 ` [PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 ` Ævar Arnfjörð Bjarmason 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King 4 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-26 16:07 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason Add a target to skip the installation of e.g. dummy "git-svn" when NO_PERL is defined. This makes it easier to generate more minimal installations, e.g. for embedded use that's never going to care about "git-svn" not being around. We do some basic sanity checking that e.g. NO_INSTALL_SCRIPT_FALLBACKS isn't set without some of NO_{PERL,PYTHON,TCLTK}, and that you don't set NO_INSTALL_SCRIPT_FALLBACKS without NO_SCRIPT_FALLBACKS. Otherwise "make install" would error out. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index cfa7bc58edf..b3c212891b6 100644 --- a/Makefile +++ b/Makefile @@ -338,6 +338,10 @@ all:: # when hardlinking a file to another name and unlinking the original file right # away (some NTFS drivers seem to zero the contents in that scenario). # +# Define NO_SCRIPT_FALLBACKS if you'd like to not generate and install +# the fallback scripts which defining NO_PERL and NO_PYTHON would +# normally produce. See also NO_INSTALL_SCRIPT_FALLBACKS. +# # Define INSTALL_SYMLINKS if you prefer to have everything that can be # symlinked between bin/ and libexec/ to use relative symlinks between # the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and @@ -351,6 +355,11 @@ all:: # Define NO_INSTALL_HARDLINKS if you prefer to use either symbolic links or # copies to install built-in git commands e.g. git-cat-file. # +# Define NO_INSTALL_SCRIPT_FALLBACKS to skip the installation of +# script fallbacks you didn't generate due to also setting +# NO_SCRIPT_FALLBACKS. Using this without also defining +# NO_SCRIPT_FALLBACKS is not supported. +# # Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the # built-ins to be linked/copied at all. # @@ -603,6 +612,19 @@ THIRD_PARTY_SOURCES = # interactive shell sessions without exporting it. unexport CDPATH +# Sanity check options +ifdef NO_INSTALL_SCRIPT_FALLBACKS +ifndef NO_SCRIPT_FALLBACKS +$(error Setting NO_INSTALL_SCRIPT_FALLBACKS is only supported if NO_SCRIPT_FALLBACKS is also set!) +endif +endif + +ifdef NO_SCRIPT_FALLBACKS +ifeq (,$(NO_PERL)$(NO_PYTHON)$(NO_TCLTK)) +$(error You should set some of NO_{PERL,PYTHON,TCLTK} when using NO_SCRIPT_FALLBACKS) +endif +endif + SCRIPT_SH += git-bisect.sh SCRIPT_SH += git-difftool--helper.sh SCRIPT_SH += git-filter-branch.sh @@ -659,10 +681,12 @@ clean-perl-script: clean-python-script: $(RM) $(SCRIPT_PYTHON_GEN) -SCRIPTS = $(SCRIPT_SH_GEN) +SCRIPTS = $(SCRIPT_SH_GEN) +ifndef NO_SCRIPT_FALLBACKS SCRIPTS += $(SCRIPT_PERL_GEN) SCRIPTS += $(SCRIPT_PYTHON_GEN) SCRIPTS += git-instaweb +endif ETAGS_TARGET = TAGS @@ -804,7 +828,9 @@ BINDIR_PROGRAMS_NEED_X += git-shell BINDIR_PROGRAMS_NEED_X += git-upload-archive BINDIR_PROGRAMS_NEED_X += git-upload-pack +ifndef NO_INSTALL_SCRIPT_FALLBACKS BINDIR_PROGRAMS_NO_X += git-cvsserver +endif # Set paths to tools early so that they can be used for version tests. ifndef SHELL_PATH @@ -2322,6 +2348,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES chmod +x $@+ && \ mv $@+ $@ else # NO_PERL +ifndef NO_SCRIPT_FALLBACKS $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh $(QUIET_GEN)$(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ @@ -2329,6 +2356,7 @@ $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh unimplemented.sh >$@+ && \ chmod +x $@+ && \ mv $@+ $@ +endif # NO_SCRIPT_FALLBACKS endif # NO_PERL # This makes sure we depend on the NO_PYTHON setting itself. @@ -2343,6 +2371,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py chmod +x $@+ && \ mv $@+ $@ else # NO_PYTHON +ifndef NO_SCRIPT_FALLBACKS $(SCRIPT_PYTHON_GEN): % : unimplemented.sh $(QUIET_GEN)$(RM) $@ $@+ && \ sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ @@ -2350,6 +2379,7 @@ $(SCRIPT_PYTHON_GEN): % : unimplemented.sh unimplemented.sh >$@+ && \ chmod +x $@+ && \ mv $@+ $@ +endif # NO_SCRIPT_FALLBACKS endif # NO_PYTHON CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \ @@ -2735,6 +2765,7 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@+ @echo NO_PTHREADS=\''$(subst ','\'',$(subst ','\'',$(NO_PTHREADS)))'\' >>$@+ @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@+ + @echo NO_SCRIPT_FALLBACKS=\''$(subst ','\'',$(subst ','\'',$(NO_SCRIPT_FALLBACKS)))'\' >>$@+ @echo NO_TEST_TOOLS=\''$(subst ','\'',$(subst ','\'',$(NO_TEST_TOOLS)))'\' >>$@+ @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@+ @echo PAGER_ENV=\''$(subst ','\'',$(subst ','\'',$(PAGER_ENV)))'\' >>$@+ -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds 2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason ` (3 preceding siblings ...) 2021-01-26 16:07 ` [PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target Ævar Arnfjörð Bjarmason @ 2021-01-26 21:16 ` Jeff King 2021-01-27 1:38 ` Junio C Hamano ` (3 more replies) 4 siblings, 4 replies; 39+ messages in thread From: Jeff King @ 2021-01-26 21:16 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano On Tue, Jan 26, 2021 at 05:07:04PM +0100, Ævar Arnfjörð Bjarmason wrote: > This small series speeds up builds where you just want to get to a > working "git" binary, but don't care about running git's own tests, or > about making/installing fallbacks for "git svn" et al (which we do > even with NO_PERL). I have to wonder if you really care about non-builtins here. If not, then doesn't "make git" do what you want? I recently did something similar, but a bit more extreme. I have a 100-patch series introducing annotations/fixes for -Wunused-parameter. I rebased it on master, and the end result had a compile error (a previously unused and annotated parameter became used). So I wanted not just to fix it, but to put the fix in the right commit. Doing: git rebase -x 'make -j16' builds each commit and stops when we hit the breakage, which is nice. But it takes a while to build, and a non-trivial bit of time is spent generating libgit.a, running the linker, making builtin hardlinks, etc. I ended up putting: objects: $(LIB_OBJS) $(BUILTIN_OBJS) git.o into my config.mak, and then "make objects" is quite fast. Probably too gross a hack to carry in our Makefile, but I was tempted to send it. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King @ 2021-01-27 1:38 ` Junio C Hamano 2021-01-27 4:34 ` Jeff King 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 3 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2021-01-27 1:38 UTC (permalink / raw) To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, git Jeff King <peff@peff.net> writes: > On Tue, Jan 26, 2021 at 05:07:04PM +0100, Ævar Arnfjörð Bjarmason wrote: > >> This small series speeds up builds where you just want to get to a >> working "git" binary, but don't care about running git's own tests, or >> about making/installing fallbacks for "git svn" et al (which we do >> even with NO_PERL). > > I have to wonder if you really care about non-builtins here. If not, > then doesn't "make git" do what you want? I had the same thought, while wondering if all the ugliness in [4/4] is really worth it. The steps 2/4 and 3/4 did look like a useful feature, but I wonder why we even need to introduce NO_TEST_TOOLS in the first place. Wouldn't it be more natural to arrange them to be built by making "test::" target depend on them? IOW, why do we need to have "all::" (our default) target depend on them? And if we are not doing [4/4], I suspect [1/4], while it is not bad as a clean-up, would become less attractive. So... ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds 2021-01-27 1:38 ` Junio C Hamano @ 2021-01-27 4:34 ` Jeff King 2021-01-27 6:07 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2021-01-27 4:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git On Tue, Jan 26, 2021 at 05:38:08PM -0800, Junio C Hamano wrote: > The steps 2/4 and 3/4 did look like a useful feature, but I wonder > why we even need to introduce NO_TEST_TOOLS in the first place. > Wouldn't it be more natural to arrange them to be built by making > "test::" target depend on them? IOW, why do we need to have "all::" > (our default) target depend on them? Hmm. That is definitely more logical, and giving "make" more information to make a good decision about what is needed. I do wonder if it would be annoying in two cases, though: - people trigger the tests in other ways besides "make test". For instance, "make && cd t && make" works, as does just "make && cd t && ./t1234". With a more clever Makefile, those would fail (or worse, run out-of-date versions of the helpers, producing confusing results). - during refactoring, I often compile-test as I go (i.e., run "make" to see which callers still need changed, then fix them, repeat). If that didn't catch test helpers, then I'd think I was done and get bit later by "make test" trying to build more code. Not the end of the world, but a minor annoyance. So I think even though I'd argue that giving "make" that extra dependency information is "more correct", we are fighting uphill against existing behavior, as well as things that make doesn't know (like that I expect to be ready to run tests as long as "make all" has finished). -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds 2021-01-27 4:34 ` Jeff King @ 2021-01-27 6:07 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-01-27 6:07 UTC (permalink / raw) To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, git Jeff King <peff@peff.net> writes: > On Tue, Jan 26, 2021 at 05:38:08PM -0800, Junio C Hamano wrote: > >> The steps 2/4 and 3/4 did look like a useful feature, but I wonder >> why we even need to introduce NO_TEST_TOOLS in the first place. >> Wouldn't it be more natural to arrange them to be built by making >> "test::" target depend on them? IOW, why do we need to have "all::" >> (our default) target depend on them? > > Hmm. That is definitely more logical, and giving "make" more information > to make a good decision about what is needed. I do wonder if it would be > annoying in two cases, though: > > - people trigger the tests in other ways besides "make test". For > instance, "make && cd t && make" works, as does just > "make && cd t && ./t1234". With a more clever Makefile, those would > fail (or worse, run out-of-date versions of the helpers, producing > confusing results). > > - during refactoring, I often compile-test as I go (i.e., run "make" > to see which callers still need changed, then fix them, repeat). > If that didn't catch test helpers, then I'd think I was done and get > bit later by "make test" trying to build more code. Not the end of > the world, but a minor annoyance. > > So I think even though I'd argue that giving "make" that extra > dependency information is "more correct", we are fighting uphill against > existing behavior, as well as things that make doesn't know (like that I > expect to be ready to run tests as long as "make all" has finished). Hmph, true, but as "make test" at the top-level merely redirects to "make -C t", I imagined that the default target in the t/Makefile would depend on doing "make -C .. test-programs" before running tests. The recursive dependencies end somewhere, though ;-) ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King 2021-01-27 1:38 ` Junio C Hamano @ 2021-01-28 18:23 ` Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (6 more replies) 2021-01-28 18:23 ` [PATCH 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason 2021-01-28 18:23 ` [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason 3 siblings, 7 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-28 18:23 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason This is a replacement for a series I sent in https://lore.kernel.org/git/20210126160708.20903-1-avarab@gmail.com/ As noted there I can just run "make git", which I'd somehow managed to miss. So that complexity isn't needed. But Jeff King suggested a hack to just get you to the point of git.o. I don't need that right now, but that seems sensible, so I implemented it. At the start of this series I've got a patch to make "all" stop redundantly depending on "FUZZ_OBJS", which also helps with such "rebase -i --exec=..." use-cases. Ævar Arnfjörð Bjarmason (6): Makefile: remove "all" on "$(FUZZ_OBJS)" Makefile: guard against TEST_OBJS in the environment Makefile: split up long OBJECTS line Makefile: sort OBJECTS assignment for subsequent change Makefile: split OBJECTS into OBJECTS and GIT_OBJS Makefile: add {program,xdiff,test,git}-objs & objects targets Makefile | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-03 1:11 ` Junio C Hamano ` (7 more replies) 2021-02-01 11:17 ` [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason ` (5 subsequent siblings) 6 siblings, 8 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason A re-send of v1 (https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com) + a trivial whitespace fix in 2/6. Due to recent vger.kernel.org difficulties 4-6 never made it on-list. Ævar Arnfjörð Bjarmason (6): Makefile: remove "all" on "$(FUZZ_OBJS)" Makefile: guard against TEST_OBJS in the environment Makefile: split up long OBJECTS line Makefile: sort OBJECTS assignment for subsequent change Makefile: split OBJECTS into OBJECTS and GIT_OBJS Makefile: add {program,xdiff,test,git}-objs & objects targets Makefile | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason @ 2021-02-03 1:11 ` Junio C Hamano 2021-02-04 7:06 ` Jeff King 2021-02-23 11:41 ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason ` (6 subsequent siblings) 7 siblings, 1 reply; 39+ messages in thread From: Junio C Hamano @ 2021-02-03 1:11 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > A re-send of v1 > (https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com) > + a trivial whitespace fix in 2/6. I'll reproduce what it said for those who are reading from sidelines: As noted there I can just run "make git", which I'd somehow managed to miss. So that complexity isn't needed. But Jeff King suggested a hack to just get you to the point of git.o. I don't need that right now, but that seems sensible, so I implemented it. At the start of this series I've got a patch to make "all" stop redundantly depending on "FUZZ_OBJS", which also helps with such "rebase -i --exec=..." use-cases. I do not see much point in the goal, not quite. You can do "make git.o" and "make git" and you do not have to build unrelated things. Isn't that already happening at the tip of 'master' (or 'maint'), or am I missing something? $ make V=1 git.o GIT_VERSION = 2.30.0 * new build flags * new prefix flags cc -o git.o -c -MF ./.depend/git.o.d -MQ git.o -MMD -MP -Werror -Wall -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -fno-common -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DHAVE_ALLOCA_H -DUSE_CURL_FOR_IMAP_SEND -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"' '-DGIT_HTML_PATH="share/doc/git-doc"' '-DGIT_MAN_PATH="share/man"' '-DGIT_INFO_PATH="share/info"' git.c ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-02-03 1:11 ` Junio C Hamano @ 2021-02-04 7:06 ` Jeff King 2021-02-04 17:49 ` Junio C Hamano 0 siblings, 1 reply; 39+ messages in thread From: Jeff King @ 2021-02-04 7:06 UTC (permalink / raw) To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git On Tue, Feb 02, 2021 at 05:11:47PM -0800, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > > > A re-send of v1 > > (https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com) > > + a trivial whitespace fix in 2/6. > > I'll reproduce what it said for those who are reading from > sidelines: > > As noted there I can just run "make git", which I'd somehow managed to > miss. So that complexity isn't needed. > > But Jeff King suggested a hack to just get you to the point of > git.o. I don't need that right now, but that seems sensible, so I > implemented it. > > At the start of this series I've got a patch to make "all" stop > redundantly depending on "FUZZ_OBJS", which also helps with such > "rebase -i --exec=..." use-cases. > > I do not see much point in the goal, not quite. You can do "make > git.o" and "make git" and you do not have to build unrelated things. > > Isn't that already happening at the tip of 'master' (or 'maint'), or > am I missing something? I guess I can take some of the blame since my name came up in the justification above. ;) The original use case I presented was quickly sifting through a series of commits for "oops, which one broke the compile". And so I wanted something like "make objects" to test each one as quickly as possible. And while that's a useful (if ugly) goal, I think it was misguided: - I usually know where the breakage is (after all, I saw it in the end state). So "make foo.o" would be faster still! (it would be even more so if the Makefile didn't insist on running GIT-VERSION-GEN, but that's another story). - the link step in "make git" is not that much slower than building the objects. And it catches more breakages (like undefined functions). It doesn't catch problems in non-builtins, though. - really, "make all" is not even that much slower than "make git". It's a 200ms difference on my machine (wall-clock; "make -j16" helps a lot here). That adds up if you are testing 100 commits, but it probably isn't worth thinking too hard about. I do like the cleanups earlier in the series (I have mixed feelings on the first patch, though; see my comments there). -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-02-04 7:06 ` Jeff King @ 2021-02-04 17:49 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-02-04 17:49 UTC (permalink / raw) To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, git Jeff King <peff@peff.net> writes: > On Tue, Feb 02, 2021 at 05:11:47PM -0800, Junio C Hamano wrote: > >> I do not see much point in the goal, not quite. You can do "make >> git.o" and "make git" and you do not have to build unrelated things. >> ... > I do like the cleanups earlier in the series (I have mixed feelings on > the first patch, though; see my comments there). Sorry that I forgot to say so, but even though I do not see much point in the stated goal, I do not mind the clean-up changes in the earlier parts of the series. I just didn't see much value in doing so, as a way to get to git.o (or "objects") target. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason 2021-02-03 1:11 ` Junio C Hamano @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 17:57 ` Junio C Hamano 2021-02-23 18:31 ` Jeff King 2021-02-23 11:41 ` [PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason ` (5 subsequent siblings) 7 siblings, 2 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Addresses feedback on v2: https://lore.kernel.org/git/20210201111715.10200-1-avarab@gmail.com/ Changes: - Added .PHONY targets as appropriate - Instead of removing fuzz-objs from "all" we now run it in the CI build instead. I think this accomplishes the goal of avoiding bitrot without needlessly compiling them on every build of git. As Jeff points out in https://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/ the use-case for having "{program-xdiff,test,git}-objs & objects" targets is a bit harder to justify. I still think they're useful, particularly for testing on e.g. slow single-core VMs or other test setups (I use the GCC farm) where I know I just want to compile e.g. "test" objects, and compiling one of them takes 1-2 seconds. It's an easy enough patch to carry, and now with 6/6 we even have an in-tree consumer of one of them. Ævar Arnfjörð Bjarmason (6): Makefile: guard against TEST_OBJS in the environment Makefile: split up long OBJECTS line Makefile: sort OBJECTS assignment for subsequent change Makefile: split OBJECTS into OBJECTS and GIT_OBJS Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Makefile | 38 ++++++++++++++++++++++++++++---------- ci/run-build-and-tests.sh | 1 + 2 files changed, 29 insertions(+), 10 deletions(-) Range-diff: 2: a50b68fe195 = 1: cf6d71dcf5a Makefile: guard against TEST_OBJS in the environment 3: 53656000ebe = 2: ad7ac896c09 Makefile: split up long OBJECTS line 4: d956624baea = 3: 575b2ab8e9c Makefile: sort OBJECTS assignment for subsequent change 5: 500ace9cfb4 = 4: 7fdaeb3616b Makefile: split OBJECTS into OBJECTS and GIT_OBJS 6: 8f7ce09e9bd ! 5: 765cf20c58c Makefile: add {program,xdiff,test,git}-objs & objects targets @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - Makefile: add {program,xdiff,test,git}-objs & objects targets + Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Add targets to compile the various *.o files we declared in commonly used *_OBJS variables. This is useful for debugging purposes, to @@ Commit message Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Makefile ## +@@ Makefile: ETAGS_TARGET = TAGS + FUZZ_OBJS += fuzz-commit-graph.o + FUZZ_OBJS += fuzz-pack-headers.o + FUZZ_OBJS += fuzz-pack-idx.o ++.PHONY: fuzz-objs ++fuzz-objs: $(FUZZ_OBJS) + + # Always build fuzz objects even if not testing, to prevent bit-rot. + all:: $(FUZZ_OBJS) @@ Makefile: PROGRAM_OBJS += http-backend.o PROGRAM_OBJS += imap-send.o PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAM_OBJS += shell.o ++.PHONY: program-objs +program-objs: $(PROGRAM_OBJS) # Binary suffix, set to .exe for Windows builds @@ Makefile: XDIFF_OBJS += xdiff/xmerge.o XDIFF_OBJS += xdiff/xpatience.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o ++.PHONY: xdiff-objs +xdiff-objs: $(XDIFF_OBJS) TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) ++.PHONY: test-objs +test-objs: $(TEST_OBJS) GIT_OBJS += $(LIB_OBJS) GIT_OBJS += $(BUILTIN_OBJS) GIT_OBJS += common-main.o GIT_OBJS += git.o ++.PHONY: git-objs +git-objs: $(GIT_OBJS) OBJECTS += $(GIT_OBJS) @@ Makefile: OBJECTS += $(FUZZ_OBJS) ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif ++.PHONY: objects +objects: $(OBJECTS) dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) 1: 20ec032b390 ! 6: bfedec4e5b4 Makefile: remove "all" on "$(FUZZ_OBJS)" @@ Metadata Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - Makefile: remove "all" on "$(FUZZ_OBJS)" + Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" - Adding this as a dependency was intentionally done in + Adding $(FUZZ_OBJS) as a dependency on "all" was intentionally done in 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). - I don't see why we need to prevent bitrot here under "all" for these - in particular, but not e.g. contrib/credential/**/*.c + Rather than needlessly build these objects which aren't required for + the build every time we make "all", let's instead move them to be + built by the CI jobs. - In any case, these files are rather trivial and from their commit log - it seems the fuzz-all target is run by a few people already. + The goal is to make sure that we don't inadvertently break these, we + can accomplish that goal by building them in CI, rather than slowing + down every build of git for everyone everywhere. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Makefile ## -@@ Makefile: FUZZ_OBJS += fuzz-commit-graph.o - FUZZ_OBJS += fuzz-pack-headers.o - FUZZ_OBJS += fuzz-pack-idx.o +@@ Makefile: FUZZ_OBJS += fuzz-pack-idx.o + .PHONY: fuzz-objs + fuzz-objs: $(FUZZ_OBJS) -# Always build fuzz objects even if not testing, to prevent bit-rot. -all:: $(FUZZ_OBJS) @@ Makefile: FUZZ_OBJS += fuzz-commit-graph.o FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS)) # Empty... -@@ Makefile: $(FUZZ_PROGRAMS): all +@@ Makefile: FUZZ_CXXFLAGS ?= $(CFLAGS) + + .PHONY: fuzz-all + +-$(FUZZ_PROGRAMS): all ++$(FUZZ_PROGRAMS): all fuzz-objs $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \ $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@ -fuzz-all: $(FUZZ_PROGRAMS) +fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS) + + ## ci/run-build-and-tests.sh ## +@@ ci/run-build-and-tests.sh: windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";; + *) ln -s "$cache_dir/.prove" t/.prove;; + esac + ++make fuzz-objs + make + case "$jobname" in + linux-gcc) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets 2021-02-23 11:41 ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason @ 2021-02-23 17:57 ` Junio C Hamano 2021-02-23 18:31 ` Jeff King 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-02-23 17:57 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Addresses feedback on v2: > https://lore.kernel.org/git/20210201111715.10200-1-avarab@gmail.com/ > > Changes: > > - Added .PHONY targets as appropriate > > - Instead of removing fuzz-objs from "all" we now run it in the CI > build instead. I think this accomplishes the goal of avoiding > bitrot without needlessly compiling them on every build of git. > > As Jeff points out in > https://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/ > the use-case for having "{program-xdiff,test,git}-objs & objects" > targets is a bit harder to justify. I like these for their clean-up value alone anyway, so ... ;-) Will queue. Thanks. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets 2021-02-23 11:41 ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason 2021-02-23 17:57 ` Junio C Hamano @ 2021-02-23 18:31 ` Jeff King 1 sibling, 0 replies; 39+ messages in thread From: Jeff King @ 2021-02-23 18:31 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano On Tue, Feb 23, 2021 at 12:41:26PM +0100, Ævar Arnfjörð Bjarmason wrote: > As Jeff points out in > https://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/ > the use-case for having "{program-xdiff,test,git}-objs & objects" > targets is a bit harder to justify. > > I still think they're useful, particularly for testing on e.g. slow > single-core VMs or other test setups (I use the GCC farm) where I know > I just want to compile e.g. "test" objects, and compiling one of them > takes 1-2 seconds. OK. I doubt I'll end up using them myself, but I'll keep my eyes open for opportunities. But I agree they are not creating any kind of maintenance burden, since people would be touching the FOO_OBJS lists both before and after your patches anyway. So it does not hurt to try. > Ævar Arnfjörð Bjarmason (6): > Makefile: guard against TEST_OBJS in the environment > Makefile: split up long OBJECTS line > Makefile: sort OBJECTS assignment for subsequent change > Makefile: split OBJECTS into OBJECTS and GIT_OBJS > Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets > Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" The first five all look good to me. I'm skeptical of the final one; I wrote more comments in response to that patch. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason 2021-02-03 1:11 ` Junio C Hamano 2021-02-23 11:41 ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 2/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason ` (4 subsequent siblings) 7 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add TEST_OBJS to the list of other *_OBJS variables we reset. We had already established this pattern when TEST_OBJS was introduced in daa99a91729 (Makefile: make sure test helpers are rebuilt when headers change, 2010-01-26), but it wasn't added to the list in that commit along with the rest. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 5a239cac20e..c3872f783f1 100644 --- a/Makefile +++ b/Makefile @@ -584,6 +584,7 @@ SCRIPT_PYTHON = SCRIPT_SH = SCRIPT_LIB = TEST_BUILTINS_OBJS = +TEST_OBJS = TEST_PROGRAMS_NEED_X = THIRD_PARTY_SOURCES = -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 2/6] Makefile: split up long OBJECTS line 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2021-02-23 11:41 ` [PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 7 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Split up the long OBJECTS line into multiple lines using the "+=" assignment we commonly use elsewhere in the Makefile when these lines get unwieldy. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index c3872f783f1..a5e389c9c36 100644 --- a/Makefile +++ b/Makefile @@ -576,6 +576,7 @@ EXTRA_CPPFLAGS = FUZZ_OBJS = FUZZ_PROGRAMS = LIB_OBJS = +OBJECTS = PROGRAM_OBJS = PROGRAMS = EXCLUDED_PROGRAMS = @@ -2372,11 +2373,15 @@ XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) -OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ - $(XDIFF_OBJS) \ - $(FUZZ_OBJS) \ - common-main.o \ - git.o + +OBJECTS += $(LIB_OBJS) +OBJECTS += $(BUILTIN_OBJS) +OBJECTS += $(PROGRAM_OBJS) +OBJECTS += $(TEST_OBJS) +OBJECTS += $(XDIFF_OBJS) +OBJECTS += $(FUZZ_OBJS) +OBJECTS += common-main.o +OBJECTS += git.o ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (3 preceding siblings ...) 2021-02-23 11:41 ` [PATCH v3 2/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 7 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Change the order of the OBJECTS assignment, this makes a follow-up change where we split it up into two variables smaller. 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 a5e389c9c36..6679153e24e 100644 --- a/Makefile +++ b/Makefile @@ -2376,12 +2376,12 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST OBJECTS += $(LIB_OBJS) OBJECTS += $(BUILTIN_OBJS) +OBJECTS += common-main.o +OBJECTS += git.o OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) -OBJECTS += common-main.o -OBJECTS += git.o ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (4 preceding siblings ...) 2021-02-23 11:41 ` [PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Ævar Arnfjörð Bjarmason 7 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add a new GIT_OBJS variable, with the objects sufficient to get to a git.o or common-main.o. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6679153e24e..8b087f68b3f 100644 --- a/Makefile +++ b/Makefile @@ -575,6 +575,7 @@ GENERATED_H = EXTRA_CPPFLAGS = FUZZ_OBJS = FUZZ_PROGRAMS = +GIT_OBJS = LIB_OBJS = OBJECTS = PROGRAM_OBJS = @@ -2374,10 +2375,12 @@ XDIFF_OBJS += xdiff/xutils.o TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) -OBJECTS += $(LIB_OBJS) -OBJECTS += $(BUILTIN_OBJS) -OBJECTS += common-main.o -OBJECTS += git.o +GIT_OBJS += $(LIB_OBJS) +GIT_OBJS += $(BUILTIN_OBJS) +GIT_OBJS += common-main.o +GIT_OBJS += git.o + +OBJECTS += $(GIT_OBJS) OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(XDIFF_OBJS) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (5 preceding siblings ...) 2021-02-23 11:41 ` [PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 11:41 ` [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Ævar Arnfjörð Bjarmason 7 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add targets to compile the various *.o files we declared in commonly used *_OBJS variables. This is useful for debugging purposes, to e.g. get to the point where we can compile a git.o. See [1] for a use-case for this target. https://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Makefile b/Makefile index 8b087f68b3f..10f7baa74b0 100644 --- a/Makefile +++ b/Makefile @@ -662,6 +662,8 @@ ETAGS_TARGET = TAGS FUZZ_OBJS += fuzz-commit-graph.o FUZZ_OBJS += fuzz-pack-headers.o FUZZ_OBJS += fuzz-pack-idx.o +.PHONY: fuzz-objs +fuzz-objs: $(FUZZ_OBJS) # Always build fuzz objects even if not testing, to prevent bit-rot. all:: $(FUZZ_OBJS) @@ -679,6 +681,8 @@ PROGRAM_OBJS += http-backend.o PROGRAM_OBJS += imap-send.o PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAM_OBJS += shell.o +.PHONY: program-objs +program-objs: $(PROGRAM_OBJS) # Binary suffix, set to .exe for Windows builds X = @@ -2372,13 +2376,19 @@ XDIFF_OBJS += xdiff/xmerge.o XDIFF_OBJS += xdiff/xpatience.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o +.PHONY: xdiff-objs +xdiff-objs: $(XDIFF_OBJS) TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) +.PHONY: test-objs +test-objs: $(TEST_OBJS) GIT_OBJS += $(LIB_OBJS) GIT_OBJS += $(BUILTIN_OBJS) GIT_OBJS += common-main.o GIT_OBJS += git.o +.PHONY: git-objs +git-objs: $(GIT_OBJS) OBJECTS += $(GIT_OBJS) OBJECTS += $(PROGRAM_OBJS) @@ -2388,6 +2398,8 @@ OBJECTS += $(FUZZ_OBJS) ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif +.PHONY: objects +objects: $(OBJECTS) dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason ` (6 preceding siblings ...) 2021-02-23 11:41 ` [PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 ` Ævar Arnfjörð Bjarmason 2021-02-23 18:28 ` Jeff King 7 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-23 11:41 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Adding $(FUZZ_OBJS) as a dependency on "all" was intentionally done in 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). Rather than needlessly build these objects which aren't required for the build every time we make "all", let's instead move them to be built by the CI jobs. The goal is to make sure that we don't inadvertently break these, we can accomplish that goal by building them in CI, rather than slowing down every build of git for everyone everywhere. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 7 ++----- ci/run-build-and-tests.sh | 1 + 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 10f7baa74b0..cfad590a194 100644 --- a/Makefile +++ b/Makefile @@ -665,9 +665,6 @@ FUZZ_OBJS += fuzz-pack-idx.o .PHONY: fuzz-objs fuzz-objs: $(FUZZ_OBJS) -# Always build fuzz objects even if not testing, to prevent bit-rot. -all:: $(FUZZ_OBJS) - FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS)) # Empty... @@ -3322,8 +3319,8 @@ FUZZ_CXXFLAGS ?= $(CFLAGS) .PHONY: fuzz-all -$(FUZZ_PROGRAMS): all +$(FUZZ_PROGRAMS): all fuzz-objs $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \ $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@ -fuzz-all: $(FUZZ_PROGRAMS) +fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS) diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index a66b5e8c75a..85d260476c5 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -10,6 +10,7 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";; *) ln -s "$cache_dir/.prove" t/.prove;; esac +make fuzz-objs make case "$jobname" in linux-gcc) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" 2021-02-23 11:41 ` [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Ævar Arnfjörð Bjarmason @ 2021-02-23 18:28 ` Jeff King 2021-02-23 19:19 ` Junio C Hamano 2021-02-28 20:13 ` Ævar Arnfjörð Bjarmason 0 siblings, 2 replies; 39+ messages in thread From: Jeff King @ 2021-02-23 18:28 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano On Tue, Feb 23, 2021 at 12:41:32PM +0100, Ævar Arnfjörð Bjarmason wrote: > Adding $(FUZZ_OBJS) as a dependency on "all" was intentionally done in > 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). > > Rather than needlessly build these objects which aren't required for > the build every time we make "all", let's instead move them to be > built by the CI jobs. > > The goal is to make sure that we don't inadvertently break these, we > can accomplish that goal by building them in CI, rather than slowing > down every build of git for everyone everywhere. The current state is that regular devs are responsible for avoiding compile breakages in the fuzz objects, even if they don't care themselves. Your earlier patches turned this into: regular devs are not on the hook for breaking fuzz objects; they are the responsibility of fuzz people. I'm OK with either of those, but this approach seems to me like the worst of both worlds. ;) If you do a refactor, you are still on the hook for breaking the fuzz objects because CI will fail (and you have to investigate it, and fix it for CI to remain a useful tool). But instead of finding out about the problem quickly as you're working, instead you push up what you think is a finished result, and then from minutes to hours later you get a notification telling you that oops, you missed a spot. I find that the shorter the error-fix-compile cycle is, the less time I waste waiting or context-switching. If we had a ton of fuzz object files that took forever to build, the savings on each build might be worth it. But AFAICT (from timing "make clean; make -j1" before and after), we are saving less than 1% of the build time (which is way less than the run-to-run noise). It doesn't seem like the right tradeoff to me. (Likewise, if other CI-only checks we have, like coccinelle, could be run at a similar cost, I'd recommend sticking them into the default developer build). One thing we _could_ do is stop building fuzz objects as part of "all", but include them for DEVELOPER=1 builds (which includes CI). That keeps them from hurting normal users (who don't actually need them), but prevents bitrot. It doesn't address your original motivation though (you as a developer would probably still be building them). -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" 2021-02-23 18:28 ` Jeff King @ 2021-02-23 19:19 ` Junio C Hamano 2021-02-28 20:13 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-02-23 19:19 UTC (permalink / raw) To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, git Jeff King <peff@peff.net> writes: > If you do a refactor, you are still on the hook for breaking the fuzz > objects because CI will fail (and you have to investigate it, and fix it > for CI to remain a useful tool). But instead of finding out about the > problem quickly as you're working, instead you push up what you think is > a finished result, and then from minutes to hours later you get a > notification telling you that oops, you missed a spot. I find that the > shorter the error-fix-compile cycle is, the less time I waste waiting or > context-switching. Thanks for writing this down so clearly. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" 2021-02-23 18:28 ` Jeff King 2021-02-23 19:19 ` Junio C Hamano @ 2021-02-28 20:13 ` Ævar Arnfjörð Bjarmason 2021-03-01 9:39 ` Jeff King 1 sibling, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-28 20:13 UTC (permalink / raw) To: Jeff King; +Cc: git, Junio C Hamano On Tue, Feb 23 2021, Jeff King wrote: > On Tue, Feb 23, 2021 at 12:41:32PM +0100, Ævar Arnfjörð Bjarmason wrote: > >> Adding $(FUZZ_OBJS) as a dependency on "all" was intentionally done in >> 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). >> >> Rather than needlessly build these objects which aren't required for >> the build every time we make "all", let's instead move them to be >> built by the CI jobs. >> >> The goal is to make sure that we don't inadvertently break these, we >> can accomplish that goal by building them in CI, rather than slowing >> down every build of git for everyone everywhere. > > The current state is that regular devs are responsible for avoiding > compile breakages in the fuzz objects, even if they don't care > themselves. Your earlier patches turned this into: regular devs are not > on the hook for breaking fuzz objects; they are the responsibility of > fuzz people. I'm OK with either of those, but this approach seems to me > like the worst of both worlds. ;) > > If you do a refactor, you are still on the hook for breaking the fuzz > objects because CI will fail (and you have to investigate it, and fix it > for CI to remain a useful tool). But instead of finding out about the > problem quickly as you're working, instead you push up what you think is > a finished result, and then from minutes to hours later you get a > notification telling you that oops, you missed a spot. I find that the > shorter the error-fix-compile cycle is, the less time I waste waiting or > context-switching. > > If we had a ton of fuzz object files that took forever to build, the > savings on each build might be worth it. But AFAICT (from timing "make > clean; make -j1" before and after), we are saving less than 1% of the > build time (which is way less than the run-to-run noise). > > It doesn't seem like the right tradeoff to me. (Likewise, if other > CI-only checks we have, like coccinelle, could be run at a similar cost, > I'd recommend sticking them into the default developer build). It's mainly psychological and doesn't contribute much to overall build time as a percentage, but I find it grating that the last thing I see before I switch away from that terminal when firing off a build on a slower GCC farm box I can only use -j1 on, is these fuzz objects taking 2-3 seconds to build, knowing I'm wasting time on something I'll never need. I think when we build something we should narrowly be compiling only the things we need, not running some sort of pseudo-CI on every developer's computer. We can have CI or other targets for that. Besides, if we were going for some sane cost-benefit here we'd have targets to try compiling with NO_CURL=1 or some other conditional setups that are actually common in the wild. > One thing we _could_ do is stop building fuzz objects as part of "all", > but include them for DEVELOPER=1 builds (which includes CI). That keeps > them from hurting normal users (who don't actually need them), but > prevents bitrot. It doesn't address your original motivation though (you > as a developer would probably still be building them). Please no. A very good thing about how DEVELOPER=1 works is that we're not doing anything extra except advisory compilation flags. It's turned on for "production" builds in a lot of settings because of that. It would also be very annoying to e.g. have some failure on Solaris or whatever, debug it with DEVELOPER=1, and then get some completely unrelated failure in the developer-only code, e.g. because we'd decided to compile all of fuzz/NO_OPENSSL/NO_CURL etc. and had some bug there. Yes that bug would be worthwhile to fix, but not right there and then. So having it under some "make all-combinations" flag or whatever would be better. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" 2021-02-28 20:13 ` Ævar Arnfjörð Bjarmason @ 2021-03-01 9:39 ` Jeff King 0 siblings, 0 replies; 39+ messages in thread From: Jeff King @ 2021-03-01 9:39 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano On Sun, Feb 28, 2021 at 09:13:54PM +0100, Ævar Arnfjörð Bjarmason wrote: > > The current state is that regular devs are responsible for avoiding > > compile breakages in the fuzz objects, even if they don't care > > themselves. Your earlier patches turned this into: regular devs are not > > on the hook for breaking fuzz objects; they are the responsibility of > > fuzz people. I'm OK with either of those, but this approach seems to me > > like the worst of both worlds. ;) > > > > If you do a refactor, you are still on the hook for breaking the fuzz > > objects because CI will fail (and you have to investigate it, and fix it > > for CI to remain a useful tool). But instead of finding out about the > > problem quickly as you're working, instead you push up what you think is > > a finished result, and then from minutes to hours later you get a > > notification telling you that oops, you missed a spot. I find that the > > shorter the error-fix-compile cycle is, the less time I waste waiting or > > context-switching. > > > > If we had a ton of fuzz object files that took forever to build, the > > savings on each build might be worth it. But AFAICT (from timing "make > > clean; make -j1" before and after), we are saving less than 1% of the > > build time (which is way less than the run-to-run noise). > > > > It doesn't seem like the right tradeoff to me. (Likewise, if other > > CI-only checks we have, like coccinelle, could be run at a similar cost, > > I'd recommend sticking them into the default developer build). > > It's mainly psychological and doesn't contribute much to overall build > time as a percentage, but I find it grating that the last thing I see > before I switch away from that terminal when firing off a build on a > slower GCC farm box I can only use -j1 on, is these fuzz objects taking > 2-3 seconds to build, knowing I'm wasting time on something I'll never > need. Sure, I find it annoying, too. And I am totally fine with saying "nope, let them bitrot if nobody cares enough about them to build". That is after all what happens with a bunch of stuff in compat/, or custom code like NO_PTHREADS, or with/without pcre, curl, etc. This just seems like a bad middle ground. > I think when we build something we should narrowly be compiling only the > things we need, not running some sort of pseudo-CI on every developer's > computer. We can have CI or other targets for that. > > Besides, if we were going for some sane cost-benefit here we'd have > targets to try compiling with NO_CURL=1 or some other conditional setups > that are actually common in the wild. Right. So that mostly just argues to me for not compiling them ever unless they are needed. I.e., dropping the CI part of your patch. > > One thing we _could_ do is stop building fuzz objects as part of "all", > > but include them for DEVELOPER=1 builds (which includes CI). That keeps > > them from hurting normal users (who don't actually need them), but > > prevents bitrot. It doesn't address your original motivation though (you > > as a developer would probably still be building them). > > Please no. A very good thing about how DEVELOPER=1 works is that we're > not doing anything extra except advisory compilation flags. It's turned > on for "production" builds in a lot of settings because of that. I'm not convinced that we should limit the DEVELOPER flag for this reason in general. The point of the flag is to add extra linting, including stopping the build if need be. If that is in tension with somebody using it for production builds, I would always choose improving the developer experience. That said, I don't at all care about linting the fuzz code, so I don't think it's a very compelling case. -Peff ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-04 6:51 ` Jeff King 2021-02-01 11:17 ` [PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason ` (4 subsequent siblings) 6 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Adding this as a dependency was intentionally done in 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). I don't see why we need to prevent bitrot here under "all" for these in particular, but not e.g. contrib/credential/**/*.c In any case, these files are rather trivial and from their commit log it seems the fuzz-all target is run by a few people already. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 4edfda3e009..65e600713c1 100644 --- a/Makefile +++ b/Makefile @@ -667,9 +667,6 @@ FUZZ_OBJS += fuzz-commit-graph.o FUZZ_OBJS += fuzz-pack-headers.o FUZZ_OBJS += fuzz-pack-idx.o -# Always build fuzz objects even if not testing, to prevent bit-rot. -all:: $(FUZZ_OBJS) - FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS)) # Empty... @@ -3321,4 +3318,4 @@ $(FUZZ_PROGRAMS): all $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \ $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@ -fuzz-all: $(FUZZ_PROGRAMS) +fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" 2021-02-01 11:17 ` [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason @ 2021-02-04 6:51 ` Jeff King 0 siblings, 0 replies; 39+ messages in thread From: Jeff King @ 2021-02-04 6:51 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Junio C Hamano On Mon, Feb 01, 2021 at 12:17:10PM +0100, Ævar Arnfjörð Bjarmason wrote: > Adding this as a dependency was intentionally done in > 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). > > I don't see why we need to prevent bitrot here under "all" for these > in particular, but not e.g. contrib/credential/**/*.c > > In any case, these files are rather trivial and from their commit log > it seems the fuzz-all target is run by a few people already. Part of me wants to love this commit, because I don't care about the fuzz code (since I don't run it myself[1]). But looking at "git log fuzz-*.c", I do think it will lead to bitrot. Many of those commits are things that do not care about fuzzing, but were just fixing up function interfaces as we go (e.g., my c8828530b, though see [2]). The difference between contrib/credential/ and this is that those credential helpers do not rely on the rest of the source. They are independent programs that can be built totally out of tree. So I dunno. This puts the responsibility for fixing bitrot onto the people who actually use them, which is nice. But often times it is easier for the person making the original change to just fix them up along with the others (because they understand the point of the change better, and also because a bunch of rot doesn't accrue over time). -Peff [1] I just ran "make fuzz-all", and it barfed at the link step. It looks like I need to specify a bunch of llvm stuff manually. So no, I'd guess not a lot of people are running this. :) [2] That one is particularly egregious because it fixed a copy-pasted version of a public function header. Yuck. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 3/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 6 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add TEST_OBJS to the list of other *_OBJS variables we reset. We had already established this pattern when TEST_OBJS was introduced in daa99a91729 (Makefile: make sure test helpers are rebuilt when headers change, 2010-01-26), but it wasn't added to the list in that commit along with the rest. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 65e600713c1..ce18d9ceffd 100644 --- a/Makefile +++ b/Makefile @@ -591,6 +591,7 @@ SCRIPT_PYTHON = SCRIPT_SH = SCRIPT_LIB = TEST_BUILTINS_OBJS = +TEST_OBJS = TEST_PROGRAMS_NEED_X = THIRD_PARTY_SOURCES = -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 3/6] Makefile: split up long OBJECTS line 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason ` (2 preceding siblings ...) 2021-02-01 11:17 ` [PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 6 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Split up the long OBJECTS line into multiple lines using the "+=" assignment we commonly use elsewhere in the Makefile when these lines get unwieldy. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index ce18d9ceffd..c7caa4b0ae3 100644 --- a/Makefile +++ b/Makefile @@ -583,6 +583,7 @@ EXTRA_CPPFLAGS = FUZZ_OBJS = FUZZ_PROGRAMS = LIB_OBJS = +OBJECTS = PROGRAM_OBJS = PROGRAMS = EXCLUDED_PROGRAMS = @@ -2383,11 +2384,15 @@ XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) -OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ - $(XDIFF_OBJS) \ - $(FUZZ_OBJS) \ - common-main.o \ - git.o + +OBJECTS += $(LIB_OBJS) +OBJECTS += $(BUILTIN_OBJS) +OBJECTS += $(PROGRAM_OBJS) +OBJECTS += $(TEST_OBJS) +OBJECTS += $(XDIFF_OBJS) +OBJECTS += $(FUZZ_OBJS) +OBJECTS += common-main.o +OBJECTS += git.o ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason ` (3 preceding siblings ...) 2021-02-01 11:17 ` [PATCH v2 3/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason 6 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Change the order of the OBJECTS assignment, this makes a follow-up change where we split it up into two variables smaller. 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 c7caa4b0ae3..e2e90424b62 100644 --- a/Makefile +++ b/Makefile @@ -2387,12 +2387,12 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST OBJECTS += $(LIB_OBJS) OBJECTS += $(BUILTIN_OBJS) +OBJECTS += common-main.o +OBJECTS += git.o OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) -OBJECTS += common-main.o -OBJECTS += git.o ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason ` (4 preceding siblings ...) 2021-02-01 11:17 ` [PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-01 11:17 ` [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason 6 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add a new GIT_OBJS variable, with the objects sufficient to get to a git.o or common-main.o. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index e2e90424b62..07763353423 100644 --- a/Makefile +++ b/Makefile @@ -582,6 +582,7 @@ GENERATED_H = EXTRA_CPPFLAGS = FUZZ_OBJS = FUZZ_PROGRAMS = +GIT_OBJS = LIB_OBJS = OBJECTS = PROGRAM_OBJS = @@ -2385,10 +2386,12 @@ XDIFF_OBJS += xdiff/xutils.o TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) -OBJECTS += $(LIB_OBJS) -OBJECTS += $(BUILTIN_OBJS) -OBJECTS += common-main.o -OBJECTS += git.o +GIT_OBJS += $(LIB_OBJS) +GIT_OBJS += $(BUILTIN_OBJS) +GIT_OBJS += common-main.o +GIT_OBJS += git.o + +OBJECTS += $(GIT_OBJS) OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(XDIFF_OBJS) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason ` (5 preceding siblings ...) 2021-02-01 11:17 ` [PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 ` Ævar Arnfjörð Bjarmason 2021-02-01 22:30 ` Junio C Hamano 6 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-02-01 11:17 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add targets to compile the various *.o files we declared in commonly used *_OBJS variables. This is useful for debugging purposes, to e.g. get to the point where we can compile a git.o. See [1] for a use-case for this target. https://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index 07763353423..31123639f65 100644 --- a/Makefile +++ b/Makefile @@ -683,6 +683,7 @@ PROGRAM_OBJS += http-backend.o PROGRAM_OBJS += imap-send.o PROGRAM_OBJS += sh-i18n--envsubst.o PROGRAM_OBJS += shell.o +program-objs: $(PROGRAM_OBJS) # Binary suffix, set to .exe for Windows builds X = @@ -2383,13 +2384,16 @@ XDIFF_OBJS += xdiff/xmerge.o XDIFF_OBJS += xdiff/xpatience.o XDIFF_OBJS += xdiff/xprepare.o XDIFF_OBJS += xdiff/xutils.o +xdiff-objs: $(XDIFF_OBJS) TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) +test-objs: $(TEST_OBJS) GIT_OBJS += $(LIB_OBJS) GIT_OBJS += $(BUILTIN_OBJS) GIT_OBJS += common-main.o GIT_OBJS += git.o +git-objs: $(GIT_OBJS) OBJECTS += $(GIT_OBJS) OBJECTS += $(PROGRAM_OBJS) @@ -2399,6 +2403,7 @@ OBJECTS += $(FUZZ_OBJS) ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif +objects: $(OBJECTS) dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) -- 2.30.0.284.gd98b1dd5eaa7 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets 2021-02-01 11:17 ` [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason @ 2021-02-01 22:30 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-02-01 22:30 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Add targets to compile the various *.o files we declared in commonly > used *_OBJS variables. This is useful for debugging purposes, to > e.g. get to the point where we can compile a git.o. See [1] for a > use-case for this target. > > https://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/ > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > Makefile | 5 +++++ > 1 file changed, 5 insertions(+) If you are not actually depositing files whose names are *-objs, please mark them as .PHONY: targets. > diff --git a/Makefile b/Makefile > index 07763353423..31123639f65 100644 > --- a/Makefile > +++ b/Makefile > @@ -683,6 +683,7 @@ PROGRAM_OBJS += http-backend.o > PROGRAM_OBJS += imap-send.o > PROGRAM_OBJS += sh-i18n--envsubst.o > PROGRAM_OBJS += shell.o > +program-objs: $(PROGRAM_OBJS) > > # Binary suffix, set to .exe for Windows builds > X = > @@ -2383,13 +2384,16 @@ XDIFF_OBJS += xdiff/xmerge.o > XDIFF_OBJS += xdiff/xpatience.o > XDIFF_OBJS += xdiff/xprepare.o > XDIFF_OBJS += xdiff/xutils.o > +xdiff-objs: $(XDIFF_OBJS) > > TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) > +test-objs: $(TEST_OBJS) > > GIT_OBJS += $(LIB_OBJS) > GIT_OBJS += $(BUILTIN_OBJS) > GIT_OBJS += common-main.o > GIT_OBJS += git.o > +git-objs: $(GIT_OBJS) > > OBJECTS += $(GIT_OBJS) > OBJECTS += $(PROGRAM_OBJS) > @@ -2399,6 +2403,7 @@ OBJECTS += $(FUZZ_OBJS) > ifndef NO_CURL > OBJECTS += http.o http-walker.o remote-curl.o > endif > +objects: $(OBJECTS) > > dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d) > dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS)))) ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King 2021-01-27 1:38 ` Junio C Hamano 2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason @ 2021-01-28 18:23 ` Ævar Arnfjörð Bjarmason 2021-01-28 18:23 ` [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason 3 siblings, 0 replies; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-28 18:23 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Adding this as a dependency was intentionally done in 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12). I don't see why we need to prevent bitrot here under "all" for these in particular, but not e.g. contrib/credential/**/*.c In any case, these files are rather trivial and from their commit log it seems the fuzz-all target is run by a few people already. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 4edfda3e009..65e600713c1 100644 --- a/Makefile +++ b/Makefile @@ -667,9 +667,6 @@ FUZZ_OBJS += fuzz-commit-graph.o FUZZ_OBJS += fuzz-pack-headers.o FUZZ_OBJS += fuzz-pack-idx.o -# Always build fuzz objects even if not testing, to prevent bit-rot. -all:: $(FUZZ_OBJS) - FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS)) # Empty... @@ -3321,4 +3318,4 @@ $(FUZZ_PROGRAMS): all $(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \ $(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@ -fuzz-all: $(FUZZ_PROGRAMS) +fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS) -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment 2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King ` (2 preceding siblings ...) 2021-01-28 18:23 ` [PATCH 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason @ 2021-01-28 18:23 ` Ævar Arnfjörð Bjarmason 2021-01-29 7:49 ` Junio C Hamano 3 siblings, 1 reply; 39+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-01-28 18:23 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason Add TEST_OBJS to the list of other *_OBJS variables we reset. We had already established this pattern when TEST_OBJS was introduced in daa99a91729 (Makefile: make sure test helpers are rebuilt when headers change, 2010-01-26), but it wasn't added to the list in that commit along with the rest. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 65e600713c1..63fa5c30b45 100644 --- a/Makefile +++ b/Makefile @@ -591,6 +591,7 @@ SCRIPT_PYTHON = SCRIPT_SH = SCRIPT_LIB = TEST_BUILTINS_OBJS = +TEST_OBJS = TEST_PROGRAMS_NEED_X = THIRD_PARTY_SOURCES = -- 2.29.2.222.g5d2a92d10f8 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment 2021-01-28 18:23 ` [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason @ 2021-01-29 7:49 ` Junio C Hamano 0 siblings, 0 replies; 39+ messages in thread From: Junio C Hamano @ 2021-01-29 7:49 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git, Jeff King Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Add TEST_OBJS to the list of other *_OBJS variables we reset. We had > already established this pattern when TEST_OBJS was introduced in > daa99a91729 (Makefile: make sure test helpers are rebuilt when headers > change, 2010-01-26), but it wasn't added to the list in that commit > along with the rest. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > Makefile | 1 + > 1 file changed, 1 insertion(+) Makes sense. I have only seen [1&2/6], and so far these look good. Hopefully I'll see [3-6/6] as well. > diff --git a/Makefile b/Makefile > index 65e600713c1..63fa5c30b45 100644 > --- a/Makefile > +++ b/Makefile > @@ -591,6 +591,7 @@ SCRIPT_PYTHON = > SCRIPT_SH = > SCRIPT_LIB = > TEST_BUILTINS_OBJS = > +TEST_OBJS = > TEST_PROGRAMS_NEED_X = > THIRD_PARTY_SOURCES = ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2021-03-01 9:42 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 16:07 [PATCH 0/4] Makefile: micro-optimize light non-test builds Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 1/4] Makefile: refactor assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-01-27 1:29 ` Junio C Hamano
2021-01-26 16:07 ` [PATCH 2/4] Makefile: refactor " Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag Ævar Arnfjörð Bjarmason
2021-01-26 16:07 ` [PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target Ævar Arnfjörð Bjarmason
2021-01-26 21:16 ` [PATCH 0/4] Makefile: micro-optimize light non-test builds Jeff King
2021-01-27 1:38 ` Junio C Hamano
2021-01-27 4:34 ` Jeff King
2021-01-27 6:07 ` Junio C Hamano
2021-01-28 18:23 ` [PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-01 11:17 ` [PATCH v2 " Ævar Arnfjörð Bjarmason
2021-02-03 1:11 ` Junio C Hamano
2021-02-04 7:06 ` Jeff King
2021-02-04 17:49 ` Junio C Hamano
2021-02-23 11:41 ` [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs " Ævar Arnfjörð Bjarmason
2021-02-23 17:57 ` Junio C Hamano
2021-02-23 18:31 ` Jeff King
2021-02-23 11:41 ` [PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-02-23 11:41 ` [PATCH v3 2/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason
2021-02-23 11:41 ` [PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-02-23 11:41 ` [PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason
2021-02-23 11:41 ` [PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-23 11:41 ` [PATCH v3 6/6] Makefile: build "$(FUZZ_OBJS)" in CI, not under "all" Ævar Arnfjörð Bjarmason
2021-02-23 18:28 ` Jeff King
2021-02-23 19:19 ` Junio C Hamano
2021-02-28 20:13 ` Ævar Arnfjörð Bjarmason
2021-03-01 9:39 ` Jeff King
2021-02-01 11:17 ` [PATCH v2 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason
2021-02-04 6:51 ` Jeff King
2021-02-01 11:17 ` [PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-02-01 11:17 ` [PATCH v2 3/6] Makefile: split up long OBJECTS line Ævar Arnfjörð Bjarmason
2021-02-01 11:17 ` [PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change Ævar Arnfjörð Bjarmason
2021-02-01 11:17 ` [PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS Ævar Arnfjörð Bjarmason
2021-02-01 11:17 ` [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets Ævar Arnfjörð Bjarmason
2021-02-01 22:30 ` Junio C Hamano
2021-01-28 18:23 ` [PATCH 1/6] Makefile: remove "all" on "$(FUZZ_OBJS)" Ævar Arnfjörð Bjarmason
2021-01-28 18:23 ` [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment Ævar Arnfjörð Bjarmason
2021-01-29 7:49 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).