user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
* latest make test failures on CentOS-7
@ 2021-09-27 12:40 Konstantin Ryabitsev
  2021-09-27 18:35 ` [PATCH] t/cmd_ipc: allow extra errors and add diagnostics Eric Wong
  2021-09-27 19:33 ` -fetch failures [was: latest make test failures on CentOS-7] Eric Wong
  0 siblings, 2 replies; 11+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-27 12:40 UTC (permalink / raw)
  To: meta

Hello:

I wanted to try the searchable /all/ from www_index, but it looks like I'm
unable to get a clean make test. Below are a few failures that I can see:

    t/cmd_ipc.t .................. 29/?
    #   Failed test 'got EMSGSIZE'
    #   at t/cmd_ipc.t line 108.
    # Looks like you failed 1 test of 42.
    t/cmd_ipc.t .................. Dubious, test returned 1 (wstat 256, 0x100)
    Failed 1/42 subtests
            (less 13 skipped subtests: 28 okay)

[...]

    t/lei-index.t ................ 11/?
    #   Failed test 'lei index imap://$HOST_PORT/t.v2.0'
    #   at /usr/local/share/public-inbox/blib/lib/PublicInbox/TestCommon.pm line 519.
    # $?=6400 err=E: eval-ed lei: Mail::IMAPClient is required for IMAP:
    # Can't locate Mail/IMAPClient.pm in @INC (@INC contains: /root/.cache/public-inbox/inline-c/lib /root/.cache/public-inbox/inline-c/lib /usr/local/share/public-inbox/blib/lib /usr/local/share/public-inbox/blib/arch /usr/local/lib64/perl5 /usr/local/share/perl5/x86_64-linux-thread-multi /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 . /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at /usr/share/perl5/vendor_perl/parent.pm line 20.
    # BEGIN failed--compilation aborted at /usr/local/share/public-inbox/blib/lib/PublicInbox/IMAPClient.pm line 13.
    # Compilation failed in require at /usr/local/share/public-inbox/blib/lib/PublicInbox/NetReader.pm line 433.
    #
    #
    # Looks like you failed 1 test of 36.
    t/lei-index.t ................ Dubious, test returned 1 (wstat 256, 0x100)
    Failed 1/36 subtests

(Normal, I don't have Mail::IMAPClient, but should probably fail differently.)

    t/lei-tag.t .................. 23/?
    #   Failed test 'lei _complete lei tag'
    #   at /usr/local/share/public-inbox/blib/lib/PublicInbox/TestCommon.pm line 519.
    # $?=6400 err=E: eval-ed lei: Modification of non-creatable array value attempted, subscript -1 at /usr/local/share/public-inbox/blib/lib/PublicInbox/LeiImport.pm line 125.
    #

    #   Failed test 'completed with labels'
    #   at t/lei-tag.t line 73.
    # Looks like you failed 2 tests of 65.
    t/lei-tag.t .................. Dubious, test returned 2 (wstat 512, 0x200)
    Failed 2/65 subtests
    t/lei-up.t ................... ok
    t/lei-watch.t ................ ok
    t/lei.t ...................... 43/?
    #   Failed test 'lei _complete lei import'
    #   at /usr/local/share/public-inbox/blib/lib/PublicInbox/TestCommon.pm line 519.
    # $?=6400 err=E: eval-ed lei: Modification of non-creatable array value attempted, subscript -1 at /usr/local/share/public-inbox/blib/lib/PublicInbox/LeiImport.pm line 125.
    #
    # Looks like you failed 1 test of 151.
    t/lei.t ...................... Dubious, test returned 1 (wstat 256, 0x100)
    Failed 1/151 subtests

[...]

    t/v2mirror.t ................. 71/? W: /tmp/pi-v2mirror-39373-Dl1N/m/git/3.git missing remote.origin.url
    fatal: not a git repository: '/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git'
    git --git-dir=/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git fetch -q failed
    Bailout called.  Further testing stopped:  -fetch failed
    FAILED--Further testing stopped: -fetch failed
    make: *** [test_dynamic] Error 255

FYI, this is git 2.31.1.

I'll be happy to help troubleshoot things as necessary.

-K

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] t/cmd_ipc: allow extra errors and add diagnostics
  2021-09-27 12:40 latest make test failures on CentOS-7 Konstantin Ryabitsev
