diff mbox series

[bug#48033] gnu: public-inbox: Update to 1.6.1.

Message ID 8d66684c511e3a36830465481851f9d86d4d6231.1619439907.git.public@yoctocell.xyz
State Accepted
Headers show
Series [bug#48033] gnu: public-inbox: Update to 1.6.1. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Xinglu Chen April 26, 2021, 12:35 p.m. UTC
* gnu/packages/mail.scm (public-inbox): Update to 1.6.1.
[synopsis]: Capitalize “git”.
[description]: Likewise.
---
I haven’t tested it thoroughly; ‘public-inbox-init‘ and
‘public-inbox-index’ seem to work fine.

Cc: Kyle since you have been somewhat active on the public-inbox ML. :)

 gnu/packages/mail.scm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)


base-commit: f365d48909156ad754a2ade45375f45b54b06bbc

Comments

Kyle Meyer April 27, 2021, 12:59 a.m. UTC | #1
Xinglu Chen writes:

> * gnu/packages/mail.scm (public-inbox): Update to 1.6.1.
> [synopsis]: Capitalize “git”.
> [description]: Likewise.
> ---
> I haven’t tested it thoroughly; ‘public-inbox-init‘ and
> ‘public-inbox-index’ seem to work fine.
>
> Cc: Kyle since you have been somewhat active on the public-inbox ML. :)

Thanks for updating this.

To add to your manual checks above (and to the passing test suite), I
tested out public-inbox-httpd with some inboxes I have locally, and
everything there seems to be working as expected.

I think the perl-filesys-notify-simple input should be dropped because
Filesys::Notify::Simple isn't used as of v1.6.0.  Linux::Inotify2 is
used instead, but it doesn't look like that's packaged for Guix yet.  I
believe public-inbox has some fallback behavior here so that
public-inbox-watch is still functional, but ideally Linux::Inotify2
would be added as an input with this update to 1.6.1.

Otherwise, this patch looks good to me, but fwiw there are a few
optional dependencies (already packaged in Guix) that could be added:

 * highlight (syntax highlight for associated code repositories)
 * perl-inline-c (nice to have for performance reasons)
 * perl-parse-recdescent (as of v1.6.0, used only for public-inbox-imapd)
Kyle Meyer April 27, 2021, 1:09 a.m. UTC | #2
Xinglu Chen writes:

