Subject: [bug#49419] [PATCH 0/4] Essential home services

From: Andrew Tropin <[email-masked]>
Date: Wed, 28 Jul 2021 08:35:59 +0300

Ludovic Courtès <[email-masked]> writes:

> Hi Andrew,
>
> Andrew Tropin <[email-masked]> skribis:
>
>> Diff with v2: Prevents unecessary calls to system*
>>
>> Please, when review finished, apply against guix-home-wip branch.
>>
>> Andrew Tropin (4):
>>   home-services: Add most essential home services
>>   home-services: Add home-run-on-change-service-type
>>   home-services: Add home-provenance-service-type
>>   home-services: Add fold-home-service-types function
>
> Thanks for sending this first patch series!
>
> How would you like to proceed?  Sending patches that add essential
> services, and then (guix scripts home …) modules?

Yep.

> I agree we should apply it all in ‘wip-guix-home’ for now.
>
> Some general comments:
>
>   • Please remove tabs from Scheme files.

From 26bfd8052d90650abc7e5ec6dbb7dd7165dfba3c Mon Sep 17 00:00:00 2001
From: Andrew Tropin <[email-masked]>
Date: Wed, 28 Jul 2021 08:22:20 +0300
Subject: [PATCH] (toberebased) gnu: home-services: Untabify a file

---
 gnu/home-services.scm | 80 +++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/gnu/home-services.scm b/gnu/home-services.scm
index 9afb70f0a7..94f0ccff7a 100644
--- a/gnu/home-services.scm
+++ b/gnu/home-services.scm
@@ -33,10 +33,10 @@
   #:use-module (ice-9 match)
 
   #:export (home-service-type
-	    home-profile-service-type
-	    home-environment-variables-service-type
-	    home-files-service-type
-	    home-run-on-first-login-service-type
+            home-profile-service-type
+            home-environment-variables-service-type
+            home-files-service-type
+            home-run-on-first-login-service-type
             home-activation-service-type
             home-run-on-change-service-type
             home-provenance-service-type
@@ -44,8 +44,8 @@
             fold-home-service-types)
 
   #:re-export (service
-	       service-type
-	       service-extension))
+               service-type
+               service-extension))
 
 ;;; Comment:
 ;;;
