[bug#32104] [PATCH 0/3] guix environment: Simplify environment creation

[bug#32104] [PATCH 0/3] guix environment: Simplify environment creation

[Ludovic Courtès]

Hello Guix!

These patches aim to simplify (guix scripts environment) by (1) factorizing
part of the search path handling machinery in (guix profiles), and (2) by
using manifests internally instead of loosely defined lists of “inputs.”

The goal is to gradually move towards a situation where (guix scripts
environment) contains nothing but UI code and related plumbing.

Ludo’.

Ludovic Courtès (3):
  profiles: Factorize 'manifest-search-paths'.
  environment: Simplify code by using manifests internally.
  profiles: Introduce 'profile-search-paths' and use it.

 guix/build/profiles.scm      |   2 +-
 guix/profiles.scm            |  26 ++++-
 guix/scripts/environment.scm | 208 ++++++++++++++---------------------
 3 files changed, 110 insertions(+), 126 deletions(-)

-- 
2.18.0

[bug#32104] [PATCH 1/3] profiles: Factorize 'manifest-search-paths'.

[Ludovic Courtès]

* guix/profiles.scm (manifest-search-paths): New procedure.
(profile-derivation)[builder]: Use it.
* guix/build/profiles.scm (build-etc/profile): Remove $PATH.
---
 guix/build/profiles.scm |  2 +-
 guix/profiles.scm       | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/guix/build/profiles.scm b/guix/build/profiles.scm
index 819688a91..df785c85a 100644
--- a/guix/build/profiles.scm
+++ b/guix/build/profiles.scm
@@ -89,7 +89,7 @@ definitions for all the SEARCH-PATHS."
 # When GUIX_PROFILE is undefined, the various environment variables refer
 # to this specific profile generation.
 \n" port)
-      (let ((variables (evaluate-search-paths (cons $PATH search-paths)
+      (let ((variables (evaluate-search-paths search-paths
                                               (list output))))
         (for-each (write-environment-variable-definition port)
                   (map (abstract-profile output) variables))))))
diff --git a/guix/profiles.scm b/guix/profiles.scm
index e6b77e8d3..88228f155 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -91,6 +91,7 @@
             manifest-lookup
             manifest-installed?
             manifest-matching-entries
+            manifest-search-paths
 
             manifest-transaction
             manifest-transaction?
@@ -545,6 +546,14 @@ no match.."
 
   (filter matches? (manifest-entries manifest)))
 
+(define (manifest-search-paths manifest)
+  "Return the list of search path specifications that apply to MANIFEST,
+including the search path specification for $PATH."
+  (delete-duplicates
+   (cons $PATH
+         (append-map manifest-entry-search-paths
+                     (manifest-entries manifest)))))
+
 \f
 ;;;
 ;;; Manifest transactions.
@@ -1367,8 +1376,7 @@ are cross-built for TARGET."
               (map sexp->search-path-specification
                    (delete-duplicates
                     '#$(map search-path-specification->sexp
-                            (append-map manifest-entry-search-paths
-                                        (manifest-entries manifest))))))
+                            (manifest-search-paths manifest)))))
 
             (build-profile #$output '#$inputs
                            #:symlink #$(if relative-symlinks?
-- 
2.18.0

[bug#32104] [PATCH 2/3] environment: Simplify code by using manifests internally.

[Ludovic Courtès]

* guix/scripts/environment.scm (strip-input-name)
(package+propagated-inputs, package-or-package+output?)
(compact): Remove.
(inputs->profile-derivation): Rename to...
(manifest->derivation): ... this.  Replace 'inputs' parameter with
'manifest'.
(input->manifest-entry): New procedure.
(package-environment-inputs): Rewrite to return a list of manifest
entries.
(options/resolve-packages): Rewrite to return a manifest.
(guix-environment): Remove 'inputs'.  Define 'paths' in terms of
'manifest-search-paths'.
---
 guix/scripts/environment.scm | 156 ++++++++++++++---------------------
 1 file changed, 61 insertions(+), 95 deletions(-)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index f8a9702b3..9a69e3b26 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2018 David Thompson <davet@gnu.org>
-;;; Copyright © 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015, 2016, 2017, 2018 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mike Gerwitz <mtg@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -102,35 +102,23 @@ existing environment variables with additional search paths."
                (newline)))
             (evaluate-profile-search-paths profile search-paths)))
 
