On Wed, Jun 03, 2020 at 08:55:39AM +0200, Mathieu Othacehe wrote:
> 
> Hey Jakub,
> 
> Thanks for fixing the cross-compilation, a full review this time :)
> 
> > +             (let* ((outdir (assoc-ref outputs "out"))
> > +                    (bindir (string-append outdir "/bin"))
> > +                    (mandir (string-append outdir "/share/man/man1"))
> > +                    (docdir (string-append outdir "/share/doc/hashcash-" ,version)))
> > +               ;; make install assumes /usr and doesn't provide a way to override it
> > +               (install-file "hashcash" bindir)
> > +               (install-file "hashcash.1" mandir)
> > +               (install-file "README" docdir)
> > +               (install-file "LICENSE" docdir)
> > +               (install-file "CHANGELOG" docdir)
> 
> I think you can set the variables INSTALL_PATH, MAN_INSTALL_PATH and
> DOC_INSTALL_PATH instead.
> 
> > +               #t))))))
> > +    (home-page "https://www.hashcash.org/")
> > +    (synopsis "Denial-of-service countermeasure")
> > +    (description "Hashcash is a proof-of-work algorithm, which has been used
> > +as a denial-of-service countermeasure technique in a number of systems.
> 
> You can remove "in a number of systems".

Note that systems means things like e-mail or bitcoin, rather than Guix
or Debian.

> > +A hashcash stamp constitutes a proof-of-work which takes a parametrizable
> > +amount of work to compute for the sender.  The recipient can verify received
> > +hashcash stamps efficiently.
> > +
> > +This package contains a command-line tool for computing and verifying hashcash
> > +stamps.")
> > +    (license license:public-domain)))
> 
> I'm also concerned by this line in the Makefile:
> 
> --8<---------------cut here---------------start------------->8---
> # request static link of -lcrypto only
> LIBCRYPTO=/usr/lib/libcrypto.a
> --8<---------------cut here---------------end--------------->8---
> 
> We should maybe add "openssl" to the inputs and fix this variable
> (that's what Nix does).

Firstly, the benchmarking code seems to be broken in Hashcash 1.22.
Downgrading to 1.21 and running hashcash -sv, I found that OpenSSL
is slower than another minting backend, and is not used by default.
I believe it would be better to not include OpenSSL in the package.
That's what Debian does, anyway.

I therefore propose only the following changes:

diff --git a/gnu/packages/networking.scm b/gnu/packages/networking.scm
index b38e75eb7f..ecd72eee26 100644
--- a/gnu/packages/networking.scm
+++ b/gnu/packages/networking.scm
@@ -3217,7 +3217,7 @@ an implementation of LLDP.  It also supports some proprietary protocols.")
                     (bindir (string-append outdir "/bin"))
                     (mandir (string-append outdir "/share/man/man1"))
                     (docdir (string-append outdir "/share/doc/hashcash-" ,version)))
-               ;; make install assumes /usr and doesn't provide a way to override it
+               ;; Install manually, as we don't need the `sha1' binary
                (install-file "hashcash" bindir)
                (install-file "hashcash.1" mandir)
                (install-file "README" docdir)

Regards,
Jakub Kądziołka