[PATCH 0/2] fix Perl 5.10.1 compatibility

[PATCH 0/2] fix Perl 5.10.1 compatibility

[Eric Wong]

Apparently, the "delete local" construct I've been using
for hashes is only in Perl 5.12+.  Stop using it, for now.

On a side note, it may be worth switching to Perl 5.12 since
"use v5.12" implies "use strict" and loads one less file...

Eric Wong (2):
  treewide: avoid "delete local" construct on hashes
  lei: avoid needless env passing to subcommands

 lib/PublicInbox/ExtSearchIdx.pm |  3 ++-
 lib/PublicInbox/Import.pm       |  4 ++--
 lib/PublicInbox/LEI.pm          | 10 ++++------
 lib/PublicInbox/LeiMirror.pm    |  6 +++---
 lib/PublicInbox/LeiToMail.pm    |  4 ++--
 lib/PublicInbox/LeiXSearch.pm   |  4 ++--
 lib/PublicInbox/Spawn.pm        |  5 ++---
 lib/PublicInbox/TestCommon.pm   |  9 +++++----
 t/spawn.t                       |  6 ++++++
 9 files changed, 28 insertions(+), 23 deletions(-)

[PATCH 1/2] treewide: avoid "delete local" construct on hashes

[Eric Wong]

Apparently this feature is only in Perl 5.12+, and we're
still on Perl 5.10.
---
 lib/PublicInbox/ExtSearchIdx.pm | 3 ++-
 lib/PublicInbox/Import.pm       | 4 ++--
 lib/PublicInbox/LEI.pm          | 6 ++----
 lib/PublicInbox/Spawn.pm        | 5 ++---
 lib/PublicInbox/TestCommon.pm   | 9 +++++----
 t/spawn.t                       | 6 ++++++
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm
index a4b3bbd5..d0c9c2f7 100644
--- a/lib/PublicInbox/ExtSearchIdx.pm
+++ b/lib/PublicInbox/ExtSearchIdx.pm
@@ -1011,8 +1011,9 @@ sub _watch_commit { # PublicInbox::DS::add_timer callback
 	delete $self->{-commit_timer};
 	eidxq_process($self, $self->{-watch_sync});
 	eidxq_release($self);
-	delete local $self->{-watch_sync}->{-regen_fmt};
+	my $fmt = delete $self->{-watch_sync}->{-regen_fmt};
 	reindex_checkpoint($self, $self->{-watch_sync});
+	$self->{-watch_sync}->{-regen_fmt} = $fmt;
 
 	# call event_step => done unless commit_timer is armed
 	PublicInbox::DS::requeue($self);
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index e803ee74..b8fa5c21 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -21,8 +21,8 @@ use POSIX qw(strftime);
 
 sub default_branch () {
 	state $default_branch = do {
-		delete local $ENV{GIT_CONFIG};
-		my $r = popen_rd([qw(git config --global init.defaultBranch)]);
+		my $r = popen_rd([qw(git config --global init.defaultBranch)],
+				 { GIT_CONFIG => undef });
 		chomp(my $h = <$r> // '');
 		close $r;
 		$h eq '' ? 'refs/heads/master' : $h;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 415a425d..31dbd01f 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -629,13 +629,11 @@ sub lei_mark {
 
 sub _config {
 	my ($self, @argv) = @_;
-	my $env = $self->{env};
-	delete local $env->{GIT_CONFIG};
-	delete local $ENV{GIT_CONFIG};
+	my %env = (%{$self->{env}}, GIT_CONFIG => undef);
 	my $cfg = _lei_cfg($self, 1);
 	my $cmd = [ qw(git config -f), $cfg->{'-f'}, @argv ];
 	my %rdr = map { $_ => $self->{$_} } (0..2);
-	waitpid(spawn($cmd, $env, \%rdr), 0);
+	waitpid(spawn($cmd, \%env, \%rdr), 0);
 }
 
 sub lei_config {
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 00e6829e..fe7aa0a8 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -358,10 +358,9 @@ sub spawn ($;$$) {
 	my $f = which($cmd->[0]) // die "$cmd->[0]: command not found\n";
 	my @env;
 	$opts ||= {};
-
-	my %env = $env ? (%ENV, %$env) : %ENV;
+	my %env = (%ENV, $env ? %$env : ());
 	while (my ($k, $v) = each %env) {
-		push @env, "$k=$v";
+		push @env, "$k=$v" if defined($v);
 	}
 	my $redir = [];
 	for my $child_fd (0..2) {
diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index ca05fa21..fc32b57f 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -483,10 +483,11 @@ SKIP: {
 	require_git(2.6, 1) or skip('git 2.6+ required for lei test', 2);
 	require_mods(qw(json DBD::SQLite Search::Xapian), 2);
 	require PublicInbox::Config;
-	delete local $ENV{XDG_DATA_HOME};
-	delete local $ENV{XDG_CONFIG_HOME};
-	local $ENV{GIT_COMMITTER_EMAIL} = 'lei@example.com';
-	local $ENV{GIT_COMMITTER_NAME} = 'lei user';
+	local %ENV = %ENV;
+	delete $ENV{XDG_DATA_HOME};
+	delete $ENV{XDG_CONFIG_HOME};
+	$ENV{GIT_COMMITTER_EMAIL} = 'lei@example.com';
+	$ENV{GIT_COMMITTER_NAME} = 'lei user';
 	my (undef, $fn, $lineno) = caller(0);
 	my $t = "$fn:$lineno";
 	require PublicInbox::Spawn;
diff --git a/t/spawn.t b/t/spawn.t
index a17b72d9..6168c1f6 100644
--- a/t/spawn.t
+++ b/t/spawn.t
@@ -121,6 +121,12 @@ EOF
 	isnt($?, 0, '$? set properly: '.$?);
 }
 
+{
+	local $ENV{GIT_CONFIG} = '/path/to/this/better/not/exist';
+	my $fh = popen_rd([qw(env)], { GIT_CONFIG => undef });
+	ok(!grep(/^GIT_CONFIG=/, <$fh>), 'GIT_CONFIG clobbered');
+}
+
 { # ->CLOSE vs ->DESTROY waitpid caller distinction
 	my @c;
 	my $fh = popen_rd(['true'], undef, { cb => sub { @c = caller } });

[PATCH 2/2] lei: avoid needless env passing to subcommands

[Eric Wong]

We already localize %ENV before calling dispatch(), so
it's needless overhead in spawn() to be checking env for
undef values in those cases.
---
 lib/PublicInbox/LEI.pm        | 4 ++--
 lib/PublicInbox/LeiMirror.pm  | 6 +++---
 lib/PublicInbox/LeiToMail.pm  | 4 ++--
 lib/PublicInbox/LeiXSearch.pm | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 31dbd01f..019b3152 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -838,8 +838,7 @@ sub poke_mua { # forces terminal MUAs to wake up and hopefully notice new mail
 # caller needs to "-t $self->{1}" to check if tty
 sub start_pager {
 	my ($self) = @_;
-	my $env = $self->{env};
-	my $fh = popen_rd([qw(git var GIT_PAGER)], $env);
+	my $fh = popen_rd([qw(git var GIT_PAGER)]);
 	chomp(my $pager = <$fh> // '');
 	close($fh) or warn "`git var PAGER' error: \$?=$?";
 	return if $pager eq 'cat' || $pager eq '';
@@ -848,6 +847,7 @@ sub start_pager {
 	pipe(my ($r, $wpager)) or return warn "pipe: $!";
 	my $rdr = { 0 => $r, 1 => $self->{1}, 2 => $self->{2} };
 	my $pgr = [ undef, @$rdr{1, 2} ];
+	my $env = $self->{env};
 	if ($self->{sock}) { # lei(1) process runs it
 		delete @$new_env{keys %$env}; # only set iff unset
 		send_exec_cmd($self, [ @$rdr{0..2} ], [$pager], $new_env);
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index f8ca1ee5..65818796 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -32,7 +32,7 @@ sub try_scrape {
 	my $curl = $self->{curl} //= PublicInbox::LeiCurl->new($lei) or return;
 	my $cmd = $curl->for_uri($lei, $uri, '--compressed');
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
-	my $fh = popen_rd($cmd, $lei->{env}, $opt);
+	my $fh = popen_rd($cmd, undef, $opt);
 	my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
 	close($fh) or return $lei->child_error($?, "@$cmd failed");
 
@@ -142,7 +142,7 @@ sub run_reap {
 	my ($lei, $cmd, $opt) = @_;
 	$lei->qerr("# @$cmd");
 	$opt->{pgid} = 0;
-	my $pid = spawn($cmd, $lei->{env}, $opt);
+	my $pid = spawn($cmd, undef, $opt);
 	my $reap = PublicInbox::OnDestroy->new($lei->can('sigint_reap'), $pid);
 	my $err = waitpid($pid, 0) == $pid ? undef : "waitpid @$cmd: $!";
 	@$reap = (); # cancel reap
@@ -205,7 +205,7 @@ sub try_manifest {
 	my $cmd = $curl->for_uri($lei, $uri);
 	$lei->qerr("# @$cmd");
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
-	my ($fh, $pid) = popen_rd($cmd, $lei->{env}, $opt);
+	my ($fh, $pid) = popen_rd($cmd, undef, $opt);
 	my $reap = PublicInbox::OnDestroy->new($lei->can('sigint_reap'), $pid);
 	my $gz = do { local $/; <$fh> } // die "read(curl $uri): $!";
 	close $fh;
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index df813064..d77005fa 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -219,7 +219,7 @@ sub _post_augment_mbox { # open a compressor process
 	my $cmd = zsfx2cmd($zsfx, undef, $lei);
 	my ($r, $w) = @{delete $lei->{zpipe}};
 	my $rdr = { 0 => $r, 1 => $lei->{1}, 2 => $lei->{2} };
-	my $pid = spawn($cmd, $lei->{env}, $rdr);
+	my $pid = spawn($cmd, undef, $rdr);
 	my $pp = gensym;
 	my $dup = bless { "pid.$pid" => $cmd }, ref($lei);
 	$dup->{$_} = $lei->{$_} for qw(2 sock);
@@ -232,7 +232,7 @@ sub _post_augment_mbox { # open a compressor process
 sub decompress_src ($$$) {
 	my ($in, $zsfx, $lei) = @_;
 	my $cmd = zsfx2cmd($zsfx, 1, $lei);
-	popen_rd($cmd, $lei->{env}, { 0 => $in, 2 => $lei->{2} });
+	popen_rd($cmd, undef, { 0 => $in, 2 => $lei->{2} });
 }
 
 sub dup_src ($) {
diff --git a/lib/PublicInbox/LeiXSearch.pm b/lib/PublicInbox/LeiXSearch.pm
index 6dcadf0a..c46aba3b 100644
--- a/lib/PublicInbox/LeiXSearch.pm
+++ b/lib/PublicInbox/LeiXSearch.pm
@@ -215,7 +215,7 @@ sub query_remote_mboxrd {
 	local $0 = "$0 query_remote_mboxrd";
 	local $SIG{TERM} = sub { exit(0) }; # for DESTROY (File::Temp, $reap)
 	my $lei = $self->{lei};
-	my ($opt, $env) = @$lei{qw(opt env)};
+	my $opt = $lei->{opt};
 	my @qform = (q => $lei->{mset_opt}->{qstr}, x => 'm');
 	push(@qform, t => 1) if $opt->{threads};
 	my $verbose = $opt->{verbose};
@@ -241,7 +241,7 @@ sub query_remote_mboxrd {
 		$uri->query_form(@qform);
 		my $cmd = $curl->for_uri($lei, $uri);
 		$lei->qerr("# $cmd");
-		my ($fh, $pid) = popen_rd($cmd, $env, $rdr);
+		my ($fh, $pid) = popen_rd($cmd, undef, $rdr);
 		$reap_curl = PublicInbox::OnDestroy->new($sigint_reap, $pid);
 		$fh = IO::Uncompress::Gunzip->new($fh);
 		PublicInbox::MboxReader->mboxrd($fh, \&each_eml, $self,