[PATCH 0/4] test fixes for latest CPAN modules

[PATCH 0/4] test fixes for latest CPAN modules

[Eric Wong]

A few test fixes for problems encountered by Konstantin on
CentOS 7 and some of the latest CPAN modules.

There's still locking problem in in t/v2mirror.t which I
have not reproduced, yet.

Eric Wong (4):
  content_id: workaround quote handling change in Email::* modules
  convert+compact: fix when running without ~/.public-inbox/config
  t/v2mda: setup emergency Maildir for test
  t/search: quiet warning from Encode.pm

 script/public-inbox-compact | 10 ++++++----
 script/public-inbox-convert | 10 ++++++----
 t/content_id.t              |  8 ++++----
 t/convert-compact.t         |  1 +
 t/search.t                  |  2 +-
 t/v2mda.t                   |  2 ++
 6 files changed, 20 insertions(+), 13 deletions(-)

[PATCH 1/4] content_id: workaround quote handling change in Email::* modules

[Eric Wong]

I'm not entirely sure where the behavior change lies, but
it seems to be in some of the latest CPAN versions of these
modules.  In any case, this only affects the test setup and
not actual behavior.

cf. https://public-inbox.org/meta/2a2bf0e1-fd1f-f8bf-95bc-dac47906ef43@linuxfoundation.org/
---
 t/content_id.t | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/content_id.t b/t/content_id.t
index 01ce65e..990eabe 100644
--- a/t/content_id.t
+++ b/t/content_id.t
@@ -23,11 +23,11 @@ my $reload = content_id(Email::MIME->new($mime->as_string));
 is($orig, $reload, 'content_id matches after serialization');
 
 foreach my $h (qw(From To Cc)) {
-	my $n = '"Quoted N\'Ame" <foo@EXAMPLE.com>';
-	$mime->header_str_set($h, "$n");
+	my $n = q("Quoted N'Ame" <foo@EXAMPLE.com>);
+	$mime->header_set($h, "$n");
 	my $q = content_id($mime);
-	is($n, $mime->header($h), "content_id does not mutate $h:");
-	$mime->header_str_set($h, 'Quoted N\'Ame <foo@example.com>');
+	is($mime->header($h), $n, "content_id does not mutate $h:");
+	$mime->header_set($h, 'Quoted N\'Ame <foo@example.com>');
 	my $nq = content_id($mime);
 	is($nq, $q, "quotes ignored in $h:");
 }

[PATCH 2/4] convert+compact: fix when running without ~/.public-inbox/config

[Eric Wong]

Some users may not have any public-inboxes configured, especially in
tests.
---
 script/public-inbox-compact | 10 ++++++----
 script/public-inbox-convert | 10 ++++++----
 t/convert-compact.t         |  1 +
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/script/public-inbox-compact b/script/public-inbox-compact
index 5f18497..d22e403 100755
--- a/script/public-inbox-compact
+++ b/script/public-inbox-compact
@@ -13,12 +13,14 @@ use File::Path qw(remove_tree);
 use PublicInbox::Spawn qw(spawn);
 my $usage = "Usage: public-inbox-compact REPO_DIR\n";
 my $dir = shift or die $usage;
-my $config = PublicInbox::Config->new;
+my $config = eval { PublicInbox::Config->new };
 my $ibx;
 $dir = abs_path($dir);
