[PATCH v3 13/13] perl Git::LoadCPAN: emit better errors under NO_PERL_CPAN_FALLBACKS

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 3, 2018, 15:38 UTC
Message-ID: <20180303153817.20270-14-avarab@gmail.com>
In-Reply-To: <20180303153817.20270-1-avarab@gmail.com>

Before my 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) on an OS package that removed the private-Error.pm copy we carried around manually removing the OS's Error.pm would yield:

    $ git add -p
    Can't locate Error.pm in @INC (you may need to install the Error module) [...]

Now, before this change we'll instead emit this more cryptic error:

    $ git add -p
    BUG: '/usr/share/perl5/Git/FromCPAN' should be a directory! at /usr/share/perl5/Git/Error.pm line 36.

This is a confusing error. Now if the new NO_PERL_CPAN_FALLBACKS option is specified and we can't find the module we'll instead emit:

    $ /tmp/git/bin/git add -p
    BUG: The 'Error' module is not here, but NO_PERL_CPAN_FALLBACKS was set!

    [...]

Where [...] is the lengthy explanation seen in the change below, which explains what the potential breakage is, and how to fix it.

The reason for checking @@NO_PERL_CPAN_FALLBACKS@@] against the empty string in Perl is as opposed to checking for a boolean value is that that's (as far as I can tell) make's idea of a string that's set, and e.g. NO_PERL_CPAN_FALLBACKS=0 is enough to set NO_PERL_CPAN_FALLBACKS.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 INSTALL              | 11 ++++++++---
 Makefile             |  5 ++++-
 perl/Git/LoadCPAN.pm | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/INSTALL b/INSTALL
index 808e07b659..60e515eaf7 100644
--- a/INSTALL
+++ b/INSTALL
@@ -88,9 +88,9 @@ Issues of note:
 	export GIT_EXEC_PATH PATH GITPERLLIB
 
  - By default (unless NO_PERL is provided) Git will ship various perl
-   scripts & libraries it needs. However, for simplicity it doesn't
-   use the ExtUtils::MakeMaker toolchain to decide where to place the
-   perl libraries. Depending on the system this can result in the perl
+   scripts. However, for simplicity it doesn't use the
+   ExtUtils::MakeMaker toolchain to decide where to place the perl
+   libraries. Depending on the system this can result in the perl
    libraries not being where you'd like them if they're expected to be
    used by things other than Git itself.
 
@@ -102,6 +102,11 @@ Issues of note:
    Will result in e.g. perllibdir=/usr/share/perl/5.26.1 on Debian,
    perllibdir=/usr/share/perl5 (which we'd use by default) on CentOS.
 
+ - Unless NO_PERL is provided Git will ship various perl libraries it
+   needs. Distributors of Git will usually want to set
+   NO_PERL_CPAN_FALLBACKS if NO_PERL is not provided to use their own
+   copies of the CPAN modules Git needs.
+
  - Git is reasonably self-sufficient, but does depend on a few external
    programs and libraries.  Git can be used without most of them by adding
    the approriate "NO_<LIBRARY>=YesPlease" to the make command line or
diff --git a/Makefile b/Makefile
index cbf0bc2344..56fc2d6f08 100644
--- a/Makefile
+++ b/Makefile
@@ -2316,11 +2316,14 @@ all:: $(LIB_PERL_GEN)
 ifndef NO_PERL_CPAN_FALLBACKS
 all:: $(LIB_CPAN_GEN)
 endif
+NO_PERL_CPAN_FALLBACKS_SQ = $(subst ','\'',$(NO_PERL_CPAN_FALLBACKS))
 endif
 
 perl/build/lib/%.pm: perl/%.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
-	sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' < $< > $@
+	sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' \
+	    -e 's|@@NO_PERL_CPAN_FALLBACKS@@|$(NO_PERL_CPAN_FALLBACKS_SQ)|g' \
+	< $< > $@
 
 perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
diff --git a/perl/Git/LoadCPAN.pm b/perl/Git/LoadCPAN.pm
index 229c1ee87d..e5585e75e8 100644
--- a/perl/Git/LoadCPAN.pm
+++ b/perl/Git/LoadCPAN.pm
@@ -19,13 +19,25 @@ attempt to load C<Module::Name> from the OS, and if that doesn't work
 will fall back on C<FromCPAN::Module::Name> shipped with Git itself.
 
 Usually distributors will not ship with Git's Git::FromCPAN tree at
-all, preferring to use their own packaging of CPAN modules instead.
+all via the C<NO_PERL_CPAN_FALLBACKS> option, preferring to use their
+own packaging of CPAN modules instead.
 
 This module is only intended to be used for code shipping in the
 C<git.git> repository. Use it for anything else at your peril!
 
 =cut
 
+# NO_PERL_CPAN_FALLBACKS_STR evades the sed search-replace from the
+# Makefile, and allows for detecting whether the module is loaded from
+# perl/Git as opposed to perl/build/Git, which is useful for one-off
+# testing without having Error.pm et al installed.
+use constant NO_PERL_CPAN_FALLBACKS_STR => '@@' . 'NO_PERL_CPAN_FALLBACKS' . '@@';
+use constant NO_PERL_CPAN_FALLBACKS => (
+	q[@@NO_PERL_CPAN_FALLBACKS@@] ne ''
+	and
+	q[@@NO_PERL_CPAN_FALLBACKS@@] ne NO_PERL_CPAN_FALLBACKS_STR
+);
+
 sub import {
 	shift;
 	my $caller = caller;
@@ -45,6 +57,25 @@ sub import {
 	} or do {
 		my $error = $@ || "Zombie Error";
 
+		if (NO_PERL_CPAN_FALLBACKS) {
+			chomp(my $error = sprintf <<'THEY_PROMISED', $module);
+BUG: The '%s' module is not here, but NO_PERL_CPAN_FALLBACKS was set!
+
+Git needs this Perl module from the CPAN, and will by default ship
+with a copy of it. This Git was built with NO_PERL_CPAN_FALLBACKS,
+meaning that whoever built it promised to provide this module.
+
+You're seeing this error because they broke that promise, and we can't
+load our fallback version, since we were asked not to install it.
+
+If you're seeing this error and didn't package Git yourself the
+package you're using is broken, or your system is broken. This error
+won't appear if Git is built without NO_PERL_CPAN_FALLBACKS (instead
+we'll use our fallback version of the module).
+THEY_PROMISED
+			die $error;
+		}
+
 		my $Git_LoadCPAN_pm_path = $INC{"Git/LoadCPAN.pm"} || die "BUG: Should have our own path from %INC!";
 
 		require File::Basename;

-- 
2.15.1.424.g9478a66081