>      (description
> -     "public-inbox implements the sharing of an email inbox via git to
> +     "public-inbox implements the sharing of an email inbox via Git to
>  complement or replace traditional mailing lists.  Readers may read via NNTP,
>  Atom feeds or HTML archives.")

As of 1.6.0, public-inbox supports running an anonymous IMAP server, so
it may be worth adding IMAP to this list.
Xinglu Chen April 27, 2021, 8:21 p.m. UTC | #3
On Mon, Apr 26 2021, Kyle Meyer wrote:

> Thanks for updating this.
>
> To add to your manual checks above (and to the passing test suite), I
> tested out public-inbox-httpd with some inboxes I have locally, and
> everything there seems to be working as expected.

Good to know.

> I think the perl-filesys-notify-simple input should be dropped because
> Filesys::Notify::Simple isn't used as of v1.6.0.  Linux::Inotify2 is
> used instead, but it doesn't look like that's packaged for Guix yet.  I
> believe public-inbox has some fallback behavior here so that
> public-inbox-watch is still functional, but ideally Linux::Inotify2
> would be added as an input with this update to 1.6.1.

Ok, I don't know much about Perl, but I will try to package
Linux::Inotify2.

> Otherwise, this patch looks good to me, but fwiw there are a few
> optional dependencies (already packaged in Guix) that could be added:
>
> * highlight (syntax highlight for associated code repositories)
> * perl-inline-c (nice to have for performance reasons)
> * perl-parse-recdescent (as of v1.6.0, used only for public-inbox-imapd)

Thanks for pointing these out, I will make sure to add them.

>>      (description
>> -     "public-inbox implements the sharing of an email inbox via git to
>> +     "public-inbox implements the sharing of an email inbox via Git to
>>  complement or replace traditional mailing lists.  Readers may read via NNTP,
>>  Atom feeds or HTML archives.")
>
> As of 1.6.0, public-inbox supports running an anonymous IMAP server, so
> it may be worth adding IMAP to this list.

Yeah, will do.

Thanks for the review!
Xinglu Chen April 27, 2021, 9:03 p.m. UTC | #4
On Tue, Apr 27 2021, Xinglu Chen wrote:

>> Otherwise, this patch looks good to me, but fwiw there are a few
>> optional dependencies (already packaged in Guix) that could be added:
>>
>> * highlight (syntax highlight for associated code repositories)
>> * perl-inline-c (nice to have for performance reasons)
>> * perl-parse-recdescent (as of v1.6.0, used only for public-inbox-imapd)
>
> Thanks for pointing these out, I will make sure to add them.

When adding ‘highlight’ to the inputs, one of the tests fail.

--8<---------------cut here---------------start------------->8---
[...]
t/filter_subjecttag.t ........ ok
t/filter_vger.t .............. ok
t/git.t ...................... ok
t/gzip_filter.t .............. ok
failed to open(filetypes.conf): No such file or directory at /tmp/guix-build-public-inbox-1.6.1.drv-0/source/blib/lib/PublicInbox/HlMod.pm line 25.
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 2 just after 1.
t/hl_mod.t ................... 
Dubious, test returned 2 (wstat 512, 0x200)
All 1 subtests passed 
t/html_index.t ............... ok
t/httpd-corner.t ............. ok
[...]

Test Summary Report
-------------------
t/hl_mod.t                 (Wstat: 512 Tests: 1 Failed: 0)
  Non-zero exit status: 2
  Parse errors: No plan found in TAP output
Files=108, Tests=4046, 241 wallclock secs ( 2.26 usr  0.33 sys + 98.46 cusr 114.51 csys = 215.56 CPU)
Result: FAIL
Failed 1/108 test programs. 0/4046 subtests failed.
make: *** [Makefile:1060: test_dynamic] Error 255
--8<---------------cut here---------------end--------------->8---

Any ideas?
Kyle Meyer April 28, 2021, 4:22 a.m. UTC | #5
Xinglu Chen writes:

> When adding ‘highlight’ to the inputs, one of the tests fail.
>
[...]
> failed to open(filetypes.conf): No such file or directory at /tmp/guix-build-public-inbox-1.6.1.drv-0/source/blib/lib/PublicInbox/HlMod.pm line 25.
> # Tests were run but no plan was declared and done_testing() was not seen.
> # Looks like your test exited with 2 just after 1.

Hmm, HlMod.pm is calling highlight's getFiletypesConfPath(), and it
looks like that returns a bare "filetypes.conf" (which doesn't exist)
rather than
"/gnu/store/...-highlight-3.62/share/highlight/config/filetypes.conf".

Here's what public-inbox's call boils down to:

--8<---------------cut here---------------start------------->8---
use highlight;
my $dir = highlight::DataDir->new;
$dir->initSearchDirectories('');
print $dir->getFiletypesConfPath('filetypes')."\n";
--8<---------------cut here---------------end--------------->8---

On my Debian Buster system that prints

  $ perl scratch.pl
  /etc/highlight/filetypes.conf

And when I go through Guix:

  $ guix environment -C --ad-hoc perl highlight -- perl scratch.pl
  filetypes.conf


I'm not sure why that would be.  Guix's highlight definition sets
HL_CONFIG_DIR, and that seems to get wired up correctly:

  $ guix environment -C --ad-hoc highlight -- highlight --print-config

  Config file search directories:
  /gnu/store/6a3jjm38qs80miyqpcb6j9madzy26plg-highlight-3.62/share/highlight/
  /gnu/store/6a3jjm38qs80miyqpcb6j9madzy26plg-highlight-3.62/share/highlight/config/

  Filetype config file:
  /gnu/store/6a3jjm38qs80miyqpcb6j9madzy26plg-highlight-3.62/share/highlight/config/filetypes.conf

  Compiler directive HL_DATA_DIR = /gnu/store/6a3jjm38qs80miyqpcb6j9madzy26plg-highlight-3.62/share/highlight/
  Compiler directive HL_CONFIG_DIR = /gnu/store/6a3jjm38qs80miyqpcb6j9madzy26plg-highlight-3.62/share/highlight/config/
Xinglu Chen April 28, 2021, 1:16 p.m. UTC | #6
On Wed, Apr 28 2021, Kyle Meyer wrote:

> Hmm, HlMod.pm is calling highlight's getFiletypesConfPath(), and it
> looks like that returns a bare "filetypes.conf" (which doesn't exist)
> rather than
> "/gnu/store/...-highlight-3.62/share/highlight/config/filetypes.conf".
>
> Here's what public-inbox's call boils down to:
> [...]

Thanks for looking into this.  I played around with the make flags for
‘highlight’, but that didn’t get me anywhere.

Maybe we could remove ‘highlight’ for now?  Also,
lib/PublicInbox/WwwHighlight.pm has this comment:

  # TODO: support highlight(1) for distros which don't package the
  # SWIG extension.  Also, there may be admins who don't want to
  # have ugly SWIG-generated code in a long-lived Perl process.

Maybe this will become true in the future. :)
Kyle Meyer April 28, 2021, 9:53 p.m. UTC | #7
Xinglu Chen writes:

> Maybe we could remove ‘highlight’ for now?

Sure, not adding it to the current definition sounds fine to me.

> Also,
> lib/PublicInbox/WwwHighlight.pm has this comment:
>
>   # TODO: support highlight(1) for distros which don't package the
>   # SWIG extension.  Also, there may be admins who don't want to
>   # have ugly SWIG-generated code in a long-lived Perl process.
>
> Maybe this will become true in the future. :)

Yeah :)  Or even pygments support:
https://public-inbox.org/meta/20191008215321.GA23962@dcvr/
Xinglu Chen April 29, 2021, 12:18 p.m. UTC | #8
On Wed, Apr 28 2021, Kyle Meyer wrote:

>> Also,
>> lib/PublicInbox/WwwHighlight.pm has this comment:
>>
>>   # TODO: support highlight(1) for distros which don't package the
>>   # SWIG extension.  Also, there may be admins who don't want to
>>   # have ugly SWIG-generated code in a long-lived Perl process.
>>
>> Maybe this will become true in the future. :)
>
> Yeah :)  Or even pygments support:
> https://public-inbox.org/meta/20191008215321.GA23962@dcvr/

