{"thread":{"id":"56561","subject":"[PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","startedAt":"2021-09-21T22:55:21Z","lastAt":"2021-09-28T19:51:43Z","messageCount":28,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"436683","messageId":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","threadId":"56561","inReplyTo":null,"subject":"[PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-21T22:55:12Z","receivedAt":"2021-09-21T22:55:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Now that my series to only build \"TAGS\" when we strictly need to has\nlanded in 1b8bd2243e7 (Merge branch 'ab/make-tags-cleanup',\n2021-09-20), let's do the same for the \"sparse\" and \"hdr-check\"\ntargets.\n\nFor *.c files we'll now generate corresponding empty *.sp and *.hco\nfiles when \"sparse\" and \"hdr-check\" are run, respectively. If either\nof those errored on the *.c file we'd fail to refresh the\ncorresponding generated file.\n\nPut together a:\n\n make -j8 all TAGS sparse hdr-check\n\nTakes around 15s on \"master\" when there's nothing new to do (we re-do\nall of \"sparse hdr-check\"), now it'll take <100ms if there's nothing\nto do, and say ~2s if I do a \"touch ref*.[ch]\".\n\nÆvar Arnfjörð Bjarmason (3):\n Makefile: make the \"sparse\" target non-.PHONY\n Makefile: do one append in %.hcc rule\n Makefile: make the \"hdr-check\" target non-.PHONY\n\n .gitignore | 2 ++\n Makefile | 21 +++++++++++++--------\n 2 files changed, 15 insertions(+), 8 deletions(-)\n\n-- \n2.33.0.1098.gf02a64c1a2d\n\n"},{"id":"436684","messageId":"patch-1.3-e03fde1b642-20210921T224944Z-avarab@gmail.com","threadId":"56561","inReplyTo":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","subject":"[PATCH 1/3] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-21T22:55:13Z","receivedAt":"2021-09-21T22:55:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"sparse\" target and its *.sp dependencies to be\nnon-.PHONY. It's now viable to run it as part of a normal compilation\ntarget, as we'll only re-generate these checks if the source *.c file\nhas changed.\n\nOn my box with -j8 it was fast before, or around 5 seconds, now it\nonly takes that long the first time, and the common case is <100ms, or\nhowever long it takes GNU make to stat the *.sp file and see that all\nthe corresponding *.c files are older.\n\nSee 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\nvariables, 2011-04-21) for the modern implementation of the sparse\ntarget being changed here.\n\nIt is critical that we use -Wsparse-error here, otherwise the error\nwould only show up once, but we'd successfully create the empty *.sp\nfile, and running a second time wouldn't show the error. I'm therefore\nnot putting it into SPARSE_FLAGS or SP_EXTRA_FLAGS, it's not optional,\nthe Makefile logic won't behave properly without it.\n\nAppending to $@ without a move is OK here because we're using the\n.DELETE_ON_ERROR Makefile feature. See 7b76d6bf221 (Makefile: add and\nuse the \".DELETE_ON_ERROR\" flag, 2021-06-29). GNU make ensures that on\nerror this file will be removed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Makefile | 8 +++++---\n 2 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..b02250a50c4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -224,6 +224,7 @@\n *.lib\n *.res\n *.sln\n+*.sp\n *.suo\n *.ncb\n *.vcproj\ndiff --git a/Makefile b/Makefile\nindex a9f9b689f0c..5b09f67aab0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2896,11 +2896,13 @@ check-sha1:: t/helper/test-tool$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+$(SP_OBJ): %.sp: %.c GIT-CFLAGS\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n-\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n+\t\t-Wsparse-error \\\n+\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< && \\\n+\t>$@\n \n-.PHONY: sparse $(SP_OBJ)\n+.PHONY: sparse\n sparse: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n-- \n2.33.0.1098.gf02a64c1a2d\n\n"},{"id":"436685","messageId":"patch-2.3-9c4cedacdce-20210921T224944Z-avarab@gmail.com","threadId":"56561","inReplyTo":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","subject":"[PATCH 2/3] Makefile: do one append in %.hcc rule","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-21T22:55:14Z","receivedAt":"2021-09-21T22:55:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a rule added in added b503a2d515e (Makefile: emulate compile in\n$(HCO) target better, 2019-09-25) to use one append, instead of \">\"\nfollowed by a \">>\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5b09f67aab0..ce4063a031a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2914,8 +2914,10 @@ HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))\n HCC = $(HCO:hco=hcc)\n \n %.hcc: %.h\n-\t@echo '#include \"git-compat-util.h\"' >$@\n-\t@echo '#include \"$<\"' >>$@\n+\t@{ \\\n+\t\techo '#include \"git-compat-util.h\"'; \\\n+\t\techo '#include \"$<\"'; \\\n+\t} >$@\n \n $(HCO): %.hco: %.hcc FORCE\n \t$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<\n-- \n2.33.0.1098.gf02a64c1a2d\n\n"},{"id":"436686","messageId":"patch-3.3-e2ad1700f9e-20210921T224944Z-avarab@gmail.com","threadId":"56561","inReplyTo":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","subject":"[PATCH 3/3] Makefile: make the \"hdr-check\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-21T22:55:15Z","receivedAt":"2021-09-21T22:55:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"For the same reason as the preceding commit (which made \"sparse\"\nnon-.PHONY), let's make the \"hdr-check\" target non-.PHONY.\n\nWe could also change this to do \"-o $@\" instead of creating an empty\nfile, but there would be no point, and it would just waste I/O and\ndisk space, we're not interested in the compilation output, just\nwhether or not we had an error.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Makefile | 7 ++++---\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex b02250a50c4..4579985e2f4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -220,6 +220,7 @@\n /cscope*\n /compile_commands.json\n *.hcc\n+*.hco\n *.obj\n *.lib\n *.res\ndiff --git a/Makefile b/Makefile\nindex ce4063a031a..7980e69955a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2919,10 +2919,11 @@ HCC = $(HCO:hco=hcc)\n \t\techo '#include \"$<\"'; \\\n \t} >$@\n \n-$(HCO): %.hco: %.hcc FORCE\n-\t$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<\n+$(HCO): %.hco: %.hcc\n+\t$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $< && \\\n+\t>$@\n \n-.PHONY: hdr-check $(HCO)\n+.PHONY: hdr-check\n hdr-check: $(HCO)\n \n .PHONY: style\n-- \n2.33.0.1098.gf02a64c1a2d\n\n"},{"id":"436702","messageId":"YUqQzn5vFDpbF5dM@coredump.intra.peff.net","threadId":"56561","inReplyTo":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-22T02:11:26Z","receivedAt":"2021-09-22T02:11:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 22, 2021 at 12:55:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Now that my series to only build \"TAGS\" when we strictly need to has\n> landed in 1b8bd2243e7 (Merge branch 'ab/make-tags-cleanup',\n> 2021-09-20), let's do the same for the \"sparse\" and \"hdr-check\"\n> targets.\n> \n> For *.c files we'll now generate corresponding empty *.sp and *.hco\n> files when \"sparse\" and \"hdr-check\" are run, respectively. If either\n> of those errored on the *.c file we'd fail to refresh the\n> corresponding generated file.\n\nAll three seem pretty reasonable to me.\n\nThough could we be confused in the sparse rule by a header file that\nchanged? The object files depend on the auto-computed dependencies or on\nLIB_H, but the sparse rule doesn't. So, with your patch:\n\n $ echo '/* ok */' >>git-compat-util.h\n $ make sparse\n [lots of output, everything ok]\n\n $ echo 'not ok' >>git-compat-util.h\n $ make sparse\n [no output; nothing is run]\n\n $ touch git.c\n $ make sparse\n git.c: note: in included file (through builtin.h):\n git-compat-util.h:1382:1: error: 'not' has implicit type\n git-compat-util.h:1382:5: error: Expected ; at end of declaration\n [...etc...]\n\nI think it's hard to use the computed dependencies here, because they're\nwritten by the compiler with explicit \".o\" targets. But we could either:\n\n 1. make them all depend on LIB_H. That's overly broad, but still\n better than the status quo; or\n\n 2. have \"foo.sp\" depend on \"foo.o\". That requires you to build things\n before doing sparse checks, but in practice most people would\n presumably _also_ be compiling anyway, I'd think.\n\nI.e., this works for me (the second \"make sparse\" in my example above\nrebuilds and shows the errors):\n\ndiff --git a/Makefile b/Makefile\nindex e44eb4a62a..a97e52eb19 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2903,7 +2903,7 @@ check-sha1:: t/helper/test-tool$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS\n+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n \t\t-Wsparse-error \\\n \t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< && \\\n\n-Peff\n"},{"id":"436704","messageId":"YUqTyJQ8euArYeVn@coredump.intra.peff.net","threadId":"56561","inReplyTo":"patch-1.3-e03fde1b642-20210921T224944Z-avarab@gmail.com","subject":"Re: [PATCH 1/3] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-22T02:24:08Z","receivedAt":"2021-09-22T02:24:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 22, 2021 at 12:55:13AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> On my box with -j8 it was fast before, or around 5 seconds, now it\n> only takes that long the first time, and the common case is <100ms, or\n> however long it takes GNU make to stat the *.sp file and see that all\n> the corresponding *.c files are older.\n\nSort of side note, but I think the main culprit here is all of the\n$(shell) invocations, etc, we run. Try:\n\n make SHELL_PATH='sh -x'\n\nto get a taste (do it twice, because the first one will actually rebuild\neverything due to the changed shell; the second one _should_ be a noop\nbut still runs a bunch of stuff).\n\nThough even that doesn't tell the whole story, as it doesn't make clear\nwhich shells are invoked by make directly. Try this:\n\n\tcat >/tmp/foo <<-\\EOF\n\t#!/bin/sh\n\techo >&2 \"==> shell $*\"\n\texec sh \"$@\"\n\tEOF\n\n\tmake SHELL_PATH=/tmp/foo\n\nThere's lots of low-hanging fruit like:\n\n==> shell -c echo TEST_SHELL_PATH=\\''/tmp/foo'\\' >>GIT-BUILD-OPTIONS+\n==> shell -c echo PERL_PATH=\\''/usr/bin/perl'\\' >>GIT-BUILD-OPTIONS+\n==> shell -c echo DIFF=\\''diff'\\' >>GIT-BUILD-OPTIONS+\n==> shell -c echo PYTHON_PATH=\\''/usr/bin/python'\\' >>GIT-BUILD-OPTIONS+\n[...and over a dozen more...]\n\nThose could easily be a single shell invocation, rather than one per\necho. Another culprit is GIT-VERSION-GEN, which we run whether it's\nneeded or not, and takes something like 25ms.\n\nIt's probably not worth spending too much time micro-optimizing here\n(and certainly it's orthogonal to your series), but there may be some\nlow-hanging fruit (although a hacky attempt at minimizing the shell\ncalls for GIT-BUILD-OPTIONS didn't seem to show any speedup, so maybe\nit's not so low-hanging after all).\n\n-Peff\n"},{"id":"436734","messageId":"c13b050c-abb9-c669-b92c-930f2b43ce10@ramsayjones.plus.com","threadId":"56561","inReplyTo":"YUqQzn5vFDpbF5dM@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-09-22T16:58:16Z","receivedAt":"2021-09-22T16:58:22Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 22/09/2021 03:11, Jeff King wrote:\n> On Wed, Sep 22, 2021 at 12:55:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n>> Now that my series to only build \"TAGS\" when we strictly need to has\n>> landed in 1b8bd2243e7 (Merge branch 'ab/make-tags-cleanup',\n>> 2021-09-20), let's do the same for the \"sparse\" and \"hdr-check\"\n>> targets.\n>>\n>> For *.c files we'll now generate corresponding empty *.sp and *.hco\n>> files when \"sparse\" and \"hdr-check\" are run, respectively. If either\n>> of those errored on the *.c file we'd fail to refresh the\n>> corresponding generated file.\n> \n> All three seem pretty reasonable to me.\n\nHeh, interesting. My initial reaction was completely negative! ;-P\n(and not just mildly negative either, but 'you must be kidding').\n\nHowever, I then thought 'I must be missing something, I'm being\nstupid and about to embarrass myself in public!'. So, I have\nbeen trying hard to understand what these patches are trying to\naccomplish and just what it is I'm missing. But, I'm coming up\nblank ...\n\nAt the heart of my unease is dependencies (or rather the lack) for\nthe 'synthetic object files' *.hco and *.sp. (Also, the addition\nof even more 'shrapnel' to the build directories - I wrote a patch\nto remove the useless *.hcc files just after commit b503a2d515e was\nincluded, but didn't get around to submitting it).\n\nSo, lets try something:\n\n $ make hdr-check\n GIT_VERSION = 2.33.0.517.g53f5cfaf01\n HDR add-interactive.h\n ...\n HDR xdiff-interface.h\n $ \n\nOK, that seems to work.\n \n $ find . -iname '*.hcc' | wc -l\n 208\n $ find . -iname '*.hco' | wc -l\n 200\n $ \n\nHmm, odd:\n \n $ find . -iname '*.hcc' | sed s/.hcc// | sort >zz\n $ find . -iname '*.hco' | sed s/.hco// | sort >xx\n $ diff zz xx\n 90d89\n < ./merge-strategies\n 137d135\n < ./reftable/slice\n 152d149\n < ./sha1-lookup\n 198,202d194\n < ./vcs-svn/fast_export\n < ./vcs-svn/line_buffer\n < ./vcs-svn/sliding_window\n < ./vcs-svn/svndiff\n < ./vcs-svn/svndump\n $ \n\n... just noticed in passing, I didn't investigate.\n\nNow, by definition, every '*.hcc' file depends on git-compat-util.h, so\nafter changing that header an 'hdr-check' should check every header:\n\n $ touch git-compat-util.h\n $ make hdr-check\n HDR git-compat-util.h\n $ \n\nHmm, disappointing! Similarly, if I change (say) 'cache.h', then all\nthe headers that '#include' that file, in addition to 'cache.h', should\nalso be checked:\n \n $ git grep -n 'include.*cache\\.h' -- '*.h' | wc -l\n 35\n $ touch cache.h\n $ make hdr-check\n HDR cache.h\n $ \n\nHmm, not quite. So, the sparse target should have similar problems:\n \n $ make sparse\n * new build flags\n SP abspath.c\n ...\n SP remote-curl.c\n $ \n\nOK, that works.\n \n $ find . -iname '*.sp' | wc -l\n 452\n $ \n \n $ make sparse\n $ touch git-compat-util.h\n $ make sparse\n $ touch git.h\n $ make sparse\n $ touch git.c\n $ make sparse\n SP git.c\n $ \n \n $ make clean\n ...\n rm -f GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS\n $ find . -iname '*.sp' | wc -l\n 452\n $ \n \nAh, yes, you may want to add the removal of the 'synthetic objects' to the\nmake clean target!\n\nAs I said, I don't quite understand what these patches want to do, so I can't\noffer any solutions. :( Well, you could *add* the necessary dependencies,\nof course, but that could lead to a rabbit hole which I would not want to\ngo down!\n\nATB,\nRamsay Jones\n\n \n\n"},{"id":"436743","messageId":"YUttpgkU6eCOxMj/@coredump.intra.peff.net","threadId":"56561","inReplyTo":"c13b050c-abb9-c669-b92c-930f2b43ce10@ramsayjones.plus.com","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-22T17:53:42Z","receivedAt":"2021-09-22T17:53:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 22, 2021 at 05:58:16PM +0100, Ramsay Jones wrote:\n\n> > All three seem pretty reasonable to me.\n> \n> Heh, interesting. My initial reaction was completely negative! ;-P\n> (and not just mildly negative either, but 'you must be kidding').\n> \n> However, I then thought 'I must be missing something, I'm being\n> stupid and about to embarrass myself in public!'. So, I have\n> been trying hard to understand what these patches are trying to\n> accomplish and just what it is I'm missing. But, I'm coming up\n> blank ...\n\nI think the point is just avoiding repeated work. If you just manually\nrun \"make sparse\" once in a while, then caching the result probably\nisn't of much value. But if you plan to run, say:\n\n git rebase -x 'make sparse'\n\nthen it would be nice for it to avoid checking the same files over and\nover.\n\n> At the heart of my unease is dependencies (or rather the lack) for\n> the 'synthetic object files' *.hco and *.sp. (Also, the addition\n> of even more 'shrapnel' to the build directories - I wrote a patch\n> to remove the useless *.hcc files just after commit b503a2d515e was\n> included, but didn't get around to submitting it).\n\nI don't consider them shrapnel if they're holding useful results. :) But\nI agree that this does make \"ls\" in the top-level increasingly cluttered\n(curiously, I find that's something I rarely do, but probably because\nI'm so used to knowing where everything is in this project).\n\nPerhaps writing them to build-cruft/%.hco instead would help there. I\nguess there may be some complications around directory creation.\n\nBut overall, I do agree that if we can't make the dependencies solid\nhere, this is not worth doing. Sacrificing correctness of the checks for\nreduced computation is not a good idea.\n\n> Hmm, odd:\n> \n> $ find . -iname '*.hcc' | sed s/.hcc// | sort >zz\n> $ find . -iname '*.hco' | sed s/.hco// | sort >xx\n> $ diff zz xx\n> 90d89\n> < ./merge-strategies\n> 137d135\n> < ./reftable/slice\n> 152d149\n> < ./sha1-lookup\n> 198,202d194\n> < ./vcs-svn/fast_export\n> < ./vcs-svn/line_buffer\n> < ./vcs-svn/sliding_window\n> < ./vcs-svn/svndiff\n> < ./vcs-svn/svndump\n> $ \n> \n> ... just noticed in passing, I didn't investigate.\n\nI think some of that is cruft from old runs. We no longer have\nvcs-svn/*.c at all, for example.\n\n> Now, by definition, every '*.hcc' file depends on git-compat-util.h, so\n> after changing that header an 'hdr-check' should check every header:\n> \n> $ touch git-compat-util.h\n> $ make hdr-check\n> HDR git-compat-util.h\n> $ \n\nYeah, I think this is similar to the header-dependency thing I brought\nup for the sparse files. My thinking was that it wouldn't matter for the\nhdr-check, because by definition if a header you include changes, then\nwe'd check it independently. But it's possible for foo.h to be fine\nitself, but bar.h which includes it to fail because of a change in foo.h\n(e.g., removing a type declaration).\n\n> Hmm, not quite. So, the sparse target should have similar problems:\n> \n> $ make sparse\n> * new build flags\n> SP abspath.c\n> ...\n> SP remote-curl.c\n> $ \n> \n> OK, that works.\n\nSort of. Your \"new build flags\" is what saved you there, which was\ntriggered by something outside of your example commands. :)\n\nAs you saw below (and I showed in my earlier email), it doesn't work in\nthe general case.\n\n> $ make clean\n> ...\n> rm -f GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS\n> $ find . -iname '*.sp' | wc -l\n> 452\n> $ \n> \n> Ah, yes, you may want to add the removal of the 'synthetic objects' to the\n> make clean target!\n\nAgreed.\n\n> As I said, I don't quite understand what these patches want to do, so I can't\n> offer any solutions. :( Well, you could *add* the necessary dependencies,\n> of course, but that could lead to a rabbit hole which I would not want to\n> go down!\n\nI think depending on the \".o\", as I mentioned earlier, is a good way of\nquickly getting those dependencies without having to specify any logic.\nEven though we don't use the \".o\" file itself, it's a proxy for \"all the\nthings the .c file depends on\".\n\n-Peff\n"},{"id":"436757","messageId":"82c3d9fc-32ff-c38a-cb44-873af1fb83d1@ramsayjones.plus.com","threadId":"56561","inReplyTo":"YUttpgkU6eCOxMj/@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-09-22T19:17:16Z","receivedAt":"2021-09-22T19:17:21Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 22/09/2021 18:53, Jeff King wrote:\n> On Wed, Sep 22, 2021 at 05:58:16PM +0100, Ramsay Jones wrote:\n> \n>>> All three seem pretty reasonable to me.\n>>\n>> Heh, interesting. My initial reaction was completely negative! ;-P\n>> (and not just mildly negative either, but 'you must be kidding').\n>>\n>> However, I then thought 'I must be missing something, I'm being\n>> stupid and about to embarrass myself in public!'. So, I have\n>> been trying hard to understand what these patches are trying to\n>> accomplish and just what it is I'm missing. But, I'm coming up\n>> blank ...\n> \n> I think the point is just avoiding repeated work. If you just manually\n> run \"make sparse\" once in a while, then caching the result probably\n> isn't of much value. But if you plan to run, say:\n> \n> git rebase -x 'make sparse'\n> \n> then it would be nice for it to avoid checking the same files over and\n> over.\n\nI haven't tried, but\n\n git rebase -x 'make CC=cgcc'\n\nmay be a better idea (for some definition of 'better' ;) ).\n(if you have been doing all recent builds with CC=cgcc, then\nthe first commit wouldn't force a complete re-build!).\n\nUsing CC=cgcc has a mixed past, sometimes working, sometimes\nnot (again for some definition of 'working'), for example:\n\n $ git checkout master\n ...\n $ make clean\n ...\n $ make CC=cgcc >out1 2>&1\n $ ./git version\n git version 2.33.0.514.g99c99ed825\n $ git describe\n v2.33.0-514-g99c99ed825\n $ grep warn out1\n imap-send.c:1461:9: warning: expression using sizeof on a function\n http.c:715:9: warning: expression using sizeof on a function\n http.c:1776:25: warning: expression using sizeof on a function\n http.c:1781:25: warning: expression using sizeof on a function\n http.c:2190:9: warning: expression using sizeof on a function\n http.c:2362:9: warning: expression using sizeof on a function\n http-walker.c:382:9: warning: expression using sizeof on a function\n http-push.c:194:9: warning: expression using sizeof on a function\n http-push.c:205:9: warning: expression using sizeof on a function\n http-push.c:206:9: warning: expression using sizeof on a function\n remote-curl.c:855:9: warning: expression using sizeof on a function\n remote-curl.c:945:17: warning: expression using sizeof on a function\n remote-curl.c:947:17: warning: expression using sizeof on a function\n remote-curl.c:1014:9: warning: expression using sizeof on a function\n $ grep error out1\n $ \n\nThe warnings are due to some gnarly macro magic in the curl headers\nwhich is normally suppressed by setting -DCURL_DISABLE_TYPECHECK in\nthe SP_EXTRA_FLAGS variable for each of those files. (see e.g the\nMakefile:2250).\n\n>> At the heart of my unease is dependencies (or rather the lack) for\n>> the 'synthetic object files' *.hco and *.sp. (Also, the addition\n>> of even more 'shrapnel' to the build directories - I wrote a patch\n>> to remove the useless *.hcc files just after commit b503a2d515e was\n>> included, but didn't get around to submitting it).\n> \n> I don't consider them shrapnel if they're holding useful results. :)\n\nHeh, yes I am a bit of a curmudgeon! :D\n\n> But overall, I do agree that if we can't make the dependencies solid\n> here, this is not worth doing. Sacrificing correctness of the checks for\n> reduced computation is not a good idea.\n\nYes, I suspect that 'make the dependencies solid' will be a\nchallenge, with drip, drip, fixes being required. (Maybe I\nam just too pessimistic - maybe we can accept good enough\nrather than perfect. Also, the sparse solution may be easier\nthan the hdr-check solution).\n\nATB,\nRamsay Jones\n"},{"id":"436759","messageId":"xmqq4kac76s0.fsf@gitster.g","threadId":"56561","inReplyTo":"c13b050c-abb9-c669-b92c-930f2b43ce10@ramsayjones.plus.com","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-22T19:24:31Z","receivedAt":"2021-09-22T19:24:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> All three seem pretty reasonable to me.\n>\n> Heh, interesting. My initial reaction was completely negative! ;-P\n> (and not just mildly negative either, but 'you must be kidding').\n>\n> However, I then thought 'I must be missing something, I'm being\n> stupid and about to embarrass myself in public!'.\n\n;-) My thoughts, exactly.\n"},{"id":"436802","messageId":"xmqqsfxw42c1.fsf@gitster.g","threadId":"56561","inReplyTo":"82c3d9fc-32ff-c38a-cb44-873af1fb83d1@ramsayjones.plus.com","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-22T23:28:46Z","receivedAt":"2021-09-22T23:28:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>>> At the heart of my unease is dependencies (or rather the lack) for\n>>> the 'synthetic object files' *.hco and *.sp. (Also, the addition\n>>> of even more 'shrapnel' to the build directories - I wrote a patch\n>>> to remove the useless *.hcc files just after commit b503a2d515e was\n>>> included, but didn't get around to submitting it).\n>> \n>> I don't consider them shrapnel if they're holding useful results. :)\n>\n> Heh, yes I am a bit of a curmudgeon! :D\n\nWe do not necessarily have to have these files immediately next to\nthe corresponding source file.\n\nFor example, we could give .shrapnel/ hierarchy to *.hco and *.sp\nfiles, so that for revision.c and compat/bswap.h, we'd create\n.shrapnel/revision.sp and .shrapnel/compat/bswap.hco files that will\nnot be so cluttering ;-)\n\n\n"},{"id":"436805","messageId":"patch-v2-1.1-059829f2195-20210923T000654Z-avarab@gmail.com","threadId":"56561","inReplyTo":"cover-0.3-00000000000-20210921T224944Z-avarab@gmail.com","subject":"[PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T00:07:16Z","receivedAt":"2021-09-23T00:07:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"sparse\" target and its *.sp dependencies to be\nnon-.PHONY. Before this change \"make sparse\" would take ~5s to re-run\nall the *.c files through \"cgcc\", after it it'll create an empty *.sp\nfile sitting alongside the *.c file, only if the *.c file or its\ndependencies are newer than the *.sp is the *.sp re-made.\n\nWe ensure that the recursive dependencies are correct by depending on\nthe *.o file, which in turn will have correct dependencies by either\ndepending on all header files, or under\n\"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n\nThis means that a plain \"make sparse\" is much slower, as we'll now\nneed to make the *.o files just to create the *.sp files, but\nincrementally creating the *.sp files is *much* faster and less\nverbose, it thus becomes viable to run \"sparse\" along with \"all\" as\ne.g. \"git rebase --exec 'make all sparse'\".\n\nOn my box with -j8 \"make sparse\" was fast before, or around 5 seconds,\nnow it only takes that long the first time, and the common case is\n<100ms, or however long it takes GNU make to stat the *.sp file and\nsee that all the corresponding *.c file and its dependencies are\nolder.\n\nSee 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\nvariables, 2011-04-21) for the modern implementation of the sparse\ntarget being changed here.\n\nIt is critical that we use -Wsparse-error here, otherwise the error\nwould only show up once, but we'd successfully create the empty *.sp\nfile, and running a second time wouldn't show the error. I'm therefore\nnot putting it into SPARSE_FLAGS or SP_EXTRA_FLAGS, it's not optional,\nthe Makefile logic won't behave properly without it.\n\nAppending to $@ without a move is OK here because we're using the\n.DELETE_ON_ERROR Makefile feature. See 7b76d6bf221 (Makefile: add and\nuse the \".DELETE_ON_ERROR\" flag, 2021-06-29). GNU make ensures that on\nerror this file will be removed.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\nRange-diff against v1:\n1: e03fde1b642 ! 1: 059829f2195 Makefile: make the \"sparse\" target non-.PHONY\n @@ Commit message\n Makefile: make the \"sparse\" target non-.PHONY\n \n Change the \"sparse\" target and its *.sp dependencies to be\n - non-.PHONY. It's now viable to run it as part of a normal compilation\n - target, as we'll only re-generate these checks if the source *.c file\n - has changed.\n + non-.PHONY. Before this change \"make sparse\" would take ~5s to re-run\n + all the *.c files through \"cgcc\", after it it'll create an empty *.sp\n + file sitting alongside the *.c file, only if the *.c file or its\n + dependencies are newer than the *.sp is the *.sp re-made.\n \n - On my box with -j8 it was fast before, or around 5 seconds, now it\n - only takes that long the first time, and the common case is <100ms, or\n - however long it takes GNU make to stat the *.sp file and see that all\n - the corresponding *.c files are older.\n + We ensure that the recursive dependencies are correct by depending on\n + the *.o file, which in turn will have correct dependencies by either\n + depending on all header files, or under\n + \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n +\n + This means that a plain \"make sparse\" is much slower, as we'll now\n + need to make the *.o files just to create the *.sp files, but\n + incrementally creating the *.sp files is *much* faster and less\n + verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n + e.g. \"git rebase --exec 'make all sparse'\".\n +\n + On my box with -j8 \"make sparse\" was fast before, or around 5 seconds,\n + now it only takes that long the first time, and the common case is\n + <100ms, or however long it takes GNU make to stat the *.sp file and\n + see that all the corresponding *.c file and its dependencies are\n + older.\n \n See 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\n variables, 2011-04-21) for the modern implementation of the sparse\n @@ Makefile: check-sha1:: t/helper/test-tool$X\n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n -$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n -+$(SP_OBJ): %.sp: %.c GIT-CFLAGS\n ++$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n -\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n +\t\t-Wsparse-error \\\n @@ Makefile: check-sha1:: t/helper/test-tool$X\n sparse: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n +@@ Makefile: clean: profile-clean coverage-clean cocciclean\n + \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n + \t$(RM) $(TEST_PROGRAMS)\n + \t$(RM) $(FUZZ_PROGRAMS)\n ++\t$(RM) $(SP_OBJ)\n + \t$(RM) $(HCC)\n + \t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n + \t$(RM) -r po/build/\n2: 9c4cedacdce < -: ----------- Makefile: do one append in %.hcc rule\n3: e2ad1700f9e < -: ----------- Makefile: make the \"hdr-check\" target non-.PHONY\n\n .gitignore | 1 +\n Makefile | 9 ++++++---\n 2 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..b02250a50c4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -224,6 +224,7 @@\n *.lib\n *.res\n *.sln\n+*.sp\n *.suo\n *.ncb\n *.vcproj\ndiff --git a/Makefile b/Makefile\nindex a9f9b689f0c..0bae65889fe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2896,11 +2896,13 @@ check-sha1:: t/helper/test-tool$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n-\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n+\t\t-Wsparse-error \\\n+\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< && \\\n+\t>$@\n \n-.PHONY: sparse $(SP_OBJ)\n+.PHONY: sparse\n sparse: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n@@ -3227,6 +3229,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n+\t$(RM) $(SP_OBJ)\n \t$(RM) $(HCC)\n \t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n-- \n2.33.0.1225.g9f062250122\n\n"},{"id":"436810","messageId":"87pmt05bww.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"xmqqsfxw42c1.fsf@gitster.g","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T01:07:04Z","receivedAt":"2021-09-23T01:16:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Sep 22 2021, Junio C Hamano wrote:\n\n> Ramsay Jones writes:\n>\n>>>> At the heart of my unease is dependencies (or rather the lack) for\n>>>> the 'synthetic object files' *.hco and *.sp. (Also, the addition\n>>>> of even more 'shrapnel' to the build directories - I wrote a patch\n>>>> to remove the useless *.hcc files just after commit b503a2d515e was\n>>>> included, but didn't get around to submitting it).\n>>> \n>>> I don't consider them shrapnel if they're holding useful results. :)\n>>\n>> Heh, yes I am a bit of a curmudgeon! :D\n>\n> We do not necessarily have to have these files immediately next to\n> the corresponding source file.\n>\n> For example, we could give .shrapnel/ hierarchy to *.hco and *.sp\n> files, so that for revision.c and compat/bswap.h, we'd create\n> .shrapnel/revision.sp and .shrapnel/compat/bswap.hco files that will\n> not be so cluttering ;-)\n\nI've got some WIP efforts in other areas to do that for some other\nrules.\n\nThe problem is that you need to \"mkdir .shrapnel\" to create a\n\".shrapnel/revision.sp\". So you need the \".shrapnel/revision.sp\" to\ndepend on the \".shrapnel\".\n\nExcept you'll find that the naïve implementation of that fails, since\nany file you create will bump the mtime of the containing directory, so\nyou'll keep re-making \".shrapnel/revision.sp\" because \".shrapnel\"\nchanged, because \".shrapnel/revision.sp\" changed...\n\nThis is why we have the \"missing_dep_dirs\" hack for\n\"COMPUTE_HEADER_DEPENDENCIES\" in the Makefile, i.e. we'll make the\ndirectory, but only if it doesn't exist, we bypass the normal \"make\"\ndependency mechanism.\n\nAnother way to deal with it is to have say a\nbuild-stuff/dropit-here/file.gen that you build from a top-leve file.c,\nthen depend on a \"build-stuff\", that does a \"mkdir -p\nbuild-stuff/dropit-here\", I used that trick in another case where the\nfile.gen were not nested.\n\nBut for creating subdirs etc. you'll quickly get into a lot of\nnastyness, which I'd prefer to just avoid here.\n\nWe already have e.g. the *.hcc files, let's leave generating these on\nthe side somewhere to some more general topic, which could also move the\n*.o files out of the top-level if we're caring about the cleanliness of\nthe top-level directory.\n"},{"id":"436811","messageId":"xmqqtuic2igl.fsf@gitster.g","threadId":"56561","inReplyTo":"87pmt05bww.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-23T01:23:22Z","receivedAt":"2021-09-23T01:23:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I've got some WIP efforts in other areas to do that for some other\n> rules.\n>\n> The problem is that you need to \"mkdir .shrapnel\" to create a\n> \".shrapnel/revision.sp\". So you need the \".shrapnel/revision.sp\" to\n> depend on the \".shrapnel\".\n>\n> Except you'll find that the naïve implementation of that fails, since\n> any file you create will bump the mtime of the containing directory, so\n> you'll keep re-making \".shrapnel/revision.sp\" because \".shrapnel\"\n> changed, because \".shrapnel/revision.sp\" changed...\n\nWe depend on GNU make anyway. Isn't its \"order-only-prerequisites\"\nfeature what you exactly want to use for the above?\n\n"},{"id":"436821","messageId":"878rzo58z4.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"xmqqtuic2igl.fsf@gitster.g","subject":"Re: [PATCH 0/3] Makefile: make \"sparse\" and \"hdr-check\" non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T02:17:16Z","receivedAt":"2021-09-23T02:20:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Sep 22 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I've got some WIP efforts in other areas to do that for some other\n>> rules.\n>>\n>> The problem is that you need to \"mkdir .shrapnel\" to create a\n>> \".shrapnel/revision.sp\". So you need the \".shrapnel/revision.sp\" to\n>> depend on the \".shrapnel\".\n>>\n>> Except you'll find that the naïve implementation of that fails, since\n>> any file you create will bump the mtime of the containing directory, so\n>> you'll keep re-making \".shrapnel/revision.sp\" because \".shrapnel\"\n>> changed, because \".shrapnel/revision.sp\" changed...\n>\n> We depend on GNU make anyway. Isn't its \"order-only-prerequisites\"\n> feature what you exactly want to use for the above?\n\nIt looks like it, and that I should probably take more time one of these\ndays to read the GNU make manual through.\n\nBut in any case, I do think that's worthwhile in general, i.e. you can\ndepend on %.h and not need to exclude generated %.h that we make\nourselves if we put that into \"gen/\" or whatever, \"clean\" also becomes a\nlot easier.\n\nBut I'd like to leave it for some future effort of moving *.o, *.sp\netc. generated files around, rather than making *.sp an odd special-case\nnow.\n"},{"id":"436866","messageId":"YUyqIpOLFDqG/kEJ@coredump.intra.peff.net","threadId":"56561","inReplyTo":"patch-v2-1.1-059829f2195-20210923T000654Z-avarab@gmail.com","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-23T16:24:02Z","receivedAt":"2021-09-23T16:24:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> We ensure that the recursive dependencies are correct by depending on\n> the *.o file, which in turn will have correct dependencies by either\n> depending on all header files, or under\n> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n> \n> This means that a plain \"make sparse\" is much slower, as we'll now\n> need to make the *.o files just to create the *.sp files, but\n> incrementally creating the *.sp files is *much* faster and less\n> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n> e.g. \"git rebase --exec 'make all sparse'\".\n\nOK. I think this solves the dependency issues sufficiently. It is a\ntradeoff that you must do the normal build in order to do the sparse\ncheck now. That is certainly fine for my workflow (I am building Git all\nthe time, and only occasionally run \"make sparse\"). I don't know if\nothers would like it less (e.g., if Ramsay is frequently running sparse\nchecks without having just built).\n\n(I'd say \"I do not care that much either way\", but then I do not care\nall that much either way about incremental sparse checks either, so I'm\nnot sure my opinion really matters).\n\n-Peff\n"},{"id":"436870","messageId":"87ilyr43v2.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"YUyqIpOLFDqG/kEJ@coredump.intra.peff.net","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T17:06:02Z","receivedAt":"2021-09-23T17:08:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 23 2021, Jeff King wrote:\n\n> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> We ensure that the recursive dependencies are correct by depending on\n>> the *.o file, which in turn will have correct dependencies by either\n>> depending on all header files, or under\n>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>> \n>> This means that a plain \"make sparse\" is much slower, as we'll now\n>> need to make the *.o files just to create the *.sp files, but\n>> incrementally creating the *.sp files is *much* faster and less\n>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>> e.g. \"git rebase --exec 'make all sparse'\".\n>\n> OK. I think this solves the dependency issues sufficiently. It is a\n> tradeoff that you must do the normal build in order to do the sparse\n> check now. That is certainly fine for my workflow (I am building Git all\n> the time, and only occasionally run \"make sparse\"). I don't know if\n> others would like it less (e.g., if Ramsay is frequently running sparse\n> checks without having just built).\n>\n> (I'd say \"I do not care that much either way\", but then I do not care\n> all that much either way about incremental sparse checks either, so I'm\n> not sure my opinion really matters).\n\nAside: As I recall you make a lot of use of ccache (as I do), so is the\n\"meh\" on incremental builds synonymous with it not being piped through\n$(CC) in this case?\n"},{"id":"436872","messageId":"YUy2tggR1tifj4Ay@coredump.intra.peff.net","threadId":"56561","inReplyTo":"87ilyr43v2.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-09-23T17:17:42Z","receivedAt":"2021-09-23T17:17:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 23, 2021 at 07:06:02PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> >> We ensure that the recursive dependencies are correct by depending on\n> >> the *.o file, which in turn will have correct dependencies by either\n> >> depending on all header files, or under\n> >> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n> >> \n> >> This means that a plain \"make sparse\" is much slower, as we'll now\n> >> need to make the *.o files just to create the *.sp files, but\n> >> incrementally creating the *.sp files is *much* faster and less\n> >> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n> >> e.g. \"git rebase --exec 'make all sparse'\".\n> >\n> > OK. I think this solves the dependency issues sufficiently. It is a\n> > tradeoff that you must do the normal build in order to do the sparse\n> > check now. That is certainly fine for my workflow (I am building Git all\n> > the time, and only occasionally run \"make sparse\"). I don't know if\n> > others would like it less (e.g., if Ramsay is frequently running sparse\n> > checks without having just built).\n> >\n> > (I'd say \"I do not care that much either way\", but then I do not care\n> > all that much either way about incremental sparse checks either, so I'm\n> > not sure my opinion really matters).\n> \n> Aside: As I recall you make a lot of use of ccache (as I do), so is the\n> \"meh\" on incremental builds synonymous with it not being piped through\n> $(CC) in this case?\n\nI do use ccache, and yeah, it makes a big difference on incremental\nbuilds of the actual object files. I don't use it for sparse, though (I\nnever really thought about it, but I don't see any reason why it would\nnot work?). Mostly I just don't run \"make sparse\" very often.\n\n-Peff\n"},{"id":"436877","messageId":"xmqqtuib199x.fsf@gitster.g","threadId":"56561","inReplyTo":"YUyqIpOLFDqG/kEJ@coredump.intra.peff.net","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-23T17:39:22Z","receivedAt":"2021-09-23T17:39:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> We ensure that the recursive dependencies are correct by depending on\n>> the *.o file, which in turn will have correct dependencies by either\n>> depending on all header files, or under\n>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>> \n>> This means that a plain \"make sparse\" is much slower, as we'll now\n>> need to make the *.o files just to create the *.sp files, but\n>> incrementally creating the *.sp files is *much* faster and less\n>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>> e.g. \"git rebase --exec 'make all sparse'\".\n>\n> OK. I think this solves the dependency issues sufficiently. It is a\n> tradeoff that you must do the normal build in order to do the sparse\n> check now. That is certainly fine for my workflow (I am building Git all\n> the time, and only occasionally run \"make sparse\"). I don't know if\n> others would like it less (e.g., if Ramsay is frequently running sparse\n> checks without having just built).\n>\n> (I'd say \"I do not care that much either way\", but then I do not care\n> all that much either way about incremental sparse checks either, so I'm\n> not sure my opinion really matters).\n\nMy build procedure runs \"make sparse\" before the primary build,\nsimply because the former tends to be much faster to fail when there\nis an issue in the code. I can understand that depending on .o is a\ncheap way to piggyback on the dependencies it has, but my latency\nwill get much slower if this goes in _and_ I keep trying to pick up\npotentially problematic patches from the list.\n\n\n\n"},{"id":"436908","messageId":"457ec039-1e26-9da9-55f6-9ea79b962bfe@ramsayjones.plus.com","threadId":"56561","inReplyTo":"xmqqtuib199x.fsf@gitster.g","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-09-23T23:28:21Z","receivedAt":"2021-09-23T23:28:32Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 23/09/2021 18:39, Junio C Hamano wrote:\n> Jeff King writes:\n> \n>> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>> We ensure that the recursive dependencies are correct by depending on\n>>> the *.o file, which in turn will have correct dependencies by either\n>>> depending on all header files, or under\n>>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>>>\n>>> This means that a plain \"make sparse\" is much slower, as we'll now\n>>> need to make the *.o files just to create the *.sp files, but\n>>> incrementally creating the *.sp files is *much* faster and less\n>>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>>> e.g. \"git rebase --exec 'make all sparse'\".\n>>\n>> OK. I think this solves the dependency issues sufficiently. It is a\n>> tradeoff that you must do the normal build in order to do the sparse\n>> check now. That is certainly fine for my workflow (I am building Git all\n>> the time, and only occasionally run \"make sparse\"). I don't know if\n>> others would like it less (e.g., if Ramsay is frequently running sparse\n>> checks without having just built).\n>>\n>> (I'd say \"I do not care that much either way\", but then I do not care\n>> all that much either way about incremental sparse checks either, so I'm\n>> not sure my opinion really matters).\n> \n> My build procedure runs \"make sparse\" before the primary build,\n> simply because the former tends to be much faster to fail when there\n> is an issue in the code. I can understand that depending on .o is a\n> cheap way to piggyback on the dependencies it has, but my latency\n> will get much slower if this goes in _and_ I keep trying to pick up\n> potentially problematic patches from the list.\n\n\nI always run 'make sparse -k >sp-out 2>&1' after having done the main\nbuild, so that is not an issue for me. Note that I always send all\noutput from each build step (for master, next and seen) to a series of\n(branch keyed) files, so that I can easily diff from branch to branch.\nAlso, as above, I use '-k' on the 'sparse' and 'hdr-check' targets to\ncollect all errors/warnings in one go.\n\nSo, this evening, with the v2 version of Ævar's patch having landed in\nthe 'seen' branch, we see this (abridged) diff between next and seen:\n\n $ diff nsp-out ssp-out\n 77a78\n > SP hook.c\n 289a291\n > SP builtin/hook.c\n 417a420\n > SP t/helper/test-reftable.c\n 449a453,478\n > SP reftable/basics.c\n...\n > SP reftable/tree_test.c\n 452a482,483\n > CC contrib/scalar/scalar.o\n > SP contrib/scalar/scalar.c\n $ \n\nSo, this almost looks normal, except for the 'CC' line! Having discovered\nsome leftover cruft from old builds yesterday:\n\n $ git ls-files | grep contrib/scalar\n contrib/scalar/.gitignore\n contrib/scalar/Makefile\n contrib/scalar/scalar.c\n contrib/scalar/scalar.txt\n contrib/scalar/t/Makefile\n contrib/scalar/t/t9099-scalar.sh\n $ ls contrib/scalar\n Makefile scalar.c scalar.o scalar.sp scalar.txt t/\n $ rm contrib/scalar/scalar.{o,sp}\n $ make\n SUBDIR git-gui\n SUBDIR gitk-git\n SUBDIR templates\n $ make sparse\n CC contrib/scalar/scalar.o\n SP contrib/scalar/scalar.c\n $ \n\nHmm, interesting, but not relevant here. So, lets play a bit:\n\n $ make sparse \n $ make git.sp\n $ make git.sp\n $ touch git.sp\n $ make git.sp\n $ touch git.c\n $ make git.sp\n CC git.o\n SP git.c\n $ touch git.o\n $ make git.sp\n SP git.c\n $ \n\nHmm, so I think it is working as designed. However, I find it to be\nmore than a little irritating (curmudgeon alert!).\n\nNote there are currently no sparse warnings in any of the branches\nI build (mainly because Junio patches them up before they hit the\ngit.kernel.org repo - I am not complaining! ;) ). However, should\nany warnings/errors appear (from my own development, say), then I\nwould make extensive use of 'make .sp' while fixing the\nproblem. Prior to this patch series, 'make .sp' would _always_\nrun sparse over the file - it would not depend on the 'mtime' or\nexistence of any other file, or run the compiler (and wouldn't leave\nany 'droppings' either). I liked that! :D\n\nSo, I still don't quite get where the 'savings' come from - maybe it\nis just me, but I don't think this improves any workflow (well not\nmine anyway). I just don't get it. :(\n\nATB,\nRamsay Jones\n\n"},{"id":"436911","messageId":"87a6k24vdc.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"457ec039-1e26-9da9-55f6-9ea79b962bfe@ramsayjones.plus.com","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-24T01:16:53Z","receivedAt":"2021-09-24T01:26:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Sep 24 2021, Ramsay Jones wrote:\n\n> On 23/09/2021 18:39, Junio C Hamano wrote:\n>> Jeff King writes:\n>> \n>>> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>>> We ensure that the recursive dependencies are correct by depending on\n>>>> the *.o file, which in turn will have correct dependencies by either\n>>>> depending on all header files, or under\n>>>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>>>>\n>>>> This means that a plain \"make sparse\" is much slower, as we'll now\n>>>> need to make the *.o files just to create the *.sp files, but\n>>>> incrementally creating the *.sp files is *much* faster and less\n>>>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>>>> e.g. \"git rebase --exec 'make all sparse'\".\n>>>\n>>> OK. I think this solves the dependency issues sufficiently. It is a\n>>> tradeoff that you must do the normal build in order to do the sparse\n>>> check now. That is certainly fine for my workflow (I am building Git all\n>>> the time, and only occasionally run \"make sparse\"). I don't know if\n>>> others would like it less (e.g., if Ramsay is frequently running sparse\n>>> checks without having just built).\n>>>\n>>> (I'd say \"I do not care that much either way\", but then I do not care\n>>> all that much either way about incremental sparse checks either, so I'm\n>>> not sure my opinion really matters).\n>> \n>> My build procedure runs \"make sparse\" before the primary build,\n>> simply because the former tends to be much faster to fail when there\n>> is an issue in the code. I can understand that depending on .o is a\n>> cheap way to piggyback on the dependencies it has, but my latency\n>> will get much slower if this goes in _and_ I keep trying to pick up\n>> potentially problematic patches from the list.\n>\n>\n> I always run 'make sparse -k >sp-out 2>&1' after having done the main\n> build, so that is not an issue for me. Note that I always send all\n> output from each build step (for master, next and seen) to a series of\n> (branch keyed) files, so that I can easily diff from branch to branch.\n> Also, as above, I use '-k' on the 'sparse' and 'hdr-check' targets to\n> collect all errors/warnings in one go.\n>\n> So, this evening, with the v2 version of Ævar's patch having landed in\n> the 'seen' branch, we see this (abridged) diff between next and seen:\n>\n> $ diff nsp-out ssp-out\n> 77a78\n> > SP hook.c\n> 289a291\n> > SP builtin/hook.c\n> 417a420\n> > SP t/helper/test-reftable.c\n> 449a453,478\n> > SP reftable/basics.c\n> ...\n> > SP reftable/tree_test.c\n> 452a482,483\n> > CC contrib/scalar/scalar.o\n> > SP contrib/scalar/scalar.c\n> $ \n>\n> So, this almost looks normal, except for the 'CC' line! Having discovered\n> some leftover cruft from old builds yesterday:\n>\n> $ git ls-files | grep contrib/scalar\n> contrib/scalar/.gitignore\n> contrib/scalar/Makefile\n> contrib/scalar/scalar.c\n> contrib/scalar/scalar.txt\n> contrib/scalar/t/Makefile\n> contrib/scalar/t/t9099-scalar.sh\n> $ ls contrib/scalar\n> Makefile scalar.c scalar.o scalar.sp scalar.txt t/\n> $ rm contrib/scalar/scalar.{o,sp}\n> $ make\n> SUBDIR git-gui\n> SUBDIR gitk-git\n> SUBDIR templates\n> $ make sparse\n> CC contrib/scalar/scalar.o\n> SP contrib/scalar/scalar.c\n> $ \n>\n> Hmm, interesting, but not relevant here. So, lets play a bit:\n>\n> $ make sparse \n> $ make git.sp\n> $ make git.sp\n> $ touch git.sp\n> $ make git.sp\n> $ touch git.c\n> $ make git.sp\n> CC git.o\n> SP git.c\n> $ touch git.o\n> $ make git.sp\n> SP git.c\n> $ \n>\n> Hmm, so I think it is working as designed. However, I find it to be\n> more than a little irritating (curmudgeon alert!).\n\nSpecifically that there's now \"SP\" lines in the output, that *.sp files\nare created at all, that they're created where they are, or some\ncombination of those thigs?\n\n> Note there are currently no sparse warnings in any of the branches\n> I build (mainly because Junio patches them up before they hit the\n> git.kernel.org repo - I am not complaining! ;) ). However, should\n> any warnings/errors appear (from my own development, say), then I\n> would make extensive use of 'make .sp' while fixing the\n> problem. Prior to this patch series, 'make .sp' would _always_\n> run sparse over the file - it would not depend on the 'mtime' or\n> existence of any other file, or run the compiler (and wouldn't leave\n> any 'droppings' either). I liked that! :D\n>\n> So, I still don't quite get where the 'savings' come from - maybe it\n> is just me, but I don't think this improves any workflow (well not\n> mine anyway). I just don't get it. :(\n\nThe point is that you can now instead of:\n\n make -jN all\n\nJust do:\n\n make -jN all sparse\n\nAnd do those checks all the time, whether it's in your your normal\nedit/compile/test cycle, or via \"git rebase --exec\", and not have it\ntake much longer than not having \"sparse\" there.\n\nSo I think you won't have any reason to run \"make .sp\" anymore.\nWhy not just run \"make all sparse\"?\n\nAs long as you have any outstanding errors in a .sp\" you'll keep\ngetting just that relevant output, and once you fix the issue the\ndependency is satisified.\n\nJust like if you've got a compile error in say usage.c now you've got no\nreason to stop running \"make all\" and start running \"make usage.o\", the\ndependency graph makes it so that you'll get the right output via \"make\nall\", and the added time from running the more general target is\ntrivial.\n"},{"id":"436914","messageId":"875yuq4u4u.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"xmqqtuib199x.fsf@gitster.g","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-24T01:30:10Z","receivedAt":"2021-09-24T01:52:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Sep 23 2021, Junio C Hamano wrote:\n\n> Jeff King writes:\n>\n>> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>> We ensure that the recursive dependencies are correct by depending on\n>>> the *.o file, which in turn will have correct dependencies by either\n>>> depending on all header files, or under\n>>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>>> \n>>> This means that a plain \"make sparse\" is much slower, as we'll now\n>>> need to make the *.o files just to create the *.sp files, but\n>>> incrementally creating the *.sp files is *much* faster and less\n>>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>>> e.g. \"git rebase --exec 'make all sparse'\".\n>>\n>> OK. I think this solves the dependency issues sufficiently. It is a\n>> tradeoff that you must do the normal build in order to do the sparse\n>> check now. That is certainly fine for my workflow (I am building Git all\n>> the time, and only occasionally run \"make sparse\"). I don't know if\n>> others would like it less (e.g., if Ramsay is frequently running sparse\n>> checks without having just built).\n>>\n>> (I'd say \"I do not care that much either way\", but then I do not care\n>> all that much either way about incremental sparse checks either, so I'm\n>> not sure my opinion really matters).\n>\n> My build procedure runs \"make sparse\" before the primary build,\n> simply because the former tends to be much faster to fail when there\n> is an issue in the code. I can understand that depending on .o is a\n> cheap way to piggyback on the dependencies it has, but my latency\n> will get much slower if this goes in _and_ I keep trying to pick up\n> potentially problematic patches from the list.\n\nWould you be OK with just running something like this instead?:\n\n make -j $(nproc) objects CC=cgcc CFLAGS=\"-no-compile -Wsparse-error -Wno-pointer-arith -Wno-memcpy-max-count\"\n\nIt gives you almost exactly the same thing as the old \"sparse\" target. I\nthink the way it worked is really not something we needed a special\ntarget for in the first place, or perhaps just a .PHONY alias.\n\nThe \"almost\" is because those -Wno-* are now per-file via\nSP_EXTRA_FLAGS.\n\nI.e. if we have a $(CC) that's willing to accept CC-like options but\njust won't create output files we can use \"objects\" (or other targets I\nadded in 029bac01a87 (Makefile: add {program,xdiff,test,git,fuzz}-objs &\nobjects targets, 2021-02-23)). Even if the tool itself created broken\n*.o files a change of $(CC) to the \"real\" compiler would ensure that\nthey'd get regenerated.\n"},{"id":"436972","messageId":"7673e7aa-9737-a9eb-74ff-e11f071356e2@ramsayjones.plus.com","threadId":"56561","inReplyTo":"87a6k24vdc.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-09-24T16:38:48Z","receivedAt":"2021-09-24T16:38:54Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 24/09/2021 02:16, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Sep 24 2021, Ramsay Jones wrote:\n> \n>> On 23/09/2021 18:39, Junio C Hamano wrote:\n>>> Jeff King writes:\n>>>\n>>>> On Thu, Sep 23, 2021 at 02:07:16AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>>>\n>>>>> We ensure that the recursive dependencies are correct by depending on\n>>>>> the *.o file, which in turn will have correct dependencies by either\n>>>>> depending on all header files, or under\n>>>>> \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n>>>>>\n>>>>> This means that a plain \"make sparse\" is much slower, as we'll now\n>>>>> need to make the *.o files just to create the *.sp files, but\n>>>>> incrementally creating the *.sp files is *much* faster and less\n>>>>> verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n>>>>> e.g. \"git rebase --exec 'make all sparse'\".\n>>>>\n>>>> OK. I think this solves the dependency issues sufficiently. It is a\n>>>> tradeoff that you must do the normal build in order to do the sparse\n>>>> check now. That is certainly fine for my workflow (I am building Git all\n>>>> the time, and only occasionally run \"make sparse\"). I don't know if\n>>>> others would like it less (e.g., if Ramsay is frequently running sparse\n>>>> checks without having just built).\n>>>>\n>>>> (I'd say \"I do not care that much either way\", but then I do not care\n>>>> all that much either way about incremental sparse checks either, so I'm\n>>>> not sure my opinion really matters).\n>>>\n>>> My build procedure runs \"make sparse\" before the primary build,\n>>> simply because the former tends to be much faster to fail when there\n>>> is an issue in the code. I can understand that depending on .o is a\n>>> cheap way to piggyback on the dependencies it has, but my latency\n>>> will get much slower if this goes in _and_ I keep trying to pick up\n>>> potentially problematic patches from the list.\n>>\n>>\n>> I always run 'make sparse -k >sp-out 2>&1' after having done the main\n>> build, so that is not an issue for me. Note that I always send all\n>> output from each build step (for master, next and seen) to a series of\n>> (branch keyed) files, so that I can easily diff from branch to branch.\n>> Also, as above, I use '-k' on the 'sparse' and 'hdr-check' targets to\n>> collect all errors/warnings in one go.\n>>\n>> So, this evening, with the v2 version of Ævar's patch having landed in\n>> the 'seen' branch, we see this (abridged) diff between next and seen:\n>>\n>> $ diff nsp-out ssp-out\n>> 77a78\n>> > SP hook.c\n>> 289a291\n>> > SP builtin/hook.c\n>> 417a420\n>> > SP t/helper/test-reftable.c\n>> 449a453,478\n>> > SP reftable/basics.c\n>> ...\n>> > SP reftable/tree_test.c\n>> 452a482,483\n>> > CC contrib/scalar/scalar.o\n>> > SP contrib/scalar/scalar.c\n>> $ \n>>\n>> So, this almost looks normal, except for the 'CC' line! Having discovered\n>> some leftover cruft from old builds yesterday:\n>>\n>> $ git ls-files | grep contrib/scalar\n>> contrib/scalar/.gitignore\n>> contrib/scalar/Makefile\n>> contrib/scalar/scalar.c\n>> contrib/scalar/scalar.txt\n>> contrib/scalar/t/Makefile\n>> contrib/scalar/t/t9099-scalar.sh\n>> $ ls contrib/scalar\n>> Makefile scalar.c scalar.o scalar.sp scalar.txt t/\n>> $ rm contrib/scalar/scalar.{o,sp}\n>> $ make\n>> SUBDIR git-gui\n>> SUBDIR gitk-git\n>> SUBDIR templates\n>> $ make sparse\n>> CC contrib/scalar/scalar.o\n>> SP contrib/scalar/scalar.c\n>> $ \n>>\n>> Hmm, interesting, but not relevant here. So, lets play a bit:\n>>\n>> $ make sparse \n>> $ make git.sp\n>> $ make git.sp\n>> $ touch git.sp\n>> $ make git.sp\n>> $ touch git.c\n>> $ make git.sp\n>> CC git.o\n>> SP git.c\n>> $ touch git.o\n>> $ make git.sp\n>> SP git.c\n>> $ \n>>\n>> Hmm, so I think it is working as designed. However, I find it to be\n>> more than a little irritating (curmudgeon alert!).\n> \n> Specifically that there's now \"SP\" lines in the output, that *.sp files\n> are created at all, that they're created where they are, or some\n> combination of those thigs?\n\nHeh, just ignore me.\n\nAlthough I haven't done much testing, I believe your patch correctly\nimplements what you intended.\n\nATB,\nRamsay Jones\n\n"},{"id":"437023","messageId":"xmqqwnn5sr1v.fsf@gitster.g","threadId":"56561","inReplyTo":"875yuq4u4u.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2] Makefile: make the \"sparse\" target non-.PHONY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-24T19:37:48Z","receivedAt":"2021-09-24T19:37:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Would you be OK with just running something like this instead?:\n>\n> make -j $(nproc) objects CC=cgcc CFLAGS=\"-no-compile -Wsparse-error -Wno-pointer-arith -Wno-memcpy-max-count\"\n\nCounting both machine and human time cost, I'd rather go with \"make\nfoo.sp\", even if it is much slower than before, than having to type\nthat, or having remember to run a custom script that encapsulates\nthe above, which is something different from just typing \"make\".\n\nOf course, I'd be happier if \"make sparse\" did dependencies right\n*and* did not involve real compilation, but if that is beyond our\ncapability, then so be it --- I do not that deeply care ;-)\n\n\n"},{"id":"437221","messageId":"patch-v3-1.1-b6ba99ca4cc-20210928T011319Z-avarab@gmail.com","threadId":"56561","inReplyTo":"patch-v2-1.1-059829f2195-20210923T000654Z-avarab@gmail.com","subject":"[PATCH v3] Makefile: add a non-.PHONY \"sparse-incr\" target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-28T01:15:02Z","receivedAt":"2021-09-28T01:15:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a \"sparse-incr\" target to compliment the existing \"sparse\"\ntarget. The new \"sparse-incr\" target doesn't depend on \"FORCE\", and\nwill create empty *.sp files as markers for the corresponding *.c file\nhaving been checked.\n\nThose *.sp files in turn depend on the *.o files, so we can be certain\nthat the dependencies are correct by either depending on all header\nfiles, or under \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers the\nrelevant file needs.\n\nThe \"sparse-incr\" target is slower on a fresh git.git checkout, as it\ndepends on the creation of the *.o files. But once the *.o and *.sp\nfiles are built it's able to incrementally update them. It's thus\nviable to run:\n\n make all sparse-incr\n\nAs part of a regular edit/compile/test cycle, or as a fast \"git rebase\n--exec\" command.\n\nOn my box (with -j8) the initial run of the \"sparse\" target takes ~5s,\nbut ~16s for \"sparse-incr\". When using CC=\"ccache cc\" the difference\nbetween the two is negligible.\n\nRunning e.g.:\n\n make grep.sp\n\nWill behave the same way and will always re-run \"cgcc\", we'll only use\nthe new dependency chain of \"sparse-incr\" is part of the MAKECMDGOALS.\n\nI think it would make sense to just remove the \"sparse\" target\nentirely, and to say that anyone who cares about the speed of an\ninitial \"sparse\" run should use \"CC='ccache cc'\". But per [1] and [2]\nthere are existing users of \"make sparse\" and \"make .sp\" that\nprefer the current semantics.\n\nI.e. per [2] want \"make .sp\" to *always* run \"sparse\", even\nthough a corresponding \"make .o\" would only re-run the \"real\"\ncompilation if needed. I don't think that makes any sense, especially\nin combination with DEVELOPER=1 which'll ensure that -Werror would\nhave made any errors in a \"make .sp\" sticky.\n\nBut since we have existing users relying on it, and I don't really\ncare at al about \"make .sp\", I just want an incremental target I\ncan use, let's create this new \"make sparse-incr\" instead of \"fixing\"\nthe existing \"make sparse\".\n\nSee 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\nvariables, 2011-04-21) for the modern implementation of the \"sparse\"\ntarget being changed here.\n\nAppending to $@ without a move is OK here because we're using the\n.DELETE_ON_ERROR Makefile feature. See 7b76d6bf221 (Makefile: add and\nuse the \".DELETE_ON_ERROR\" flag, 2021-06-29). GNU make ensures that on\nerror this file will be removed.\n\n1. https://lore.kernel.org/git/xmqqtuib199x.fsf@gitster.g/\n2. https://lore.kernel.org/git/457ec039-1e26-9da9-55f6-9ea79b962bfe@ramsayjones.plus.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\nRange-diff against v2:\n1: 059829f2195 ! 1: b6ba99ca4cc Makefile: make the \"sparse\" target non-.PHONY\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - Makefile: make the \"sparse\" target non-.PHONY\n -\n - Change the \"sparse\" target and its *.sp dependencies to be\n - non-.PHONY. Before this change \"make sparse\" would take ~5s to re-run\n - all the *.c files through \"cgcc\", after it it'll create an empty *.sp\n - file sitting alongside the *.c file, only if the *.c file or its\n - dependencies are newer than the *.sp is the *.sp re-made.\n -\n - We ensure that the recursive dependencies are correct by depending on\n - the *.o file, which in turn will have correct dependencies by either\n - depending on all header files, or under\n - \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers it needs.\n -\n - This means that a plain \"make sparse\" is much slower, as we'll now\n - need to make the *.o files just to create the *.sp files, but\n - incrementally creating the *.sp files is *much* faster and less\n - verbose, it thus becomes viable to run \"sparse\" along with \"all\" as\n - e.g. \"git rebase --exec 'make all sparse'\".\n -\n - On my box with -j8 \"make sparse\" was fast before, or around 5 seconds,\n - now it only takes that long the first time, and the common case is\n - <100ms, or however long it takes GNU make to stat the *.sp file and\n - see that all the corresponding *.c file and its dependencies are\n - older.\n + Makefile: add a non-.PHONY \"sparse-incr\" target\n +\n + Add a \"sparse-incr\" target to compliment the existing \"sparse\"\n + target. The new \"sparse-incr\" target doesn't depend on \"FORCE\", and\n + will create empty *.sp files as markers for the corresponding *.c file\n + having been checked.\n +\n + Those *.sp files in turn depend on the *.o files, so we can be certain\n + that the dependencies are correct by either depending on all header\n + files, or under \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers the\n + relevant file needs.\n +\n + The \"sparse-incr\" target is slower on a fresh git.git checkout, as it\n + depends on the creation of the *.o files. But once the *.o and *.sp\n + files are built it's able to incrementally update them. It's thus\n + viable to run:\n +\n + make all sparse-incr\n +\n + As part of a regular edit/compile/test cycle, or as a fast \"git rebase\n + --exec\" command.\n +\n + On my box (with -j8) the initial run of the \"sparse\" target takes ~5s,\n + but ~16s for \"sparse-incr\". When using CC=\"ccache cc\" the difference\n + between the two is negligible.\n +\n + Running e.g.:\n +\n + make grep.sp\n +\n + Will behave the same way and will always re-run \"cgcc\", we'll only use\n + the new dependency chain of \"sparse-incr\" is part of the MAKECMDGOALS.\n +\n + I think it would make sense to just remove the \"sparse\" target\n + entirely, and to say that anyone who cares about the speed of an\n + initial \"sparse\" run should use \"CC='ccache cc'\". But per [1] and [2]\n + there are existing users of \"make sparse\" and \"make .sp\" that\n + prefer the current semantics.\n +\n + I.e. per [2] want \"make .sp\" to *always* run \"sparse\", even\n + though a corresponding \"make .o\" would only re-run the \"real\"\n + compilation if needed. I don't think that makes any sense, especially\n + in combination with DEVELOPER=1 which'll ensure that -Werror would\n + have made any errors in a \"make .sp\" sticky.\n +\n + But since we have existing users relying on it, and I don't really\n + care at al about \"make .sp\", I just want an incremental target I\n + can use, let's create this new \"make sparse-incr\" instead of \"fixing\"\n + the existing \"make sparse\".\n \n See 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\n - variables, 2011-04-21) for the modern implementation of the sparse\n + variables, 2011-04-21) for the modern implementation of the \"sparse\"\n target being changed here.\n \n - It is critical that we use -Wsparse-error here, otherwise the error\n - would only show up once, but we'd successfully create the empty *.sp\n - file, and running a second time wouldn't show the error. I'm therefore\n - not putting it into SPARSE_FLAGS or SP_EXTRA_FLAGS, it's not optional,\n - the Makefile logic won't behave properly without it.\n -\n Appending to $@ without a move is OK here because we're using the\n .DELETE_ON_ERROR Makefile feature. See 7b76d6bf221 (Makefile: add and\n use the \".DELETE_ON_ERROR\" flag, 2021-06-29). GNU make ensures that on\n error this file will be removed.\n \n + 1. https://lore.kernel.org/git/xmqqtuib199x.fsf@gitster.g/\n + 2. https://lore.kernel.org/git/457ec039-1e26-9da9-55f6-9ea79b962bfe@ramsayjones.plus.com/\n +\n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n ## .gitignore ##\n @@ Makefile: check-sha1:: t/helper/test-tool$X\n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n -$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n -+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n ++define cmd_run_sparse\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n --\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n -+\t\t-Wsparse-error \\\n -+\t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< && \\\n -+\t>$@\n + \t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n ++endef\n ++\n ++ifneq ($(filter sparse-incr,$(MAKECMDGOALS)),sparse-incr)\n ++ifneq ($(filter sparse,$(MAKECMDGOALS)),)\n ++$(error The sparse and sparse-incr targets cannot be combined!)\n ++endif\n ++$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n ++\t$(cmd_run_sparse)\n ++else\n ++$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n ++\t$(cmd_run_sparse) >$@\n ++endif\n \n -.PHONY: sparse $(SP_OBJ)\n -+.PHONY: sparse\n ++.PHONY: sparse sparse-incr\n sparse: $(SP_OBJ)\n ++sparse-incr: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n + ifndef GCRYPT_SHA256\n @@ Makefile: clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n\n .gitignore | 1 +\n Makefile | 18 ++++++++++++++++--\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..b02250a50c4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -224,6 +224,7 @@\n *.lib\n *.res\n *.sln\n+*.sp\n *.suo\n *.ncb\n *.vcproj\ndiff --git a/Makefile b/Makefile\nindex a9f9b689f0c..59270d5b056 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2896,12 +2896,25 @@ check-sha1:: t/helper/test-tool$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+define cmd_run_sparse\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n \t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n+endef\n+\n+ifneq ($(filter sparse-incr,$(MAKECMDGOALS)),sparse-incr)\n+ifneq ($(filter sparse,$(MAKECMDGOALS)),)\n+$(error The sparse and sparse-incr targets cannot be combined!)\n+endif\n+$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+\t$(cmd_run_sparse)\n+else\n+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n+\t$(cmd_run_sparse) >$@\n+endif\n \n-.PHONY: sparse $(SP_OBJ)\n+.PHONY: sparse sparse-incr\n sparse: $(SP_OBJ)\n+sparse-incr: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n ifndef GCRYPT_SHA256\n@@ -3227,6 +3240,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n+\t$(RM) $(SP_OBJ)\n \t$(RM) $(HCC)\n \t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n-- \n2.33.0.1326.g5e4342b7bef\n\n"},{"id":"437222","messageId":"patch-v4-1.1-f31fa3e8282-20210928T014023Z-avarab@gmail.com","threadId":"56561","inReplyTo":"patch-v3-1.1-b6ba99ca4cc-20210928T011319Z-avarab@gmail.com","subject":"[PATCH v4] Makefile: add a non-.PHONY \"sparse-incr\" target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-28T01:43:31Z","receivedAt":"2021-09-28T01:44:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a \"sparse-incr\" target to compliment the existing \"sparse\"\ntarget. The new \"sparse-incr\" target doesn't depend on \"FORCE\", and\nwill create empty *.sp files as markers for the corresponding *.c file\nhaving been checked.\n\nThose *.sp files in turn depend on the *.o files, so we can be certain\nthat the dependencies are correct by either depending on all header\nfiles, or under \"COMPUTE_HEADER_DEPENDENCIES=yes\" the headers the\nrelevant file needs.\n\nThe \"sparse-incr\" target is slower on a fresh git.git checkout, as it\ndepends on the creation of the *.o files. But once the *.o and *.sp\nfiles are built it's able to incrementally update them. It's thus\nviable to run:\n\n make all sparse-incr\n\nAs part of a regular edit/compile/test cycle, or as a fast \"git rebase\n--exec\" command.\n\nOn my box (with -j8) the initial run of the \"sparse\" target takes ~5s,\nbut ~16s for \"sparse-incr\". When using CC=\"ccache cc\" the difference\nbetween the two is negligible.\n\nRunning e.g.:\n\n make grep.sp\n\nWill behave the same way and will always re-run \"cgcc\", we'll only use\nthe new dependency chain of \"sparse-incr\" is part of the MAKECMDGOALS.\n\nI think it would make sense to just remove the \"sparse\" target\nentirely, and to say that anyone who cares about the speed of an\ninitial \"sparse\" run should use \"CC='ccache cc'\". But per [1] and [2]\nthere are existing users of \"make sparse\" and \"make .sp\" that\nprefer the current semantics.\n\nI.e. per [2] want \"make .sp\" to *always* run \"sparse\", even\nthough a corresponding \"make .o\" would only re-run the \"real\"\ncompilation if needed. I don't think that makes any sense, especially\nin combination with DEVELOPER=1 which'll ensure that -Werror would\nhave made any errors in a \"make .sp\" sticky.\n\nBut since we have existing users relying on it, and I don't really\ncare at al about \"make .sp\", I just want an incremental target I\ncan use, let's create this new \"make sparse-incr\" instead of \"fixing\"\nthe existing \"make sparse\".\n\nSee 0bcd9ae85d7 (sparse: Fix errors due to missing target-specific\nvariables, 2011-04-21) for the modern implementation of the \"sparse\"\ntarget being changed here.\n\nAppending to $@ without a move is OK here because we're using the\n.DELETE_ON_ERROR Makefile feature. See 7b76d6bf221 (Makefile: add and\nuse the \".DELETE_ON_ERROR\" flag, 2021-06-29). GNU make ensures that on\nerror this file will be removed.\n\n1. https://lore.kernel.org/git/xmqqtuib199x.fsf@gitster.g/\n2. https://lore.kernel.org/git/457ec039-1e26-9da9-55f6-9ea79b962bfe@ramsayjones.plus.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nNot so long since v3, it was hopelessly broken, sorry. I was doing\nsome last-minute tweaking of the ifeq/ifneq conditions, so any target\nin the Makefile that wasn't \"sparse\" or \"sparse-incr\" broke.\n\nNow we do the right thing by just splitting up the check, if you\nsupply \"sparse\" and \"sparse-incr\" (in any order, and even among other\ntargets) we'll error, they're incompatible.\n\nThen we separately check if the \"sparse-incr\" target has been\nspecified, if it has we'll use the new dependency mechanism, if not\nwe'll use the old behavior.\n\nRange-diff against v3:\n1: b6ba99ca4cc ! 1: f31fa3e8282 Makefile: add a non-.PHONY \"sparse-incr\" target\n @@ Makefile: check-sha1:: t/helper/test-tool$X\n \t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n +endef\n +\n -+ifneq ($(filter sparse-incr,$(MAKECMDGOALS)),sparse-incr)\n -+ifneq ($(filter sparse,$(MAKECMDGOALS)),)\n ++ifeq ($(sort $(filter sparse sparse-incr,$(MAKECMDGOALS))),sparse sparse-incr)\n +$(error The sparse and sparse-incr targets cannot be combined!)\n +endif\n ++\n ++ifneq ($(filter sparse-incr,$(MAKECMDGOALS)),sparse-incr)\n +$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n +\t$(cmd_run_sparse)\n +else\n\n .gitignore | 1 +\n Makefile | 19 +++++++++++++++++--\n 2 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..b02250a50c4 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -224,6 +224,7 @@\n *.lib\n *.res\n *.sln\n+*.sp\n *.suo\n *.ncb\n *.vcproj\ndiff --git a/Makefile b/Makefile\nindex a9f9b689f0c..fd623523394 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2896,12 +2896,26 @@ check-sha1:: t/helper/test-tool$X\n \n SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))\n \n-$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+define cmd_run_sparse\n \t$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \\\n \t\t$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $<\n+endef\n+\n+ifeq ($(sort $(filter sparse sparse-incr,$(MAKECMDGOALS))),sparse sparse-incr)\n+$(error The sparse and sparse-incr targets cannot be combined!)\n+endif\n+\n+ifneq ($(filter sparse-incr,$(MAKECMDGOALS)),sparse-incr)\n+$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n+\t$(cmd_run_sparse)\n+else\n+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS\n+\t$(cmd_run_sparse) >$@\n+endif\n \n-.PHONY: sparse $(SP_OBJ)\n+.PHONY: sparse sparse-incr\n sparse: $(SP_OBJ)\n+sparse-incr: $(SP_OBJ)\n \n EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n ifndef GCRYPT_SHA256\n@@ -3227,6 +3241,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n+\t$(RM) $(SP_OBJ)\n \t$(RM) $(HCC)\n \t$(RM) -r bin-wrappers $(dep_dirs) $(compdb_dir) compile_commands.json\n \t$(RM) -r po/build/\n-- \n2.33.0.1326.g5e4342b7bef\n\n"},{"id":"437338","messageId":"xmqqy27gk321.fsf@gitster.g","threadId":"56561","inReplyTo":"patch-v4-1.1-f31fa3e8282-20210928T014023Z-avarab@gmail.com","subject":"Re: [PATCH v4] Makefile: add a non-.PHONY \"sparse-incr\" target","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-28T17:44:38Z","receivedAt":"2021-09-28T17:44:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I.e. per [2] want \"make .sp\" to *always* run \"sparse\", even\n> though a corresponding \"make .o\" would only re-run the \"real\"\n> compilation if needed.\n\nIt is different from my reading. If .c and what it includes\nhaven't changed, it would be fine not to run \"sparse\". The point of\nrunning \"make .sp\" is to see it fail if there is something\nproblematic. If there is any room for the word \"*always*\" to come\ninto the observation, it would be more like \"if we cannot make it\nfollow the usual dependency rules like the real compilation, we can\nlive with it always running, as it is fast enough\". If we can make\nit honor the dependencies, that would give the best of both worlds,\nand we do not have to add an extra target.\n\n"},{"id":"437347","messageId":"8735powkad.fsf@evledraar.gmail.com","threadId":"56561","inReplyTo":"xmqqy27gk321.fsf@gitster.g","subject":"Re: [PATCH v4] Makefile: add a non-.PHONY \"sparse-incr\" target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-28T19:45:19Z","receivedAt":"2021-09-28T19:51:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Sep 28 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> I.e. per [2] want \"make .sp\" to *always* run \"sparse\", even\n>> though a corresponding \"make .o\" would only re-run the \"real\"\n>> compilation if needed.\n>\n> It is different from my reading. If .c and what it includes\n> haven't changed, it would be fine not to run \"sparse\".\n\nI was attempting to summarize this part of Ramsay Jones's comments in \n\n [...] Prior to this patch series, 'make .sp' would _always_\n run sparse over the file - it would not depend on the 'mtime' or\n existence of any other file, or run the compiler (and wouldn't leave\n any 'droppings' either). I liked that! :D\n\n> The point of\n> running \"make .sp\" is to see it fail if there is something\n> problematic. If there is any room for the word \"*always*\" to come\n> into the observation, it would be more like \"if we cannot make it\n> follow the usual dependency rules like the real compilation, we can\n> live with it always running, as it is fast enough\". If we can make\n> it honor the dependencies, that would give the best of both worlds,\n> and we do not have to add an extra target.\n\n...which I think describe something different than what you're\ndescribing here.\n\nI.e. I was under the impression that you didn't mind the incremental\npart of it (but Ramsey did), but just didn't want the initial \"make\nsparse\" to take much longer due to the *.o file compilation.\n\nIn any case, we can always tweak the \"make .sp\" later. I don't\nmind it working the way it did before, I think the only time I ever make\nindividual files is when generating the assembly output for them. So as\nlong as I've got a \"make sparse-incr\" target I can use.\n\nDo you think that approach in this v4 is OK to move forward?\n"}]}