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-ASN: 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 DA75C1F5AE; Wed, 21 Jul 2021 14:07:06 +0000 (UTC) Date: Wed, 21 Jul 2021 14:07:06 +0000 From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 3/2] lei: auto-refresh watches in config, cancel missing Message-ID: <20210721140706.GA9975@dcvr> References: <20210719085935.1986-1-e@80x24.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210719085935.1986-1-e@80x24.org> List-Id: This makes behavior less surprising on restarts as we no longer lose state on restarts, so there's no need to manually run "lei add-watch" to re-enable watches. This also allows us to transparently handle changes if somebody edits the lei config file directly or via git-config(1). --- lib/PublicInbox/DirIdle.pm | 5 +++- lib/PublicInbox/LEI.pm | 50 +++++++++++++++++++++++++-------- lib/PublicInbox/LeiNoteEvent.pm | 2 +- t/lei-watch.t | 39 +++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 13 deletions(-) diff --git a/lib/PublicInbox/DirIdle.pm b/lib/PublicInbox/DirIdle.pm index 7031e5fd..65896f95 100644 --- a/lib/PublicInbox/DirIdle.pm +++ b/lib/PublicInbox/DirIdle.pm @@ -56,10 +56,13 @@ sub new { sub add_watches { my ($self, $dirs, $gone) = @_; my $fl = $MAIL_IN | ($gone ? $MAIL_GONE : 0); + my @ret; for my $d (@$dirs) { - $self->{inot}->watch($d, $fl); + my $w = $self->{inot}->watch($d, $fl) or next; + push @ret, $w; } PublicInbox::FakeInotify::poll_once($self) if !$ino_cls; + @ret } sub rm_watches { diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm index b92d7512..52c551cf 100644 --- a/lib/PublicInbox/LEI.pm +++ b/lib/PublicInbox/LEI.pm @@ -36,7 +36,7 @@ my $GLP_PASS = Getopt::Long::Parser->new; $GLP_PASS->configure(qw(gnu_getopt no_ignore_case auto_abbrev pass_through)); our %PATH2CFG; # persistent for socket daemon -our $MDIR2CFGPATH; # /path/to/maildir => { /path/to/config => undef } +our $MDIR2CFGPATH; # /path/to/maildir => { /path/to/config => [ ino watches ] } # TBD: this is a documentation mechanism to show a subcommand # (may) pass options through to another command: @@ -820,6 +820,8 @@ sub _lei_cfg ($;$) { } } $self->{cfg} = $PATH2CFG{$f} = $cfg; + refresh_watches($self); + $cfg; } sub _lei_store ($;$) { @@ -1353,36 +1355,62 @@ sub watch_state_ok ($) { $state =~ /\Apause|(?:import|index|tag)-(?:ro|rw)\z/; } +sub cancel_maildir_watch ($$) { + my ($d, $cfg_f) = @_; + my $w = delete $MDIR2CFGPATH->{$d}->{$cfg_f}; + scalar(keys %{$MDIR2CFGPATH->{$d}}) or + delete $MDIR2CFGPATH->{$d}; + for my $x (@{$w // []}) { $x->cancel } +} + sub refresh_watches { my ($lei) = @_; my $cfg = _lei_cfg($lei) or return; - $cfg->{-env} //= { %{$lei->{env}}, PWD => '/' }; # for cfg2lei + my $old = $cfg->{-watches}; my $watches = $cfg->{-watches} //= {}; - require PublicInbox::LeiWatch; + my %seen; + my $cfg_f = $cfg->{'-f'}; for my $w (grep(/\Awatch\..+\.state\z/, keys %$cfg)) { my $url = substr($w, length('watch.'), -length('.state')); - my $lw = $watches->{$w} //= PublicInbox::LeiWatch->new($url); + require PublicInbox::LeiWatch; + my $lw = $watches->{$url} //= PublicInbox::LeiWatch->new($url); + $seen{$url} = undef; my $state = $cfg->get_1("watch.$url", 'state'); if (!watch_state_ok($state)) { $lei->err("watch.$url.state=$state not supported"); next; } - my $f = $cfg->{'-f'}; if ($url =~ /\Amaildir:(.+)/i) { my $d = File::Spec->canonpath($1); if ($state eq 'pause') { - delete $MDIR2CFGPATH->{$d}->{$f}; - scalar(keys %{$MDIR2CFGPATH->{$d}}) or - delete $MDIR2CFGPATH->{$d}; - } elsif (!exists($MDIR2CFGPATH->{$d}->{$f})) { - $dir_idle->add_watches(["$d/cur", "$d/new"], 1); - $MDIR2CFGPATH->{$d}->{$f} = undef; + cancel_maildir_watch($d, $cfg_f); + } elsif (!exists($MDIR2CFGPATH->{$d}->{$cfg_f})) { + my @w = $dir_idle->add_watches( + ["$d/cur", "$d/new"], 1); + push @{$MDIR2CFGPATH->{$d}->{$cfg_f}}, @w if @w; } } else { # TODO: imap/nntp/jmap $lei->child_error(1, "E: watch $url not supported, yet"); } } + if ($old) { # cull old non-existent entries + for my $url (keys %$old) { + next if exists $seen{$url}; + delete $old->{$url}; + if ($url =~ /\Amaildir:(.+)/i) { + my $d = File::Spec->canonpath($1); + cancel_maildir_watch($d, $cfg_f); + } else { # TODO: imap/nntp/jmap + $lei->child_error(1, "E: watch $url TODO"); + } + } + } + if (scalar keys %$watches) { + $cfg->{-env} //= { %{$lei->{env}}, PWD => '/' }; # for cfg2lei + } else { + delete $cfg->{-watches}; + } } 1; diff --git a/lib/PublicInbox/LeiNoteEvent.pm b/lib/PublicInbox/LeiNoteEvent.pm index bf15cd26..d6511cf6 100644 --- a/lib/PublicInbox/LeiNoteEvent.pm +++ b/lib/PublicInbox/LeiNoteEvent.pm @@ -14,7 +14,7 @@ sub flush_lei ($) { my ($lei) = @_; if (my $lne = delete $lei->{cfg}->{-lei_note_event}) { $lne->wq_close(1, undef, $lei); # runs _lei_wq_eof; - } else { # lms_clear_src calls only: + } elsif ($lei->{sto}) { # lms_clear_src calls only: my $wait = $lei->{sto}->ipc_do('done'); } } diff --git a/t/lei-watch.t b/t/lei-watch.t index 3a2f9e64..492f6c1d 100644 --- a/t/lei-watch.t +++ b/t/lei-watch.t @@ -13,9 +13,27 @@ $have_fast_inotify or my ($ro_home, $cfg_path) = setup_public_inboxes; test_lei(sub { my $md = "$ENV{HOME}/md"; + my $cfg_f = "$ENV{HOME}/.config/lei/config"; my $md2 = $md.'2'; lei_ok 'ls-watch'; is($lei_out, '', 'nothing in ls-watch, yet'); + + my ($ino_fdinfo, $ino_contents); + SKIP: { + $have_fast_inotify && $^O eq 'linux' or + skip 'Linux/inotify-only internals check', 1; + lei_ok 'daemon-pid'; chomp(my $pid = $lei_out); + skip 'missing /proc/$PID/fd', 1 if !-d "/proc/$pid/fd"; + my @ino = grep { + readlink($_) =~ /\binotify\b/ + } glob("/proc/$pid/fd/*"); + is(scalar(@ino), 1, 'only one inotify FD'); + my $ino_fd = (split('/', $ino[0]))[-1]; + $ino_fdinfo = "/proc/$pid/fdinfo/$ino_fd"; + open my $fh, '<', $ino_fdinfo or xbail "open $ino_fdinfo: $!"; + $ino_contents = [ <$fh> ]; + } + if (0) { # TODO my $url = 'imaps://example.com/foo.bar.0'; lei_ok([qw(add-watch --state=pause), $url], undef, {}); @@ -44,6 +62,27 @@ test_lei(sub { my $e2 = eml_load($f2[0]); my $e1 = eml_load("$f[0]S"); is_deeply($e2, $e1, 'results match'); + + SKIP: { + $ino_fdinfo or skip 'Linux/inotify-only watch check', 1; + open my $fh, '<', $ino_fdinfo or xbail "open $ino_fdinfo: $!"; + my $cmp = [ <$fh> ]; + ok(scalar(@$cmp) > scalar(@$ino_contents), + 'inotify has Maildir watches'); + } + + is(xsys(qw(git config -f), $cfg_f, + '--remove-section', "watch.maildir:$md"), + 0, 'unset config state'); + lei_ok 'ls-watch', \'refresh watches'; + is($lei_out, '', 'no watches left'); + + SKIP: { + $ino_fdinfo or skip 'Linux/inotify-only removal removal', 1; + open my $fh, '<', $ino_fdinfo or xbail "open $ino_fdinfo: $!"; + my $cmp = [ <$fh> ]; + is_deeply($cmp, $ino_contents, 'inotify Maildir watches gone'); + }; }); done_testing;