@ 2021-09-27 18:35 ` Eric Wong
  2021-09-27 18:51   ` Konstantin Ryabitsev
  2021-09-27 19:33 ` -fetch failures [was: latest make test failures on CentOS-7] Eric Wong
  1 sibling, 1 reply; 11+ messages in thread
From: Eric Wong @ 2021-09-27 18:35 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: meta

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
>     t/cmd_ipc.t .................. 29/?
>     #   Failed test 'got EMSGSIZE'
>     #   at t/cmd_ipc.t line 108.
>     # Looks like you failed 1 test of 42.
>     t/cmd_ipc.t .................. Dubious, test returned 1 (wstat 256, 0x100)
>     Failed 1/42 subtests
>             (less 13 skipped subtests: 28 okay)

I think this is either caused by too much RAM and/or
/proc/sys/net/core/wmem_* being larger-than-expected
(both default to 212992 for me).

This fixes failures when I set both wmem_max and wmem_default
to 2129920 (10x its default value).

-------------8<-----------
Subject: [PATCH] t/cmd_ipc: allow extra errors and add diagnostics

Apparently, sendmsg can fail in less common ways when
network buffers are gigantic.  Add some diagnostics for
future failures, as well.

Link: https://public-inbox.org/meta/20210927124056.kj5okiefvs4ztk27@meerkat.local/
---
 t/cmd_ipc.t | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/t/cmd_ipc.t b/t/cmd_ipc.t
index c5e715a1cd8a..dd90fa2a63e4 100644
--- a/t/cmd_ipc.t
+++ b/t/cmd_ipc.t
@@ -85,7 +85,9 @@ my $do_test = sub { SKIP: {
 			$nsent += $n;
 			fail "sent 0 bytes" if $n == 0;
 		}
-		ok($!{EAGAIN}, "hit EAGAIN on send $desc");
+		ok($!{EAGAIN} || $!{ETOOMANYREFS},
+			"hit EAGAIN || ETOOMANYREFS on send $desc") or
+			diag "send failed with: $!";
 		ok($nsent > 0, 'sent some bytes');
 
 		socketpair($s1, $s2, AF_UNIX, $type, 0) or BAIL_OUT $!;
@@ -105,8 +107,9 @@ my $do_test = sub { SKIP: {
 				diag "sent $nr, retrying with more";
 				$nr += 2 * 1024 * 1024;
 			} else {
-				ok($!{EMSGSIZE}, 'got EMSGSIZE');
-				# diag "$nr bytes hits EMSGSIZE";
+				ok($!{EMSGSIZE} || $!{ENOBUFS},
+					'got EMSGSIZE or ENOBUFS') or
+					diag "$nr bytes fails with: $!";
 				last;
 			}
 		}

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] t/cmd_ipc: allow extra errors and add diagnostics
  2021-09-27 18:35 ` [PATCH] t/cmd_ipc: allow extra errors and add diagnostics Eric Wong
@ 2021-09-27 18:51   ` Konstantin Ryabitsev
  0 siblings, 0 replies; 11+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-27 18:51 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

On Mon, Sep 27, 2021 at 01:35:36PM -0500, Eric Wong wrote:
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> >     t/cmd_ipc.t .................. 29/?
> >     #   Failed test 'got EMSGSIZE'
> >     #   at t/cmd_ipc.t line 108.
> >     # Looks like you failed 1 test of 42.
> >     t/cmd_ipc.t .................. Dubious, test returned 1 (wstat 256, 0x100)
> >     Failed 1/42 subtests
> >             (less 13 skipped subtests: 28 okay)
> 
> I think this is either caused by too much RAM and/or
> /proc/sys/net/core/wmem_* being larger-than-expected
> (both default to 212992 for me).
> 
> This fixes failures when I set both wmem_max and wmem_default
> to 2129920 (10x its default value).

Confirmed:

t/cmd_ipc.t .................. ok

Thanks,
-K

^ permalink raw reply	[flat|nested] 11+ messages in thread

* -fetch failures [was: latest make test failures on CentOS-7]
  2021-09-27 12:40 latest make test failures on CentOS-7 Konstantin Ryabitsev
  2021-09-27 18:35 ` [PATCH] t/cmd_ipc: allow extra errors and add diagnostics Eric Wong