-(define (strip-input-name input)
-  "Remove the name element from the tuple INPUT."
+(define (input->manifest-entry input)
+  "Return a manifest entry for INPUT, or #f if INPUT does not correspond to a
+package."
   (match input
-    ((_ package) package)
-    ((_ package output)
-     (list package output))))
-
-(define (package+propagated-inputs package output)
-  "Return the union of PACKAGE's OUTPUT and its transitive propagated inputs."
-  (cons (list package output)
-        (map strip-input-name
-             (package-transitive-propagated-inputs package))))
-
-(define (package-or-package+output? expr)
-  "Return #t if EXPR is a package or a 2 element list consisting of a package
-and an output string."
-  (match expr
-    ((or (? package?) ; bare package object
-         ((? package?) (? string?))) ; package+output tuple
-     #t)
-    (_ #f)))
+    ((_ (? package? package))
+     (package->manifest-entry package))
+    ((_ (? package? package) output)
+     (package->manifest-entry package output))
+    (_
+     #f)))
 
 (define (package-environment-inputs package)
-  "Return a list of the transitive input packages for PACKAGE."
+  "Return a list of manifest entries corresponding to the transitive input
+packages for PACKAGE."
   ;; Remove non-package inputs such as origin records.
-  (filter package-or-package+output?
-          (map strip-input-name
-               (bag-transitive-inputs
-                (package->bag package)))))
+  (filter-map input->manifest-entry
+              (bag-transitive-inputs (package->bag package))))
 
 (define (show-help)
   (display (G_ "Usage: guix environment [OPTION]... PACKAGE... [-- COMMAND...]
@@ -287,55 +275,50 @@ COMMAND or an interactive shell in that environment.\n"))
             (_ memo)))
         '() alist))
 
-(define (compact lst)
-  "Remove all #f elements from LST."
-  (filter identity lst))
-
 (define (options/resolve-packages opts)
-  "Return OPTS with package specification strings replaced by actual
-packages."
-  (define (package->output package mode)
-    (match package
-      ((? package?)
-       (list mode package "out"))
-      (((? package? package) (? string? output))
-       (list mode package output))))
+  "Return OPTS with package specification strings replaced by manifest entries
+for the corresponding packages."
+  (define (manifest-entry=? e1 e2)
+    (and (eq? (manifest-entry-item e1) (manifest-entry-item e2))
+         (string=? (manifest-entry-output e1)
+                   (manifest-entry-output e2))))
 
   (define (packages->outputs packages mode)
     (match packages
-      ((? package-or-package+output? package) ; single package
-       (list (package->output package mode)))
-      (((? package-or-package+output?) ...) ; many packages
-       (map (cut package->output <> mode) packages))))
+      ((? package? package)
+       (if (eq? mode 'ad-hoc-package)
+           (list (package->manifest-entry package))
+           (package-environment-inputs package)))
+      (((? package? package) (? string? output))
+       (if (eq? mode 'ad-hoc-package)
+           (list (package->manifest-entry package output))
+           (package-environment-inputs package)))
+      ((lst ...)
+       (append-map (cut packages->outputs <> mode) lst))))
 
-  (define (manifest->outputs manifest)
-    (map (lambda (entry)
-           (cons 'ad-hoc-package ; manifests are implicitly ad-hoc
-                 (if (package? (manifest-entry-item entry))
-                     (list (manifest-entry-item entry)
-                           (manifest-entry-output entry))
-                     ;; Direct store paths have no output.
-                     (list (manifest-entry-item entry)))))
-         (manifest-entries manifest)))
-
-  (compact
-   (append-map (match-lambda
-                 (('package mode (? string? spec))
-                  (let-values (((package output)
-                                (specification->package+output spec)))
-                    (list (list mode package output))))
-                 (('expression mode str)
-                  ;; Add all the outputs of the package STR evaluates to.
-                  (packages->outputs (read/eval str) mode))
-                 (('load mode file)
-                  ;; Add all the outputs of the package defined in FILE.
-                  (let ((module (make-user-module '())))
-                    (packages->outputs (load* file module) mode)))
-                 (('manifest . file)
-                  (let ((module (make-user-module '((guix profiles) (gnu)))))
-                    (manifest->outputs (load* file module))))
-                 (_ '(#f)))
-               opts)))
+  (manifest
+   (delete-duplicates
+    (append-map (match-lambda
+                  (('package 'ad-hoc-package (? string? spec))
+                   (let-values (((package output)
+                                 (specification->package+output spec)))
+                     (list (package->manifest-entry package output))))
+                  (('package 'package (? string? spec))
+                   (package-environment-inputs
+                    (specification->package+output spec)))
+                  (('expression mode str)
+                   ;; Add all the outputs of the package STR evaluates to.
+                   (packages->outputs (read/eval str) mode))
+                  (('load mode file)
+                   ;; Add all the outputs of the package defined in FILE.
+                   (let ((module (make-user-module '())))
+                     (packages->outputs (load* file module) mode)))
+                  (('manifest . file)
+                   (let ((module (make-user-module '((guix profiles) (gnu)))))
+                     (manifest-entries (load* file module))))
+                  (_ '()))
+                opts)
+    manifest-entry=?)))
 
 (define* (build-environment derivations opts)
   "Build the DERIVATIONS required by the environment using the build options
@@ -350,11 +333,10 @@ in OPTS."
           (return #f)
           (built-derivations derivations)))))
 
-(define (inputs->profile-derivation inputs system bootstrap?)
-  "Return the derivation for a profile consisting of INPUTS for SYSTEM.
-BOOTSTRAP?  specifies whether to use the bootstrap Guile to build the
-profile."
-  (profile-derivation (packages->manifest inputs)
+(define (manifest->derivation manifest system bootstrap?)
+  "Return the derivation for a profile of MANIFEST.
+BOOTSTRAP? specifies whether to use the bootstrap Guile to build the profile."
+  (profile-derivation manifest
                       #:system system
 
                       ;; Packages can have conflicting inputs, or explicit
@@ -671,25 +653,9 @@ message if any test fails."
                                ;; within the container.
                                '("/bin/sh")
                                (list %default-shell))))
-           (packages   (options/resolve-packages opts))
+           (manifest   (options/resolve-packages opts))
            (mappings   (pick-all opts 'file-system-mapping))
-           (inputs     (delete-duplicates
-                        (append-map (match-lambda
-                                      (('ad-hoc-package package output)
-                                       (package+propagated-inputs package
-                                                                  output))
-                                      (('package package _)
-                                       (package-environment-inputs package)))
-                                    packages)))
-           (paths      (delete-duplicates
-                        (cons $PATH
-                              (append-map (match-lambda
-                                            ((or ((? package? p) _ ...)
-                                                 (? package? p))
-                                             (package-native-search-paths p))
-                                            (_ '()))
-                                          inputs))
-                        eq?)))
+           (paths      (manifest-search-paths manifest)))
 
       (when container? (assert-container-features))
 
@@ -714,8 +680,8 @@ message if any test fails."
             (mlet* %store-monad ((bash       (environment-bash container?
                                                                bootstrap?
                                                                system))
-                                 (prof-drv   (inputs->profile-derivation
-                                              inputs system bootstrap?))
+                                 (prof-drv   (manifest->derivation
+                                              manifest system bootstrap?))
                                  (profile -> (derivation->output-path prof-drv))
                                  (gc-root -> (assoc-ref opts 'gc-root)))
 
-- 
2.18.0

[bug#32104] [PATCH 3/3] profiles: Introduce 'profile-search-paths' and use it.

[Ludovic Courtès]

* guix/profiles.scm (profile-search-paths): New procedure.
* guix/scripts/environment.scm (evaluate-search-paths): Remove.
(create-environment): Replace 'paths' with 'manifest'.  Use
'profile-search-paths' instead of 'evaluate-search-paths'.
(show-search-paths): Likewise.
(launch-environment): Replace 'paths' with 'manifest'.  Make 'pure?' a
keyword parameter.
(launch-environment/fork, launch-environment/container): Likewise.
(guix-environment): Remove 'paths' variable.  Adjust callers of the
above procedures accordingly.
---
 guix/profiles.scm            | 14 ++++++++++
 guix/scripts/environment.scm | 54 +++++++++++++++++-------------------
 2 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 88228f155..d2a794b18 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -110,6 +110,7 @@
             ca-certificate-bundle
             %default-profile-hooks
             profile-derivation
+            profile-search-paths
 
             generation-number
             generation-numbers
@@ -1400,6 +1401,19 @@ are cross-built for TARGET."
                       ;; to have no substitute to offer.
                       #:substitutable? #f)))
 
+(define* (profile-search-paths profile
+                               #:optional (manifest (profile-manifest profile))
+                               #:key (getenv (const #f)))
+  "Read the manifest of PROFILE and evaluate the values of search path
+environment variables required by PROFILE; return a list of
+specification/value pairs.  If MANIFEST is not #f, it is assumed to be the
+manifest of PROFILE, which avoids rereading it.
+
+Use GETENV to determine the current settings and report only settings not
+already effective."
+  (evaluate-search-paths (manifest-search-paths manifest)
+                         (list profile) getenv))
+
 (define (profile-regexp profile)
   "Return a regular expression that matches PROFILE's name and number."
   (make-regexp (string-append "^" (regexp-quote (basename profile))
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 9a69e3b26..1c04800e4 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -49,11 +49,6 @@
   #:use-module (srfi srfi-98)
   #:export (guix-environment))
 
-(define (evaluate-profile-search-paths profile search-paths)
-  "Evaluate SEARCH-PATHS, a list of search-path specifications, for the
-directories in PROFILE, the store path of a profile."
-  (evaluate-search-paths search-paths (list profile)))
-
 ;; Protect some env vars from purification.  Borrowed from nix-shell.
 (define %precious-variables
   '("HOME" "USER" "LOGNAME" "DISPLAY" "TERM" "TZ" "PAGER"))
@@ -70,8 +65,8 @@ as 'HOME' and 'USER' are left untouched."
                       (((names . _) ...)
                        names)))))
 
-(define (create-environment profile paths pure?)
-  "Set the environment variables specified by PATHS for PROFILE.  When PURE?
+(define* (create-environment profile manifest #:key pure?)
+  "Set the environment variables specified by MANIFEST for PROFILE.  When PURE?
 is #t, unset the variables in the current environment.  Otherwise, augment
 existing environment variables with additional search paths."
   (when pure? (purify-environment))
@@ -84,23 +79,23 @@ existing environment variables with additional search paths."
                                  (string-append value separator current)
                                  value)
                              value)))))
-            (evaluate-profile-search-paths profile paths))
+            (profile-search-paths profile manifest))
 
   ;; Give users a way to know that they're in 'guix environment', so they can
   ;; adjust 'PS1' accordingly, for instance.  Set it to PROFILE so users can
   ;; conveniently access its contents.
   (setenv "GUIX_ENVIRONMENT" profile))
 
-(define (show-search-paths profile search-paths pure?)
-  "Display SEARCH-PATHS applied to PROFILE.  When PURE? is #t, do not augment
-existing environment variables with additional search paths."
+(define* (show-search-paths profile manifest #:key pure?)
+  "Display the search paths of MANIFEST applied to PROFILE.  When PURE? is #t,
+do not augment existing environment variables with additional search paths."
   (for-each (match-lambda
               ((search-path . value)
                (display
                 (search-path-definition search-path value
                                         #:kind (if pure? 'exact 'prefix)))
                (newline)))
-            (evaluate-profile-search-paths profile search-paths)))
+            (profile-search-paths profile manifest)))
 
 (define (input->manifest-entry input)
   "Return a manifest entry for INPUT, or #f if INPUT does not correspond to a
@@ -379,32 +374,34 @@ and suitable for 'exit'."
 (define exit/status (compose exit status->exit-code))
 (define primitive-exit/status (compose primitive-exit status->exit-code))
 
-(define (launch-environment command inputs paths pure?)
+(define* (launch-environment command profile manifest
+                             #:key pure?)
   "Run COMMAND in a new environment containing INPUTS, using the native search
 paths defined by the list PATHS.  When PURE?, pre-existing environment
 variables are cleared before setting the new ones."
   ;; Properly handle SIGINT, so pressing C-c in an interactive terminal
   ;; application works.
   (sigaction SIGINT SIG_DFL)
-  (create-environment inputs paths pure?)
+  (create-environment profile manifest #:pure? pure?)
   (match command
     ((program . args)
      (apply execlp program program args))))
 
-(define (launch-environment/fork command inputs paths pure?)
-  "Run COMMAND in a new process with an environment containing INPUTS, using
-the native search paths defined by the list PATHS.  When PURE?, pre-existing
-environment variables are cleared before setting the new ones."
+(define* (launch-environment/fork command profile manifest #:key pure?)
+  "Run COMMAND in a new process with an environment containing PROFILE, with
+the search paths specified by MANIFEST.  When PURE?, pre-existing environment
+variables are cleared before setting the new ones."
   (match (primitive-fork)
-    (0 (launch-environment command inputs paths pure?))
+    (0 (launch-environment command profile manifest
+                           #:pure? pure?))
     (pid (match (waitpid pid)
            ((_ . status) status)))))
 
 (define* (launch-environment/container #:key command bash user user-mappings
-                                       profile paths link-profile? network?)
+                                       profile manifest link-profile? network?)
   "Run COMMAND within a container that features the software in PROFILE.
-Environment variables are set according to PATHS, a list of native search
-paths.  The global shell is BASH, a file name for a GNU Bash binary in the
+Environment variables are set according to the search paths of MANIFEST.
+The global shell is BASH, a file name for a GNU Bash binary in the
 store.  When NETWORK?, access to the host system network is permitted.
 USER-MAPPINGS, a list of file system mappings, contains the user-specified
 host file systems to mount inside the container.  If USER is not #f, each
@@ -496,7 +493,7 @@ will be used for the passwd entry.  LINK-PROFILE? creates a symbolic link from
             (primitive-exit/status
              ;; A container's environment is already purified, so no need to
              ;; request it be purified again.
-             (launch-environment command profile paths #f)))
+             (launch-environment command profile manifest #:pure? #f)))
           #:namespaces (if network?
                            (delq 'net %namespaces) ; share host network
                            %namespaces)))))))
@@ -654,8 +651,7 @@ message if any test fails."
                                '("/bin/sh")
                                (list %default-shell))))
            (manifest   (options/resolve-packages opts))
-           (mappings   (pick-all opts 'file-system-mapping))
-           (paths      (manifest-search-paths manifest)))
+           (mappings   (pick-all opts 'file-system-mapping)))
 
       (when container? (assert-container-features))
 
@@ -700,7 +696,7 @@ message if any test fails."
                  ((assoc-ref opts 'dry-run?)
                   (return #t))
                  ((assoc-ref opts 'search-paths)
-                  (show-search-paths profile paths pure?)
+                  (show-search-paths profile manifest #:pure? pure?)
                   (return #t))
                  (container?
                   (let ((bash-binary
@@ -713,11 +709,11 @@ message if any test fails."
                                                   #:user user
                                                   #:user-mappings mappings
                                                   #:profile profile
-                                                  #:paths paths
+                                                  #:manifest manifest
                                                   #:link-profile? link-prof?
                                                   #:network? network?)))
                  (else
                   (return
                    (exit/status
-                    (launch-environment/fork command profile
-                                             paths pure?)))))))))))))
+                    (launch-environment/fork command profile manifest
+                                             #:pure? pure?)))))))))))))
-- 
2.18.0

[bug#32104] [PATCH 0/3] guix environment: Simplify environment creation

[Thompson, David]

Hey Ludo,

On Mon, Jul 9, 2018 at 7:31 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> Hello Guix!
>
> These patches aim to simplify (guix scripts environment) by (1) factorizing
> part of the search path handling machinery in (guix profiles), and (2) by
> using manifests internally instead of loosely defined lists of “inputs.”
>
> The goal is to gradually move towards a situation where (guix scripts
> environment) contains nothing but UI code and related plumbing.

This is a great initiative. Using manifests for everything makes a lot
of sense. All the patches look good to me!

- Dave

bug#32104: [PATCH 0/3] guix environment: Simplify environment creation

[Ludovic Courtès]

Hey David,

"Thompson, David" <dthompson2@worcester.edu> skribis:

> On Mon, Jul 9, 2018 at 7:31 AM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Hello Guix!
>>
>> These patches aim to simplify (guix scripts environment) by (1) factorizing
>> part of the search path handling machinery in (guix profiles), and (2) by
>> using manifests internally instead of loosely defined lists of “inputs.”
>>
>> The goal is to gradually move towards a situation where (guix scripts
>> environment) contains nothing but UI code and related plumbing.
>
> This is a great initiative. Using manifests for everything makes a lot
> of sense. All the patches look good to me!

Pushed now, thanks for reviewing!

Ludo’.