[bug#42031] [PATCH] gnu: Add autofs.

[Tobias Geerinckx-Rice via Guix-patches via <guix-patches@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 7565 bytes --]

Oleg,

Thank you for the patch!

I'm superbly biased[1] but 3 cruel people have insisted we review 
each others' patches so here you go :-)

Oleg told me on #guix that this is basically a straight port from 
Nix[0].  The Nix package is very… not good?  There's nothing worth 
scavenging.  A ‘reviewed’ version of Oleg's patch ends up 
identical to mine, but without the SASL support.

Oleg Pykhalov 写道：
> * gnu/packages/file-systems.scm (autofs): New variable.
> ---
>  gnu/packages/file-systems.scm | 77 
>  +++++++++++++++++++++++++++++++++++
>  1 file changed, 77 insertions(+)
>
> diff --git a/gnu/packages/file-systems.scm 
> b/gnu/packages/file-systems.scm
> index dd72152c51..c8d881e542 100644
> --- a/gnu/packages/file-systems.scm
> +++ b/gnu/packages/file-systems.scm
> @@ -36,6 +36,7 @@
>    #:use-module (gnu packages acl)
>    #:use-module (gnu packages attr)
>    #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages base)
>    #:use-module (gnu packages bison)
>    #:use-module (gnu packages check)
>    #:use-module (gnu packages compression)
> @@ -57,6 +58,7 @@
>    #:use-module (gnu packages python-xyz)
>    #:use-module (gnu packages readline)
>    #:use-module (gnu packages rsync)
> +  #:use-module (gnu packages sssd)
>    #:use-module (gnu packages sqlite)
>    #:use-module (gnu packages tls)
>    #:use-module (gnu packages valgrind)
> @@ -782,3 +784,78 @@ directory onto a single drive and create 
> FreeDesktop.org Trash specification
>  compatible directories.")
>        (home-page "https://github.com/trapexit/mergerfs-tools")
>        (license license:isc))))
> +
> +(define-public autofs
> +  (package
> +    (name "autofs")
> +    (version "5.1.6")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> + 
> "https://mirrors.edge.kernel.org/pub/linux/daemons/autofs/v"
> +                    (version-major version) "/autofs-" version 
> ".tar.xz"))
> +              (sha256
> +               (base32
> + 
> "1vya21mb4izj3khcr3flibv7xc15vvx2v0rjfk5yd31qnzcy7pnx"))))
> +    (build-system gnu-build-system)
> +    (inputs `(("util-linux" ,util-linux)
> +              ("nfs-utils" ,nfs-utils)
> +              ("kmod" ,kmod)
> +              ("e2fsprogs" ,e2fsprogs)
> +              ("sssd" ,sssd)
> +              ("linux-headers" ,linux-libre-headers)
> +              ("libtirpc", libtirpc)
> +              ("binutils" ,binutils)
> +              ("rpcsvc-proto" ,rpcsvc-proto) ;for 'rpcgen'
> +              ("libxml2" ,libxml2)))

Order *inputs alphabetically.

rpcsvc-proto should be native.  So would binutils, but including 
it at all is probably a mistake.

linux-libre-headers aren't necessary.  Nor is kmod.

Sometimes, configure scripts check for things just for fun.

> +    (native-inputs
> +     `(("flex" ,flex)
> +       ("bison" ,bison)
> +       ("pkg-config" ,pkg-config)))
> +    (arguments
> +     `(#:tests? #f ; no tests

Despite the ‘warning: foo not found, disabling the foo tests’ 
./configure warnings I didn't find them either.

> +       #:validate-runpath? #f

This is a sign that something is broken & needs to be fixed, not 
ignored.

> +       #:configure-flags '("--enable-force-shutdown"

INSTALL describes this as:

   --enable-force-shutdown
   This option enables the use of the USR1 signal to force an
   unconditional unlink umount of all mounts at shutdown.

Safety aside, I read that as ‘requires init/service to actually 
send the signal on shutdown to have any effect’.  There's no 
autofs service that does so yet.

I don't have much of an opinion about it.  Does anyone think it's 
useful or unsafe?

> "--with-path=$PATH"

In Nix, everything between ''fun quotes'' is evaluated by a shell. 
In Guix, unless ./configure does its own eval somewhere (I didn't 
check), this just sets the path to (literally) "$PATH".

> +                           "--enable-ignore-busy")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'pre-configure
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (setenv "exportsssldir" (string-append (assoc-ref 
> inputs "sssd")
> + 
> "/lib/sssd/modules"))
> +             (setenv "HAVE_SSS_AUTOFS" "1")

Pass these as #:configure-flags when possible — it's declarative®.

> +             (setenv "YACC" (string-append (assoc-ref inputs 
> "bison")
> +                                           "/bin/yacc"))
> +             (setenv "RANLIB" (string-append (assoc-ref inputs 
> "binutils")
> +                                             "/bin/ranlib"))
> +             (setenv "RPCGEN" (string-append (assoc-ref inputs 
> "rpcsvc-proto")
> +                                             "/bin/rpcgen"))
> +             (setenv "LEX" (string-append (assoc-ref inputs 
> "flex")
> +                                          "/bin/flex"))
> +             (setenv "MOUNT" (string-append (assoc-ref inputs 
> "util-linux")
> +                                            "/bin/mount"))
> +             (setenv "MOUNT_NFS" (string-append (assoc-ref 
> inputs "nfs-utils")
> + 
> "/sbin/mount.nfs"))
> +             (setenv "UMOUNT" (string-append (assoc-ref inputs 
> "util-linux")
> +                                             "/bin/umount"))
> +             (setenv "MODPROBE" (string-append (assoc-ref 
> inputs "kmod")
> + 
> "/bin/modprobe"))
> +             (setenv "E2FSCK" (string-append (assoc-ref inputs 
> "e2fsprogs")
> + 
> "/sbin/fsck.ext2"))
> +             (setenv "E3FSCK" (string-append (assoc-ref inputs 
> "e2fsprogs")
> + 
> "/sbin/fsck.ext3"))
> +             (setenv "E4FSCK" (string-append (assoc-ref inputs 
> "e2fsprogs")
> + 
> "/sbin/fsck.ext4"))

All of this can be avoided with:

+         (add-before 'configure 'fix-hard-coded-search-path
+           (lambda _
+             (substitute* "configure"
+               (("^searchpath=\".*\"")
+                "searchpath=\"$PATH\""))
+             #t))

allowing the configure script to do the work of finding things.

> +             ;; Allow <rpc/rpc.h> & co. to be found.
> +             (setenv "CPATH" (string-append (assoc-ref inputs 
> "libtirpc")
> +                                            "/include/tirpc"))
> +             ;; Makefile.rules defines a usable STRIP only 
> without the env var.
> +             (unsetenv "STRIP")
> +             #t)))))

Here too: I don't know if you simply copied these from Nix[0] 
because they were there, or if you encountered errors without 
them, but neither should be needed.  I suspect STRIP is only 
needed because of the bogus binutils input above, if at all.

> +    (home-page 
> "https://www.kernel.org/pub/linux/daemons/autofs/")
> +    (synopsis "Kernel-based automounter")

Which kernel?  ;-)

> +    (description "Autofs controls the operation of the 
> automount daemons.  The
> +automount daemons automatically mount filesystems when they are 
> used and
> +unmount them after a period of inactivity.  This is done based 
> on a set of
> +pre-configured maps.")

The first sentence describes the autofs *command*.

> +    (license license:gpl2)))

Kind regards,

T G-R

[0]: 
https://github.com/NixOS/nixpkgs/blob/84cf00f98031e93f389f1eb93c4a7374a33cc0a9/pkgs/os-specific/linux/autofs/default.nix
[1]: https://issues.guix.gnu.org/42033

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]