@ 2021-09-27 19:33 ` Eric Wong
  2021-09-27 19:45   ` Konstantin Ryabitsev
  1 sibling, 1 reply; 11+ messages in thread
From: Eric Wong @ 2021-09-27 19:33 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: meta

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
>     t/v2mirror.t ................. 71/? W: /tmp/pi-v2mirror-39373-Dl1N/m/git/3.git missing remote.origin.url
>     fatal: not a git repository: '/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git'
>     git --git-dir=/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git fetch -q failed
>     Bailout called.  Further testing stopped:  -fetch failed
>     FAILED--Further testing stopped: -fetch failed
>     make: *** [test_dynamic] Error 255
> 
> FYI, this is git 2.31.1.

I'm not seeing this at all with 2.31.1, 2.20, 2.33...
I wonder if there's a permissions problem or some latent GIT_*
var in env...

Are you testing as root?  I think that would be broken, yes.
The partial fetching would need some work to support working as
root.

> I'll be happy to help troubleshoot things as necessary.

Otherwise, dumping the config and some more info could help...

diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index 7881b402e3f6..a5e4c43c039b 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -122,6 +122,10 @@ sub do_fetch { # main entry point
 				$epoch = $nr;
 			} else {
 				warn "W: $edir missing remote.origin.url\n";
+				my $pid = spawn([qw(git config -l)], undef,
+					{ 1 => $lei->{2}, 2 => $lei->{2} });
+				waitpid($pid, 0);
+				$lei->child_error($?) if $?;
 			}
 		}
 		@epochs = grep { !$skip->{$_} } @epochs if $skip;
diff --git a/t/v2mirror.t b/t/v2mirror.t
index 63d17ebfebba..e3936919306b 100644
--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -5,6 +5,7 @@ use v5.10.1;
 use PublicInbox::TestCommon;
 use File::Path qw(remove_tree make_path);
 use Cwd qw(abs_path);
+use Carp ();
 use PublicInbox::Spawn qw(which);
 require_git(2.6);
 require_cmd('curl');
