Package: guix-patches;
Reported by: Andrew Tropin <andrew <at> trop.in>
Date: Mon, 5 Jul 2021 15:36:01 UTC
Severity: normal
Tags: patch
Merged with 49546, 49547, 49548, 49549
Done: Oleg Pykhalov <go.wigust <at> gmail.com>
Bug is archived. No further changes may be made.
Message #61 received at 49419 <at> debbugs.gnu.org (full text, mbox):
From: Andrew Tropin <andrew <at> trop.in> To: Ludovic Courtès <ludo <at> gnu.org> Cc: 49419 <at> debbugs.gnu.org Subject: Re: bug#49419: [PATCH 0/4] Essential home services Date: Wed, 28 Jul 2021 08:35:59 +0300
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> writes: > Hi Andrew, > > Andrew Tropin <andrew <at> trop.in> 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.
[0001-toberebased-gnu-home-services-Untabify-a-file.patch (text/x-patch, inline)]
From 26bfd8052d90650abc7e5ec6dbb7dd7165dfba3c Mon Sep 17 00:00:00 2001
From: Andrew Tropin <andrew <at> trop.in>
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
[Message part 3 (text/plain, inline)]
> • 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!
[signature.asc (application/pgp-signature, inline)]
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.