diff mbox series

[bug#78603] services: readymedia: Respect SUDO_HOME if configuring for home.

Message ID abfbb47782cfe6c6800b86731658c4ed9c4e524a.1748337976.git.sughosha@disroot.org
State New
Headers
Series [bug#78603] services: readymedia: Respect SUDO_HOME if configuring for home. |

Commit Message

Sughosha May 27, 2025, 9:26 a.m. UTC 
  This fixes the service that is configured for a home environment,
defined with "guix-home-service-type" in a system configuration, using "sudo",
with "/root" as "$HOME" instead of the required home directory.

* gnu/services/upnp.scm (readymedia-configuration)[cache-directory]: Respect
SUDO_HOME if configuring for home.
[log-directory]: Ditto.

Change-Id: Ie6905c0b83608f91582671cde9d866079178f192
---
 gnu/services/upnp.scm | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)


base-commit: c15f786f8936502249b639220997094fdbf7f1e8

Comments

Sergey Trofimov May 27, 2025, 11:45 a.m. UTC | #1 
Hi Sughosha,

Sughosha <sughosha@disroot.org> writes:

> This fixes the service that is configured for a home environment,
> defined with "guix-home-service-type" in a system configuration, using "sudo",
> with "/root" as "$HOME" instead of the required home directory.
>
> * gnu/services/upnp.scm (readymedia-configuration)[cache-directory]: Respect
> SUDO_HOME if configuring for home.
> [log-directory]: Ditto.
>
> Change-Id: Ie6905c0b83608f91582671cde9d866079178f192
> ---
>  gnu/services/upnp.scm | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/gnu/services/upnp.scm b/gnu/services/upnp.scm
> index 8267b1e53af..edd55594e38 100644
> --- a/gnu/services/upnp.scm
> +++ b/gnu/services/upnp.scm
> @@ -74,19 +74,22 @@ (define-record-type* <readymedia-configuration>
>          (default #f))
>    (cache-directory readymedia-configuration-cache-directory
>                     (default (if for-home?
> -                                (string-append (or (getenv "XDG_CACHE_HOME")
> -                                                   (string-append
> -                                                    (getenv "HOME") "/.cache"))
> -                                               "/readymedia")
> -                              %readymedia-default-cache-directory)))
> +                                (if (getenv "XDG_CACHE_HOME")
> +                                    (string-append (getenv "XDG_CACHE_HOME")
> +                                                   "/readymedia")
> +                                    (string-append (or (getenv "SUDO_HOME")
> +                                                       (getenv "HOME"))
> +                                                   "/.cache/readymedia"))
> +                                %readymedia-default-cache-directory)))

That's a brittle solution: this code runs when the file is loaded,
setting the defaults to the values of host environment. This is not what
you generally want, because the target environment might be different,
i.e. I might be building home profile for another user and the resulting
config should contain their username.

