[bug#49601] [PATCH] services: transmission: Create downloads directory.

[Simon South <simon@simonsouth.net>]

Simon South <simon@simonsouth.net> writes:
> Let me see if I can duplicate that...

I haven't been able to duplicate whatever issue it was I saw originally.
Likely it had to do with the download directory being placed on a
separate volume and thus matching a mount point already declared in the
operating-system declaration.  Probably the system test should be
expanded to check this kind of configuration works but a quick test on
my end shows that at least for existing setups, it is not a problem.

Morgan, just a few pieces of feedback then:

> This fixes a bug where transmission doesn't have permission to create
> it's download directory. This bug occurs when download-dir is

"it's" should be "its" (no apostrophe) here, the possessive form.

> +                              (,download-dir #o755))

Again, the permissions here should be "#o750".

Finally, I recommend updating the system test (in
gnu/tests/file-sharing.scm) to verify the download directory is created
correctly, now that this functionality has been added.  I've included a
patch below that does this for you.  To verify this works, apply the
patch and run the test suite with

    make check-system TESTS="transmission-daemon"

The Guix manual has more information[0].

Otherwise, looks good to me.  Thanks for submitting this patch.

[0] https://guix.gnu.org/en/manual/en/html_node/Running-the-Test-Suite.html

-- 
Simon South
simon@simonsouth.net


diff --git a/gnu/tests/file-sharing.scm b/gnu/tests/file-sharing.scm
index 9a8ee6a593..d27a206c4f 100644
--- a/gnu/tests/file-sharing.scm
+++ b/gnu/tests/file-sharing.scm
@@ -34,6 +34,8 @@
 (define %transmission-daemon-group "transmission")
 
 (define %transmission-daemon-config-dir "/var/lib/transmission-daemon")
+(define %transmission-daemon-download-dir
+  (string-append %transmission-daemon-config-dir "/downloads"))
 (define %transmission-daemon-watch-dir
   (string-append %transmission-daemon-config-dir "/watch"))
 (define %transmission-daemon-incomplete-dir
@@ -110,8 +112,9 @@
                 #t)
              marionette))
 
-          ;; Make sure Transmission Daemon's configuration directory has been
-          ;; created with the correct ownership and permissions.
+          ;; Make sure Transmission Daemon's configuration and download
+          ;; directories have been created with the correct ownership and
+          ;; permissions.
           (test-assert "configuration directory exists"
             (marionette-eval
              '(eq? (stat:type (stat #$%transmission-daemon-config-dir))
@@ -132,6 +135,26 @@
                     #o750)
              marionette))
 
+          (test-assert "download directory exists"
+            (marionette-eval
+             '(eq? (stat:type (stat #$%transmission-daemon-download-dir))
+                   'directory)
+             marionette))
+          (test-assert "download directory has correct ownership"
+            (marionette-eval
+             '(let ((download-dir (stat #$%transmission-daemon-download-dir))
+                    (transmission-user (getpwnam #$%transmission-daemon-user)))
+                (and (eqv? (stat:uid download-dir)
+                           (passwd:uid transmission-user))
+                     (eqv? (stat:gid download-dir)
+                           (passwd:gid transmission-user))))
+             marionette))
+          (test-assert "download directory has expected permissions"
+            (marionette-eval
+             '(eqv? (stat:perms (stat #$%transmission-daemon-download-dir))
+                    #o750)
+             marionette))
+
           ;; Make sure the incomplete-downloads and watch directories have been
           ;; created with the correct ownership and permissions.
           (test-assert "incomplete-downloads directory exists"