[PATCH] t/lei-import: skip strace for restricted systems

2023-11-10 Thread Eric Wong
Systems with Yama can restrict ptrace(2) (the underlying syscall
used by strace(1)) and make it difficult to test error handling
via error injection.  Just skip the tests on such systems since
it's probably not worth the effort to start using prctl(2) to
enable the test on such systems.
---
 lib/PublicInbox/TestCommon.pm | 18 +++---
 t/lei-import.t|  7 +++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/lib/PublicInbox/TestCommon.pm b/lib/PublicInbox/TestCommon.pm
index 46e6a538..b84886a0 100644
--- a/lib/PublicInbox/TestCommon.pm
+++ b/lib/PublicInbox/TestCommon.pm
@@ -935,13 +935,25 @@ sub cfg_new ($;@) {
 }
 
 our $strace_cmd;
-sub strace () {
+sub strace (@) {
+   my ($for_daemon) = @_;
skip 'linux only test' if $^O ne 'linux';
+   if ($for_daemon) {
+   my $f = '/proc/sys/kernel/yama/ptrace_scope';
+   # TODO: we could fiddle with prctl in the daemon to make
+   # things work, but I'm not sure it's worth it...
+   state $ps = do {
+   my $fh;
+   CORE::open($fh, '<', $f) ? readline($fh) : 0;
+   };
+   chomp $ps;
+   skip "strace unusable on daemons\n$f is `$ps' (!= 0)" if $ps;
+   }
require_cmd('strace', 1);
 }
 
-sub strace_inject () {
-   my $cmd = strace;
+sub strace_inject (;$) {
+   my $cmd = strace(@_);
state $ver = do {
require PublicInbox::Spawn;
my $v = PublicInbox::Spawn::run_qx([$cmd, '--version']);
diff --git a/t/lei-import.t b/t/lei-import.t
index 6ad4c97b..1edd607d 100644
--- a/t/lei-import.t
+++ b/t/lei-import.t
@@ -155,19 +155,18 @@ do {
 like($lei_out, qr/\bbin\b/, 'commit-delay eventually commits');
 
 SKIP: {
-   my $strace = strace_inject; # skips if strace is old or non-Linux
+   my $strace = strace_inject(1); # skips if strace is old or non-Linux
my $tmpdir = tmpdir;
my $tr = "$tmpdir/tr";
-   my $cmd = [ $strace, "-o$tr", '-f',
+   my $cmd = [ $strace, '-q', "-o$tr", '-f',
"-P", File::Spec->rel2abs('t/plack-qp.eml'),
'-e', 'inject=readv,read:error=EIO'];
lei_ok qw(daemon-pid);
chomp(my $daemon_pid = $lei_out);
push @$cmd, '-p', $daemon_pid;
-   my $strace_opt = { 1 => \my $out, 2 => \my $err };
require PublicInbox::Spawn;
require PublicInbox::AutoReap;
-   my $pid = PublicInbox::Spawn::spawn($cmd, \%ENV, $strace_opt);
+   my $pid = PublicInbox::Spawn::spawn($cmd, \%ENV);
my $ar = PublicInbox::AutoReap->new($pid);
tick; # wait for strace to attach
ok(!lei(qw(import -F eml t/plack-qp.eml)),



Re: [RFC v2] www: add topics_(new|active).(html|atom) endpoints

2023-11-10 Thread Eric Wong
Konstantin Ryabitsev  wrote:
> On Fri, Nov 10, 2023 at 03:09:59AM +, Eric Wong wrote:
> > That said, the Atom feeds generated by this RFC includes full
> > messages because that's the easiest way to tie into our existing
> > Atom generation code, so it's currently slower than the HTML
> > version which never retrieves git blobs.
> 
> That's fine, actually, because this lets people read the full message to
> figure out if they are interested in the rest of the thread or not.

Alright, pushed out as commit 363c043a8a3f379a69802fc566112fcd8f1e750c



Re: [PATCH] public-inbox-mda: use status codes where applicable

2023-11-10 Thread Leah Neukirchen
Eric Wong  writes:

> Leah Neukirchen  wrote:
>> Many MTA understand these and map them to sensible SMTP error messages.
>> 
>> Inability to find an inbox results in "5.1.1 user unknown".
>> Misformatted messages are rejected with "5.6.0 data format error".
>> Unsupported inbox versions are reported as "5.3.5 local configuration error".
>> 
>> All of these are interpreted as permanent failures.
>
> Resurrecting an ancient topic...
>
>> diff --git a/script/public-inbox-mda b/script/public-inbox-mda
>> index 766d58a..1f1252a 100755
>> --- a/script/public-inbox-mda
>> +++ b/script/public-inbox-mda
>> @@ -38,8 +38,8 @@ my $config = PublicInbox::Config->new;
>>  my $recipient = $ENV{ORIGINAL_RECIPIENT};
>>  defined $recipient or die "ORIGINAL_RECIPIENT not defined in ENV\n";
>
> Btw, our current code still dies if ORIGINAL_RECIPIENT is unset
> instead of using a sysexit.h code.
>
> Should that be changed to EX_USAGE or EX_NOUSER instead of die?

I guess EX_NOUSER is appropriate here.

-- 
Leah Neukirchenhttps://leahneukirchen.org



Re: [PATCH] public-inbox-mda: use status codes where applicable

2023-11-10 Thread Eric Wong
Leah Neukirchen  wrote:
> Many MTA understand these and map them to sensible SMTP error messages.
> 
> Inability to find an inbox results in "5.1.1 user unknown".
> Misformatted messages are rejected with "5.6.0 data format error".
> Unsupported inbox versions are reported as "5.3.5 local configuration error".
> 
> All of these are interpreted as permanent failures.

Resurrecting an ancient topic...

> diff --git a/script/public-inbox-mda b/script/public-inbox-mda
> index 766d58a..1f1252a 100755
> --- a/script/public-inbox-mda
> +++ b/script/public-inbox-mda
> @@ -38,8 +38,8 @@ my $config = PublicInbox::Config->new;
>  my $recipient = $ENV{ORIGINAL_RECIPIENT};
>  defined $recipient or die "ORIGINAL_RECIPIENT not defined in ENV\n";

Btw, our current code still dies if ORIGINAL_RECIPIENT is unset
instead of using a sysexit.h code.

Should that be changed to EX_USAGE or EX_NOUSER instead of die?

Since we already use EX_NOUSER right below:

>  my $dst = $config->lookup($recipient); # first check
> -defined $dst or do_exit(1);
> -my $main_repo = $dst->{mainrepo} or do_exit(1);
> +defined $dst or do_exit(67); # EX_NOUSER 5.1.1 user unknown
> +my $main_repo = $dst->{mainrepo} or do_exit(67);

Just something I noticed this while making unrelated changes to -mda...



Re: [RFC v2] www: add topics_(new|active).(html|atom) endpoints

2023-11-10 Thread Konstantin Ryabitsev
On Fri, Nov 10, 2023 at 03:09:59AM +, Eric Wong wrote:
> > Yes, actually thinking about this some more, perhaps it makes sense to 
> > expose
> > this as an RSS feed feature (maybe even exclusively as an RSS feed 
> > feature?).
> 
> I assume Atom is OK?  I don't know of any widely-used feed readers
> which only do RSS without Atom support.  IIRC Atom is less ambiguous
> and supports the in-reply-to extension.

Yes, sorry, I know they aren't the same thing, but in my head Atom is just a
form of RSS (perhaps for the same reason why everyone says "rss reader" but
nobody says "atom reader").

> That said, the Atom feeds generated by this RFC includes full
> messages because that's the easiest way to tie into our existing
> Atom generation code, so it's currently slower than the HTML
> version which never retrieves git blobs.

That's fine, actually, because this lets people read the full message to
figure out if they are interested in the rest of the thread or not.

> > Have two different feeds:
> > 
> > - new topics: just all the new threads
> > - hot topics: NN most active threads (kinda lkml.org's "hottest messages")
> 
> I'm not sure if `hot' means it's the most read (not just replied-to);
> but tracking read counts isn't something that scales on decentralized
> systems.  So I'm naming it "active" instead...

Sounds good to me.

> > Have this available per-list and for the extindex -- I think this would be
> > a great feature that we can point people at as a mechanism to keep an eye on
> > overall activity.
> 
> Yeah, lots of the WWW and lei code works transparently between extindex
> and regular inboxes:
> 
> extindex:
> https://yhbt.net/lore/all/topics_new.atom
> https://yhbt.net/lore/all/topics_active.atom
> https://yhbt.net/lore/all/topics_new.html
> https://yhbt.net/lore/all/topics_active.html
> 
> v2:
> https://yhbt.net/lore/lkml/topics_new.atom
> https://yhbt.net/lore/lkml/topics_active.atom
> https://yhbt.net/lore/lkml/topics_new.html
> https://yhbt.net/lore/lkml/topics_active.html
> 
> v1:
> https://public-inbox.org/git/topics_new.atom
> https://public-inbox.org/git/topics_active.atom
> https://public-inbox.org/git/topics_new.html
> https://public-inbox.org/git/topics_active.html

This is great, thank you!

-K