{"thread":{"id":"55054","subject":"[PATCH 0/4] Makefile: micro-optimize light non-test builds","startedAt":"2021-01-26T16:10:04Z","lastAt":"2021-03-01T09:40:58Z","messageCount":39,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"415307","messageId":"20210126160708.20903-1-avarab@gmail.com","threadId":"55054","inReplyTo":null,"subject":"[PATCH 0/4] Makefile: micro-optimize light non-test builds","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-26T16:07:04Z","receivedAt":"2021-01-26T16:10:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This small series speeds up builds where you just want to get to a\nworking \"git\" binary, but don't care about running git's own tests, or\nabout making/installing fallbacks for \"git svn\" et al (which we do\neven with NO_PERL).\n\nÆvar Arnfjörð Bjarmason (4):\n Makefile: refactor assignment for subsequent change\n Makefile: refactor for subsequent change\n Makefile: add a NO_TEST_TOOLS flag\n Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target\n\n Makefile | 48 +++++++++++++++++++++++++++++++++++++++++++-----\n t/test-lib.sh | 5 +++++\n 2 files changed, 48 insertions(+), 5 deletions(-)\n\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415308","messageId":"20210126160708.20903-2-avarab@gmail.com","threadId":"55054","inReplyTo":"20210126160708.20903-1-avarab@gmail.com","subject":"[PATCH 1/4] Makefile: refactor assignment for subsequent change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-26T16:07:05Z","receivedAt":"2021-01-26T16:10:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor a multi-line assignment into a form that'll lend itself\nbetter to having \"ifdef\" split it up in a follow-up commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e009..36c7b8fa08b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -656,10 +656,10 @@ clean-perl-script:\n clean-python-script:\n \t$(RM) $(SCRIPT_PYTHON_GEN)\n \n-SCRIPTS = $(SCRIPT_SH_GEN) \\\n-\t $(SCRIPT_PERL_GEN) \\\n-\t $(SCRIPT_PYTHON_GEN) \\\n-\t git-instaweb\n+SCRIPTS = $(SCRIPT_SH_GEN)\n+SCRIPTS += $(SCRIPT_PERL_GEN)\n+SCRIPTS += $(SCRIPT_PYTHON_GEN)\n+SCRIPTS += git-instaweb\n \n ETAGS_TARGET = TAGS\n \n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415309","messageId":"20210126160708.20903-4-avarab@gmail.com","threadId":"55054","inReplyTo":"20210126160708.20903-1-avarab@gmail.com","subject":"[PATCH 3/4] Makefile: add a NO_TEST_TOOLS flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-26T16:07:07Z","receivedAt":"2021-01-26T16:10:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a NO_TEST_TOOLS flag to build an installable git, but one that\ncan't run \"make test\". This is useful e.g. in CI environments where\nyou'd like to run external tests against a built git, but have no\ndesire to run git's own tests.\n\nOn my 8 core machine this saves me around 1 second out of an otherwise\n11-12 second build time. So it doesn't make all the difference, but\nwhen you're wanting to run tests against a lot of git versions it adds\nup.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 6 ++++++\n t/test-lib.sh | 5 +++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 4031fb1b22f..cfa7bc58edf 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -309,6 +309,9 @@ all::\n #\n # Define NO_TCLTK if you do not want Tcl/Tk GUI.\n #\n+# Define NO_TEST_TOOLS if you'd like to skip building the assets\n+# required to run the tests. \n+#\n # Define SANE_TEXT_GREP to \"-a\" if you use recent versions of GNU grep\n # and egrep that are pickier when their input contains non-ASCII data.\n #\n@@ -2732,6 +2735,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@+\n \t@echo NO_PTHREADS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PTHREADS)))'\\' >>$@+\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n+\t@echo NO_TEST_TOOLS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_TEST_TOOLS)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n \t@echo DC_SHA1=\\''$(subst ','\\'',$(subst ','\\'',$(DC_SHA1)))'\\' >>$@+\n@@ -2787,8 +2791,10 @@ GIT-PYTHON-VARS: FORCE\n fi\n endif\n \n+ifndef NO_TEST_TOOLS\n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n TEST_TOOLS = $(TEST_PROGRAMS) $(test_bindir_programs)\n+endif\n \n all:: $(TEST_TOOLS)\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 03c1c0836f1..4029cd18031 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -55,6 +55,11 @@ then\n \texit 1\n fi\n . \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n+if test -n \"$NO_TEST_TOOLS\"\n+then\n+\techo >&2 'error: NO_TEST_TOOLS=$NO_TEST_TOOLS set in GIT-BUILD-OPTIONS, cannot run tests!.'\n+\texit 1\n+fi\n export PERL_PATH SHELL_PATH\n \n # Disallow the use of abbreviated options in the test suite by default\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415310","messageId":"20210126160708.20903-5-avarab@gmail.com","threadId":"55054","inReplyTo":"20210126160708.20903-1-avarab@gmail.com","subject":"[PATCH 4/4] Makefile: add a NO_{INSTALL_,}SCRIPT_FALLBACKS target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-26T16:07:08Z","receivedAt":"2021-01-26T16:10:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a target to skip the installation of e.g. dummy \"git-svn\" when\nNO_PERL is defined. This makes it easier to generate more minimal\ninstallations, e.g. for embedded use that's never going to care about\n\"git-svn\" not being around.\n\nWe do some basic sanity checking that e.g. NO_INSTALL_SCRIPT_FALLBACKS\nisn't set without some of NO_{PERL,PYTHON,TCLTK}, and that you don't\nset NO_INSTALL_SCRIPT_FALLBACKS without NO_SCRIPT_FALLBACKS. Otherwise\n\"make install\" would error out.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 33 ++++++++++++++++++++++++++++++++-\n 1 file changed, 32 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex cfa7bc58edf..b3c212891b6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -338,6 +338,10 @@ all::\n # when hardlinking a file to another name and unlinking the original file right\n # away (some NTFS drivers seem to zero the contents in that scenario).\n #\n+# Define NO_SCRIPT_FALLBACKS if you'd like to not generate and install\n+# the fallback scripts which defining NO_PERL and NO_PYTHON would\n+# normally produce. See also NO_INSTALL_SCRIPT_FALLBACKS.\n+#\n # Define INSTALL_SYMLINKS if you prefer to have everything that can be\n # symlinked between bin/ and libexec/ to use relative symlinks between\n # the two. This option overrides NO_CROSS_DIRECTORY_HARDLINKS and\n@@ -351,6 +355,11 @@ all::\n # Define NO_INSTALL_HARDLINKS if you prefer to use either symbolic links or\n # copies to install built-in git commands e.g. git-cat-file.\n #\n+# Define NO_INSTALL_SCRIPT_FALLBACKS to skip the installation of\n+# script fallbacks you didn't generate due to also setting\n+# NO_SCRIPT_FALLBACKS. Using this without also defining\n+# NO_SCRIPT_FALLBACKS is not supported.\n+#\n # Define SKIP_DASHED_BUILT_INS if you do not need the dashed versions of the\n # built-ins to be linked/copied at all.\n #\n@@ -603,6 +612,19 @@ THIRD_PARTY_SOURCES =\n # interactive shell sessions without exporting it.\n unexport CDPATH\n \n+# Sanity check options\n+ifdef NO_INSTALL_SCRIPT_FALLBACKS\n+ifndef NO_SCRIPT_FALLBACKS\n+$(error Setting NO_INSTALL_SCRIPT_FALLBACKS is only supported if NO_SCRIPT_FALLBACKS is also set!)\n+endif\n+endif\n+\n+ifdef NO_SCRIPT_FALLBACKS\n+ifeq (,$(NO_PERL)$(NO_PYTHON)$(NO_TCLTK))\n+$(error You should set some of NO_{PERL,PYTHON,TCLTK} when using NO_SCRIPT_FALLBACKS)\n+endif\n+endif\n+\n SCRIPT_SH += git-bisect.sh\n SCRIPT_SH += git-difftool--helper.sh\n SCRIPT_SH += git-filter-branch.sh\n@@ -659,10 +681,12 @@ clean-perl-script:\n clean-python-script:\n \t$(RM) $(SCRIPT_PYTHON_GEN)\n \n-SCRIPTS = $(SCRIPT_SH_GEN)\n+SCRIPTS = $(SCRIPT_SH_GEN)\n+ifndef NO_SCRIPT_FALLBACKS\n SCRIPTS += $(SCRIPT_PERL_GEN)\n SCRIPTS += $(SCRIPT_PYTHON_GEN)\n SCRIPTS += git-instaweb\n+endif\n \n ETAGS_TARGET = TAGS\n \n@@ -804,7 +828,9 @@ BINDIR_PROGRAMS_NEED_X += git-shell\n BINDIR_PROGRAMS_NEED_X += git-upload-archive\n BINDIR_PROGRAMS_NEED_X += git-upload-pack\n \n+ifndef NO_INSTALL_SCRIPT_FALLBACKS\n BINDIR_PROGRAMS_NO_X += git-cvsserver\n+endif\n \n # Set paths to tools early so that they can be used for version tests.\n ifndef SHELL_PATH\n@@ -2322,6 +2348,7 @@ git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PERL\n+ifndef NO_SCRIPT_FALLBACKS\n $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2329,6 +2356,7 @@ $(SCRIPT_PERL_GEN) git-instaweb: % : unimplemented.sh\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n+endif # NO_SCRIPT_FALLBACKS\n endif # NO_PERL\n \n # This makes sure we depend on the NO_PYTHON setting itself.\n@@ -2343,6 +2371,7 @@ $(SCRIPT_PYTHON_GEN): % : %.py\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n else # NO_PYTHON\n+ifndef NO_SCRIPT_FALLBACKS\n $(SCRIPT_PYTHON_GEN): % : unimplemented.sh\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n@@ -2350,6 +2379,7 @@ $(SCRIPT_PYTHON_GEN): % : unimplemented.sh\n \t unimplemented.sh >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n+endif # NO_SCRIPT_FALLBACKS\n endif # NO_PYTHON\n \n CONFIGURE_RECIPE = $(RM) configure configure.ac+ && \\\n@@ -2735,6 +2765,7 @@ GIT-BUILD-OPTIONS: FORCE\n \t@echo NO_PERL=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PERL)))'\\' >>$@+\n \t@echo NO_PTHREADS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PTHREADS)))'\\' >>$@+\n \t@echo NO_PYTHON=\\''$(subst ','\\'',$(subst ','\\'',$(NO_PYTHON)))'\\' >>$@+\n+\t@echo NO_SCRIPT_FALLBACKS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_SCRIPT_FALLBACKS)))'\\' >>$@+\n \t@echo NO_TEST_TOOLS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_TEST_TOOLS)))'\\' >>$@+\n \t@echo NO_UNIX_SOCKETS=\\''$(subst ','\\'',$(subst ','\\'',$(NO_UNIX_SOCKETS)))'\\' >>$@+\n \t@echo PAGER_ENV=\\''$(subst ','\\'',$(subst ','\\'',$(PAGER_ENV)))'\\' >>$@+\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415311","messageId":"20210126160708.20903-3-avarab@gmail.com","threadId":"55054","inReplyTo":"20210126160708.20903-1-avarab@gmail.com","subject":"[PATCH 2/4] Makefile: refactor for subsequent change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-26T16:07:06Z","receivedAt":"2021-01-26T16:10:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Refactor the list of test programs into a handy variable. This does\nnot matter now, but will make a subsequent change smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 36c7b8fa08b..4031fb1b22f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2788,8 +2788,9 @@ GIT-PYTHON-VARS: FORCE\n endif\n \n test_bindir_programs := $(patsubst %,bin-wrappers/%,$(BINDIR_PROGRAMS_NEED_X) $(BINDIR_PROGRAMS_NO_X) $(TEST_PROGRAMS_NEED_X))\n+TEST_TOOLS = $(TEST_PROGRAMS) $(test_bindir_programs)\n \n-all:: $(TEST_PROGRAMS) $(test_bindir_programs)\n+all:: $(TEST_TOOLS)\n \n bin-wrappers/%: wrap-for-bin.sh\n \t@mkdir -p bin-wrappers\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415364","messageId":"YBCGtd9if0qtuQxx@coredump.intra.peff.net","threadId":"55054","inReplyTo":"20210126160708.20903-1-avarab@gmail.com","subject":"Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-26T21:16:37Z","receivedAt":"2021-01-26T22:19:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 26, 2021 at 05:07:04PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> This small series speeds up builds where you just want to get to a\n> working \"git\" binary, but don't care about running git's own tests, or\n> about making/installing fallbacks for \"git svn\" et al (which we do\n> even with NO_PERL).\n\nI have to wonder if you really care about non-builtins here. If not,\nthen doesn't \"make git\" do what you want?\n\nI recently did something similar, but a bit more extreme. I have a\n100-patch series introducing annotations/fixes for -Wunused-parameter. I\nrebased it on master, and the end result had a compile error (a\npreviously unused and annotated parameter became used). So I wanted not\njust to fix it, but to put the fix in the right commit.\n\nDoing:\n\n git rebase -x 'make -j16'\n\nbuilds each commit and stops when we hit the breakage, which is nice.\nBut it takes a while to build, and a non-trivial bit of time is spent\ngenerating libgit.a, running the linker, making builtin hardlinks, etc.\n\nI ended up putting:\n\n objects: $(LIB_OBJS) $(BUILTIN_OBJS) git.o\n\ninto my config.mak, and then \"make objects\" is quite fast. Probably too\ngross a hack to carry in our Makefile, but I was tempted to send it.\n\n-Peff\n"},{"id":"415379","messageId":"xmqq5z3jyxa7.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"YBCGtd9if0qtuQxx@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-27T01:38:08Z","receivedAt":"2021-01-27T04:18:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Jan 26, 2021 at 05:07:04PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> This small series speeds up builds where you just want to get to a\n>> working \"git\" binary, but don't care about running git's own tests, or\n>> about making/installing fallbacks for \"git svn\" et al (which we do\n>> even with NO_PERL).\n>\n> I have to wonder if you really care about non-builtins here. If not,\n> then doesn't \"make git\" do what you want?\n\nI had the same thought, while wondering if all the ugliness in [4/4]\nis really worth it.\n\nThe steps 2/4 and 3/4 did look like a useful feature, but I wonder\nwhy we even need to introduce NO_TEST_TOOLS in the first place.\nWouldn't it be more natural to arrange them to be built by making\n\"test::\" target depend on them? IOW, why do we need to have \"all::\"\n(our default) target depend on them?\n\nAnd if we are not doing [4/4], I suspect [1/4], while it is not bad\nas a clean-up, would become less attractive.\n\nSo...\n\n"},{"id":"415380","messageId":"xmqqa6svyxo2.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"20210126160708.20903-2-avarab@gmail.com","subject":"Re: [PATCH 1/4] Makefile: refactor assignment for subsequent change","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-27T01:29:49Z","receivedAt":"2021-01-27T04:20:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Refactor a multi-line assignment into a form that'll lend itself\n> better to having \"ifdef\" split it up in a follow-up commit.\n\nHmph, it would do even better to start with an empty definition\nwithout treating the first one specially, i.e. ...\n\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Makefile | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 4edfda3e009..36c7b8fa08b 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -656,10 +656,10 @@ clean-perl-script:\n> clean-python-script:\n> \t$(RM) $(SCRIPT_PYTHON_GEN)\n> \n> -SCRIPTS = $(SCRIPT_SH_GEN) \\\n> -\t $(SCRIPT_PERL_GEN) \\\n> -\t $(SCRIPT_PYTHON_GEN) \\\n> -\t git-instaweb\n> +SCRIPTS = $(SCRIPT_SH_GEN)\n\n... if this becomes ...\n\nSCRIPTS =\nSCRIPTS += $(SCRIPT_SH_GEN)\n\n... instead, no?\n\n> +SCRIPTS += $(SCRIPT_PERL_GEN)\n> +SCRIPTS += $(SCRIPT_PYTHON_GEN)\n> +SCRIPTS += git-instaweb\n> \n> ETAGS_TARGET = TAGS\n"},{"id":"415388","messageId":"YBDtO82x5sBAs/6L@coredump.intra.peff.net","threadId":"55054","inReplyTo":"xmqq5z3jyxa7.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-01-27T04:34:03Z","receivedAt":"2021-01-27T05:42:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 26, 2021 at 05:38:08PM -0800, Junio C Hamano wrote:\n\n> The steps 2/4 and 3/4 did look like a useful feature, but I wonder\n> why we even need to introduce NO_TEST_TOOLS in the first place.\n> Wouldn't it be more natural to arrange them to be built by making\n> \"test::\" target depend on them? IOW, why do we need to have \"all::\"\n> (our default) target depend on them?\n\nHmm. That is definitely more logical, and giving \"make\" more information\nto make a good decision about what is needed. I do wonder if it would be\nannoying in two cases, though:\n\n - people trigger the tests in other ways besides \"make test\". For\n instance, \"make && cd t && make\" works, as does just\n \"make && cd t && ./t1234\". With a more clever Makefile, those would\n fail (or worse, run out-of-date versions of the helpers, producing\n confusing results).\n\n - during refactoring, I often compile-test as I go (i.e., run \"make\"\n to see which callers still need changed, then fix them, repeat).\n If that didn't catch test helpers, then I'd think I was done and get\n bit later by \"make test\" trying to build more code. Not the end of\n the world, but a minor annoyance.\n\nSo I think even though I'd argue that giving \"make\" that extra\ndependency information is \"more correct\", we are fighting uphill against\nexisting behavior, as well as things that make doesn't know (like that I\nexpect to be ready to run tests as long as \"make all\" has finished).\n\n-Peff\n"},{"id":"415391","messageId":"xmqq1re6zzec.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"YBDtO82x5sBAs/6L@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] Makefile: micro-optimize light non-test builds","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-27T06:07:07Z","receivedAt":"2021-01-27T06:15:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Jan 26, 2021 at 05:38:08PM -0800, Junio C Hamano wrote:\n>\n>> The steps 2/4 and 3/4 did look like a useful feature, but I wonder\n>> why we even need to introduce NO_TEST_TOOLS in the first place.\n>> Wouldn't it be more natural to arrange them to be built by making\n>> \"test::\" target depend on them? IOW, why do we need to have \"all::\"\n>> (our default) target depend on them?\n>\n> Hmm. That is definitely more logical, and giving \"make\" more information\n> to make a good decision about what is needed. I do wonder if it would be\n> annoying in two cases, though:\n>\n> - people trigger the tests in other ways besides \"make test\". For\n> instance, \"make && cd t && make\" works, as does just\n> \"make && cd t && ./t1234\". With a more clever Makefile, those would\n> fail (or worse, run out-of-date versions of the helpers, producing\n> confusing results).\n>\n> - during refactoring, I often compile-test as I go (i.e., run \"make\"\n> to see which callers still need changed, then fix them, repeat).\n> If that didn't catch test helpers, then I'd think I was done and get\n> bit later by \"make test\" trying to build more code. Not the end of\n> the world, but a minor annoyance.\n>\n> So I think even though I'd argue that giving \"make\" that extra\n> dependency information is \"more correct\", we are fighting uphill against\n> existing behavior, as well as things that make doesn't know (like that I\n> expect to be ready to run tests as long as \"make all\" has finished).\n\nHmph, true, but as \"make test\" at the top-level merely redirects to\n\"make -C t\", I imagined that the default target in the t/Makefile\nwould depend on doing \"make -C .. test-programs\" before running\ntests.\n\nThe recursive dependencies end somewhere, though ;-)\n\n \n"},{"id":"415518","messageId":"20210128182310.26787-1-avarab@gmail.com","threadId":"55054","inReplyTo":"YBCGtd9if0qtuQxx@coredump.intra.peff.net","subject":"[PATCH 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-28T18:23:04Z","receivedAt":"2021-01-28T18:25:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a replacement for a series I sent in\nhttps://lore.kernel.org/git/20210126160708.20903-1-avarab@gmail.com/\n\nAs noted there I can just run \"make git\", which I'd somehow managed to\nmiss. So that complexity isn't needed.\n\nBut Jeff King suggested a hack to just get you to the point of\ngit.o. I don't need that right now, but that seems sensible, so I\nimplemented it.\n\nAt the start of this series I've got a patch to make \"all\" stop\nredundantly depending on \"FUZZ_OBJS\", which also helps with such\n\"rebase -i --exec=...\" use-cases.\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"\n Makefile: guard against TEST_OBJS in the environment\n Makefile: split up long OBJECTS line\n Makefile: sort OBJECTS assignment for subsequent change\n Makefile: split OBJECTS into OBJECTS and GIT_OBJS\n Makefile: add {program,xdiff,test,git}-objs & objects targets\n\n Makefile | 29 ++++++++++++++++++++---------\n 1 file changed, 20 insertions(+), 9 deletions(-)\n\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415519","messageId":"20210128182310.26787-2-avarab@gmail.com","threadId":"55054","inReplyTo":"YBCGtd9if0qtuQxx@coredump.intra.peff.net","subject":"[PATCH 1/6] Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-28T18:23:05Z","receivedAt":"2021-01-28T18:26:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Adding this as a dependency was intentionally done in\n5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n\nI don't see why we need to prevent bitrot here under \"all\" for these\nin particular, but not e.g. contrib/credential/**/*.c\n\nIn any case, these files are rather trivial and from their commit log\nit seems the fuzz-all target is run by a few people already.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e009..65e600713c1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -667,9 +667,6 @@ FUZZ_OBJS += fuzz-commit-graph.o\n FUZZ_OBJS += fuzz-pack-headers.o\n FUZZ_OBJS += fuzz-pack-idx.o\n \n-# Always build fuzz objects even if not testing, to prevent bit-rot.\n-all:: $(FUZZ_OBJS)\n-\n FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))\n \n # Empty...\n@@ -3321,4 +3318,4 @@ $(FUZZ_PROGRAMS): all\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n-fuzz-all: $(FUZZ_PROGRAMS)\n+fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS)\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415520","messageId":"20210128182310.26787-3-avarab@gmail.com","threadId":"55054","inReplyTo":"YBCGtd9if0qtuQxx@coredump.intra.peff.net","subject":"[PATCH 2/6] Makefile: guard against TEST_OBJS in the environment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-28T18:23:06Z","receivedAt":"2021-01-28T18:26:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add TEST_OBJS to the list of other *_OBJS variables we reset. We had\nalready established this pattern when TEST_OBJS was introduced in\ndaa99a91729 (Makefile: make sure test helpers are rebuilt when headers\nchange, 2010-01-26), but it wasn't added to the list in that commit\nalong with the rest.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 65e600713c1..63fa5c30b45 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -591,6 +591,7 @@ SCRIPT_PYTHON =\n SCRIPT_SH =\n SCRIPT_LIB =\n TEST_BUILTINS_OBJS =\n+TEST_OBJS = \n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415564","messageId":"xmqqbld8tc7a.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"20210128182310.26787-3-avarab@gmail.com","subject":"Re: [PATCH 2/6] Makefile: guard against TEST_OBJS in the environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-29T07:49:13Z","receivedAt":"2021-01-29T07:51:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Add TEST_OBJS to the list of other *_OBJS variables we reset. We had\n> already established this pattern when TEST_OBJS was introduced in\n> daa99a91729 (Makefile: make sure test helpers are rebuilt when headers\n> change, 2010-01-26), but it wasn't added to the list in that commit\n> along with the rest.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Makefile | 1 +\n> 1 file changed, 1 insertion(+)\n\nMakes sense. I have only seen [1&2/6], and so far these look good.\nHopefully I'll see [3-6/6] as well.\n\n> diff --git a/Makefile b/Makefile\n> index 65e600713c1..63fa5c30b45 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -591,6 +591,7 @@ SCRIPT_PYTHON =\n> SCRIPT_SH =\n> SCRIPT_LIB =\n> TEST_BUILTINS_OBJS =\n> +TEST_OBJS = \n> TEST_PROGRAMS_NEED_X =\n> THIRD_PARTY_SOURCES =\n"},{"id":"415732","messageId":"20210201111715.10200-1-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:09Z","receivedAt":"2021-02-01T11:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A re-send of v1\n(https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com)\n+ a trivial whitespace fix in 2/6.\n\nDue to recent vger.kernel.org difficulties 4-6 never made it on-list.\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"\n Makefile: guard against TEST_OBJS in the environment\n Makefile: split up long OBJECTS line\n Makefile: sort OBJECTS assignment for subsequent change\n Makefile: split OBJECTS into OBJECTS and GIT_OBJS\n Makefile: add {program,xdiff,test,git}-objs & objects targets\n\n Makefile | 29 ++++++++++++++++++++---------\n 1 file changed, 20 insertions(+), 9 deletions(-)\n\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415733","messageId":"20210201111715.10200-2-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 1/6] Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:10Z","receivedAt":"2021-02-01T11:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Adding this as a dependency was intentionally done in\n5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n\nI don't see why we need to prevent bitrot here under \"all\" for these\nin particular, but not e.g. contrib/credential/**/*.c\n\nIn any case, these files are rather trivial and from their commit log\nit seems the fuzz-all target is run by a few people already.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e009..65e600713c1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -667,9 +667,6 @@ FUZZ_OBJS += fuzz-commit-graph.o\n FUZZ_OBJS += fuzz-pack-headers.o\n FUZZ_OBJS += fuzz-pack-idx.o\n \n-# Always build fuzz objects even if not testing, to prevent bit-rot.\n-all:: $(FUZZ_OBJS)\n-\n FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))\n \n # Empty...\n@@ -3321,4 +3318,4 @@ $(FUZZ_PROGRAMS): all\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n-fuzz-all: $(FUZZ_PROGRAMS)\n+fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS)\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415734","messageId":"20210201111715.10200-4-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 3/6] Makefile: split up long OBJECTS line","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:12Z","receivedAt":"2021-02-01T11:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Split up the long OBJECTS line into multiple lines using the \"+=\"\nassignment we commonly use elsewhere in the Makefile when these lines\nget unwieldy.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex ce18d9ceffd..c7caa4b0ae3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -583,6 +583,7 @@ EXTRA_CPPFLAGS =\n FUZZ_OBJS =\n FUZZ_PROGRAMS =\n LIB_OBJS =\n+OBJECTS =\n PROGRAM_OBJS =\n PROGRAMS =\n EXCLUDED_PROGRAMS =\n@@ -2383,11 +2384,15 @@ XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n-OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n-\t$(XDIFF_OBJS) \\\n-\t$(FUZZ_OBJS) \\\n-\tcommon-main.o \\\n-\tgit.o\n+\n+OBJECTS += $(LIB_OBJS)\n+OBJECTS += $(BUILTIN_OBJS)\n+OBJECTS += $(PROGRAM_OBJS)\n+OBJECTS += $(TEST_OBJS)\n+OBJECTS += $(XDIFF_OBJS)\n+OBJECTS += $(FUZZ_OBJS)\n+OBJECTS += common-main.o\n+OBJECTS += git.o\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415735","messageId":"20210201111715.10200-3-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 2/6] Makefile: guard against TEST_OBJS in the environment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:11Z","receivedAt":"2021-02-01T11:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add TEST_OBJS to the list of other *_OBJS variables we reset. We had\nalready established this pattern when TEST_OBJS was introduced in\ndaa99a91729 (Makefile: make sure test helpers are rebuilt when headers\nchange, 2010-01-26), but it wasn't added to the list in that commit\nalong with the rest.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 65e600713c1..ce18d9ceffd 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -591,6 +591,7 @@ SCRIPT_PYTHON =\n SCRIPT_SH =\n SCRIPT_LIB =\n TEST_BUILTINS_OBJS =\n+TEST_OBJS =\n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415736","messageId":"20210201111715.10200-5-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 4/6] Makefile: sort OBJECTS assignment for subsequent change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:13Z","receivedAt":"2021-02-01T11:18:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the order of the OBJECTS assignment, this makes a follow-up\nchange where we split it up into two variables smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c7caa4b0ae3..e2e90424b62 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2387,12 +2387,12 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST\n \n OBJECTS += $(LIB_OBJS)\n OBJECTS += $(BUILTIN_OBJS)\n+OBJECTS += common-main.o\n+OBJECTS += git.o\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n OBJECTS += $(FUZZ_OBJS)\n-OBJECTS += common-main.o\n-OBJECTS += git.o\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415737","messageId":"20210201111715.10200-7-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:15Z","receivedAt":"2021-02-01T11:18:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add targets to compile the various *.o files we declared in commonly\nused *_OBJS variables. This is useful for debugging purposes, to\ne.g. get to the point where we can compile a git.o. See [1] for a\nuse-case for this target.\n\nhttps://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 07763353423..31123639f65 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -683,6 +683,7 @@ PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += imap-send.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += shell.o\n+program-objs: $(PROGRAM_OBJS)\n \n # Binary suffix, set to .exe for Windows builds\n X =\n@@ -2383,13 +2384,16 @@ XDIFF_OBJS += xdiff/xmerge.o\n XDIFF_OBJS += xdiff/xpatience.o\n XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n+xdiff-objs: $(XDIFF_OBJS)\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+test-objs: $(TEST_OBJS)\n \n GIT_OBJS += $(LIB_OBJS)\n GIT_OBJS += $(BUILTIN_OBJS)\n GIT_OBJS += common-main.o\n GIT_OBJS += git.o\n+git-objs: $(GIT_OBJS)\n \n OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n@@ -2399,6 +2403,7 @@ OBJECTS += $(FUZZ_OBJS)\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n+objects: $(OBJECTS)\n \n dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)\n dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415738","messageId":"20210201111715.10200-6-avarab@gmail.com","threadId":"55054","inReplyTo":"20210128182310.26787-1-avarab@gmail.com","subject":"[PATCH v2 5/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-01T11:17:14Z","receivedAt":"2021-02-01T11:18:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a new GIT_OBJS variable, with the objects sufficient to get to a\ngit.o or common-main.o.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e2e90424b62..07763353423 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -582,6 +582,7 @@ GENERATED_H =\n EXTRA_CPPFLAGS =\n FUZZ_OBJS =\n FUZZ_PROGRAMS =\n+GIT_OBJS =\n LIB_OBJS =\n OBJECTS =\n PROGRAM_OBJS =\n@@ -2385,10 +2386,12 @@ XDIFF_OBJS += xdiff/xutils.o\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n \n-OBJECTS += $(LIB_OBJS)\n-OBJECTS += $(BUILTIN_OBJS)\n-OBJECTS += common-main.o\n-OBJECTS += git.o\n+GIT_OBJS += $(LIB_OBJS)\n+GIT_OBJS += $(BUILTIN_OBJS)\n+GIT_OBJS += common-main.o\n+GIT_OBJS += git.o\n+\n+OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"415838","messageId":"xmqq1rdzjuaa.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"20210201111715.10200-7-avarab@gmail.com","subject":"Re: [PATCH v2 6/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-01T22:30:05Z","receivedAt":"2021-02-01T22:31:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Add targets to compile the various *.o files we declared in commonly\n> used *_OBJS variables. This is useful for debugging purposes, to\n> e.g. get to the point where we can compile a git.o. See [1] for a\n> use-case for this target.\n>\n> https://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Makefile | 5 +++++\n> 1 file changed, 5 insertions(+)\n\nIf you are not actually depositing files whose names are *-objs,\nplease mark them as .PHONY: targets.\n\n> diff --git a/Makefile b/Makefile\n> index 07763353423..31123639f65 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -683,6 +683,7 @@ PROGRAM_OBJS += http-backend.o\n> PROGRAM_OBJS += imap-send.o\n> PROGRAM_OBJS += sh-i18n--envsubst.o\n> PROGRAM_OBJS += shell.o\n> +program-objs: $(PROGRAM_OBJS)\n> \n> # Binary suffix, set to .exe for Windows builds\n> X =\n> @@ -2383,13 +2384,16 @@ XDIFF_OBJS += xdiff/xmerge.o\n> XDIFF_OBJS += xdiff/xpatience.o\n> XDIFF_OBJS += xdiff/xprepare.o\n> XDIFF_OBJS += xdiff/xutils.o\n> +xdiff-objs: $(XDIFF_OBJS)\n> \n> TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n> +test-objs: $(TEST_OBJS)\n> \n> GIT_OBJS += $(LIB_OBJS)\n> GIT_OBJS += $(BUILTIN_OBJS)\n> GIT_OBJS += common-main.o\n> GIT_OBJS += git.o\n> +git-objs: $(GIT_OBJS)\n> \n> OBJECTS += $(GIT_OBJS)\n> OBJECTS += $(PROGRAM_OBJS)\n> @@ -2399,6 +2403,7 @@ OBJECTS += $(FUZZ_OBJS)\n> ifndef NO_CURL\n> \tOBJECTS += http.o http-walker.o remote-curl.o\n> endif\n> +objects: $(OBJECTS)\n> \n> dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)\n> dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n"},{"id":"415986","messageId":"xmqqft2edkfg.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-03T01:11:47Z","receivedAt":"2021-02-03T01:12:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> A re-send of v1\n> (https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com)\n> + a trivial whitespace fix in 2/6.\n\nI'll reproduce what it said for those who are reading from\nsidelines:\n\n As noted there I can just run \"make git\", which I'd somehow managed to\n miss. So that complexity isn't needed.\n\n But Jeff King suggested a hack to just get you to the point of\n git.o. I don't need that right now, but that seems sensible, so I\n implemented it.\n\n At the start of this series I've got a patch to make \"all\" stop\n redundantly depending on \"FUZZ_OBJS\", which also helps with such\n \"rebase -i --exec=...\" use-cases.\n\nI do not see much point in the goal, not quite. You can do \"make\ngit.o\" and \"make git\" and you do not have to build unrelated things.\n\nIsn't that already happening at the tip of 'master' (or 'maint'), or\nam I missing something?\n\n\n$ make V=1 git.o\nGIT_VERSION = 2.30.0\n * new build flags\n * new prefix flags\ncc -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\n"},{"id":"416139","messageId":"YBuZVfnc0ECuoKkK@coredump.intra.peff.net","threadId":"55054","inReplyTo":"20210201111715.10200-2-avarab@gmail.com","subject":"Re: [PATCH v2 1/6] Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-04T06:51:01Z","receivedAt":"2021-02-04T06:52:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 01, 2021 at 12:17:10PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Adding this as a dependency was intentionally done in\n> 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n> \n> I don't see why we need to prevent bitrot here under \"all\" for these\n> in particular, but not e.g. contrib/credential/**/*.c\n> \n> In any case, these files are rather trivial and from their commit log\n> it seems the fuzz-all target is run by a few people already.\n\nPart of me wants to love this commit, because I don't care about the\nfuzz code (since I don't run it myself[1]).\n\nBut looking at \"git log fuzz-*.c\", I do think it will lead to bitrot.\nMany of those commits are things that do not care about fuzzing, but\nwere just fixing up function interfaces as we go (e.g., my c8828530b,\nthough see [2]).\n\nThe difference between contrib/credential/ and this is that those\ncredential helpers do not rely on the rest of the source. They are\nindependent programs that can be built totally out of tree.\n\nSo I dunno. This puts the responsibility for fixing bitrot onto the\npeople who actually use them, which is nice. But often times it is\neasier for the person making the original change to just fix them up\nalong with the others (because they understand the point of the change\nbetter, and also because a bunch of rot doesn't accrue over time).\n\n-Peff\n\n[1] I just ran \"make fuzz-all\", and it barfed at the link step. It looks\n like I need to specify a bunch of llvm stuff manually. So no, I'd\n guess not a lot of people are running this. :)\n\n[2] That one is particularly egregious because it fixed a copy-pasted\n version of a public function header. Yuck.\n"},{"id":"416140","messageId":"YBuc5iOCCHk4fPqs@coredump.intra.peff.net","threadId":"55054","inReplyTo":"xmqqft2edkfg.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-04T07:06:14Z","receivedAt":"2021-02-04T07:07:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 02, 2021 at 05:11:47PM -0800, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > A re-send of v1\n> > (https://lore.kernel.org/git/20210128182310.26787-1-avarab@gmail.com)\n> > + a trivial whitespace fix in 2/6.\n> \n> I'll reproduce what it said for those who are reading from\n> sidelines:\n> \n> As noted there I can just run \"make git\", which I'd somehow managed to\n> miss. So that complexity isn't needed.\n> \n> But Jeff King suggested a hack to just get you to the point of\n> git.o. I don't need that right now, but that seems sensible, so I\n> implemented it.\n> \n> At the start of this series I've got a patch to make \"all\" stop\n> redundantly depending on \"FUZZ_OBJS\", which also helps with such\n> \"rebase -i --exec=...\" use-cases.\n> \n> I do not see much point in the goal, not quite. You can do \"make\n> git.o\" and \"make git\" and you do not have to build unrelated things.\n> \n> Isn't that already happening at the tip of 'master' (or 'maint'), or\n> am I missing something?\n\nI guess I can take some of the blame since my name came up in the\njustification above. ;)\n\nThe original use case I presented was quickly sifting through a series\nof commits for \"oops, which one broke the compile\". And so I wanted\nsomething like \"make objects\" to test each one as quickly as possible.\nAnd while that's a useful (if ugly) goal, I think it was misguided:\n\n - I usually know where the breakage is (after all, I saw it in the end\n state). So \"make foo.o\" would be faster still! (it would be even\n more so if the Makefile didn't insist on running GIT-VERSION-GEN,\n but that's another story).\n\n - the link step in \"make git\" is not that much slower than building\n the objects. And it catches more breakages (like undefined\n functions). It doesn't catch problems in non-builtins, though.\n\n - really, \"make all\" is not even that much slower than \"make git\".\n It's a 200ms difference on my machine (wall-clock; \"make -j16\" helps\n a lot here). That adds up if you are testing 100 commits, but it\n probably isn't worth thinking too hard about.\n\nI do like the cleanups earlier in the series (I have mixed feelings on\nthe first patch, though; see my comments there).\n\n-Peff\n"},{"id":"416154","messageId":"xmqqo8gz910s.fsf@gitster.c.googlers.com","threadId":"55054","inReplyTo":"YBuc5iOCCHk4fPqs@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/6] Makefile: add {program,xdiff,test,git}-objs & objects targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-04T17:49:07Z","receivedAt":"2021-02-04T17:56:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Feb 02, 2021 at 05:11:47PM -0800, Junio C Hamano wrote:\n>\n>> I do not see much point in the goal, not quite. You can do \"make\n>> git.o\" and \"make git\" and you do not have to build unrelated things.\n>> ...\n> I do like the cleanups earlier in the series (I have mixed feelings on\n> the first patch, though; see my comments there).\n\nSorry that I forgot to say so, but even though I do not see much\npoint in the stated goal, I do not mind the clean-up changes in the\nearlier parts of the series. I just didn't see much value in doing\nso, as a way to get to git.o (or \"objects\") target.\n\n"},{"id":"417554","messageId":"20210223114132.24345-1-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:26Z","receivedAt":"2021-02-23T11:42:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Addresses feedback on v2:\nhttps://lore.kernel.org/git/20210201111715.10200-1-avarab@gmail.com/\n\nChanges:\n\n - Added .PHONY targets as appropriate\n\n - Instead of removing fuzz-objs from \"all\" we now run it in the CI\n build instead. I think this accomplishes the goal of avoiding\n bitrot without needlessly compiling them on every build of git.\n\nAs Jeff points out in\nhttps://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/\nthe use-case for having \"{program-xdiff,test,git}-objs & objects\"\ntargets is a bit harder to justify.\n\nI still think they're useful, particularly for testing on e.g. slow\nsingle-core VMs or other test setups (I use the GCC farm) where I know\nI just want to compile e.g. \"test\" objects, and compiling one of them\ntakes 1-2 seconds.\n\nIt's an easy enough patch to carry, and now with 6/6 we even have an\nin-tree consumer of one of them.\n\nÆvar Arnfjörð Bjarmason (6):\n Makefile: guard against TEST_OBJS in the environment\n Makefile: split up long OBJECTS line\n Makefile: sort OBJECTS assignment for subsequent change\n Makefile: split OBJECTS into OBJECTS and GIT_OBJS\n Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets\n Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"\n\n Makefile | 38 ++++++++++++++++++++++++++++----------\n ci/run-build-and-tests.sh | 1 +\n 2 files changed, 29 insertions(+), 10 deletions(-)\n\nRange-diff:\n2: a50b68fe195 = 1: cf6d71dcf5a Makefile: guard against TEST_OBJS in the environment\n3: 53656000ebe = 2: ad7ac896c09 Makefile: split up long OBJECTS line\n4: d956624baea = 3: 575b2ab8e9c Makefile: sort OBJECTS assignment for subsequent change\n5: 500ace9cfb4 = 4: 7fdaeb3616b Makefile: split OBJECTS into OBJECTS and GIT_OBJS\n6: 8f7ce09e9bd ! 5: 765cf20c58c Makefile: add {program,xdiff,test,git}-objs & objects targets\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - Makefile: add {program,xdiff,test,git}-objs & objects targets\n + Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets\n \n Add targets to compile the various *.o files we declared in commonly\n used *_OBJS variables. This is useful for debugging purposes, to\n @@ Commit message\n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## Makefile ##\n +@@ Makefile: ETAGS_TARGET = TAGS\n + FUZZ_OBJS += fuzz-commit-graph.o\n + FUZZ_OBJS += fuzz-pack-headers.o\n + FUZZ_OBJS += fuzz-pack-idx.o\n ++.PHONY: fuzz-objs\n ++fuzz-objs: $(FUZZ_OBJS)\n + \n + # Always build fuzz objects even if not testing, to prevent bit-rot.\n + all:: $(FUZZ_OBJS)\n @@ Makefile: PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += imap-send.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += shell.o\n ++.PHONY: program-objs\n +program-objs: $(PROGRAM_OBJS)\n \n # Binary suffix, set to .exe for Windows builds\n @@ Makefile: XDIFF_OBJS += xdiff/xmerge.o\n XDIFF_OBJS += xdiff/xpatience.o\n XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n ++.PHONY: xdiff-objs\n +xdiff-objs: $(XDIFF_OBJS)\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n ++.PHONY: test-objs\n +test-objs: $(TEST_OBJS)\n \n GIT_OBJS += $(LIB_OBJS)\n GIT_OBJS += $(BUILTIN_OBJS)\n GIT_OBJS += common-main.o\n GIT_OBJS += git.o\n ++.PHONY: git-objs\n +git-objs: $(GIT_OBJS)\n \n OBJECTS += $(GIT_OBJS)\n @@ Makefile: OBJECTS += $(FUZZ_OBJS)\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n ++.PHONY: objects\n +objects: $(OBJECTS)\n \n dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)\n1: 20ec032b390 ! 6: bfedec4e5b4 Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - Makefile: remove \"all\" on \"$(FUZZ_OBJS)\"\n + Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"\n \n - Adding this as a dependency was intentionally done in\n + Adding $(FUZZ_OBJS) as a dependency on \"all\" was intentionally done in\n 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n \n - I don't see why we need to prevent bitrot here under \"all\" for these\n - in particular, but not e.g. contrib/credential/**/*.c\n + Rather than needlessly build these objects which aren't required for\n + the build every time we make \"all\", let's instead move them to be\n + built by the CI jobs.\n \n - In any case, these files are rather trivial and from their commit log\n - it seems the fuzz-all target is run by a few people already.\n + The goal is to make sure that we don't inadvertently break these, we\n + can accomplish that goal by building them in CI, rather than slowing\n + down every build of git for everyone everywhere.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## Makefile ##\n -@@ Makefile: FUZZ_OBJS += fuzz-commit-graph.o\n - FUZZ_OBJS += fuzz-pack-headers.o\n - FUZZ_OBJS += fuzz-pack-idx.o\n +@@ Makefile: FUZZ_OBJS += fuzz-pack-idx.o\n + .PHONY: fuzz-objs\n + fuzz-objs: $(FUZZ_OBJS)\n \n -# Always build fuzz objects even if not testing, to prevent bit-rot.\n -all:: $(FUZZ_OBJS)\n @@ Makefile: FUZZ_OBJS += fuzz-commit-graph.o\n FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))\n \n # Empty...\n -@@ Makefile: $(FUZZ_PROGRAMS): all\n +@@ Makefile: FUZZ_CXXFLAGS ?= $(CFLAGS)\n + \n + .PHONY: fuzz-all\n + \n +-$(FUZZ_PROGRAMS): all\n ++$(FUZZ_PROGRAMS): all fuzz-objs\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n -fuzz-all: $(FUZZ_PROGRAMS)\n +fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS)\n +\n + ## ci/run-build-and-tests.sh ##\n +@@ ci/run-build-and-tests.sh: windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n + *) ln -s \"$cache_dir/.prove\" t/.prove;;\n + esac\n + \n ++make fuzz-objs\n + make\n + case \"$jobname\" in\n + linux-gcc)\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417555","messageId":"20210223114132.24345-2-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 1/6] Makefile: guard against TEST_OBJS in the environment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:27Z","receivedAt":"2021-02-23T11:42:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add TEST_OBJS to the list of other *_OBJS variables we reset. We had\nalready established this pattern when TEST_OBJS was introduced in\ndaa99a91729 (Makefile: make sure test helpers are rebuilt when headers\nchange, 2010-01-26), but it wasn't added to the list in that commit\nalong with the rest.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 5a239cac20e..c3872f783f1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -584,6 +584,7 @@ SCRIPT_PYTHON =\n SCRIPT_SH =\n SCRIPT_LIB =\n TEST_BUILTINS_OBJS =\n+TEST_OBJS =\n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417556","messageId":"20210223114132.24345-3-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 2/6] Makefile: split up long OBJECTS line","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:28Z","receivedAt":"2021-02-23T11:42:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Split up the long OBJECTS line into multiple lines using the \"+=\"\nassignment we commonly use elsewhere in the Makefile when these lines\nget unwieldy.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c3872f783f1..a5e389c9c36 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -576,6 +576,7 @@ EXTRA_CPPFLAGS =\n FUZZ_OBJS =\n FUZZ_PROGRAMS =\n LIB_OBJS =\n+OBJECTS =\n PROGRAM_OBJS =\n PROGRAMS =\n EXCLUDED_PROGRAMS =\n@@ -2372,11 +2373,15 @@ XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n-OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n-\t$(XDIFF_OBJS) \\\n-\t$(FUZZ_OBJS) \\\n-\tcommon-main.o \\\n-\tgit.o\n+\n+OBJECTS += $(LIB_OBJS)\n+OBJECTS += $(BUILTIN_OBJS)\n+OBJECTS += $(PROGRAM_OBJS)\n+OBJECTS += $(TEST_OBJS)\n+OBJECTS += $(XDIFF_OBJS)\n+OBJECTS += $(FUZZ_OBJS)\n+OBJECTS += common-main.o\n+OBJECTS += git.o\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417557","messageId":"20210223114132.24345-5-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 4/6] Makefile: split OBJECTS into OBJECTS and GIT_OBJS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:30Z","receivedAt":"2021-02-23T11:42:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a new GIT_OBJS variable, with the objects sufficient to get to a\ngit.o or common-main.o.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 6679153e24e..8b087f68b3f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -575,6 +575,7 @@ GENERATED_H =\n EXTRA_CPPFLAGS =\n FUZZ_OBJS =\n FUZZ_PROGRAMS =\n+GIT_OBJS =\n LIB_OBJS =\n OBJECTS =\n PROGRAM_OBJS =\n@@ -2374,10 +2375,12 @@ XDIFF_OBJS += xdiff/xutils.o\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n \n-OBJECTS += $(LIB_OBJS)\n-OBJECTS += $(BUILTIN_OBJS)\n-OBJECTS += common-main.o\n-OBJECTS += git.o\n+GIT_OBJS += $(LIB_OBJS)\n+GIT_OBJS += $(BUILTIN_OBJS)\n+GIT_OBJS += common-main.o\n+GIT_OBJS += git.o\n+\n+OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417558","messageId":"20210223114132.24345-4-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 3/6] Makefile: sort OBJECTS assignment for subsequent change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:29Z","receivedAt":"2021-02-23T11:42:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the order of the OBJECTS assignment, this makes a follow-up\nchange where we split it up into two variables smaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a5e389c9c36..6679153e24e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2376,12 +2376,12 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST\n \n OBJECTS += $(LIB_OBJS)\n OBJECTS += $(BUILTIN_OBJS)\n+OBJECTS += common-main.o\n+OBJECTS += git.o\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n OBJECTS += $(FUZZ_OBJS)\n-OBJECTS += common-main.o\n-OBJECTS += git.o\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417559","messageId":"20210223114132.24345-6-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 5/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:31Z","receivedAt":"2021-02-23T11:43:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add targets to compile the various *.o files we declared in commonly\nused *_OBJS variables. This is useful for debugging purposes, to\ne.g. get to the point where we can compile a git.o. See [1] for a\nuse-case for this target.\n\nhttps://lore.kernel.org/git/YBCGtd9if0qtuQxx@coredump.intra.peff.net/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 8b087f68b3f..10f7baa74b0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -662,6 +662,8 @@ ETAGS_TARGET = TAGS\n FUZZ_OBJS += fuzz-commit-graph.o\n FUZZ_OBJS += fuzz-pack-headers.o\n FUZZ_OBJS += fuzz-pack-idx.o\n+.PHONY: fuzz-objs\n+fuzz-objs: $(FUZZ_OBJS)\n \n # Always build fuzz objects even if not testing, to prevent bit-rot.\n all:: $(FUZZ_OBJS)\n@@ -679,6 +681,8 @@ PROGRAM_OBJS += http-backend.o\n PROGRAM_OBJS += imap-send.o\n PROGRAM_OBJS += sh-i18n--envsubst.o\n PROGRAM_OBJS += shell.o\n+.PHONY: program-objs\n+program-objs: $(PROGRAM_OBJS)\n \n # Binary suffix, set to .exe for Windows builds\n X =\n@@ -2372,13 +2376,19 @@ XDIFF_OBJS += xdiff/xmerge.o\n XDIFF_OBJS += xdiff/xpatience.o\n XDIFF_OBJS += xdiff/xprepare.o\n XDIFF_OBJS += xdiff/xutils.o\n+.PHONY: xdiff-objs\n+xdiff-objs: $(XDIFF_OBJS)\n \n TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+.PHONY: test-objs\n+test-objs: $(TEST_OBJS)\n \n GIT_OBJS += $(LIB_OBJS)\n GIT_OBJS += $(BUILTIN_OBJS)\n GIT_OBJS += common-main.o\n GIT_OBJS += git.o\n+.PHONY: git-objs\n+git-objs: $(GIT_OBJS)\n \n OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n@@ -2388,6 +2398,8 @@ OBJECTS += $(FUZZ_OBJS)\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\n endif\n+.PHONY: objects\n+objects: $(OBJECTS)\n \n dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)\n dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417560","messageId":"20210223114132.24345-7-avarab@gmail.com","threadId":"55054","inReplyTo":"20210201111715.10200-1-avarab@gmail.com","subject":"[PATCH v3 6/6] Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-23T11:41:32Z","receivedAt":"2021-02-23T11:43:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Adding $(FUZZ_OBJS) as a dependency on \"all\" was intentionally done in\n5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n\nRather than needlessly build these objects which aren't required for\nthe build every time we make \"all\", let's instead move them to be\nbuilt by the CI jobs.\n\nThe goal is to make sure that we don't inadvertently break these, we\ncan accomplish that goal by building them in CI, rather than slowing\ndown every build of git for everyone everywhere.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 7 ++-----\n ci/run-build-and-tests.sh | 1 +\n 2 files changed, 3 insertions(+), 5 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 10f7baa74b0..cfad590a194 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -665,9 +665,6 @@ FUZZ_OBJS += fuzz-pack-idx.o\n .PHONY: fuzz-objs\n fuzz-objs: $(FUZZ_OBJS)\n \n-# Always build fuzz objects even if not testing, to prevent bit-rot.\n-all:: $(FUZZ_OBJS)\n-\n FUZZ_PROGRAMS += $(patsubst %.o,%,$(FUZZ_OBJS))\n \n # Empty...\n@@ -3322,8 +3319,8 @@ FUZZ_CXXFLAGS ?= $(CFLAGS)\n \n .PHONY: fuzz-all\n \n-$(FUZZ_PROGRAMS): all\n+$(FUZZ_PROGRAMS): all fuzz-objs\n \t$(QUIET_LINK)$(CXX) $(FUZZ_CXXFLAGS) $(LIB_OBJS) $(BUILTIN_OBJS) \\\n \t\t$(XDIFF_OBJS) $(EXTLIBS) git.o $@.o $(LIB_FUZZING_ENGINE) -o $@\n \n-fuzz-all: $(FUZZ_PROGRAMS)\n+fuzz-all: $(FUZZ_PROGRAMS) $(FUZZ_OBJS)\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex a66b5e8c75a..85d260476c5 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -10,6 +10,7 @@ windows*) cmd //c mklink //j t\\\\.prove \"$(cygpath -aw \"$cache_dir/.prove\")\";;\n *) ln -s \"$cache_dir/.prove\" t/.prove;;\n esac\n \n+make fuzz-objs\n make\n case \"$jobname\" in\n linux-gcc)\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417571","messageId":"xmqqblca4qg7.fsf@gitster.g","threadId":"55054","inReplyTo":"20210223114132.24345-1-avarab@gmail.com","subject":"Re: [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-23T17:57:44Z","receivedAt":"2021-02-23T17:59:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Addresses feedback on v2:\n> https://lore.kernel.org/git/20210201111715.10200-1-avarab@gmail.com/\n>\n> Changes:\n>\n> - Added .PHONY targets as appropriate\n>\n> - Instead of removing fuzz-objs from \"all\" we now run it in the CI\n> build instead. I think this accomplishes the goal of avoiding\n> bitrot without needlessly compiling them on every build of git.\n>\n> As Jeff points out in\n> https://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/\n> the use-case for having \"{program-xdiff,test,git}-objs & objects\"\n> targets is a bit harder to justify.\n\nI like these for their clean-up value alone anyway, so ... ;-)\n\nWill queue. Thanks.\n\n"},{"id":"417575","messageId":"YDVJZnmTiBYZ4iPM@coredump.intra.peff.net","threadId":"55054","inReplyTo":"20210223114132.24345-7-avarab@gmail.com","subject":"Re: [PATCH v3 6/6] Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-23T18:28:54Z","receivedAt":"2021-02-23T18:29:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 23, 2021 at 12:41:32PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Adding $(FUZZ_OBJS) as a dependency on \"all\" was intentionally done in\n> 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n> \n> Rather than needlessly build these objects which aren't required for\n> the build every time we make \"all\", let's instead move them to be\n> built by the CI jobs.\n> \n> The goal is to make sure that we don't inadvertently break these, we\n> can accomplish that goal by building them in CI, rather than slowing\n> down every build of git for everyone everywhere.\n\nThe current state is that regular devs are responsible for avoiding\ncompile breakages in the fuzz objects, even if they don't care\nthemselves. Your earlier patches turned this into: regular devs are not\non the hook for breaking fuzz objects; they are the responsibility of\nfuzz people. I'm OK with either of those, but this approach seems to me\nlike the worst of both worlds. ;)\n\nIf you do a refactor, you are still on the hook for breaking the fuzz\nobjects because CI will fail (and you have to investigate it, and fix it\nfor CI to remain a useful tool). But instead of finding out about the\nproblem quickly as you're working, instead you push up what you think is\na finished result, and then from minutes to hours later you get a\nnotification telling you that oops, you missed a spot. I find that the\nshorter the error-fix-compile cycle is, the less time I waste waiting or\ncontext-switching.\n\nIf we had a ton of fuzz object files that took forever to build, the\nsavings on each build might be worth it. But AFAICT (from timing \"make\nclean; make -j1\" before and after), we are saving less than 1% of the\nbuild time (which is way less than the run-to-run noise).\n\nIt doesn't seem like the right tradeoff to me. (Likewise, if other\nCI-only checks we have, like coccinelle, could be run at a similar cost,\nI'd recommend sticking them into the default developer build).\n\nOne thing we _could_ do is stop building fuzz objects as part of \"all\",\nbut include them for DEVELOPER=1 builds (which includes CI). That keeps\nthem from hurting normal users (who don't actually need them), but\nprevents bitrot. It doesn't address your original motivation though (you\nas a developer would probably still be building them).\n\n-Peff\n"},{"id":"417576","messageId":"YDVJ9wumbORxTmJ5@coredump.intra.peff.net","threadId":"55054","inReplyTo":"20210223114132.24345-1-avarab@gmail.com","subject":"Re: [PATCH v3 0/6] Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-23T18:31:19Z","receivedAt":"2021-02-23T18:32:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 23, 2021 at 12:41:26PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> As Jeff points out in\n> https://lore.kernel.org/git/YBuc5iOCCHk4fPqs@coredump.intra.peff.net/\n> the use-case for having \"{program-xdiff,test,git}-objs & objects\"\n> targets is a bit harder to justify.\n> \n> I still think they're useful, particularly for testing on e.g. slow\n> single-core VMs or other test setups (I use the GCC farm) where I know\n> I just want to compile e.g. \"test\" objects, and compiling one of them\n> takes 1-2 seconds.\n\nOK. I doubt I'll end up using them myself, but I'll keep my eyes open\nfor opportunities. But I agree they are not creating any kind of\nmaintenance burden, since people would be touching the FOO_OBJS lists\nboth before and after your patches anyway. So it does not hurt to try.\n\n> Ævar Arnfjörð Bjarmason (6):\n> Makefile: guard against TEST_OBJS in the environment\n> Makefile: split up long OBJECTS line\n> Makefile: sort OBJECTS assignment for subsequent change\n> Makefile: split OBJECTS into OBJECTS and GIT_OBJS\n> Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets\n> Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"\n\nThe first five all look good to me. I'm skeptical of the final one; I\nwrote more comments in response to that patch.\n\n-Peff\n"},{"id":"417584","messageId":"xmqq35xm383m.fsf@gitster.g","threadId":"55054","inReplyTo":"YDVJZnmTiBYZ4iPM@coredump.intra.peff.net","subject":"Re: [PATCH v3 6/6] Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-23T19:19:25Z","receivedAt":"2021-02-23T19:22:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> If you do a refactor, you are still on the hook for breaking the fuzz\n> objects because CI will fail (and you have to investigate it, and fix it\n> for CI to remain a useful tool). But instead of finding out about the\n> problem quickly as you're working, instead you push up what you think is\n> a finished result, and then from minutes to hours later you get a\n> notification telling you that oops, you missed a spot. I find that the\n> shorter the error-fix-compile cycle is, the less time I waste waiting or\n> context-switching.\n\nThanks for writing this down so clearly.\n"},{"id":"417997","messageId":"87wnusj6gt.fsf@evledraar.gmail.com","threadId":"55054","inReplyTo":"YDVJZnmTiBYZ4iPM@coredump.intra.peff.net","subject":"Re: [PATCH v3 6/6] Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-28T20:13:54Z","receivedAt":"2021-02-28T20:14:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 23 2021, Jeff King wrote:\n\n> On Tue, Feb 23, 2021 at 12:41:32PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Adding $(FUZZ_OBJS) as a dependency on \"all\" was intentionally done in\n>> 5e472150800 (fuzz: add basic fuzz testing target., 2018-10-12).\n>> \n>> Rather than needlessly build these objects which aren't required for\n>> the build every time we make \"all\", let's instead move them to be\n>> built by the CI jobs.\n>> \n>> The goal is to make sure that we don't inadvertently break these, we\n>> can accomplish that goal by building them in CI, rather than slowing\n>> down every build of git for everyone everywhere.\n>\n> The current state is that regular devs are responsible for avoiding\n> compile breakages in the fuzz objects, even if they don't care\n> themselves. Your earlier patches turned this into: regular devs are not\n> on the hook for breaking fuzz objects; they are the responsibility of\n> fuzz people. I'm OK with either of those, but this approach seems to me\n> like the worst of both worlds. ;)\n>\n> If you do a refactor, you are still on the hook for breaking the fuzz\n> objects because CI will fail (and you have to investigate it, and fix it\n> for CI to remain a useful tool). But instead of finding out about the\n> problem quickly as you're working, instead you push up what you think is\n> a finished result, and then from minutes to hours later you get a\n> notification telling you that oops, you missed a spot. I find that the\n> shorter the error-fix-compile cycle is, the less time I waste waiting or\n> context-switching.\n>\n> If we had a ton of fuzz object files that took forever to build, the\n> savings on each build might be worth it. But AFAICT (from timing \"make\n> clean; make -j1\" before and after), we are saving less than 1% of the\n> build time (which is way less than the run-to-run noise).\n>\n> It doesn't seem like the right tradeoff to me. (Likewise, if other\n> CI-only checks we have, like coccinelle, could be run at a similar cost,\n> I'd recommend sticking them into the default developer build).\n\nIt's mainly psychological and doesn't contribute much to overall build\ntime as a percentage, but I find it grating that the last thing I see\nbefore I switch away from that terminal when firing off a build on a\nslower GCC farm box I can only use -j1 on, is these fuzz objects taking\n2-3 seconds to build, knowing I'm wasting time on something I'll never\nneed.\n\nI think when we build something we should narrowly be compiling only the\nthings we need, not running some sort of pseudo-CI on every developer's\ncomputer. We can have CI or other targets for that.\n\nBesides, if we were going for some sane cost-benefit here we'd have\ntargets to try compiling with NO_CURL=1 or some other conditional setups\nthat are actually common in the wild.\n\n> One thing we _could_ do is stop building fuzz objects as part of \"all\",\n> but include them for DEVELOPER=1 builds (which includes CI). That keeps\n> them from hurting normal users (who don't actually need them), but\n> prevents bitrot. It doesn't address your original motivation though (you\n> as a developer would probably still be building them).\n\nPlease no. A very good thing about how DEVELOPER=1 works is that we're\nnot doing anything extra except advisory compilation flags. It's turned\non for \"production\" builds in a lot of settings because of that.\n\nIt would also be very annoying to e.g. have some failure on Solaris or\nwhatever, debug it with DEVELOPER=1, and then get some completely\nunrelated failure in the developer-only code, e.g. because we'd decided\nto compile all of fuzz/NO_OPENSSL/NO_CURL etc. and had some bug there.\n\nYes that bug would be worthwhile to fix, but not right there and\nthen. So having it under some \"make all-combinations\" flag or whatever\nwould be better.\n"},{"id":"418019","messageId":"YDy2WMMg4asR8E4B@coredump.intra.peff.net","threadId":"55054","inReplyTo":"87wnusj6gt.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3 6/6] Makefile: build \"$(FUZZ_OBJS)\" in CI, not under \"all\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-01T09:39:36Z","receivedAt":"2021-03-01T09:40:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 28, 2021 at 09:13:54PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > The current state is that regular devs are responsible for avoiding\n> > compile breakages in the fuzz objects, even if they don't care\n> > themselves. Your earlier patches turned this into: regular devs are not\n> > on the hook for breaking fuzz objects; they are the responsibility of\n> > fuzz people. I'm OK with either of those, but this approach seems to me\n> > like the worst of both worlds. ;)\n> >\n> > If you do a refactor, you are still on the hook for breaking the fuzz\n> > objects because CI will fail (and you have to investigate it, and fix it\n> > for CI to remain a useful tool). But instead of finding out about the\n> > problem quickly as you're working, instead you push up what you think is\n> > a finished result, and then from minutes to hours later you get a\n> > notification telling you that oops, you missed a spot. I find that the\n> > shorter the error-fix-compile cycle is, the less time I waste waiting or\n> > context-switching.\n> >\n> > If we had a ton of fuzz object files that took forever to build, the\n> > savings on each build might be worth it. But AFAICT (from timing \"make\n> > clean; make -j1\" before and after), we are saving less than 1% of the\n> > build time (which is way less than the run-to-run noise).\n> >\n> > It doesn't seem like the right tradeoff to me. (Likewise, if other\n> > CI-only checks we have, like coccinelle, could be run at a similar cost,\n> > I'd recommend sticking them into the default developer build).\n> \n> It's mainly psychological and doesn't contribute much to overall build\n> time as a percentage, but I find it grating that the last thing I see\n> before I switch away from that terminal when firing off a build on a\n> slower GCC farm box I can only use -j1 on, is these fuzz objects taking\n> 2-3 seconds to build, knowing I'm wasting time on something I'll never\n> need.\n\nSure, I find it annoying, too. And I am totally fine with saying \"nope,\nlet them bitrot if nobody cares enough about them to build\". That is\nafter all what happens with a bunch of stuff in compat/, or custom code\nlike NO_PTHREADS, or with/without pcre, curl, etc.\n\nThis just seems like a bad middle ground.\n\n> I think when we build something we should narrowly be compiling only the\n> things we need, not running some sort of pseudo-CI on every developer's\n> computer. We can have CI or other targets for that.\n> \n> Besides, if we were going for some sane cost-benefit here we'd have\n> targets to try compiling with NO_CURL=1 or some other conditional setups\n> that are actually common in the wild.\n\nRight. So that mostly just argues to me for not compiling them ever\nunless they are needed. I.e., dropping the CI part of your patch.\n\n> > One thing we _could_ do is stop building fuzz objects as part of \"all\",\n> > but include them for DEVELOPER=1 builds (which includes CI). That keeps\n> > them from hurting normal users (who don't actually need them), but\n> > prevents bitrot. It doesn't address your original motivation though (you\n> > as a developer would probably still be building them).\n> \n> Please no. A very good thing about how DEVELOPER=1 works is that we're\n> not doing anything extra except advisory compilation flags. It's turned\n> on for \"production\" builds in a lot of settings because of that.\n\nI'm not convinced that we should limit the DEVELOPER flag for this\nreason in general. The point of the flag is to add extra linting,\nincluding stopping the build if need be. If that is in tension with\nsomebody using it for production builds, I would always choose improving\nthe developer experience.\n\nThat said, I don't at all care about linting the fuzz code, so I don't\nthink it's a very compelling case.\n\n-Peff\n"}]}