-$config->each_inbox(sub {
-	$ibx = $_[0] if abs_path($_[0]->{mainrepo}) eq $dir
-});
+if ($config) {
+	$config->each_inbox(sub {
+		$ibx = $_[0] if abs_path($_[0]->{mainrepo}) eq $dir
+	});
+}
 unless ($ibx) {
 	warn "W: $dir not configured in ".
 		PublicInbox::Config::default_file() . "\n";
diff --git a/script/public-inbox-convert b/script/public-inbox-convert
index 2979a0c..bd8fb98 100755
--- a/script/public-inbox-convert
+++ b/script/public-inbox-convert
@@ -25,12 +25,14 @@ my $old_dir = shift or die $usage;
 my $new_dir = shift or die $usage;
 die "$new_dir exists\n" if -d $new_dir;
 die "$old_dir not a directory\n" unless -d $old_dir;
-my $config = PublicInbox::Config->new;
+my $config = eval { PublicInbox::Config->new };
 $old_dir = abs_path($old_dir);
 my $old;
-$config->each_inbox(sub {
-	$old = $_[0] if abs_path($_[0]->{mainrepo}) eq $old_dir;
-});
+if ($config) {
+	$config->each_inbox(sub {
+		$old = $_[0] if abs_path($_[0]->{mainrepo}) eq $old_dir;
+	});
+}
 unless ($old) {
 	warn "W: $old_dir not configured in " .
 		PublicInbox::Config::default_file() . "\n";
diff --git a/t/convert-compact.t b/t/convert-compact.t
index ced4541..e923200 100644
--- a/t/convert-compact.t
+++ b/t/convert-compact.t
@@ -54,6 +54,7 @@ foreach (@xdir) {
 }
 
 local $ENV{PATH} = "blib/script:$ENV{PATH}";
+local $ENV{PI_CONFIG} = '/dev/null';
 open my $err, '>>', "$tmpdir/err.log" or die "open: err.log $!\n";
 open my $out, '>>', "$tmpdir/out.log" or die "open: out.log $!\n";
 my $rdr = { 1 => fileno($out), 2 => fileno($err) };

[PATCH 3/4] t/v2mda: setup emergency Maildir for test

[Eric Wong]

We can't expect ~/.public-inbox to exist for tests nor should
we be writing to non-temporary directories during tests.
---
 t/v2mda.t | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/v2mda.t b/t/v2mda.t
index ca1bb09..f386289 100644
--- a/t/v2mda.t
+++ b/t/v2mda.t
@@ -36,6 +36,8 @@ ok(-f "blib/script/public-inbox-mda", '-mda exists');
 my $main_bin = getcwd()."/t/main-bin";
 local $ENV{PI_DIR} = "$tmpdir/foo";
 local $ENV{PATH} = "$main_bin:blib/script:$ENV{PATH}";
+local $ENV{PI_EMERGENCY} = "$tmpdir/fail";
+ok(mkdir "$tmpdir/fail");
 my @cmd = (qw(public-inbox-init -V2), $ibx->{name},
 		$ibx->{mainrepo}, 'http://localhost/test',
 		$ibx->{address}->[0]);

[PATCH 4/4] t/search: quiet warning from Encode.pm

[Eric Wong]

This was probably a typo on my part, and quiets a warning:
  Argument contains empty address at .../Email/MIME/Encode.pm line 70

Tested with Email::MIME 1.946
---
 t/search.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/search.t b/t/search.t
index 54f2db9..9a90fd5 100644
--- a/t/search.t
+++ b/t/search.t
@@ -214,7 +214,7 @@ $ibx->with_umask(sub {
 			# No References:
 			# 'References' => '<root@s> <last@s> <'.$long_mid.'>',
 			'In-Reply-To' => "<$long_mid>",
-			From => '"no1 <no1@example.com>',
+			From => 'no1 <no1@example.com>',
 			To => 'list@example.com',
 		],
 		body => "no References\n");

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Konstantin Ryabitsev]

[-- Attachment #1.1: Type: text/plain, Size: 567 bytes --]

On 05/11/18 15:20, Eric Wong wrote:
> A few test fixes for problems encountered by Konstantin on
> CentOS 7 and some of the latest CPAN modules.
> 
> There's still locking problem in in t/v2mirror.t which I
> have not reproduced, yet.

Thanks, Eric!

I can confirm that v2mirror.t is the only test that's still failing now.
If you need help troubleshooting that, I'll be happy to help. Feel free
to jump into #korg on linuxnet (or #kernel.org on freenode).

Best,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> I can confirm that v2mirror.t is the only test that's still failing now.
> If you need help troubleshooting that, I'll be happy to help. Feel free
> to jump into #korg on linuxnet (or #kernel.org on freenode).

I have to run out in a bit, and prefer to keep as much as
possible over email for archival purposes.

Which Xapian (both Perl bindings and libxapian*.so) are you running?

Can you try flipping the parallel flag and perhaps provide "strace -f"
output with/without parallel.   (strace -f prove -lv t/v2mirror.t)

--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -38,7 +38,7 @@ ok($ibx, 'inbox found');
 $ibx->{version} = 2;
 my $v2w = PublicInbox::V2Writable->new($ibx, 1);
 ok $v2w, 'v2w loaded';
-$v2w->{parallel} = 0;
+$v2w->{parallel} = 1;
 my $mime = PublicInbox::MIME->new(<<'');
 From: Me <me@example.com>
 To: You <you@example.com>


Thanks for the feedback!

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Konstantin Ryabitsev]

[-- Attachment #1.1.1: Type: text/plain, Size: 592 bytes --]

On 05/11/18 15:38, Eric Wong wrote:
> Which Xapian (both Perl bindings and libxapian*.so) are you running?

[root@pdx-tst-mricon-1 share]# rpm -q xapian-core
xapian-core-1.2.22-1.el7.x86_64
[root@pdx-tst-mricon-1 share]# cpanm --info Search::Xapian
OLLY/Search-Xapian-1.2.25.0.tar.gz

> Can you try flipping the parallel flag and perhaps provide "strace -f"
> output with/without parallel.   (strace -f prove -lv t/v2mirror.t)

Tests pass with parallel = 1, strace for both attached.

Best,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation

[-- Attachment #1.1.2: v2mirror-parallel-0.xz --]
[-- Type: application/x-xz, Size: 212808 bytes --]

[-- Attachment #1.1.3: v2mirror-parallel-1.xz --]
[-- Type: application/x-xz, Size: 268008 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On 05/11/18 15:38, Eric Wong wrote:
> > Which Xapian (both Perl bindings and libxapian*.so) are you running?
> 
> [root@pdx-tst-mricon-1 share]# rpm -q xapian-core
> xapian-core-1.2.22-1.el7.x86_64

Ah, took me a while to realize what was going on :x
The Xapian files processes were lacking O_CLOEXEC and FD_CLOEXEC
usage and this was a problem in Xapian >= 1.2.21 && <= 1.2.24

Relevant Xapian commit should be e953a10dc4f0cc8e604fd2082c87b638c6a3382b
("Set CLOEXEC on the lock file fd and pipe")

I suspect the following patch works around the problem with or
without parallel=0

--- a/t/v2mirror.t
+++ b/t/v2mirror.t
@@ -50,7 +50,7 @@ for my $i (1..9) {
 	$mime->header_set('Subject', "subject = $i");
 	ok($v2w->add($mime), "add msg $i OK");
 }
-$v2w->barrier;
+$v2w->done;
 
 my %opts = (
 	LocalAddr => '127.0.0.1',

Can you confirm?  I don't have 1.2.22 handy anymore, but I
reported this problem and got Olly to fix it back in 2016.
Thanks.

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Konstantin Ryabitsev]

On Wed, 16 May 2018 at 01:12, Eric Wong <e@80x24.org> wrote:
> Ah, took me a while to realize what was going on :x
> The Xapian files processes were lacking O_CLOEXEC and FD_CLOEXEC
> usage and this was a problem in Xapian >= 1.2.21 && <= 1.2.24

> Relevant Xapian commit should be e953a10dc4f0cc8e604fd2082c87b638c6a3382b
> ("Set CLOEXEC on the lock file fd and pipe")

> I suspect the following patch works around the problem with or
> without parallel=0

> --- a/t/v2mirror.t
> +++ b/t/v2mirror.t
> @@ -50,7 +50,7 @@ for my $i (1..9) {
>          $mime->header_set('Subject', "subject = $i");
>          ok($v2w->add($mime), "add msg $i OK");
>   }
> -$v2w->barrier;
> +$v2w->done;

>   my %opts = (
>          LocalAddr => '127.0.0.1',

> Can you confirm?  I don't have 1.2.22 handy anymore, but I
> reported this problem and got Olly to fix it back in 2016.

Sorry I missed your reply! Yes, you're correct -- applying the above patch
makes all tests pass. I'm guessing it's also the reason why I'm seeing the
"already locked" error in my other tests, too:

$ public-inbox-index /var/lib/public-inbox/meta.git
Exception: Unable to get write lock on
/var/lib/public-inbox/meta.git/public-inbox/xapian15: already locked

Hope this helps.
-K

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Sorry I missed your reply! Yes, you're correct -- applying the above patch
> makes all tests pass. I'm guessing it's also the reason why I'm seeing the
> "already locked" error in my other tests, too:
> 
> $ public-inbox-index /var/lib/public-inbox/meta.git
> Exception: Unable to get write lock on
> /var/lib/public-inbox/meta.git/public-inbox/xapian15: already locked

OK, that Xapian bug should be worked around in
https://public-inbox.org/meta/20180524083216.30440-1-e@80x24.org/

And pushed as commit c339a14b11c57e7b330f4efef6903bcbea2b3463
("workaround Xapian OFD locks w/o close-on-exec")

Thanks for the testing!

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Konstantin Ryabitsev]

[-- Attachment #1.1: Type: text/plain, Size: 1758 bytes --]

On 05/24/18 04:41, Eric Wong wrote:
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
>> Sorry I missed your reply! Yes, you're correct -- applying the above patch
>> makes all tests pass. I'm guessing it's also the reason why I'm seeing the
>> "already locked" error in my other tests, too:
>>
>> $ public-inbox-index /var/lib/public-inbox/meta.git
>> Exception: Unable to get write lock on
>> /var/lib/public-inbox/meta.git/public-inbox/xapian15: already locked
> 
> OK, that Xapian bug should be worked around in
> https://public-inbox.org/meta/20180524083216.30440-1-e@80x24.org/
> 
> And pushed as commit c339a14b11c57e7b330f4efef6903bcbea2b3463
> ("workaround Xapian OFD locks w/o close-on-exec")

Hmm... I'm still getting the same error, though, if I try to re-run
public-inbox-index:

$ git clone --mirror https://public-inbox.org/meta
Cloning into bare repository 'meta.git'...
remote: Counting objects: 7524, done.
remote: Compressing objects: 100% (164/164), done.
remote: Total 7524 (delta 52), reused 0 (delta 0)
Receiving objects: 100% (7524/7524), 3.80 MiB | 13.91 MiB/s, done.
Resolving deltas: 100% (3770/3770), done.
$ export PI_CONFIG=/etc/public-inbox/config
$ public-inbox-index /var/lib/public-inbox/meta.git

The above succeeds, but if I try to run it again right away:

$ public-inbox-index /var/lib/public-inbox/meta.git
Exception: Unable to get write lock on
/var/lib/public-inbox/meta.git/public-inbox/xapian15: already locked

Is this easier solved by upgrading to the version of xapian > 1.2.22? I
can probably nag the Epel packager to do that if it's a legitimate bug
in Xapian.

Best,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> On 05/24/18 04:41, Eric Wong wrote:
> > OK, that Xapian bug should be worked around in
> > https://public-inbox.org/meta/20180524083216.30440-1-e@80x24.org/
> > 
> > And pushed as commit c339a14b11c57e7b330f4efef6903bcbea2b3463
> > ("workaround Xapian OFD locks w/o close-on-exec")
> 
> Hmm... I'm still getting the same error, though, if I try to re-run
> public-inbox-index:
> 
> $ git clone --mirror https://public-inbox.org/meta

<snip>

> $ public-inbox-index /var/lib/public-inbox/meta.git
> 
> The above succeeds, but if I try to run it again right away:
> 
> $ public-inbox-index /var/lib/public-inbox/meta.git
> Exception: Unable to get write lock on
> /var/lib/public-inbox/meta.git/public-inbox/xapian15: already locked

Odd, I tried that exact sequence with the v1.2.22 tag in
xapian.git on my Debian machine to reproduce the problem
and make that patch.

> Is this easier solved by upgrading to the version of xapian > 1.2.22? I
> can probably nag the Epel packager to do that if it's a legitimate bug
> in Xapian.

Maybe... I wonder if there's other patches in the Epel package
or other system-level differences causing this.

Re: [PATCH 0/4] test fixes for latest CPAN modules

[Konstantin Ryabitsev]

On Wed, May 30, 2018 at 11:57:19AM +0000, Eric Wong wrote:
>> Is this easier solved by upgrading to the version of xapian > 1.2.22? I
>> can probably nag the Epel packager to do that if it's a legitimate bug
>> in Xapian.
>
>Maybe... I wonder if there's other patches in the Epel package
>or other system-level differences causing this.

I'm fine with that -- in fact, I already requested that the package in
EPEL is updated to the latest 2.2.

We can just leave things as-is, as I definitely don't see the same issue
with 2.2.25.

-K