unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH 3/4] gnu: shadow: User shells point to current profile
@ 2015-04-07 17:37 Andy Wingo
  2015-04-17  8:50 ` Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Wingo @ 2015-04-07 17:37 UTC (permalink / raw)
  To: guix-devel

* gnu/system/shadow.scm (<user-account>): "shell" field is a string,
  and should point to the current profile.  This allows pkexec's
  dubious check that $SHELL is a valid shell to succeed.
* gnu/services/base.scm (guix-build-accounts):
* gnu/services/networking.scm (bitlbee-service):
* gnu/system.scm (user-account->gexp): Update users.
---
 gnu/services/base.scm       | 2 +-
 gnu/services/networking.scm | 3 +--
 gnu/system.scm              | 2 +-
 gnu/system/shadow.scm       | 4 ++--
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 956fa7e..f936b17 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -587,7 +587,7 @@ starting at FIRST-UID, and under GID."
 
              (comment (format #f "Guix Build User ~2d" n))
              (home-directory "/var/empty")
-             (shell #~(string-append #$shadow "/sbin/nologin"))))
+             (shell "/run/current-system/profile/sbin/nologin")))
           1+
           1))
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index af8dd43..b63ce14 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -305,8 +305,7 @@ configuration file."
                             (system? #t)
                             (comment "BitlBee daemon user")
                             (home-directory "/var/empty")
-                            (shell #~(string-append #$shadow
-                                                    "/sbin/nologin")))))))))
+                            (shell "/run/current-system/profile/sbin/nologin"))))))))
 
 (define* (wicd-service #:key (wicd wicd))
   "Return a service that runs @url{https://launchpad.net/wicd,Wicd}, a network
diff --git a/gnu/system.scm b/gnu/system.scm
index 6cf12df..71c5ce8 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -588,7 +588,7 @@ fi\n"))
       #$(user-account-supplementary-groups account)
       #$(user-account-comment account)
       #$(user-account-home-directory account)
-      ,#$(user-account-shell account)             ; this one is a gexp
+      #$(user-account-shell account)
       #$(user-account-password account)
       #$(user-account-system? account)))
 
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index 16b9e4b..3bf1214 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -67,8 +67,8 @@
                         (default '()))            ; list of strings
   (comment        user-account-comment (default ""))
   (home-directory user-account-home-directory)
-  (shell          user-account-shell              ; gexp
-                  (default #~(string-append #$bash "/bin/bash")))
+  (shell          user-account-shell              ; string
+                  (default "/run/current-system/profile/bin/bash"))
   (system?        user-account-system?            ; Boolean
                   (default #f)))
 
-- 
2.2.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/4] gnu: shadow: User shells point to current profile
  2015-04-07 17:37 [PATCH 3/4] gnu: shadow: User shells point to current profile Andy Wingo
@ 2015-04-17  8:50 ` Ludovic Courtès
  2015-04-17 10:49   ` 宋文武
  0 siblings, 1 reply; 4+ messages in thread
From: Ludovic Courtès @ 2015-04-17  8:50 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guix-devel

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

Andy Wingo <wingo@pobox.com> skribis:

> -             (shell #~(string-append #$shadow "/sbin/nologin"))))
> +             (shell "/run/current-system/profile/sbin/nologin")))

[...]

> -                            (shell #~(string-append #$shadow
> -                                                    "/sbin/nologin")))))))))
> +                            (shell "/run/current-system/profile/sbin/nologin"))))))))

[...]

> +  (shell          user-account-shell              ; string
> +                  (default "/run/current-system/profile/bin/bash"))

The problem I see with this approach is that it will only work if the
shell is actually install in the global profile, and it’s really a
workaround: users could still use a gexp as for the ‘shell’ field.

I think we should instead generate /etc/shells based on the ‘shell’
field of each user account, so that it matches exactly what’s being
used:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3563 bytes --]

diff --git a/gnu/system.scm b/gnu/system.scm
index 6cf12df..0df8323 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -405,30 +405,47 @@ settings for 'guix.el' to work out-of-the-box."
                           (chdir #$output)
                           (symlink #$file "site-start.el")))))
 
+(define (user-shells os)
+  "Return the list of shells used by the accounts of OS.  These may be gexps
+or strings."
+  (mlet %store-monad ((accounts (operating-system-accounts os)))
+    (return (map user-account-shell accounts))))
+
+(define (shells-file shells)
+  "Return a derivation that builds a shell list for use as /etc/shells based
+on SHELLS.  /etc/shells is used by xterm, polkit, and other programs."
+  (gexp->derivation "shells"
+                    #~(begin
+                        (use-modules (srfi srfi-1))
+
+                        (define shells
+                          (delete-duplicates (list #$@shells)))
+
+                        (call-with-output-file #$output
+                          (lambda (port)
+                            (display "\
+/bin/sh
+/run/current-system/profile/bin/sh
+/run/current-system/profile/bin/bash\n" port)
+                            (for-each (lambda (shell)
+                                        (display shell port)
+                                        (newline port))
+                                      shells))))))
+
 (define* (etc-directory #:key
                         (locale "C") (timezone "Europe/Paris")
                         (issue "Hello!\n")
                         (skeletons '())
                         (pam-services '())
                         (profile "/run/current-system/profile")
-                        hosts-file nss
+                        hosts-file nss (shells '())
                         (sudoers ""))
   "Return a derivation that builds the static part of the /etc directory."
   (mlet* %store-monad
       ((pam.d      (pam-services->directory pam-services))
        (sudoers    (text-file "sudoers" sudoers))
        (login.defs (text-file "login.defs" "# Empty for now.\n"))
-
-       ;; /etc/shells is used by xterm and other programs.   We don't check
-       ;; whether these shells are installed, should be OK.
-       (shells     (text-file "shells"
-                              "\
-/bin/sh
-/run/current-system/profile/bin/sh
-/run/current-system/profile/bin/bash
-/run/current-system/profile/bin/fish
-/run/current-system/profile/bin/tcsh
-/run/current-system/profile/bin/zsh\n"))
+       (shells     (shells-file shells))
        (emacs      (emacs-site-directory))
        (issue      (text-file "issue" issue))
        (nsswitch   (text-file "nsswitch.conf"
@@ -543,7 +560,8 @@ fi\n"))
        (profile-drv (operating-system-profile os))
        (skeletons   (operating-system-skeletons os))
        (/etc/hosts  (or (operating-system-hosts-file os)
-                        (default-/etc/hosts (operating-system-host-name os)))))
+                        (default-/etc/hosts (operating-system-host-name os))))
+       (shells      (user-shells os)))
    (etc-directory #:pam-services pam-services
                   #:skeletons skeletons
                   #:issue (operating-system-issue os)
@@ -551,6 +569,7 @@ fi\n"))
                   #:nss (operating-system-name-service-switch os)
                   #:timezone (operating-system-timezone os)
                   #:hosts-file /etc/hosts
+                  #:shells shells
                   #:sudoers (operating-system-sudoers os)
                   #:profile profile-drv)))
 

[-- Attachment #3: Type: text/plain, Size: 52 bytes --]


Thoughts?  宋文武, WDYT?

Thanks,
Ludo’.

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/4] gnu: shadow: User shells point to current profile
  2015-04-17  8:50 ` Ludovic Courtès
@ 2015-04-17 10:49   ` 宋文武
  2015-04-17 11:54     ` Ludovic Courtès
  0 siblings, 1 reply; 4+ messages in thread
From: 宋文武 @ 2015-04-17 10:49 UTC (permalink / raw)
  To: Ludovic Courtès, Andy Wingo; +Cc: guix-devel

Ludovic Courtès <ludo@gnu.org> writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> -             (shell #~(string-append #$shadow "/sbin/nologin"))))
>> +             (shell "/run/current-system/profile/sbin/nologin")))
>
> [...]
>
>> -                            (shell #~(string-append #$shadow
>> -                                                    "/sbin/nologin")))))))))
>> +                            (shell "/run/current-system/profile/sbin/nologin"))))))))
>
> [...]
>
>> +  (shell          user-account-shell              ; string
>> +                  (default "/run/current-system/profile/bin/bash"))
>
> The problem I see with this approach is that it will only work if the
> shell is actually install in the global profile, and it’s really a
> workaround: users could still use a gexp as for the ‘shell’ field.
>
> I think we should instead generate /etc/shells based on the ‘shell’
> field of each user account, so that it matches exactly what’s being
> used:
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index 6cf12df..0df8323 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -405,30 +405,47 @@ settings for 'guix.el' to work out-of-the-box."
>                            (chdir #$output)
>                            (symlink #$file "site-start.el")))))
>  
> +(define (user-shells os)
> +  "Return the list of shells used by the accounts of OS.  These may be gexps
> +or strings."
> +  (mlet %store-monad ((accounts (operating-system-accounts os)))
> +    (return (map user-account-shell accounts))))
> +
> +(define (shells-file shells)
> +  "Return a derivation that builds a shell list for use as /etc/shells based
> +on SHELLS.  /etc/shells is used by xterm, polkit, and other programs."
> +  (gexp->derivation "shells"
> +                    #~(begin
> +                        (use-modules (srfi srfi-1))
> +
> +                        (define shells
> +                          (delete-duplicates (list #$@shells)))
> +
> +                        (call-with-output-file #$output
> +                          (lambda (port)
> +                            (display "\
> +/bin/sh
> +/run/current-system/profile/bin/sh
> +/run/current-system/profile/bin/bash\n" port)
> +                            (for-each (lambda (shell)
> +                                        (display shell port)
> +                                        (newline port))
> +                                      shells))))))
> +
>  (define* (etc-directory #:key
>                          (locale "C") (timezone "Europe/Paris")
>                          (issue "Hello!\n")
>                          (skeletons '())
>                          (pam-services '())
>                          (profile "/run/current-system/profile")
> -                        hosts-file nss
> +                        hosts-file nss (shells '())
>                          (sudoers ""))
>    "Return a derivation that builds the static part of the /etc directory."
>    (mlet* %store-monad
>        ((pam.d      (pam-services->directory pam-services))
>         (sudoers    (text-file "sudoers" sudoers))
>         (login.defs (text-file "login.defs" "# Empty for now.\n"))
> -
> -       ;; /etc/shells is used by xterm and other programs.   We don't check
> -       ;; whether these shells are installed, should be OK.
> -       (shells     (text-file "shells"
> -                              "\
> -/bin/sh
> -/run/current-system/profile/bin/sh
> -/run/current-system/profile/bin/bash
> -/run/current-system/profile/bin/fish
> -/run/current-system/profile/bin/tcsh
> -/run/current-system/profile/bin/zsh\n"))
> +       (shells     (shells-file shells))
>         (emacs      (emacs-site-directory))
>         (issue      (text-file "issue" issue))
>         (nsswitch   (text-file "nsswitch.conf"
> @@ -543,7 +560,8 @@ fi\n"))
>         (profile-drv (operating-system-profile os))
>         (skeletons   (operating-system-skeletons os))
>         (/etc/hosts  (or (operating-system-hosts-file os)
> -                        (default-/etc/hosts (operating-system-host-name os)))))
> +                        (default-/etc/hosts (operating-system-host-name os))))
> +       (shells      (user-shells os)))
>     (etc-directory #:pam-services pam-services
>                    #:skeletons skeletons
>                    #:issue (operating-system-issue os)
> @@ -551,6 +569,7 @@ fi\n"))
>                    #:nss (operating-system-name-service-switch os)
>                    #:timezone (operating-system-timezone os)
>                    #:hosts-file /etc/hosts
> +                  #:shells shells
>                    #:sudoers (operating-system-sudoers os)
>                    #:profile profile-drv)))
>  
>
> Thoughts?  宋文武, WDYT?
Totally argee, and I find that my xterm will only work correctly
when /etc/shell contains '/gnu/store/...-bash-.../bin/bash',
otherwise launch a new xterm in an opened one will spawn 'sh'
instead of 'bash'.
>
> Thanks,
> Ludo’.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/4] gnu: shadow: User shells point to current profile
  2015-04-17 10:49   ` 宋文武
@ 2015-04-17 11:54     ` Ludovic Courtès
  0 siblings, 0 replies; 4+ messages in thread
From: Ludovic Courtès @ 2015-04-17 11:54 UTC (permalink / raw)
  To: 宋文武; +Cc: Andy Wingo, guix-devel

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

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I think we should instead generate /etc/shells based on the ‘shell’
>> field of each user account, so that it matches exactly what’s being
>> used:

[...]

> Totally argee, and I find that my xterm will only work correctly
> when /etc/shell contains '/gnu/store/...-bash-.../bin/bash',
> otherwise launch a new xterm in an opened one will spawn 'sh'
> instead of 'bash'.

Right.  Committed as 8e974b9.  Let us know if you notice a regression!

Thanks for the quick feedback,
Ludo’.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-04-17 11:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-07 17:37 [PATCH 3/4] gnu: shadow: User shells point to current profile Andy Wingo
2015-04-17  8:50 ` Ludovic Courtès
2015-04-17 10:49   ` 宋文武
2015-04-17 11:54     ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).