{"thread":{"id":"57923","subject":"[PATCH] Makefile: build 'gitweb' in the default target","startedAt":"2022-05-25T20:57:04Z","lastAt":"2022-06-28T10:16:29Z","messageCount":31,"participants":["SZEDER Gábor","Ævar Arnfjörð Bjarmason","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"456116","messageId":"20220525205651.825669-1-szeder.dev@gmail.com","threadId":"57923","inReplyTo":null,"subject":"[PATCH] Makefile: build 'gitweb' in the default target","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2022-05-25T20:56:51Z","receivedAt":"2022-05-25T20:57:04Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Our Makefile's default target used to build 'gitweb', though\nindirectly: the 'all' target depended on 'git-instaweb', which in turn\ndepended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\nbetween git-instaweb and gitweb, 2015-05-29) removed the latter\ndependency, and for good reasons (quoting its commit message):\n\n \"1. git-instaweb has no build-time dependency on gitweb; it\n is a run-time dependency\n\n 2. gitweb is a directory that we want to recursively make\n in. As a result, its recipe is marked .PHONY, which\n causes \"make\" to rebuild git-instaweb every time it is\n run.\"\n\nSince then a simple 'make' doesn't build 'gitweb'.\n\nLuckily, installing 'gitweb' is not broken: although 'make install'\ndoesn't depend on the 'gitweb' target, it runs 'make -C gitweb\ninstall' unconditionally, which does generate all the necessary files\nfor 'gitweb' and installs them. However, if someone runs 'make &&\nsudo make install', then those files in the 'gitweb' directory will be\ngenerated and owned by root, which is not nice.\n\nList 'gitweb' as a direct dependency of the default target, so a plain\n'make' will build it.\n\nSigned-off-by: SZEDER Gábor \n---\n Makefile | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex f8bccfab5e..ee74892b33 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2188,6 +2188,8 @@ ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n \n+all:: gitweb\n+\n all::\n ifndef NO_TCLTK\n \t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all\n-- \n2.36.1.427.gb7c35dfc0c\n\n"},{"id":"456135","messageId":"220526.86k0a96sv2.gmgdl@evledraar.gmail.com","threadId":"57923","inReplyTo":"20220525205651.825669-1-szeder.dev@gmail.com","subject":"Re: [PATCH] Makefile: build 'gitweb' in the default target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-26T00:14:33Z","receivedAt":"2022-05-26T00:27:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, May 25 2022, SZEDER Gábor wrote:\n\n> Our Makefile's default target used to build 'gitweb', though\n> indirectly: the 'all' target depended on 'git-instaweb', which in turn\n> depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\n> between git-instaweb and gitweb, 2015-05-29) removed the latter\n> dependency, and for good reasons (quoting its commit message):\n>\n> \"1. git-instaweb has no build-time dependency on gitweb; it\n> is a run-time dependency\n>\n> 2. gitweb is a directory that we want to recursively make\n> in. As a result, its recipe is marked .PHONY, which\n> causes \"make\" to rebuild git-instaweb every time it is\n> run.\"\n>\n> Since then a simple 'make' doesn't build 'gitweb'.\n>\n> Luckily, installing 'gitweb' is not broken: although 'make install'\n> doesn't depend on the 'gitweb' target, it runs 'make -C gitweb\n> install' unconditionally, which does generate all the necessary files\n> for 'gitweb' and installs them. However, if someone runs 'make &&\n> sudo make install', then those files in the 'gitweb' directory will be\n> generated and owned by root, which is not nice.\n>\n> List 'gitweb' as a direct dependency of the default target, so a plain\n> 'make' will build it.\n>\n> Signed-off-by: SZEDER Gábor \n> ---\n> Makefile | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/Makefile b/Makefile\n> index f8bccfab5e..ee74892b33 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2188,6 +2188,8 @@ ifneq (,$X)\n> \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n> endif\n> \n> +all:: gitweb\n> +\n> all::\n> ifndef NO_TCLTK\n> \t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all\n\nIn various recent patches & some upcoming ones I plan to submit I've\nbeen trying to get the runtime of a noop \"make\" runs down, which really\nhelps e.g. with \"git rebase -x make ...\" running faster on a large\nseries.\n\nWhile you're right that this wasn't intentional to begin with, we have\nlacked the \"gitweb\" as part of the default target since v2.4.5 now, and\nadding it back is a major performance regression on noop \"make\" runs:\n\t\n\t$ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5\n\tBenchmark 1: make NO_TCLTK=Y' in 'HEAD~1\n\t Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]\n\t Range (min … max): 102.2 ms … 105.2 ms 5 runs\n\t \n\tBenchmark 2: make NO_TCLTK=Y' in 'HEAD~0\n\t Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]\n\t Range (min … max): 189.2 ms … 193.3 ms 5 runs\n\t \n\tBenchmark 3: make NO_TCLTK=' in 'HEAD~1\n\t Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]\n\t Range (min … max): 266.7 ms … 277.3 ms 5 runs\n\t \n\tBenchmark 4: make NO_TCLTK=' in 'HEAD~0\n\t Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]\n\t Range (min … max): 356.6 ms … 360.0 ms 5 runs\n\t \n\tSummary\n\t 'make NO_TCLTK=Y' in 'HEAD~1' ran\n\t 1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'\n\t 2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'\n\t 3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'\n\nI.e. this is with your patch here applied as HEAD~0 and HEAD~1 being\n'master'.\n\nI think given that that a better solution would be to just declare this\nas a feature at this point, especially as gitweb/INSTALL notes that the\nway to install it is:\n\n $ make prefix=/usr gitweb ;# as yourself\n # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root\n\nOr we could just fold gitweb/Makefile into the main Makefile, unlike\ngitk and git-gui it's not externally maintained, and most of it is\nshimmying to work around not being part of the main Makefile (which it\nstrongly inter-depends on anyway).\n"},{"id":"456151","messageId":"Yo8y3AHWa3PChLwd@coredump.intra.peff.net","threadId":"57923","inReplyTo":"220526.86k0a96sv2.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: build 'gitweb' in the default target","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-05-26T07:57:16Z","receivedAt":"2022-05-26T07:57:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > Our Makefile's default target used to build 'gitweb', though\n> > indirectly: the 'all' target depended on 'git-instaweb', which in turn\n> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\n> > between git-instaweb and gitweb, 2015-05-29) removed the latter\n> > dependency, and for good reasons (quoting its commit message):\n> [...]\n> In various recent patches & some upcoming ones I plan to submit I've\n> been trying to get the runtime of a noop \"make\" runs down, which really\n> helps e.g. with \"git rebase -x make ...\" running faster on a large\n> series.\n> \n> While you're right that this wasn't intentional to begin with, we have\n> lacked the \"gitweb\" as part of the default target since v2.4.5 now, and\n> adding it back is a major performance regression on noop \"make\" runs:\n\nYes, I don't think building gitweb is worth the performance cost.\nSpeeding things up was part of my original goal in e25c7cc146. It would\nbe one thing if this were a recent change and somebody might be broken\nor confused by it not being built by default. But after 7 years, I think\nthe question is: why _would_ we want to change the status quo and build\ngitweb by default?\n\nTo exercise its Makefile for bugs, I guess, but IMHO it is not worth\ninflicting that on random developers. People who care about gitweb (if\nany) can build it themselves. I'd be even happier if it were just\ncarried in its own tree.\n\n-Peff\n"},{"id":"456233","messageId":"20220526213305.GA1707@szeder.dev","threadId":"57923","inReplyTo":"220526.86k0a96sv2.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: build 'gitweb' in the default target","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2022-05-26T21:33:05Z","receivedAt":"2022-05-26T21:33:13Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, May 25 2022, SZEDER Gábor wrote:\n> \n> > Our Makefile's default target used to build 'gitweb', though\n> > indirectly: the 'all' target depended on 'git-instaweb', which in turn\n> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\n> > between git-instaweb and gitweb, 2015-05-29) removed the latter\n> > dependency, and for good reasons (quoting its commit message):\n> >\n> > \"1. git-instaweb has no build-time dependency on gitweb; it\n> > is a run-time dependency\n> >\n> > 2. gitweb is a directory that we want to recursively make\n> > in. As a result, its recipe is marked .PHONY, which\n> > causes \"make\" to rebuild git-instaweb every time it is\n> > run.\"\n> >\n> > Since then a simple 'make' doesn't build 'gitweb'.\n> >\n> > Luckily, installing 'gitweb' is not broken: although 'make install'\n> > doesn't depend on the 'gitweb' target, it runs 'make -C gitweb\n> > install' unconditionally, which does generate all the necessary files\n> > for 'gitweb' and installs them. However, if someone runs 'make &&\n> > sudo make install', then those files in the 'gitweb' directory will be\n> > generated and owned by root, which is not nice.\n> >\n> > List 'gitweb' as a direct dependency of the default target, so a plain\n> > 'make' will build it.\n> >\n> > Signed-off-by: SZEDER Gábor \n> > ---\n> > Makefile | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/Makefile b/Makefile\n> > index f8bccfab5e..ee74892b33 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -2188,6 +2188,8 @@ ifneq (,$X)\n> > \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n> > endif\n> > \n> > +all:: gitweb\n> > +\n> > all::\n> > ifndef NO_TCLTK\n> > \t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all\n> \n> In various recent patches & some upcoming ones I plan to submit I've\n> been trying to get the runtime of a noop \"make\" runs down, which really\n> helps e.g. with \"git rebase -x make ...\" running faster on a large\n> series.\n> \n> While you're right that this wasn't intentional to begin with, we have\n> lacked the \"gitweb\" as part of the default target since v2.4.5 now, and\n> adding it back is a major performance regression on noop \"make\" runs:\n\nI think that generating stuff, potentially as root, during 'make\ninstall' is a more severe regression, than this noop make slowdown,\nwhich in practice tends to be lost in the noise anyway. Even in an\nunrealistic case (it doesn't modify any C source files explicitly, let\nalone a frequently included header file) like this:\n\n $ git checkout fddc3b420f^\n $ make\n [...]\n $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done\n $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10\n [...]\n real\t0m31.026s\n user\t0m46.897s\n sys\t0m11.492s\n $ git checkout fddc3b420f\n $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done\n $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10\n [...]\n real\t0m30.865s\n user\t0m48.315s\n sys\t0m12.125s\n\nHrm, it actually ended up slightly faster.\n\n> \t$ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5\n> \tBenchmark 1: make NO_TCLTK=Y' in 'HEAD~1\n> \t Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]\n> \t Range (min … max): 102.2 ms … 105.2 ms 5 runs\n> \t \n> \tBenchmark 2: make NO_TCLTK=Y' in 'HEAD~0\n> \t Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]\n> \t Range (min … max): 189.2 ms … 193.3 ms 5 runs\n> \t \n> \tBenchmark 3: make NO_TCLTK=' in 'HEAD~1\n> \t Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]\n> \t Range (min … max): 266.7 ms … 277.3 ms 5 runs\n> \t \n> \tBenchmark 4: make NO_TCLTK=' in 'HEAD~0\n> \t Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]\n> \t Range (min … max): 356.6 ms … 360.0 ms 5 runs\n> \t \n> \tSummary\n> \t 'make NO_TCLTK=Y' in 'HEAD~1' ran\n> \t 1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'\n> \t 2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'\n> \t 3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'\n> \n> I.e. this is with your patch here applied as HEAD~0 and HEAD~1 being\n> 'master'.\n> \n> I think given that that a better solution would be to just declare this\n> as a feature at this point\n\nAs long as 'make install' installs 'gitweb', I don't think that's an\noption.\n\n> especially as gitweb/INSTALL notes that the\n> way to install it is:\n> \n> $ make prefix=/usr gitweb ;# as yourself\n> # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root\n\nOr are you suggesting not to install 'gitweb' during 'make install'?\nI'm fine with that, but I doubt I will argue about it convincingly in\na commit message.\n\n> Or we could just fold gitweb/Makefile into the main Makefile, unlike\n> gitk and git-gui it's not externally maintained, and most of it is\n> shimmying to work around not being part of the main Makefile (which it\n> strongly inter-depends on anyway).\n"},{"id":"456272","messageId":"220527.861qwf489s.gmgdl@evledraar.gmail.com","threadId":"57923","inReplyTo":"20220526213305.GA1707@szeder.dev","subject":"Re: [PATCH] Makefile: build 'gitweb' in the default target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-27T09:23:29Z","receivedAt":"2022-05-27T09:47:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 26 2022, SZEDER Gábor wrote:\n\n> On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Wed, May 25 2022, SZEDER Gábor wrote:\n>> \n>> > Our Makefile's default target used to build 'gitweb', though\n>> > indirectly: the 'all' target depended on 'git-instaweb', which in turn\n>> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\n>> > between git-instaweb and gitweb, 2015-05-29) removed the latter\n>> > dependency, and for good reasons (quoting its commit message):\n>> >\n>> > \"1. git-instaweb has no build-time dependency on gitweb; it\n>> > is a run-time dependency\n>> >\n>> > 2. gitweb is a directory that we want to recursively make\n>> > in. As a result, its recipe is marked .PHONY, which\n>> > causes \"make\" to rebuild git-instaweb every time it is\n>> > run.\"\n>> >\n>> > Since then a simple 'make' doesn't build 'gitweb'.\n>> >\n>> > Luckily, installing 'gitweb' is not broken: although 'make install'\n>> > doesn't depend on the 'gitweb' target, it runs 'make -C gitweb\n>> > install' unconditionally, which does generate all the necessary files\n>> > for 'gitweb' and installs them. However, if someone runs 'make &&\n>> > sudo make install', then those files in the 'gitweb' directory will be\n>> > generated and owned by root, which is not nice.\n>> >\n>> > List 'gitweb' as a direct dependency of the default target, so a plain\n>> > 'make' will build it.\n>> >\n>> > Signed-off-by: SZEDER Gábor \n>> > ---\n>> > Makefile | 2 ++\n>> > 1 file changed, 2 insertions(+)\n>> >\n>> > diff --git a/Makefile b/Makefile\n>> > index f8bccfab5e..ee74892b33 100644\n>> > --- a/Makefile\n>> > +++ b/Makefile\n>> > @@ -2188,6 +2188,8 @@ ifneq (,$X)\n>> > \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n>> > endif\n>> > \n>> > +all:: gitweb\n>> > +\n>> > all::\n>> > ifndef NO_TCLTK\n>> > \t$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all\n>> \n>> In various recent patches & some upcoming ones I plan to submit I've\n>> been trying to get the runtime of a noop \"make\" runs down, which really\n>> helps e.g. with \"git rebase -x make ...\" running faster on a large\n>> series.\n>> \n>> While you're right that this wasn't intentional to begin with, we have\n>> lacked the \"gitweb\" as part of the default target since v2.4.5 now, and\n>> adding it back is a major performance regression on noop \"make\" runs:\n>\n> I think that generating stuff, potentially as root, during 'make\n> install' is a more severe regression, than this noop make slowdown,\n> which in practice tends to be lost in the noise anyway.\n\nIt really isn't lost in the noise, e.g. a common use-case of mine is to\nhave let's say a 10 patch series that I want to frequently run a spot\ncheck like this on:\n\n git rebase -i -x 'make'\n\nOn my system before your patch (this is with my config.mak, so it has\ne.g. NO_TCLTK=Y) it's ~105ms per run, with your change ~200ms[1].\n\t\nSo that's 1s before of \"make\" overhead, now 2s. Is it the end of the\nworld? No, but it does add up. In particular with ccache we can end up\nmostly spending time on make itself deciding what it's going to do.\n\nFor comparison running EDITOR=cat git rebase -i -x 'echo make' HEAD~10\ntakes around 100ms in total for me.\n\n> unrealistic case (it doesn't modify any C source files explicitly, let\n> alone a frequently included header file) like this:\n\nYeah that's fair, of course no-op runs are where it matters the\nmost. But e.g. for forcing a hook.c change (including re-linking \"git\")\nit's 10% slower[2] overall if we use ccache.\n\n> $ git checkout fddc3b420f^\n> $ make\n> [...]\n> $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done\n> $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10\n> [...]\n> real\t0m31.026s\n> user\t0m46.897s\n> sys\t0m11.492s\n> $ git checkout fddc3b420f\n> $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done\n> $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10\n> [...]\n> real\t0m30.865s\n> user\t0m48.315s\n> sys\t0m12.125s\n>\n> Hrm, it actually ended up slightly faster.\n\nI should have said that I create a \"version\" file in my checkout dir,\ne.g.: 2.36.GIT-dev\n\nOtherwise you end up with a guarnteed recompile-re-link every time HEAD\nchanges, so it's likely lost in that noise.\n\nI just run this (via a script):\n\n echo $(/usr/bin/git grep -h -o -P '(?<=^DEF_VER=v).*' 'HEAD:GIT-VERSION-GEN')-dev\n\nAnd echo it to the top-level \"version\". Aside from what we're discussing\nhere you might want to try that, your \"git version\" won't be correct,\nbut for that low cost (I do generate it for the version I actually\ninstall) you'll get actual no-op runs in the case of e.g. only modifying\ntests.\n\n>> \t$ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5\n>> \tBenchmark 1: make NO_TCLTK=Y' in 'HEAD~1\n>> \t Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]\n>> \t Range (min … max): 102.2 ms … 105.2 ms 5 runs\n>> \t \n>> \tBenchmark 2: make NO_TCLTK=Y' in 'HEAD~0\n>> \t Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]\n>> \t Range (min … max): 189.2 ms … 193.3 ms 5 runs\n>> \t \n>> \tBenchmark 3: make NO_TCLTK=' in 'HEAD~1\n>> \t Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]\n>> \t Range (min … max): 266.7 ms … 277.3 ms 5 runs\n>> \t \n>> \tBenchmark 4: make NO_TCLTK=' in 'HEAD~0\n>> \t Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]\n>> \t Range (min … max): 356.6 ms … 360.0 ms 5 runs\n>> \t \n>> \tSummary\n>> \t 'make NO_TCLTK=Y' in 'HEAD~1' ran\n>> \t 1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'\n>> \t 2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'\n>> \t 3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'\n>> \n>> I.e. this is with your patch here applied as HEAD~0 and HEAD~1 being\n>> 'master'.\n>> \n>> I think given that that a better solution would be to just declare this\n>> as a feature at this point\n>\n> As long as 'make install' installs 'gitweb', I don't think that's an\n> option.\n\nYes, I'm not saying that we shouldn't fix this no matter what, but just\nsuggesting that perhaps we come up with a better solution.\n\n>> especially as gitweb/INSTALL notes that the\n>> way to install it is:\n>> \n>> $ make prefix=/usr gitweb ;# as yourself\n>> # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root\n>\n> Or are you suggesting not to install 'gitweb' during 'make install'?\n> I'm fine with that, but I doubt I will argue about it convincingly in\n> a commit message.\n\nWe could also:\n\n * $(error out if install is in MAKECMDGOALS and we don't have those\n generated files, that's a few-line ifdef change to the Makefile,\n \"build it first\".\n\n * Just fold gitweb/Makefile into the top-level Makefile\n\n>> Or we could just fold gitweb/Makefile into the main Makefile, unlike\n>> gitk and git-gui it's not externally maintained, and most of it is\n>> shimmying to work around not being part of the main Makefile (which it\n>> strongly inter-depends on anyway).\n\n1.\n\t$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make' 'make'\n\tBenchmark 1: make' in 'HEAD~1\n\t Time (mean ± σ): 106.9 ms ± 1.3 ms [User: 86.0 ms, System: 33.1 ms]\n\t Range (min … max): 105.3 ms … 110.2 ms 27 runs\n\t\n\tBenchmark 2: make' in 'HEAD~0\n\t Time (mean ± σ): 199.6 ms ± 2.5 ms [User: 162.7 ms, System: 57.3 ms]\n\t Range (min … max): 197.4 ms … 207.4 ms 15 runs\n\t\n\tSummary\n\t 'make' in 'HEAD~1' ran\n\t 1.87 ± 0.03 times faster than 'make' in 'HEAD~0'\n2.\n\t\n\t$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make' 'make -W hook.c NO_TCLTK=Y CC=\"ccache cc\"' --warmup 1 -r 5\n\tBenchmark 1: make -W hook.c NO_TCLTK=Y CC=\"ccache cc\"' in 'HEAD~1\n\t Time (mean ± σ): 742.7 ms ± 7.1 ms [User: 1965.5 ms, System: 561.6 ms]\n\t Range (min … max): 730.3 ms … 748.2 ms 5 runs\n\t \n\tBenchmark 2: make -W hook.c NO_TCLTK=Y CC=\"ccache cc\"' in 'HEAD~0\n\t Time (mean ± σ): 819.2 ms ± 7.4 ms [User: 2013.9 ms, System: 583.3 ms]\n\t Range (min … max): 811.0 ms … 830.0 ms 5 runs\n\t \n\tSummary\n\t 'make -W hook.c NO_TCLTK=Y CC=\"ccache cc\"' in 'HEAD~1' ran\n\t 1.10 ± 0.01 times faster than 'make -W hook.c NO_TCLTK=Y CC=\"ccache cc\"' in 'HEAD~0'\n\t\n"},{"id":"456411","messageId":"patch-v2-1.7-14361617ca6-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:54Z","receivedAt":"2022-05-31T17:46:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the '.PHONY' definition so that it's split up and accompanies the\nrelevant as they're defined. This will make a subsequent diff smaller\nas we'll remove some of these, and won't need to re-edit the\nnow-removed '.PHONY' line.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex f13e23c4de4..abb5c9f9ab6 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -1,5 +1,6 @@\n # The default target of this Makefile is...\n all::\n+.PHONY: all\n \n # Define V=1 to have a more verbose compile.\n #\n@@ -45,6 +46,7 @@ HIGHLIGHT_BIN = highlight\n -include config.mak\n \n # determine version\n+.PHONY: .FORCE-GIT-VERSION-FILE\n ../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n \n@@ -152,6 +154,7 @@ GITWEB_REPLACE = \\\n \t-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n \t-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'\n \n+.PHONY: FORCE\n GITWEB-BUILD-OPTIONS: FORCE\n \t@rm -f $@+\n \t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n@@ -171,15 +174,18 @@ static/gitweb.js: $(GITWEB_JSLIB_FILES)\n \n ### Testing rules\n \n+.PHONY: test\n test:\n \t$(MAKE) -C ../t gitweb-test\n \n+.PHONY: test-installed\n test-installed:\n \tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n \t\t$(MAKE) -C ../t gitweb-test\n \n ### Installation rules\n \n+.PHONY: install\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n@@ -188,10 +194,8 @@ install: all\n \n ### Cleaning rules\n \n+.PHONY: clean\n clean:\n \t$(RM) gitweb.cgi static/gitweb.js \\\n \t\tstatic/gitweb.min.js static/gitweb.min.css \\\n \t\tGITWEB-BUILD-OPTIONS\n-\n-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n-\n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456412","messageId":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"220527.861qwf489s.gmgdl@evledraar.gmail.com","subject":"[PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:53Z","receivedAt":"2022-05-31T17:46:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The $subject is a proposed re-roll of SZEDER's\nhttps://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;\nAs noted downthread of that fix having the Makefile invoke \"make -C\ngitweb\" again would slow us down on NOOP runs by quite a bit.\n\nThis series fixes the same regression with an amended 7/7 version of\nSZEDER's fix, but starts out by having the top-level Makefile include\nthe gitweb/Makefile.\n\nThis series is smaller than it looks, most of the commits preceding\n5-6/7 are there to make that diff smaller and easier to read, by\nsplitting up earlier changes into non-functional changes.\n\nFor this re-roll the equivalent of the \"git hyperfine\" command I\nposted in [2] will return;\n\n\tSummary\n\t 'make NO_TCLTK=Y' in 'origin/master' ran\n\t 1.00 ± 0.11 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'\n\t 2.64 ± 0.26 times faster than 'make NO_TCLTK=' in 'origin/master'\n\t 2.64 ± 0.24 times faster than 'make NO_TCLTK=' in 'HEAD~0'\n\nI.e. we are no slower or faster than before, but now \"make && sudo\nmake install-gitweb\" will only copy already-generated files from the\n\"make\" command in the \"sudo\" step, as intended.\n\n1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com\n2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/\n\nSZEDER Gábor (1):\n Makefile: build 'gitweb' in the default target\n\nÆvar Arnfjörð Bjarmason (6):\n gitweb/Makefile: define all .PHONY prerequisites inline\n gitweb/Makefile: add a $(GITWEB_ALL) variable\n gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars\n gitweb/Makefile: prepare to merge into top-level Makefile\n gitweb: remove \"test\" and \"test-installed\" targets\n gitweb/Makefile: include in top-level Makefile\n\n Makefile | 24 ++++----\n gitweb/Makefile | 143 +++++++++++++++---------------------------------\n t/Makefile | 4 --\n 3 files changed, 59 insertions(+), 112 deletions(-)\n\nRange-diff against v1:\n1: 1bbffa8a2b6 < -: ----------- Makefile: build 'gitweb' in the default target\n-: ----------- > 1: 14361617ca6 gitweb/Makefile: define all .PHONY prerequisites inline\n-: ----------- > 2: 7d920a13518 gitweb/Makefile: add a $(GITWEB_ALL) variable\n-: ----------- > 3: e14a5b73061 gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars\n-: ----------- > 4: 02e26ca8ce2 gitweb/Makefile: prepare to merge into top-level Makefile\n-: ----------- > 5: caf376f3dd9 gitweb: remove \"test\" and \"test-installed\" targets\n-: ----------- > 6: b423cd58f6b gitweb/Makefile: include in top-level Makefile\n-: ----------- > 7: 69428540886 Makefile: build 'gitweb' in the default target\n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456413","messageId":"patch-v2-2.7-7d920a13518-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 2/7] gitweb/Makefile: add a $(GITWEB_ALL) variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:55Z","receivedAt":"2022-05-31T17:46:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Declare the targets that the \"all\" target depends on with a new\n$(GITWEB_ALL) variable. This will help to reduce churn in subsequent\ncommits.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex abb5c9f9ab6..733b60f925e 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -54,6 +54,11 @@ ifneq ($(MAKECMDGOALS),clean)\n -include ../GIT-VERSION-FILE\n endif\n \n+# What targets we'll add to 'all' for \"make gitweb\"\n+GITWEB_ALL =\n+GITWEB_ALL += gitweb.cgi\n+GITWEB_ALL += static/gitweb.js\n+\n ### Build rules\n \n SHELL_PATH ?= $(SHELL)\n@@ -92,7 +97,7 @@ ifndef V\n endif\n endif\n \n-all:: gitweb.cgi static/gitweb.js\n+all:: $(GITWEB_ALL)\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456414","messageId":"patch-v2-3.7-e14a5b73061-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 3/7] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:56Z","receivedAt":"2022-05-31T17:46:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the variable definitions for the $(GITWEB_CSS) and $(GITWEB_JS)\nso that we have a clear separation between what we use as \"in\" files,\nv.s. our \"min\" files. We can now make the appending to $(GITWEB_FILES)\nunconditional, since $(GITWEB_{JS,CSS}) is either the \"min\" or\nnon-\"min\" version. This reduces the duplication within the file.\n\nWhile we're at it let's initialize \"GITWEB_JSLIB_FILES\" as we normally\ndo with such variables.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 35 ++++++++++++++++++-----------------\n 1 file changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 733b60f925e..a8752c1f11c 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -31,10 +31,12 @@ GITWEB_STRICT_EXPORT =\n GITWEB_BASE_URL =\n GITWEB_LIST =\n GITWEB_HOMETEXT = indextext.html\n-GITWEB_CSS = static/gitweb.css\n+GITWEB_CSS_IN = static/gitweb.css\n+GITWEB_CSS = $(GITWEB_CSS_IN)\n GITWEB_LOGO = static/git-logo.png\n GITWEB_FAVICON = static/git-favicon.png\n-GITWEB_JS = static/gitweb.js\n+GITWEB_JS_IN = static/gitweb.js\n+GITWEB_JS = $(GITWEB_JS_IN)\n GITWEB_SITE_HTML_HEAD_STRING =\n GITWEB_SITE_HEADER =\n GITWEB_SITE_FOOTER =\n@@ -57,7 +59,7 @@ endif\n # What targets we'll add to 'all' for \"make gitweb\"\n GITWEB_ALL =\n GITWEB_ALL += gitweb.cgi\n-GITWEB_ALL += static/gitweb.js\n+GITWEB_ALL += $(GITWEB_JS)\n \n ### Build rules\n \n@@ -101,25 +103,23 @@ all:: $(GITWEB_ALL)\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n+GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n-GITWEB_FILES += static/gitweb.min.js\n-GITWEB_JS = static/gitweb.min.js\n-all:: static/gitweb.min.js\n-static/gitweb.min.js: static/gitweb.js GITWEB-BUILD-OPTIONS\n+GITWEB_JS = $(GITWEB_JS_MIN)\n+all:: $(GITWEB_JS_MIN)\n+$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n-else\n-GITWEB_FILES += static/gitweb.js\n endif\n+GITWEB_FILES += $(GITWEB_JS)\n \n+GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n-GITWEB_FILES += static/gitweb.min.css\n-GITWEB_CSS = static/gitweb.min.css\n-all:: static/gitweb.min.css\n-static/gitweb.min.css: static/gitweb.css GITWEB-BUILD-OPTIONS\n+GITWEB_CSS = $(GITWEB_CSS_MIN)\n+all:: $(GITWEB_CSS_MIN)\n+$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n-else\n-GITWEB_FILES += static/gitweb.css\n endif\n+GITWEB_FILES += $(GITWEB_CSS)\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n@@ -127,6 +127,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png\n #\n # js/lib/common-lib.js should be always first, then js/lib/*.js,\n # then the rest of files; js/gitweb.js should be last (if it exists)\n+GITWEB_JSLIB_FILES =\n GITWEB_JSLIB_FILES += static/js/lib/common-lib.js\n GITWEB_JSLIB_FILES += static/js/lib/datetime.js\n GITWEB_JSLIB_FILES += static/js/lib/cookies.js\n@@ -201,6 +202,6 @@ install: all\n \n .PHONY: clean\n clean:\n-\t$(RM) gitweb.cgi static/gitweb.js \\\n-\t\tstatic/gitweb.min.js static/gitweb.min.css \\\n+\t$(RM) gitweb.cgi $(GITWEB_JS_IN) \\\n+\t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n \t\tGITWEB-BUILD-OPTIONS\n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456415","messageId":"patch-v2-4.7-02e26ca8ce2-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 4/7] gitweb/Makefile: prepare to merge into top-level Makefile","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:57Z","receivedAt":"2022-05-31T17:46:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since the \"gitweb/Makefile\" was split out from the top-level Makefile\nin 62331ef1637 (gitweb: Makefile improvements, 2010-01-30) we've kept\nthe inter-dependencies between the two, and worse have dealt with a\nlot of duplication as a result.\n\nIn preparation for merging the two again add a MAK_DIR_GITWEB variable\nto various rules in it. This will allow us to set this variable to\n\"gitweb/\" as we include it in the top-level Makefile, which will\nminimize the size of the subsequent diff.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex a8752c1f11c..74e896767e9 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -2,6 +2,8 @@\n all::\n .PHONY: all\n \n+MAK_DIR_GITWEB =\n+\n # Define V=1 to have a more verbose compile.\n #\n # Define JSMIN to point to JavaScript minifier that functions as\n@@ -106,8 +108,9 @@ GITWEB_PROGRAMS = gitweb.cgi\n GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n GITWEB_JS = $(GITWEB_JS_MIN)\n-all:: $(GITWEB_JS_MIN)\n-$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS\n+all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n endif\n GITWEB_FILES += $(GITWEB_JS)\n@@ -115,8 +118,9 @@ GITWEB_FILES += $(GITWEB_JS)\n GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n GITWEB_CSS = $(GITWEB_CSS_MIN)\n-all:: $(GITWEB_CSS_MIN)\n-$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS\n+all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n endif\n GITWEB_FILES += $(GITWEB_CSS)\n@@ -161,19 +165,20 @@ GITWEB_REPLACE = \\\n \t-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'\n \n .PHONY: FORCE\n-GITWEB-BUILD-OPTIONS: FORCE\n+$(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS: FORCE\n \t@rm -f $@+\n \t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n \t@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@\n \n-gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n \t\t$(GITWEB_REPLACE) $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-static/gitweb.js: $(GITWEB_JSLIB_FILES)\n+$(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tcat $^ >$@+ && \\\n \tmv $@+ $@\n@@ -194,14 +199,16 @@ test-installed:\n .PHONY: install\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n-\t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n+\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \\\n+\t\t'$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n-\t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n+\t$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \\\n+\t\t'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n ### Cleaning rules\n \n .PHONY: clean\n clean:\n-\t$(RM) gitweb.cgi $(GITWEB_JS_IN) \\\n+\t$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \\\n \t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n-\t\tGITWEB-BUILD-OPTIONS\n+\t\tGITWEB-BUILD-OPTIONS)\n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456416","messageId":"patch-v2-5.7-caf376f3dd9-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 5/7] gitweb: remove \"test\" and \"test-installed\" targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:58Z","receivedAt":"2022-05-31T17:46:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the special \"test\" targets for gitweb added in\n958a8467216 (gitweb/Makefile: Add 'test' and 'test-installed' targets,\n2010-09-26). Unlike e.g. \"contrib/scalar\" and \"contrib/subtree\" the\n\"gitweb\" tests themselves live in our top-level t/ directory.\n\nIt therefore doesn't make sense to maintain this indirection, no more\nthan it would to have a \"git-send-email-test\". By dropping it we'll\nalso free other tests to use the t95*.sh prefix.\n\nThese removed targets are unlikely to be used by anyone, and to the\nextent that they are we can easily use an invocation like this\ninstead:\n\n\tmake test T='t[0-9]*gitweb*.sh'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 11 -----------\n t/Makefile | 4 ----\n 2 files changed, 15 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 74e896767e9..eaf0cfcf80e 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -183,17 +183,6 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_\n \tcat $^ >$@+ && \\\n \tmv $@+ $@\n \n-### Testing rules\n-\n-.PHONY: test\n-test:\n-\t$(MAKE) -C ../t gitweb-test\n-\n-.PHONY: test-installed\n-test-installed:\n-\tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n-\t\t$(MAKE) -C ../t gitweb-test\n-\n ### Installation rules\n \n .PHONY: install\ndiff --git a/t/Makefile b/t/Makefile\nindex 056ce55dcc9..7f56e52f767 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,7 +35,6 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n CHAINLINTTMP_SQ = $(subst ','\\'',$(CHAINLINTTMP))\n \n T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n-TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))\n TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n@@ -112,9 +111,6 @@ aggregate-results:\n \t\techo \"$$f\"; \\\n \tdone | '$(SHELL_PATH_SQ)' ./aggregate-results.sh\n \n-gitweb-test:\n-\t$(MAKE) $(TGITWEB)\n-\n valgrind:\n \t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n \n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456417","messageId":"patch-v2-7.7-69428540886-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 7/7] Makefile: build 'gitweb' in the default target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:46:00Z","receivedAt":"2022-05-31T17:46:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: SZEDER Gábor \n\nOur Makefile's default target used to build 'gitweb', though\nindirectly: the 'all' target depended on 'git-instaweb', which in turn\ndepended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\nbetween git-instaweb and gitweb, 2015-05-29) removed the latter\ndependency, and for good reasons (quoting its commit message):\n\n \"1. git-instaweb has no build-time dependency on gitweb; it\n is a run-time dependency\n\n 2. gitweb is a directory that we want to recursively make\n in. As a result, its recipe is marked .PHONY, which\n causes \"make\" to rebuild git-instaweb every time it is\n run.\"\n\nSince then a simple 'make' doesn't build 'gitweb'.\n\nLuckily, installing 'gitweb' is not broken: although 'make install'\ndoesn't depend on the 'gitweb' target, it runs 'make -C gitweb\ninstall' unconditionally, which does generate all the necessary files\nfor 'gitweb' and installs them. However, if someone runs 'make &&\nsudo make install', then those files in the 'gitweb' directory will be\ngenerated and owned by root, which is not nice.\n\nList 'gitweb' as a direct dependency of the default target, so a plain\n'make' will build it.\n\nSigned-off-by: SZEDER Gábor \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 d251838c554..63057b10634 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3042,6 +3042,7 @@ include gitweb/Makefile\n \n .PHONY: gitweb\n gitweb: $(MAK_DIR_GITWEB_ALL)\n+all:: gitweb\n \n ### Installation rules\n \n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456418","messageId":"patch-v2-6.7-b423cd58f6b-20220531T173805Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v2 6/7] gitweb/Makefile: include in top-level Makefile","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-05-31T17:45:59Z","receivedAt":"2022-05-31T17:46:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Include the gitweb/Makefile in the top-level Makefile rather than\ncalling it as a sub-Makefile. As noted in the thread starting at at\n[1] (in particular [2]) we'll pay a high cost on NOOP runs of \"make\"\njust to figure out that we have nothing to do for \"make gitweb\".\n\nThe \"gitweb\" script also isn't maintained out-of-tree, unlike\n\"gitk-git\" or \"git-gui\", which both have their own \"Makefile\". Other\nparts of it are already integrated into our main Makefiles, e.g. the\ndocumentation is built by Documentation/Makefile since\n07ea4df2780 (gitweb: Add gitweb(1) manpage for gitweb itself,\n2011-10-16).\n\n1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com/\n2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nPerhaps a better solution is to simply move all of this code to the\ntop-level gitweb/Makefile, but on the other hand being able to include\nthe gitweb/Makefile to fold it into the top-level one without a larger\ndiff makes this sort of change much easier to review.\n\nAnd, as shown here we'll have \"make -C gitweb\" now $(error) out, so\nthere's no chance of confusion in practice.\n\n Makefile | 23 +++++++------\n gitweb/Makefile | 91 +++++++++----------------------------------------\n 2 files changed, 29 insertions(+), 85 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 18ca6744a50..d251838c554 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -538,6 +538,7 @@ gitexecdir = libexec/git-core\n mergetoolsdir = $(gitexecdir)/mergetools\n sharedir = $(prefix)/share\n gitwebdir = $(sharedir)/gitweb\n+gitwebstaticdir = $(gitwebdir)/static\n perllibdir = $(sharedir)/perl5\n localedir = $(sharedir)/locale\n template_dir = share/git-core/templates\n@@ -556,7 +557,7 @@ localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))\n htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))\n perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))\n \n-export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir\n+export prefix bindir sharedir sysconfdir perllibdir localedir\n \n # Set our default programs\n CC = cc\n@@ -2071,6 +2072,7 @@ htmldir_relative_SQ = $(subst ','\\'',$(htmldir_relative))\n prefix_SQ = $(subst ','\\'',$(prefix))\n perllibdir_relative_SQ = $(subst ','\\'',$(perllibdir_relative))\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))\n+gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebstaticdir))\n \n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n TEST_SHELL_PATH_SQ = $(subst ','\\'',$(TEST_SHELL_PATH))\n@@ -2399,10 +2401,6 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile\n perllibdir:\n \t@echo '$(perllibdir_SQ)'\n \n-.PHONY: gitweb\n-gitweb:\n-\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n-\n git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n@@ -3036,6 +3034,15 @@ coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.c\n \n .PHONY: coccicheck coccicheck-pending\n \n+# \"Sub\"-Makefiles, not really because they can't be run stand-alone,\n+# only there to contain directory-specific rules and variables\n+## gitweb/Makefile inclusion:\n+MAK_DIR_GITWEB = gitweb/\n+include gitweb/Makefile\n+\n+.PHONY: gitweb\n+gitweb: $(MAK_DIR_GITWEB_ALL)\n+\n ### Installation rules\n \n ifneq ($(filter /%,$(firstword $(template_dir))),)\n@@ -3108,7 +3115,6 @@ ifndef NO_PERL\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'\n \t(cd perl/build/lib && $(TAR) cf - .) | \\\n \t(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)\n-\t$(MAKE) -C gitweb install\n endif\n ifndef NO_TCLTK\n \t$(MAKE) -C gitk-git install\n@@ -3163,10 +3169,8 @@ endif\n \t\t cp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; } \\\n \tdone\n \n-.PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf\n+.PHONY: install-doc install-man install-man-perl install-html install-info install-pdf\n .PHONY: quick-install-doc quick-install-man quick-install-html\n-install-gitweb:\n-\t$(MAKE) -C gitweb install\n \n install-doc: install-man-perl\n \t$(MAKE) -C Documentation install\n@@ -3310,7 +3314,6 @@ clean: profile-clean coverage-clean cocciclean\n \t$(MAKE) -C Documentation/ clean\n \t$(RM) Documentation/GIT-EXCLUDED-PROGRAMS\n ifndef NO_PERL\n-\t$(MAKE) -C gitweb clean\n \t$(RM) -r perl/build/\n endif\n \t$(MAKE) -C templates/ clean\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex eaf0cfcf80e..66fceb9e94f 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -1,11 +1,8 @@\n-# The default target of this Makefile is...\n-all::\n-.PHONY: all\n-\n-MAK_DIR_GITWEB =\n+ifndef MAK_DIR_GITWEB\n+$(error do not run gitweb/Makefile stand-alone anymore. The \"gitweb\" and \\\n+\"install-gitweb\" targets now live in the top-level Makefile)\n+endif\n \n-# Define V=1 to have a more verbose compile.\n-#\n # Define JSMIN to point to JavaScript minifier that functions as\n # a filter to have static/gitweb.js minified.\n #\n@@ -13,13 +10,6 @@ MAK_DIR_GITWEB =\n # version of static/gitweb.css\n #\n \n-prefix ?= $(HOME)\n-bindir ?= $(prefix)/bin\n-gitwebdir ?= /var/www/cgi-bin\n-\n-RM ?= rm -f\n-INSTALL ?= install\n-\n # default configuration for gitweb\n GITWEB_CONFIG = gitweb_config.perl\n GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n@@ -44,71 +34,19 @@ GITWEB_SITE_HEADER =\n GITWEB_SITE_FOOTER =\n HIGHLIGHT_BIN = highlight\n \n-# include user config\n--include ../config.mak.autogen\n--include ../config.mak\n--include config.mak\n-\n-# determine version\n-.PHONY: .FORCE-GIT-VERSION-FILE\n-../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n-\t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n-\n-ifneq ($(MAKECMDGOALS),clean)\n--include ../GIT-VERSION-FILE\n-endif\n-\n # What targets we'll add to 'all' for \"make gitweb\"\n GITWEB_ALL =\n GITWEB_ALL += gitweb.cgi\n GITWEB_ALL += $(GITWEB_JS)\n \n-### Build rules\n-\n-SHELL_PATH ?= $(SHELL)\n-PERL_PATH ?= /usr/bin/perl\n-\n-# Shell quote;\n-bindir_SQ = $(subst ','\\'',$(bindir))#'\n-gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n-gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n-SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n-DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n-\n-# Quiet generation (unless V=1)\n-QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n-QUIET_SUBDIR1 =\n-\n-ifneq ($(findstring $(MAKEFLAGS),w),w)\n-PRINT_DIR = --no-print-directory\n-else # \"make -w\"\n-NO_SUBDIR = :\n-endif\n-\n-ifneq ($(findstring $(MAKEFLAGS),s),s)\n-ifndef V\n-\tQUIET = @\n-\tQUIET_GEN = $(QUIET)echo ' ' GEN $@;\n-\tQUIET_SUBDIR0 = +@subdir=\n-\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n-\t $(MAKE) $(PRINT_DIR) -C $$subdir\n-\texport V\n-\texport QUIET\n-\texport QUIET_GEN\n-\texport QUIET_SUBDIR0\n-\texport QUIET_SUBDIR1\n-endif\n-endif\n-\n-all:: $(GITWEB_ALL)\n+MAK_DIR_GITWEB_ALL = $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_ALL))\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n GITWEB_JS = $(GITWEB_JS_MIN)\n-all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n@@ -118,7 +56,7 @@ GITWEB_FILES += $(GITWEB_JS)\n GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n GITWEB_CSS = $(GITWEB_CSS_MIN)\n-all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n@@ -185,19 +123,22 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_\n \n ### Installation rules\n \n-.PHONY: install\n-install: all\n+.PHONY: install-gitweb\n+install-gitweb: $(MAK_DIR_GITWEB_ALL)\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n-\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \\\n-\t\t'$(DESTDIR_SQ)$(gitwebdir_SQ)'\n+\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \\\n \t\t'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n+ifndef NO_PERL\n+install: install-gitweb\n+endif\n \n ### Cleaning rules\n \n-.PHONY: clean\n-clean:\n+.PHONY: gitweb-clean\n+gitweb-clean:\n \t$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \\\n \t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n \t\tGITWEB-BUILD-OPTIONS)\n+clean: gitweb-clean\n-- \n2.36.1.1103.g036c05811b0\n\n"},{"id":"456733","messageId":"xmqqa6ap8z55.fsf@gitster.g","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-06T17:44:54Z","receivedAt":"2022-06-06T17:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The $subject is a proposed re-roll of SZEDER's\n> https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;\n> As noted downthread of that fix having the Makefile invoke \"make -C\n> gitweb\" again would slow us down on NOOP runs by quite a bit.\n\nIt would be nice to hear comments SZEDER and others, even if the\ncomments are clear negative or positive.\n\nThanks.\n"},{"id":"457542","messageId":"20220620083202.GB1689@szeder.dev","threadId":"57923","inReplyTo":"xmqqa6ap8z55.fsf@gitster.g","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2022-06-20T08:32:02Z","receivedAt":"2022-06-20T08:32:09Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > The $subject is a proposed re-roll of SZEDER's\n> > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;\n> > As noted downthread of that fix having the Makefile invoke \"make -C\n> > gitweb\" again would slow us down on NOOP runs by quite a bit.\n> \n> It would be nice to hear comments SZEDER and others, even if the\n> comments are clear negative or positive.\n\nWell, my itch is scratched, so I'm fine with it :)\n\nI think Peff has a point by questioning whether we should build and\ninstall gitweb by default... I don't have an opinion about that, but\nif we do want to build it by default, then IMO doing it in the main\nMakefile is the way to go, so I think in that case this patch series\ngoes in the right direction.\n\n\nOn a related note: this is not the first time when something was not\ngenerated during a regular 'make' but only during 'sudo make install',\nsee e.g. 2530afd351 (Makefile: generate Git(3pm) as dependency of the\n'doc' and 'man' targets, 2018-02-15). Perhaps it would be worth to\nextend the CI jobs with a 'make install/install-doc' command, and\nerror out if it generated any files that were not present before, so\nwe could catch similar issues early on.\n\n"},{"id":"457590","messageId":"YrFphmtLuHVkI7yr@coredump.intra.peff.net","threadId":"57923","inReplyTo":"20220620083202.GB1689@szeder.dev","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-06-21T06:47:34Z","receivedAt":"2022-06-21T06:47:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 20, 2022 at 10:32:02AM +0200, SZEDER Gábor wrote:\n\n> On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:\n> > Ævar Arnfjörð Bjarmason writes:\n> > \n> > > The $subject is a proposed re-roll of SZEDER's\n> > > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;\n> > > As noted downthread of that fix having the Makefile invoke \"make -C\n> > > gitweb\" again would slow us down on NOOP runs by quite a bit.\n> > \n> > It would be nice to hear comments SZEDER and others, even if the\n> > comments are clear negative or positive.\n> \n> Well, my itch is scratched, so I'm fine with it :)\n> \n> I think Peff has a point by questioning whether we should build and\n> install gitweb by default... I don't have an opinion about that, but\n> if we do want to build it by default, then IMO doing it in the main\n> Makefile is the way to go, so I think in that case this patch series\n> goes in the right direction.\n\nI hadn't realized the full situation when I was arguing earlier that \"we\nhave not been building it for several years\". You raised the point that\nwe do auto-build it in \"make install\", so it would be a change of\nbehavior to stop doing so.\n\nI still find it hard to care too much about backwards compatibility for\nbuilding gitweb (or really gitweb at all, for that matter). But my main\ncomplaint was foisting another recursive Makefile and its performance\nand troubles on developers at large, and I think Ævar's patches deal\nwith it. So I'm OK with the direction.\n\nI admit I didn't look _too_ closely at them, but they overall seemed\nsensible to me. Two things I noted:\n\n - I wondered if \"make NO_PERL=1\" would complain about \"gitweb\" being\n in the default targets. It doesn't, but it does actually build\n gitweb, which seems a little weird. I don't think we actually rely\n on perl during the build (e.g., no \"perl -c\" checks or anything),\n and the t950x tests seem to respect NO_PERL and avoid running the\n generated file. So maybe it's OK?\n\n - Speaking of backwards compatibility: after this series, \"cd gitweb\n && make\" yields an error. It's got a nice message telling you what\n to do, but it's likely breaking distro scripts. Again, I'm not sure\n I care, but if the point of the exercise was to avoid breaking\n things, well...\n\n-Peff\n"},{"id":"457689","messageId":"220622.86r13hkp2c.gmgdl@evledraar.gmail.com","threadId":"57923","inReplyTo":"YrFphmtLuHVkI7yr@coredump.intra.peff.net","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-22T09:27:54Z","receivedAt":"2022-06-22T09:47:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jun 21 2022, Jeff King wrote:\n\n> On Mon, Jun 20, 2022 at 10:32:02AM +0200, SZEDER Gábor wrote:\n>\n>> On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:\n>> > Ævar Arnfjörð Bjarmason writes:\n>> > \n>> > > The $subject is a proposed re-roll of SZEDER's\n>> > > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;\n>> > > As noted downthread of that fix having the Makefile invoke \"make -C\n>> > > gitweb\" again would slow us down on NOOP runs by quite a bit.\n>> > \n>> > It would be nice to hear comments SZEDER and others, even if the\n>> > comments are clear negative or positive.\n>> \n>> Well, my itch is scratched, so I'm fine with it :)\n>> \n>> I think Peff has a point by questioning whether we should build and\n>> install gitweb by default... I don't have an opinion about that, but\n>> if we do want to build it by default, then IMO doing it in the main\n>> Makefile is the way to go, so I think in that case this patch series\n>> goes in the right direction.\n>\n> I hadn't realized the full situation when I was arguing earlier that \"we\n> have not been building it for several years\". You raised the point that\n> we do auto-build it in \"make install\", so it would be a change of\n> behavior to stop doing so.\n>\n> I still find it hard to care too much about backwards compatibility for\n> building gitweb (or really gitweb at all, for that matter). But my main\n> complaint was foisting another recursive Makefile and its performance\n> and troubles on developers at large, and I think Ævar's patches deal\n> with it. So I'm OK with the direction.\n>\n> I admit I didn't look _too_ closely at them, but they overall seemed\n> sensible to me. Two things I noted:\n>\n> - I wondered if \"make NO_PERL=1\" would complain about \"gitweb\" being\n> in the default targets. It doesn't, but it does actually build\n> gitweb, which seems a little weird. I don't think we actually rely\n> on perl during the build (e.g., no \"perl -c\" checks or anything),\n> and the t950x tests seem to respect NO_PERL and avoid running the\n> generated file. So maybe it's OK?\n\nI think it's arguably a bug, but as you note we build/test etc. without\nerrors, and I think it's restoring the state before e25c7cc146\n(Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).\n\nArguably we should replace with a stub script like git-svn et al, and\narguably we should leave it, as you're more likely to e.g. run gitweb on\na webserver, so even if you build a \"no perl\" package, perhaps it's\nconvenient to have \"gitweb\" part of it, and then on that one box that\nruns it you'll install perl...\n\n> - Speaking of backwards compatibility: after this series, \"cd gitweb\n> && make\" yields an error. It's got a nice message telling you what\n> to do, but it's likely breaking distro scripts. Again, I'm not sure\n> I care, but if the point of the exercise was to avoid breaking\n> things, well...\n\nI think that's OK, having maintained those sorts of build scripts in a\npast life.\n\nI.e. when you upgrade the package it's a minor hassle, and the error\ntells you exactly what to do, and the fix is a 2-3 lines in your recipe\nat most.\n\nI could make gitweb/Makefile \"fake it\", but as argued in the patches I\nthink this trade-off makes more sense. Having it run in some \"dual mode\"\nwould be a maintenance hassle.\n\nMost of the reason for keeping gitweb/Makefile around (as opposed to the\ntop-level Makefile absorbing it) was to be able to emit that message to\nbe friendly to downstream packagers.\n"},{"id":"457714","messageId":"xmqq7d587lqx.fsf@gitster.g","threadId":"57923","inReplyTo":"220622.86r13hkp2c.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-22T15:37:10Z","receivedAt":"2022-06-22T15:37:19Z","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 wondered if \"make NO_PERL=1\" would complain about \"gitweb\" being\n>> in the default targets. It doesn't, but it does actually build\n>> gitweb, which seems a little weird. I don't think we actually rely\n>> on perl during the build (e.g., no \"perl -c\" checks or anything),\n>> and the t950x tests seem to respect NO_PERL and avoid running the\n>> generated file. So maybe it's OK?\n>\n> I think it's arguably a bug, but as you note we build/test etc. without\n> errors, and I think it's restoring the state before e25c7cc146\n> (Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).\n>\n> Arguably we should replace with a stub script like git-svn et al, and\n> arguably we should leave it, as you're more likely to e.g. run gitweb on\n> a webserver, so even if you build a \"no perl\" package, perhaps it's\n> convenient to have \"gitweb\" part of it, and then on that one box that\n> runs it you'll install perl...\n\nIt is perfectly acceptable to \"make DESTDIR=... install\" and tar up\nthe result on a host with NO_PERL and extract it on the target that\nis capable to run gitweb, isn't it? As long as \"make NO_PERL=1\"\ngives exactly the gitweb as a build without NO_PERL, that should be\nOK, I would think. I would think what you have is in a good state.\n\n>> - Speaking of backwards compatibility: after this series, \"cd gitweb\n>> && make\" yields an error. It's got a nice message telling you what\n>> to do, but it's likely breaking distro scripts. Again, I'm not sure\n>> I care, but if the point of the exercise was to avoid breaking\n>> things, well...\n>\n> I think that's OK, having maintained those sorts of build scripts in a\n> past life.\n>\n> I.e. when you upgrade the package it's a minor hassle, and the error\n> tells you exactly what to do, and the fix is a 2-3 lines in your recipe\n> at most.\n\nWe could easily add \"cd .. && make gitweb\" to gitweb/Makefile with\nthe same \"minor hassle\" but that needs to be done just once, instead\nof having to be done once per packager, so I am not sure the above\nargues for a good tradeoff.\n\nThanks.\n"},{"id":"457769","messageId":"220623.865ykrll0j.gmgdl@evledraar.gmail.com","threadId":"57923","inReplyTo":"xmqq7d587lqx.fsf@gitster.g","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T10:29:58Z","receivedAt":"2022-06-23T10:41:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jun 22 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> - I wondered if \"make NO_PERL=1\" would complain about \"gitweb\" being\n>>> in the default targets. It doesn't, but it does actually build\n>>> gitweb, which seems a little weird. I don't think we actually rely\n>>> on perl during the build (e.g., no \"perl -c\" checks or anything),\n>>> and the t950x tests seem to respect NO_PERL and avoid running the\n>>> generated file. So maybe it's OK?\n>>\n>> I think it's arguably a bug, but as you note we build/test etc. without\n>> errors, and I think it's restoring the state before e25c7cc146\n>> (Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).\n>>\n>> Arguably we should replace with a stub script like git-svn et al, and\n>> arguably we should leave it, as you're more likely to e.g. run gitweb on\n>> a webserver, so even if you build a \"no perl\" package, perhaps it's\n>> convenient to have \"gitweb\" part of it, and then on that one box that\n>> runs it you'll install perl...\n>\n> It is perfectly acceptable to \"make DESTDIR=... install\" and tar up\n> the result on a host with NO_PERL and extract it on the target that\n> is capable to run gitweb, isn't it? As long as \"make NO_PERL=1\"\n> gives exactly the gitweb as a build without NO_PERL, that should be\n> OK, I would think. I would think what you have is in a good state.\n\nI think so, but it is inconsistent with how we treat the rest of the\n*.perl scripts, which I think is what Jeff is correctly pointing\nout.\n\nI.e. if you do this your git-send-email, git-cvsserver, git-svn\netc. will be replaced by this shellscript:\n\n\t#!/bin/sh\n\n\techo >&2 \"fatal: git was built without support for $(basename $0) (NO_PERL=Y).\"\n\texit 128\n\n>>> - Speaking of backwards compatibility: after this series, \"cd gitweb\n>>> && make\" yields an error. It's got a nice message telling you what\n>>> to do, but it's likely breaking distro scripts. Again, I'm not sure\n>>> I care, but if the point of the exercise was to avoid breaking\n>>> things, well...\n>>\n>> I think that's OK, having maintained those sorts of build scripts in a\n>> past life.\n>>\n>> I.e. when you upgrade the package it's a minor hassle, and the error\n>> tells you exactly what to do, and the fix is a 2-3 lines in your recipe\n>> at most.\n>\n> We could easily add \"cd .. && make gitweb\" to gitweb/Makefile with\n> the same \"minor hassle\" but that needs to be done just once, instead\n> of having to be done once per packager, so I am not sure the above\n> argues for a good tradeoff.\n\nTrue, but I think critically in this case we've never documented that\nyou should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL\nhas always documented and assumed that you run these from the top-level.\n\nThere's no \"make gitweb\" target in the sub-Makefile, and the\ninstructions make it clear that it's the top-level, as we also suggest:\n\n make configure &&\n [... no cd-ing to gitweb/ ...] &&\n make gitweb\n\n(and there's no \"configure\" there either).\n\nSo I think this will Just Work for anyone who's packaging this already,\n\nI was just going above & beyond in being friendly by emitting that new\nmessage saying you should be running \"make\" at the top-level in case\nanyone built this in a way that we never documented, but which happened\nto work before.\n"},{"id":"457817","messageId":"xmqq7d57x8qo.fsf@gitster.g","threadId":"57923","inReplyTo":"220623.865ykrll0j.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-23T23:25:51Z","receivedAt":"2022-06-23T23:25:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> We could easily add \"cd .. && make gitweb\" to gitweb/Makefile with\n>> the same \"minor hassle\" but that needs to be done just once, instead\n>> of having to be done once per packager, so I am not sure the above\n>> argues for a good tradeoff.\n>\n> True, but I think critically in this case we've never documented that\n> you should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL\n> has always documented and assumed that you run these from the top-level.\n\nWell, I do not think Makefiles document much of their targets in\ngeneral. If its first/default target has a reasonable name, like\n\"all\", people expect \"cd there && make all\" would do the right\nthing.\n\nSo I do not think \"we never documented\" is a good excuse. What the\ncurrent users have been doing and are expecting to keep working is\nwhat counts. If they are used to see \"cd gitweb && make\" working,\nperhaps instead of giving an unfriendly $(error do not run) at the\nbeginning of gitweb/Makefile that is designed to trigger only when\nthey did that (instead of running 'make gitweb' from the top), it\nwould be trivial to have the rule to \"cd .. && $(MAKE) gitweb\"\nthere, no?\n\n\n\n"},{"id":"457820","messageId":"220624.86bkuikidi.gmgdl@evledraar.gmail.com","threadId":"57923","inReplyTo":"xmqq7d57x8qo.fsf@gitster.g","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-23T23:45:35Z","receivedAt":"2022-06-24T00:36:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 23 2022, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> We could easily add \"cd .. && make gitweb\" to gitweb/Makefile with\n>>> the same \"minor hassle\" but that needs to be done just once, instead\n>>> of having to be done once per packager, so I am not sure the above\n>>> argues for a good tradeoff.\n>>\n>> True, but I think critically in this case we've never documented that\n>> you should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL\n>> has always documented and assumed that you run these from the top-level.\n>\n> Well, I do not think Makefiles document much of their targets in\n> general. If its first/default target has a reasonable name, like\n> \"all\", people expect \"cd there && make all\" would do the right\n> thing.\n>\n> So I do not think \"we never documented\" is a good excuse. What the\n> current users have been doing and are expecting to keep working is\n> what counts. If they are used to see \"cd gitweb && make\" working,\n> perhaps instead of giving an unfriendly $(error do not run) at the\n> beginning of gitweb/Makefile that is designed to trigger only when\n> they did that (instead of running 'make gitweb' from the top), it\n> would be trivial to have the rule to \"cd .. && $(MAKE) gitweb\"\n> there, no?\n\nI can re-roll it with that change if you insist. It would close the door\non further unifying the two Makefiles in the future (well, we could keep\nthe wrapper in place).\n\nI have a script I use to see how big the impact of this sort of thing\nwould be in practice, i.e. I download downstream package recipies, which\nare found at (name, relative path & urls). I also manually get the AIX\npackage:\n\t\n\tfreebsd-ports devel/git https://github.com/freebsd/freebsd-ports.git \n\topenbsd-ports devel/git https://github.com/openbsd/ports.git \n\tnetbsd-pkgsrc devel/git-base https://github.com/NetBSD/pkgsrc.git \n\tdragonflybsd-dports devel/git https://github.com/DragonFlyBSD/DPorts.git \n\tfedora . https://src.fedoraproject.org/rpms/git \n\tdebian debian https://repo.or.cz/git/debian.git \n\tgentoo dev-vcs/git https://github.com/gentoo/gentoo.git \n\tarch git/trunk https://github.com/archlinux/svntogit-packages.git \n\tnix pkgs/applications/version-management/git-and-tools/git https://github.com/NixOS/nixpkgs.git \n\talpine main/git https://github.com/alpinelinux/aports.git https://git.alpinelinux.org/aports \n\tgit_osx_installer . https://github.com/timcharper/git_osx_installer.git \n\thomebrew-core Formula/git.rb https://github.com/Homebrew/homebrew-core.git \n\tmacports-ports devel/git https://github.com/macports/macports-ports.git\n\nLooking through all of those none of them do anything with\ngitweb/Makefile. I.e. all \"make gitweb\" at the top-level, or simply rely\non \"make install\" to install it.\n\nFreeBSD and NetBSD are monkeypatching our Makefile to emulate a \"I don't\nwant gitweb please!\", which they'll need to do before/after this series\n(but we could helpfully provide them a config knob).\n\nAnyway, if you want \"make gitweb\" and \"make gitweb-install\" in the\nsubdirectory to work I can patch it, but per the above I think it would\nbe useful to pretty much nobody.\n\nI could use around the same amount of effort to give FreeBSD and NetBSD\na \"don't install gitweb please\" know instead, what do you think?:)\n"},{"id":"457822","messageId":"xmqq35fuyiaj.fsf@gitster.g","threadId":"57923","inReplyTo":"220624.86bkuikidi.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 0/7] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-06-24T01:14:12Z","receivedAt":"2022-06-24T01:14:18Z","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 be in practice, i.e. I download downstream package recipies, which\n> are found at (name, relative path & urls). I also manually get the AIX\n> package:\n> ...\n> \tmacports-ports devel/git https://github.com/macports/macports-ports.git\n>\n> Looking through all of those none of them do anything with\n> gitweb/Makefile. I.e. all \"make gitweb\" at the top-level, or simply rely\n> on \"make install\" to install it.\n\nNow we are talking. Giving that as a datapoint upfront in the\nproposed log message would have saved needless round-trip, I would\nthink.\n\nThanks.\n"},{"id":"458020","messageId":"patch-v3-1.8-8e85151cf3d-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:55Z","receivedAt":"2022-06-28T10:16:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the '.PHONY' definition so that it's split up and accompanies the\nrelevant as they're defined. This will make a subsequent diff smaller\nas we'll remove some of these, and won't need to re-edit the\nnow-removed '.PHONY' line.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex f13e23c4de4..abb5c9f9ab6 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -1,5 +1,6 @@\n # The default target of this Makefile is...\n all::\n+.PHONY: all\n \n # Define V=1 to have a more verbose compile.\n #\n@@ -45,6 +46,7 @@ HIGHLIGHT_BIN = highlight\n -include config.mak\n \n # determine version\n+.PHONY: .FORCE-GIT-VERSION-FILE\n ../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n \t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n \n@@ -152,6 +154,7 @@ GITWEB_REPLACE = \\\n \t-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \\\n \t-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'\n \n+.PHONY: FORCE\n GITWEB-BUILD-OPTIONS: FORCE\n \t@rm -f $@+\n \t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n@@ -171,15 +174,18 @@ static/gitweb.js: $(GITWEB_JSLIB_FILES)\n \n ### Testing rules\n \n+.PHONY: test\n test:\n \t$(MAKE) -C ../t gitweb-test\n \n+.PHONY: test-installed\n test-installed:\n \tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n \t\t$(MAKE) -C ../t gitweb-test\n \n ### Installation rules\n \n+.PHONY: install\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n@@ -188,10 +194,8 @@ install: all\n \n ### Cleaning rules\n \n+.PHONY: clean\n clean:\n \t$(RM) gitweb.cgi static/gitweb.js \\\n \t\tstatic/gitweb.min.js static/gitweb.min.css \\\n \t\tGITWEB-BUILD-OPTIONS\n-\n-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE\n-\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458021","messageId":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com","subject":"[PATCH v3 0/8] gitweb: fix \"make\" not including \"gitweb\" without NOOP run slowdowns","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:54Z","receivedAt":"2022-06-28T10:16:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The $subject is a mostly trivial re-roll of\nhttps://lore.kernel.org/git/cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com/.\n\nChanges since v3:\n\n * Correct the commit message of 7/8 to account for SZEDER's commit\n being included in this (that part was also incorrect in v2).\n\n * As noted in\n https://lore.kernel.org/git/220624.86bkuikidi.gmgdl@evledraar.gmail.com/\n I went over various git packages in the wild to see if my changes\n here would break things\n\n I did find that not having a wrapper gitweb/Makefile would break\n OpenBSD's package, but it's a one-line fix.\n\n But to make up for it I saw that a semi-common pattern was to\n manually munge our Makefile to get rid of \"gitweb\" or\n \"gitweb-install\" targets. All of {Free,Net,Dragonfly}BSD were doing\n that. They'll now happily be able to use a NO_GITWEB=Y flag\n instead, so hopefully the small amount of disruption here makes up\n for itself.\n\nSZEDER Gábor (1):\n Makefile: build 'gitweb' in the default target\n\nÆvar Arnfjörð Bjarmason (7):\n gitweb/Makefile: define all .PHONY prerequisites inline\n gitweb/Makefile: add a $(GITWEB_ALL) variable\n gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars\n gitweb/Makefile: prepare to merge into top-level Makefile\n gitweb: remove \"test\" and \"test-installed\" targets\n gitweb/Makefile: include in top-level Makefile\n gitweb/Makefile: add a \"NO_GITWEB\" parameter\n\n Makefile | 31 +++++++----\n gitweb/Makefile | 145 ++++++++++++++++--------------------------------\n t/Makefile | 4 --\n 3 files changed, 68 insertions(+), 112 deletions(-)\n\nRange-diff against v2:\n1: 14361617ca6 = 1: 8e85151cf3d gitweb/Makefile: define all .PHONY prerequisites inline\n2: 7d920a13518 = 2: 5c9895949aa gitweb/Makefile: add a $(GITWEB_ALL) variable\n3: e14a5b73061 = 3: 2f4db54923d gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars\n4: 02e26ca8ce2 = 4: d38b553a2e6 gitweb/Makefile: prepare to merge into top-level Makefile\n5: caf376f3dd9 = 5: 6c2d7b30524 gitweb: remove \"test\" and \"test-installed\" targets\n6: b423cd58f6b = 6: 5640587b9ae gitweb/Makefile: include in top-level Makefile\n7: 69428540886 ! 7: 571c9c10319 Makefile: build 'gitweb' in the default target\n @@ Commit message\n Since then a simple 'make' doesn't build 'gitweb'.\n \n Luckily, installing 'gitweb' is not broken: although 'make install'\n - doesn't depend on the 'gitweb' target, it runs 'make -C gitweb\n - install' unconditionally, which does generate all the necessary files\n + doesn't depend on the 'gitweb' target, it has a dependency on the\n + 'install-gitweb' target, which does generate all the necessary files\n for 'gitweb' and installs them. However, if someone runs 'make &&\n sudo make install', then those files in the 'gitweb' directory will be\n generated and owned by root, which is not nice.\n-: ----------- > 8: 0c8f26ee876 gitweb/Makefile: add a \"NO_GITWEB\" parameter\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458022","messageId":"patch-v3-2.8-5c9895949aa-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 2/8] gitweb/Makefile: add a $(GITWEB_ALL) variable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:56Z","receivedAt":"2022-06-28T10:16:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Declare the targets that the \"all\" target depends on with a new\n$(GITWEB_ALL) variable. This will help to reduce churn in subsequent\ncommits.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex abb5c9f9ab6..733b60f925e 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -54,6 +54,11 @@ ifneq ($(MAKECMDGOALS),clean)\n -include ../GIT-VERSION-FILE\n endif\n \n+# What targets we'll add to 'all' for \"make gitweb\"\n+GITWEB_ALL =\n+GITWEB_ALL += gitweb.cgi\n+GITWEB_ALL += static/gitweb.js\n+\n ### Build rules\n \n SHELL_PATH ?= $(SHELL)\n@@ -92,7 +97,7 @@ ifndef V\n endif\n endif\n \n-all:: gitweb.cgi static/gitweb.js\n+all:: $(GITWEB_ALL)\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458023","messageId":"patch-v3-3.8-2f4db54923d-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 3/8] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:57Z","receivedAt":"2022-06-28T10:16:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the variable definitions for the $(GITWEB_CSS) and $(GITWEB_JS)\nso that we have a clear separation between what we use as \"in\" files,\nv.s. our \"min\" files. We can now make the appending to $(GITWEB_FILES)\nunconditional, since $(GITWEB_{JS,CSS}) is either the \"min\" or\nnon-\"min\" version. This reduces the duplication within the file.\n\nWhile we're at it let's initialize \"GITWEB_JSLIB_FILES\" as we normally\ndo with such variables.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 35 ++++++++++++++++++-----------------\n 1 file changed, 18 insertions(+), 17 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 733b60f925e..a8752c1f11c 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -31,10 +31,12 @@ GITWEB_STRICT_EXPORT =\n GITWEB_BASE_URL =\n GITWEB_LIST =\n GITWEB_HOMETEXT = indextext.html\n-GITWEB_CSS = static/gitweb.css\n+GITWEB_CSS_IN = static/gitweb.css\n+GITWEB_CSS = $(GITWEB_CSS_IN)\n GITWEB_LOGO = static/git-logo.png\n GITWEB_FAVICON = static/git-favicon.png\n-GITWEB_JS = static/gitweb.js\n+GITWEB_JS_IN = static/gitweb.js\n+GITWEB_JS = $(GITWEB_JS_IN)\n GITWEB_SITE_HTML_HEAD_STRING =\n GITWEB_SITE_HEADER =\n GITWEB_SITE_FOOTER =\n@@ -57,7 +59,7 @@ endif\n # What targets we'll add to 'all' for \"make gitweb\"\n GITWEB_ALL =\n GITWEB_ALL += gitweb.cgi\n-GITWEB_ALL += static/gitweb.js\n+GITWEB_ALL += $(GITWEB_JS)\n \n ### Build rules\n \n@@ -101,25 +103,23 @@ all:: $(GITWEB_ALL)\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n+GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n-GITWEB_FILES += static/gitweb.min.js\n-GITWEB_JS = static/gitweb.min.js\n-all:: static/gitweb.min.js\n-static/gitweb.min.js: static/gitweb.js GITWEB-BUILD-OPTIONS\n+GITWEB_JS = $(GITWEB_JS_MIN)\n+all:: $(GITWEB_JS_MIN)\n+$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n-else\n-GITWEB_FILES += static/gitweb.js\n endif\n+GITWEB_FILES += $(GITWEB_JS)\n \n+GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n-GITWEB_FILES += static/gitweb.min.css\n-GITWEB_CSS = static/gitweb.min.css\n-all:: static/gitweb.min.css\n-static/gitweb.min.css: static/gitweb.css GITWEB-BUILD-OPTIONS\n+GITWEB_CSS = $(GITWEB_CSS_MIN)\n+all:: $(GITWEB_CSS_MIN)\n+$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n-else\n-GITWEB_FILES += static/gitweb.css\n endif\n+GITWEB_FILES += $(GITWEB_CSS)\n \n GITWEB_FILES += static/git-logo.png static/git-favicon.png\n \n@@ -127,6 +127,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png\n #\n # js/lib/common-lib.js should be always first, then js/lib/*.js,\n # then the rest of files; js/gitweb.js should be last (if it exists)\n+GITWEB_JSLIB_FILES =\n GITWEB_JSLIB_FILES += static/js/lib/common-lib.js\n GITWEB_JSLIB_FILES += static/js/lib/datetime.js\n GITWEB_JSLIB_FILES += static/js/lib/cookies.js\n@@ -201,6 +202,6 @@ install: all\n \n .PHONY: clean\n clean:\n-\t$(RM) gitweb.cgi static/gitweb.js \\\n-\t\tstatic/gitweb.min.js static/gitweb.min.css \\\n+\t$(RM) gitweb.cgi $(GITWEB_JS_IN) \\\n+\t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n \t\tGITWEB-BUILD-OPTIONS\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458024","messageId":"patch-v3-4.8-d38b553a2e6-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 4/8] gitweb/Makefile: prepare to merge into top-level Makefile","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:58Z","receivedAt":"2022-06-28T10:16:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since the \"gitweb/Makefile\" was split out from the top-level Makefile\nin 62331ef1637 (gitweb: Makefile improvements, 2010-01-30) we've kept\nthe inter-dependencies between the two, and worse have dealt with a\nlot of duplication as a result.\n\nIn preparation for merging the two again add a MAK_DIR_GITWEB variable\nto various rules in it. This will allow us to set this variable to\n\"gitweb/\" as we include it in the top-level Makefile, which will\nminimize the size of the subsequent diff.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 29 ++++++++++++++++++-----------\n 1 file changed, 18 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex a8752c1f11c..74e896767e9 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -2,6 +2,8 @@\n all::\n .PHONY: all\n \n+MAK_DIR_GITWEB =\n+\n # Define V=1 to have a more verbose compile.\n #\n # Define JSMIN to point to JavaScript minifier that functions as\n@@ -106,8 +108,9 @@ GITWEB_PROGRAMS = gitweb.cgi\n GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n GITWEB_JS = $(GITWEB_JS_MIN)\n-all:: $(GITWEB_JS_MIN)\n-$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS\n+all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n endif\n GITWEB_FILES += $(GITWEB_JS)\n@@ -115,8 +118,9 @@ GITWEB_FILES += $(GITWEB_JS)\n GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n GITWEB_CSS = $(GITWEB_CSS_MIN)\n-all:: $(GITWEB_CSS_MIN)\n-$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS\n+all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n endif\n GITWEB_FILES += $(GITWEB_CSS)\n@@ -161,19 +165,20 @@ GITWEB_REPLACE = \\\n \t-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'\n \n .PHONY: FORCE\n-GITWEB-BUILD-OPTIONS: FORCE\n+$(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS: FORCE\n \t@rm -f $@+\n \t@echo \"x\" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) \"$(JSMIN)|$(CSSMIN)\" >$@+\n \t@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@\n \n-gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\n \t\t$(GITWEB_REPLACE) $< >$@+ && \\\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-static/gitweb.js: $(GITWEB_JSLIB_FILES)\n+$(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))\n \t$(QUIET_GEN)$(RM) $@ $@+ && \\\n \tcat $^ >$@+ && \\\n \tmv $@+ $@\n@@ -194,14 +199,16 @@ test-installed:\n .PHONY: install\n install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n-\t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n+\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \\\n+\t\t'$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n-\t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n+\t$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \\\n+\t\t'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \n ### Cleaning rules\n \n .PHONY: clean\n clean:\n-\t$(RM) gitweb.cgi $(GITWEB_JS_IN) \\\n+\t$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \\\n \t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n-\t\tGITWEB-BUILD-OPTIONS\n+\t\tGITWEB-BUILD-OPTIONS)\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458025","messageId":"patch-v3-5.8-6c2d7b30524-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 5/8] gitweb: remove \"test\" and \"test-installed\" targets","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:15:59Z","receivedAt":"2022-06-28T10:16:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the special \"test\" targets for gitweb added in\n958a8467216 (gitweb/Makefile: Add 'test' and 'test-installed' targets,\n2010-09-26). Unlike e.g. \"contrib/scalar\" and \"contrib/subtree\" the\n\"gitweb\" tests themselves live in our top-level t/ directory.\n\nIt therefore doesn't make sense to maintain this indirection, no more\nthan it would to have a \"git-send-email-test\". By dropping it we'll\nalso free other tests to use the t95*.sh prefix.\n\nThese removed targets are unlikely to be used by anyone, and to the\nextent that they are we can easily use an invocation like this\ninstead:\n\n\tmake test T='t[0-9]*gitweb*.sh'\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n gitweb/Makefile | 11 -----------\n t/Makefile | 4 ----\n 2 files changed, 15 deletions(-)\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 74e896767e9..eaf0cfcf80e 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -183,17 +183,6 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_\n \tcat $^ >$@+ && \\\n \tmv $@+ $@\n \n-### Testing rules\n-\n-.PHONY: test\n-test:\n-\t$(MAKE) -C ../t gitweb-test\n-\n-.PHONY: test-installed\n-test-installed:\n-\tGITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \\\n-\t\t$(MAKE) -C ../t gitweb-test\n-\n ### Installation rules\n \n .PHONY: install\ndiff --git a/t/Makefile b/t/Makefile\nindex 056ce55dcc9..7f56e52f767 100644\n--- a/t/Makefile\n+++ b/t/Makefile\n@@ -35,7 +35,6 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\\'',$(TEST_RESULTS_DIRECTORY))\n CHAINLINTTMP_SQ = $(subst ','\\'',$(CHAINLINTTMP))\n \n T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n-TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))\n TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n@@ -112,9 +111,6 @@ aggregate-results:\n \t\techo \"$$f\"; \\\n \tdone | '$(SHELL_PATH_SQ)' ./aggregate-results.sh\n \n-gitweb-test:\n-\t$(MAKE) $(TGITWEB)\n-\n valgrind:\n \t$(MAKE) GIT_TEST_OPTS=\"$(GIT_TEST_OPTS) --valgrind\"\n \n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458026","messageId":"patch-v3-6.8-5640587b9ae-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 6/8] gitweb/Makefile: include in top-level Makefile","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:16:00Z","receivedAt":"2022-06-28T10:16:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Include the gitweb/Makefile in the top-level Makefile rather than\ncalling it as a sub-Makefile. As noted in the thread starting at at\n[1] (in particular [2]) we'll pay a high cost on NOOP runs of \"make\"\njust to figure out that we have nothing to do for \"make gitweb\".\n\nThe \"gitweb\" script also isn't maintained out-of-tree, unlike\n\"gitk-git\" or \"git-gui\", which both have their own \"Makefile\". Other\nparts of it are already integrated into our main Makefiles, e.g. the\ndocumentation is built by Documentation/Makefile since\n07ea4df2780 (gitweb: Add gitweb(1) manpage for gitweb itself,\n2011-10-16).\n\n1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com/\n2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 23 +++++++------\n gitweb/Makefile | 91 +++++++++----------------------------------------\n 2 files changed, 29 insertions(+), 85 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 04d0fd1fe60..a5f29c11681 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -544,6 +544,7 @@ gitexecdir = libexec/git-core\n mergetoolsdir = $(gitexecdir)/mergetools\n sharedir = $(prefix)/share\n gitwebdir = $(sharedir)/gitweb\n+gitwebstaticdir = $(gitwebdir)/static\n perllibdir = $(sharedir)/perl5\n localedir = $(sharedir)/locale\n template_dir = share/git-core/templates\n@@ -562,7 +563,7 @@ localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))\n htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))\n perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))\n \n-export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir\n+export prefix bindir sharedir sysconfdir perllibdir localedir\n \n # Set our default programs\n CC = cc\n@@ -2089,6 +2090,7 @@ htmldir_relative_SQ = $(subst ','\\'',$(htmldir_relative))\n prefix_SQ = $(subst ','\\'',$(prefix))\n perllibdir_relative_SQ = $(subst ','\\'',$(perllibdir_relative))\n gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))\n+gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebstaticdir))\n \n SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))\n TEST_SHELL_PATH_SQ = $(subst ','\\'',$(TEST_SHELL_PATH))\n@@ -2417,10 +2419,6 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile\n perllibdir:\n \t@echo '$(perllibdir_SQ)'\n \n-.PHONY: gitweb\n-gitweb:\n-\t$(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all\n-\n git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tchmod +x $@+ && \\\n@@ -3149,6 +3147,15 @@ coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.c\n \n .PHONY: coccicheck coccicheck-pending\n \n+# \"Sub\"-Makefiles, not really because they can't be run stand-alone,\n+# only there to contain directory-specific rules and variables\n+## gitweb/Makefile inclusion:\n+MAK_DIR_GITWEB = gitweb/\n+include gitweb/Makefile\n+\n+.PHONY: gitweb\n+gitweb: $(MAK_DIR_GITWEB_ALL)\n+\n ### Installation rules\n \n ifneq ($(filter /%,$(firstword $(template_dir))),)\n@@ -3221,7 +3228,6 @@ ifndef NO_PERL\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'\n \t(cd perl/build/lib && $(TAR) cf - .) | \\\n \t(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)\n-\t$(MAKE) -C gitweb install\n endif\n ifndef NO_TCLTK\n \t$(MAKE) -C gitk-git install\n@@ -3276,10 +3282,8 @@ endif\n \t\t cp \"$$execdir/git-remote-http$X\" \"$$execdir/$$p\" || exit; } \\\n \tdone\n \n-.PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf\n+.PHONY: install-doc install-man install-man-perl install-html install-info install-pdf\n .PHONY: quick-install-doc quick-install-man quick-install-html\n-install-gitweb:\n-\t$(MAKE) -C gitweb install\n \n install-doc: install-man-perl\n \t$(MAKE) -C Documentation install\n@@ -3425,7 +3429,6 @@ clean: profile-clean coverage-clean cocciclean\n \t$(MAKE) -C Documentation/ clean\n \t$(RM) Documentation/GIT-EXCLUDED-PROGRAMS\n ifndef NO_PERL\n-\t$(MAKE) -C gitweb clean\n \t$(RM) -r perl/build/\n endif\n \t$(MAKE) -C templates/ clean\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex eaf0cfcf80e..66fceb9e94f 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -1,11 +1,8 @@\n-# The default target of this Makefile is...\n-all::\n-.PHONY: all\n-\n-MAK_DIR_GITWEB =\n+ifndef MAK_DIR_GITWEB\n+$(error do not run gitweb/Makefile stand-alone anymore. The \"gitweb\" and \\\n+\"install-gitweb\" targets now live in the top-level Makefile)\n+endif\n \n-# Define V=1 to have a more verbose compile.\n-#\n # Define JSMIN to point to JavaScript minifier that functions as\n # a filter to have static/gitweb.js minified.\n #\n@@ -13,13 +10,6 @@ MAK_DIR_GITWEB =\n # version of static/gitweb.css\n #\n \n-prefix ?= $(HOME)\n-bindir ?= $(prefix)/bin\n-gitwebdir ?= /var/www/cgi-bin\n-\n-RM ?= rm -f\n-INSTALL ?= install\n-\n # default configuration for gitweb\n GITWEB_CONFIG = gitweb_config.perl\n GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf\n@@ -44,71 +34,19 @@ GITWEB_SITE_HEADER =\n GITWEB_SITE_FOOTER =\n HIGHLIGHT_BIN = highlight\n \n-# include user config\n--include ../config.mak.autogen\n--include ../config.mak\n--include config.mak\n-\n-# determine version\n-.PHONY: .FORCE-GIT-VERSION-FILE\n-../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE\n-\t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n-\n-ifneq ($(MAKECMDGOALS),clean)\n--include ../GIT-VERSION-FILE\n-endif\n-\n # What targets we'll add to 'all' for \"make gitweb\"\n GITWEB_ALL =\n GITWEB_ALL += gitweb.cgi\n GITWEB_ALL += $(GITWEB_JS)\n \n-### Build rules\n-\n-SHELL_PATH ?= $(SHELL)\n-PERL_PATH ?= /usr/bin/perl\n-\n-# Shell quote;\n-bindir_SQ = $(subst ','\\'',$(bindir))#'\n-gitwebdir_SQ = $(subst ','\\'',$(gitwebdir))#'\n-gitwebstaticdir_SQ = $(subst ','\\'',$(gitwebdir)/static)#'\n-SHELL_PATH_SQ = $(subst ','\\'',$(SHELL_PATH))#'\n-PERL_PATH_SQ = $(subst ','\\'',$(PERL_PATH))#'\n-DESTDIR_SQ = $(subst ','\\'',$(DESTDIR))#'\n-\n-# Quiet generation (unless V=1)\n-QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir\n-QUIET_SUBDIR1 =\n-\n-ifneq ($(findstring $(MAKEFLAGS),w),w)\n-PRINT_DIR = --no-print-directory\n-else # \"make -w\"\n-NO_SUBDIR = :\n-endif\n-\n-ifneq ($(findstring $(MAKEFLAGS),s),s)\n-ifndef V\n-\tQUIET = @\n-\tQUIET_GEN = $(QUIET)echo ' ' GEN $@;\n-\tQUIET_SUBDIR0 = +@subdir=\n-\tQUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \\\n-\t $(MAKE) $(PRINT_DIR) -C $$subdir\n-\texport V\n-\texport QUIET\n-\texport QUIET_GEN\n-\texport QUIET_SUBDIR0\n-\texport QUIET_SUBDIR1\n-endif\n-endif\n-\n-all:: $(GITWEB_ALL)\n+MAK_DIR_GITWEB_ALL = $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_ALL))\n \n GITWEB_PROGRAMS = gitweb.cgi\n \n GITWEB_JS_MIN = static/gitweb.min.js\n ifdef JSMIN\n GITWEB_JS = $(GITWEB_JS_MIN)\n-all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)\n $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)\n \t$(QUIET_GEN)$(JSMIN) <$< >$@\n@@ -118,7 +56,7 @@ GITWEB_FILES += $(GITWEB_JS)\n GITWEB_CSS_MIN = static/gitweb.min.css\n ifdef CSSMIN\n GITWEB_CSS = $(GITWEB_CSS_MIN)\n-all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)\n $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS\n $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)\n \t$(QUIET_GEN)$(CSSMIN) <$< >$@\n@@ -185,19 +123,22 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_\n \n ### Installation rules\n \n-.PHONY: install\n-install: all\n+.PHONY: install-gitweb\n+install-gitweb: $(MAK_DIR_GITWEB_ALL)\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n-\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \\\n-\t\t'$(DESTDIR_SQ)$(gitwebdir_SQ)'\n+\t$(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \\\n \t\t'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n+ifndef NO_PERL\n+install: install-gitweb\n+endif\n \n ### Cleaning rules\n \n-.PHONY: clean\n-clean:\n+.PHONY: gitweb-clean\n+gitweb-clean:\n \t$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \\\n \t\t$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \\\n \t\tGITWEB-BUILD-OPTIONS)\n+clean: gitweb-clean\n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458027","messageId":"patch-v3-7.8-571c9c10319-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 7/8] Makefile: build 'gitweb' in the default target","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:16:01Z","receivedAt":"2022-06-28T10:16:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: SZEDER Gábor \n\nOur Makefile's default target used to build 'gitweb', though\nindirectly: the 'all' target depended on 'git-instaweb', which in turn\ndepended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency\nbetween git-instaweb and gitweb, 2015-05-29) removed the latter\ndependency, and for good reasons (quoting its commit message):\n\n \"1. git-instaweb has no build-time dependency on gitweb; it\n is a run-time dependency\n\n 2. gitweb is a directory that we want to recursively make\n in. As a result, its recipe is marked .PHONY, which\n causes \"make\" to rebuild git-instaweb every time it is\n run.\"\n\nSince then a simple 'make' doesn't build 'gitweb'.\n\nLuckily, installing 'gitweb' is not broken: although 'make install'\ndoesn't depend on the 'gitweb' target, it has a dependency on the\n'install-gitweb' target, which does generate all the necessary files\nfor 'gitweb' and installs them. However, if someone runs 'make &&\nsudo make install', then those files in the 'gitweb' directory will be\ngenerated and owned by root, which is not nice.\n\nList 'gitweb' as a direct dependency of the default target, so a plain\n'make' will build it.\n\nSigned-off-by: SZEDER Gábor \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 a5f29c11681..d150be4dc32 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -3155,6 +3155,7 @@ include gitweb/Makefile\n \n .PHONY: gitweb\n gitweb: $(MAK_DIR_GITWEB_ALL)\n+all:: gitweb\n \n ### Installation rules\n \n-- \n2.37.0.880.gf07d56b18ba\n\n"},{"id":"458028","messageId":"patch-v3-8.8-0c8f26ee876-20220628T100936Z-avarab@gmail.com","threadId":"57923","inReplyTo":"cover-v3-0.8-00000000000-20220628T100936Z-avarab@gmail.com","subject":"[PATCH v3 8/8] gitweb/Makefile: add a \"NO_GITWEB\" parameter","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-06-28T10:16:02Z","receivedAt":"2022-06-28T10:16:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From looking at the {Free,Net,Dragonfly}BSD packages for git[1]\nthey've been monkeypatching \"gitweb\" out of the Makefile, let's be\nnicer and provide a NO_GITWEB=Y for their use.\n\nFor the \"all\" target this allows for optionally restoring what's been\nthe status quo before the preceding commit, but now we'll also behave\ncorrectly on the subsequent \"make install\".\n\nAs before our installation of gitweb can be suppressed with\nNO_PERL. For backwards compatibility the NO_PERL=Y flag by itself\nstill doesn't change whether or not we build gitweb, unlike the new\nNO_GITWEB=Y flag.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 7 +++++++\n gitweb/Makefile | 2 ++\n 2 files changed, 9 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex d150be4dc32..92bea603394 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -309,6 +309,11 @@ include shared.mak\n # distributions that want to use their packaged versions of Perl\n # modules, instead of the fallbacks shipped with Git.\n #\n+# Define NO_GITWEB if you do not want to build or install\n+# 'gitweb'. Note that defining NO_PERL currently has the same effect\n+# on not installing gitweb, but not on whether it's built in the\n+# gitweb/ directory.\n+#\n # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python\n # but /usr/bin/python2.7 or /usr/bin/python3 on some platforms).\n #\n@@ -3155,7 +3160,9 @@ include gitweb/Makefile\n \n .PHONY: gitweb\n gitweb: $(MAK_DIR_GITWEB_ALL)\n+ifndef NO_GITWEB\n all:: gitweb\n+endif\n \n ### Installation rules\n \ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 66fceb9e94f..3b68ab2d672 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -130,9 +130,11 @@ install-gitweb: $(MAK_DIR_GITWEB_ALL)\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \\\n \t\t'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n+ifndef NO_GITWEB\n ifndef NO_PERL\n install: install-gitweb\n endif\n+endif\n \n ### Cleaning rules\n \n-- \n2.37.0.880.gf07d56b18ba\n\n"}]}