perl: check for perl warnings while running tests

We set "use warnings" in most of our perl code to catch problems. But as the name implies, warnings just emit a message to stderr and don't otherwise affect the program. So our tests are quite likely to miss that warnings are being spewed, as most of them do not look at stderr. We could ask perl to make all warnings fatal, but this is likely annoying for non-developers, who would rather have a running program with a warning than something that refuses to work at all. So instead, let's teach the perl code to respect an environment variable (GIT_PERL_FATAL_WARNINGS) to increase the severity of the warnings. This can be set for day-to-day running if people want to be really pedantic, but the primary use is to trigger it within the test suite. We could also trigger that for every test run, but likewise even the tests failing may be annoying to distro builders, etc (just as -Werror would be for compiling C code). So we'll tie it to a special test-mode variable (GIT_TEST_PERL_FATAL_WARNINGS) that can be set in the environment or as a Makefile knob, and we'll automatically turn the knob when DEVELOPER=1 is set. That should give developers and CI the more careful view without disrupting normal users or packagers. Note that the mapping from the GIT_TEST_* form to the GIT_* form in test-lib.sh is necessary even if they had the same name: the perl scripts need it to be normalized to a perl truth value, and we also have to make sure it's exported (we might have gotten it from the environment, but we might also have gotten it from GIT-BUILD-OPTIONS directly). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 21, 2020 at 23:24 UTC 5338ed2b26ebf91de246c59e525ad254dcbc7a84
22 files changed +30 -19
Makefile
+3
@@ -2767,6 +2767,9 @@ ifdef GIT_INTEROP_MAKE_OPTS
2767 endif
2768 ifdef GIT_TEST_INDEX_VERSION
2769 @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
2770 +endif
2771 +ifdef GIT_TEST_PERL_FATAL_WARNINGS
2772 + @echo GIT_TEST_PERL_FATAL_WARNINGS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_PERL_FATAL_WARNINGS)))'\' >>$@+
2773 endif
2774 @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
2775
config.mak.dev
+2
@@ -45,3 +45,5 @@ ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)
45 DEVELOPER_CFLAGS += -Wno-uninitialized
46 endif
47 endif
48 +
49 +GIT_TEST_PERL_FATAL_WARNINGS = YesPlease
git-svn.perl
+1 -1
@@ -2,7 +2,7 @@
2 # Copyright (C) 2006, Eric Wong <normalperson@yhbt.net>
3 # License: GPL v2 or later
4 use 5.008;
5 -use warnings;
5 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
6 use strict;
7 use vars qw/ $AUTHOR $VERSION
8 $oid $oid_short $oid_length
perl/FromCPAN/Error.pm
+1 -1
@@ -12,7 +12,7 @@
12 package Error;
13
14 use strict;
15 -use warnings;
15 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
16
17 use vars qw($VERSION);
18 use 5.004;
perl/Git.pm
+1 -1
@@ -9,7 +9,7 @@ package Git;
9
10 use 5.008;
11 use strict;
12 -use warnings;
12 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
13
14 use File::Temp ();
15 use File::Spec ();
perl/Git/I18N.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::I18N;
2 use 5.008;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 BEGIN {
6 require Exporter;
7 if ($] < 5.008003) {
perl/Git/IndexInfo.pm
+1 -1
@@ -1,6 +1,6 @@
1 package Git::IndexInfo;
2 use strict;
3 -use warnings;
3 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
4 use Git qw/command_input_pipe command_close_pipe/;
5
6 sub new {
perl/Git/LoadCPAN.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::LoadCPAN;
2 use 5.008;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5
6 =head1 NAME
7
perl/Git/LoadCPAN/Error.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::LoadCPAN::Error;
2 use 5.008;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 use Git::LoadCPAN (
6 module => 'Error',
7 import => 1,
perl/Git/LoadCPAN/Mail/Address.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::LoadCPAN::Mail::Address;
2 use 5.008;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 use Git::LoadCPAN (
6 module => 'Mail::Address',
7 import => 0,
perl/Git/Packet.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::Packet;
2 use 5.008;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 BEGIN {
6 require Exporter;
7 if ($] < 5.008003) {
perl/Git/SVN.pm
+1 -1
@@ -1,6 +1,6 @@
1 package Git::SVN;
2 use strict;
3 -use warnings;
3 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
4 use Fcntl qw/:DEFAULT :seek/;
5 use constant rev_map_fmt => 'NH*';
6 use vars qw/$_no_metadata
perl/Git/SVN/Editor.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::SVN::Editor;
2 use vars qw/@ISA $_rmdir $_cp_similarity $_find_copies_harder $_rename_limit/;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 use SVN::Core;
6 use SVN::Delta;
7 use Carp qw/croak/;
perl/Git/SVN/Fetcher.pm
+1 -1
@@ -3,7 +3,7 @@ use vars qw/@ISA $_ignore_regex $_include_regex $_preserve_empty_dirs
3 $_placeholder_filename @deleted_gpath %added_placeholder
4 $repo_id/;
5 use strict;
6 -use warnings;
6 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
7 use SVN::Delta;
8 use Carp qw/croak/;
9 use File::Basename qw/dirname/;
perl/Git/SVN/GlobSpec.pm
+1 -1
@@ -1,6 +1,6 @@
1 package Git::SVN::GlobSpec;
2 use strict;
3 -use warnings;
3 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
4
5 sub new {
6 my ($class, $glob, $pattern_ok) = @_;
perl/Git/SVN/Log.pm
+1 -1
@@ -1,6 +1,6 @@
1 package Git::SVN::Log;
2 use strict;
3 -use warnings;
3 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
4 use Git::SVN::Utils qw(fatal);
5 use Git qw(command
6 command_oneline
perl/Git/SVN/Memoize/YAML.pm
+1 -1
@@ -1,5 +1,5 @@
1 package Git::SVN::Memoize::YAML;
2 -use warnings;
2 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
3 use strict;
4 use YAML::Any ();
5
perl/Git/SVN/Migration.pm
+1 -1
@@ -33,7 +33,7 @@ package Git::SVN::Migration;
33 # possible if noMetadata or useSvmProps are set; but should
34 # be no problem for users that use the (sensible) defaults.
35 use strict;
36 -use warnings;
36 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
37 use Carp qw/croak/;
38 use File::Path qw/mkpath/;
39 use File::Basename qw/dirname basename/;
perl/Git/SVN/Prompt.pm
+1 -1
@@ -1,6 +1,6 @@
1 package Git::SVN::Prompt;
2 use strict;
3 -use warnings;
3 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
4 require SVN::Core;
5 use vars qw/$_no_auth_cache $_username/;
6
perl/Git/SVN/Ra.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::SVN::Ra;
2 use vars qw/@ISA $config_dir $_ignore_refs_regex $_log_window_size/;
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5 use Memoize;
6 use Git::SVN::Utils qw(
7 canonicalize_url
perl/Git/SVN/Utils.pm
+1 -1
@@ -1,7 +1,7 @@
1 package Git::SVN::Utils;
2
3 use strict;
4 -use warnings;
4 +use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
5
6 use SVN::Core;
7
t/test-lib.sh
+6
@@ -499,6 +499,12 @@ then
499 export GIT_INDEX_VERSION
500 fi
501
502 +if test -n "$GIT_TEST_PERL_FATAL_WARNINGS"
503 +then
504 + GIT_PERL_FATAL_WARNINGS=1
505 + export GIT_PERL_FATAL_WARNINGS
506 +fi
507 +
508 # Add libc MALLOC and MALLOC_PERTURB test
509 # only if we are not executing the test with valgrind
510 if test -n "$valgrind" ||