* [PATCH 1/4] lei blob|rediff: fix usage of lei->fail
2023-09-22 21:13 [PATCH 0/4] small lei fixes Eric Wong
@ 2023-09-22 21:13 ` Eric Wong
2023-09-22 21:13 ` [PATCH 2/4] lei: improve ->fail internal API Eric Wong
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-09-22 21:13 UTC (permalink / raw)
To: meta
lei->fail only takes one message argument, presently;
but it's probably a good idea to change the API...
---
lib/PublicInbox/LeiBlob.pm | 2 +-
lib/PublicInbox/LeiRediff.pm | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 1692289c..5fc6d902 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -158,7 +158,7 @@ sub lei_blob {
if ($lxs->remotes) {
require PublicInbox::LeiRemote;
$lei->{curl} //= which('curl') or return
- $lei->fail('curl needed for', $lxs->remotes);
+ $lei->fail('curl needed for '.join(', ',$lxs->remotes));
$lei->_lei_store(1)->write_prepare($lei);
}
require PublicInbox::SolverGit;
diff --git a/lib/PublicInbox/LeiRediff.pm b/lib/PublicInbox/LeiRediff.pm
index c312d90f..9cf95c08 100644
--- a/lib/PublicInbox/LeiRediff.pm
+++ b/lib/PublicInbox/LeiRediff.pm
@@ -268,7 +268,7 @@ sub lei_rediff {
if ($lxs->remotes) {
require PublicInbox::LeiRemote;
$lei->{curl} //= which('curl') or return
- $lei->fail('curl needed for', $lxs->remotes);
+ $lei->fail('curl needed for '.join(', ',$lxs->remotes));
}
$lei->ale->refresh_externals($lxs, $lei);
my $self = bless {
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] lei: improve ->fail internal API
2023-09-22 21:13 [PATCH 0/4] small lei fixes Eric Wong
2023-09-22 21:13 ` [PATCH 1/4] lei blob|rediff: fix usage of lei->fail Eric Wong
@ 2023-09-22 21:13 ` Eric Wong
2023-09-22 21:13 ` [PATCH 3/4] lei_to_mail: drop awkward duplication of $lei object Eric Wong
2023-09-22 21:13 ` [PATCH 4/4] lei: use File::Temp for listing saved searches Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-09-22 21:13 UTC (permalink / raw)
To: meta
Allow the exit code to be the first argument intead of the last
to match our ->child_error, as well as the BSD err(3) API.
We'll also avoid shifting user-passed exit codes so $? can be
passed as-is without losing signal information.
---
lib/PublicInbox/LEI.pm | 19 ++++++++++++-------
lib/PublicInbox/LeiToMail.pm | 2 +-
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index c61ce76d..368f9357 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -29,6 +29,7 @@ use File::Path ();
use File::Spec;
use Carp ();
use Sys::Syslog qw(openlog syslog closelog);
+use Scalar::Util qw(looks_like_number);
our $quit = \&CORE::exit;
our ($current_lei, $errors_log, $listener, $oldset, $dir_idle);
my $GLP = Getopt::Long::Parser->new;
@@ -518,13 +519,17 @@ sub sigpipe_handler { # handles SIGPIPE from @WQ_KEYS workers
fail_handler($_[0], 13, delete $_[0]->{1});
}
-sub fail ($$;$) {
- my ($self, $msg, $exit_code) = @_;
- local $current_lei = $self;
- $self->{failed}++;
- warn(substr($msg, -1, 1) eq "\n" ? $msg : "$msg\n") if defined $msg;
- $self->{pkt_op_p}->pkt_do('fail_handler') if $self->{pkt_op_p};
- x_it($self, ($exit_code // 1) << 8);
+sub fail ($;@) {
+ my ($lei, @msg) = @_;
+ my $exit_code = looks_like_number($msg[0]) ? shift(@msg) : undef;
+ local $current_lei = $lei;
+ $lei->{failed}++;
+ if (@msg) {
+ push @msg, "\n" if substr($msg[-1], -1, 1);
+ warn @msg;
+ }
+ $lei->{pkt_op_p}->pkt_do('fail_handler') if $lei->{pkt_op_p};
+ x_it($lei, $exit_code // (1 << 8));
undef;
}
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index e357ee00..7c7967c8 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -154,7 +154,7 @@ sub reap_compress { # awaitpid callback
my ($pid, $lei) = @_;
my $cmd = delete $lei->{"pid.$pid"};
return if $? == 0;
- $lei->fail("@$cmd failed", $? >> 8);
+ $lei->fail($?, "@$cmd failed");
}
sub _post_augment_mbox { # open a compressor process from top-level process
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] lei_to_mail: drop awkward duplication of $lei object
2023-09-22 21:13 [PATCH 0/4] small lei fixes Eric Wong
2023-09-22 21:13 ` [PATCH 1/4] lei blob|rediff: fix usage of lei->fail Eric Wong
2023-09-22 21:13 ` [PATCH 2/4] lei: improve ->fail internal API Eric Wong
@ 2023-09-22 21:13 ` Eric Wong
2023-09-22 21:13 ` [PATCH 4/4] lei: use File::Temp for listing saved searches Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-09-22 21:13 UTC (permalink / raw)
To: meta
Our awaitpid API now exists and ProcessPipe uses it, so it's
immune to cyclic references. Thus there's no need to create
a duplicate of the lei object to prevent leaks.
---
lib/PublicInbox/LeiToMail.pm | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm
index 7c7967c8..4adcc33e 100644
--- a/lib/PublicInbox/LeiToMail.pm
+++ b/lib/PublicInbox/LeiToMail.pm
@@ -151,10 +151,9 @@ sub git_to_mail { # git->cat_async callback
}
sub reap_compress { # awaitpid callback
- my ($pid, $lei) = @_;
- my $cmd = delete $lei->{"pid.$pid"};
- return if $? == 0;
- $lei->fail($?, "@$cmd failed");
+ my ($pid, $lei, $cmd, $old_out) = @_;
+ $lei->{1} = $old_out;
+ $lei->fail($?, "@$cmd failed") if $?;
}
sub _post_augment_mbox { # open a compressor process from top-level process
@@ -165,9 +164,8 @@ sub _post_augment_mbox { # open a compressor process from top-level process
my $rdr = { 0 => $r, 1 => $lei->{1}, 2 => $lei->{2}, pgid => 0 };
my $pid = spawn($cmd, undef, $rdr);
my $pp = gensym;
- my $dup = bless { "pid.$pid" => $cmd }, ref($lei);
- $dup->{$_} = $lei->{$_} for qw(2 sock);
- tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w, \&reap_compress, $dup;
+ tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w,
+ \&reap_compress, $lei, $cmd, $lei->{1};
$lei->{1} = $pp;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] lei: use File::Temp for listing saved searches
2023-09-22 21:13 [PATCH 0/4] small lei fixes Eric Wong
` (2 preceding siblings ...)
2023-09-22 21:13 ` [PATCH 3/4] lei_to_mail: drop awkward duplication of $lei object Eric Wong
@ 2023-09-22 21:13 ` Eric Wong
3 siblings, 0 replies; 5+ messages in thread
From: Eric Wong @ 2023-09-22 21:13 UTC (permalink / raw)
To: meta
I have no idea how badly my brain malfunctioned here when I
wrote the code to create a temporary file without O_EXCL :x
I'm still not sure if users have enough saved searches for
justifying a cache, here.
---
lib/PublicInbox/LeiSavedSearch.pm | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm
index e5396342..2811c46d 100644
--- a/lib/PublicInbox/LeiSavedSearch.pm
+++ b/lib/PublicInbox/LeiSavedSearch.pm
@@ -3,8 +3,7 @@
# pretends to be like LeiDedupe and also PublicInbox::Inbox
package PublicInbox::LeiSavedSearch;
-use strict;
-use v5.10.1;
+use v5.12;
use parent qw(PublicInbox::Lock);
use PublicInbox::Git;
use PublicInbox::OverIdx;
@@ -14,6 +13,8 @@ use PublicInbox::Spawn qw(run_die);
use PublicInbox::ContentHash qw(git_sha);
use PublicInbox::MID qw(mids_for_index);
use PublicInbox::SHA qw(sha256_hex);
+use File::Temp ();
+use IO::Handle ();
our $LOCAL_PFX = qr!\A(?:maildir|mh|mbox.+|mmdf|v2):!i; # TODO: put in LeiToMail?
# move this to PublicInbox::Config if other things use it:
@@ -76,20 +77,17 @@ sub list {
my $lss_dir = $lei->share_path.'/saved-searches';
return () unless -d $lss_dir;
# TODO: persist the cache? Use another format?
- my $f = $lei->cache_dir."/saved-tmp.$$.".time.'.config';
- open my $fh, '>', $f or die "open $f: $!";
+ my $fh = File::Temp->new(TEMPLATE => 'lss_list-XXXX', TMPDIR => 1) or
+ die "File::Temp->new: $!";
print $fh "[include]\n";
for my $p (glob("$lss_dir/*/lei.saved-search")) {
print $fh "\tpath = ", cquote_val($p), "\n";
}
- close $fh or die "close $f: $!";
- my $cfg = $lei->cfg_dump($f);
- unlink($f);
+ $fh->flush or die "flush: $fh";
+ my $cfg = $lei->cfg_dump($fh->filename);
my $out = $cfg ? $cfg->get_all('lei.q.output') : [];
- map {;
- s!$LOCAL_PFX!!;
- $_;
- } @$out
+ s!$LOCAL_PFX!! for @$out;;
+ @$out;
}
sub translate_dedupe ($$) {
^ permalink raw reply related [flat|nested] 5+ messages in thread