diff mbox series

[bug#45692,v3,2/3] gnu: Add zfs-auto-snapshot.

Message ID 2c97vRukDGd4mnnPwPzLZOBq6qR5EqiBvaevyGcaoh1zc8NQkJO8y2DvtaMuafb6R9m5_Hhizx41nJjyggYPiHNXmnYKkQCejKsw_n1-wec=@protonmail.com
State Accepted
Headers show
Series None | expand

Commit Message

raid5atemyhomework March 22, 2021, 2:34 p.m. UTC
From 481dc87f77743b7a282777656e78615ea57d19cc Mon Sep 17 00:00:00 2001
From: raid5atemyhomework <raid5atemyhomework@protonmail.com>
Date: Sun, 14 Mar 2021 16:40:47 +0800
Subject: [PATCH 2/3] gnu: Add zfs-auto-snapshot.

* gnu/packages/file-systems.scm (zfs-auto-snapshot): New variable.
---
 gnu/packages/file-systems.scm | 75 +++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

--
2.31.0

Comments

Danny Milosavljevic May 11, 2021, 2:05 p.m. UTC | #1
Hi,

> +                (invoke "make" "install"
> +                        "PREFIX="
> +                        (string-append "DESTDIR=" out)))

Are you sure about that?

Usually, DESTDIR is in order to supply a temporary build root (which will not
be referred-to by the installed programs--because that directory will be
deleted after the build) and PREFIX is in order to supply an actual
installation directory (installed programs can refer to that whenever they
want).

Is it different for this program?

Also, I'd suggest to use #:make-flags so other phases can also see the PREFIX
chosen.  Otherwise, those could erroneously default to another prefix (for
loading of config files at runtime etc).

What do you think?
raid5atemyhomework May 13, 2021, 1:21 a.m. UTC | #2
Hello Danny,

> Hi,
>
> > -                  (invoke "make" "install"
> >
> >
> > -                          "PREFIX="
> >
> >
> > -                          (string-append "DESTDIR=" out)))
> >
> >
>
> Are you sure about that?
>
> Usually, DESTDIR is in order to supply a temporary build root (which will not
> be referred-to by the installed programs--because that directory will be
> deleted after the build) and PREFIX is in order to supply an actual
> installation directory (installed programs can refer to that whenever they
> want).
>
> Is it different for this program?
>
> Also, I'd suggest to use #:make-flags so other phases can also see the PREFIX
> chosen. Otherwise, those could erroneously default to another prefix (for
> loading of config files at runtime etc).
>
> What do you think?

Yes, I am quite sure; the program uses a custom `Makefile` that is fairly simple and can be trivially quoted here for your review:

```Makefile
PREFIX := /usr/local

all:

install:
	install -d $(DESTDIR)/etc/cron.d
	install -d $(DESTDIR)/etc/cron.daily
	install -d $(DESTDIR)/etc/cron.hourly
	install -d $(DESTDIR)/etc/cron.weekly
	install -d $(DESTDIR)/etc/cron.monthly
	install -m 0644 etc/zfs-auto-snapshot.cron.frequent $(DESTDIR)/etc/cron.d/zfs-auto-snapshot
	install etc/zfs-auto-snapshot.cron.hourly   $(DESTDIR)/etc/cron.hourly/zfs-auto-snapshot
	install etc/zfs-auto-snapshot.cron.daily    $(DESTDIR)/etc/cron.daily/zfs-auto-snapshot
	install etc/zfs-auto-snapshot.cron.weekly   $(DESTDIR)/etc/cron.weekly/zfs-auto-snapshot
	install etc/zfs-auto-snapshot.cron.monthly  $(DESTDIR)/etc/cron.monthly/zfs-auto-snapshot
	install -d $(DESTDIR)$(PREFIX)/share/man/man8
	install -m 0644 src/zfs-auto-snapshot.8 $(DESTDIR)$(PREFIX)/share/man/man8/zfs-auto-snapshot.8
	install -d $(DESTDIR)$(PREFIX)/sbin
	install src/zfs-auto-snapshot.sh $(DESTDIR)$(PREFIX)/sbin/zfs-auto-snapshot

uninstall:
	rm $(DESTDIR)/etc/cron.d/zfs-auto-snapshot
	rm $(DESTDIR)/etc/cron.hourly/zfs-auto-snapshot
	rm $(DESTDIR)/etc/cron.daily/zfs-auto-snapshot
	rm $(DESTDIR)/etc/cron.weekly/zfs-auto-snapshot
	rm $(DESTDIR)/etc/cron.monthly/zfs-auto-snapshot
	rm $(DESTDIR)$(PREFIX)/share/man/man8/zfs-auto-snapshot.8
	rm $(DESTDIR)$(PREFIX)/sbin/zfs-auto-snapshot
```

