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 280C41F8EF for ; Sat, 27 Jun 2020 10:04:02 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 10/34] watch: remove Filesys::Notify::Simple dependency Date: Sat, 27 Jun 2020 10:03:36 +0000 Message-Id: <20200627100400.9871-11-e@yhbt.net> In-Reply-To: <20200627100400.9871-1-e@yhbt.net> References: <20200627100400.9871-1-e@yhbt.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Since we already use inotify and EVFILT_VNODE (kqueue) in -imapd, we might as well use them directly in -watch, too. This will allow public-inbox-watch to use PublicInbox::DS for timers to watch newsgroups/mailboxes and have saner signal handling in future commits. --- Documentation/public-inbox-watch.pod | 3 +- INSTALL | 8 --- MANIFEST | 2 + Makefile.PL | 4 -- ci/deps.perl | 1 - lib/PublicInbox/DirIdle.pm | 50 ++++++++++++++++ lib/PublicInbox/In2Tie.pm | 13 +++++ lib/PublicInbox/InboxIdle.pm | 11 +--- lib/PublicInbox/TestCommon.pm | 6 +- lib/PublicInbox/WatchMaildir.pm | 39 +++++++------ script/public-inbox-watch | 3 +- t/dir_idle.t | 6 ++ t/imapd.t | 6 +- t/watch_filter_rubylang.t | 2 +- t/watch_maildir.t | 85 +++++++++++++++++++++++++--- t/watch_maildir_v2.t | 2 +- t/watch_multiple_headers.t | 2 +- 17 files changed, 182 insertions(+), 61 deletions(-) create mode 100644 lib/PublicInbox/DirIdle.pm create mode 100644 t/dir_idle.t diff --git a/Documentation/public-inbox-watch.pod b/Documentation/public-inbox-watch.pod index 8a3ef076a51..bf3c9bd4bb9 100644 --- a/Documentation/public-inbox-watch.pod +++ b/Documentation/public-inbox-watch.pod @@ -48,8 +48,7 @@ of large Maildirs. Upon startup, it scans the mailbox for new messages to be imported while it was not running. -Currently, only Maildirs are supported and the -L Perl module is required. +Currently, only Maildirs are supported. For now, IMAP users should use tools such as L or L to bidirectionally sync their IMAP diff --git a/INSTALL b/INSTALL index 80cee7535f8..05e0f95e914 100644 --- a/INSTALL +++ b/INSTALL @@ -132,18 +132,10 @@ above, so there is no need to explicitly install them: (optional for stale FD cleanup in daemons, typically installed alongside Perl5) -- Filesys::Notify::Simple deb: libfilesys-notify-simple-perl - pkg: p5-Filesys-Notify-Simple - rpm: perl-Filesys-Notify-Simple - (for public-inbox-watch, pulled in by Plack) - - Linux::Inotify2 deb: liblinux-inotify2-perl rpm: perl-Linux-Inotify2 (for public-inbox-watch on Linux) -- Filesys::Notify::KQueue pkg: p5-Filesys-Notify-KQueue - (for public-inbox-watch on FreeBSD) - - IO::Compress (::Gzip) deb: perl-modules (or libio-compress-perl) pkg: perl5 rpm: perl-IO-Compress diff --git a/MANIFEST b/MANIFEST index 9d1a4e4a8b1..035c45bf498 100644 --- a/MANIFEST +++ b/MANIFEST @@ -106,6 +106,7 @@ lib/PublicInbox/DS.pm lib/PublicInbox/DSKQXS.pm lib/PublicInbox/DSPoll.pm lib/PublicInbox/Daemon.pm +lib/PublicInbox/DirIdle.pm lib/PublicInbox/DummyInbox.pm lib/PublicInbox/Emergency.pm lib/PublicInbox/Eml.pm @@ -243,6 +244,7 @@ t/content_hash.t t/convert-compact.t t/data/0001.patch t/data/message_embed.eml +t/dir_idle.t t/ds-kqxs.t t/ds-leak.t t/ds-poll.t diff --git a/Makefile.PL b/Makefile.PL index 472baa9b080..8d90ad46cf6 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -138,10 +138,6 @@ WriteMakefile( # Plack is needed for public-inbox-httpd and PublicInbox::WWW # 'Plack' => 0, - # Filesys::Notify::Simple is pulled in by Plack, but also - # needed by public-inbox-watch (for now) - # 'Filesys::Notify::Simple' => 0, - # TODO: this should really be made optional... 'URI::Escape' => 0, diff --git a/ci/deps.perl b/ci/deps.perl index 48aaa9e46d4..501f51129e7 100755 --- a/ci/deps.perl +++ b/ci/deps.perl @@ -32,7 +32,6 @@ my $profiles = { BSD::Resource DBD::SQLite DBI - Filesys::Notify::Simple Inline::C Net::Server Plack diff --git a/lib/PublicInbox/DirIdle.pm b/lib/PublicInbox/DirIdle.pm new file mode 100644 index 00000000000..ffceda66530 --- /dev/null +++ b/lib/PublicInbox/DirIdle.pm @@ -0,0 +1,50 @@ +# Copyright (C) 2020 all contributors +# License: AGPL-3.0+ + +# Used by public-inbox-watch for Maildir (and possibly MH in the future) +package PublicInbox::DirIdle; +use strict; +use base 'PublicInbox::DS'; +use fields qw(inot); +use PublicInbox::Syscall qw(EPOLLIN EPOLLET); +use PublicInbox::In2Tie; + +my ($MAIL_IN, $ino_cls); +if ($^O eq 'linux' && eval { require Linux::Inotify2; 1 }) { + $MAIL_IN = Linux::Inotify2::IN_MOVED_TO() | + Linux::Inotify2::IN_CREATE(); + $ino_cls = 'Linux::Inotify2'; +} elsif (eval { require PublicInbox::KQNotify }) { + $MAIL_IN = PublicInbox::KQNotify::MOVED_TO_OR_CREATE(); + $ino_cls = 'PublicInbox::KQNotify'; +} else { + require PublicInbox::FakeInotify; + $MAIL_IN = PublicInbox::FakeInotify::MOVED_TO_OR_CREATE(); +} + +sub new { + my ($class, $dirs, $cb) = @_; + my $self = fields::new($class); + my $inot; + if ($ino_cls) { + $inot = $ino_cls->new or die "E: $ino_cls->new: $!"; + my $io = PublicInbox::In2Tie::io($inot); + $self->SUPER::new($io, EPOLLIN | EPOLLET); + } else { + require PublicInbox::FakeInotify; + $inot = PublicInbox::FakeInotify->new; # starts timer + } + + # Linux::Inotify2->watch or similar + $inot->watch($_, $MAIL_IN, $cb) for @$dirs; + $self->{inot} = $inot; + $self; +} + +sub event_step { + my ($self) = @_; + eval { $self->{inot}->poll }; # Linux::Inotify2::poll + warn "$self->{inot}->poll err: $@\n" if $@; +} + +1; diff --git a/lib/PublicInbox/In2Tie.pm b/lib/PublicInbox/In2Tie.pm index db1dc1045c1..7dee362724e 100644 --- a/lib/PublicInbox/In2Tie.pm +++ b/lib/PublicInbox/In2Tie.pm @@ -5,6 +5,19 @@ # on Linux::Inotify2 objects package PublicInbox::In2Tie; use strict; +use Symbol qw(gensym); + +sub io { + my $in2 = $_[0]; + $in2->blocking(0); + if ($in2->can('on_overflow')) { + # broadcasts everything on overflow + $in2->on_overflow(undef); + } + my $io = gensym; + tie *$io, __PACKAGE__, $in2; + $io; +} sub TIEHANDLE { my ($class, $in2) = @_; diff --git a/lib/PublicInbox/InboxIdle.pm b/lib/PublicInbox/InboxIdle.pm index 97e9d53250e..ba8200aef05 100644 --- a/lib/PublicInbox/InboxIdle.pm +++ b/lib/PublicInbox/InboxIdle.pm @@ -6,7 +6,6 @@ use strict; use base qw(PublicInbox::DS); use fields qw(pi_config inot pathmap); use Cwd qw(abs_path); -use Symbol qw(gensym); use PublicInbox::Syscall qw(EPOLLIN EPOLLET); my $IN_MODIFY = 0x02; # match Linux inotify my $ino_cls; @@ -55,14 +54,8 @@ sub new { my $inot; if ($ino_cls) { $inot = $ino_cls->new or die "E: $ino_cls->new: $!"; - my $sock = gensym; - tie *$sock, 'PublicInbox::In2Tie', $inot; - $inot->blocking(0); - if ($inot->can('on_overflow')) { - # broadcasts everything on overflow - $inot->on_overflow(undef); - } - $self->SUPER::new($sock, EPOLLIN | EPOLLET); + my $io = PublicInbox::In2Tie::io($inot); + $self->SUPER::new($io, EPOLLIN | EPOLLET); } else { require PublicInbox::FakeInotify; $inot = PublicInbox::FakeInotify->new; diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm index dc360135569..b252810fca5 100644 --- a/lib/PublicInbox/TestCommon.pm +++ b/lib/PublicInbox/TestCommon.pm @@ -10,7 +10,7 @@ use Fcntl qw(FD_CLOEXEC F_SETFD F_GETFD :seek); use POSIX qw(dup2); use IO::Socket::INET; our @EXPORT = qw(tmpdir tcp_server tcp_connect require_git require_mods - run_script start_script key2sub xsys xqx eml_load); + run_script start_script key2sub xsys xqx eml_load tick); sub eml_load ($) { my ($path, $cb) = @_; @@ -418,4 +418,8 @@ sub DESTROY { $self->join('TERM'); } +package PublicInbox::TestCommon::InboxWakeup; +use strict; +sub on_inbox_unlock { ${$_[0]}->($_[1]) } + 1; diff --git a/lib/PublicInbox/WatchMaildir.pm b/lib/PublicInbox/WatchMaildir.pm index fea7d5ef9ee..22f190366a4 100644 --- a/lib/PublicInbox/WatchMaildir.pm +++ b/lib/PublicInbox/WatchMaildir.pm @@ -119,19 +119,6 @@ sub _done_for_now { } } -sub _try_fsn_paths { - my ($self, $scan_re, $paths) = @_; - foreach (@$paths) { - my $path = $_->{path}; - if ($path =~ $scan_re) { - scan($self, $path); - } else { - _try_path($self, $path); - } - } - _done_for_now($self); -} - sub remove_eml_i { # each_inbox callback my ($ibx, $arg) = @_; my ($self, $eml, $loc) = @$arg; @@ -225,16 +212,28 @@ sub quit { sub watch_fs { my ($self) = @_; + require PublicInbox::DirIdle; my $scan = File::Temp->newdir("public-inbox-watch.$$.scan.XXXXXX", TMPDIR => 1); my $scandir = $self->{scandir} = $scan->dirname; - my $re = qr!\A$scandir/!; - my $cb = sub { _try_fsn_paths($self, $re, \@_) }; - - eval { require Filesys::Notify::Simple } or - die "Filesys::Notify::Simple is currently required for $0\n"; - my $fsn = Filesys::Notify::Simple->new([@{$self->{mdir}}, $scandir]); - $fsn->wait($cb) until $self->{quit}; + my $scan_re = qr!\A$scandir/!; + my $done = sub { + delete $self->{done_timer}; + _done_for_now($self); + }; + my $cb = sub { + my $path = $_[0]->fullname; + if ($path =~ $scan_re) { + scan($self, $path); + } else { + _try_path($self, $path); + } + $self->{done_timer} //= PublicInbox::DS::requeue($done); + }; + my $di = PublicInbox::DirIdle->new([@{$self->{mdir}}, $scandir], $cb); + PublicInbox::DS->SetPostLoopCallback(sub { !$self->{quit} }); + PublicInbox::DS->EventLoop; + _done_for_now($self); } # returns the git config section name, e.g [imap "imaps://user@example.com"] diff --git a/script/public-inbox-watch b/script/public-inbox-watch index 645abeda971..2057066a2a9 100755 --- a/script/public-inbox-watch +++ b/script/public-inbox-watch @@ -21,6 +21,7 @@ if ($watch_md) { $watch_md->quit if $watch_md; $watch_md = undef; }; - alarm(1); + # --no-scan is only intended for testing atm, undocumented. + alarm(1) unless (grep(/\A--no-scan\z/, @ARGV)); $watch_md->watch while ($watch_md); } diff --git a/t/dir_idle.t b/t/dir_idle.t new file mode 100644 index 00000000000..587599e83ff --- /dev/null +++ b/t/dir_idle.t @@ -0,0 +1,6 @@ +#!perl -w +# Copyright (C) 2020 all contributors +# License: AGPL-3.0+ +use Test::More; +use_ok 'PublicInbox::DirIdle'; +done_testing; diff --git a/t/imapd.t b/t/imapd.t index 3f31743df2e..cc87a127851 100644 --- a/t/imapd.t +++ b/t/imapd.t @@ -462,7 +462,7 @@ ok($mic->logout, 'logged out'); PublicInbox::DS->Reset; my $ii = PublicInbox::InboxIdle->new($cfg); my $cb = sub { PublicInbox::DS->SetPostLoopCallback(sub {}) }; - my $obj = bless \$cb, 'InboxWakeup'; + my $obj = bless \$cb, 'PublicInbox::TestCommon::InboxWakeup'; $cfg->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) }); open my $err, '+>', undef or BAIL_OUT $!; my $w = start_script(['-watch'], undef, { 2 => $err }); @@ -488,7 +488,3 @@ unlike($eout, qr/wide/i, 'no Wide character warnings'); unlike($eout, qr/uninitialized/i, 'no uninitialized warnings'); done_testing; - -package InboxWakeup; -use strict; -sub on_inbox_unlock { ${$_[0]}->() } diff --git a/t/watch_filter_rubylang.t b/t/watch_filter_rubylang.t index 2e7d402e5fa..db48cb2ffde 100644 --- a/t/watch_filter_rubylang.t +++ b/t/watch_filter_rubylang.t @@ -6,7 +6,7 @@ use PublicInbox::TestCommon; use Test::More; use PublicInbox::Eml; use PublicInbox::Config; -require_mods(qw(Filesys::Notify::Simple DBD::SQLite Search::Xapian)); +require_mods(qw(DBD::SQLite Search::Xapian)); use_ok 'PublicInbox::WatchMaildir'; use_ok 'PublicInbox::Emergency'; my ($tmpdir, $for_destroy) = tmpdir(); diff --git a/t/watch_maildir.t b/t/watch_maildir.t index 33a3458bd4d..a2c09b0351b 100644 --- a/t/watch_maildir.t +++ b/t/watch_maildir.t @@ -7,7 +7,6 @@ use Cwd; use PublicInbox::Config; use PublicInbox::TestCommon; use PublicInbox::Import; -require_mods(qw(Filesys::Notify::Simple)); my ($tmpdir, $for_destroy) = tmpdir(); my $git_dir = "$tmpdir/test.git"; my $maildir = "$tmpdir/md"; @@ -47,14 +46,22 @@ EOF 'only got the spam folder to watch'); } -my $config = PublicInbox::Config->new(\<', $cfg_path or BAIL_OUT $!; + print $fh <new($cfg_path); PublicInbox::WatchMaildir->new($config)->scan('full'); my $git = PublicInbox::Git->new($git_dir); my @list = $git->qx(qw(rev-list refs/heads/master)); @@ -136,6 +143,70 @@ More majordomo info at http://vger.kernel.org/majordomo-info.html\n); like($$mref, qr/something\n\z/s, 'message scrubbed on import'); } +# end-to-end test which actually uses inotify/kevent +{ + my $env = { PI_CONFIG => $cfg_path }; + $git->cleanup; + + # n.b. --no-scan is only intended for testing atm + my $wm = start_script([qw(-watch --no-scan)], $env); + my $eml = eml_load('t/data/0001.patch'); + $eml->header_set('Cc', $addr); + my $em = PublicInbox::Emergency->new($maildir); + $em->prepare(\($eml->as_string)); + + use_ok 'PublicInbox::InboxIdle'; + use_ok 'PublicInbox::DS'; + my $delivered = 0; + my $cb = sub { + my ($ibx) = @_; + diag "message delivered to `$ibx->{name}'"; + $delivered++; + }; + PublicInbox::DS->Reset; + my $ii = PublicInbox::InboxIdle->new($config); + my $obj = bless \$cb, 'PublicInbox::TestCommon::InboxWakeup'; + $config->each_inbox(sub { $_[0]->subscribe_unlock('ident', $obj) }); + PublicInbox::DS->SetPostLoopCallback(sub { $delivered == 0 }); + + # wait for -watch to setup inotify watches + my $sleep = 1; + if (eval { require Linux::Inotify2 } && -d "/proc/$wm->{pid}/fd") { + my $end = time + 2; + my (@ino, @ino_info); + do { + @ino = grep { + (readlink($_)//'') =~ /\binotify\b/ + } glob("/proc/$wm->{pid}/fd/*"); + } until (@ino || time > $end || !tick); + if (scalar(@ino) == 1) { + my $ino_fd = (split('/', $ino[0]))[-1]; + my $ino_fdinfo = "/proc/$wm->{pid}/fdinfo/$ino_fd"; + while (time < $end && open(my $fh, '<', $ino_fdinfo)) { + @ino_info = grep(/^inotify wd:/, <$fh>); + last if @ino_info >= 4; + tick; + } + $sleep = undef if @ino_info >= 4; + } + } + if ($sleep) { + diag "waiting ${sleep}s for -watch to start up"; + sleep $sleep; + } + + $em->commit; # wake -watch up + diag 'waiting for -watch to import new message'; + PublicInbox::DS->EventLoop; + $wm->kill; + $wm->join; + $ii->close; + PublicInbox::DS->Reset; + my $head = $git->qx(qw(cat-file commit HEAD)); + my $subj = $eml->header('Subject'); + like($head, qr/^\Q$subj\E/sm, 'new commit made'); +} + sub is_maildir { my ($dir) = @_; PublicInbox::WatchMaildir::is_maildir($dir); diff --git a/t/watch_maildir_v2.t b/t/watch_maildir_v2.t index 19a2da77070..6cc8b6ff0e9 100644 --- a/t/watch_maildir_v2.t +++ b/t/watch_maildir_v2.t @@ -8,7 +8,7 @@ use PublicInbox::Config; use PublicInbox::TestCommon; use PublicInbox::Import; require_git(2.6); -require_mods(qw(Search::Xapian DBD::SQLite Filesys::Notify::Simple)); +require_mods(qw(Search::Xapian DBD::SQLite)); require PublicInbox::V2Writable; my ($tmpdir, $for_destroy) = tmpdir(); my $inboxdir = "$tmpdir/v2"; diff --git a/t/watch_multiple_headers.t b/t/watch_multiple_headers.t index 3a39eba9e0e..0ee96d5ff89 100644 --- a/t/watch_multiple_headers.t +++ b/t/watch_multiple_headers.t @@ -5,7 +5,7 @@ use Test::More; use PublicInbox::Config; use PublicInbox::TestCommon; require_git(2.6); -require_mods(qw(Search::Xapian DBD::SQLite Filesys::Notify::Simple)); +require_mods(qw(Search::Xapian DBD::SQLite)); my ($tmpdir, $for_destroy) = tmpdir(); my $inboxdir = "$tmpdir/v2"; my $maildir = "$tmpdir/md";