[bug#43614] [PATCH 0/2] Record and replay package transformation options

[bug#43614] [PATCH 0/2] Record and replay package transformation options

[Ludovic Courtès]

Hello Guix!

This patch set is to record in a profile’s ‘manifest’ the package
transformation options that were in effect when it was created and
to replay them when upgrading.

Let’s say you do:

  guix install emacs-next --with-branch=emacs-next=master

When you later run:

  guix upgrade

the ‘--with-branch’ option will be in effect, meaning that it’ll
again pull the latest Emacs and build it.

It’s a contrived example because it relies on external resources;
preserving ‘--with-input’ & co. probably makes more sense.

The kind of options I have in mind that are particularly important
to preserve are options that parameterize packages and the
hypothetical option that wraps an installed package into a “POLA
wrapper” as was discussed recently (the wrapper spawns the actual
program in a container).

Users of ‘guix package -m’ are unaffected by all this.  After all,
they can already program whatever transformations they want in
their manifest (though it would be nice to make those
transformations more readily usable at the Scheme level!).

Thoughts?

Ludo’.

Ludovic Courtès (2):
  guix build: Record package transformations in manifest entries.
  guix package: Re-apply package transformation when upgrading.

 doc/guix.texi                 | 27 ++++++++++++
 guix/scripts/build.scm        | 80 +++++++++++++++++++++++++++--------
 guix/scripts/pack.scm         | 29 +++++++------
 guix/scripts/package.scm      | 33 ++++++++++-----
 tests/guix-package-aliases.sh |  6 +++
 tests/guix-package.sh         | 17 +++++++-
 tests/packages.scm            | 23 ++++++++++
 7 files changed, 174 insertions(+), 41 deletions(-)

-- 
2.28.0

[bug#43614] [PATCH 1/2] guix build: Record package transformations in manifest entries.

[Ludovic Courtès]

With this change, package transformation options used while building a
manifest are saved in the metadata of the manifest entries.

* guix/scripts/build.scm (transformation-procedure): New procedure.
(options->transformation)[applicable]: Use it.  Change to a list of
key/value/proc tuples instead of key/proc pairs.
[package-with-transformation-properties, tagged-object]: New
procedures.  Use them.
(package-transformations, manifest-entry-with-transformations): New
procedures.
* guix/scripts/pack.scm (guix-pack)[with-transformations]: New
procedure.
Use it.
* guix/scripts/package.scm (process-actions)[transform-entry]: Use it.
* tests/guix-package-aliases.sh: Add test.
---
 guix/scripts/build.scm        | 80 +++++++++++++++++++++++++++--------
 guix/scripts/pack.scm         | 29 +++++++------
 guix/scripts/package.scm      | 13 +++---
 tests/guix-package-aliases.sh |  6 +++
 4 files changed, 93 insertions(+), 35 deletions(-)

diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index 38e0516c95..c8529856f7 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -61,6 +61,7 @@
 
             %transformation-options
             options->transformation
+            manifest-entry-with-transformations
             show-transformation-options-help
 
             guix-build
@@ -405,6 +406,14 @@ a checkout of the Git repository at the given URL."
     (with-commit . ,transform-package-source-commit)
     (with-git-url . ,transform-package-source-git-url)))
 
+(define (transformation-procedure key)
+  "Return the transformation procedure associated with KEY, a symbol such as
+'with-source', or #f if there is none."
+  (any (match-lambda
+         ((k . proc)
+          (and (eq? k key) proc)))
+       %transformations))
+
 (define %transformation-options
   ;; The command-line interface to the above transformations.
   (let ((parser (lambda (symbol)
@@ -454,32 +463,69 @@ derivation, etc.), applies the transformations specified by OPTS."
     ;; order in which they appear on the command line.
     (filter-map (match-lambda
                   ((key . value)
-                   (match (any (match-lambda
-                                 ((k . proc)
-                                  (and (eq? k key) proc)))
-                               %transformations)
+                   (match (transformation-procedure key)
                      (#f
                       #f)
                      (transform
                       ;; XXX: We used to pass TRANSFORM a list of several
                       ;; arguments, but we now pass only one, assuming that
                       ;; transform composes well.
-                      (cons key (transform (list value)))))))
+                      (list key value (transform (list value)))))))
                 (reverse opts)))
 
+  (define (package-with-transformation-properties p)
+    (package/inherit p
+      (properties `((transformations
+                     . ,(map (match-lambda
+                               ((key value _)
+                                (cons key value)))
+                             applicable))
+                    ,@(package-properties p)))))
+
   (lambda (store obj)
-    (fold (match-lambda*
-            (((name . transform) obj)
-             (let ((new (transform store obj)))
-               (when (eq? new obj)
-                 (warning (G_ "transformation '~a' had no effect on ~a~%")
-                          name
-                          (if (package? obj)
-                              (package-full-name obj)
-                              obj)))
-               new)))
-          obj
-          applicable)))
+    (define (tagged-object new)
+      (if (and (not (eq? obj new))
+               (package? new) (not (null? applicable)))
+          (package-with-transformation-properties new)
+          new))
+
+    (tagged-object
+     (fold (match-lambda*
+             (((name value transform) obj)
+              (let ((new (transform store obj)))
+                (when (eq? new obj)
+                  (warning (G_ "transformation '~a' had no effect on ~a~%")
+                           name
+                           (if (package? obj)
+                               (package-full-name obj)
+                               obj)))
+                new)))
+           obj
+           applicable))))
+
+(define (package-transformations package)
+  "Return the transformations applied to PACKAGE according to its properties."
+  (match (assq-ref (package-properties package) 'transformations)
+    (#f '())
+    (transformations transformations)))
+
+(define (manifest-entry-with-transformations entry)
+  "Return ENTRY with an additional 'transformations' property if it's not
+already there."
+  (let ((properties (manifest-entry-properties entry)))
+    (if (assq 'transformations properties)
+        entry
+        (let ((item (manifest-entry-item entry)))
+          (manifest-entry
+            (inherit entry)
+            (properties
+             (match (and (package? item)
+                         (package-transformations item))
+               ((or #f '())
+                properties)
+               (transformations
+                `((transformations . ,transformations)
+                  ,@properties)))))))))
 
 \f
 ;;;
diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
index 379e6a3ac6..39bbb55eaf 100644
--- a/guix/scripts/pack.scm
+++ b/guix/scripts/pack.scm
@@ -1134,19 +1134,24 @@ Create a bundle of PACKAGE.\n"))
                manifest))
             identity))
 
+      (define (with-transformations manifest)
+        (map-manifest-entries manifest-entry-with-transformations
+                              manifest))
+
       (with-provenance
-       (cond
-        ((and (not (null? manifests)) (not (null? packages)))
-         (leave (G_ "both a manifest and a package list were given~%")))
-        ((not (null? manifests))
-         (concatenate-manifests
-          (map (lambda (file)
-                 (let ((user-module (make-user-module
-                                     '((guix profiles) (gnu)))))
-                   (load* file user-module)))
-               manifests)))
-        (else
-         (packages->manifest packages))))))
+       (with-transformations
+        (cond
+         ((and (not (null? manifests)) (not (null? packages)))
+          (leave (G_ "both a manifest and a package list were given~%")))
+         ((not (null? manifests))
+          (concatenate-manifests
+           (map (lambda (file)
+                  (let ((user-module (make-user-module
+                                      '((guix profiles) (gnu)))))
+                    (load* file user-module)))
+                manifests)))
+         (else
+          (packages->manifest packages)))))))
 
   (with-error-handling
     (with-store store
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 7e7c37eac4..83f8c123d9 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -864,12 +864,13 @@ processed, #f otherwise."
 
   (define (transform-entry entry)
     (let ((item (transform store (manifest-entry-item entry))))
-      (manifest-entry
-        (inherit entry)
-        (item item)
-        (version (if (package? item)
-                     (package-version item)
-                     (manifest-entry-version entry))))))
+      (manifest-entry-with-transformations
+       (manifest-entry
+         (inherit entry)
+         (item item)
+         (version (if (package? item)
+                      (package-version item)
+                      (manifest-entry-version entry)))))))
 
   (when (equal? profile %current-profile)
     ;; Normally the daemon created %CURRENT-PROFILE when we connected, unless
diff --git a/tests/guix-package-aliases.sh b/tests/guix-package-aliases.sh
index e24bff3a56..5ad65830d7 100644
--- a/tests/guix-package-aliases.sh
+++ b/tests/guix-package-aliases.sh
@@ -40,6 +40,12 @@ if guix install -r guile-bootstrap -p "$profile" --bootstrap
 then false; else true; fi
 test -x "$profile/bin/guile"
 
+# Use a package transformation option and make sure it's recorded.
+guix install --bootstrap guile-bootstrap -p "$profile" \
+     --with-input=libreoffice=inkscape
+test -x "$profile/bin/guile"
+grep "libreoffice=inkscape" "$profile/manifest"
+
 guix upgrade --version
 guix upgrade -n
 guix upgrade gui.e -n
-- 
2.28.0

[bug#43614] [PATCH 2/2] guix package: Re-apply package transformation when upgrading.

[Ludovic Courtès]

* guix/scripts/package.scm (transaction-upgrade-entry)[upgrade]: Add
'transform' parameter.  Pass PKG through it.  Use
'manifest-entry-with-transformations'.
Call 'options->transformation' to get the transformation procedure.
* tests/guix-package.sh: Add 'guix package -u' test.
* tests/packages.scm ("transaction-upgrade-entry, transformation options preserved"):
New test.
* doc/guix.texi (Invoking guix package): Mention that transformations
are preserved across upgrades.
(Package Transformation Options): Likewise.
---
 doc/guix.texi            | 27 +++++++++++++++++++++++++++
 guix/scripts/package.scm | 20 +++++++++++++++-----
 tests/guix-package.sh    | 17 ++++++++++++++++-
 tests/packages.scm       | 23 +++++++++++++++++++++++
 4 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 6b2c749bc7..81dbe95e9f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -3082,6 +3082,29 @@ in the distribution currently installed.  To update your distribution,
 you should regularly run @command{guix pull} (@pxref{Invoking guix
 pull}).
 
+@cindex package transformations, upgrades
+When upgrading, package transformations that were originally applied
+when creating the profile are automatically re-applied (@pxref{Package
+Transformation Options}).  For example, assume you first installed Emacs
+from the tip of its development branch with:
+
+@example
+guix install emacs-next --with-branch=emacs-next=master
+@end example
+
+Next time you run @command{guix upgrade}, Guix will again pull the tip
+of the Emacs development branch and build @code{emacs-next} from that
+checkout.
+
+Note that transformation options such as @option{--with-branch} and
+@option{--with-source} depend on external state; it is up to you to
+ensure that they work as expected.  You can also discard a
+transformations that apply to a package by running:
+
+@example
+guix install @var{package}
+@end example
+
 @item --do-not-upgrade[=@var{regexp} @dots{}]
 When used together with the @option{--upgrade} option, do @emph{not}
 upgrade any packages whose name matches a @var{regexp}.  For example, to
@@ -9135,6 +9158,10 @@ This is a convenient way to create customized packages on the fly
 without having to type in the definitions of package variants
 (@pxref{Defining Packages}).
 
+Package transformation options are preserved across upgrades:
+@command{guix upgrade} attempts to apply transformation options
+initially used when creating the profile to the upgraded packages.
+
 @table @code
 
 @item --with-source=@var{source}
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 83f8c123d9..2f04652634 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -218,12 +218,13 @@ non-zero relevance score."
         (output (manifest-entry-output old)))
       transaction)))
 