Notice how it does not modify any of the source files at all, so the sources do not refer to anything provided in `$PREFIX`.  However, it does create directories in `$DESTDIR`, so it has to use `$DESTDIR`.

Thanks
raid5atemyhomework
Danny Milosavljevic May 13, 2021, 1:08 p.m. UTC | #3
Hi,

thanks for the explanation!

Pushed zfs-auto-snapshot to guix master as commit ba3b295a3ee956ac7500b5f9bb1d151b28ab30ed.
diff mbox series

Patch

diff --git a/gnu/packages/file-systems.scm b/gnu/packages/file-systems.scm
index 198653c639..6877f89f6e 100644
--- a/gnu/packages/file-systems.scm
+++ b/gnu/packages/file-systems.scm
@@ -976,6 +976,81 @@  originally developed for Solaris and is now maintained by the OpenZFS
 community.")
     (license license:cddl1.0)))

+(define-public zfs-auto-snapshot
+  (package
+    (name "zfs-auto-snapshot")
+    (version "1.2.4")
+    (source
+      (origin
+        (method url-fetch)
+        (uri (string-append
+               "https://github.com/zfsonlinux/zfs-auto-snapshot/archive/upstream/"
+               version ".tar.gz"))
+        (sha256
+          (base32 "16ry1w43i44xc67gr73x6fa48ninfhqxr498ad4m3kya93vp2zrh"))))
+    (build-system gnu-build-system)
+    (inputs
+      ;; Note: if you are inheriting from the above zfs package in order
+      ;; to provide a specific stable kernel version, you should also
+      ;; inherit this package and replace the sole input below.
+      `(("zfs" ,zfs)))
+    (arguments
+      `(#:tests? #f ; No tests
+        #:phases
+        (modify-phases %standard-phases
+          (delete 'configure)
+          (delete 'build)
+          ;; Guix System may not have a traditional cron system, but
+          ;; the cron scripts installed by this package are convenient
+          ;; to use as targets for an mcron job specification, so make
+          ;; sure they can be run in-store.
+          (add-before 'install 'fix-scripts
+            (lambda* (#:key outputs inputs #:allow-other-keys)
+              (let* ((out                (assoc-ref outputs "out"))
+                     (zfs-auto-snapshot  (string-append
+                                           out
+                                           "/sbin/zfs-auto-snapshot"))
+                     (zfs-package        (assoc-ref inputs "zfs"))
+                     (zpool              (string-append
+                                           zfs-package
+                                           "/sbin/zpool"))
+                     (zfs                (string-append
+                                           zfs-package
+                                           "/sbin/zfs")))
+                (substitute* '("etc/zfs-auto-snapshot.cron.daily"
+                               "etc/zfs-auto-snapshot.cron.frequent"
+                               "etc/zfs-auto-snapshot.cron.hourly"
+                               "etc/zfs-auto-snapshot.cron.monthly"
+                               "etc/zfs-auto-snapshot.cron.weekly")
+                  (("zfs-auto-snapshot")
+                   zfs-auto-snapshot))
+                (substitute* "src/zfs-auto-snapshot.sh"
+                  (("LC_ALL=C zfs list")
+                   (string-append "LC_ALL=C " zfs " list"))
+                  (("LC_ALL=C zpool status")
+                   (string-append "LC_ALL=C " zpool " status"))
+                  (("zfs snapshot")
+                   (string-append zfs " snapshot"))
+                  (("zfs destroy")
+                   (string-append zfs " destroy"))))))
+          ;; Provide DESTDIR and PREFIX on make command.
+          (replace 'install
+            (lambda* (#:key outputs #:allow-other-keys)
+              (let ((out (assoc-ref outputs "out")))
+                (invoke "make" "install"
+                        "PREFIX="
+                        (string-append "DESTDIR=" out)))
+              #t)))))
+    (home-page "https://github.com/zfsonlinux/zfs-auto-snapshot")
+    (synopsis "Automatically create, rotate, and destroy periodic ZFS snapshots")
+    (description
+      "An alternative implementation of the zfs-auto-snapshot service for Linux
+that is compatible with zfs-linux (now OpenZFS) and zfs-fuse.
+
+On Guix System, you will need to invoke the included shell scripts as @code{job}
+definitions in your @code{operating-system} declaration.")
+    (license license:gpl2+)))
+
 (define-public mergerfs
   (package
     (name "mergerfs")