Re: [PATCH v2 0/7] gitweb: fix "make" not including "gitweb" without NOOP run slowdowns

[Jeff King <peff@peff.net>]

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