That would be even better :)

I will send an updated series for this.
Xinglu Chen April 29, 2021, 12:37 p.m. UTC | #9
Changes since v1:
- Add some optional dependencies for ‘public-inbox’.

- Follow upstream and replace ‘perl-filesys-notify-simple’ with
  ‘perl-linux-inotify2’.

- Mention IMAP support in the description.

Xinglu Chen (2):
  gnu: Add perl-linux-inotify2.
  gnu: public-inbox: Update to 1.6.1.

 gnu/packages/mail.scm | 19 +++++++++++++------
 gnu/packages/perl.scm | 23 +++++++++++++++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)


base-commit: 510fbf7f9360ec5c7b93a45c2fa7ce168a15d9ae
Xinglu Chen May 1, 2021, 12:04 p.m. UTC | #10
Changes since v2:
- Removed unused import (gnu packages pretty-print) from (gnu packages
mail).

Xinglu Chen (2):
  gnu: Add perl-linux-inotify2.
  gnu: public-inbox: Update to 1.6.1.

 gnu/packages/mail.scm | 18 ++++++++++++------
 gnu/packages/perl.scm | 23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)


base-commit: c04dfb39f62f764b1c3988d9b0fcedc8221afa67
Ludovic Courtès May 3, 2021, 4:27 p.m. UTC | #11
Hi,

Xinglu Chen <public@yoctocell.xyz> skribis:

> Xinglu Chen (2):
>   gnu: Add perl-linux-inotify2.
>   gnu: public-inbox: Update to 1.6.1.
>
>  gnu/packages/mail.scm | 18 ++++++++++++------
>  gnu/packages/perl.scm | 23 +++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 6 deletions(-)

Applied both patches, thank you, and thanks Kyle for reviewing!

Ludo’.
diff mbox series

Patch

diff --git a/gnu/packages/mail.scm b/gnu/packages/mail.scm
index 64c84d145d..43291ac267 100644
--- a/gnu/packages/mail.scm
+++ b/gnu/packages/mail.scm
@@ -4082,7 +4082,7 @@  Git and exports them in maildir format or to an MDA through a pipe.")
 (define-public public-inbox
   (package
     (name "public-inbox")
-    (version "1.5.0")
+    (version "1.6.1")
     (source
      (origin (method git-fetch)
              (uri (git-reference
@@ -4090,7 +4090,7 @@  Git and exports them in maildir format or to an MDA through a pipe.")
                    (commit (string-append "v" version))))
              (sha256
               (base32
-               "03zj7shdl3vibs7k5lr673bwcf8j1xx8is3mjz34ca4cdh6p5j2k"))
+               "0mlwnp5knr7rk9kv8grlh342wsq2193m22zs83cjn9p7x9r2x5f9"))
              (file-name (git-file-name name version))))
     (build-system perl-build-system)
     (arguments
@@ -4156,9 +4156,9 @@  Git and exports them in maildir format or to an MDA through a pipe.")
        ("perl-ipc-run" ,perl-ipc-run)
        ("perl-xml-feed" ,perl-xml-feed)))
     (home-page "https://public-inbox.org/README.html")
-    (synopsis "Archive mailing lists in git repositories")
+    (synopsis "Archive mailing lists in Git repositories")
     (description
-     "public-inbox implements the sharing of an email inbox via git to
+     "public-inbox implements the sharing of an email inbox via Git to
 complement or replace traditional mailing lists.  Readers may read via NNTP,
 Atom feeds or HTML archives.")
     (license license:agpl3+)))