various perl fixes

I'd been meaning to submit 3/* once my Makefile.PL removal landed in master, but noticed a few more things along the way, including the issue fixed in 1/* which I just noted in <CACBZZX7xC37W5+MLtYSrBaPawh+QfOSqci_rFOp_ukVi4fp6Gg@mail.gmail.com>, and while I was at it resolved some of my long-standing TODOs noted in <[email protected]>, and more.

If you're CC'd on this series it's because one of the commit messages mentions a commit you authored.

[45]/* do not cleanly pass the default --check, so they have .gitattributes files to make them pass, as noted in 4/* it's much easier if we can just use this upstream code as-is, and not accumulate our own typo/whitespace etc. fixes along the way, which just makes subsequent updates harder.

Ævar Arnfjörð Bjarmason (8):
perl: *.pm files should not have the executable bit
perl: move CPAN loader wrappers to another namespace
perl: generalize the Git::LoadCPAN facility
perl: update our ancient copy of Error.pm
perl: update our copy of Mail::Address
git-send-email: unconditionally use Net::{SMTP,Domain}
gitweb: hard-depend on the Digest::MD5 5.8 module
perl: hard-depend on the File::{Temp,Spec} modules

 contrib/examples/git-difftool.perl    |   2 +-
 git-send-email.perl                   |  28 +--
 gitweb/INSTALL                        |   3 +-
 gitweb/gitweb.perl                    |  17 +-
 perl/Git.pm                           |   7 +-
 perl/Git/Error.pm                     |  46 ----
 perl/Git/FromCPAN/.gitattributes      |   1 +
 perl/Git/FromCPAN/Error.pm            | 296 +++++++++++++++++++----
 perl/Git/FromCPAN/Mail/.gitattributes |   1 +
 perl/Git/FromCPAN/Mail/Address.pm     | 436 +++++++++++++---------------------
 perl/Git/LoadCPAN.pm                  |  74 ++++++
 perl/Git/LoadCPAN/Error.pm            |  10 +
 perl/Git/LoadCPAN/Mail/Address.pm     |  10 +
 perl/Git/Mail/Address.pm              |  24 --
 14 files changed, 537 insertions(+), 418 deletions(-)
 delete mode 100644 perl/Git/Error.pm
 create mode 100644 perl/Git/FromCPAN/.gitattributes
 create mode 100644 perl/Git/FromCPAN/Mail/.gitattributes
 create mode 100644 perl/Git/LoadCPAN.pm
 create mode 100644 perl/Git/LoadCPAN/Error.pm
 create mode 100644 perl/Git/LoadCPAN/Mail/Address.pm
 delete mode 100755 perl/Git/Mail/Address.pm

2.15.1.424.g9478a66081

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05) had the executable bit set, this should not be the case with *.pm files, it breaks nothing, but is redundant and confusing as none of the other files have it, and it's never executed as a stand-alone program.

---
 perl/Git/Mail/Address.pm | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 perl/Git/Mail/Address.pm

old mode 100755
new mode 100644

2.15.1.424.g9478a66081

Hi,

Needs a period somewhere to break up the long sentence with comma splices. How about:

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05) had the executable bit set. That bit should not be set for *.pm files. It breaks nothing but but it is redundant and confusing as none of the other files have it and these files are never executed as stand-alone programs.

With or without such a tweak,

Thanks.

On Wed, Feb 14 2018, Jonathan Nieder jotted:

Thanks, used this with s/but but/but/

Move the Git::Error and Git::Mail::Address wrappers to the Git::LoadCPAN::Loader::* namespace, e.g. Git::LoadCPAN::Error, that module will then either load Error from CPAN (if installed on the OS), or use Git::FromCPAN::Error.

When I added the Error wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) I didn't think about how confusing it would be to have these modules sitting in the same tree as our normal modules. Let's put these all into Git::{Load,From}CPAN::* to clearly distinguish them from the rest.

This also makes things a bit less confusing since there was already a Git::Error namespace ever since 8b9150e3e3 ("Git.pm: Handle failed commands' output", 2006-06-24).

---
 contrib/examples/git-difftool.perl      | 2 +-
 git-send-email.perl                     | 4 ++--
 perl/Git.pm                             | 2 +-
 perl/Git/{ => LoadCPAN}/Error.pm        | 8 ++++----
 perl/Git/{ => LoadCPAN}/Mail/Address.pm | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)
 rename perl/Git/{ => LoadCPAN}/Error.pm (78%)
 rename perl/Git/{ => LoadCPAN}/Mail/Address.pm (69%)

index fb0fd0b84b..b2ea80f9ed 100755
@@ -13,7 +13,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use File::Basename qw(dirname);
 use File::Copy;
 use File::Find;

index bbf4deaa0d..85bb6482f2 100755
@@ -26,11 +26,11 @@ use Text::ParseWords;
 use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catdir catfile);
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
-use Git::Mail::Address;
+use Git::LoadCPAN::Mail::Address;
 
 Getopt::Long::Configure qw/ pass_through /;
 

index 9d60d7948b..8e02ee2cca 100644
@@ -101,7 +101,7 @@ increase notwithstanding).
 
 
 use Carp qw(carp croak); # but croak is bad - throw instead
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use IPC::Open2 qw(open2);
 use Fcntl qw(SEEK_SET SEEK_CUR);

similarity index 78%
rename from perl/Git/Error.pm
rename to perl/Git/LoadCPAN/Error.pm
index 09bbc97390..3513fe745b 100644
@@ -1,11 +1,11 @@
-package Git::Error;
+package Git::LoadCPAN::Error;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Error - Wrapper for the L<Error> module, in case it's not installed
+Git::LoadCPAN::Error - Wrapper for the L<Error> module, in case it's not installed
 
 =head1 DESCRIPTION
 

@@ -26,13 +26,13 @@ sub import {
     } or do {
 	my $error = $@ || "Zombie Error";
 
-	my $Git_Error_pm_path = $INC{"Git/Error.pm"} || die "BUG: Should have our own path from %INC!";
+	my $Git_Error_pm_path = $INC{"Git/LoadCPAN/Error.pm"} || die "BUG: Should have our own path from %INC!";
 
 	require File::Basename;
 	my $Git_Error_pm_root = File::Basename::dirname($Git_Error_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_Error_pm_path'!";
 
 	require File::Spec;
-	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, 'FromCPAN');
+	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, '..', 'FromCPAN');
 	die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
 
 	local @INC = ($Git_pm_FromCPAN_root, @INC);

similarity index 69%
rename from perl/Git/Mail/Address.pm
rename to perl/Git/LoadCPAN/Mail/Address.pm
index 2ce3e84670..879c2f5cd1 100644
@@ -1,11 +1,11 @@
-package Git::Mail::Address;
+package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
+Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
 
 =head1 DESCRIPTION
 

2.15.1.424.g9478a66081

[...]

Makes sense.

Change the two wrappers to load from CPAN (local OS) or our own copy to do so via the same codepath.

I added the Error.pm wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10), and shortly afterwards Matthieu Moy added a wrapper for Mail::Address in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05).

His was simpler since Mail::Address doesn't have an "import" method, but didn't do the same sanity checking, e.g. a missing FromCPAN directory (which OS packages are likely not to have) wouldn't be explicitly warned about.

Now both use a modification of the previously Error.pm-specific codepath, which has been amended to take the module to load as parameter, as well as whether or not that module has an import method.

---
 perl/Git/LoadCPAN.pm              | 74 +++++++++++++++++++++++++++++++++++++++
 perl/Git/LoadCPAN/Error.pm        | 44 +++--------------------
 perl/Git/LoadCPAN/Mail/Address.pm | 22 +++---------
 3 files changed, 82 insertions(+), 58 deletions(-)
 create mode 100644 perl/Git/LoadCPAN.pm

new file mode 100644
index 0000000000..2262812654
@@ -0,0 +1,74 @@
+package Git::LoadCPAN;
+use 5.008;
+use strict;
+use warnings;
+
+=head1 NAME
+
+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 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
+
+sub import {
+    shift;
+    my $caller = caller;
+    my %args = @_;
+    my $module = exists $args{module} ? delete $args{module} : die "BUG: Expected 'module' parameter!";
+    my $import = exists $args{import} ? delete $args{import} : die "BUG: Expected 'import' parameter!";
+    die "BUG: Too many arguments!" if keys %args;
+
+    # Foo::Bar to Foo/Bar.pm
+    my $package_pm = $module;
+    $package_pm =~ s[::][/]g;
+    $package_pm .= '.pm';
+
+    eval {
+	require $package_pm;
+	1;
+    } or do {
+	my $error = $@ || "Zombie Error";
+
+	my $Git_LoadCPAN_pm_path = $INC{"Git/LoadCPAN.pm"} || die "BUG: Should have our own path from %INC!";
+
+	require File::Basename;
+	my $Git_LoadCPAN_pm_root = File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
+
+	require File::Spec;
+	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 'FromCPAN');
+	die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
+
+	local @INC = ($Git_pm_FromCPAN_root, @INC);
+	require $package_pm;
+    };
+
+    if ($import) {
+	no strict 'refs';
+	*{"${caller}::import"}= sub {
+	    shift;
+	    use strict 'refs';
+	    unshift @_, $module;
+	    goto &{"${module}::import"};
+	};
+	use strict 'refs';
+    }
+}
+
+1;

index 879c2f5cd1..a13f0e58cf 100644
@@ -2,23 +2,9 @@ package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
-
-=head1 NAME
-
-Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
-
-=head1 DESCRIPTION
-
-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
-
-eval {
-    require Mail::Address;
-    1;
-} or do {
-    require Git::FromCPAN::Mail::Address;
-};
+use Git::LoadCPAN (
+    module => 'Mail::Address',
+    import => 0,
+);
 
 1;

2.15.1.424.g9478a66081

nit: I think with s/to load/that load/ this will be easier to read.

I'm having trouble parsing this. Mail::Address didn't do the same sanity checking or his didn't?

The comma before e.g. should be a period or semicolon, since it's starting a new independent clause.

Does "now" mean before this patch or after this patch? Usually commit messages describe the status quo without the patch in the present tense and the change the patch will make in the imperative. So this could say:

Update both to use a common implementation based on the previous Error.pm loader.

[...]

[...]

Hi Jonathan,

Heh, a similar bias is what led me to suggest a Makefile knob to prevent installing the fallbacks. I neglected to add you to the Cc list on that reply. But I was thinking of Debian and other distributions whom I know would similarly want to exclude Git/FromCPAN from their git packages. I know it's a simple rm, but it might as well be a simple rm in one place than spread across each package. :)

It may still be worth considering whether it's reasonable to make Mail::Address and Error.pm hard requirements, but I'm not sure if there are any platforms where such a requirement would be a pain. If there are folks here who are happy to maintain this fallback method, then a simple Makefile knob gives us the best of both worlds.

Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Historian, n. A broad-gauge gossip.
    -- Ambrose Bierce, "The Devil's Dictionary"

On Wed, Feb 14 2018, Jonathan Nieder jotted:

All good feeedback, thanks. Incorporated into v2 which I'm about to submit.

I can't remember when, but at some point this was discussed on list, and the consensus was that us using perl should be kept as a non-invasive implementation detail that would be as small of a pain as possible for users.

It's easy for distros to package these modules, but for users building from source who know nothing about perl it can be a pain.

I also think it's very useful to avoid the side-discussion about not using some useful CPAN module in the future just because it's not widely used, but would be perfect for some use-case of ours.

I think there's a very good argument to be made for inverting the NO_PERL_CPAN_FALLBACKS logic, but my soon to be submitted v2 keeps it off by default.

Thanks.

Thanks, that sucked. Changed to \t. I still haven't gotten around to hacking my $EDITOR settings for git.git (like for C & SH).

That would include the platform I'm maintaining, where perl is currently pretty handcuffed and blindfolded (including completion code misinterprets). CPAN isn't currently an option, but might be soon.

Know perl I do. Use it not, can I. ;-)

Cool, thanks.

Cheers,
Randall

-- Brief whoami:
NonStop developer since approximately NonStop(211288444200000000) UNIX developer since approximately 421664400
-- In my real life, I talk too much.

Hi Ævar,

---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

index 5bcd83ddf3..c4e035e5bf 100644
@@ -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

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Man has made use of his intelligence, he invented stupidity.
    -- Remy De Gourmant

On Thu, Feb 15 2018, Todd Zullinger jotted:

    $(TAR) cf - .)

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

[I dropped [email protected] from the Cc: list, as it bounced on my previous reply.]

Either is an improvement. Starting with NO_PERL_ seems like a slightly better bikeshed color. :)

Teaching Git::LoadCPAN to never fallback sounds like a good idea. At least then if the packager intended to avoid the fallbacks and didn't get it right the error message could be more useful.

Hopefully that's not a common problem for packagers though. (And adding the Makefile knob was intended to help make it easier for packagers to achieve this common goal.)

What about moving perl/Git/FromCPAN to perl/FromCPAN and then including perl/FromCPAN in LIB_PERL{,_GEN} only if NO_PERL_CPAN_FALLBACKS is unset?

LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
+ifndef NO_PERL_CPAN_FALLBACKS
+LIB_PERL += $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm) +endif
LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))

I haven't tested that at all, so it could be broken in many ways.

Thanks,

Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The nice thing about egotists is that they don't talk about other
people.
    -- Lucille S. Harper

On Thu, Feb 15 2018, Todd Zullinger jotted:

Yes that's a much better idea, it evades the whole problem of conflating the perl/Git* glob.

I did test this yesterday and it seems to work well. Here it is in patch form, in case it's helpful to you for the next version. It might well be better as part of a commit teaching Git::LoadCPAN to respect NO_PERL_CPAN_FALLBACKS.

---- 8< ----
From: Todd Zullinger <[email protected]>
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.

---
 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%)

index 5bcd83ddf3..838b3c6393 100644
@@ -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.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #

@@ -2297,6 +2300,9 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
 	$(QUIET_MSGFMT)mkdir -p $(dir $@) && $(MSGFMT) -o $@ $<
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
+ifndef NO_PERL_CPAN_FALLBACKS
+LIB_PERL += $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+endif
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
 
 ifndef NO_PERL

similarity index 100%
rename from perl/Git/FromCPAN/.gitattributes
rename to perl/FromCPAN/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Error.pm
rename to perl/FromCPAN/Error.pm

similarity index 100%
rename from perl/Git/FromCPAN/Mail/.gitattributes
rename to perl/FromCPAN/Mail/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Mail/Address.pm
rename to perl/FromCPAN/Mail/Address.pm

2.16.1

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Damn you and your estrogenical treachery!
    -- Stewie Griffin

Hi,

Nice! I like it.

[...]

Hi Jonathan,

[...]

Hmm, a positive variable like HAVE_CPAN_MODULES is appealing.

I don't know about listing the modules, as those seem likely to change and then the comment becomes stale. It's nice to have a shorter name. I could easily go back and forth. Hopefully some other folks will chime in with preferences.

That's a good catch. In checking how this ends up when not setting NO_PERL_CPAN_FALLBACKS, we end up installing FromCPAN at the root of $perllibdir rather than under the Git dir.

While we could probably fix Git::LoadCPAN, I doubt we want to pollute the namespace. ;) So we'll want to ensure the files get put in the right place from the start. I'll try to fix that up.

Thanks for the careful eyes, as usual.

Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Happiness is like peeing on yourself. Everyone can see it, but only
you can feel its warmth

I ended up adding a second variable to hold the FromCPAN wildcard match, as I couldn't come up with any clean way to do it in the same LIB_PERL{,_GEN} vars.

I tested it with and without NO_PERL_CPAN_FALLBACKS set and with and without the system Error and Mail::Address modules installed. There's nothing which automatically removes the perl/build/lib/Git/FromCPAN tree if make was run without NO_PERL_CPAN_FALLBACKS set and then re-run with it. I don't know if that's something we'd care to support or not. I suspect it's simpler to require 'make clean' between such runs.

(I had to restore the FromCPAN copy of Address.pm, as noted in <[email protected]>, of course.)

Here's what it looks like now (still subject to changing the NO_PERL_CPAN_FALLBACKS variable).

---- 8> ----
From: Todd Zullinger <[email protected]>
Subject: [PATCH] Makefile: add NO_PERL_CPAN_FALLBACKS knob

We include some perl modules which are not part of the core perl install, as a convenience. This allows us to rely on those modules in our perl-based tools and scripts without requiring users to install the modules from CPAN or their operating system packages.

Users whose operating system provides these modules and packagers of Git often don't want to ship or use these bundled modules. Allow these users to set NO_PERL_CPAN_FALLBACKS to avoid installing the bundled modules.

---
 Makefile                                    | 14 ++++++++++++++
 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, 14 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%)

index bdd50069af..49244fb116 100644
@@ -296,6 +296,10 @@ 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 bundled
+# copies of CPAN modules that serve as a fallback in case the modules are
+# not available on the system.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #

@@ -2300,15 +2304,25 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
+ifndef NO_PERL_CPAN_FALLBACKS
+LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/Git/%.pm,$(LIB_CPAN))
+endif
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
+all:: $(LIB_CPAN_GEN)
+endif
 endif
 
 perl/build/lib/%.pm: perl/%.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	sed -e 's|@@LOCALEDIR@@|$(localedir_SQ)|g' < $< > $@
 
+perl/build/lib/Git/FromCPAN/%.pm: perl/FromCPAN/%.pm
+	$(QUIET_GEN)mkdir -p $(dir $@) && cp $< $@
+
 perl/build/man/man3/Git.3pm: perl/Git.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \
 	pod2man $< $@

similarity index 100%
rename from perl/Git/FromCPAN/.gitattributes
rename to perl/FromCPAN/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Error.pm
rename to perl/FromCPAN/Error.pm

similarity index 100%
rename from perl/Git/FromCPAN/Mail/.gitattributes
rename to perl/FromCPAN/Mail/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Mail/Address.pm
rename to perl/FromCPAN/Mail/Address.pm

2.16.2

I thought about using '$(INSTALL) -m 644' instead of 'cp' to
allow users to set 'INSTALL = install -p' to preserve the
timestamp of the FromCPAN files, but that would only be
useful to users installing from the same tarball version
multiple times.  The intersection of users who care that
much about consistent timestamps and don't care about using
system perl modules seems small.

-- 
Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A person who smiles in the face of adversity ... probably has a
scapegoat.

The Error.pm shipped with Git as a fallback if there was no Error.pm on the system was released in April 2006, there's been dozens of releases since then, the latest at August 7, 2017, let's update to that.

This undoes a local hack we'd accumulated in 96bc4de85c ("Eliminate Scalar::Util usage from private-Error.pm", 2006-07-26), it's been redundant since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24).

This also undoes 3a51467b94 ("Typo fix: replacing it's -> its", 2013-04-13). This is the Nth time I find that some upstream code of ours (in contrib/, in sha1dc/ and now in perl/ ...) has diverged from upstream because of some tree-wide typo fixing. Let's not do those fixes against upstream projects, it's more valuable that we have a 1=1 mapping to upstream than to fix typos in docs we never even generate from this code. If someone wants to fix typos in them fine, but they should do it with a patch to upstream which git.git can then incorporate.

The upstream code doesn't cleanly pass a --check, so I'm adding a .gitattributes file for similar reasons as done for sha1dc in 5d184f468e ("sha1dc: ignore indent-with-non-tab whitespace violations", 2017-06-06).

---
 perl/Git/FromCPAN/.gitattributes |   1 +
 perl/Git/FromCPAN/Error.pm       | 296 +++++++++++++++++++++++++++++++++------
 2 files changed, 256 insertions(+), 41 deletions(-)
 create mode 100644 perl/Git/FromCPAN/.gitattributes

new file mode 100644
index 0000000000..8b64fc5e22
@@ -0,0 +1 @@
+/Error.pm whitespace=-blank-at-eof

2.15.1.424.g9478a66081

Comma splices:
s/, there's/. There's/
s/, let's/. Let's/

The one piece of information I was curious about that this (quite clear, thank you) commit message is missing is what changed in the intervening time. Is this just about keeping up with upstream to make it easy to keep up later, or has upstream made any useful changes? E.g. did any API or behaviors get better?

Related: do we have to worry about in-tree users taking advantage of improved API and packagers forgetting to add a dependency on the new version? Do we declare the minimal required Error.pm version somewhere (e.g. in the INSTALL file)?

[...]

Most of the added lines are documentation, so this diffstat doesn't look half-bad.

Thanks for writing it.

Jonathan

On Wed, Feb 14 2018, Jonathan Nieder jotted:

Will note that in v2, nothing we need changed, this is just a matter of keeping up, and e.g. if I run Git on Debian I get a few month old Error.pm, but if I build Git from source I get one that's more than a decade old.

We don't, but if that situation arises we could just start doing 'use <module> <version>' to declare it, see "perldoc -f use", but right now we work on seemingly every version of these modules ever released.

Update our copy of Mail::Address from 2.19 (Aug 22, 2017) to 2.20 (Jan 23, 2018). This should be a trivial update[1] but it seems the version Matthieu Moy imported in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05) doesn't correspond to any 2.19 version found on the CPAN. From the comment at the top of the file it looks like some OS version with the POD stripped, and with different indentation.

Let's instead use the upstream version as-is, and without copyright notices stripped. Like Error.pm this doesn't cleanly pass --check, so add a .gitattributes file to ignore the errors.

1. https://metacpan.org/diff/file?target=MARKOV/MailTools-2.20/lib%2FMail%2FAddress.pod&source=MARKOV%2FMailTools-2.19%2Flib%2FMail%2FAddress.pod

---
 perl/Git/FromCPAN/Mail/.gitattributes |   1 +
 perl/Git/FromCPAN/Mail/Address.pm     | 436 +++++++++++++---------------------
 2 files changed, 163 insertions(+), 274 deletions(-)
 create mode 100644 perl/Git/FromCPAN/Mail/.gitattributes

new file mode 100644
index 0000000000..94f3e5bb86
@@ -0,0 +1 @@
+/Address.pm whitespace=-trailing-space

2.15.1.424.g9478a66081

Were there changes other than the POD stripping?

Yikes re the stripped POD with license notice. Thanks for fixing it.

No.

I should have mentionned it in the commit message, but the one I took was from:

http://cpansearch.perl.org/src/MARKOV/MailTools-2.19/lib/Mail/Address.pm

i.e. following the "source" link from:

http://search.cpan.org/~markov/MailTools-2.19/lib/Mail/Address.pod

The link name suggested it was the actual source code but indeed it's a pre-processed file with the POD stripped.

It would make sense to indicate explicitly where this file is from in this commit's message to avoid having the same discussion next time someone upgrades the package.

Matthieu Moy
https://matthieu-moy.fr/

On Thu, Feb 15 2018, Matthieu Moy jotted:

I see that I'm being a complete idiot and added the *.pod file in the distro instead of the *.pm file, I got it from metacpan and didn't check it carefully enough (and only tested with system Error.pm removed, not Mail::Address), sorry. So yours was the right version. Will fix in a re-roll.

The Net::SMTP and Net::Domain were both first released with perl v5.7.3, since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's no reason to conditionally require this anymore.

This conditional loading was initially added in 87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP", 2006-06-01) for Net::SMTP and 134550fe21 ("git-send-email.perl - try to give real name of the calling host to HELO/EHLO", 2010-03-14) for Net::Domain, both of which predate the hard dependency on 5.8.

---
 git-send-email.perl | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

index 85bb6482f2..69bd443245 100755
@@ -1143,10 +1143,9 @@ sub valid_fqdn {
 sub maildomain_net {
 	my $maildomain;
 
-	if (eval { require Net::Domain; 1 }) {
-		my $domain = Net::Domain::domainname();
-		$maildomain = $domain if valid_fqdn($domain);
-	}
+	require Net::Domain;
+	my $domain = Net::Domain::domainname();
+	$maildomain = $domain if valid_fqdn($domain);
 
 	return $maildomain;
 }

@@ -1154,17 +1153,16 @@ sub maildomain_net {
 sub maildomain_mta {
 	my $maildomain;
 
-	if (eval { require Net::SMTP; 1 }) {
-		for my $host (qw(mailhost localhost)) {
-			my $smtp = Net::SMTP->new($host);
-			if (defined $smtp) {
-				my $domain = $smtp->domain;
-				$smtp->quit;
+	require Net::SMTP;
+	for my $host (qw(mailhost localhost)) {
+		my $smtp = Net::SMTP->new($host);
+		if (defined $smtp) {
+			my $domain = $smtp->domain;
+			$smtp->quit;
 
-				$maildomain = $domain if valid_fqdn($domain);
+			$maildomain = $domain if valid_fqdn($domain);
 
-				last if $maildomain;
-			}
+			last if $maildomain;
 		}
 	}
 

2.15.1.424.g9478a66081

On Wed, Feb 14 2018, Jonathan Nieder jotted:

FWIW this is easily found out for any given module by running corelist <module name> on a system with perl installed:

    $ corelist File::Spec
    Data for 2017-01-14
    File::Spec was first released with perl 5.00405

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require Digest::MD5 anymore. It was released with perl v5.7.3.

The initial introduction of the dependency in
e9fdd74e53 ("gitweb: (gr)avatar support", 2009-06-30) says as much, this also undoes part of the later 2e9c8789b7 ("gitweb: Mention optional Perl modules in INSTALL", 2011-02-04) since gitweb will always be run on at least 5.8, so there's no need to mention Digest::MD5 as a required module in the documentation, let's instead say that we require perl 5.8.

---
 gitweb/INSTALL     |  3 +--
 gitweb/gitweb.perl | 17 +++++------------
 2 files changed, 6 insertions(+), 14 deletions(-)

index 408f2859d3..a58e6b3c44 100644
@@ -29,12 +29,11 @@ Requirements
 ------------
 
  - Core git tools
- - Perl
+ - Perl 5.8
  - Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.
  - web server
 
 The following optional Perl modules are required for extra features
- - Digest::MD5 - for gravatar support
  - CGI::Fast and FCGI - for running gitweb as FastCGI script
  - HTML::TagCloud - for fancy tag cloud in project list view
  - HTTP::Date or Time::ParseDate - to support If-Modified-Since for feeds

index 2417057f2b..8f48e3c02e 100755
@@ -490,7 +490,6 @@ our %feature = (
 	# Currently available providers are gravatar and picon.
 	# If an unknown provider is specified, the feature is disabled.
 
-	# Gravatar depends on Digest::MD5.
 	# Picon currently relies on the indiana.edu database.
 
 	# To enable system wide have in $GITWEB_CONFIG

@@ -1166,18 +1165,11 @@ sub configure_gitweb_features {
 	our @snapshot_fmts = gitweb_get_feature('snapshot');
 	@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
 
-	# check that the avatar feature is set to a known provider name,
-	# and for each provider check if the dependencies are satisfied.
-	# if the provider name is invalid or the dependencies are not met,
-	# reset $git_avatar to the empty string.
+	# check that the avatar feature is set to a known provider
+	# name, if the provider name is invalid, reset $git_avatar to
+	# the empty string.
 	our ($git_avatar) = gitweb_get_feature('avatar');
-	if ($git_avatar eq 'gravatar') {
-		$git_avatar = '' unless (eval { require Digest::MD5; 1; });
-	} elsif ($git_avatar eq 'picon') {
-		# no dependencies
-	} else {
-		$git_avatar = '';
-	}
+	$git_avatar = '' unless $git_avatar =~ /^(?:gravatar|picon)$/s;
 
 	our @extra_branch_refs = gitweb_get_feature('extra-branch-refs');
 	@extra_branch_refs = filter_and_validate_refs (@extra_branch_refs);

@@ -2165,6 +2157,7 @@ sub picon_url {
 sub gravatar_url {
 	my $email = lc shift;
 	my $size = shift;
+	require Digest::MD5;
 	$avatar_cache{$email} ||=
 		"//www.gravatar.com/avatar/" .
 			Digest::MD5::md5_hex($email) . "?s=";

2.15.1.424.g9478a66081

Makes sense, and I like the diffstat.

[...]

[...]

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require File::Temp and File::Spec anymore. They were first released with perl versions v5.6.1 and 5.00405, respectively.

This code was originally added in c14c8ceb13 ("Git.pm: Make File::Spec and File::Temp requirement lazy", 2008-08-15), presumably to make Git.pm work on 5.6.0.

---
 perl/Git.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 8e02ee2cca..221e827e83 100644
@@ -1324,8 +1324,9 @@ sub _temp_cache {
 }
 
 sub _verify_require {
-	eval { require File::Temp; require File::Spec; };
-	$@ and throw Error::Simple($@);
+	require File::Temp;
+	require File::Spec;
+	return;
 }
 
 =item temp_reset ( FILEHANDLE )

2.15.1.424.g9478a66081

On Wed, Feb 14 2018, Jonathan Nieder jotted:

I was just going for the minimal change, but yeah, that makes sense. Will do that in v2.

Yes, if we are making them hard dependency, unless a major part of the perl/Git.pm can be used without hitting _verify_require in the current code, there is no point using "require only if we use it" pattern. As _verify_require is used even in the Git::repository constructor, I think it makes more sense to have "use" upfront for these two modules.

Addresses comments against v1, and ships with a new NO_PERL_CPAN_FALLBACKS knob (thanks Todd!) which distributions can turn on to not get our FromCPAN copies. Details below.

Todd Zullinger (1):
Makefile: add NO_PERL_CPAN_FALLBACKS knob

Some of this was split off into into my "perl: move the perl/Git/FromCPAN tree to perl/FromCPAN", and I added "This option is intended[...]" to the Makefile documentation.

Ævar Arnfjörð Bjarmason (12):
perl: *.pm files should not have the executable bit

JN: Rephrased commit message.

Git.pm: remove redundant "use strict" from sub-package Git.pm: add the "use warnings" pragma

These are both new, something I noticed and seems sensible to fix while we're at it.

gitweb: hard-depend on the Digest::MD5 5.8 module Git.pm: hard-depend on the File::{Temp,Spec} modules
git-send-email: unconditionally use Net::{SMTP,Domain}

These all "use" the modules now, instead of using "require".

JN: Removed a comment from the gitweb code that isn't needed anymore.

Clarifications / fixes to the commit messages.

perl: update our ancient copy of Error.pm

Rephrased commit message.

perl: update our copy of Mail::Address

Actually ships with the working version of Mail::Address now (oops!), which makes the patch much smaller, and requires less explanation.

perl: move CPAN loader wrappers to another namespace

JN: Commit message phrasing.

perl: generalize the Git::LoadCPAN facility

JN: Commit message phrasing, and s/ /\t/g.

perl: move the perl/Git/FromCPAN tree to perl/FromCPAN perl Git::LoadCPAN: emit better errors under NO_PERL_CPAN_FALLBACKS

These are both new. There's now a NO_PERL_CPAN_FALLBACKS option for distributors to turn on to stop us from installing these CPAN fallbacks.

13/13 updates the docs, and emits a better error than what we emit now in master if the CPAN module we expect from the OS isn't installed.

 INSTALL                                 |  11 +-
 Makefile                                |  16 +-
 contrib/examples/git-difftool.perl      |   2 +-
 git-send-email.perl                     |  28 ++-
 gitweb/INSTALL                          |   3 +-
 gitweb/gitweb.perl                      |  17 +-
 perl/FromCPAN/.gitattributes            |   1 +
 perl/{Git => }/FromCPAN/Error.pm        | 296 +++++++++++++++++++++++++++-----
 perl/{Git => }/FromCPAN/Mail/Address.pm |   8 +-
 perl/Git.pm                             |  14 +-
 perl/Git/Error.pm                       |  46 -----
 perl/Git/LoadCPAN.pm                    | 104 +++++++++++
 perl/Git/LoadCPAN/Error.pm              |  10 ++
 perl/Git/LoadCPAN/Mail/Address.pm       |  10 ++
 perl/Git/Mail/Address.pm                |  24 ---
 15 files changed, 432 insertions(+), 158 deletions(-)
 create mode 100644 perl/FromCPAN/.gitattributes
 rename perl/{Git => }/FromCPAN/Error.pm (72%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (96%)
 delete mode 100644 perl/Git/Error.pm
 create mode 100644 perl/Git/LoadCPAN.pm
 create mode 100644 perl/Git/LoadCPAN/Error.pm
 create mode 100644 perl/Git/LoadCPAN/Mail/Address.pm
 delete mode 100755 perl/Git/Mail/Address.pm

2.15.1.424.g9478a66081

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05) had the executable bit set. That bit should not be set for *.pm files. It breaks nothing but it is redundant and confusing as none of the other files have it and these files are never executed as stand-alone programs.

---
 perl/Git/Mail/Address.pm | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 perl/Git/Mail/Address.pm

old mode 100755
new mode 100644

2.15.1.424.g9478a66081

In Perl the "use strict/warnings" pragmas are lexical, thus there's no reason to do:

    package Foo;
    use strict;
    package Bar;
    use strict;
    $x = 5;

To satisfy the desire that the undeclared $x variable will be spotted at compile-time. It's enough to include the first "use strict".

This functionally changes nothing, but makes a subsequent change where "use warnings" will be added to Git.pm less confusing and less verbose, since as with "strict" we'll only need to do that at the top of the file.

Changes code initially added in a6065b548f ("Git.pm: Try to support ActiveState output pipe", 2006-06-25).

---
 perl/Git.pm | 1 -
 1 file changed, 1 deletion(-)

index 9d60d7948b..99e5d943af 100644
@@ -1692,7 +1692,6 @@ sub DESTROY {
 # Pipe implementation for ActiveState Perl.
 
 package Git::activestate_pipe;
-use strict;
 
 sub TIEHANDLE {
 	my ($class, @params) = @_;

2.15.1.424.g9478a66081

Amend Git.pm to load the "warnings" pragma like the rest of the code in perl/ in addition to the existing "strict" pragma. This is considered the bare minimum best practice in Perl.

Ever since this code was introduced in b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24) it's only been using "strict", not "warnings".

This leaves contrib/buildsystems/Generators/{QMake,VCproj}.pm and contrib/mw-to-git/Git/Mediawiki.pm without "use warnings". Amending those would be a sensible follow-up change, but I don't have an easy way to test those so I'm not changing them.

---
 perl/Git.pm | 1 +
 1 file changed, 1 insertion(+)

index 99e5d943af..7ec16e6dde 100644
@@ -9,6 +9,7 @@ package Git;
 
 use 5.008;
 use strict;
+use warnings;
 
 
 BEGIN {

2.15.1.424.g9478a66081

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require Digest::MD5 anymore. It was released with perl v5.7.3[1]

The initial introduction of the dependency in
e9fdd74e53 ("gitweb: (gr)avatar support", 2009-06-30) says as much, this also undoes part of the later 2e9c8789b7 ("gitweb: Mention optional Perl modules in INSTALL", 2011-02-04) since gitweb will always be run on at least 5.8, so there's no need to mention Digest::MD5 as a required module in the documentation, let's instead say that we require perl 5.8.

As with the preceding Net::{SMTP,Domain} change we now unconditionally "use" the module instead.

1. $ corelist Digest::MD5
Data for 2015-02-14
Digest::MD5 was first released with perl v5.7.3

---
 gitweb/INSTALL     |  3 +--
 gitweb/gitweb.perl | 17 ++++-------------
 2 files changed, 5 insertions(+), 15 deletions(-)

index 408f2859d3..a58e6b3c44 100644
@@ -29,12 +29,11 @@ Requirements
 ------------
 
  - Core git tools
- - Perl
+ - Perl 5.8
  - Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.
  - web server
 
 The following optional Perl modules are required for extra features
- - Digest::MD5 - for gravatar support
  - CGI::Fast and FCGI - for running gitweb as FastCGI script
  - HTML::TagCloud - for fancy tag cloud in project list view
  - HTTP::Date or Time::ParseDate - to support If-Modified-Since for feeds

index 2417057f2b..2594a4badb 100755
@@ -20,6 +20,8 @@ use Fcntl ':mode';
 use File::Find qw();
 use File::Basename qw(basename);
 use Time::HiRes qw(gettimeofday tv_interval);
+use Digest::MD5 qw(md5_hex);
+
 binmode STDOUT, ':utf8';
 
 if (!defined($CGI::VERSION) || $CGI::VERSION < 4.08) {

@@ -490,7 +492,6 @@ our %feature = (
 	# Currently available providers are gravatar and picon.
 	# If an unknown provider is specified, the feature is disabled.
 
-	# Gravatar depends on Digest::MD5.
 	# Picon currently relies on the indiana.edu database.
 
 	# To enable system wide have in $GITWEB_CONFIG

@@ -1166,18 +1167,8 @@ sub configure_gitweb_features {
 	our @snapshot_fmts = gitweb_get_feature('snapshot');
 	@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
 
-	# check that the avatar feature is set to a known provider name,
-	# and for each provider check if the dependencies are satisfied.
-	# if the provider name is invalid or the dependencies are not met,
-	# reset $git_avatar to the empty string.
 	our ($git_avatar) = gitweb_get_feature('avatar');
-	if ($git_avatar eq 'gravatar') {
-		$git_avatar = '' unless (eval { require Digest::MD5; 1; });
-	} elsif ($git_avatar eq 'picon') {
-		# no dependencies
-	} else {
-		$git_avatar = '';
-	}
+	$git_avatar = '' unless $git_avatar =~ /^(?:gravatar|picon)$/s;
 
 	our @extra_branch_refs = gitweb_get_feature('extra-branch-refs');
 	@extra_branch_refs = filter_and_validate_refs (@extra_branch_refs);

@@ -2167,7 +2158,7 @@ sub gravatar_url {
 	my $size = shift;
 	$avatar_cache{$email} ||=
 		"//www.gravatar.com/avatar/" .
-			Digest::MD5::md5_hex($email) . "?s=";
+			md5_hex($email) . "?s=";
 	return $avatar_cache{$email} . $size;
 }
 

2.15.1.424.g9478a66081

This is patch 4/13 but the Net::{SMTP,Domain} change doesn't come until 6/13, so saying "preceding" is confusing.

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require File::Temp and File::Spec anymore. They were first released with perl versions v5.6.1 and 5.00405, respectively.

This code was originally added in c14c8ceb13 ("Git.pm: Make File::Spec and File::Temp requirement lazy", 2008-08-15), presumably to make Git.pm work on 5.6.0.

---
 perl/Git.pm | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

index 7ec16e6dde..151b0e7144 100644
@@ -11,6 +11,8 @@ use 5.008;
 use strict;
 use warnings;
 
+use File::Temp ();
+use File::Spec ();
 
 BEGIN {
 

@@ -190,7 +192,6 @@ sub repository {
 		};
 
 		if ($dir) {
-			_verify_require();
 			File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
 			$opts{Repository} = abs_path($dir);
 

@@ -1289,8 +1290,6 @@ sub temp_release {
 sub _temp_cache {
 	my ($self, $name) = _maybe_self(@_);
 
-	_verify_require();
-
 	my $temp_fd = \$TEMP_FILEMAP{$name};
 	if (defined $$temp_fd and $$temp_fd->opened) {
 		if ($TEMP_FILES{$$temp_fd}{locked}) {

@@ -1324,11 +1323,6 @@ sub _temp_cache {
 	$$temp_fd;
 }
 
-sub _verify_require {
-	eval { require File::Temp; require File::Spec; };
-	$@ and throw Error::Simple($@);
-}
-
 =item temp_reset ( FILEHANDLE )
 
 Truncates and resets the position of the C<FILEHANDLE>.

2.15.1.424.g9478a66081

The Net::SMTP and Net::Domain were both first released with perl v5.7.3[1], since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's no reason to conditionally require this anymore.

This conditional loading was initially added in 87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP", 2006-06-01) for Net::SMTP and 134550fe21 ("git-send-email.perl - try to give real name of the calling host to HELO/EHLO", 2010-03-14) for Net::Domain, both of which predate the hard dependency on 5.8.

Since they're guaranteed to be installed now let's "use" them instead. The cost of loading them both is trivial given what git-send-email does (~15ms on my system), and it's better to not defer any potential loading errors until runtime.

This patch is better viewed with -w, which shows that the only change in the last two hunks is removing the "if eval" wrapper block.

1. $ parallel 'corelist {}' ::: Net::{SMTP,Domain} Data for 2015-02-14
Net::SMTP was first released with perl v5.7.3

Data for 2015-02-14
Net::Domain was first released with perl v5.7.3

---
 git-send-email.perl | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

index bbf4deaa0d..d5a4826a1c 100755
@@ -31,6 +31,8 @@ use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
 use Git::Mail::Address;
+use Net::Domain ();
+use Net::SMTP ();
 
 Getopt::Long::Configure qw/ pass_through /;
 

@@ -1143,10 +1145,8 @@ sub valid_fqdn {
 sub maildomain_net {
 	my $maildomain;
 
-	if (eval { require Net::Domain; 1 }) {
-		my $domain = Net::Domain::domainname();
-		$maildomain = $domain if valid_fqdn($domain);
-	}
+	my $domain = Net::Domain::domainname();
+	$maildomain = $domain if valid_fqdn($domain);
 
 	return $maildomain;
 }

@@ -1154,17 +1154,15 @@ sub maildomain_net {
 sub maildomain_mta {
 	my $maildomain;
 
-	if (eval { require Net::SMTP; 1 }) {
-		for my $host (qw(mailhost localhost)) {
-			my $smtp = Net::SMTP->new($host);
-			if (defined $smtp) {
-				my $domain = $smtp->domain;
-				$smtp->quit;
+	for my $host (qw(mailhost localhost)) {
+		my $smtp = Net::SMTP->new($host);
+		if (defined $smtp) {
+			my $domain = $smtp->domain;
+			$smtp->quit;
 
-				$maildomain = $domain if valid_fqdn($domain);
+			$maildomain = $domain if valid_fqdn($domain);
 
-				last if $maildomain;
-			}
+			last if $maildomain;
 		}
 	}
 

2.15.1.424.g9478a66081

s/this/them/ or s/this/these/

The Error.pm shipped with Git as a fallback if there was no Error.pm on the system was released in April 2006. There's been dozens of releases since then, the latest at August 7, 2017. Let's update to that.

I don't know of anything we need from this new release or which this fixes. This change is simply a matter of keeping up with upstream. Before this users who'd install git via their package system would get an up-to-date Error.pm, but if it's installed from source they'd get one more than a decade old.

This undoes a local hack we'd accumulated in 96bc4de85c ("Eliminate Scalar::Util usage from private-Error.pm", 2006-07-26), it's been redundant since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24).

This also undoes 3a51467b94 ("Typo fix: replacing it's -> its", 2013-04-13). This is the Nth time I find that some upstream code of ours (in contrib/, in sha1dc/ and now in perl/ ...) has diverged from upstream because of some tree-wide typo fixing. Let's not do those fixes against upstream projects, it's more valuable that we have a 1=1 mapping to upstream than to fix typos in docs we never even generate from this code. If someone wants to fix typos in them fine, but they should do it with a patch to upstream which git.git can then incorporate.

The upstream code doesn't cleanly pass a --check, so I'm adding a .gitattributes file for similar reasons as done for sha1dc in 5d184f468e ("sha1dc: ignore indent-with-non-tab whitespace violations", 2017-06-06).

The updated source was retrieved from
https://fastapi.metacpan.org/source/SHLOMIF/Error-0.17025/lib/Error.pm

---
 perl/Git/FromCPAN/.gitattributes |   1 +
 perl/Git/FromCPAN/Error.pm       | 296 +++++++++++++++++++++++++++++++++------
 2 files changed, 256 insertions(+), 41 deletions(-)
 create mode 100644 perl/Git/FromCPAN/.gitattributes

new file mode 100644
index 0000000000..8b64fc5e22
@@ -0,0 +1 @@
+/Error.pm whitespace=-blank-at-eof

2.15.1.424.g9478a66081

Update our copy of Mail::Address from 2.19 (Aug 22, 2017) to 2.20 (Jan 23, 2018). Like the preceding Error.pm update this is done simply to keep up-to-date with upstream, and as can be shown from the diff there's no functional changes.

The updated source was retrieved from
https://fastapi.metacpan.org/source/MARKOV/MailTools-2.20/lib/Mail/Address.pm

---
 perl/Git/FromCPAN/Mail/Address.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index 13b2ff7d05..683d490b2b 100644
@@ -1,10 +1,14 @@
-# Copyrights 1995-2017 by [Mark Overmeer <[email protected]>].
+# Copyrights 1995-2018 by [Mark Overmeer].
 #  For other contributors see ChangeLog.
 # See the manual pages for details on the licensing terms.
 # Pod stripped from pm file by OODoc 2.02.
+# This code is part of the bundle MailTools.  Meta-POD processed with
+# OODoc into POD and HTML manual-pages.  See README.md for Copyright.
+# Licensed under the same terms as Perl itself.
+
 package Mail::Address;
 use vars '$VERSION';
-$VERSION = '2.19';
+$VERSION = '2.20';
 
 use strict;
 

2.15.1.424.g9478a66081

Move the Git::Error and Git::Mail::Address wrappers to the Git::LoadCPAN::Loader::* namespace, e.g. Git::LoadCPAN::Error. That module will then either load Error from CPAN (if installed on the OS), or use Git::FromCPAN::Error.

When I added the Error wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) I didn't think about how confusing it would be to have these modules sitting in the same tree as our normal modules. Let's put these all into Git::{Load,From}CPAN::* to clearly distinguish them from the rest.

This also makes things a bit less confusing since there was already a Git::Error namespace ever since 8b9150e3e3 ("Git.pm: Handle failed commands' output", 2006-06-24).

---
 contrib/examples/git-difftool.perl      | 2 +-
 git-send-email.perl                     | 4 ++--
 perl/Git.pm                             | 2 +-
 perl/Git/{ => LoadCPAN}/Error.pm        | 8 ++++----
 perl/Git/{ => LoadCPAN}/Mail/Address.pm | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)
 rename perl/Git/{ => LoadCPAN}/Error.pm (78%)
 rename perl/Git/{ => LoadCPAN}/Mail/Address.pm (69%)

index fb0fd0b84b..b2ea80f9ed 100755
@@ -13,7 +13,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use File::Basename qw(dirname);
 use File::Copy;
 use File::Find;

index d5a4826a1c..1ec22c5ef3 100755
@@ -26,13 +26,13 @@ use Text::ParseWords;
 use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catdir catfile);
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
-use Git::Mail::Address;
 use Net::Domain ();
 use Net::SMTP ();
+use Git::LoadCPAN::Mail::Address;
 
 Getopt::Long::Configure qw/ pass_through /;
 

index 151b0e7144..9f246c7988 100644
@@ -104,7 +104,7 @@ increase notwithstanding).
 
 
 use Carp qw(carp croak); # but croak is bad - throw instead
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use IPC::Open2 qw(open2);
 use Fcntl qw(SEEK_SET SEEK_CUR);

similarity index 78%
rename from perl/Git/Error.pm
rename to perl/Git/LoadCPAN/Error.pm
index 09bbc97390..3513fe745b 100644
@@ -1,11 +1,11 @@
-package Git::Error;
+package Git::LoadCPAN::Error;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Error - Wrapper for the L<Error> module, in case it's not installed
+Git::LoadCPAN::Error - Wrapper for the L<Error> module, in case it's not installed
 
 =head1 DESCRIPTION
 

@@ -26,13 +26,13 @@ sub import {
     } or do {
 	my $error = $@ || "Zombie Error";
 
-	my $Git_Error_pm_path = $INC{"Git/Error.pm"} || die "BUG: Should have our own path from %INC!";
+	my $Git_Error_pm_path = $INC{"Git/LoadCPAN/Error.pm"} || die "BUG: Should have our own path from %INC!";
 
 	require File::Basename;
 	my $Git_Error_pm_root = File::Basename::dirname($Git_Error_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_Error_pm_path'!";
 
 	require File::Spec;
-	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, 'FromCPAN');
+	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, '..', 'FromCPAN');
 	die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
 
 	local @INC = ($Git_pm_FromCPAN_root, @INC);

similarity index 69%
rename from perl/Git/Mail/Address.pm
rename to perl/Git/LoadCPAN/Mail/Address.pm
index 2ce3e84670..879c2f5cd1 100644
@@ -1,11 +1,11 @@
-package Git::Mail::Address;
+package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
+Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
 
 =head1 DESCRIPTION
 

2.15.1.424.g9478a66081

Change the two wrappers that load from CPAN (local OS) or our own copy to do so via the same codepath.

I added the Error.pm wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10), and shortly afterwards Matthieu Moy added a wrapper for Mail::Address in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05).

His loader was simpler since Mail::Address doesn't have an "import" method, but didn't do the same sanity checking; For example, a missing FromCPAN directory (which OS packages are likely not to have) wouldn't be explicitly warned about as a "BUG: ...".

Update both to use a common implementation based on the previous Error.pm loader. Which has been amended to take the module to load as parameter, as well as whether or not that module has an import method.

This loader should be generic enough to handle almost all CPAN modules out there, some use some crazy loading magic and wouldn't like being wrapped like this, but that would be immediately obvious, and we'd find out right away since the module wouldn't work at all.

---
 perl/Git/LoadCPAN.pm              | 74 +++++++++++++++++++++++++++++++++++++++
 perl/Git/LoadCPAN/Error.pm        | 44 +++--------------------
 perl/Git/LoadCPAN/Mail/Address.pm | 22 +++---------
 3 files changed, 82 insertions(+), 58 deletions(-)
 create mode 100644 perl/Git/LoadCPAN.pm

new file mode 100644
index 0000000000..1568c177fe
@@ -0,0 +1,74 @@
+package Git::LoadCPAN;
+use 5.008;
+use strict;
+use warnings;
+
+=head1 NAME
+
+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 distributors will not ship with Git's Git::FromCPAN tree at
+all, 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
+
+sub import {
+	shift;
+	my $caller = caller;
+	my %args = @_;
+	my $module = exists $args{module} ? delete $args{module} : die "BUG: Expected 'module' parameter!";
+	my $import = exists $args{import} ? delete $args{import} : die "BUG: Expected 'import' parameter!";
+	die "BUG: Too many arguments!" if keys %args;
+
+	# Foo::Bar to Foo/Bar.pm
+	my $package_pm = $module;
+	$package_pm =~ s[::][/]g;
+	$package_pm .= '.pm';
+
+	eval {
+		require $package_pm;
+		1;
+	} or do {
+		my $error = $@ || "Zombie Error";
+
+		my $Git_LoadCPAN_pm_path = $INC{"Git/LoadCPAN.pm"} || die "BUG: Should have our own path from %INC!";
+
+		require File::Basename;
+		my $Git_LoadCPAN_pm_root = File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
+
+		require File::Spec;
+		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 'FromCPAN');
+		die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
+
+		local @INC = ($Git_pm_FromCPAN_root, @INC);
+		require $package_pm;
+	};
+
+	if ($import) {
+		no strict 'refs';
+		*{"${caller}::import"} = sub {
+			shift;
+			use strict 'refs';
+			unshift @_, $module;
+			goto &{"${module}::import"};
+		};
+		use strict 'refs';
+	}
+}
+
+1;

index 879c2f5cd1..f70a4f064c 100644
@@ -2,23 +2,9 @@ package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
-
-=head1 NAME
-
-Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
-
-=head1 DESCRIPTION
-
-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
-
-eval {
-    require Mail::Address;
-    1;
-} or do {
-    require Git::FromCPAN::Mail::Address;
-};
+use Git::LoadCPAN (
+	module => 'Mail::Address',
+	import => 0,
+);
 
 1;

2.15.1.424.g9478a66081

Move the CPAN modules that have lived under perl/Git/FromCPAN since my 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) to perl/FromCPAN.

A subsequent change will teach the Makefile to only install these copies of CPAN modules if a flag that distro packagers would like to set isn't set. Due to how the wildcard globbing is being done it's much easier to accomplish that if they're moved to their own directory.

---
 Makefile                                | 3 +++
 perl/{Git => }/FromCPAN/.gitattributes  | 0
 perl/{Git => }/FromCPAN/Error.pm        | 0
 perl/{Git => }/FromCPAN/Mail/Address.pm | 0
 perl/Git/LoadCPAN.pm                    | 5 ++---
 5 files changed, 5 insertions(+), 3 deletions(-)
 rename perl/{Git => }/FromCPAN/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Error.pm (100%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

index c56fdc14ca..7b97699864 100644
@@ -2300,9 +2300,12 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
+LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+all:: $(LIB_CPAN_GEN)
 endif
 
 perl/build/lib/%.pm: perl/%.pm

similarity index 100%
rename from perl/Git/FromCPAN/.gitattributes
rename to perl/FromCPAN/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Error.pm
rename to perl/FromCPAN/Error.pm

similarity index 100%
rename from perl/Git/FromCPAN/Mail/Address.pm
rename to perl/FromCPAN/Mail/Address.pm

index 1568c177fe..229c1ee87d 100644
@@ -16,8 +16,7 @@ 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.
+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.

@@ -52,7 +51,7 @@ sub import {
 		my $Git_LoadCPAN_pm_root = File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
 
 		require File::Spec;
-		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 'FromCPAN');
+		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, '..', 'FromCPAN');
 		die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
 
 		local @INC = ($Git_pm_FromCPAN_root, @INC);

2.15.1.424.g9478a66081

From: Todd Zullinger <[email protected]>

We include some perl modules which are not part of the core perl install, as a convenience. This allows us to rely on those modules in our perl-based tools and scripts without requiring users to install the modules from CPAN or their operating system packages.

Users whose operating system provides these modules and packagers of Git often don't want to ship or use these bundled modules. Allow these users to set NO_PERL_CPAN_FALLBACKS to avoid installing the bundled modules.

---
 Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

index 7b97699864..518c5f6be0 100644
@@ -296,6 +296,12 @@ 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 bundled
+# copies of CPAN modules that serve as a fallback in case the modules
+# are not available on the system. This option is intended for
+# distributions that want to use their packaged versions of Perl
+# modules, instead of the fallbacks shipped with Git.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #

@@ -2305,8 +2311,10 @@ LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
 all:: $(LIB_CPAN_GEN)
 endif
+endif
 
 perl/build/lib/%.pm: perl/%.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \

2.15.1.424.g9478a66081

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.

---
 INSTALL              | 11 ++++++++---
 Makefile             |  5 ++++-
 perl/Git/LoadCPAN.pm | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 5 deletions(-)

index 808e07b659..60e515eaf7 100644
@@ -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

index 518c5f6be0..4e0cdb3ca4 100644
@@ -2314,11 +2314,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 $@) && \

index 229c1ee87d..e5585e75e8 100644
@@ -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

Same as v2 except rebased on master & a couple of commit message fixes, thanks to Eric Sunshine (thanks!). tbdiff with v2:

4: 0d67af28fb ! 4: 8ad874c944 gitweb: hard-depend on the Digest::MD5 5.8 module
@@ -14,9 +14,6 @@
always be run on at least 5.8, so there's no need to mention Digest::MD5 as a required module in the documentation, let's instead say that we require perl 5.8.
-
- As with the preceding Net::{SMTP,Domain} change we now unconditionally
- "use" the module instead.

1. $ corelist Digest::MD5
Data for 2015-02-14
[...]
6: 92429af3a3 ! 6: 3f44312821 git-send-email: unconditionally use Net::{SMTP,Domain}
@@ -5,7 +5,7 @@
The Net::SMTP and Net::Domain were both first released with perl v5.7.3[1], since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's
- no reason to conditionally require this anymore.
+ no reason to conditionally require them anymore.

This conditional loading was initially added in 87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP",

Todd Zullinger (1):
Makefile: add NO_PERL_CPAN_FALLBACKS knob

Ævar Arnfjörð Bjarmason (12):
perl: *.pm files should not have the executable bit Git.pm: remove redundant "use strict" from sub-package Git.pm: add the "use warnings" pragma
gitweb: hard-depend on the Digest::MD5 5.8 module Git.pm: hard-depend on the File::{Temp,Spec} modules
git-send-email: unconditionally use Net::{SMTP,Domain}
perl: update our ancient copy of Error.pm
perl: update our copy of Mail::Address
perl: move CPAN loader wrappers to another namespace
perl: generalize the Git::LoadCPAN facility
perl: move the perl/Git/FromCPAN tree to perl/FromCPAN perl Git::LoadCPAN: emit better errors under NO_PERL_CPAN_FALLBACKS

 INSTALL                                 |  11 +-
 Makefile                                |  16 +-
 contrib/examples/git-difftool.perl      |   2 +-
 git-send-email.perl                     |  28 ++-
 gitweb/INSTALL                          |   3 +-
 gitweb/gitweb.perl                      |  17 +-
 perl/FromCPAN/.gitattributes            |   1 +
 perl/{Git => }/FromCPAN/Error.pm        | 296 +++++++++++++++++++++++++++-----
 perl/{Git => }/FromCPAN/Mail/Address.pm |   8 +-
 perl/Git.pm                             |  14 +-
 perl/Git/Error.pm                       |  46 -----
 perl/Git/LoadCPAN.pm                    | 104 +++++++++++
 perl/Git/LoadCPAN/Error.pm              |  10 ++
 perl/Git/LoadCPAN/Mail/Address.pm       |  10 ++
 perl/Git/Mail/Address.pm                |  24 ---
 15 files changed, 432 insertions(+), 158 deletions(-)
 create mode 100644 perl/FromCPAN/.gitattributes
 rename perl/{Git => }/FromCPAN/Error.pm (72%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (96%)
 delete mode 100644 perl/Git/Error.pm
 create mode 100644 perl/Git/LoadCPAN.pm
 create mode 100644 perl/Git/LoadCPAN/Error.pm
 create mode 100644 perl/Git/LoadCPAN/Mail/Address.pm
 delete mode 100755 perl/Git/Mail/Address.pm

2.15.1.424.g9478a66081

The Git::Mail::Address file added in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05) had the executable bit set. That bit should not be set for *.pm files. It breaks nothing but it is redundant and confusing as none of the other files have it and these files are never executed as stand-alone programs.

---
 perl/Git/Mail/Address.pm | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 => 100644 perl/Git/Mail/Address.pm

old mode 100755
new mode 100644

2.15.1.424.g9478a66081

In Perl the "use strict/warnings" pragmas are lexical, thus there's no reason to do:

    package Foo;
    use strict;
    package Bar;
    use strict;
    $x = 5;

To satisfy the desire that the undeclared $x variable will be spotted at compile-time. It's enough to include the first "use strict".

This functionally changes nothing, but makes a subsequent change where "use warnings" will be added to Git.pm less confusing and less verbose, since as with "strict" we'll only need to do that at the top of the file.

Changes code initially added in a6065b548f ("Git.pm: Try to support ActiveState output pipe", 2006-06-25).

---
 perl/Git.pm | 1 -
 1 file changed, 1 deletion(-)

index 9d60d7948b..99e5d943af 100644
@@ -1692,7 +1692,6 @@ sub DESTROY {
 # Pipe implementation for ActiveState Perl.
 
 package Git::activestate_pipe;
-use strict;
 
 sub TIEHANDLE {
 	my ($class, @params) = @_;

2.15.1.424.g9478a66081

Amend Git.pm to load the "warnings" pragma like the rest of the code in perl/ in addition to the existing "strict" pragma. This is considered the bare minimum best practice in Perl.

Ever since this code was introduced in b1edc53d06 ("Introduce Git.pm (v4)", 2006-06-24) it's only been using "strict", not "warnings".

This leaves contrib/buildsystems/Generators/{QMake,VCproj}.pm and contrib/mw-to-git/Git/Mediawiki.pm without "use warnings". Amending those would be a sensible follow-up change, but I don't have an easy way to test those so I'm not changing them.

---
 perl/Git.pm | 1 +
 1 file changed, 1 insertion(+)

index 99e5d943af..7ec16e6dde 100644
@@ -9,6 +9,7 @@ package Git;
 
 use 5.008;
 use strict;
+use warnings;
 
 
 BEGIN {

2.15.1.424.g9478a66081

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require Digest::MD5 anymore. It was released with perl v5.7.3[1]

The initial introduction of the dependency in
e9fdd74e53 ("gitweb: (gr)avatar support", 2009-06-30) says as much, this also undoes part of the later 2e9c8789b7 ("gitweb: Mention optional Perl modules in INSTALL", 2011-02-04) since gitweb will always be run on at least 5.8, so there's no need to mention Digest::MD5 as a required module in the documentation, let's instead say that we require perl 5.8.

1. $ corelist Digest::MD5
Data for 2015-02-14
Digest::MD5 was first released with perl v5.7.3

---
 gitweb/INSTALL     |  3 +--
 gitweb/gitweb.perl | 17 ++++-------------
 2 files changed, 5 insertions(+), 15 deletions(-)

index 408f2859d3..a58e6b3c44 100644
@@ -29,12 +29,11 @@ Requirements
 ------------
 
  - Core git tools
- - Perl
+ - Perl 5.8
  - Perl modules: CGI, Encode, Fcntl, File::Find, File::Basename.
  - web server
 
 The following optional Perl modules are required for extra features
- - Digest::MD5 - for gravatar support
  - CGI::Fast and FCGI - for running gitweb as FastCGI script
  - HTML::TagCloud - for fancy tag cloud in project list view
  - HTTP::Date or Time::ParseDate - to support If-Modified-Since for feeds

index 2417057f2b..2594a4badb 100755
@@ -20,6 +20,8 @@ use Fcntl ':mode';
 use File::Find qw();
 use File::Basename qw(basename);
 use Time::HiRes qw(gettimeofday tv_interval);
+use Digest::MD5 qw(md5_hex);
+
 binmode STDOUT, ':utf8';
 
 if (!defined($CGI::VERSION) || $CGI::VERSION < 4.08) {

@@ -490,7 +492,6 @@ our %feature = (
 	# Currently available providers are gravatar and picon.
 	# If an unknown provider is specified, the feature is disabled.
 
-	# Gravatar depends on Digest::MD5.
 	# Picon currently relies on the indiana.edu database.
 
 	# To enable system wide have in $GITWEB_CONFIG

@@ -1166,18 +1167,8 @@ sub configure_gitweb_features {
 	our @snapshot_fmts = gitweb_get_feature('snapshot');
 	@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
 
-	# check that the avatar feature is set to a known provider name,
-	# and for each provider check if the dependencies are satisfied.
-	# if the provider name is invalid or the dependencies are not met,
-	# reset $git_avatar to the empty string.
 	our ($git_avatar) = gitweb_get_feature('avatar');
-	if ($git_avatar eq 'gravatar') {
-		$git_avatar = '' unless (eval { require Digest::MD5; 1; });
-	} elsif ($git_avatar eq 'picon') {
-		# no dependencies
-	} else {
-		$git_avatar = '';
-	}
+	$git_avatar = '' unless $git_avatar =~ /^(?:gravatar|picon)$/s;
 
 	our @extra_branch_refs = gitweb_get_feature('extra-branch-refs');
 	@extra_branch_refs = filter_and_validate_refs (@extra_branch_refs);

@@ -2167,7 +2158,7 @@ sub gravatar_url {
 	my $size = shift;
 	$avatar_cache{$email} ||=
 		"//www.gravatar.com/avatar/" .
-			Digest::MD5::md5_hex($email) . "?s=";
+			md5_hex($email) . "?s=";
 	return $avatar_cache{$email} . $size;
 }
 

2.15.1.424.g9478a66081

Since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24), we've depended on 5.8, so there's no reason to conditionally require File::Temp and File::Spec anymore. They were first released with perl versions v5.6.1 and 5.00405, respectively.

This code was originally added in c14c8ceb13 ("Git.pm: Make File::Spec and File::Temp requirement lazy", 2008-08-15), presumably to make Git.pm work on 5.6.0.

---
 perl/Git.pm | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

index 7ec16e6dde..151b0e7144 100644
@@ -11,6 +11,8 @@ use 5.008;
 use strict;
 use warnings;
 
+use File::Temp ();
+use File::Spec ();
 
 BEGIN {
 

@@ -190,7 +192,6 @@ sub repository {
 		};
 
 		if ($dir) {
-			_verify_require();
 			File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;
 			$opts{Repository} = abs_path($dir);
 

@@ -1289,8 +1290,6 @@ sub temp_release {
 sub _temp_cache {
 	my ($self, $name) = _maybe_self(@_);
 
-	_verify_require();
-
 	my $temp_fd = \$TEMP_FILEMAP{$name};
 	if (defined $$temp_fd and $$temp_fd->opened) {
 		if ($TEMP_FILES{$$temp_fd}{locked}) {

@@ -1324,11 +1323,6 @@ sub _temp_cache {
 	$$temp_fd;
 }
 
-sub _verify_require {
-	eval { require File::Temp; require File::Spec; };
-	$@ and throw Error::Simple($@);
-}
-
 =item temp_reset ( FILEHANDLE )
 
 Truncates and resets the position of the C<FILEHANDLE>.

2.15.1.424.g9478a66081

The Net::SMTP and Net::Domain were both first released with perl v5.7.3[1], since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24) we've depended on 5.8, so there's no reason to conditionally require them anymore.

This conditional loading was initially added in 87840620fd ("send-email: only 'require' instead of 'use' Net::SMTP", 2006-06-01) for Net::SMTP and 134550fe21 ("git-send-email.perl - try to give real name of the calling host to HELO/EHLO", 2010-03-14) for Net::Domain, both of which predate the hard dependency on 5.8.

Since they're guaranteed to be installed now let's "use" them instead. The cost of loading them both is trivial given what git-send-email does (~15ms on my system), and it's better to not defer any potential loading errors until runtime.

This patch is better viewed with -w, which shows that the only change in the last two hunks is removing the "if eval" wrapper block.

1. $ parallel 'corelist {}' ::: Net::{SMTP,Domain} Data for 2015-02-14
Net::SMTP was first released with perl v5.7.3

Data for 2015-02-14
Net::Domain was first released with perl v5.7.3

---
 git-send-email.perl | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

index bbf4deaa0d..d5a4826a1c 100755
@@ -31,6 +31,8 @@ use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
 use Git::Mail::Address;
+use Net::Domain ();
+use Net::SMTP ();
 
 Getopt::Long::Configure qw/ pass_through /;
 

@@ -1143,10 +1145,8 @@ sub valid_fqdn {
 sub maildomain_net {
 	my $maildomain;
 
-	if (eval { require Net::Domain; 1 }) {
-		my $domain = Net::Domain::domainname();
-		$maildomain = $domain if valid_fqdn($domain);
-	}
+	my $domain = Net::Domain::domainname();
+	$maildomain = $domain if valid_fqdn($domain);
 
 	return $maildomain;
 }

@@ -1154,17 +1154,15 @@ sub maildomain_net {
 sub maildomain_mta {
 	my $maildomain;
 
-	if (eval { require Net::SMTP; 1 }) {
-		for my $host (qw(mailhost localhost)) {
-			my $smtp = Net::SMTP->new($host);
-			if (defined $smtp) {
-				my $domain = $smtp->domain;
-				$smtp->quit;
+	for my $host (qw(mailhost localhost)) {
+		my $smtp = Net::SMTP->new($host);
+		if (defined $smtp) {
+			my $domain = $smtp->domain;
+			$smtp->quit;
 
-				$maildomain = $domain if valid_fqdn($domain);
+			$maildomain = $domain if valid_fqdn($domain);
 
-				last if $maildomain;
-			}
+			last if $maildomain;
 		}
 	}
 

2.15.1.424.g9478a66081

The Error.pm shipped with Git as a fallback if there was no Error.pm on the system was released in April 2006. There's been dozens of releases since then, the latest at August 7, 2017. Let's update to that.

I don't know of anything we need from this new release or which this fixes. This change is simply a matter of keeping up with upstream. Before this users who'd install git via their package system would get an up-to-date Error.pm, but if it's installed from source they'd get one more than a decade old.

This undoes a local hack we'd accumulated in 96bc4de85c ("Eliminate Scalar::Util usage from private-Error.pm", 2006-07-26), it's been redundant since my d48b284183 ("perl: bump the required Perl version to 5.8 from 5.6.[21]", 2010-09-24).

This also undoes 3a51467b94 ("Typo fix: replacing it's -> its", 2013-04-13). This is the Nth time I find that some upstream code of ours (in contrib/, in sha1dc/ and now in perl/ ...) has diverged from upstream because of some tree-wide typo fixing. Let's not do those fixes against upstream projects, it's more valuable that we have a 1=1 mapping to upstream than to fix typos in docs we never even generate from this code. If someone wants to fix typos in them fine, but they should do it with a patch to upstream which git.git can then incorporate.

The upstream code doesn't cleanly pass a --check, so I'm adding a .gitattributes file for similar reasons as done for sha1dc in 5d184f468e ("sha1dc: ignore indent-with-non-tab whitespace violations", 2017-06-06).

The updated source was retrieved from
https://fastapi.metacpan.org/source/SHLOMIF/Error-0.17025/lib/Error.pm

---
 perl/Git/FromCPAN/.gitattributes |   1 +
 perl/Git/FromCPAN/Error.pm       | 296 +++++++++++++++++++++++++++++++++------
 2 files changed, 256 insertions(+), 41 deletions(-)
 create mode 100644 perl/Git/FromCPAN/.gitattributes

new file mode 100644
index 0000000000..8b64fc5e22
@@ -0,0 +1 @@
+/Error.pm whitespace=-blank-at-eof

2.15.1.424.g9478a66081

Update our copy of Mail::Address from 2.19 (Aug 22, 2017) to 2.20 (Jan 23, 2018). Like the preceding Error.pm update this is done simply to keep up-to-date with upstream, and as can be shown from the diff there's no functional changes.

The updated source was retrieved from
https://fastapi.metacpan.org/source/MARKOV/MailTools-2.20/lib/Mail/Address.pm

---
 perl/Git/FromCPAN/Mail/Address.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index 13b2ff7d05..683d490b2b 100644
@@ -1,10 +1,14 @@
-# Copyrights 1995-2017 by [Mark Overmeer <[email protected]>].
+# Copyrights 1995-2018 by [Mark Overmeer].
 #  For other contributors see ChangeLog.
 # See the manual pages for details on the licensing terms.
 # Pod stripped from pm file by OODoc 2.02.
+# This code is part of the bundle MailTools.  Meta-POD processed with
+# OODoc into POD and HTML manual-pages.  See README.md for Copyright.
+# Licensed under the same terms as Perl itself.
+
 package Mail::Address;
 use vars '$VERSION';
-$VERSION = '2.19';
+$VERSION = '2.20';
 
 use strict;
 

2.15.1.424.g9478a66081

Move the Git::Error and Git::Mail::Address wrappers to the Git::LoadCPAN::Loader::* namespace, e.g. Git::LoadCPAN::Error. That module will then either load Error from CPAN (if installed on the OS), or use Git::FromCPAN::Error.

When I added the Error wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) I didn't think about how confusing it would be to have these modules sitting in the same tree as our normal modules. Let's put these all into Git::{Load,From}CPAN::* to clearly distinguish them from the rest.

This also makes things a bit less confusing since there was already a Git::Error namespace ever since 8b9150e3e3 ("Git.pm: Handle failed commands' output", 2006-06-24).

---
 contrib/examples/git-difftool.perl      | 2 +-
 git-send-email.perl                     | 4 ++--
 perl/Git.pm                             | 2 +-
 perl/Git/{ => LoadCPAN}/Error.pm        | 8 ++++----
 perl/Git/{ => LoadCPAN}/Mail/Address.pm | 4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)
 rename perl/Git/{ => LoadCPAN}/Error.pm (78%)
 rename perl/Git/{ => LoadCPAN}/Mail/Address.pm (69%)

index fb0fd0b84b..b2ea80f9ed 100755
@@ -13,7 +13,7 @@
 use 5.008;
 use strict;
 use warnings;
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use File::Basename qw(dirname);
 use File::Copy;
 use File::Find;

index d5a4826a1c..1ec22c5ef3 100755
@@ -26,13 +26,13 @@ use Text::ParseWords;
 use Term::ANSIColor;
 use File::Temp qw/ tempdir tempfile /;
 use File::Spec::Functions qw(catdir catfile);
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use Git;
 use Git::I18N;
-use Git::Mail::Address;
 use Net::Domain ();
 use Net::SMTP ();
+use Git::LoadCPAN::Mail::Address;
 
 Getopt::Long::Configure qw/ pass_through /;
 

index 151b0e7144..9f246c7988 100644
@@ -104,7 +104,7 @@ increase notwithstanding).
 
 
 use Carp qw(carp croak); # but croak is bad - throw instead
-use Git::Error qw(:try);
+use Git::LoadCPAN::Error qw(:try);
 use Cwd qw(abs_path cwd);
 use IPC::Open2 qw(open2);
 use Fcntl qw(SEEK_SET SEEK_CUR);

similarity index 78%
rename from perl/Git/Error.pm
rename to perl/Git/LoadCPAN/Error.pm
index 09bbc97390..3513fe745b 100644
@@ -1,11 +1,11 @@
-package Git::Error;
+package Git::LoadCPAN::Error;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Error - Wrapper for the L<Error> module, in case it's not installed
+Git::LoadCPAN::Error - Wrapper for the L<Error> module, in case it's not installed
 
 =head1 DESCRIPTION
 

@@ -26,13 +26,13 @@ sub import {
     } or do {
 	my $error = $@ || "Zombie Error";
 
-	my $Git_Error_pm_path = $INC{"Git/Error.pm"} || die "BUG: Should have our own path from %INC!";
+	my $Git_Error_pm_path = $INC{"Git/LoadCPAN/Error.pm"} || die "BUG: Should have our own path from %INC!";
 
 	require File::Basename;
 	my $Git_Error_pm_root = File::Basename::dirname($Git_Error_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_Error_pm_path'!";
 
 	require File::Spec;
-	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, 'FromCPAN');
+	my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_Error_pm_root, '..', 'FromCPAN');
 	die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
 
 	local @INC = ($Git_pm_FromCPAN_root, @INC);

similarity index 69%
rename from perl/Git/Mail/Address.pm
rename to perl/Git/LoadCPAN/Mail/Address.pm
index 2ce3e84670..879c2f5cd1 100644
@@ -1,11 +1,11 @@
-package Git::Mail::Address;
+package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
 
 =head1 NAME
 
-Git::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
+Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
 
 =head1 DESCRIPTION
 

2.15.1.424.g9478a66081

Change the two wrappers that load from CPAN (local OS) or our own copy to do so via the same codepath.

I added the Error.pm wrapper in 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10), and shortly afterwards Matthieu Moy added a wrapper for Mail::Address in bd869f67b9 ("send-email: add and use a local copy of Mail::Address", 2018-01-05).

His loader was simpler since Mail::Address doesn't have an "import" method, but didn't do the same sanity checking; For example, a missing FromCPAN directory (which OS packages are likely not to have) wouldn't be explicitly warned about as a "BUG: ...".

Update both to use a common implementation based on the previous Error.pm loader. Which has been amended to take the module to load as parameter, as well as whether or not that module has an import method.

This loader should be generic enough to handle almost all CPAN modules out there, some use some crazy loading magic and wouldn't like being wrapped like this, but that would be immediately obvious, and we'd find out right away since the module wouldn't work at all.

---
 perl/Git/LoadCPAN.pm              | 74 +++++++++++++++++++++++++++++++++++++++
 perl/Git/LoadCPAN/Error.pm        | 44 +++--------------------
 perl/Git/LoadCPAN/Mail/Address.pm | 22 +++---------
 3 files changed, 82 insertions(+), 58 deletions(-)
 create mode 100644 perl/Git/LoadCPAN.pm

new file mode 100644
index 0000000000..1568c177fe
@@ -0,0 +1,74 @@
+package Git::LoadCPAN;
+use 5.008;
+use strict;
+use warnings;
+
+=head1 NAME
+
+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 distributors will not ship with Git's Git::FromCPAN tree at
+all, 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
+
+sub import {
+	shift;
+	my $caller = caller;
+	my %args = @_;
+	my $module = exists $args{module} ? delete $args{module} : die "BUG: Expected 'module' parameter!";
+	my $import = exists $args{import} ? delete $args{import} : die "BUG: Expected 'import' parameter!";
+	die "BUG: Too many arguments!" if keys %args;
+
+	# Foo::Bar to Foo/Bar.pm
+	my $package_pm = $module;
+	$package_pm =~ s[::][/]g;
+	$package_pm .= '.pm';
+
+	eval {
+		require $package_pm;
+		1;
+	} or do {
+		my $error = $@ || "Zombie Error";
+
+		my $Git_LoadCPAN_pm_path = $INC{"Git/LoadCPAN.pm"} || die "BUG: Should have our own path from %INC!";
+
+		require File::Basename;
+		my $Git_LoadCPAN_pm_root = File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
+
+		require File::Spec;
+		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 'FromCPAN');
+		die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
+
+		local @INC = ($Git_pm_FromCPAN_root, @INC);
+		require $package_pm;
+	};
+
+	if ($import) {
+		no strict 'refs';
+		*{"${caller}::import"} = sub {
+			shift;
+			use strict 'refs';
+			unshift @_, $module;
+			goto &{"${module}::import"};
+		};
+		use strict 'refs';
+	}
+}
+
+1;

index 879c2f5cd1..f70a4f064c 100644
@@ -2,23 +2,9 @@ package Git::LoadCPAN::Mail::Address;
 use 5.008;
 use strict;
 use warnings;
-
-=head1 NAME
-
-Git::LoadCPAN::Mail::Address - Wrapper for the L<Mail::Address> module, in case it's not installed
-
-=head1 DESCRIPTION
-
-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
-
-eval {
-    require Mail::Address;
-    1;
-} or do {
-    require Git::FromCPAN::Mail::Address;
-};
+use Git::LoadCPAN (
+	module => 'Mail::Address',
+	import => 0,
+);
 
 1;

2.15.1.424.g9478a66081

Move the CPAN modules that have lived under perl/Git/FromCPAN since my 20d2a30f8f ("Makefile: replace perl/Makefile.PL with simple make rules", 2017-12-10) to perl/FromCPAN.

A subsequent change will teach the Makefile to only install these copies of CPAN modules if a flag that distro packagers would like to set isn't set. Due to how the wildcard globbing is being done it's much easier to accomplish that if they're moved to their own directory.

---
 Makefile                                | 3 +++
 perl/{Git => }/FromCPAN/.gitattributes  | 0
 perl/{Git => }/FromCPAN/Error.pm        | 0
 perl/{Git => }/FromCPAN/Mail/Address.pm | 0
 perl/Git/LoadCPAN.pm                    | 5 ++---
 5 files changed, 5 insertions(+), 3 deletions(-)
 rename perl/{Git => }/FromCPAN/.gitattributes (100%)
 rename perl/{Git => }/FromCPAN/Error.pm (100%)
 rename perl/{Git => }/FromCPAN/Mail/Address.pm (100%)

index 5ae9616e0a..026b9fb6d6 100644
@@ -2302,9 +2302,12 @@ po/build/locale/%/LC_MESSAGES/git.mo: po/%.po
 
 LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)
 LIB_PERL_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_PERL))
+LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)
+LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+all:: $(LIB_CPAN_GEN)
 endif
 
 perl/build/lib/%.pm: perl/%.pm

similarity index 100%
rename from perl/Git/FromCPAN/.gitattributes
rename to perl/FromCPAN/.gitattributes

similarity index 100%
rename from perl/Git/FromCPAN/Error.pm
rename to perl/FromCPAN/Error.pm

similarity index 100%
rename from perl/Git/FromCPAN/Mail/Address.pm
rename to perl/FromCPAN/Mail/Address.pm

index 1568c177fe..229c1ee87d 100644
@@ -16,8 +16,7 @@ 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.
+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.

@@ -52,7 +51,7 @@ sub import {
 		my $Git_LoadCPAN_pm_root = File::Basename::dirname($Git_LoadCPAN_pm_path) || die "BUG: Can't figure out lib/Git dirname from '$Git_LoadCPAN_pm_path'!";
 
 		require File::Spec;
-		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, 'FromCPAN');
+		my $Git_pm_FromCPAN_root = File::Spec->catdir($Git_LoadCPAN_pm_root, '..', 'FromCPAN');
 		die "BUG: '$Git_pm_FromCPAN_root' should be a directory!" unless -d $Git_pm_FromCPAN_root;
 
 		local @INC = ($Git_pm_FromCPAN_root, @INC);

2.15.1.424.g9478a66081

From: Todd Zullinger <[email protected]>

We include some perl modules which are not part of the core perl install, as a convenience. This allows us to rely on those modules in our perl-based tools and scripts without requiring users to install the modules from CPAN or their operating system packages.

Users whose operating system provides these modules and packagers of Git often don't want to ship or use these bundled modules. Allow these users to set NO_PERL_CPAN_FALLBACKS to avoid installing the bundled modules.

---
 Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

index 026b9fb6d6..cbf0bc2344 100644
@@ -296,6 +296,12 @@ 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 bundled
+# copies of CPAN modules that serve as a fallback in case the modules
+# are not available on the system. This option is intended for
+# distributions that want to use their packaged versions of Perl
+# modules, instead of the fallbacks shipped with Git.
+#
 # Define PYTHON_PATH to the path of your Python binary (often /usr/bin/python
 # but /usr/bin/python2.7 on some platforms).
 #

@@ -2307,8 +2313,10 @@ LIB_CPAN_GEN := $(patsubst perl/%.pm,perl/build/lib/%.pm,$(LIB_CPAN))
 
 ifndef NO_PERL
 all:: $(LIB_PERL_GEN)
+ifndef NO_PERL_CPAN_FALLBACKS
 all:: $(LIB_CPAN_GEN)
 endif
+endif
 
 perl/build/lib/%.pm: perl/%.pm
 	$(QUIET_GEN)mkdir -p $(dir $@) && \

2.15.1.424.g9478a66081

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.

---
 INSTALL              | 11 ++++++++---
 Makefile             |  5 ++++-
 perl/Git/LoadCPAN.pm | 33 ++++++++++++++++++++++++++++++++-
 3 files changed, 44 insertions(+), 5 deletions(-)

index 808e07b659..60e515eaf7 100644
@@ -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

index cbf0bc2344..56fc2d6f08 100644
@@ -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 $@) && \

index 229c1ee87d..e5585e75e8 100644
@@ -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

Noting that the series was rebased is a good thing to do, but a statement of the fact alone without justification makes readers wonder if there was a valid reason why it was rebased, or it was just because the 'master' saw more commits that do not have any impact on what the series does. For the former, saying something like "A recently graduated $name topic added another instance of the same kind of glitch this series addresses, so the series has been rebased to cover that codepath as well." would make it very helpful.

Thanks. Will queue. After scanning it again, I think this is ready for "next".