-  (define (upgrade entry)
+  (define (upgrade entry transform)
     (match entry
       (($ <manifest-entry> name version output (? string? path))
        (match (find-best-packages-by-name name #f)
          ((pkg . rest)
-          (let ((candidate-version (package-version pkg)))
+          (let* ((pkg               (transform store pkg))
+                 (candidate-version (package-version pkg)))
             (match (package-superseded pkg)
               ((? package? new)
                (supersede entry new))
@@ -231,12 +232,14 @@ non-zero relevance score."
                (case (version-compare candidate-version version)
                  ((>)
                   (manifest-transaction-install-entry
-                   (package->manifest-entry* pkg output)
+                   (manifest-entry-with-transformations
+                    (package->manifest-entry* pkg output))
                    transaction))
                  ((<)
                   transaction)
                  ((=)
-                  (let* ((new (package->manifest-entry* pkg output)))
+                  (let* ((new (manifest-entry-with-transformations
+                               (package->manifest-entry* pkg output))))
                     ;; Here we want to determine whether the NEW actually
                     ;; differs from ENTRY, but we need to intercept
                     ;; 'build-things' calls because they would prevent us from
@@ -255,7 +258,14 @@ non-zero relevance score."
 
   (if (manifest-transaction-removal-candidate? entry transaction)
       transaction
-      (upgrade entry)))
+
+      ;; Upgrade ENTRY, preserving transformation options listed in its
+      ;; properties.
+      (let ((transform (options->transformation
+                        (or (assq-ref (manifest-entry-properties entry)
+                                      'transformations)
+                            '()))))
+        (upgrade entry transform))))
 
 \f
 ;;;
diff --git a/tests/guix-package.sh b/tests/guix-package.sh
index 1f955257be..c7ef07c89a 100644
--- a/tests/guix-package.sh
+++ b/tests/guix-package.sh
@@ -1,5 +1,5 @@
 # GNU Guix --- Functional package management for GNU
-# Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo@gnu.org>
+# Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 # Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
 #
 # This file is part of GNU Guix.
@@ -193,6 +193,21 @@ grep -E 'emacs[[:blank:]]+42\.5\.9rc7' "$tmpfile"
 rm "$emacs_tarball" "$tmpfile"
 rmdir "$module_dir"
 
+# Install with package transformations.
+guix install --bootstrap -p "$profile" sed --with-input=sed=guile-bootstrap
+grep "sed=guile-bootstrap" "$profile/manifest"
+test "$(readlink -f "$profile/bin/guile")" \
+     = "$(guix build guile-bootstrap)/bin/guile"
+test ! -f "$profile/bin/sed"
+
+# Make sure the package transformation is preserved.
+guix package --bootstrap -p "$profile" -u
+grep "sed=guile-bootstrap" "$profile/manifest"
+test "$(readlink -f "$profile/bin/guile")" \
+     = "$(guix build guile-bootstrap)/bin/guile"
+test ! -f "$profile/bin/sed"
+rm "$profile" "$profile"-[0-9]-link
+
 # Profiles with a relative file name.  Make sure we don't create dangling
 # symlinks--see bug report at
 # <https://lists.gnu.org/archive/html/guix-devel/2018-07/msg00036.html>.
diff --git a/tests/packages.scm b/tests/packages.scm
index cbd0503733..b1885f70d2 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -185,6 +185,29 @@
                  (string=? (manifest-pattern-version pattern) "1")
                  (string=? (manifest-pattern-output pattern) "out")))))))
 
+(test-equal "transaction-upgrade-entry, transformation options preserved"
+  (derivation-file-name (package-derivation %store grep))
+
+  (let* ((old   (dummy-package "sed" (version "1")))
+         (props '((transformations . ((with-input . "sed=grep")))))
+         (tx    (transaction-upgrade-entry
+                 %store
+                 (manifest-entry
+                   (inherit (package->manifest-entry old))
+                   (properties props)
+                   (item (string-append (%store-prefix) "/"
+                                        (make-string 32 #\e) "-foo-1")))
+                 (manifest-transaction))))
+    (match (manifest-transaction-install tx)
+      (((? manifest-entry? entry))
+       (and (string=? (manifest-entry-version entry)
+                      (package-version grep))
+            (string=? (manifest-entry-name entry)
+                      (package-name grep))
+            (equal? (manifest-entry-properties entry) props)
+            (derivation-file-name
+             (package-derivation %store (manifest-entry-item entry))))))))
+
 (test-assert "transaction-upgrade-entry, grafts"
   ;; Ensure that, when grafts are enabled, 'transaction-upgrade-entry' doesn't
   ;; try to build stuff.
-- 
2.28.0

bug#43614: [PATCH 0/2] Record and replay package transformation options

[Ludovic Courtès]

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

>   guix build: Record package transformations in manifest entries.
>   guix package: Re-apply package transformation when upgrading.

Pushed as 8e1907a72430aa989125b053573ef0897c480697 along with a news
entry.

Ludo’.