[bug#49494,7/7] gnu: Add nncp.

* gnu/packages/uucp.scm (nncp): New variable.
---
 gnu/packages/uucp.scm | 99 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

Hi,

I have a few suggestions for this one as well.

Arun Isaac <arunisaac@systemreboot.net> writes:

> * gnu/packages/uucp.scm (nncp): New variable.
> ---
>  gnu/packages/uucp.scm | 99 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
>
> diff --git a/gnu/packages/uucp.scm b/gnu/packages/uucp.scm
> index 1bb4fdb975..2d4d1ae8d0 100644
> --- a/gnu/packages/uucp.scm
> +++ b/gnu/packages/uucp.scm
> @@ -1,5 +1,6 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2014 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2021 Arun Isaac <arunisaac@systemreboot.net>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -17,6 +18,8 @@
>  ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
>  
>  (define-module (gnu packages uucp)
> +  #:use-module (gnu packages golang)
> +  #:use-module (gnu packages texinfo)
>    #:use-module (guix licenses)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> @@ -54,3 +57,99 @@
>  set of utilities for remotely transferring files, email and net news
>  between computers.")
>      (license gpl2+)))
> +
> +(define-public nncp
> +  (package
> +    (name "nncp")
> +    (version "7.2.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "http://www.nncpgo.org/download/nncp-"
> +                           version ".tar.xz"))
> +       (sha256
> +        (base32
> +         "0xsh5zc6i8nbcsi06r65lpp26hz2zb4vh0pzbkivdd69hrxvknvh"))
> +       (modules '((ice-9 ftw)
> +                  (guix build utils)))
> +       (snippet
> +        '(begin
> +           ;; Unbundle dependencies.
> +           ;; TODO: go.cypherpunks.ru was down at the time of
> +           ;; packaging. Unbundle go.cypherpunks dependencies as well once it
> +           ;; comes back online.
> +           (for-each (lambda (file)
> +                       (unless (member file (list "." ".." "go.cypherpunks.ru"))
> +                         (delete-file-recursively (string-append "src/vendor/" file))))
> +                     (scandir "src/vendor"))
> +           ;; Delete built documentation.
> +           (delete-file "doc/nncp.info")
> +           #t))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f                      ; tests fail

It is not a good idea to just disable tests without knowing why they
fail (and leaving a comment explaining why). 

> +       #:modules ((guix build gnu-build-system)
> +                  ((guix build go-build-system) #:prefix go:)
> +                  (guix build union)
                     ^ this module isn't necessary

> +                  (guix build utils))
> +       #:imported-modules (,@%gnu-build-system-modules
> +                           (guix build union)
> +                           (guix build go-build-system))

This can probably just be
  #:imported-modules ,%go-build-system-modules

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'unpack 'setup-go-environment
> +           (assoc-ref go:%standard-phases 'setup-go-environment))
> +         (add-after 'unpack 'go-unpack
> +           (lambda* (#:key source #:allow-other-keys)
> +             ;; Copy source to GOPATH.
> +             (copy-recursively "src" "../src/go.cypherpunks.ru/nncp/v7")
> +             ;; Move bundled dependencies to GOPATH.
> +             (for-each (lambda (dependency)
> +                         (rename-file (string-append "src/vendor/go.cypherpunks.ru/"
> +                                                     dependency)
> +                                      (string-append "../src/go.cypherpunks.ru/"
> +                                                     dependency)))
> +                       (list "balloon" "recfile"))
> +             ;; Delete empty bundled dependencies directory.
> +             (delete-file-recursively "src/vendor")))
> +         (replace 'configure
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             ;; Set output directories.
> +             (let ((out (assoc-ref outputs "out")))
> +               (setenv "BINDIR" (string-append out "/bin"))
> +               (setenv "INFODIR" (string-append out "/share/info"))
> +               (setenv "DOCDIR" (string-append out "/share/doc/nncp")))

Consider perhaps:
  (setenv "DOCDIR" (string-append out "/share/doc/nncp"
                                  ,(package-version this-package)))

Does CFGPATH need to be set?

> +             ;; Remove module flags.
> +             (substitute* (list "bin/default.do" "test.do")
> +               ((" -mod=vendor") "")
> +               ((" -m") "")))))))

I took a quick look at the source and it looks like you'll also need:

  (substitute* '("src/toss_test.go" "src/pipe.go")
    (("/bin/sh") (which "sh")))
  (substitute* "src/toss_test.go"
    (("; cat") (string-append "; " (which "cat"))))

