* [PATCH] Makefile: build 'gitweb' in the default target
@ 2022-05-25 20:56 SZEDER Gábor
2022-05-26 0:14 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 31+ messages in thread
From: SZEDER Gábor @ 2022-05-25 20:56 UTC (permalink / raw)
To: git; +Cc: SZEDER Gábor
Our Makefile's default target used to build 'gitweb', though
indirectly: the 'all' target depended on 'git-instaweb', which in turn
depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
between git-instaweb and gitweb, 2015-05-29) removed the latter
dependency, and for good reasons (quoting its commit message):
"1. git-instaweb has no build-time dependency on gitweb; it
is a run-time dependency
2. gitweb is a directory that we want to recursively make
in. As a result, its recipe is marked .PHONY, which
causes "make" to rebuild git-instaweb every time it is
run."
Since then a simple 'make' doesn't build 'gitweb'.
Luckily, installing 'gitweb' is not broken: although 'make install'
doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
install' unconditionally, which does generate all the necessary files
for 'gitweb' and installs them. However, if someone runs 'make &&
sudo make install', then those files in the 'gitweb' directory will be
generated and owned by root, which is not nice.
List 'gitweb' as a direct dependency of the default target, so a plain
'make' will build it.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Makefile b/Makefile
index f8bccfab5e..ee74892b33 100644
--- a/Makefile
+++ b/Makefile
@@ -2188,6 +2188,8 @@ ifneq (,$X)
$(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';)
endif
+all:: gitweb
+
all::
ifndef NO_TCLTK
$(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
--
2.36.1.427.gb7c35dfc0c
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: build 'gitweb' in the default target
2022-05-25 20:56 [PATCH] Makefile: build 'gitweb' in the default target SZEDER Gábor
@ 2022-05-26 0:14 ` Ævar Arnfjörð Bjarmason
2022-05-26 7:57 ` Jeff King
2022-05-26 21:33 ` SZEDER Gábor
0 siblings, 2 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-26 0:14 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Wed, May 25 2022, SZEDER Gábor wrote:
> Our Makefile's default target used to build 'gitweb', though
> indirectly: the 'all' target depended on 'git-instaweb', which in turn
> depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
> between git-instaweb and gitweb, 2015-05-29) removed the latter
> dependency, and for good reasons (quoting its commit message):
>
> "1. git-instaweb has no build-time dependency on gitweb; it
> is a run-time dependency
>
> 2. gitweb is a directory that we want to recursively make
> in. As a result, its recipe is marked .PHONY, which
> causes "make" to rebuild git-instaweb every time it is
> run."
>
> Since then a simple 'make' doesn't build 'gitweb'.
>
> Luckily, installing 'gitweb' is not broken: although 'make install'
> doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
> install' unconditionally, which does generate all the necessary files
> for 'gitweb' and installs them. However, if someone runs 'make &&
> sudo make install', then those files in the 'gitweb' directory will be
> generated and owned by root, which is not nice.
>
> List 'gitweb' as a direct dependency of the default target, so a plain
> 'make' will build it.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index f8bccfab5e..ee74892b33 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2188,6 +2188,8 @@ ifneq (,$X)
> $(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';)
> endif
>
> +all:: gitweb
> +
> all::
> ifndef NO_TCLTK
> $(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
In various recent patches & some upcoming ones I plan to submit I've
been trying to get the runtime of a noop "make" runs down, which really
helps e.g. with "git rebase -x make ..." running faster on a large
series.
While you're right that this wasn't intentional to begin with, we have
lacked the "gitweb" as part of the default target since v2.4.5 now, and
adding it back is a major performance regression on noop "make" runs:
$ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5
Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]
Range (min … max): 102.2 ms … 105.2 ms 5 runs
Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]
Range (min … max): 189.2 ms … 193.3 ms 5 runs
Benchmark 3: make NO_TCLTK=' in 'HEAD~1
Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]
Range (min … max): 266.7 ms … 277.3 ms 5 runs
Benchmark 4: make NO_TCLTK=' in 'HEAD~0
Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]
Range (min … max): 356.6 ms … 360.0 ms 5 runs
Summary
'make NO_TCLTK=Y' in 'HEAD~1' ran
1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'
2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'
3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'
I.e. this is with your patch here applied as HEAD~0 and HEAD~1 being
'master'.
I think given that that a better solution would be to just declare this
as a feature at this point, especially as gitweb/INSTALL notes that the
way to install it is:
$ make prefix=/usr gitweb ;# as yourself
# make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root
Or we could just fold gitweb/Makefile into the main Makefile, unlike
gitk and git-gui it's not externally maintained, and most of it is
shimmying to work around not being part of the main Makefile (which it
strongly inter-depends on anyway).
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: build 'gitweb' in the default target
2022-05-26 0:14 ` Ævar Arnfjörð Bjarmason
@ 2022-05-26 7:57 ` Jeff King
2022-05-26 21:33 ` SZEDER Gábor
1 sibling, 0 replies; 31+ messages in thread
From: Jeff King @ 2022-05-26 7:57 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: SZEDER Gábor, git
On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:
> > Our Makefile's default target used to build 'gitweb', though
> > indirectly: the 'all' target depended on 'git-instaweb', which in turn
> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
> > between git-instaweb and gitweb, 2015-05-29) removed the latter
> > dependency, and for good reasons (quoting its commit message):
> [...]
> In various recent patches & some upcoming ones I plan to submit I've
> been trying to get the runtime of a noop "make" runs down, which really
> helps e.g. with "git rebase -x make ..." running faster on a large
> series.
>
> While you're right that this wasn't intentional to begin with, we have
> lacked the "gitweb" as part of the default target since v2.4.5 now, and
> adding it back is a major performance regression on noop "make" runs:
Yes, I don't think building gitweb is worth the performance cost.
Speeding things up was part of my original goal in e25c7cc146. It would
be one thing if this were a recent change and somebody might be broken
or confused by it not being built by default. But after 7 years, I think
the question is: why _would_ we want to change the status quo and build
gitweb by default?
To exercise its Makefile for bugs, I guess, but IMHO it is not worth
inflicting that on random developers. People who care about gitweb (if
any) can build it themselves. I'd be even happier if it were just
carried in its own tree.
-Peff
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: build 'gitweb' in the default target
2022-05-26 0:14 ` Ævar Arnfjörð Bjarmason
2022-05-26 7:57 ` Jeff King
@ 2022-05-26 21:33 ` SZEDER Gábor
2022-05-27 9:23 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 31+ messages in thread
From: SZEDER Gábor @ 2022-05-26 21:33 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git
On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, May 25 2022, SZEDER Gábor wrote:
>
> > Our Makefile's default target used to build 'gitweb', though
> > indirectly: the 'all' target depended on 'git-instaweb', which in turn
> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
> > between git-instaweb and gitweb, 2015-05-29) removed the latter
> > dependency, and for good reasons (quoting its commit message):
> >
> > "1. git-instaweb has no build-time dependency on gitweb; it
> > is a run-time dependency
> >
> > 2. gitweb is a directory that we want to recursively make
> > in. As a result, its recipe is marked .PHONY, which
> > causes "make" to rebuild git-instaweb every time it is
> > run."
> >
> > Since then a simple 'make' doesn't build 'gitweb'.
> >
> > Luckily, installing 'gitweb' is not broken: although 'make install'
> > doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
> > install' unconditionally, which does generate all the necessary files
> > for 'gitweb' and installs them. However, if someone runs 'make &&
> > sudo make install', then those files in the 'gitweb' directory will be
> > generated and owned by root, which is not nice.
> >
> > List 'gitweb' as a direct dependency of the default target, so a plain
> > 'make' will build it.
> >
> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> > ---
> > Makefile | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index f8bccfab5e..ee74892b33 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2188,6 +2188,8 @@ ifneq (,$X)
> > $(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';)
> > endif
> >
> > +all:: gitweb
> > +
> > all::
> > ifndef NO_TCLTK
> > $(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
>
> In various recent patches & some upcoming ones I plan to submit I've
> been trying to get the runtime of a noop "make" runs down, which really
> helps e.g. with "git rebase -x make ..." running faster on a large
> series.
>
> While you're right that this wasn't intentional to begin with, we have
> lacked the "gitweb" as part of the default target since v2.4.5 now, and
> adding it back is a major performance regression on noop "make" runs:
I think that generating stuff, potentially as root, during 'make
install' is a more severe regression, than this noop make slowdown,
which in practice tends to be lost in the noise anyway. Even in an
unrealistic case (it doesn't modify any C source files explicitly, let
alone a frequently included header file) like this:
$ git checkout fddc3b420f^
$ make
[...]
$ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done
$ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10
[...]
real 0m31.026s
user 0m46.897s
sys 0m11.492s
$ git checkout fddc3b420f
$ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done
$ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10
[...]
real 0m30.865s
user 0m48.315s
sys 0m12.125s
Hrm, it actually ended up slightly faster.
> $ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5
> Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
> Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]
> Range (min … max): 102.2 ms … 105.2 ms 5 runs
>
> Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
> Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]
> Range (min … max): 189.2 ms … 193.3 ms 5 runs
>
> Benchmark 3: make NO_TCLTK=' in 'HEAD~1
> Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]
> Range (min … max): 266.7 ms … 277.3 ms 5 runs
>
> Benchmark 4: make NO_TCLTK=' in 'HEAD~0
> Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]
> Range (min … max): 356.6 ms … 360.0 ms 5 runs
>
> Summary
> 'make NO_TCLTK=Y' in 'HEAD~1' ran
> 1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'
> 2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'
> 3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'
>
> I.e. this is with your patch here applied as HEAD~0 and HEAD~1 being
> 'master'.
>
> I think given that that a better solution would be to just declare this
> as a feature at this point
As long as 'make install' installs 'gitweb', I don't think that's an
option.
> especially as gitweb/INSTALL notes that the
> way to install it is:
>
> $ make prefix=/usr gitweb ;# as yourself
> # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root
Or are you suggesting not to install 'gitweb' during 'make install'?
I'm fine with that, but I doubt I will argue about it convincingly in
a commit message.
> Or we could just fold gitweb/Makefile into the main Makefile, unlike
> gitk and git-gui it's not externally maintained, and most of it is
> shimmying to work around not being part of the main Makefile (which it
> strongly inter-depends on anyway).
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] Makefile: build 'gitweb' in the default target
2022-05-26 21:33 ` SZEDER Gábor
@ 2022-05-27 9:23 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-27 9:23 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
On Thu, May 26 2022, SZEDER Gábor wrote:
> On Thu, May 26, 2022 at 02:14:33AM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, May 25 2022, SZEDER Gábor wrote:
>>
>> > Our Makefile's default target used to build 'gitweb', though
>> > indirectly: the 'all' target depended on 'git-instaweb', which in turn
>> > depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
>> > between git-instaweb and gitweb, 2015-05-29) removed the latter
>> > dependency, and for good reasons (quoting its commit message):
>> >
>> > "1. git-instaweb has no build-time dependency on gitweb; it
>> > is a run-time dependency
>> >
>> > 2. gitweb is a directory that we want to recursively make
>> > in. As a result, its recipe is marked .PHONY, which
>> > causes "make" to rebuild git-instaweb every time it is
>> > run."
>> >
>> > Since then a simple 'make' doesn't build 'gitweb'.
>> >
>> > Luckily, installing 'gitweb' is not broken: although 'make install'
>> > doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
>> > install' unconditionally, which does generate all the necessary files
>> > for 'gitweb' and installs them. However, if someone runs 'make &&
>> > sudo make install', then those files in the 'gitweb' directory will be
>> > generated and owned by root, which is not nice.
>> >
>> > List 'gitweb' as a direct dependency of the default target, so a plain
>> > 'make' will build it.
>> >
>> > Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> > ---
>> > Makefile | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index f8bccfab5e..ee74892b33 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -2188,6 +2188,8 @@ ifneq (,$X)
>> > $(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';)
>> > endif
>> >
>> > +all:: gitweb
>> > +
>> > all::
>> > ifndef NO_TCLTK
>> > $(QUIET_SUBDIR0)git-gui $(QUIET_SUBDIR1) gitexecdir='$(gitexec_instdir_SQ)' all
>>
>> In various recent patches & some upcoming ones I plan to submit I've
>> been trying to get the runtime of a noop "make" runs down, which really
>> helps e.g. with "git rebase -x make ..." running faster on a large
>> series.
>>
>> While you're right that this wasn't intentional to begin with, we have
>> lacked the "gitweb" as part of the default target since v2.4.5 now, and
>> adding it back is a major performance regression on noop "make" runs:
>
> I think that generating stuff, potentially as root, during 'make
> install' is a more severe regression, than this noop make slowdown,
> which in practice tends to be lost in the noise anyway.
It really isn't lost in the noise, e.g. a common use-case of mine is to
have let's say a 10 patch series that I want to frequently run a spot
check like this on:
git rebase -i -x 'make'
On my system before your patch (this is with my config.mak, so it has
e.g. NO_TCLTK=Y) it's ~105ms per run, with your change ~200ms[1].
So that's 1s before of "make" overhead, now 2s. Is it the end of the
world? No, but it does add up. In particular with ccache we can end up
mostly spending time on make itself deciding what it's going to do.
For comparison running EDITOR=cat git rebase -i -x 'echo make' HEAD~10
takes around 100ms in total for me.
> unrealistic case (it doesn't modify any C source files explicitly, let
> alone a frequently included header file) like this:
Yeah that's fair, of course no-op runs are where it matters the
most. But e.g. for forcing a hook.c change (including re-linking "git")
it's 10% slower[2] overall if we use ccache.
> $ git checkout fddc3b420f^
> $ make
> [...]
> $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done
> $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10
> [...]
> real 0m31.026s
> user 0m46.897s
> sys 0m11.492s
> $ git checkout fddc3b420f
> $ for i in {1..10} ; do git commit --allow-empty -q -m $i ; done
> $ time git rebase -x 'make -j8 NO_TCLTK=Y >/dev/null' HEAD~10
> [...]
> real 0m30.865s
> user 0m48.315s
> sys 0m12.125s
>
> Hrm, it actually ended up slightly faster.
I should have said that I create a "version" file in my checkout dir,
e.g.: 2.36.GIT-dev
Otherwise you end up with a guarnteed recompile-re-link every time HEAD
changes, so it's likely lost in that noise.
I just run this (via a script):
echo $(/usr/bin/git grep -h -o -P '(?<=^DEF_VER=v).*' 'HEAD:GIT-VERSION-GEN')-dev
And echo it to the top-level "version". Aside from what we're discussing
here you might want to try that, your "git version" won't be correct,
but for that low cost (I do generate it for the version I actually
install) you'll get actual no-op runs in the case of e.g. only modifying
tests.
>> $ git hyperfine -L rev HEAD~1,HEAD~0 -L t Y, -s 'make' 'make NO_TCLTK={t}' --warmup 1 -r 5
>> Benchmark 1: make NO_TCLTK=Y' in 'HEAD~1
>> Time (mean ± σ): 103.6 ms ± 1.1 ms [User: 83.8 ms, System: 32.1 ms]
>> Range (min … max): 102.2 ms … 105.2 ms 5 runs
>>
>> Benchmark 2: make NO_TCLTK=Y' in 'HEAD~0
>> Time (mean ± σ): 191.4 ms ± 1.6 ms [User: 151.0 ms, System: 60.5 ms]
>> Range (min … max): 189.2 ms … 193.3 ms 5 runs
>>
>> Benchmark 3: make NO_TCLTK=' in 'HEAD~1
>> Time (mean ± σ): 272.0 ms ± 5.0 ms [User: 206.3 ms, System: 83.3 ms]
>> Range (min … max): 266.7 ms … 277.3 ms 5 runs
>>
>> Benchmark 4: make NO_TCLTK=' in 'HEAD~0
>> Time (mean ± σ): 358.3 ms ± 1.4 ms [User: 282.7 ms, System: 104.0 ms]
>> Range (min … max): 356.6 ms … 360.0 ms 5 runs
>>
>> Summary
>> 'make NO_TCLTK=Y' in 'HEAD~1' ran
>> 1.85 ± 0.02 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'
>> 2.63 ± 0.06 times faster than 'make NO_TCLTK=' in 'HEAD~1'
>> 3.46 ± 0.04 times faster than 'make NO_TCLTK=' in 'HEAD~0'
>>
>> I.e. this is with your patch here applied as HEAD~0 and HEAD~1 being
>> 'master'.
>>
>> I think given that that a better solution would be to just declare this
>> as a feature at this point
>
> As long as 'make install' installs 'gitweb', I don't think that's an
> option.
Yes, I'm not saying that we shouldn't fix this no matter what, but just
suggesting that perhaps we come up with a better solution.
>> especially as gitweb/INSTALL notes that the
>> way to install it is:
>>
>> $ make prefix=/usr gitweb ;# as yourself
>> # make gitwebdir=/var/www/cgi-bin install-gitweb ;# as root
>
> Or are you suggesting not to install 'gitweb' during 'make install'?
> I'm fine with that, but I doubt I will argue about it convincingly in
> a commit message.
We could also:
* $(error out if install is in MAKECMDGOALS and we don't have those
generated files, that's a few-line ifdef change to the Makefile,
"build it first".
* Just fold gitweb/Makefile into the top-level Makefile
>> Or we could just fold gitweb/Makefile into the main Makefile, unlike
>> gitk and git-gui it's not externally maintained, and most of it is
>> shimmying to work around not being part of the main Makefile (which it
>> strongly inter-depends on anyway).
1.
$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make' 'make'
Benchmark 1: make' in 'HEAD~1
Time (mean ± σ): 106.9 ms ± 1.3 ms [User: 86.0 ms, System: 33.1 ms]
Range (min … max): 105.3 ms … 110.2 ms 27 runs
Benchmark 2: make' in 'HEAD~0
Time (mean ± σ): 199.6 ms ± 2.5 ms [User: 162.7 ms, System: 57.3 ms]
Range (min … max): 197.4 ms … 207.4 ms 15 runs
Summary
'make' in 'HEAD~1' ran
1.87 ± 0.03 times faster than 'make' in 'HEAD~0'
2.
$ git hyperfine -L rev HEAD~1,HEAD~0 -s 'make' 'make -W hook.c NO_TCLTK=Y CC="ccache cc"' --warmup 1 -r 5
Benchmark 1: make -W hook.c NO_TCLTK=Y CC="ccache cc"' in 'HEAD~1
Time (mean ± σ): 742.7 ms ± 7.1 ms [User: 1965.5 ms, System: 561.6 ms]
Range (min … max): 730.3 ms … 748.2 ms 5 runs
Benchmark 2: make -W hook.c NO_TCLTK=Y CC="ccache cc"' in 'HEAD~0
Time (mean ± σ): 819.2 ms ± 7.4 ms [User: 2013.9 ms, System: 583.3 ms]
Range (min … max): 811.0 ms … 830.0 ms 5 runs
Summary
'make -W hook.c NO_TCLTK=Y CC="ccache cc"' in 'HEAD~1' ran
1.10 ± 0.01 times faster than 'make -W hook.c NO_TCLTK=Y CC="ccache cc"' in 'HEAD~0'
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-05-27 9:23 ` Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
` (8 more replies)
0 siblings, 9 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
The $subject is a proposed re-roll of SZEDER's
https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;
As noted downthread of that fix having the Makefile invoke "make -C
gitweb" again would slow us down on NOOP runs by quite a bit.
This series fixes the same regression with an amended 7/7 version of
SZEDER's fix, but starts out by having the top-level Makefile include
the gitweb/Makefile.
This series is smaller than it looks, most of the commits preceding
5-6/7 are there to make that diff smaller and easier to read, by
splitting up earlier changes into non-functional changes.
For this re-roll the equivalent of the "git hyperfine" command I
posted in [2] will return;
Summary
'make NO_TCLTK=Y' in 'origin/master' ran
1.00 ± 0.11 times faster than 'make NO_TCLTK=Y' in 'HEAD~0'
2.64 ± 0.26 times faster than 'make NO_TCLTK=' in 'origin/master'
2.64 ± 0.24 times faster than 'make NO_TCLTK=' in 'HEAD~0'
I.e. we are no slower or faster than before, but now "make && sudo
make install-gitweb" will only copy already-generated files from the
"make" command in the "sudo" step, as intended.
1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com
2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/
SZEDER Gábor (1):
Makefile: build 'gitweb' in the default target
Ævar Arnfjörð Bjarmason (6):
gitweb/Makefile: define all .PHONY prerequisites inline
gitweb/Makefile: add a $(GITWEB_ALL) variable
gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
gitweb/Makefile: prepare to merge into top-level Makefile
gitweb: remove "test" and "test-installed" targets
gitweb/Makefile: include in top-level Makefile
Makefile | 24 ++++----
gitweb/Makefile | 143 +++++++++++++++---------------------------------
t/Makefile | 4 --
3 files changed, 59 insertions(+), 112 deletions(-)
Range-diff against v1:
1: 1bbffa8a2b6 < -: ----------- Makefile: build 'gitweb' in the default target
-: ----------- > 1: 14361617ca6 gitweb/Makefile: define all .PHONY prerequisites inline
-: ----------- > 2: 7d920a13518 gitweb/Makefile: add a $(GITWEB_ALL) variable
-: ----------- > 3: e14a5b73061 gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
-: ----------- > 4: 02e26ca8ce2 gitweb/Makefile: prepare to merge into top-level Makefile
-: ----------- > 5: caf376f3dd9 gitweb: remove "test" and "test-installed" targets
-: ----------- > 6: b423cd58f6b gitweb/Makefile: include in top-level Makefile
-: ----------- > 7: 69428540886 Makefile: build 'gitweb' in the default target
--
2.36.1.1103.g036c05811b0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 2/7] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Move the '.PHONY' definition so that it's split up and accompanies the
relevant as they're defined. This will make a subsequent diff smaller
as we'll remove some of these, and won't need to re-edit the
now-removed '.PHONY' line.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index f13e23c4de4..abb5c9f9ab6 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -1,5 +1,6 @@
# The default target of this Makefile is...
all::
+.PHONY: all
# Define V=1 to have a more verbose compile.
#
@@ -45,6 +46,7 @@ HIGHLIGHT_BIN = highlight
-include config.mak
# determine version
+.PHONY: .FORCE-GIT-VERSION-FILE
../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
@@ -152,6 +154,7 @@ GITWEB_REPLACE = \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+.PHONY: FORCE
GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
@@ -171,15 +174,18 @@ static/gitweb.js: $(GITWEB_JSLIB_FILES)
### Testing rules
+.PHONY: test
test:
$(MAKE) -C ../t gitweb-test
+.PHONY: test-installed
test-installed:
GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
$(MAKE) -C ../t gitweb-test
### Installation rules
+.PHONY: install
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
@@ -188,10 +194,8 @@ install: all
### Cleaning rules
+.PHONY: clean
clean:
$(RM) gitweb.cgi static/gitweb.js \
static/gitweb.min.js static/gitweb.min.css \
GITWEB-BUILD-OPTIONS
-
-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
-
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 2/7] gitweb/Makefile: add a $(GITWEB_ALL) variable
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 3/7] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Declare the targets that the "all" target depends on with a new
$(GITWEB_ALL) variable. This will help to reduce churn in subsequent
commits.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index abb5c9f9ab6..733b60f925e 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -54,6 +54,11 @@ ifneq ($(MAKECMDGOALS),clean)
-include ../GIT-VERSION-FILE
endif
+# What targets we'll add to 'all' for "make gitweb"
+GITWEB_ALL =
+GITWEB_ALL += gitweb.cgi
+GITWEB_ALL += static/gitweb.js
+
### Build rules
SHELL_PATH ?= $(SHELL)
@@ -92,7 +97,7 @@ ifndef V
endif
endif
-all:: gitweb.cgi static/gitweb.js
+all:: $(GITWEB_ALL)
GITWEB_PROGRAMS = gitweb.cgi
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 3/7] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 2/7] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 4/7] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Change the variable definitions for the $(GITWEB_CSS) and $(GITWEB_JS)
so that we have a clear separation between what we use as "in" files,
v.s. our "min" files. We can now make the appending to $(GITWEB_FILES)
unconditional, since $(GITWEB_{JS,CSS}) is either the "min" or
non-"min" version. This reduces the duplication within the file.
While we're at it let's initialize "GITWEB_JSLIB_FILES" as we normally
do with such variables.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 733b60f925e..a8752c1f11c 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -31,10 +31,12 @@ GITWEB_STRICT_EXPORT =
GITWEB_BASE_URL =
GITWEB_LIST =
GITWEB_HOMETEXT = indextext.html
-GITWEB_CSS = static/gitweb.css
+GITWEB_CSS_IN = static/gitweb.css
+GITWEB_CSS = $(GITWEB_CSS_IN)
GITWEB_LOGO = static/git-logo.png
GITWEB_FAVICON = static/git-favicon.png
-GITWEB_JS = static/gitweb.js
+GITWEB_JS_IN = static/gitweb.js
+GITWEB_JS = $(GITWEB_JS_IN)
GITWEB_SITE_HTML_HEAD_STRING =
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -57,7 +59,7 @@ endif
# What targets we'll add to 'all' for "make gitweb"
GITWEB_ALL =
GITWEB_ALL += gitweb.cgi
-GITWEB_ALL += static/gitweb.js
+GITWEB_ALL += $(GITWEB_JS)
### Build rules
@@ -101,25 +103,23 @@ all:: $(GITWEB_ALL)
GITWEB_PROGRAMS = gitweb.cgi
+GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
-GITWEB_FILES += static/gitweb.min.js
-GITWEB_JS = static/gitweb.min.js
-all:: static/gitweb.min.js
-static/gitweb.min.js: static/gitweb.js GITWEB-BUILD-OPTIONS
+GITWEB_JS = $(GITWEB_JS_MIN)
+all:: $(GITWEB_JS_MIN)
+$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS
$(QUIET_GEN)$(JSMIN) <$< >$@
-else
-GITWEB_FILES += static/gitweb.js
endif
+GITWEB_FILES += $(GITWEB_JS)
+GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
-GITWEB_FILES += static/gitweb.min.css
-GITWEB_CSS = static/gitweb.min.css
-all:: static/gitweb.min.css
-static/gitweb.min.css: static/gitweb.css GITWEB-BUILD-OPTIONS
+GITWEB_CSS = $(GITWEB_CSS_MIN)
+all:: $(GITWEB_CSS_MIN)
+$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS
$(QUIET_GEN)$(CSSMIN) <$< >$@
-else
-GITWEB_FILES += static/gitweb.css
endif
+GITWEB_FILES += $(GITWEB_CSS)
GITWEB_FILES += static/git-logo.png static/git-favicon.png
@@ -127,6 +127,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png
#
# js/lib/common-lib.js should be always first, then js/lib/*.js,
# then the rest of files; js/gitweb.js should be last (if it exists)
+GITWEB_JSLIB_FILES =
GITWEB_JSLIB_FILES += static/js/lib/common-lib.js
GITWEB_JSLIB_FILES += static/js/lib/datetime.js
GITWEB_JSLIB_FILES += static/js/lib/cookies.js
@@ -201,6 +202,6 @@ install: all
.PHONY: clean
clean:
- $(RM) gitweb.cgi static/gitweb.js \
- static/gitweb.min.js static/gitweb.min.css \
+ $(RM) gitweb.cgi $(GITWEB_JS_IN) \
+ $(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
GITWEB-BUILD-OPTIONS
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 4/7] gitweb/Makefile: prepare to merge into top-level Makefile
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2022-05-31 17:45 ` [PATCH v2 3/7] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 5/7] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Since the "gitweb/Makefile" was split out from the top-level Makefile
in 62331ef1637 (gitweb: Makefile improvements, 2010-01-30) we've kept
the inter-dependencies between the two, and worse have dealt with a
lot of duplication as a result.
In preparation for merging the two again add a MAK_DIR_GITWEB variable
to various rules in it. This will allow us to set this variable to
"gitweb/" as we include it in the top-level Makefile, which will
minimize the size of the subsequent diff.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index a8752c1f11c..74e896767e9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -2,6 +2,8 @@
all::
.PHONY: all
+MAK_DIR_GITWEB =
+
# Define V=1 to have a more verbose compile.
#
# Define JSMIN to point to JavaScript minifier that functions as
@@ -106,8 +108,9 @@ GITWEB_PROGRAMS = gitweb.cgi
GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
GITWEB_JS = $(GITWEB_JS_MIN)
-all:: $(GITWEB_JS_MIN)
-$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS
+all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)
$(QUIET_GEN)$(JSMIN) <$< >$@
endif
GITWEB_FILES += $(GITWEB_JS)
@@ -115,8 +118,9 @@ GITWEB_FILES += $(GITWEB_JS)
GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
GITWEB_CSS = $(GITWEB_CSS_MIN)
-all:: $(GITWEB_CSS_MIN)
-$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS
+all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)
$(QUIET_GEN)$(CSSMIN) <$< >$@
endif
GITWEB_FILES += $(GITWEB_CSS)
@@ -161,19 +165,20 @@ GITWEB_REPLACE = \
-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
.PHONY: FORCE
-GITWEB-BUILD-OPTIONS: FORCE
+$(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@
-gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
$(GITWEB_REPLACE) $< >$@+ && \
chmod +x $@+ && \
mv $@+ $@
-static/gitweb.js: $(GITWEB_JSLIB_FILES)
+$(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
$(QUIET_GEN)$(RM) $@ $@+ && \
cat $^ >$@+ && \
mv $@+ $@
@@ -194,14 +199,16 @@ test-installed:
.PHONY: install
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
- $(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+ $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \
+ '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
- $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+ $(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \
+ '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
### Cleaning rules
.PHONY: clean
clean:
- $(RM) gitweb.cgi $(GITWEB_JS_IN) \
+ $(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \
$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
- GITWEB-BUILD-OPTIONS
+ GITWEB-BUILD-OPTIONS)
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 5/7] gitweb: remove "test" and "test-installed" targets
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2022-05-31 17:45 ` [PATCH v2 4/7] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 6/7] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Remove the special "test" targets for gitweb added in
958a8467216 (gitweb/Makefile: Add 'test' and 'test-installed' targets,
2010-09-26). Unlike e.g. "contrib/scalar" and "contrib/subtree" the
"gitweb" tests themselves live in our top-level t/ directory.
It therefore doesn't make sense to maintain this indirection, no more
than it would to have a "git-send-email-test". By dropping it we'll
also free other tests to use the t95*.sh prefix.
These removed targets are unlikely to be used by anyone, and to the
extent that they are we can easily use an invocation like this
instead:
make test T='t[0-9]*gitweb*.sh'
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 11 -----------
t/Makefile | 4 ----
2 files changed, 15 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 74e896767e9..eaf0cfcf80e 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -183,17 +183,6 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_
cat $^ >$@+ && \
mv $@+ $@
-### Testing rules
-
-.PHONY: test
-test:
- $(MAKE) -C ../t gitweb-test
-
-.PHONY: test-installed
-test-installed:
- GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
- $(MAKE) -C ../t gitweb-test
-
### Installation rules
.PHONY: install
diff --git a/t/Makefile b/t/Makefile
index 056ce55dcc9..7f56e52f767 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -35,7 +35,6 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
-TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
@@ -112,9 +111,6 @@ aggregate-results:
echo "$$f"; \
done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh
-gitweb-test:
- $(MAKE) $(TGITWEB)
-
valgrind:
$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 6/7] gitweb/Makefile: include in top-level Makefile
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2022-05-31 17:45 ` [PATCH v2 5/7] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:45 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:46 ` [PATCH v2 7/7] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:45 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Include the gitweb/Makefile in the top-level Makefile rather than
calling it as a sub-Makefile. As noted in the thread starting at at
[1] (in particular [2]) we'll pay a high cost on NOOP runs of "make"
just to figure out that we have nothing to do for "make gitweb".
The "gitweb" script also isn't maintained out-of-tree, unlike
"gitk-git" or "git-gui", which both have their own "Makefile". Other
parts of it are already integrated into our main Makefiles, e.g. the
documentation is built by Documentation/Makefile since
07ea4df2780 (gitweb: Add gitweb(1) manpage for gitweb itself,
2011-10-16).
1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com/
2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Perhaps a better solution is to simply move all of this code to the
top-level gitweb/Makefile, but on the other hand being able to include
the gitweb/Makefile to fold it into the top-level one without a larger
diff makes this sort of change much easier to review.
And, as shown here we'll have "make -C gitweb" now $(error) out, so
there's no chance of confusion in practice.
Makefile | 23 +++++++------
gitweb/Makefile | 91 +++++++++----------------------------------------
2 files changed, 29 insertions(+), 85 deletions(-)
diff --git a/Makefile b/Makefile
index 18ca6744a50..d251838c554 100644
--- a/Makefile
+++ b/Makefile
@@ -538,6 +538,7 @@ gitexecdir = libexec/git-core
mergetoolsdir = $(gitexecdir)/mergetools
sharedir = $(prefix)/share
gitwebdir = $(sharedir)/gitweb
+gitwebstaticdir = $(gitwebdir)/static
perllibdir = $(sharedir)/perl5
localedir = $(sharedir)/locale
template_dir = share/git-core/templates
@@ -556,7 +557,7 @@ localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))
-export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir
+export prefix bindir sharedir sysconfdir perllibdir localedir
# Set our default programs
CC = cc
@@ -2071,6 +2072,7 @@ htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
prefix_SQ = $(subst ','\'',$(prefix))
perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
+gitwebstaticdir_SQ = $(subst ','\'',$(gitwebstaticdir))
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
@@ -2399,10 +2401,6 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
perllibdir:
@echo '$(perllibdir_SQ)'
-.PHONY: gitweb
-gitweb:
- $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
-
git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
chmod +x $@+ && \
@@ -3036,6 +3034,15 @@ coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.c
.PHONY: coccicheck coccicheck-pending
+# "Sub"-Makefiles, not really because they can't be run stand-alone,
+# only there to contain directory-specific rules and variables
+## gitweb/Makefile inclusion:
+MAK_DIR_GITWEB = gitweb/
+include gitweb/Makefile
+
+.PHONY: gitweb
+gitweb: $(MAK_DIR_GITWEB_ALL)
+
### Installation rules
ifneq ($(filter /%,$(firstword $(template_dir))),)
@@ -3108,7 +3115,6 @@ ifndef NO_PERL
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
(cd perl/build/lib && $(TAR) cf - .) | \
(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
- $(MAKE) -C gitweb install
endif
ifndef NO_TCLTK
$(MAKE) -C gitk-git install
@@ -3163,10 +3169,8 @@ endif
cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
done
-.PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf
+.PHONY: install-doc install-man install-man-perl install-html install-info install-pdf
.PHONY: quick-install-doc quick-install-man quick-install-html
-install-gitweb:
- $(MAKE) -C gitweb install
install-doc: install-man-perl
$(MAKE) -C Documentation install
@@ -3310,7 +3314,6 @@ clean: profile-clean coverage-clean cocciclean
$(MAKE) -C Documentation/ clean
$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
ifndef NO_PERL
- $(MAKE) -C gitweb clean
$(RM) -r perl/build/
endif
$(MAKE) -C templates/ clean
diff --git a/gitweb/Makefile b/gitweb/Makefile
index eaf0cfcf80e..66fceb9e94f 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -1,11 +1,8 @@
-# The default target of this Makefile is...
-all::
-.PHONY: all
-
-MAK_DIR_GITWEB =
+ifndef MAK_DIR_GITWEB
+$(error do not run gitweb/Makefile stand-alone anymore. The "gitweb" and \
+"install-gitweb" targets now live in the top-level Makefile)
+endif
-# Define V=1 to have a more verbose compile.
-#
# Define JSMIN to point to JavaScript minifier that functions as
# a filter to have static/gitweb.js minified.
#
@@ -13,13 +10,6 @@ MAK_DIR_GITWEB =
# version of static/gitweb.css
#
-prefix ?= $(HOME)
-bindir ?= $(prefix)/bin
-gitwebdir ?= /var/www/cgi-bin
-
-RM ?= rm -f
-INSTALL ?= install
-
# default configuration for gitweb
GITWEB_CONFIG = gitweb_config.perl
GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf
@@ -44,71 +34,19 @@ GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
HIGHLIGHT_BIN = highlight
-# include user config
--include ../config.mak.autogen
--include ../config.mak
--include config.mak
-
-# determine version
-.PHONY: .FORCE-GIT-VERSION-FILE
-../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
- $(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
-
-ifneq ($(MAKECMDGOALS),clean)
--include ../GIT-VERSION-FILE
-endif
-
# What targets we'll add to 'all' for "make gitweb"
GITWEB_ALL =
GITWEB_ALL += gitweb.cgi
GITWEB_ALL += $(GITWEB_JS)
-### Build rules
-
-SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
-
-# Shell quote;
-bindir_SQ = $(subst ','\'',$(bindir))#'
-gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
-gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
-DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
-
-# Quiet generation (unless V=1)
-QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1 =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
- QUIET = @
- QUIET_GEN = $(QUIET)echo ' ' GEN $@;
- QUIET_SUBDIR0 = +@subdir=
- QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \
- $(MAKE) $(PRINT_DIR) -C $$subdir
- export V
- export QUIET
- export QUIET_GEN
- export QUIET_SUBDIR0
- export QUIET_SUBDIR1
-endif
-endif
-
-all:: $(GITWEB_ALL)
+MAK_DIR_GITWEB_ALL = $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_ALL))
GITWEB_PROGRAMS = gitweb.cgi
GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
GITWEB_JS = $(GITWEB_JS_MIN)
-all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)
$(QUIET_GEN)$(JSMIN) <$< >$@
@@ -118,7 +56,7 @@ GITWEB_FILES += $(GITWEB_JS)
GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
GITWEB_CSS = $(GITWEB_CSS_MIN)
-all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)
$(QUIET_GEN)$(CSSMIN) <$< >$@
@@ -185,19 +123,22 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_
### Installation rules
-.PHONY: install
-install: all
+.PHONY: install-gitweb
+install-gitweb: $(MAK_DIR_GITWEB_ALL)
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
- $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \
- '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+ $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \
'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+ifndef NO_PERL
+install: install-gitweb
+endif
### Cleaning rules
-.PHONY: clean
-clean:
+.PHONY: gitweb-clean
+gitweb-clean:
$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \
$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
GITWEB-BUILD-OPTIONS)
+clean: gitweb-clean
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v2 7/7] Makefile: build 'gitweb' in the default target
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2022-05-31 17:45 ` [PATCH v2 6/7] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
@ 2022-05-31 17:46 ` Ævar Arnfjörð Bjarmason
2022-06-06 17:44 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Junio C Hamano
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
8 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-05-31 17:46 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
From: SZEDER Gábor <szeder.dev@gmail.com>
Our Makefile's default target used to build 'gitweb', though
indirectly: the 'all' target depended on 'git-instaweb', which in turn
depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
between git-instaweb and gitweb, 2015-05-29) removed the latter
dependency, and for good reasons (quoting its commit message):
"1. git-instaweb has no build-time dependency on gitweb; it
is a run-time dependency
2. gitweb is a directory that we want to recursively make
in. As a result, its recipe is marked .PHONY, which
causes "make" to rebuild git-instaweb every time it is
run."
Since then a simple 'make' doesn't build 'gitweb'.
Luckily, installing 'gitweb' is not broken: although 'make install'
doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
install' unconditionally, which does generate all the necessary files
for 'gitweb' and installs them. However, if someone runs 'make &&
sudo make install', then those files in the 'gitweb' directory will be
generated and owned by root, which is not nice.
List 'gitweb' as a direct dependency of the default target, so a plain
'make' will build it.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index d251838c554..63057b10634 100644
--- a/Makefile
+++ b/Makefile
@@ -3042,6 +3042,7 @@ include gitweb/Makefile
.PHONY: gitweb
gitweb: $(MAK_DIR_GITWEB_ALL)
+all:: gitweb
### Installation rules
--
2.36.1.1103.g036c05811b0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2022-05-31 17:46 ` [PATCH v2 7/7] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
@ 2022-06-06 17:44 ` Junio C Hamano
2022-06-20 8:32 ` SZEDER Gábor
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
8 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2022-06-06 17:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, SZEDER Gábor, Jeff King
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> The $subject is a proposed re-roll of SZEDER's
> https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;
> As noted downthread of that fix having the Makefile invoke "make -C
> gitweb" again would slow us down on NOOP runs by quite a bit.
It would be nice to hear comments SZEDER and others, even if the
comments are clear negative or positive.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-06 17:44 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Junio C Hamano
@ 2022-06-20 8:32 ` SZEDER Gábor
2022-06-21 6:47 ` Jeff King
0 siblings, 1 reply; 31+ messages in thread
From: SZEDER Gábor @ 2022-06-20 8:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð Bjarmason, git, Jeff King
On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
> > The $subject is a proposed re-roll of SZEDER's
> > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;
> > As noted downthread of that fix having the Makefile invoke "make -C
> > gitweb" again would slow us down on NOOP runs by quite a bit.
>
> It would be nice to hear comments SZEDER and others, even if the
> comments are clear negative or positive.
Well, my itch is scratched, so I'm fine with it :)
I think Peff has a point by questioning whether we should build and
install gitweb by default... I don't have an opinion about that, but
if we do want to build it by default, then IMO doing it in the main
Makefile is the way to go, so I think in that case this patch series
goes in the right direction.
On a related note: this is not the first time when something was not
generated during a regular 'make' but only during 'sudo make install',
see e.g. 2530afd351 (Makefile: generate Git(3pm) as dependency of the
'doc' and 'man' targets, 2018-02-15). Perhaps it would be worth to
extend the CI jobs with a 'make install/install-doc' command, and
error out if it generated any files that were not present before, so
we could catch similar issues early on.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-20 8:32 ` SZEDER Gábor
@ 2022-06-21 6:47 ` Jeff King
2022-06-22 9:27 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 31+ messages in thread
From: Jeff King @ 2022-06-21 6:47 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Junio C Hamano, Ævar Arnfjörð Bjarmason, git
On Mon, Jun 20, 2022 at 10:32:02AM +0200, SZEDER Gábor wrote:
> On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:
> > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >
> > > The $subject is a proposed re-roll of SZEDER's
> > > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;
> > > As noted downthread of that fix having the Makefile invoke "make -C
> > > gitweb" again would slow us down on NOOP runs by quite a bit.
> >
> > It would be nice to hear comments SZEDER and others, even if the
> > comments are clear negative or positive.
>
> Well, my itch is scratched, so I'm fine with it :)
>
> I think Peff has a point by questioning whether we should build and
> install gitweb by default... I don't have an opinion about that, but
> if we do want to build it by default, then IMO doing it in the main
> Makefile is the way to go, so I think in that case this patch series
> goes in the right direction.
I hadn't realized the full situation when I was arguing earlier that "we
have not been building it for several years". You raised the point that
we do auto-build it in "make install", so it would be a change of
behavior to stop doing so.
I still find it hard to care too much about backwards compatibility for
building gitweb (or really gitweb at all, for that matter). But my main
complaint was foisting another recursive Makefile and its performance
and troubles on developers at large, and I think Ævar's patches deal
with it. So I'm OK with the direction.
I admit I didn't look _too_ closely at them, but they overall seemed
sensible to me. Two things I noted:
- I wondered if "make NO_PERL=1" would complain about "gitweb" being
in the default targets. It doesn't, but it does actually build
gitweb, which seems a little weird. I don't think we actually rely
on perl during the build (e.g., no "perl -c" checks or anything),
and the t950x tests seem to respect NO_PERL and avoid running the
generated file. So maybe it's OK?
- Speaking of backwards compatibility: after this series, "cd gitweb
&& make" yields an error. It's got a nice message telling you what
to do, but it's likely breaking distro scripts. Again, I'm not sure
I care, but if the point of the exercise was to avoid breaking
things, well...
-Peff
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-21 6:47 ` Jeff King
@ 2022-06-22 9:27 ` Ævar Arnfjörð Bjarmason
2022-06-22 15:37 ` Junio C Hamano
0 siblings, 1 reply; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-22 9:27 UTC (permalink / raw)
To: Jeff King; +Cc: SZEDER Gábor, Junio C Hamano, git
On Tue, Jun 21 2022, Jeff King wrote:
> On Mon, Jun 20, 2022 at 10:32:02AM +0200, SZEDER Gábor wrote:
>
>> On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:
>> > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> >
>> > > The $subject is a proposed re-roll of SZEDER's
>> > > https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com;
>> > > As noted downthread of that fix having the Makefile invoke "make -C
>> > > gitweb" again would slow us down on NOOP runs by quite a bit.
>> >
>> > It would be nice to hear comments SZEDER and others, even if the
>> > comments are clear negative or positive.
>>
>> Well, my itch is scratched, so I'm fine with it :)
>>
>> I think Peff has a point by questioning whether we should build and
>> install gitweb by default... I don't have an opinion about that, but
>> if we do want to build it by default, then IMO doing it in the main
>> Makefile is the way to go, so I think in that case this patch series
>> goes in the right direction.
>
> I hadn't realized the full situation when I was arguing earlier that "we
> have not been building it for several years". You raised the point that
> we do auto-build it in "make install", so it would be a change of
> behavior to stop doing so.
>
> I still find it hard to care too much about backwards compatibility for
> building gitweb (or really gitweb at all, for that matter). But my main
> complaint was foisting another recursive Makefile and its performance
> and troubles on developers at large, and I think Ævar's patches deal
> with it. So I'm OK with the direction.
>
> I admit I didn't look _too_ closely at them, but they overall seemed
> sensible to me. Two things I noted:
>
> - I wondered if "make NO_PERL=1" would complain about "gitweb" being
> in the default targets. It doesn't, but it does actually build
> gitweb, which seems a little weird. I don't think we actually rely
> on perl during the build (e.g., no "perl -c" checks or anything),
> and the t950x tests seem to respect NO_PERL and avoid running the
> generated file. So maybe it's OK?
I think it's arguably a bug, but as you note we build/test etc. without
errors, and I think it's restoring the state before e25c7cc146
(Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).
Arguably we should replace with a stub script like git-svn et al, and
arguably we should leave it, as you're more likely to e.g. run gitweb on
a webserver, so even if you build a "no perl" package, perhaps it's
convenient to have "gitweb" part of it, and then on that one box that
runs it you'll install perl...
> - Speaking of backwards compatibility: after this series, "cd gitweb
> && make" yields an error. It's got a nice message telling you what
> to do, but it's likely breaking distro scripts. Again, I'm not sure
> I care, but if the point of the exercise was to avoid breaking
> things, well...
I think that's OK, having maintained those sorts of build scripts in a
past life.
I.e. when you upgrade the package it's a minor hassle, and the error
tells you exactly what to do, and the fix is a 2-3 lines in your recipe
at most.
I could make gitweb/Makefile "fake it", but as argued in the patches I
think this trade-off makes more sense. Having it run in some "dual mode"
would be a maintenance hassle.
Most of the reason for keeping gitweb/Makefile around (as opposed to the
top-level Makefile absorbing it) was to be able to emit that message to
be friendly to downstream packagers.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-22 9:27 ` Ævar Arnfjörð Bjarmason
@ 2022-06-22 15:37 ` Junio C Hamano
2022-06-23 10:29 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2022-06-22 15:37 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, SZEDER Gábor, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> - I wondered if "make NO_PERL=1" would complain about "gitweb" being
>> in the default targets. It doesn't, but it does actually build
>> gitweb, which seems a little weird. I don't think we actually rely
>> on perl during the build (e.g., no "perl -c" checks or anything),
>> and the t950x tests seem to respect NO_PERL and avoid running the
>> generated file. So maybe it's OK?
>
> I think it's arguably a bug, but as you note we build/test etc. without
> errors, and I think it's restoring the state before e25c7cc146
> (Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).
>
> Arguably we should replace with a stub script like git-svn et al, and
> arguably we should leave it, as you're more likely to e.g. run gitweb on
> a webserver, so even if you build a "no perl" package, perhaps it's
> convenient to have "gitweb" part of it, and then on that one box that
> runs it you'll install perl...
It is perfectly acceptable to "make DESTDIR=... install" and tar up
the result on a host with NO_PERL and extract it on the target that
is capable to run gitweb, isn't it? As long as "make NO_PERL=1"
gives exactly the gitweb as a build without NO_PERL, that should be
OK, I would think. I would think what you have is in a good state.
>> - Speaking of backwards compatibility: after this series, "cd gitweb
>> && make" yields an error. It's got a nice message telling you what
>> to do, but it's likely breaking distro scripts. Again, I'm not sure
>> I care, but if the point of the exercise was to avoid breaking
>> things, well...
>
> I think that's OK, having maintained those sorts of build scripts in a
> past life.
>
> I.e. when you upgrade the package it's a minor hassle, and the error
> tells you exactly what to do, and the fix is a 2-3 lines in your recipe
> at most.
We could easily add "cd .. && make gitweb" to gitweb/Makefile with
the same "minor hassle" but that needs to be done just once, instead
of having to be done once per packager, so I am not sure the above
argues for a good tradeoff.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-22 15:37 ` Junio C Hamano
@ 2022-06-23 10:29 ` Ævar Arnfjörð Bjarmason
2022-06-23 23:25 ` Junio C Hamano
0 siblings, 1 reply; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-23 10:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, SZEDER Gábor, git
On Wed, Jun 22 2022, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> - I wondered if "make NO_PERL=1" would complain about "gitweb" being
>>> in the default targets. It doesn't, but it does actually build
>>> gitweb, which seems a little weird. I don't think we actually rely
>>> on perl during the build (e.g., no "perl -c" checks or anything),
>>> and the t950x tests seem to respect NO_PERL and avoid running the
>>> generated file. So maybe it's OK?
>>
>> I think it's arguably a bug, but as you note we build/test etc. without
>> errors, and I think it's restoring the state before e25c7cc146
>> (Makefile: drop dependency between git-instaweb and gitweb, 2015-05-29).
>>
>> Arguably we should replace with a stub script like git-svn et al, and
>> arguably we should leave it, as you're more likely to e.g. run gitweb on
>> a webserver, so even if you build a "no perl" package, perhaps it's
>> convenient to have "gitweb" part of it, and then on that one box that
>> runs it you'll install perl...
>
> It is perfectly acceptable to "make DESTDIR=... install" and tar up
> the result on a host with NO_PERL and extract it on the target that
> is capable to run gitweb, isn't it? As long as "make NO_PERL=1"
> gives exactly the gitweb as a build without NO_PERL, that should be
> OK, I would think. I would think what you have is in a good state.
I think so, but it is inconsistent with how we treat the rest of the
*.perl scripts, which I think is what Jeff is correctly pointing
out.
I.e. if you do this your git-send-email, git-cvsserver, git-svn
etc. will be replaced by this shellscript:
#!/bin/sh
echo >&2 "fatal: git was built without support for $(basename $0) (NO_PERL=Y)."
exit 128
>>> - Speaking of backwards compatibility: after this series, "cd gitweb
>>> && make" yields an error. It's got a nice message telling you what
>>> to do, but it's likely breaking distro scripts. Again, I'm not sure
>>> I care, but if the point of the exercise was to avoid breaking
>>> things, well...
>>
>> I think that's OK, having maintained those sorts of build scripts in a
>> past life.
>>
>> I.e. when you upgrade the package it's a minor hassle, and the error
>> tells you exactly what to do, and the fix is a 2-3 lines in your recipe
>> at most.
>
> We could easily add "cd .. && make gitweb" to gitweb/Makefile with
> the same "minor hassle" but that needs to be done just once, instead
> of having to be done once per packager, so I am not sure the above
> argues for a good tradeoff.
True, but I think critically in this case we've never documented that
you should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL
has always documented and assumed that you run these from the top-level.
There's no "make gitweb" target in the sub-Makefile, and the
instructions make it clear that it's the top-level, as we also suggest:
make configure &&
[... no cd-ing to gitweb/ ...] &&
make gitweb
(and there's no "configure" there either).
So I think this will Just Work for anyone who's packaging this already,
I was just going above & beyond in being friendly by emitting that new
message saying you should be running "make" at the top-level in case
anyone built this in a way that we never documented, but which happened
to work before.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-23 10:29 ` Ævar Arnfjörð Bjarmason
@ 2022-06-23 23:25 ` Junio C Hamano
2022-06-23 23:45 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 31+ messages in thread
From: Junio C Hamano @ 2022-06-23 23:25 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, SZEDER Gábor, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> We could easily add "cd .. && make gitweb" to gitweb/Makefile with
>> the same "minor hassle" but that needs to be done just once, instead
>> of having to be done once per packager, so I am not sure the above
>> argues for a good tradeoff.
>
> True, but I think critically in this case we've never documented that
> you should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL
> has always documented and assumed that you run these from the top-level.
Well, I do not think Makefiles document much of their targets in
general. If its first/default target has a reasonable name, like
"all", people expect "cd there && make all" would do the right
thing.
So I do not think "we never documented" is a good excuse. What the
current users have been doing and are expecting to keep working is
what counts. If they are used to see "cd gitweb && make" working,
perhaps instead of giving an unfriendly $(error do not run) at the
beginning of gitweb/Makefile that is designed to trigger only when
they did that (instead of running 'make gitweb' from the top), it
would be trivial to have the rule to "cd .. && $(MAKE) gitweb"
there, no?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-23 23:25 ` Junio C Hamano
@ 2022-06-23 23:45 ` Ævar Arnfjörð Bjarmason
2022-06-24 1:14 ` Junio C Hamano
0 siblings, 1 reply; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-23 23:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, SZEDER Gábor, git
On Thu, Jun 23 2022, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> We could easily add "cd .. && make gitweb" to gitweb/Makefile with
>>> the same "minor hassle" but that needs to be done just once, instead
>>> of having to be done once per packager, so I am not sure the above
>>> argues for a good tradeoff.
>>
>> True, but I think critically in this case we've never documented that
>> you should be running gitweb/Makefile directly. I.e. the gitweb/INSTALL
>> has always documented and assumed that you run these from the top-level.
>
> Well, I do not think Makefiles document much of their targets in
> general. If its first/default target has a reasonable name, like
> "all", people expect "cd there && make all" would do the right
> thing.
>
> So I do not think "we never documented" is a good excuse. What the
> current users have been doing and are expecting to keep working is
> what counts. If they are used to see "cd gitweb && make" working,
> perhaps instead of giving an unfriendly $(error do not run) at the
> beginning of gitweb/Makefile that is designed to trigger only when
> they did that (instead of running 'make gitweb' from the top), it
> would be trivial to have the rule to "cd .. && $(MAKE) gitweb"
> there, no?
I can re-roll it with that change if you insist. It would close the door
on further unifying the two Makefiles in the future (well, we could keep
the wrapper in place).
I have a script I use to see how big the impact of this sort of thing
would be in practice, i.e. I download downstream package recipies, which
are found at (name, relative path & urls). I also manually get the AIX
package:
freebsd-ports devel/git https://github.com/freebsd/freebsd-ports.git
openbsd-ports devel/git https://github.com/openbsd/ports.git
netbsd-pkgsrc devel/git-base https://github.com/NetBSD/pkgsrc.git
dragonflybsd-dports devel/git https://github.com/DragonFlyBSD/DPorts.git
fedora . https://src.fedoraproject.org/rpms/git
debian debian https://repo.or.cz/git/debian.git
gentoo dev-vcs/git https://github.com/gentoo/gentoo.git
arch git/trunk https://github.com/archlinux/svntogit-packages.git
nix pkgs/applications/version-management/git-and-tools/git https://github.com/NixOS/nixpkgs.git
alpine main/git https://github.com/alpinelinux/aports.git https://git.alpinelinux.org/aports
git_osx_installer . https://github.com/timcharper/git_osx_installer.git
homebrew-core Formula/git.rb https://github.com/Homebrew/homebrew-core.git
macports-ports devel/git https://github.com/macports/macports-ports.git
Looking through all of those none of them do anything with
gitweb/Makefile. I.e. all "make gitweb" at the top-level, or simply rely
on "make install" to install it.
FreeBSD and NetBSD are monkeypatching our Makefile to emulate a "I don't
want gitweb please!", which they'll need to do before/after this series
(but we could helpfully provide them a config knob).
Anyway, if you want "make gitweb" and "make gitweb-install" in the
subdirectory to work I can patch it, but per the above I think it would
be useful to pretty much nobody.
I could use around the same amount of effort to give FreeBSD and NetBSD
a "don't install gitweb please" know instead, what do you think?:)
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-06-23 23:45 ` Ævar Arnfjörð Bjarmason
@ 2022-06-24 1:14 ` Junio C Hamano
0 siblings, 0 replies; 31+ messages in thread
From: Junio C Hamano @ 2022-06-24 1:14 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, SZEDER Gábor, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> would be in practice, i.e. I download downstream package recipies, which
> are found at (name, relative path & urls). I also manually get the AIX
> package:
> ...
> macports-ports devel/git https://github.com/macports/macports-ports.git
>
> Looking through all of those none of them do anything with
> gitweb/Makefile. I.e. all "make gitweb" at the top-level, or simply rely
> on "make install" to install it.
Now we are talking. Giving that as a datapoint upfront in the
proposed log message would have saved needless round-trip, I would
think.
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 0/8] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
` (7 preceding siblings ...)
2022-06-06 17:44 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Junio C Hamano
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
` (7 more replies)
8 siblings, 8 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
The $subject is a mostly trivial re-roll of
https://lore.kernel.org/git/cover-v2-0.7-00000000000-20220531T173805Z-avarab@gmail.com/.
Changes since v3:
* Correct the commit message of 7/8 to account for SZEDER's commit
being included in this (that part was also incorrect in v2).
* As noted in
https://lore.kernel.org/git/220624.86bkuikidi.gmgdl@evledraar.gmail.com/
I went over various git packages in the wild to see if my changes
here would break things
I did find that not having a wrapper gitweb/Makefile would break
OpenBSD's package, but it's a one-line fix.
But to make up for it I saw that a semi-common pattern was to
manually munge our Makefile to get rid of "gitweb" or
"gitweb-install" targets. All of {Free,Net,Dragonfly}BSD were doing
that. They'll now happily be able to use a NO_GITWEB=Y flag
instead, so hopefully the small amount of disruption here makes up
for itself.
SZEDER Gábor (1):
Makefile: build 'gitweb' in the default target
Ævar Arnfjörð Bjarmason (7):
gitweb/Makefile: define all .PHONY prerequisites inline
gitweb/Makefile: add a $(GITWEB_ALL) variable
gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
gitweb/Makefile: prepare to merge into top-level Makefile
gitweb: remove "test" and "test-installed" targets
gitweb/Makefile: include in top-level Makefile
gitweb/Makefile: add a "NO_GITWEB" parameter
Makefile | 31 +++++++----
gitweb/Makefile | 145 ++++++++++++++++--------------------------------
t/Makefile | 4 --
3 files changed, 68 insertions(+), 112 deletions(-)
Range-diff against v2:
1: 14361617ca6 = 1: 8e85151cf3d gitweb/Makefile: define all .PHONY prerequisites inline
2: 7d920a13518 = 2: 5c9895949aa gitweb/Makefile: add a $(GITWEB_ALL) variable
3: e14a5b73061 = 3: 2f4db54923d gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
4: 02e26ca8ce2 = 4: d38b553a2e6 gitweb/Makefile: prepare to merge into top-level Makefile
5: caf376f3dd9 = 5: 6c2d7b30524 gitweb: remove "test" and "test-installed" targets
6: b423cd58f6b = 6: 5640587b9ae gitweb/Makefile: include in top-level Makefile
7: 69428540886 ! 7: 571c9c10319 Makefile: build 'gitweb' in the default target
@@ Commit message
Since then a simple 'make' doesn't build 'gitweb'.
Luckily, installing 'gitweb' is not broken: although 'make install'
- doesn't depend on the 'gitweb' target, it runs 'make -C gitweb
- install' unconditionally, which does generate all the necessary files
+ doesn't depend on the 'gitweb' target, it has a dependency on the
+ 'install-gitweb' target, which does generate all the necessary files
for 'gitweb' and installs them. However, if someone runs 'make &&
sudo make install', then those files in the 'gitweb' directory will be
generated and owned by root, which is not nice.
-: ----------- > 8: 0c8f26ee876 gitweb/Makefile: add a "NO_GITWEB" parameter
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 2/8] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Move the '.PHONY' definition so that it's split up and accompanies the
relevant as they're defined. This will make a subsequent diff smaller
as we'll remove some of these, and won't need to re-edit the
now-removed '.PHONY' line.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index f13e23c4de4..abb5c9f9ab6 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -1,5 +1,6 @@
# The default target of this Makefile is...
all::
+.PHONY: all
# Define V=1 to have a more verbose compile.
#
@@ -45,6 +46,7 @@ HIGHLIGHT_BIN = highlight
-include config.mak
# determine version
+.PHONY: .FORCE-GIT-VERSION-FILE
../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
@@ -152,6 +154,7 @@ GITWEB_REPLACE = \
-e 's|++GITWEB_SITE_FOOTER++|$(GITWEB_SITE_FOOTER)|g' \
-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
+.PHONY: FORCE
GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
@@ -171,15 +174,18 @@ static/gitweb.js: $(GITWEB_JSLIB_FILES)
### Testing rules
+.PHONY: test
test:
$(MAKE) -C ../t gitweb-test
+.PHONY: test-installed
test-installed:
GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
$(MAKE) -C ../t gitweb-test
### Installation rules
+.PHONY: install
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
@@ -188,10 +194,8 @@ install: all
### Cleaning rules
+.PHONY: clean
clean:
$(RM) gitweb.cgi static/gitweb.js \
static/gitweb.min.js static/gitweb.min.css \
GITWEB-BUILD-OPTIONS
-
-.PHONY: all clean install test test-installed .FORCE-GIT-VERSION-FILE FORCE
-
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 2/8] gitweb/Makefile: add a $(GITWEB_ALL) variable
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 3/8] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Declare the targets that the "all" target depends on with a new
$(GITWEB_ALL) variable. This will help to reduce churn in subsequent
commits.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index abb5c9f9ab6..733b60f925e 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -54,6 +54,11 @@ ifneq ($(MAKECMDGOALS),clean)
-include ../GIT-VERSION-FILE
endif
+# What targets we'll add to 'all' for "make gitweb"
+GITWEB_ALL =
+GITWEB_ALL += gitweb.cgi
+GITWEB_ALL += static/gitweb.js
+
### Build rules
SHELL_PATH ?= $(SHELL)
@@ -92,7 +97,7 @@ ifndef V
endif
endif
-all:: gitweb.cgi static/gitweb.js
+all:: $(GITWEB_ALL)
GITWEB_PROGRAMS = gitweb.cgi
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 3/8] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 2/8] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 4/8] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Change the variable definitions for the $(GITWEB_CSS) and $(GITWEB_JS)
so that we have a clear separation between what we use as "in" files,
v.s. our "min" files. We can now make the appending to $(GITWEB_FILES)
unconditional, since $(GITWEB_{JS,CSS}) is either the "min" or
non-"min" version. This reduces the duplication within the file.
While we're at it let's initialize "GITWEB_JSLIB_FILES" as we normally
do with such variables.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 733b60f925e..a8752c1f11c 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -31,10 +31,12 @@ GITWEB_STRICT_EXPORT =
GITWEB_BASE_URL =
GITWEB_LIST =
GITWEB_HOMETEXT = indextext.html
-GITWEB_CSS = static/gitweb.css
+GITWEB_CSS_IN = static/gitweb.css
+GITWEB_CSS = $(GITWEB_CSS_IN)
GITWEB_LOGO = static/git-logo.png
GITWEB_FAVICON = static/git-favicon.png
-GITWEB_JS = static/gitweb.js
+GITWEB_JS_IN = static/gitweb.js
+GITWEB_JS = $(GITWEB_JS_IN)
GITWEB_SITE_HTML_HEAD_STRING =
GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
@@ -57,7 +59,7 @@ endif
# What targets we'll add to 'all' for "make gitweb"
GITWEB_ALL =
GITWEB_ALL += gitweb.cgi
-GITWEB_ALL += static/gitweb.js
+GITWEB_ALL += $(GITWEB_JS)
### Build rules
@@ -101,25 +103,23 @@ all:: $(GITWEB_ALL)
GITWEB_PROGRAMS = gitweb.cgi
+GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
-GITWEB_FILES += static/gitweb.min.js
-GITWEB_JS = static/gitweb.min.js
-all:: static/gitweb.min.js
-static/gitweb.min.js: static/gitweb.js GITWEB-BUILD-OPTIONS
+GITWEB_JS = $(GITWEB_JS_MIN)
+all:: $(GITWEB_JS_MIN)
+$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS
$(QUIET_GEN)$(JSMIN) <$< >$@
-else
-GITWEB_FILES += static/gitweb.js
endif
+GITWEB_FILES += $(GITWEB_JS)
+GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
-GITWEB_FILES += static/gitweb.min.css
-GITWEB_CSS = static/gitweb.min.css
-all:: static/gitweb.min.css
-static/gitweb.min.css: static/gitweb.css GITWEB-BUILD-OPTIONS
+GITWEB_CSS = $(GITWEB_CSS_MIN)
+all:: $(GITWEB_CSS_MIN)
+$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS
$(QUIET_GEN)$(CSSMIN) <$< >$@
-else
-GITWEB_FILES += static/gitweb.css
endif
+GITWEB_FILES += $(GITWEB_CSS)
GITWEB_FILES += static/git-logo.png static/git-favicon.png
@@ -127,6 +127,7 @@ GITWEB_FILES += static/git-logo.png static/git-favicon.png
#
# js/lib/common-lib.js should be always first, then js/lib/*.js,
# then the rest of files; js/gitweb.js should be last (if it exists)
+GITWEB_JSLIB_FILES =
GITWEB_JSLIB_FILES += static/js/lib/common-lib.js
GITWEB_JSLIB_FILES += static/js/lib/datetime.js
GITWEB_JSLIB_FILES += static/js/lib/cookies.js
@@ -201,6 +202,6 @@ install: all
.PHONY: clean
clean:
- $(RM) gitweb.cgi static/gitweb.js \
- static/gitweb.min.js static/gitweb.min.css \
+ $(RM) gitweb.cgi $(GITWEB_JS_IN) \
+ $(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
GITWEB-BUILD-OPTIONS
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 4/8] gitweb/Makefile: prepare to merge into top-level Makefile
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2022-06-28 10:15 ` [PATCH v3 3/8] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 5/8] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Since the "gitweb/Makefile" was split out from the top-level Makefile
in 62331ef1637 (gitweb: Makefile improvements, 2010-01-30) we've kept
the inter-dependencies between the two, and worse have dealt with a
lot of duplication as a result.
In preparation for merging the two again add a MAK_DIR_GITWEB variable
to various rules in it. This will allow us to set this variable to
"gitweb/" as we include it in the top-level Makefile, which will
minimize the size of the subsequent diff.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index a8752c1f11c..74e896767e9 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -2,6 +2,8 @@
all::
.PHONY: all
+MAK_DIR_GITWEB =
+
# Define V=1 to have a more verbose compile.
#
# Define JSMIN to point to JavaScript minifier that functions as
@@ -106,8 +108,9 @@ GITWEB_PROGRAMS = gitweb.cgi
GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
GITWEB_JS = $(GITWEB_JS_MIN)
-all:: $(GITWEB_JS_MIN)
-$(GITWEB_JS_MIN): $(GITWEB_JS_IN) GITWEB-BUILD-OPTIONS
+all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)
$(QUIET_GEN)$(JSMIN) <$< >$@
endif
GITWEB_FILES += $(GITWEB_JS)
@@ -115,8 +118,9 @@ GITWEB_FILES += $(GITWEB_JS)
GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
GITWEB_CSS = $(GITWEB_CSS_MIN)
-all:: $(GITWEB_CSS_MIN)
-$(GITWEB_CSS_MIN): $(GITWEB_CSS_IN) GITWEB-BUILD-OPTIONS
+all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)
$(QUIET_GEN)$(CSSMIN) <$< >$@
endif
GITWEB_FILES += $(GITWEB_CSS)
@@ -161,19 +165,20 @@ GITWEB_REPLACE = \
-e 's|++HIGHLIGHT_BIN++|$(HIGHLIGHT_BIN)|g'
.PHONY: FORCE
-GITWEB-BUILD-OPTIONS: FORCE
+$(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS: FORCE
@rm -f $@+
@echo "x" '$(PERL_PATH_SQ)' $(GITWEB_REPLACE) "$(JSMIN)|$(CSSMIN)" >$@+
@cmp -s $@+ $@ && rm -f $@+ || mv -f $@+ $@
-gitweb.cgi: gitweb.perl GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
+$(MAK_DIR_GITWEB)gitweb.cgi: $(MAK_DIR_GITWEB)gitweb.perl
$(QUIET_GEN)$(RM) $@ $@+ && \
sed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \
$(GITWEB_REPLACE) $< >$@+ && \
chmod +x $@+ && \
mv $@+ $@
-static/gitweb.js: $(GITWEB_JSLIB_FILES)
+$(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_FILES))
$(QUIET_GEN)$(RM) $@ $@+ && \
cat $^ >$@+ && \
mv $@+ $@
@@ -194,14 +199,16 @@ test-installed:
.PHONY: install
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
- $(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+ $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \
+ '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
- $(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+ $(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \
+ '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
### Cleaning rules
.PHONY: clean
clean:
- $(RM) gitweb.cgi $(GITWEB_JS_IN) \
+ $(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \
$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
- GITWEB-BUILD-OPTIONS
+ GITWEB-BUILD-OPTIONS)
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 5/8] gitweb: remove "test" and "test-installed" targets
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2022-06-28 10:15 ` [PATCH v3 4/8] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:15 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 6/8] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:15 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Remove the special "test" targets for gitweb added in
958a8467216 (gitweb/Makefile: Add 'test' and 'test-installed' targets,
2010-09-26). Unlike e.g. "contrib/scalar" and "contrib/subtree" the
"gitweb" tests themselves live in our top-level t/ directory.
It therefore doesn't make sense to maintain this indirection, no more
than it would to have a "git-send-email-test". By dropping it we'll
also free other tests to use the t95*.sh prefix.
These removed targets are unlikely to be used by anyone, and to the
extent that they are we can easily use an invocation like this
instead:
make test T='t[0-9]*gitweb*.sh'
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
gitweb/Makefile | 11 -----------
t/Makefile | 4 ----
2 files changed, 15 deletions(-)
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 74e896767e9..eaf0cfcf80e 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -183,17 +183,6 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_
cat $^ >$@+ && \
mv $@+ $@
-### Testing rules
-
-.PHONY: test
-test:
- $(MAKE) -C ../t gitweb-test
-
-.PHONY: test-installed
-test-installed:
- GITWEB_TEST_INSTALLED='$(DESTDIR_SQ)$(gitwebdir_SQ)' \
- $(MAKE) -C ../t gitweb-test
-
### Installation rules
.PHONY: install
diff --git a/t/Makefile b/t/Makefile
index 056ce55dcc9..7f56e52f767 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -35,7 +35,6 @@ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
-TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))
THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
@@ -112,9 +111,6 @@ aggregate-results:
echo "$$f"; \
done | '$(SHELL_PATH_SQ)' ./aggregate-results.sh
-gitweb-test:
- $(MAKE) $(TGITWEB)
-
valgrind:
$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 6/8] gitweb/Makefile: include in top-level Makefile
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2022-06-28 10:15 ` [PATCH v3 5/8] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:16 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 7/8] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 8/8] gitweb/Makefile: add a "NO_GITWEB" parameter Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
Include the gitweb/Makefile in the top-level Makefile rather than
calling it as a sub-Makefile. As noted in the thread starting at at
[1] (in particular [2]) we'll pay a high cost on NOOP runs of "make"
just to figure out that we have nothing to do for "make gitweb".
The "gitweb" script also isn't maintained out-of-tree, unlike
"gitk-git" or "git-gui", which both have their own "Makefile". Other
parts of it are already integrated into our main Makefiles, e.g. the
documentation is built by Documentation/Makefile since
07ea4df2780 (gitweb: Add gitweb(1) manpage for gitweb itself,
2011-10-16).
1. https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com/
2. https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 23 +++++++------
gitweb/Makefile | 91 +++++++++----------------------------------------
2 files changed, 29 insertions(+), 85 deletions(-)
diff --git a/Makefile b/Makefile
index 04d0fd1fe60..a5f29c11681 100644
--- a/Makefile
+++ b/Makefile
@@ -544,6 +544,7 @@ gitexecdir = libexec/git-core
mergetoolsdir = $(gitexecdir)/mergetools
sharedir = $(prefix)/share
gitwebdir = $(sharedir)/gitweb
+gitwebstaticdir = $(gitwebdir)/static
perllibdir = $(sharedir)/perl5
localedir = $(sharedir)/locale
template_dir = share/git-core/templates
@@ -562,7 +563,7 @@ localedir_relative = $(patsubst $(prefix)/%,%,$(localedir))
htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))
-export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir
+export prefix bindir sharedir sysconfdir perllibdir localedir
# Set our default programs
CC = cc
@@ -2089,6 +2090,7 @@ htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
prefix_SQ = $(subst ','\'',$(prefix))
perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative))
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
+gitwebstaticdir_SQ = $(subst ','\'',$(gitwebstaticdir))
SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
@@ -2417,10 +2419,6 @@ GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
perllibdir:
@echo '$(perllibdir_SQ)'
-.PHONY: gitweb
-gitweb:
- $(QUIET_SUBDIR0)gitweb $(QUIET_SUBDIR1) all
-
git-instaweb: git-instaweb.sh GIT-SCRIPT-DEFINES
$(QUIET_GEN)$(cmd_munge_script) && \
chmod +x $@+ && \
@@ -3149,6 +3147,15 @@ coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.c
.PHONY: coccicheck coccicheck-pending
+# "Sub"-Makefiles, not really because they can't be run stand-alone,
+# only there to contain directory-specific rules and variables
+## gitweb/Makefile inclusion:
+MAK_DIR_GITWEB = gitweb/
+include gitweb/Makefile
+
+.PHONY: gitweb
+gitweb: $(MAK_DIR_GITWEB_ALL)
+
### Installation rules
ifneq ($(filter /%,$(firstword $(template_dir))),)
@@ -3221,7 +3228,6 @@ ifndef NO_PERL
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
(cd perl/build/lib && $(TAR) cf - .) | \
(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
- $(MAKE) -C gitweb install
endif
ifndef NO_TCLTK
$(MAKE) -C gitk-git install
@@ -3276,10 +3282,8 @@ endif
cp "$$execdir/git-remote-http$X" "$$execdir/$$p" || exit; } \
done
-.PHONY: install-gitweb install-doc install-man install-man-perl install-html install-info install-pdf
+.PHONY: install-doc install-man install-man-perl install-html install-info install-pdf
.PHONY: quick-install-doc quick-install-man quick-install-html
-install-gitweb:
- $(MAKE) -C gitweb install
install-doc: install-man-perl
$(MAKE) -C Documentation install
@@ -3425,7 +3429,6 @@ clean: profile-clean coverage-clean cocciclean
$(MAKE) -C Documentation/ clean
$(RM) Documentation/GIT-EXCLUDED-PROGRAMS
ifndef NO_PERL
- $(MAKE) -C gitweb clean
$(RM) -r perl/build/
endif
$(MAKE) -C templates/ clean
diff --git a/gitweb/Makefile b/gitweb/Makefile
index eaf0cfcf80e..66fceb9e94f 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -1,11 +1,8 @@
-# The default target of this Makefile is...
-all::
-.PHONY: all
-
-MAK_DIR_GITWEB =
+ifndef MAK_DIR_GITWEB
+$(error do not run gitweb/Makefile stand-alone anymore. The "gitweb" and \
+"install-gitweb" targets now live in the top-level Makefile)
+endif
-# Define V=1 to have a more verbose compile.
-#
# Define JSMIN to point to JavaScript minifier that functions as
# a filter to have static/gitweb.js minified.
#
@@ -13,13 +10,6 @@ MAK_DIR_GITWEB =
# version of static/gitweb.css
#
-prefix ?= $(HOME)
-bindir ?= $(prefix)/bin
-gitwebdir ?= /var/www/cgi-bin
-
-RM ?= rm -f
-INSTALL ?= install
-
# default configuration for gitweb
GITWEB_CONFIG = gitweb_config.perl
GITWEB_CONFIG_SYSTEM = /etc/gitweb.conf
@@ -44,71 +34,19 @@ GITWEB_SITE_HEADER =
GITWEB_SITE_FOOTER =
HIGHLIGHT_BIN = highlight
-# include user config
--include ../config.mak.autogen
--include ../config.mak
--include config.mak
-
-# determine version
-.PHONY: .FORCE-GIT-VERSION-FILE
-../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
- $(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
-
-ifneq ($(MAKECMDGOALS),clean)
--include ../GIT-VERSION-FILE
-endif
-
# What targets we'll add to 'all' for "make gitweb"
GITWEB_ALL =
GITWEB_ALL += gitweb.cgi
GITWEB_ALL += $(GITWEB_JS)
-### Build rules
-
-SHELL_PATH ?= $(SHELL)
-PERL_PATH ?= /usr/bin/perl
-
-# Shell quote;
-bindir_SQ = $(subst ','\'',$(bindir))#'
-gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
-gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))#'
-PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))#'
-DESTDIR_SQ = $(subst ','\'',$(DESTDIR))#'
-
-# Quiet generation (unless V=1)
-QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir
-QUIET_SUBDIR1 =
-
-ifneq ($(findstring $(MAKEFLAGS),w),w)
-PRINT_DIR = --no-print-directory
-else # "make -w"
-NO_SUBDIR = :
-endif
-
-ifneq ($(findstring $(MAKEFLAGS),s),s)
-ifndef V
- QUIET = @
- QUIET_GEN = $(QUIET)echo ' ' GEN $@;
- QUIET_SUBDIR0 = +@subdir=
- QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \
- $(MAKE) $(PRINT_DIR) -C $$subdir
- export V
- export QUIET
- export QUIET_GEN
- export QUIET_SUBDIR0
- export QUIET_SUBDIR1
-endif
-endif
-
-all:: $(GITWEB_ALL)
+MAK_DIR_GITWEB_ALL = $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_ALL))
GITWEB_PROGRAMS = gitweb.cgi
GITWEB_JS_MIN = static/gitweb.min.js
ifdef JSMIN
GITWEB_JS = $(GITWEB_JS_MIN)
-all:: $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_JS_MIN)
$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
$(MAK_DIR_GITWEB)$(GITWEB_JS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_JS_IN)
$(QUIET_GEN)$(JSMIN) <$< >$@
@@ -118,7 +56,7 @@ GITWEB_FILES += $(GITWEB_JS)
GITWEB_CSS_MIN = static/gitweb.min.css
ifdef CSSMIN
GITWEB_CSS = $(GITWEB_CSS_MIN)
-all:: $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
+GITWEB_ALL += $(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN)
$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)GITWEB-BUILD-OPTIONS
$(MAK_DIR_GITWEB)$(GITWEB_CSS_MIN): $(MAK_DIR_GITWEB)$(GITWEB_CSS_IN)
$(QUIET_GEN)$(CSSMIN) <$< >$@
@@ -185,19 +123,22 @@ $(MAK_DIR_GITWEB)static/gitweb.js: $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_JSLIB_
### Installation rules
-.PHONY: install
-install: all
+.PHONY: install-gitweb
+install-gitweb: $(MAK_DIR_GITWEB_ALL)
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebdir_SQ)'
- $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) \
- '$(DESTDIR_SQ)$(gitwebdir_SQ)'
+ $(INSTALL) -m 755 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_PROGRAMS)) '$(DESTDIR_SQ)$(gitwebdir_SQ)'
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \
'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+ifndef NO_PERL
+install: install-gitweb
+endif
### Cleaning rules
-.PHONY: clean
-clean:
+.PHONY: gitweb-clean
+gitweb-clean:
$(RM) $(addprefix $(MAK_DIR_GITWEB),gitweb.cgi $(GITWEB_JS_IN) \
$(GITWEB_JS_MIN) $(GITWEB_CSS_MIN) \
GITWEB-BUILD-OPTIONS)
+clean: gitweb-clean
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 7/8] Makefile: build 'gitweb' in the default target
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2022-06-28 10:16 ` [PATCH v3 6/8] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:16 ` Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 8/8] gitweb/Makefile: add a "NO_GITWEB" parameter Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
From: SZEDER Gábor <szeder.dev@gmail.com>
Our Makefile's default target used to build 'gitweb', though
indirectly: the 'all' target depended on 'git-instaweb', which in turn
depended on 'gitweb'. Then e25c7cc146 (Makefile: drop dependency
between git-instaweb and gitweb, 2015-05-29) removed the latter
dependency, and for good reasons (quoting its commit message):
"1. git-instaweb has no build-time dependency on gitweb; it
is a run-time dependency
2. gitweb is a directory that we want to recursively make
in. As a result, its recipe is marked .PHONY, which
causes "make" to rebuild git-instaweb every time it is
run."
Since then a simple 'make' doesn't build 'gitweb'.
Luckily, installing 'gitweb' is not broken: although 'make install'
doesn't depend on the 'gitweb' target, it has a dependency on the
'install-gitweb' target, which does generate all the necessary files
for 'gitweb' and installs them. However, if someone runs 'make &&
sudo make install', then those files in the 'gitweb' directory will be
generated and owned by root, which is not nice.
List 'gitweb' as a direct dependency of the default target, so a plain
'make' will build it.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index a5f29c11681..d150be4dc32 100644
--- a/Makefile
+++ b/Makefile
@@ -3155,6 +3155,7 @@ include gitweb/Makefile
.PHONY: gitweb
gitweb: $(MAK_DIR_GITWEB_ALL)
+all:: gitweb
### Installation rules
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 8/8] gitweb/Makefile: add a "NO_GITWEB" parameter
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2022-06-28 10:16 ` [PATCH v3 7/8] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
@ 2022-06-28 10:16 ` Ævar Arnfjörð Bjarmason
7 siblings, 0 replies; 31+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-06-28 10:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, SZEDER Gábor, Jeff King,
Ævar Arnfjörð Bjarmason
From looking at the {Free,Net,Dragonfly}BSD packages for git[1]
they've been monkeypatching "gitweb" out of the Makefile, let's be
nicer and provide a NO_GITWEB=Y for their use.
For the "all" target this allows for optionally restoring what's been
the status quo before the preceding commit, but now we'll also behave
correctly on the subsequent "make install".
As before our installation of gitweb can be suppressed with
NO_PERL. For backwards compatibility the NO_PERL=Y flag by itself
still doesn't change whether or not we build gitweb, unlike the new
NO_GITWEB=Y flag.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Makefile | 7 +++++++
gitweb/Makefile | 2 ++
2 files changed, 9 insertions(+)
diff --git a/Makefile b/Makefile
index d150be4dc32..92bea603394 100644
--- a/Makefile
+++ b/Makefile
@@ -309,6 +309,11 @@ include shared.mak
# distributions that want to use their packaged versions of Perl
# modules, instead of the fallbacks shipped with Git.
#
+# Define NO_GITWEB if you do not want to build or install
+# 'gitweb'. Note that defining NO_PERL currently has the same effect
+# on not installing gitweb, but not on whether it's built in the
+# gitweb/ directory.
+#
# Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
# but /usr/bin/python2.7 or /usr/bin/python3 on some platforms).
#
@@ -3155,7 +3160,9 @@ include gitweb/Makefile
.PHONY: gitweb
gitweb: $(MAK_DIR_GITWEB_ALL)
+ifndef NO_GITWEB
all:: gitweb
+endif
### Installation rules
diff --git a/gitweb/Makefile b/gitweb/Makefile
index 66fceb9e94f..3b68ab2d672 100644
--- a/gitweb/Makefile
+++ b/gitweb/Makefile
@@ -130,9 +130,11 @@ install-gitweb: $(MAK_DIR_GITWEB_ALL)
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
$(INSTALL) -m 644 $(addprefix $(MAK_DIR_GITWEB),$(GITWEB_FILES)) \
'$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'
+ifndef NO_GITWEB
ifndef NO_PERL
install: install-gitweb
endif
+endif
### Cleaning rules
--
2.37.0.880.gf07d56b18ba
^ permalink raw reply related [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-06-28 10:19 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25 20:56 [PATCH] Makefile: build 'gitweb' in the default target SZEDER Gábor
2022-05-26 0:14 ` Ævar Arnfjörð Bjarmason
2022-05-26 7:57 ` Jeff King
2022-05-26 21:33 ` SZEDER Gábor
2022-05-27 9:23 ` Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 1/7] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 2/7] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 3/7] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 4/7] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 5/7] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
2022-05-31 17:45 ` [PATCH v2 6/7] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
2022-05-31 17:46 ` [PATCH v2 7/7] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
2022-06-06 17:44 ` [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns Junio C Hamano
2022-06-20 8:32 ` SZEDER Gábor
2022-06-21 6:47 ` Jeff King
2022-06-22 9:27 ` Ævar Arnfjörð Bjarmason
2022-06-22 15:37 ` Junio C Hamano
2022-06-23 10:29 ` Ævar Arnfjörð Bjarmason
2022-06-23 23:25 ` Junio C Hamano
2022-06-23 23:45 ` Ævar Arnfjörð Bjarmason
2022-06-24 1:14 ` Junio C Hamano
2022-06-28 10:15 ` [PATCH v3 0/8] " Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 1/8] gitweb/Makefile: define all .PHONY prerequisites inline Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 2/8] gitweb/Makefile: add a $(GITWEB_ALL) variable Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 3/8] gitweb/Makefile: clear up and de-duplicate the gitweb.{css,js} vars Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 4/8] gitweb/Makefile: prepare to merge into top-level Makefile Ævar Arnfjörð Bjarmason
2022-06-28 10:15 ` [PATCH v3 5/8] gitweb: remove "test" and "test-installed" targets Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 6/8] gitweb/Makefile: include in top-level Makefile Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 7/8] Makefile: build 'gitweb' in the default target Ævar Arnfjörð Bjarmason
2022-06-28 10:16 ` [PATCH v3 8/8] gitweb/Makefile: add a "NO_GITWEB" parameter Ævar Arnfjörð Bjarmason
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).