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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Feb 16, 2018, 22:03 UTC
Message-ID: <20180216220300.GC216564@aiede.svl.corp.google.com>
In-Reply-To: <20180216175519.GQ27038@zaya.teonanacatl.net>

Hi,

Todd Zullinger wrote:

Show 20 quoted lines

> Subject: [PATCH] Makefile: add NO_PERL_CPAN_FALLBACKS 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_FALLBACKS to easily avoid installing these fallback perl
> CPAN modules.
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
> ---
>  Makefile                                    | 6 ++++++
>  perl/{Git => }/FromCPAN/.gitattributes      | 0
>  perl/{Git => }/FromCPAN/Error.pm            | 0
>  perl/{Git => }/FromCPAN/Mail/.gitattributes | 0
>  perl/{Git => }/FromCPAN/Mail/Address.pm     | 0
>  5 files changed, 6 insertions(+)
>  rename perl/{Git => }/FromCPAN/.gitattributes (100%)
>  rename perl/{Git => }/FromCPAN/Error.pm (100%)
>  rename perl/{Git => }/FromCPAN/Mail/.gitattributes (100%)
>  rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

Nice!  I like it.

[...]

Show 7 quoted lines

> +++ 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_FALLBACKS if you do not want to install fallbacks for
> +# perl CPAN modules.

nit: Looking at this as a naive user, it's not obvious what kind of fallbacks are meant. How about:

	Define NO_PERL_CPAN_FALLBACKS if you do not want to install bundled
	copies of CPAN modules that serve as a fallback in case the modules are
	not available on the system.

Or perhaps:

	Define HAVE_CPAN_MODULES if you have Error.pm and Mail::Address installed
	and do not want to install the fallback copies from perl/FromCPAN.

Would this patch need to update the loader to expect the modules in the new location?

Thanks, Jonathan