Which also makes the tests succeed.

> +    (inputs
> +     `(("go" ,go)))
> +    (native-inputs
> +     `(("texinfo" ,texinfo)))
> +    (propagated-inputs
> +     `(("go-github-com-davecgh-go-xdr" ,go-github-com-davecgh-go-xdr)
> +       ("go-github-com-dustin-go-humanize" ,go-github-com-dustin-go-humanize)
> +       ("go-github-com-flynn-noise" ,go-github-com-flynn-noise)
> +       ("go-github-com-gorhill-cronexpr" ,go-github-com-gorhill-cronexpr)
> +       ("go-github-com-hjson-hjson-go" ,go-github-com-hjson-hjson-go)
> +       ("go-github-com-klauspost-compress" ,go-github-com-klauspost-compress)
> +       ("go-golang-org-x-crypto" ,go-golang-org-x-crypto)
> +       ("go-golang-org-x-net" ,go-golang-org-x-net)
> +       ("go-golang-org-x-term" ,go-golang-org-x-term)
> +       ("go-lukechampine-com-blake3" ,go-lukechampine-com-blake3)))

Since this is an end-user package, these can be regular inputs.

I also notice that nncp can use `sendmail`; should `sendmail` be an
input as well?

> +    (home-page "http://www.nncpgo.org/")
> +    (synopsis "Store and forward utilities")
> +    (description "NNCP (Node to Node copy) is a collection of utilities
> +simplifying secure store-and-forward files, mail and command exchanging.
> +These utilities are intended to help build up small size (dozens of nodes)
> +ad-hoc friend-to-friend (F2F) statically routed darknet delay-tolerant
> +networks for fire-and-forget secure reliable files, file requests, Internet
> +mail and commands transmission.  All packets are integrity checked, end-to-end
> +encrypted, explicitly authenticated by known participants public keys.  Onion
> +encryption is applied to relayed packets.  Each node acts both as a client and
> +server, can use push and poll behaviour model.  Multicasting areas, offline
> +sneakernet/floppynet, dead drops, sequential and append-only CD-ROM/tape
> +storages, air-gapped computers and online TCP daemon with full-duplex
> +resumable data transmission exists are all supported.")
> +    (license gpl3)))

This package is also retaining references to the Go compiler package;
re-adding this phase from go-build-system fixes that:

  (add-after 'install 'remove-go-references
             (assoc-ref go:%standard-phases 'remove-go-references))

--
Sarah

Hi Sarah,

I have pushed patches 1-6 to master after implementing your suggestion
for patch 6 (klauspost-compress). I'm sending a WIP v2 of patch 7 (nncp)
in a following email. The tests are failing despite implementing your
suggestion. Any help in that regard would be much appreciated.

>> +(define-public nncp
>> +  (package
>> +    (name "nncp")
>> +    (version "7.2.0")

In patch v2, I have updated to the latest version 7.5.0.

>> +    (build-system gnu-build-system)
>> +    (arguments
>> +     `(#:tests? #f                      ; tests fail
>
> It is not a good idea to just disable tests without knowing why they
> fail (and leaving a comment explaining why).

True, I agree.

>> +       #:modules ((guix build gnu-build-system)
>> +                  ((guix build go-build-system) #:prefix go:)
>> +                  (guix build union)
>                      ^ this module isn't necessary
>

[...]

>> +                  (guix build utils))
>> +       #:imported-modules (,@%gnu-build-system-modules
>> +                           (guix build union)
>> +                           (guix build go-build-system))
>
> This can probably just be
>   #:imported-modules ,%go-build-system-modules

Good catch! Implemented both suggestions.

>> +               (setenv "BINDIR" (string-append out "/bin"))
>> +               (setenv "INFODIR" (string-append out "/share/info"))
>> +               (setenv "DOCDIR" (string-append out "/share/doc/nncp")))
>
> Consider perhaps:
>   (setenv "DOCDIR" (string-append out "/share/doc/nncp"
>                                   ,(package-version this-package)))

I've removed the version number from DOCDIR since that's what most
packages are doing. Even the configure phase of the gnu-build-system
does not put the version number in docdir. Only the
install-license-files of the gnu-build-system puts the version number
in, and that's probably a bug.

> Does CFGPATH need to be set?