@@ -102,7 +103,9 @@ my @new_epochs;
 my $fetch_each_epoch = sub {
 	my %before = map { $_ => 1 } glob("$tmpdir/m/git/*");
 	run_script([qw(-fetch --exit-code -q)], undef, {-C => "$tmpdir/m"}) or
-		xbail '-fetch fail';
+		xbail('-fetch fail ',
+			[ xqx([which('find'), "$tmpdir/m", qw(-type f -ls) ]) ],
+			Carp::longmess());
 	is($?, 0, '--exit-code 0 after fetch updated');
 	my @after = grep { !$before{$_} } glob("$tmpdir/m/git/*");
 	push @new_epochs, @after;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: -fetch failures [was: latest make test failures on CentOS-7]
  2021-09-27 19:33 ` -fetch failures [was: latest make test failures on CentOS-7] Eric Wong
@ 2021-09-27 19:45   ` Konstantin Ryabitsev
  2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
  0 siblings, 1 reply; 11+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-27 19:45 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

On Mon, Sep 27, 2021 at 07:33:46PM +0000, Eric Wong wrote:
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> >     t/v2mirror.t ................. 71/? W: /tmp/pi-v2mirror-39373-Dl1N/m/git/3.git missing remote.origin.url
> >     fatal: not a git repository: '/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git'
> >     git --git-dir=/tmp/pi-v2mirror-39373-Dl1N/m/git/3.git fetch -q failed
> >     Bailout called.  Further testing stopped:  -fetch failed
> >     FAILED--Further testing stopped: -fetch failed
> >     make: *** [test_dynamic] Error 255
> > 
> > FYI, this is git 2.31.1.
> 
> I'm not seeing this at all with 2.31.1, 2.20, 2.33...
> I wonder if there's a permissions problem or some latent GIT_*
> var in env...
> 
> Are you testing as root?  I think that would be broken, yes.

Yes, this is testing as root, largely because automatic deployment makes it
hard to do the checkout/make/make test as an unprivileged user. I appreciate
that in most other scenarios the final step would be "sudo make install" but
when things are installed via configuration management, the process usually
runs as root already and su-ing to a user for "make test" is adding more
complication to the process.

> The partial fetching would need some work to support working as
> root.

Ah. Just catch this with check if if id=0 and skipping the test as "known not
to work for this use-case."

-K

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 0/3] fixes for odd/old/missing dependencies
  2021-09-27 19:45   ` Konstantin Ryabitsev
@ 2021-09-27 21:05     ` Eric Wong
  2021-09-27 21:05       ` [PATCH 1/3] fetch: support running as root Eric Wong
                         ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Eric Wong @ 2021-09-27 21:05 UTC (permalink / raw)
  To: meta, Konstantin Ryabitsev

On Mon, 27 Sep 2021 15:45:20 -0400, Konstantin Ryabitsev wrote:
> On Mon, Sep 27, 2021 at 07:33:46PM +0000, Eric Wong wrote:
> > The partial fetching would need some work to support working as
> > root.
> 
> Ah. Just catch this with check if if id=0 and skipping the test as "known not
> to work for this use-case."

It wasn't much to add support for root, actually.  The rest of
the stuff should be fixed, too.

Eric Wong (3):
  fetch: support running as root
  t/lei-index: IMAP and NNTP dependencies are optional
  lei completion: workaround old Perl bug

 lib/PublicInbox/Fetch.pm              | 15 ++++++++++--
 lib/PublicInbox/LeiExportKw.pm        |  2 +-
 lib/PublicInbox/LeiImport.pm          |  4 +--
 lib/PublicInbox/LeiLsMailSource.pm    |  4 +--
 lib/PublicInbox/LeiRefreshMailSync.pm |  2 +-
 t/lei-index.t                         | 35 ++++++++++++++++++---------
 t/v2mirror.t                          | 13 +++++++---
 7 files changed, 52 insertions(+), 23 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] fetch: support running as root
  2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
@ 2021-09-27 21:05       ` Eric Wong
  2021-09-27 21:05       ` [PATCH 2/3] t/lei-index: IMAP and NNTP dependencies are optional Eric Wong
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2021-09-27 21:05 UTC (permalink / raw)
  To: meta; +Cc: Konstantin Ryabitsev

The "-w" perlop always succeeds as root, so we need to check
st_mode for writability bits to detect directories we shouldn't
write to.

Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Link: https://public-inbox.org/meta/20210927124056.kj5okiefvs4ztk27@meerkat.local/
---
 lib/PublicInbox/Fetch.pm | 15 +++++++++++++--
 t/v2mirror.t             | 13 ++++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index 7881b402e3f6..5ada1f49e4dc 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -6,7 +6,7 @@ use strict;
 use v5.10.1;
 use parent qw(PublicInbox::IPC);
 use URI ();
-use PublicInbox::Spawn qw(popen_rd run_die);
+use PublicInbox::Spawn qw(popen_rd run_die spawn);
 use PublicInbox::Admin;
 use PublicInbox::LEI;
 use PublicInbox::LeiCurl;
@@ -95,6 +95,13 @@ sub get_fingerprint2 {
 	Digest::SHA::sha256(do { local $/; <$rd> });
 }
 
+sub writable_dir ($) {
+	my ($dir) = @_;
+	return unless -d $dir && -w _;
+	my @st = stat($dir);
+	$st[2] & 0222; # any writable bits set? (in case of root)
+}
+
 sub do_fetch { # main entry point
 	my ($cls, $lei, $cd) = @_;
 	my $ibx_ver;
@@ -112,7 +119,7 @@ sub do_fetch { # main entry point
 		my ($git_url, $epoch);
 		for my $nr (@epochs) { # try newest epoch, first
 			my $edir = "$dir/git/$nr.git";
-			unless (-d $edir && -w _) { # must be writable dir
+			if (!writable_dir($edir)) {
 				$skip->{$nr} = 1;
 				next;
 			}
@@ -122,6 +129,10 @@ sub do_fetch { # main entry point
 				$epoch = $nr;
 			} else {
 				warn "W: $edir missing remote.origin.url\n";
+				my $pid = spawn([qw(git config -l)], undef,
+					{ 1 => $lei->{2}, 2 => $lei->{2} });
+				waitpid($pid, 0);
+				$lei->child_error($?) if $?;
 			}
 		}
 		@epochs = grep { !$skip->{$_} } @epochs if $skip;
diff --git a/t/v2mirror.t b/t/v2mirror.t
index 63d17ebfebba..37d64e83e53b 100644
--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -5,6 +5,7 @@ use v5.10.1;
 use PublicInbox::TestCommon;
 use File::Path qw(remove_tree make_path);
 use Cwd qw(abs_path);
+use Carp ();
 use PublicInbox::Spawn qw(which);
 require_git(2.6);
 require_cmd('curl');
@@ -102,7 +103,9 @@ my @new_epochs;
 my $fetch_each_epoch = sub {
 	my %before = map { $_ => 1 } glob("$tmpdir/m/git/*");
 	run_script([qw(-fetch --exit-code -q)], undef, {-C => "$tmpdir/m"}) or
-		xbail '-fetch fail';
+		xbail('-fetch fail ',
+			[ xqx([which('find'), "$tmpdir/m", qw(-type f -ls) ]) ],
+			Carp::longmess());
 	is($?, 0, '--exit-code 0 after fetch updated');
 	my @after = grep { !$before{$_} } glob("$tmpdir/m/git/*");
 	push @new_epochs, @after;
@@ -273,6 +276,10 @@ if ('test read-only epoch dirs') {
 	my @g = glob("$dst/git/*.git");
 	my @w = grep { -w $_ } @g;
 	my @r = grep { ! -w $_ } @g;
+	if ($> == 0) {
+		@w = grep { (stat($_))[2] & 0200 } @g;
+		@r = grep { !((stat($_))[2] & 0200) } @g;
+	}
 	is(scalar(@w), 1, 'one writable directory');
 	my ($w) = ($w[0] =~ m!/([0-9]+)\.git\z!);
 	is((grep {
@@ -287,7 +294,7 @@ if ('test read-only epoch dirs') {
 			"http://$host:$port/v2/", $dst]);
 	my @g2 = glob("$dst/git/*.git") ;
 	is_deeply(\@g2, \@g, 'cloned again');
-	is(scalar(grep { -w $_ } @g2), scalar(@w) + 1,
+	is(scalar(grep { (stat($_))[2] & 0200 } @g2), scalar(@w) + 1,
 		'got one more cloned epoch');
 
 	# make 0.git writable and fetch into it, relies on culled manifest
@@ -377,7 +384,7 @@ EOM
 	@cmd = (qw(-clone -q --epoch=~0), "http://$host:$port/v2", $dst);
 	run_script(\@cmd, undef, { 2 => \($err = '') });
 	is($?, 0, 'partial scraping clone on old PublicInbox::WWW');
-	my @g_last = grep { -w $_ } glob("$dst/git/*.git");
+	my @g_last = grep { (stat($_))[2] & 0200 } glob("$dst/git/*.git");
 	is_deeply(\@g_last, [ $g_all[-1] ], 'partial clone of ~0 worked');
 
 	chmod(0755, $g_all[0]) or xbail "chmod $!";

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] t/lei-index: IMAP and NNTP dependencies are optional
  2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
  2021-09-27 21:05       ` [PATCH 1/3] fetch: support running as root Eric Wong