@@ -76,7 +76,7 @@ directory containing the given entries."
                 (extensions '())
                 (compose identity)
                 (extend home-derivation)
-		(default-value '())
+                (default-value '())
                 (description
                  "Build the home environment top-level directory,
 which in turn refers to everything the home environment needs: its
@@ -130,12 +130,12 @@ exported."
     (fold
      (lambda (x acc)
        (when (equal? (car x) (car acc))
-	 (warning
-	  (G_ "duplicate definition for `~a' environment variable ~%") (car x)))
+         (warning
+          (G_ "duplicate definition for `~a' environment variable ~%") (car x)))
        x)
      (cons "" "")
      (sort vars (lambda (a b)
-		  (string<? (car a) (car b))))))
+                  (string<? (car a) (car b))))))
 
   (warn-about-duplicate-defenitions)
   (with-monad
@@ -145,7 +145,7 @@ exported."
        ;; TODO: It's necessary to source ~/.guix-profile too
        ;; on foreign distros
        ,(apply mixed-text-file "setup-environment"
-	       "\
+               "\
 HOME_ENVIRONMENT=$HOME/.guix-home
 GUIX_PROFILE=\"$HOME_ENVIRONMENT/profile\"
 PROFILE_FILE=\"$HOME_ENVIRONMENT/profile/etc/profile\"
@@ -174,25 +174,25 @@ esac
 
 "
 
-	       (append-map
-		(match-lambda
-		  ((key . #f)
-		   '())
-		  ((key . #t)
-		   (list "export " key "\n"))
-		  ((key . value)
+               (append-map
+                (match-lambda
+                  ((key . #f)
+                   '())
+                  ((key . #t)
+                   (list "export " key "\n"))
+                  ((key . value)
                    (list "export " key "=" value "\n")))
-		vars)))))))
+                vars)))))))
 
 (define home-environment-variables-service-type
   (service-type (name 'home-environment-variables)
                 (extensions
                  (list (service-extension
-			home-service-type
+                        home-service-type
                         environment-variables->setup-environment-script)))
                 (compose concatenate)
                 (extend append)
-		(default-value '())
+                (default-value '())
                 (description "Set the environment variables.")))
 
 (define (files->files-directory files)
@@ -227,7 +227,7 @@ directory containing FILES."
                                           files-entry)))
                 (compose concatenate)
                 (extend append)
-		(default-value '())
+                (default-value '())
                 (description "Configuration files for programs that
 will be put in @file{~/.guix-home/files}.")))
 
@@ -235,32 +235,32 @@ will be put in @file{~/.guix-home/files}.")))
   (gexp->script
    "on-first-login"
    #~(let* ((xdg-runtime-dir (or (getenv "XDG_RUNTIME_DIR")
-				 (format #f "/run/user/~a" (getuid))))
-	    (flag-file-path (string-append
-			     xdg-runtime-dir "/on-first-login-executed"))
-	    (touch (lambda (file-name)
-		     (call-with-output-file file-name (const #t)))))
+                                 (format #f "/run/user/~a" (getuid))))
+            (flag-file-path (string-append
+                             xdg-runtime-dir "/on-first-login-executed"))
+            (touch (lambda (file-name)
+                     (call-with-output-file file-name (const #t)))))
        ;; XDG_RUNTIME_DIR dissapears on logout, that means such trick
        ;; allows to launch on-first-login script on first login only
        ;; after complete logout/reboot.
        (when (not (file-exists? flag-file-path))
-	 (begin #$@gexps (touch flag-file-path))))))
+         (begin #$@gexps (touch flag-file-path))))))
 
 (define (on-first-login-script-entry m-on-first-login)
   "Return, as a monadic value, an entry for the on-first-login script
 in the home environment directory."
   (mlet %store-monad ((on-first-login m-on-first-login))
-	(return `(("on-first-login" ,on-first-login)))))
+        (return `(("on-first-login" ,on-first-login)))))
 
 (define home-run-on-first-login-service-type
   (service-type (name 'home-run-on-first-login)
                 (extensions
                  (list (service-extension
-			home-service-type
+                        home-service-type
                         on-first-login-script-entry)))
                 (compose identity)
                 (extend compute-on-first-login-script)
-		(default-value #f)
+                (default-value #f)
                 (description "Run gexps on first user login.  Can be
 extended with one gexp.")))
 
@@ -281,18 +281,18 @@ extended with one gexp.")))
                               #f))))
        (if (file-exists? (he-init-file new-home))
            (let* ((port   ((@ (ice-9 popen) open-input-pipe)
-		           (format #f "source ~a && env"
+                           (format #f "source ~a && env"
                                    (he-init-file new-home))))
-	          (result ((@ (ice-9 rdelim) read-delimited) "" port))
-	          (vars (map (lambda (x)
+                  (result ((@ (ice-9 rdelim) read-delimited) "" port))
+                  (vars (map (lambda (x)
                                (let ((si (string-index x #\=)))
                                  (cons (string-take x si)
                                        (string-drop x (1+ si)))))
-			     ((@ (srfi srfi-1) remove)
-			      string-null?
+                             ((@ (srfi srfi-1) remove)
+                              string-null?
                               (string-split result #\newline)))))
-	     (close-port port)
-	     (map (lambda (x) (setenv (car x) (cdr x))) vars)
+             (close-port port)
+             (map (lambda (x) (setenv (car x) (cdr x))) vars)
 
              (setenv "GUIX_NEW_HOME" new-home)
              (setenv "GUIX_OLD_HOME" old-home)
@@ -319,11 +319,11 @@ in the home environment directory."
   (service-type (name 'home-activation)
                 (extensions
                  (list (service-extension
-			home-service-type
+                        home-service-type
                         activation-script-entry)))
                 (compose identity)
                 (extend compute-activation-script)
-		(default-value #f)
+                (default-value #f)
                 (description "Run gexps to activate the current
 generation of home environment and update the state of the home
 directory.  @command{activate} script automatically called during
-- 
2.32.0

> • Please do not write documentation in commit logs.  For example,
>     patch #1 explains the different service types, but to me, that’d
>     belong in a comment or (better yet) in a section of the manual.  For
>     commit logs, we use ChangeLog style:
>
>       https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html
>
>     It’s OK if you don’t get the fine points right from the start,
>     committers can tweak it for you.  :-)

True, I forgot to add ChangeLog style parts to first two patches.  The
rest of commit message originally was just an explanation for reviewers
to provide a context, but yep it's already looks like a documentation)

>
> • When there are tests or documentation, add them in the commit that
>     adds the corresponding functionality.

Wanted to add documentation with a separate patch series to make patch
series to wip-guix-home be smaller and easier for review, but probably
you are right, I should add related documentation in the same series.

>   • Regarding module names: what about putting everything in the (gnu
>     home …) name space.  For services, I wonder if we could simply use
>     (gnu services home), for the essential services, and other (gnu
>     services …) module, but that assumes some code can be shared between
>     System and Home.  Thoughts?

There was a thread on rde-devel about moving home services to (gnu
services ...), in the second half of the first response I provided some
arguments against this change.
https://lists.sr.ht/~abcdw/rde-devel/%3C87y2cqifpx.fsf%40yoctocell.xyz%3E

However, I can miss some niceties, so I still open for discussion if you
think that arguments from the thread isn't valid anymore or not valid at
all.

> I’ll look at the actual patches later, but I invite others to chime in
> too.  :-)

Cool, I'll wait for the review of the code and will prepare a new
version of patch series after that.

Thank you for the comments!