On Tue, Jun 21 2022, Jeff King wrote:

On Mon, Jun 20, 2022 at 10:32:02AM +0200, SZEDER Gábor wrote:

quoted

On Mon, Jun 06, 2022 at 10:44:54AM -0700, Junio C Hamano wrote:

quoted

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

The $subject is a proposed re-roll of SZEDER's
https://lore.kernel.org/git/20220525205651.825669-1-szeder.dev@gmail.com (local);
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.