[bug#27419] [PATCH] gnu: Add unbound.

[Ludovic Courtès]

Hi Gregor,

Gregor Giesen <address@hidden> skribis:

> From fac67e33fe0501ddcad3a1e75f20b4954f36834b Mon Sep 17 00:00:00 2001
> From: Gregor Giesen <address@hidden>
> Date: Sun, 18 Jun 2017 14:27:34 +0200
> Subject: [PATCH 1/1] gnu: Add unbound.
>
> * gnu/packages/dns.scm (unbound): New variable.

Nice!  Overall LGTM, so I’m commenting on minor issues:

> +    (outputs '("out" "python"))
> +    (inputs
> +     `(("expat" ,expat)
> +       ("flex" ,flex)
> +       ("libevent" ,libevent)
> +       ("protobuf" ,protobuf)
> +       ("python" ,python-3)
> +       ("python-wrapper" ,python-wrapper)
> +       ("openssl" ,openssl)
> +       ("swig" ,swig)))

I think SWIG should go to ‘native-inputs’ because it’s only used at
build time.

> +       (modify-phases
> +           %standard-phases

I would make this a single line.  :-)

> +         (add-after 'configure 'fix-python-site-package-path
> +           ;; Move python modules into their own output.
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((pyout (assoc-ref outputs "python"))
> +                   (ver ,(version-major+minor (package-version python))))
> +               (substitute* "Makefile"
> +                 (("^PYTHON_SITE_PKG=.*$")
> +                  (string-append
> +                   "PYTHON_SITE_PKG="
> +                   pyout "/lib/python-" ver "/site-packages\n"))))))

Please make sure the phase explicitly returns #t to indicate success.

> +         (add-before 'check 'fix-missing-nss-for-tests
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (use-modules (guix build utils))

The ‘use-modules’ line is not needed.

> +             (let* ((source (assoc-ref %build-inputs "source"))
> +                    (gcc (assoc-ref %build-inputs "gcc")))
> +               (call-with-output-file "/tmp/nss_preload.c"
> +                 (lambda (port)
> +                   (display "#include <stdlib.h>

As discussed on help-guix, I would prefer using ‘substitute*’ to modify
all the unit tests.  That would reduce complexity and be potentially
more robust.  Does that sound feasible without much hassle?

> +               (substitute* "Makefile"
> +                 (("./unittest")
> +                  "LD_PRELOAD=/tmp/nss_preload.so ./unittest"))))))))

Also return #t.

> +    (home-page "https://www.unbound.net";)
> +    (synopsis "Validating, recursive, and caching DNS resolver")
> +    (description
> +     "Unbound is a recursive-only caching DNS server which can perform 
> DNSSEC validation of results.  It implements only a minimal amount of 
> authoritative service to prevent leakage to the root nameservers: forward 
> lookups for localhost, reverse for 127.0.0.1 and ::1, and NXDOMAIN for zones 
> served by AS112.  Stub and forward zones are supported.")

Please wrap lines to 80 chars as ‘guix lint’ should suggest.  Also you
can write @code{127.0.0.1} and @code{::1}.

That’s it, thank you for this first package!

Ludo’.