@ 2021-09-27 21:05       ` Eric Wong
  2021-09-27 21:05       ` [PATCH 3/3] lei completion: workaround old Perl bug Eric Wong
  2021-09-27 21:27       ` [PATCH 0/3] fixes for odd/old/missing dependencies Konstantin Ryabitsev
  3 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2021-09-27 21:05 UTC (permalink / raw)
  To: meta; +Cc: Konstantin Ryabitsev

"lei index" support for IMAP and NNTP is incomplete, so there's
no point in requiring them.

Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Link: https://public-inbox.org/meta/20210927124056.kj5okiefvs4ztk27@meerkat.local/
---
 t/lei-index.t | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/t/lei-index.t b/t/lei-index.t
index eeda5196f411..aab8f7e63f06 100644
--- a/t/lei-index.t
+++ b/t/lei-index.t
@@ -3,21 +3,30 @@
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 use strict; use v5.10.1; use PublicInbox::TestCommon;
 use File::Spec;
-require_mods(qw(lei -nntpd));
+require_mods(qw(lei));
 my ($ro_home, $cfg_path) = setup_public_inboxes;
 my ($tmpdir, $for_destroy) = tmpdir;
 my $env = { PI_CONFIG => $cfg_path };
+my $srv = {};
 
-my $sock = tcp_server;
-my $cmd = [ '-nntpd', '-W0', "--stdout=$tmpdir/n1", "--stderr=$tmpdir/n2" ];
-my $nntpd = start_script($cmd, $env, { 3 => $sock }) or BAIL_OUT("-nntpd $?");
-my $nntp_host_port = tcp_host_port($sock);
+SKIP: {
+	require_mods(qw(-nntpd Net::NNTP), 1);
+	my $rdr = { 3 => tcp_server };
+	$srv->{nntpd} = start_script(
+		[qw(-nntpd -W0), "--stdout=$tmpdir/n1", "--stderr=$tmpdir/n2"],
+		$env, $rdr) or xbail "nntpd: $?";
+	$srv->{nntp_host_port} = tcp_host_port($rdr->{3});
+}
+
+SKIP: {
+	require_mods(qw(-imapd Mail::IMAPClient), 1);
+	my $rdr = { 3 => tcp_server };
+	$srv->{imapd} = start_script(
+		[qw(-imapd -W0), "--stdout=$tmpdir/i1", "--stderr=$tmpdir/i2"],
+		$env, $rdr) or xbail("-imapd $?");
+	$srv->{imap_host_port} = tcp_host_port($rdr->{3});
+}
 
-$sock = tcp_server;
-$cmd = [ '-imapd', '-W0', "--stdout=$tmpdir/i1", "--stderr=$tmpdir/i2" ];
-my $imapd = start_script($cmd, $env, { 3 => $sock }) or BAIL_OUT("-imapd $?");
-my $imap_host_port = tcp_host_port($sock);
-undef $sock;
 for ('', qw(cur new)) {
 	mkdir "$tmpdir/md/$_" or xbail "mkdir: $!";
 	mkdir "$tmpdir/md1/$_" or xbail "mkdir: $!";
@@ -77,8 +86,10 @@ test_lei({ tmpdir => $tmpdir }, sub {
 	is_deeply(json_utf8->decode($lei_out)->[0]->{'kw'},
 		['seen'], 'keyword set');
 
-	lei_ok('index', "nntp://$nntp_host_port/t.v2");
-	lei_ok('index', "imap://$imap_host_port/t.v2.0");
+	$srv->{nntpd} and
+		lei_ok('index', "nntp://$srv->{nntp_host_port}/t.v2");
+	$srv->{imapd} and
+		lei_ok('index', "imap://$srv->{imap_host_port}/t.v2.0");
 	is_deeply([xqx($all_obj)], \@objs, 'no new objects from NNTP+IMAP');
 
 	lei_ok qw(q m:multipart-html-sucks@11);

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] lei completion: workaround old Perl bug
  2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
  2021-09-27 21:05       ` [PATCH 1/3] fetch: support running as root Eric Wong
  2021-09-27 21:05       ` [PATCH 2/3] t/lei-index: IMAP and NNTP dependencies are optional Eric Wong
@ 2021-09-27 21:05       ` Eric Wong
  2021-09-27 21:27       ` [PATCH 0/3] fixes for odd/old/missing dependencies Konstantin Ryabitsev
  3 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2021-09-27 21:05 UTC (permalink / raw)
  To: meta; +Cc: Konstantin Ryabitsev

While `$argv[-1]' is `undef' on an empty @argv, using `$argv[-1]'
as a subroutine argument would fail incorrectly with:

    Modification of non-creatable array value attempted, subscript -1 at ...

...even though we'd never attempt to modify @_ itself in the
subroutines being called.  Work around the bug (tested on
5.16.3) by passing `undef' explicitly when `$argv[-1]' is
already `undef'.

Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
Link: https://public-inbox.org/meta/20210927124056.kj5okiefvs4ztk27@meerkat.local/
---
 lib/PublicInbox/LeiExportKw.pm        | 2 +-
 lib/PublicInbox/LeiImport.pm          | 4 ++--
 lib/PublicInbox/LeiLsMailSource.pm    | 4 ++--
 lib/PublicInbox/LeiRefreshMailSync.pm | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/PublicInbox/LeiExportKw.pm b/lib/PublicInbox/LeiExportKw.pm
index cea9beeb5694..0b65c2762633 100644
--- a/lib/PublicInbox/LeiExportKw.pm
+++ b/lib/PublicInbox/LeiExportKw.pm
@@ -131,7 +131,7 @@ sub _complete_export_kw {
 	my $match_cb = $lei->complete_url_prepare(\@argv);
 	# filter-out read-only sources:
 	my @k = grep(!m!(?://;AUTH=ANONYMOUS\@|\A(?:nntps?|s?news)://)!,
-			$lms->folders($argv[-1], 1));
+			$lms->folders($argv[-1] // undef, 1));
 	my @m = map { $match_cb->($_) } @k;
 	@m ? @m : @k;
 }
diff --git a/lib/PublicInbox/LeiImport.pm b/lib/PublicInbox/LeiImport.pm
index 397292d4c787..69d63ab6b397 100644
--- a/lib/PublicInbox/LeiImport.pm
+++ b/lib/PublicInbox/LeiImport.pm
@@ -122,11 +122,11 @@ sub lei_import { # the main "lei import" method
 sub _complete_import {
 	my ($lei, @argv) = @_;
 	my ($re, $cur, $match_cb) = $lei->complete_url_prepare(\@argv);
-	my @k = $lei->url_folder_cache->keys($argv[-1], 1);
+	my @k = $lei->url_folder_cache->keys($argv[-1] // undef, 1);
 	my @m = map { $match_cb->($_) } @k;
 	my %f = map { $_ => 1 } (@m ? @m : @k);
 	if (my $lms = $lei->lms) {
-		@k = $lms->folders($argv[-1], 1);
+		@k = $lms->folders($argv[-1] // undef, 1);
 		@m = map { $match_cb->($_) } @k;
 		if (@m) { @f{@m} = @m } else { @f{@k} = @k }
 	}
diff --git a/lib/PublicInbox/LeiLsMailSource.pm b/lib/PublicInbox/LeiLsMailSource.pm
index 1db15d5742b5..7c3c0cda18d6 100644
--- a/lib/PublicInbox/LeiLsMailSource.pm
+++ b/lib/PublicInbox/LeiLsMailSource.pm
@@ -107,11 +107,11 @@ sub lei_ls_mail_source {
 sub _complete_ls_mail_source {
 	my ($lei, @argv) = @_;
 	my $match_cb = $lei->complete_url_prepare(\@argv);
-	my @k = $lei->url_folder_cache->keys($argv[-1], 1);
+	my @k = $lei->url_folder_cache->keys($argv[-1] // undef, 1);
 	my @m = map { $match_cb->($_) } @k;
 	my %f = map { $_ => 1 } (@m ? @m : @k);
 	if (my $lms = $lei->lms) {
-		@k = $lms->folders($argv[-1], 1);
+		@k = $lms->folders($argv[-1] // undef, 1);
 		@m = map { $match_cb->($_) } grep(m!\A[a-z]+://!, @k);
 		if (@m) { @f{@m} = @m } else { @f{@k} = @k }
 	}
diff --git a/lib/PublicInbox/LeiRefreshMailSync.pm b/lib/PublicInbox/LeiRefreshMailSync.pm
index eb842843b51d..0cb9f3dccd48 100644
--- a/lib/PublicInbox/LeiRefreshMailSync.pm
+++ b/lib/PublicInbox/LeiRefreshMailSync.pm
@@ -101,7 +101,7 @@ sub _complete_refresh_mail_sync {
 	my ($lei, @argv) = @_;
 	my $lms = $lei->lms or return ();
 	my $match_cb = $lei->complete_url_prepare(\@argv);
-	my @k = $lms->folders($argv[-1], 1);
+	my @k = $lms->folders($argv[-1] // undef, 1);
 	my @m = map { $match_cb->($_) } @k;
 	@m ? @m : @k
 }

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fixes for odd/old/missing dependencies
  2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
                         ` (2 preceding siblings ...)
  2021-09-27 21:05       ` [PATCH 3/3] lei completion: workaround old Perl bug Eric Wong
@ 2021-09-27 21:27       ` Konstantin Ryabitsev
  2021-09-27 21:40         ` Eric Wong
  3 siblings, 1 reply; 11+ messages in thread
From: Konstantin Ryabitsev @ 2021-09-27 21:27 UTC (permalink / raw)
  To: Eric Wong; +Cc: meta

On Mon, Sep 27, 2021 at 04:05:42PM -0500, Eric Wong wrote:
> On Mon, 27 Sep 2021 15:45:20 -0400, Konstantin Ryabitsev wrote:
> > On Mon, Sep 27, 2021 at 07:33:46PM +0000, Eric Wong wrote:
> > > The partial fetching would need some work to support working as
> > > root.
> > 
> > Ah. Just catch this with check if if id=0 and skipping the test as "known not
> > to work for this use-case."
> 
> It wasn't much to add support for root, actually.  The rest of
> the stuff should be fixed, too.

Yep, I can confirm that after applying these in addition to the one I applied
earlier, all tests pass.

Thanks, Eric!

-K

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] fixes for odd/old/missing dependencies
  2021-09-27 21:27       ` [PATCH 0/3] fixes for odd/old/missing dependencies Konstantin Ryabitsev
