GNU bug report logs - #41501
[PATCH] Add mergerfs/mergerfs-tools

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41501 in the body.
You can then email your comments to 41501 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] Add mergerfs/mergerfs-tools
Date: Sun, 24 May 2020 10:54:48 +0200

[Message part 1 (text/plain, inline)]

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.

Cheers,
Lars

[0002-gnu-Add-mergerfs-tools.patch (text/x-diff, attachment)]

[0001-gnu-Add-mergerfs.patch (text/x-diff, attachment)]

Message #8 received at 41501 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>, 41501 <at> debbugs.gnu.org
Subject: Re: [bug#41501] [PATCH] Add mergerfs/mergerfs-tools
Date: Sat, 30 May 2020 14:50:32 +0200

[Message part 1 (text/plain, inline)]

Lars-Dominik Braun <lars <at> 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 <at> 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 <at> 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!

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 41501 <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 41501 <at> debbugs.gnu.org
Subject: Re: [bug#41501] [PATCH] Add mergerfs/mergerfs-tools
Date: Sun, 31 May 2020 08:25:27 +0200

[Message part 1 (text/plain, inline)]

Hi,

> > (file-system
> >  (device "/storage/disk*")
> >  (mount-point "/storage/pool")
> >  (type "mergerfs")
> >  (flags '('allow_other, 'use_ino, "moveonenospc=true", "category.create=mfs"))
> >  (check? #f))
> Oh, fun.  I suppose we'll have to add support for mergerfs in the system
> configuration to deal with it.
maybe a generic

	(device (literal "/storage/disk*"))

would do? I’m sure there are more FUSE filesystems out there which
trigger these checks. And I don’t think adding an exception like the one
for NFS is a good solution long-term.

> Can you send updated patches?  TIA!
All done, see attachment. I hope I did not miss anything.

Cheers,
Lars

[0002-gnu-Add-mergerfs-tools.patch (text/x-diff, attachment)]

[0001-gnu-Add-mergerfs.patch (text/x-diff, attachment)]

Message #16 received at 41501-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Marius Bakke <mbakke <at> fastmail.com>, 41501-done <at> debbugs.gnu.org
Subject: Re: [bug#41501] [PATCH] Add mergerfs/mergerfs-tools
Date: Wed, 03 Jun 2020 18:04:22 +0200

Hi,

Lars-Dominik Braun <lars <at> 6xq.net> skribis:

>>From 7aa69a86f8933c3d833ae3beb53840ded9115978 Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars <at> 6xq.net>
> Date: Sat, 30 May 2020 19:10:55 +0200
> Subject: [PATCH 2/2] gnu: Add mergerfs-tools.
>
> * gnu/packages/storage.scm (mergerfs-tools): New variable.

[...]

>>From 4b2500e04f956df0c038ba4b71d91f01b2919d1e Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars <at> 6xq.net>
> Date: Sat, 30 May 2020 19:10:30 +0200
> Subject: [PATCH 1/2] gnu: Add mergerfs.
>
> * gnu/packages/storage.scm (mergerfs): New variable.

I think it addresses the issues Marius wrote about, so I went ahead and
applied them.

> +             ;; These were copied from the package libfuse.

[...]

> +    ;; mergerfs bundles a heavily modified copy of libfuse.

This is not great.  Did you try building against vanilla libfuse?

Anyway, thank you!

Ludo’.

Message #19 received at 41501-done <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Marius Bakke <mbakke <at> fastmail.com>, 41501-done <at> debbugs.gnu.org
Subject: Re: [bug#41501] [PATCH] Add mergerfs/mergerfs-tools
Date: Wed, 3 Jun 2020 19:13:42 +0200

Hi Ludo,

> > +    ;; mergerfs bundles a heavily modified copy of libfuse.
> This is not great.  Did you try building against vanilla libfuse?
no, I looked at the git changelog of their libfuse tree and it appeared
to contain substantial changes, otherwise I would’ve unvendored it.
Judging by the documentation[1] the upstream libfuse is not supported.

Lars

[1] https://github.com/trapexit/mergerfs#why-was-libfuse-embedded-into-mergerfs

Message #22 received at 41501-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Marius Bakke <mbakke <at> fastmail.com>, 41501-done <at> debbugs.gnu.org
Subject: Re: [bug#41501] [PATCH] Add mergerfs/mergerfs-tools
Date: Thu, 04 Jun 2020 11:57:32 +0200

Hi,

Lars-Dominik Braun <lars <at> 6xq.net> skribis:

>> > +    ;; mergerfs bundles a heavily modified copy of libfuse.
>> This is not great.  Did you try building against vanilla libfuse?
> no, I looked at the git changelog of their libfuse tree and it appeared
> to contain substantial changes, otherwise I would’ve unvendored it.
> Judging by the documentation[1] the upstream libfuse is not supported.

I see.

> [1] https://github.com/trapexit/mergerfs#why-was-libfuse-embedded-into-mergerfs

I don’t buy their arguments :-), but from the Guix viewpoint, we’ve done
our best.

Thanks for explaining!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.