Check how `syncthing-configuration` works. I think that the simplest
solution would be to use relative paths for installations to user homes.
The shepherd service is run from the $HOME, so just
`db_dir=.cache/readymedia` should work. If the user wants to use XDG
dirs, they should pass the correct values explicitly.
Sughosha May 27, 2025, 1:27 p.m. UTC | #2 
On Tuesday, May 27, 2025 5:15:10 PM GMT+5:30 Sergey Trofimov wrote:
> Hi Sughosha,
> 
> Sughosha <sughosha@disroot.org> writes:
> > This fixes the service that is configured for a home environment,
> > defined with "guix-home-service-type" in a system configuration, using
> > "sudo", with "/root" as "$HOME" instead of the required home directory.
> > 
> > * gnu/services/upnp.scm (readymedia-configuration)[cache-directory]:
> > Respect SUDO_HOME if configuring for home.
> > [log-directory]: Ditto.
> > 
> > Change-Id: Ie6905c0b83608f91582671cde9d866079178f192
> > ---
> > 
> >  gnu/services/upnp.scm | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/gnu/services/upnp.scm b/gnu/services/upnp.scm
> > index 8267b1e53af..edd55594e38 100644
> > --- a/gnu/services/upnp.scm
> > +++ b/gnu/services/upnp.scm
> > @@ -74,19 +74,22 @@ (define-record-type* <readymedia-configuration>
> > 
> >          (default #f))
> >    
> >    (cache-directory readymedia-configuration-cache-directory
> >    
> >                     (default (if for-home?
> > 
> > -                                (string-append (or (getenv
> > "XDG_CACHE_HOME") -                                                  
> > (string-append -                                                   
> > (getenv "HOME") "/.cache")) -                                            
> >   "/readymedia")
> > -                              %readymedia-default-cache-directory)))
> > +                                (if (getenv "XDG_CACHE_HOME")
> > +                                    (string-append (getenv
> > "XDG_CACHE_HOME") +                                                  
> > "/readymedia") +                                    (string-append (or
> > (getenv "SUDO_HOME") +                                                   
> >    (getenv "HOME")) +                                                  
> > "/.cache/readymedia")) +                               
> > %readymedia-default-cache-directory)))
> That's a brittle solution: this code runs when the file is loaded,
> setting the defaults to the values of host environment. This is not what
> you generally want, because the target environment might be different,
> i.e. I might be building home profile for another user and the resulting
> config should contain their username.
> 
> Check how `syncthing-configuration` works. I think that the simplest
> solution would be to use relative paths for installations to user homes.
> The shepherd service is run from the $HOME, so just
> `db_dir=.cache/readymedia` should work. If the user wants to use XDG
> dirs, they should pass the correct values explicitly.
Using relative paths without any variable is working. I will send v2 patch.
Sergey Trofimov May 27, 2025, 1:56 p.m. UTC | #3 
Hi Sughosha,

Sughosha <sughosha@disroot.org> writes:
[...]
> Using relative paths without any variable is working. I will send v2 patch.
>

I've also noticed a couple other issues:
- `(getuid)` used in `readymedia-configuration->config-file`. It could
be removed altogether, no need to specify `user=` for home installations
- log_dir seem to be ineffective when starting the app with -s. It logs
to stdout when running in foreground. You can just remove this parameter
- #:log-file for a shepherd service should be located either in
%user-log-dir or "/var/log". Please check e.g. mcron service
- not sure if least-authority-wrapper would work for symlinked media
directories (see wide_links config option)
diff mbox series

Patch

diff --git a/gnu/services/upnp.scm b/gnu/services/upnp.scm
index 8267b1e53af..edd55594e38 100644
--- a/gnu/services/upnp.scm
+++ b/gnu/services/upnp.scm
@@ -74,19 +74,22 @@  (define-record-type* <readymedia-configuration>
         (default #f))
   (cache-directory readymedia-configuration-cache-directory
                    (default (if for-home?
-                                (string-append (or (getenv "XDG_CACHE_HOME")
-                                                   (string-append
-                                                    (getenv "HOME") "/.cache"))
-                                               "/readymedia")
-                              %readymedia-default-cache-directory)))
+                                (if (getenv "XDG_CACHE_HOME")
+                                    (string-append (getenv "XDG_CACHE_HOME")
+                                                   "/readymedia")
+                                    (string-append (or (getenv "SUDO_HOME")
+                                                       (getenv "HOME"))
+                                                   "/.cache/readymedia"))
+                                %readymedia-default-cache-directory)))
   (log-directory readymedia-configuration-log-directory
                  (default (if for-home?
-                              (string-append (or (getenv "XDG_STATE_HOME")
-                                                 (string-append
-                                                  (getenv "HOME")
-                                                  "/.local/state"))
-                                             "/readymedia")
-                            %readymedia-default-log-directory)))
+                              (if (getenv "XDG_STATE_HOME")
+                                  (string-append (getenv "XDG_STATE_HOME")
+                                                 "/readymedia")
+                                  (string-append (or (getenv "SUDO_HOME")
+                                                     (getenv "HOME"))
+                                                 "/.local/state/readymedia"))
+                              %readymedia-default-log-directory)))
   (friendly-name readymedia-configuration-friendly-name
                  (default #f))
   (media-directories readymedia-configuration-media-directories)