I have now set CFGPATH TO /etc/nncp.hjson.

> I took a quick look at the source and it looks like you'll also need:
>
>   (substitute* '("src/toss_test.go" "src/pipe.go")
>     (("/bin/sh") (which "sh")))
>   (substitute* "src/toss_test.go"
>     (("; cat") (string-append "; " (which "cat"))))
>
> Which also makes the tests succeed.

Good catch, but tests still don't succeed (at least on my machine).

>> +    (inputs
>> +     `(("go" ,go)))

I have moved go to native-inputs.

>> +    (native-inputs
>> +     `(("texinfo" ,texinfo)))
>> +    (propagated-inputs
>> +     `(("go-github-com-davecgh-go-xdr" ,go-github-com-davecgh-go-xdr)
>> +       ("go-github-com-dustin-go-humanize" ,go-github-com-dustin-go-humanize)
>> +       ("go-github-com-flynn-noise" ,go-github-com-flynn-noise)
>> +       ("go-github-com-gorhill-cronexpr" ,go-github-com-gorhill-cronexpr)
>> +       ("go-github-com-hjson-hjson-go" ,go-github-com-hjson-hjson-go)
>> +       ("go-github-com-klauspost-compress" ,go-github-com-klauspost-compress)
>> +       ("go-golang-org-x-crypto" ,go-golang-org-x-crypto)
>> +       ("go-golang-org-x-net" ,go-golang-org-x-net)
>> +       ("go-golang-org-x-term" ,go-golang-org-x-term)
>> +       ("go-lukechampine-com-blake3" ,go-lukechampine-com-blake3)))
>
> Since this is an end-user package, these can be regular inputs.

Done!

> I also notice that nncp can use `sendmail`; should `sendmail` be an
> input as well?

I think sendmail need not be an input. There are many sendmail
compatible implementations and we can leave it up to the user to install
one in their profile and configure nncp accordingly.

> This package is also retaining references to the Go compiler package;
> re-adding this phase from go-build-system fixes that:
>
>   (add-after 'install 'remove-go-references
>              (assoc-ref go:%standard-phases 'remove-go-references))

Done!

Thanks,
Arun

Hi,

Arun Isaac <arunisaac@systemreboot.net> writes:

> Hi Sarah,
>
> I have pushed patches 1-6 to master after implementing your suggestion
> for patch 6 (klauspost-compress). I'm sending a WIP v2 of patch 7 (nncp)
> in a following email. The tests are failing despite implementing your
> suggestion. Any help in that regard would be much appreciated.

Your patch applies on master (fba107e), builds, and passes tests for me;
I'm on x86-64. What's your platform?

[...]

> I've removed the version number from DOCDIR since that's what most
> packages are doing. Even the configure phase of the gnu-build-system
> does not put the version number in docdir. Only the
> install-license-files of the gnu-build-system puts the version number
> in, and that's probably a bug.

Hmm, something to investigate.

>
>> Does CFGPATH need to be set?
>
> I have now set CFGPATH TO /etc/nncp.hjson.

When I run any of the executables, I get:

main.go:73: Error during initialization: stat /usr/local/etc/nncp.hjson:
no such file or directory

[...]

>> I also notice that nncp can use `sendmail`; should `sendmail` be an
>> input as well?
>
> I think sendmail need not be an input. There are many sendmail
> compatible implementations and we can leave it up to the user to install
> one in their profile and configure nncp accordingly.

Makes sense.

> Thanks,
> Arun

Glad to be of help :)

--
Sarah

Hi Sarah,

> Your patch applies on master (fba107e), builds, and passes tests for me;
> I'm on x86-64. What's your platform?

I'm on x86_64 too. I tried on master eb46c6c5c8. I have attached the
complete build log. The tests seem to have timed out after 10
minutes. Could it be something to do with memory usage? I have 4 GB of
RAM.

>> I've removed the version number from DOCDIR since that's what most
>> packages are doing. Even the configure phase of the gnu-build-system
>> does not put the version number in docdir. Only the
>> install-license-files of the gnu-build-system puts the version number
>> in, and that's probably a bug.
>
> Hmm, something to investigate.

Yes, a patch would be welcome. :-)

>>> Does CFGPATH need to be set?
>>
>> I have now set CFGPATH TO /etc/nncp.hjson.
>
> When I run any of the executables, I get:
>
> main.go:73: Error during initialization: stat /usr/local/etc/nncp.hjson:
> no such file or directory

I couldn't reproduce this error message. Which command are you running?
But, I do see that the output of say `nncp-xfer` lists
/usr/local/etc/nncp.hjson as the default configuration path. I will
investigate.

Thanks!
Arun

Hi Sarah,

>> Your patch applies on master (fba107e), builds, and passes tests for me;
>> I'm on x86-64. What's your platform?
>
> I'm on x86_64 too. I tried on master eb46c6c5c8. I have attached the
> complete build log. The tests seem to have timed out after 10
> minutes. Could it be something to do with memory usage? I have 4 GB of
> RAM.

I set up a tmpfs file system at /tmp. And, curiously, now the package
builds successfully. No idea why this is happening, though.

>>>> Does CFGPATH need to be set?
>>>
>>> I have now set CFGPATH TO /etc/nncp.hjson.
>>
>> When I run any of the executables, I get:
>>
>> main.go:73: Error during initialization: stat /usr/local/etc/nncp.hjson:
>> no such file or directory
>
> I couldn't reproduce this error message. Which command are you running?
> But, I do see that the output of say `nncp-xfer` lists
> /usr/local/etc/nncp.hjson as the default configuration path. I will
> investigate.

That leaves only the CFGPATH issue. I'll figure it out and send a v3
patch.

Regards,
Arun

Hi Arun,

Arun Isaac <arunisaac@systemreboot.net> writes:

> Hi Sarah,
>
>>> Your patch applies on master (fba107e), builds, and passes tests for me;
>>> I'm on x86-64. What's your platform?
>>
>> I'm on x86_64 too. I tried on master eb46c6c5c8. I have attached the
>> complete build log. The tests seem to have timed out after 10
>> minutes. Could it be something to do with memory usage? I have 4 GB of
>> RAM.
>
> I set up a tmpfs file system at /tmp. And, curiously, now the package
> builds successfully. No idea why this is happening, though.

I actually just figured this out as well. You're on a rotational disk,
aren't you? Since nncp makes heavy use of the spool file (which I think
it mocks in /tmp for testing), I think that's why your tests are taking
so long and timing out. Tests complete for me on a SSD in ~40s. (All the
error messages are red herrings; they're expected errors.) If this is
the case, it might be worth it to increase the test timeout for those
who use --no-substitutes.

>
>>>>> Does CFGPATH need to be set?
>>>>
>>>> I have now set CFGPATH TO /etc/nncp.hjson.
>>>
>>> When I run any of the executables, I get:
>>>
>>> main.go:73: Error during initialization: stat /usr/local/etc/nncp.hjson:
>>> no such file or directory
>>
>> I couldn't reproduce this error message. Which command are you running?
>> But, I do see that the output of say `nncp-xfer` lists
>> /usr/local/etc/nncp.hjson as the default configuration path. I will
>> investigate.

Apologies, I got the error with `nncp-stat` and `nncp-check`.

>
> That leaves only the CFGPATH issue. I'll figure it out and send a v3
> patch.
>
> Regards,
> Arun

--
Sarah

Hi Sarah,

>> I set up a tmpfs file system at /tmp. And, curiously, now the package
>> builds successfully. No idea why this is happening, though.
>
> I actually just figured this out as well. You're on a rotational disk,
> aren't you?

Yep.

> Since nncp makes heavy use of the spool file (which I think
> it mocks in /tmp for testing), I think that's why your tests are taking
> so long and timing out. Tests complete for me on a SSD in ~40s. (All the
> error messages are red herrings; they're expected errors.) If this is
> the case, it might be worth it to increase the test timeout for those
> who use --no-substitutes.

Indeed, that makes sense! I have now disabled the timeout. Without a
tmpfs, the tests take ~47 minutes on my rotational disk, but they do
complete successfully.

>>>>>> Does CFGPATH need to be set?
>>>>>
>>>>> I have now set CFGPATH TO /etc/nncp.hjson.
>>>>
>>>> When I run any of the executables, I get:
>>>>
>>>> main.go:73: Error during initialization: stat /usr/local/etc/nncp.hjson:
>>>> no such file or directory

I fixed this as well. `go list` wasn't finding the correct module path,
and thus the correct cfgpath variable was not being set.

I'm sending a v3 of the patch. Let me know if everything looks good, and
I'll push. Thanks for the comprehensive review! :-) The package looks
much better now.

Regards,
Arun