Oleg Pykhalov <go.wigust@gmail.com> writes:

> * gnu/packages/networking.scm (net-snmp): New variable.

Hello!  This patch reminded me I had an ancient patch for Net-SNMP too,
but got stuck on a single test failure and forgot all about it.

I see you've disabled tests altogether which is a neat workaround.
However I'm hoping we can consolidate our efforts and just disable the
one (or was it two) tests that are failing.

Comments inline, my patch attached at the end.

> ---
>  gnu/packages/networking.scm | 76 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 76 insertions(+)

I chose to add a (gnu packages snmp) module, but that may be overkill
indeed.

[...]

> +(define-public net-snmp
> +  (package
> +    (name "net-snmp")
> +    (version "5.7.3")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "mirror://sourceforge/net-snmp/net-snmp/" version
> +                           "/net-snmp-" version ".tar.gz"))
> +       (sha256
> +        (base32
> +         "1w5l9w0sgi1zkzq8ww6kc6fzq7ljq59z2d9ks6bdq1vp7ihqkvqj"))
> +       (patches
> +        (map (lambda (file hash)
> +               (origin
> +                 (method url-fetch)
> +                 (uri (string-append
> +                       "https://git.alpinelinux.org\
> +/cgit/aports/plain/main/net-snmp/"
> +                       file "?id=f25d3fb08341b60b6ccef424399f060dfcf3f1a5"))
> +                 (sha256
> +                  (base32
> +                   hash))))
> +             '("CVE-2015-5621.patch"
> +               "fix-Makefile-PL.patch"
> +               "fix-includes.patch"
> +               "netsnmp-swinst-crash.patch"
> +               "remove-U64-typedef.patch")
> +             '("0mg2mlfb45fnv7m1k9wckrqjfizipyvrl1q4dn1r0zc774mm7zjc"
> +               "1pd85sy04n76q1ri3l33f0zpnnw76nd5mcny2j39ilzp76bjfik5"
> +               "0zpkbb6k366qpq4dax5wknwprhwnhighcp402mlm7950d39zfa3m"
> +               "0gh164wy6zfiwiszh58fsvr25k0ns14r3099664qykgpmickkqid"
> +               "0jcpcpgx4z9k1w0x6km0132n67qc29mz6cialwfjm02l76q2yk5n")))))

Instead of adding all these patches, I opted to use the 5.8
pre-release instead.  No strong opinion, but since it's a new package
and 5.8 is "just around the corner" I think that's fine.  WDYT?

Also note that this packages bundles a copy of OpenSSL, which should be
purged.

> +    (build-system gnu-build-system)
> +    (native-inputs
> +     `(("autoconf" ,autoconf)
> +       ("automake" ,automake)
> +       ("libtool" ,libtool)))

Why are these needed?  Because of the patches?

> +    (inputs
> +     `(("file" ,file)
> +       ("perl" ,perl)
> +       ("openssl" ,openssl)))

"file" is an implicit input.  Can you add a comment about why it's
needed here (I guess it's referenced somewhere?)?

> +    (arguments
> +     `(#:tests? #f
> +       #:configure-flags
> +       (list "--with-default-snmp-version=3"
> +             "--with-sys-location=Unknown"
> +             "--with-sys-contact=root@unknown"
> +             "--with-logfile=/var/log/net-snmpd.log"
> +             "--with-persistent-directory=/var/lib/net-snmp"
> +             (string-append "--with-openssl="
> +                            (assoc-ref %build-inputs "openssl"))
> +             "--with-mnttab=/proc/mounts")
> +       #:make-flags
> +       (let ((out (assoc-ref %outputs "out")))
> +         (list (string-append "INSTALLSITEARCH=" out
> +                              "/lib/perl5/site_perl/" ,(package-version perl)
> +                              "/x86_64-linux-thread-multi")
> +               (string-append"INSTALLSITEMAN3DIR=" out "/share/man/man3")))
> +
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'autoreconf
> +           (lambda _
> +             (invoke "autoreconf" "-vfi"))))))
> +    (home-page "http://net-snmp.sourceforge.net/")
> +    (synopsis "Clients and server for the SNMP network monitoring protocol")
> +    (description "The Simple Network Management Protocol (SNMP) provides a
> +framework for the exchange of management information between agents (servers)
> +and clients.
> +
> +The Net-SNMP applications are a collection of command line clients for issuing
> +SNMP requests to agents.")
> +    (license license:bsd-3)))

The main license is actually CMU/UCDs "Historic Permission Notice and
Disclaimer", which is not in Guix.  Do you think it's worth adding, or
should we simply use a non-copyleft style URI here?

My patch attached: