[PATCH 1/3] services: Add 'session-environment-service'.

[PATCH 1/3] services: Add 'session-environment-service'.

[宋文武]

* gnu/services/base.scm (session-environment-service): New procedure.
(session-environment-service-type): New variable.
(environment-variables->environment-file): New procedure.
---
 gnu/services/base.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index c242c7d..d599c52 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -48,6 +49,8 @@
             device-mapping-service
             swap-service
             user-processes-service
+            session-environment-service
+            session-environment-service-type
             host-name-service
             console-keymap-service
             console-font-service
@@ -368,6 +371,36 @@ stopped before 'kill' is called."
 
 \f
 ;;;
+;;; System-wide environment variables.
+;;;
+
+(define (environment-variables->environment-file vars)
+  "Return a file for pam_env(8) that contains environment variables VARS."
+  (apply mixed-text-file "environment"
+         (fold-right (lambda (pair result)
+                       (cons* (car pair) "=" (cdr pair) "\n" result))
+                     '() vars)))
+
+(define session-environment-service-type
+  (service-type
+   (name 'session-environment)
+   (extensions
+    (list (service-extension
+           etc-service-type
+           (lambda (vars)
+             (list `("environment"
+                     ,(environment-variables->environment-file vars)))))))))
+
+(define (session-environment-service vars)
+  "Return a service that builds the @file{/etc/environment}, which can be read
+by PAM-aware applications to set environment variables for sessions.
+
+VARS should be an association list in which both the keys and the values are
+strings or string-valued gexps."
+  (service session-environment-service-type vars))
+
+\f
+;;;
 ;;; Console & co.
 ;;;
 
-- 
2.5.0

[PATCH 2/3] system: Add /etc/environment.

[宋文武]

* gnu/system.scm: (operating-system-environment-variables): New procedure.
Adapted and moved environment variables from /etc/profile.
(operating-system-etc-service): Adjust accordingly.
(essential-services): Add an instance of SESSION-ENVIRONMENT-SERVICE-TYPE.
---
 gnu/system.scm | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 2755d85..ff981d9 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -299,6 +299,8 @@ a container or that of a \"bare metal\" system."
                                     (operating-system-groups os))
                             (operating-system-skeletons os))
            (operating-system-etc-service os)
+           (session-environment-service
+            (operating-system-environment-variables os))
            host-name procs root-fs unmount
            (service setuid-program-service-type
                     (operating-system-setuid-programs os))
@@ -417,18 +419,6 @@ directory."
         ;; Startup file for POSIX-compliant login shells, which set system-wide
         ;; environment variables.
         (profile    (mixed-text-file "profile"  "\
-export LANG=\"" (operating-system-locale os) "\"
-export TZ=\"" (operating-system-timezone os) "\"
-export TZDIR=\"" tzdata "/share/zoneinfo\"
-
-# Tell 'modprobe' & co. where to look for modules.
-export LINUX_MODULE_DIRECTORY=/run/booted-system/kernel/lib/modules
-
-# These variables are honored by OpenSSL (libssl) and Git.
-export SSL_CERT_DIR=/etc/ssl/certs
-export SSL_CERT_FILE=\"$SSL_CERT_DIR/ca-certificates.crt\"
-export GIT_SSL_CAINFO=\"$SSL_CERT_FILE\"
-
 # Crucial variables that could be missing in the profiles' 'etc/profile'
 # because they would require combining both profiles.
 # FIXME: See <http://bugs.gnu.org/20255>.
@@ -458,13 +448,6 @@ else
   export PATH=\"$HOME/.guix-profile/bin:$PATH\"
 fi
 
-# Append the directory of 'site-start.el' to the search path.
-export EMACSLOADPATH=:/etc/emacs
-
-# By default, applications that use D-Bus, such as Emacs, abort at startup
-# when /etc/machine-id is missing.  Make sure these warnings are non-fatal.
-export DBUS_FATAL_WARNINGS=0
-
 # Allow Aspell to find dictionaries installed in the user profile.
 export ASPELL_CONF=\"dict-dir $HOME/.guix-profile/lib/aspell\"
 
@@ -561,6 +544,24 @@ use 'plain-file' instead~%")
    (fold-services (operating-system-services os)
                   #:target-type etc-service-type)))
 
+(define (operating-system-environment-variables os)
+  "Return the environment variables of OS for
+@var{session-environment-service-type}, to be used in @file{/etc/environment}."
+  `(("LANG" . ,(operating-system-locale os))
+    ("TZ" . ,(operating-system-timezone os))
+    ("TZDIR" . ,#~(string-append #$tzdata "/share/zoneinfo"))
+    ;; Tell 'modprobe' & co. where to look for modules.
+    ("LINUX_MODULE_DIRECTORY" . "/run/booted-system/kernel/lib/modules")
+    ;; These variables are honored by OpenSSL (libssl) and Git.
+    ("SSL_CERT_DIR" . "/etc/ssl/certs")
+    ("SSL_CERT_FILE" . "/etc/ssl/certs/ca-certificates.crt")
+    ("GIT_SSL_CAINFO" . "/etc/ssl/certs/ca-certificates.crt")
+    ;; Append the directory of 'site-start.el' to the search path.
+    ("EMACSLOADPATH" . ":/etc/emacs")
+    ;; By default, applications that use D-Bus, such as Emacs, abort at startup
+    ;; when /etc/machine-id is missing.  Make sure these warnings are non-fatal.
+    ("DBUS_FATAL_WARNINGS" . "0")))
+
 (define %setuid-programs
   ;; Default set of setuid-root programs.
   (let ((shadow (@ (gnu packages admin) shadow)))
-- 
2.5.0

[PATCH 3/3] system: pam: Honor /etc/environment.

[宋文武]

* gnu/system/pam.scm (unix-pam-service): Add pam_env module to the session group.
---
 gnu/system/pam.scm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
index d8470f0..9823916 100644
--- a/gnu/system/pam.scm
+++ b/gnu/system/pam.scm
@@ -128,7 +128,10 @@ dumped in /etc/pam.d/NAME, where NAME is the name of SERVICE."
 (define unix-pam-service
   (let ((unix (pam-entry
                (control "required")
-               (module "pam_unix.so"))))
+               (module "pam_unix.so")))
+        (env  (pam-entry
+               (control "required")
+               (module "pam_env.so"))))
     (lambda* (name #:key allow-empty-passwords? motd)
       "Return a standard Unix-style PAM service for NAME.  When
 ALLOW-EMPTY-PASSWORDS? is true, allow empty passwords.  When MOTD is true, it
@@ -150,13 +153,13 @@ should be a file-like object used as the message-of-the-day."
                           ;; Store SHA-512 encrypted passwords in /etc/shadow.
                           (arguments '("sha512" "shadow")))))
          (session (if motd
-                      (list unix
+                      (list env unix
                             (pam-entry
                              (control "optional")
                              (module "pam_motd.so")
                              (arguments
                               (list #~(string-append "motd=" #$motd)))))
-                      (list unix))))))))
+                      (list env unix))))))))
 
 (define (rootok-pam-service command)
   "Return a PAM service for COMMAND such that 'root' does not need to
-- 
2.5.0

Re: [PATCH 1/3] services: Add 'session-environment-service'.

[Alex Kost]

宋文武 (2015-12-02 16:38 +0300) wrote:

> +(define (environment-variables->environment-file vars)
> +  "Return a file for pam_env(8) that contains environment variables VARS."
> +  (apply mixed-text-file "environment"
> +         (fold-right (lambda (pair result)
> +                       (cons* (car pair) "=" (cdr pair) "\n" result))
> +                     '() vars)))

Hm, car and cdr.  Wouldn't it be clearer to do it like this:

  (map (match-lambda
         ((name . value)
          (string-append name "=" value "\n")))
       vars)

-- 
Alex

Re: [PATCH 1/3] services: Add 'session-environment-service'.

[宋文武]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

Alex Kost <alezost@gmail.com> writes:

> 宋文武 (2015-12-02 16:38 +0300) wrote:
>
>> +(define (environment-variables->environment-file vars)
>> +  "Return a file for pam_env(8) that contains environment variables VARS."
>> +  (apply mixed-text-file "environment"
>> +         (fold-right (lambda (pair result)
>> +                       (cons* (car pair) "=" (cdr pair) "\n" result))
>> +                     '() vars)))
>
> Hm, car and cdr.  Wouldn't it be clearer to do it like this:
>
>   (map (match-lambda
>          ((name . value)
>           (string-append name "=" value "\n")))
>        vars)
Yes, match-lambda is more clear, thanks!
(I have to use list instead of string-append so that gexps can work)

Updated:


[-- Attachment #2: services: Add session-environment-service. --]
[-- Type: text/x-patch, Size: 2540 bytes --]

From 0ce1986d4cfca3d40bed4aaf93942cf9caaefb60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <iyzsong@gmail.com>
Date: Wed, 2 Dec 2015 19:59:29 +0800
Subject: [PATCH] services: Add 'session-environment-service'.

* gnu/services/base.scm (session-environment-service): New procedure.
(session-environment-service-type): New variable.
(environment-variables->environment-file): New procedure.
---
 gnu/services/base.scm | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index c242c7d..a86e8e0 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2015 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -48,6 +49,8 @@
             device-mapping-service
             swap-service
             user-processes-service
+            session-environment-service
+            session-environment-service-type
             host-name-service
             console-keymap-service
             console-font-service
@@ -368,6 +371,39 @@ stopped before 'kill' is called."
 
 \f
 ;;;
+;;; System-wide environment variables.
+;;;
+
+(define (environment-variables->environment-file vars)
+  "Return a file for pam_env(8) that contains environment variables VARS."
+  (apply mixed-text-file "environment"
+         (append-map (match-lambda
+                       ((key . value)
+                        (list key "=" value "\n")))
+                     vars)))
+
+(define session-environment-service-type
+  (service-type
+   (name 'session-environment)
+   (extensions
+    (list (service-extension
+           etc-service-type
+           (lambda (vars)
+             (list `("environment"
+                     ,(environment-variables->environment-file vars)))))))
+   (compose concatenate)
+   (extend append)))
+
+(define (session-environment-service vars)
+  "Return a service that builds the @file{/etc/environment}, which can be read
+by PAM-aware applications to set environment variables for sessions.
+
+VARS should be an association list in which both the keys and the values are
+strings or string-valued gexps."
+  (service session-environment-service-type vars))
+
+\f
+;;;
 ;;; Console & co.
 ;;;
 
-- 
2.5.0

Re: [PATCH 1/3] services: Add 'session-environment-service'.

[Ludovic Courtès]

iyzsong@member.fsf.org (宋文武) skribis:

> Alex Kost <alezost@gmail.com> writes:
>
>> 宋文武 (2015-12-02 16:38 +0300) wrote:
>>
>>> +(define (environment-variables->environment-file vars)
>>> +  "Return a file for pam_env(8) that contains environment variables VARS."
>>> +  (apply mixed-text-file "environment"
>>> +         (fold-right (lambda (pair result)
>>> +                       (cons* (car pair) "=" (cdr pair) "\n" result))
>>> +                     '() vars)))
>>
>> Hm, car and cdr.  Wouldn't it be clearer to do it like this:
>>
>>   (map (match-lambda
>>          ((name . value)
>>           (string-append name "=" value "\n")))
>>        vars)
> Yes, match-lambda is more clear, thanks!

Indeed.  :-)

> From 0ce1986d4cfca3d40bed4aaf93942cf9caaefb60 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= <iyzsong@gmail.com>
> Date: Wed, 2 Dec 2015 19:59:29 +0800
> Subject: [PATCH] services: Add 'session-environment-service'.
>
> * gnu/services/base.scm (session-environment-service): New procedure.
> (session-environment-service-type): New variable.
> (environment-variables->environment-file): New procedure.

LGTM, thanks!

Ludo’.

Re: [PATCH 2/3] system: Add /etc/environment.

[Ludovic Courtès]

宋文武 <iyzsong@gmail.com> skribis:

> * gnu/system.scm: (operating-system-environment-variables): New procedure.
> Adapted and moved environment variables from /etc/profile.
> (operating-system-etc-service): Adjust accordingly.
> (essential-services): Add an instance of SESSION-ENVIRONMENT-SERVICE-TYPE.

This looks good, but it must be applied after patch 3/3.  Otherwise,
we’d end up with a system that has an empty /etc/profile and an
/etc/environment file that is not honored.

Makes sense?

Thanks!

Ludo’.

Re: [PATCH 3/3] system: pam: Honor /etc/environment.

[Ludovic Courtès]

宋文武 <iyzsong@gmail.com> skribis:

> * gnu/system/pam.scm (unix-pam-service): Add pam_env module to the session group.

[...]

> +        (env  (pam-entry
> +               (control "required")
> +               (module "pam_env.so"))))

Please mention /etc/environment above.

Otherwise LGTM, thank you!

This patch is OK for now, but we should soon arrange it so that
‘session-environment-service’ extends ‘pam-root-service-type’ in a way
that adds ‘pam_env’ to the session management group of all the PAM
services.

This could be done by extending ‘pam-root-service-type’ with a
procedure.  That procedure would automatically be mapped over all the
PAM services to compute the final PAM services.

The same approach could then be used for pam_elogind.

Thoughts?

Ludo’.

Re: [PATCH 2/3] system: Add /etc/environment.

[宋文武]

ludo@gnu.org (Ludovic Courtès) writes:

> 宋文武 <iyzsong@gmail.com> skribis:
>
>> * gnu/system.scm: (operating-system-environment-variables): New procedure.
>> Adapted and moved environment variables from /etc/profile.
>> (operating-system-etc-service): Adjust accordingly.
>> (essential-services): Add an instance of SESSION-ENVIRONMENT-SERVICE-TYPE.
>
> This looks good, but it must be applied after patch 3/3.  Otherwise,
> we’d end up with a system that has an empty /etc/profile and an
> /etc/environment file that is not honored.
Sure, pushed, thanks for review!