From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 004261F9FD for ; Fri, 5 Mar 2021 01:38:29 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH] lei q: fix --import-before default and FIFO output Date: Thu, 4 Mar 2021 18:38:29 -0700 Message-Id: <20210305013829.19135-1-e@80x24.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: commit 6c551bffd75afb41d9b5e4774068abe7e06ed0e7 ("lei q: --import-augment for mbox and mbox.gz") added a check to in _pre_augment_mbox for the option being a ref() to distinguish between default values and user-supplied values (which are non-ref SCALARs from Getopt::Long). However, LeiQuery failed to use a SCALAR ref as the default value, making the check in _pre_augment_mbox useless. We now update LeiQuery to use \1 instead of 1 as the default value so "lei q -f mboxrd ..." to stdout works once again. Unfortunately, testing with redirects pointed to regular files didn't trigger the code paths being updated. Testing with a FIFO revealed further bugs in the FIFO handling code which are also fixed in this commit. We'll also update the $lei->out error message to be less-specific about "stdout" and use the term "output", instead, since LeiToMail replaces stdout for all mbox outputs. --- lib/PublicInbox/LEI.pm | 2 +- lib/PublicInbox/LeiQuery.pm | 2 +- lib/PublicInbox/LeiToMail.pm | 8 ++++++-- t/lei-q-kw.t | 25 +++++++++++++++++++------ 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index 50276a50..50c0a885 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -431,7 +431,7 @@ sub out ($;@) { my $self = shift; return if print { $self->{1} // return } @_; # likely return note_sigpipe($self, 1) if $! == EPIPE; - my $err = "error writing to stdout: $!"; + my $err = "error writing to output: $!"; delete $self->{1}; fail($self, $err); } diff --git a/lib/PublicInbox/LeiQuery.pm b/lib/PublicInbox/LeiQuery.pm index 493a8382..623b92cd 100644 --- a/lib/PublicInbox/LeiQuery.pm +++ b/lib/PublicInbox/LeiQuery.pm @@ -52,7 +52,7 @@ sub lei_q { my $sto = $self->_lei_store(1); my $lse = $sto->search; if (($opt->{'import-remote'} //= 1) | - ($opt->{'import-before'} //= 1)) { + (($opt->{'import-before'} //= \1) ? 1 : 0)) { $sto->write_prepare($self); } if ($opt->{'local'} //= scalar(@only) ? 0 : 1) { diff --git a/lib/PublicInbox/LeiToMail.pm b/lib/PublicInbox/LeiToMail.pm index 1e2060fe..13b4f672 100644 --- a/lib/PublicInbox/LeiToMail.pm +++ b/lib/PublicInbox/LeiToMail.pm @@ -223,8 +223,6 @@ sub _post_augment_mbox { # open a compressor process $dup->{$_} = $lei->{$_} for qw(2 sock); tie *$pp, 'PublicInbox::ProcessPipe', $pid, $w, \&reap_compress, $dup; $lei->{1} = $pp; - die 'BUG: unexpected {ovv}->{lock_path}' if $lei->{ovv}->{lock_path}; - $lei->{ovv}->ovv_out_lk_init; } sub decompress_src ($$$) { @@ -495,6 +493,7 @@ sub _pre_augment_mbox { my $out = $lei->{1}; if ($dst ne '/dev/stdout') { if (-p $dst) { + $out = undef; open $out, '>', $dst or die "open($dst): $!"; } elsif (-f _ || !-e _) { require PublicInbox::MboxLock; @@ -521,6 +520,11 @@ sub _pre_augment_mbox { if (($self->{zsfx}) = ($dst =~ /\.($zsfx_allow)\z/)) { pipe(my ($r, $w)) or die "pipe: $!"; $lei->{zpipe} = [ $r, $w ]; + $lei->{ovv}->{lock_path} and + die 'BUG: unexpected {ovv}->{lock_path}'; + $lei->{ovv}->ovv_out_lk_init; + } elsif (!$self->{seekable} && !$lei->{ovv}->{lock_path}) { + $lei->{ovv}->ovv_out_lk_init; } $lei->{1} = $out; undef; diff --git a/t/lei-q-kw.t b/t/lei-q-kw.t index 9daeb5b1..917a2c53 100644 --- a/t/lei-q-kw.t +++ b/t/lei-q-kw.t @@ -7,7 +7,15 @@ use Fcntl qw(SEEK_SET O_RDONLY O_NONBLOCK); use IO::Uncompress::Gunzip qw(gunzip); use IO::Compress::Gzip qw(gzip); use PublicInbox::MboxReader; +use PublicInbox::LeiToMail; use PublicInbox::Spawn qw(popen_rd); +my $exp = { + '' => eml_load('t/plack-qp.eml'), + '' => eml_load('t/utf8.eml'), +}; +$exp->{''}->header_set('Status', 'OR'); +$exp->{''}->header_set('Status', 'O'); + test_lei(sub { lei_ok(qw(import -F eml t/plack-qp.eml)); my $o = "$ENV{HOME}/dst"; @@ -42,6 +50,17 @@ SKIP: { ok(!lei(qw(q --import-before bogus -o), "mboxrd:$o"), '--import-before fails on non-seekable output'); is(do { local $/; <$cat> }, '', 'no output on FIFO'); + close $cat; + $cat = popen_rd(['cat', $o]); + lei_ok(qw(q m:qp@example.com -o), "mboxrd:$o"); + my $buf = do { local $/; <$cat> }; + open my $fh, '<', \$buf or BAIL_OUT $!; + PublicInbox::MboxReader->mboxrd($fh, sub { + my ($eml) = @_; + $eml->header_set('Status', 'OR'); + is_deeply($eml, $exp->{''}, + 'FIFO output works as expected'); + }); }; lei_ok qw(import -F eml t/utf8.eml), \'for augment test'; @@ -66,12 +85,6 @@ my $write_file = sub { } }; -my $exp = { - '' => eml_load('t/plack-qp.eml'), - '' => eml_load('t/utf8.eml'), -}; -$exp->{''}->header_set('Status', 'OR'); -$exp->{''}->header_set('Status', 'O'); for my $sfx ('', '.gz') { $o = "$ENV{HOME}/dst.mboxrd$sfx"; lei_ok(qw(q -o), "mboxrd:$o", qw(m:qp@example.com));