[bug#41501] [PATCH] Add mergerfs/mergerfs-tools

[Marius Bakke <mbakke@fastmail.com>]

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

Lars-Dominik Braun <lars@6xq.net> writes:

> Hi,
>
> the attached patch series adds mergerfs, an union file system, and
> associated tools mergerfs-tools. I’ve been running it for a while now
> and it seems to be stable. Unfortunately I’m not able to mount it in the
> system configuration via
>
> (file-system
>  (device "/storage/disk*")
>  (mount-point "/storage/pool")
>  (type "mergerfs")
>  (flags '('allow_other, 'use_ino, "moveonenospc=true", "category.create=mfs"))
>  (check? #f))
>
> because device is a pattern and not an actual file system path.

Oh, fun.  I suppose we'll have to add support for mergerfs in the system
configuration to deal with it.

> From ac0ff2afbbcc63d9b6b7b448877f54b58e975668 Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sun, 24 May 2020 10:48:02 +0200
> Subject: [PATCH 2/2] gnu: Add mergerfs-tools.
>
> * gnu/packages/storage.scm (mergerfs-tools): New variable.

I think mergerfs is better suited in file-systems.scm.  Can you rebase
these patches accordingly?

> diff --git a/gnu/packages/storage.scm b/gnu/packages/storage.scm
> index b8090c7eaa..ee5967aff6 100644
> --- a/gnu/packages/storage.scm
> +++ b/gnu/packages/storage.scm
> @@ -24,6 +24,8 @@
>    #:use-module (guix utils)
>    #:use-module (guix build-system cmake)
>    #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system copy)
> +  #:use-module (guix git-download)
>    #:use-module (gnu packages)
>    #:use-module (gnu packages admin)
>    #:use-module (gnu packages assembly)
> @@ -46,6 +48,7 @@
>    #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages python)
>    #:use-module (gnu packages python-xyz)
> +  #:use-module (gnu packages rsync)
>    #:use-module (gnu packages sphinx)
>    #:use-module (gnu packages tls)
>    #:use-module (gnu packages web)
> @@ -299,3 +302,53 @@ storage protocols (S3, NFS, and others) through the RADOS gateway.")
>                 license:isc
>                 ;; imported libfuse code
>                 license:gpl2 license:lgpl2.0))))
> +
> +(define-public mergerfs-tools
> +  (let ((commit "c926779d87458d103f3b674603bf97801ae2486d")
> +        (revision "1"))
> +    (package
> +      (name "mergerfs-tools")
> +      ;; unreleased, no version

Please use full sentences in code comments, i.e. capitalizations and
full stops.

> +      (version (git-version "0" revision commit))

The convention is to use "0.0" for situations like these, mainly because
it looks funnier.

> +      (source
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url "https://github.com/trapexit/mergerfs-tools.git")
> +               (commit commit)))
> +         (file-name (git-file-name name version))
> +         (sha256
> +          (base32
> +           "04hhwcib0xv4cf1mkj8zrp2aqpxkncml9iqg4m1mz6a5zhzsk0vm"))))
> +      (build-system copy-build-system)
> +      (inputs
> +       `(("python" ,python)
> +         ("python-xattr" ,python-xattr)
> +         ("rsync" ,rsync)))
> +      (arguments
> +       '(#:install-plan
> +         '(("src/" "bin/"))
> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'patch-paths
> +             (lambda* (#:key inputs #:allow-other-keys)
> +               (substitute* (find-files "src" "^mergerfs\\.")
> +                 (("'rsync'")
> +                  (string-append "'" (assoc-ref inputs "rsync") "/bin/rsync'"))
> +                 (("'rm'")
> +                  (string-append "'" (assoc-ref inputs "coreutils") "/bin/rm'")))
> +               (substitute* "src/mergerfs.mktrash"
> +                 (("xattr")
> +                  (string-append (assoc-ref inputs "python-xattr") "/bin/xattr"))
> +                 (("mkdir")
> +                  (string-append (assoc-ref inputs "coreutils") "/bin/mkdir")))
> +               #t)))))
> +      (synopsis "Optional tools to help manage data in a mergerfs pool")

I think we can drop 'optional' from here.

> +      (description
> +       "Audit permissions and ownership of files and directories, duplicates
> +        files & directories across branches in a pool, find and remove
> +        duplicate files, balance pool drives, consolidate files in a single
> +        mergerfs directory onto a single drive and create FreeDesktop.org Trash
> +        specification compatible directories.")

These lines should not be indented apart from the first one.

Also, the description reads somewhat unnatural to me.  Taken literally,
the description makes it sound like this package can do all that without
special support from anything?

It would be good to start along the lines of "mergerfs-tools is a suite
of programs that can ..." and throw in that it needs a mergerfs to
actually work.

> From 529a3aa70ab1a3079f2c5ab20fb776e92e2ba1cf Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sun, 24 May 2020 09:53:30 +0200
> Subject: [PATCH 1/2] gnu: Add mergerfs.
>
> * gnu/packages/storage.scm (mergerfs): New variable.

Can you move this too to file-systems.scm?

[...]

> +(define-public mergerfs
> +  (package
> +    (name "mergerfs")
> +    (version "2.29.0")
> +    ;; mergerfs bundles a heavily modified copy of libfuse

Full sentences please.  :-)

Maybe even an "XXX" in this case.

> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://github.com/trapexit/mergerfs/releases/download/"
> +                           version "/mergerfs-" version ".tar.gz"))
> +       (sha256
> +        (base32
> +         "17gizw4vgbqqjd2ykkfpp276942jb5qclp0lkiwkmq1yjgyjqfmk"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f                      ; no tests exist
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (add-after 'unpack 'fix-paths
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (setenv "CC" "gcc")
> +             ;; These were copied from the package libfuse
> +             (substitute* '("libfuse/lib/mount_util.c" "libfuse/util/mount_util.c")
> +               (("/bin/(u?)mount" _ maybe-u)
> +                (string-append (assoc-ref inputs "util-linux")
> +                               "/bin/" maybe-u "mount")))
> +             (substitute* '("libfuse/util/mount.mergerfs.c")
> +               (("/bin/sh")
> +                (which "sh")))
> +             ;; The Makefile does not allow overriding PREFIX via make variables
> +             (substitute* '("Makefile" "libfuse/Makefile")
> +               (("= /usr/local") (string-append "= " (assoc-ref outputs "out")))
> +               ;; cannot chown as build user
> +               (("chown root:root") "true"))
> +             #t)))))
> +    (inputs `(("util-linux" ,util-linux)))
> +    (home-page "https://github.com/trapexit/mergerfs")
> +    (synopsis
> +     "Featureful union filesystem")

This line break is unnecessary.

> +    (description
> +     "mergerfs is a union filesystem geared towards simplifying storage and
> +          management of files across numerous commodity storage devices.  It is
> +          similar to mhddfs, unionfs, and aufs.")

No indentation here.

> +    (license (list
> +               ;; mergerfs
> +               license:isc
> +               ;; imported libfuse code
> +               license:gpl2 license:lgpl2.0))))

These would do well as margin comments, i.e.:

  license:isc                    ;mergerfs
  license:gpl2 license:lgpl2.0   ;imported libfuse code

Can you send updated patches?  TIA!

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