Re: [PATCH 3/8] perl: generalize the Git::LoadCPAN facility

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Feb 15 2018, Todd Zullinger jotted:

> Hi Ævar,
>
> Ævar Arnfjörð Bjarmason wrote:
>> +Git::LoadCPAN - Wrapper for loading modules from the CPAN (OS) or Git's own copy
>> +
>> +=head1 DESCRIPTION
>> +
>> +The Perl code in Git depends on some modules from the CPAN, but we
>> +don't want to make those a hard requirement for anyone building from
>> +source.
>> +
>> +Therefore the L<Git::LoadCPAN> namespace shipped with Git contains
>> +wrapper modules like C<Git::LoadCPAN::Module::Name> that will first
>> +attempt to load C<Module::Name> from the OS, and if that doesn't work
>> +will fall back on C<Git::FromCPAN::Module::Name> shipped with Git
>> +itself.
>> +
>> +Usually OS's will not ship with Git's Git::FromCPAN tree at all,
>> +preferring to use their own packaging of CPAN modules instead.
>
> This is something I wondered about.  What's the recommended
> method to ensure git packaged for an OS/distribution doesn't
> ever use the fallbacks?  Remove $perllibdir/Git/FromCPAN
> after make install?
>
> If so, would it be useful to add a Makefile knob to not
> install the FromCPAN bits, which may be generally useful to
> packagers?
>
> Something like the following, perhaps?
>
> (I'd feel bad suggesting this without a patch, after all the
> work you've already done to simplify and improve the perl
> bits.)
>
> ---- 8< ----
> From: Todd Zullinger <tmz@pobox.com>
> Date: Wed, 14 Feb 2018 23:00:30 -0500
> Subject: [PATCH] Makefile: add NO_PERL_CPAN to disable fallback module install
>
> As noted in perl/Git/LoadCPAN.pm, operating system packages often don't
> want to ship Git::FromCPAN tree at all, preferring to use their own
> packaging of CPAN modules instead.  Allow such packagers to set
> NO_PERL_CPAN to easily avoid installing these fallback perl CPAN
> modules.
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 5bcd83ddf3..c4e035e5bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -296,6 +296,9 @@ all::
>  #
>  # Define NO_PERL if you do not want Perl scripts or libraries at all.
>  #
> +# Define NO_PERL_CPAN if you do not want to install fallbacks for perl CPAN
> +# modules.
> +#
>  # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
>  # but /usr/bin/python2.7 on some platforms).
>  #
> @@ -2572,6 +2575,7 @@ ifndef NO_GETTEXT
>  endif
>  ifndef NO_PERL
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
> +	test -z "$(NO_PERL_CPAN)" || rm -rf perl/build/lib/Git/FromCPAN
>  	(cd perl/build/lib && $(TAR) cf - .) | \
>  	(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
>  	$(MAKE) -C gitweb install
> --
> 2.16.1
>
> I don't particularly like NO_PERL_CPAN, but I'm confident
> someone else will suggest an obviously better name.
>
> I thought about moving the 'rm -rf Git/FromCPAN' after the
> tar/untar, to keep the files in place for the tests.  No
> tests seem to rely on those local files, so I stuck with
> removing them before.  That diff was:
>
> --- a/Makefile
> +++ b/Makefile
> @@ -2574,6 +2574,7 @@
>  	$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perllibdir_SQ)'
>  	(cd perl/build/lib && $(TAR) cf - .) | \
>  	(cd '$(DESTDIR_SQ)$(perllibdir_SQ)' && umask 022 && $(TAR) xof -)
> +	test -n "$(NO_PERL_CPAN)" && rm -rf '$(DESTDIR_SQ)$(perllibdir_SQ)'/Git/FromCPAN
>  	$(MAKE) -C gitweb install
>  endif
>  ifndef NO_TCLTK

That makes sense, I'll incorporate that in a re-roll. I like
NO_PERL_CPAN_FALLBACKS or just NO_CPAN_FALLBACKS better.

I'd really like to find some solution that works differently though,
because with this approach we'll run the full test suite against a
system where our fallbacks will be in place (although if the OS
distributor has done as promised we won't use them), and then just
remove this at 'make install' time, also meaning we'll re-gen it before
running 'make install' again, only to rm it again.

The former issue we could deal with by munging the Git::LoadCPAN file so
it knows about NO_PERL_CPAN_FALLBACKS, and will always refuse to use the
fallbacks if that's set. That's a good idea anyway, because right now if
you e.g. uninstall Error.pm on Debian (which strips the CPAN fallbacks)
you get a cryptic "BUG: ..." message, it should instead say "we couldn't
get this module the OS promised we'd have" or something to that effect.

The latter is trickier, I don't see an easy way to coerce the Makefile
into not copying the FromCPAN directory without going back to a
hardcoded list again, the easiest thing is probably to turn that:

    $(TAR) cf - .)

Into:

    $(TAR) cf - $(find ... -not ....)

Or something like that to get all the stuff that isn't the Git/FromCPAN
directory.

Other suggestions most welcome.