On Wed, Jun 22 2022, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
