Hi, thanks for the reply.

Marius Bakke <marius@gnu.org> writes:

>
> Avoid hard-coding versions in URLs.  You can use the version-major+minor
> procedure to create that "2.7" URL component.

Done!

> This got merged in 2017, and still not released!
>
> Looking at the pull request[0], this URL is a squashed version of 7
> commits.  While convenient, this trick has failed before due to subtle
> changes in GitHub machinery, eventually leading to a hash mismatch[1].
>
> Taking 7 patches directly is no fun either, so I wonder if we can just
> stick with OpenSSL 1.0 for now?
>
Fine! Using openssl-1.0 now.

>> +                           ;; powerman is required (where is it hosted?)
>> +                           "--without-powerman"
>
> Can you elaborate on powerman being required?  Is the package not usable
> without it?
>
Expanded a bit, following nut readme.

>> +                           ;; cgi requires libgd
>> +                           "--without-cgi")
>
> We do have libgd (in the 'gd' package).
>
Oh, I don't know how I missed it.  Added!  Now cgi is working (output is
in folder /cgi-bin, is this correct for guix?)

Since nut release is really old, it used to use gdlib-config.  It has 
been removed and replaced by pkg-config.  However, the patch [1] does
not apply.  So I had to do the "terrible" fix of calling pkg-config
myself (in the new "fix-ligdb-check" phase).  Terrible because I wanted
to call it in guile and save the output to the file, but the only way I
know of is with popen (and I could not find another package doing this
(while some package like python-libmpsse are just inserting the call
like I did here).

>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-before 'configure 'update-configure
>> +           ;; required to apply the patch to the ./configure script
>> +           (lambda _
>> +             (invoke "autoconf"))))))
>
> This may not be necessary due to the 'bootstrap' phase (but I haven't
> verified).
>
Well, now that I switched to openssl-1.0 it sure is not needed anymore.
But previously I remember without the autoreconf call, it was not working.

>> +    (native-inputs `(("autoconf" ,autoconf)n
>> +                     ("cppunit" ,cppunit)
>> +                     ("pkg-config" ,pkg-config)))
>> +    (inputs `(("avahi" ,avahi)
>> +              ("libusb" ,libusb-compat)
>> +              ("libltdl" ,libltdl)
>> +              ("freeipmi" ,freeipmi)
>> +              ("neon" ,neon)
>> +              ("openssl" ,openssl)
>> +              ("net-snmp" ,net-snmp)))
>
> The indentation of these are a bit weird.  :-)
>
ops :)

>> +    (home-page "https://networkupstools.org")
>> +    (license
>> +     (list
>> +      ;; - most files under gpl2+
>> +      license:gpl2+
>> +      ;; - scripts/python/ under gpl3+
>> +      license:gpl3+
>> +      ;; - scripts/perl/Nut.pm same as perl (either gpl1+ or artistic)
>> +      license:gpl1+ license:artistic2.0))
>
> Great job at listing licenses!
>
>> +    (synopsis "Collection of programs for monitoring and administering UPS")
>> +    (description "Network UPS Tools is a collection of programs which provide
>> +a common interface for monitoring and administering UPS, PDU and SCD hardware.
>> +It uses a layered approach to connect all of the parts.  Drivers are provided
>> +for a wide assortment of equipment.")))
>
> Maybe add @acronym{UPS, Uninterruptible Power Supply},
> @acronym{PDU, ...} etc, but no strong opinion.
>
Sure!

> Can you send an updated patch?

I also added the phase "fix-search-paths" which fixes nut-scanner.
nut scanner now finds my ups device (but I still could not get it to
work, not sure if the problem is in my config or in this package.  I'd
wait to merge this until someone can try.

Thanks again,
Nicolò