@ 2021-09-27 21:40         ` Eric Wong
  0 siblings, 0 replies; 11+ messages in thread
From: Eric Wong @ 2021-09-27 21:40 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: meta

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Yep, I can confirm that after applying these in addition to the one I applied
> earlier, all tests pass.
> 
> Thanks, Eric!

No problem, thanks for the bug reports.  Btw, I was getting some
t/lei-auto-watch.t failures the other week, even after a
reliability fix(*) on a slow CentOS 7 VM, but can't reproduce
the failure anymore...

(*) 2f993ff30854 (t/lei-auto-watch: improve test reliability, 2021-09-02)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-09-27 21:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 12:40 latest make test failures on CentOS-7 Konstantin Ryabitsev
2021-09-27 18:35 ` [PATCH] t/cmd_ipc: allow extra errors and add diagnostics Eric Wong
2021-09-27 18:51   ` Konstantin Ryabitsev
2021-09-27 19:33 ` -fetch failures [was: latest make test failures on CentOS-7] Eric Wong
2021-09-27 19:45   ` Konstantin Ryabitsev
2021-09-27 21:05     ` [PATCH 0/3] fixes for odd/old/missing dependencies Eric Wong
2021-09-27 21:05       ` [PATCH 1/3] fetch: support running as root Eric Wong
2021-09-27 21:05       ` [PATCH 2/3] t/lei-index: IMAP and NNTP dependencies are optional Eric Wong
2021-09-27 21:05       ` [PATCH 3/3] lei completion: workaround old Perl bug Eric Wong
2021-09-27 21:27       ` [PATCH 0/3] fixes for odd/old/missing dependencies Konstantin Ryabitsev
2021-09-27 21:40         ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).