[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[Ludovic Courtès]

Hello Guix!

This patch set aims to hit two bird hunters with one stone:
initially my goal was to fix <https://issues.guix.gnu.org/42156>,
which has become a hindrance to the use of package transformation
options, but it also ends up addressing the fact that those
options did not, until now, rewrite implicit dependencies.

Concretely, the following commands had no effect thus far:

  guix build python-itsdangerous --with-input=python=python2
  guix build hello --with-input=gcc=gcc-toolchain@10

In both cases, this is because the input we want to change is
an implicit input.  This patch set fixes that, and it fixes
<https://issues.guix.gnu.org/42156> as a side effect.

This opens new possibilities.  ‘--with-input=python=python2’ is one
of them, but ‘--with-input=gcc=gcc-toolchain@10’ is not (it fails
to build for obscure reasons that I’ll fix in ‘core-updates’, and
it rebuilds the world anyway, which is not practical).  Another
thing that I find interesting is:

  $ guix build hello --with-graft=glibc=glibc@2.29
  /gnu/store/6jv7icpjbl3pvj24db2q2fmdly9vgp4d-hello-2.10
  $ /gnu/store/6jv7icpjbl3pvj24db2q2fmdly9vgp4d-hello-2.10/bin/hello 
  Saluton, mondo!
  $ LD_TRACE_LOADED_OBJECTS=yes /gnu/store/6jv7icpjbl3pvj24db2q2fmdly9vgp4d-hello-2.10/bin/hello
          linux-vdso.so.1 (0x00007ffcc87f8000)
          libgcc_s.so.1 => /gnu/store/1sqr5fa6jbksqmi7hibqaffixy3b1j0y-gcc-7.5.0-lib/lib/libgcc_s.so.1 (0x00007f3f36c3c000)
          libc.so.6 => /gnu/store/i4iqkjx34r3nmwwblfmkbsbsd3pgapfp-glibc-2.29/lib/libc.so.6 (0x00007f3f36a82000)
          /gnu/store/i4iqkjx34r3nmwwblfmkbsbsd3pgapfp-glibc-2.29/lib/ld-linux-x86-64.so.2 (0x00007f3f36c57000)

It “relinks” the package against a different libc, assuming the
ABI is compatible (this particular example downgrades glibc, probably
less useful in the real world than upgrading; it works for ‘hello’
but not for ‘inkscape’ due to ABI differences.)

Feedback welcome!

Ludo’.

Ludovic Courtès (4):
  packages: 'package-mapping' can recurse on implicit inputs.
  packages: 'package-input-rewriting/spec' can rewrite implicit
    dependencies.
  packages: 'package-mapping' correctly recurses into 'replacement'.
  packages: 'package-input-rewriting' has a #:deep? parameter.

 doc/guix.texi           |  28 ++++----
 gnu/packages/guile.scm  |   6 +-
 guix/packages.scm       | 153 ++++++++++++++++++++++++++++------------
 tests/guix-build.sh     |  11 +++
 tests/packages.scm      | 149 ++++++++++++++++++++++++++++++++++++--
 tests/scripts-build.scm |  12 +++-
 6 files changed, 291 insertions(+), 68 deletions(-)

-- 
2.28.0

[bug#43578] [PATCH 1/4] packages: 'package-mapping' can recurse on implicit inputs.

[Ludovic Courtès]

* guix/packages.scm (build-system-with-package-mapping): New procedure.
(package-mapping): Add #:deep? and honor it.
* tests/packages.scm ("package-mapping"): Compare the direct inputs of
the bag of P0 and that of P1.
("package-mapping, deep"): New test.
---
 doc/guix.texi      |  5 ++--
 guix/packages.scm  | 65 +++++++++++++++++++++++++++++++++++-----------
 tests/packages.scm | 36 ++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f7e2204b53..4595008c4f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -6294,10 +6294,11 @@ A more generic procedure to rewrite a package dependency graph is
 @code{package-mapping}: it supports arbitrary changes to nodes in the
 graph.
 
-@deffn {Scheme Procedure} package-mapping @var{proc} [@var{cut?}]
+@deffn {Scheme Procedure} package-mapping @var{proc} [@var{cut?}] [#:deep? #f]
 Return a procedure that, given a package, applies @var{proc} to all the packages
 depended on and returns the resulting package.  The procedure stops recursion
-when @var{cut?} returns true for a given package.
+when @var{cut?} returns true for a given package.  When @var{deep?} is true, @var{proc} is
+applied to implicit inputs as well.
 @end deffn
 
 @menu
diff --git a/guix/packages.scm b/guix/packages.scm
index 6598bd3149..171fd048ef 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -968,10 +968,31 @@ packages they depend on, recursively."
                    (vhash-consq package #t visited)
                    (fold set-insert closure dependencies))))))))
 
-(define* (package-mapping proc #:optional (cut? (const #f)))
+(define (build-system-with-package-mapping bs rewrite)
+  "Return a variant of BS, a build system, that rewrites a bag's inputs by
+passing them through REWRITE, a procedure that takes an input tuplet and
+returns a \"rewritten\" input tuplet."
+  (define lower
+    (build-system-lower bs))
+
+  (define (lower* . args)
+    (let ((lowered (apply lower args)))
+      (bag
+        (inherit lowered)
+        (build-inputs (map rewrite (bag-build-inputs lowered)))
+        (host-inputs (map rewrite (bag-host-inputs lowered)))
+        (target-inputs (map rewrite (bag-target-inputs lowered))))))
+
+  (build-system
+    (inherit bs)
+    (lower lower*)))
+
+(define* (package-mapping proc #:optional (cut? (const #f))
+                          #:key deep?)
   "Return a procedure that, given a package, applies PROC to all the packages
 depended on and returns the resulting package.  The procedure stops recursion
-when CUT? returns true for a given package."
+when CUT? returns true for a given package.  When DEEP? is true, PROC is
+applied to implicit inputs as well."
   (define (rewrite input)
     (match input
       ((label (? package? package) outputs ...)
@@ -980,21 +1001,35 @@ when CUT? returns true for a given package."
       (_
        input)))
 
+  (define mapping-property
+    ;; Property indicating whether the package has already been processed.
+    (gensym " package-mapping-done"))
+
   (define replace
     (mlambdaq (p)
-      ;; Return a variant of P with PROC applied to P and its explicit
-      ;; dependencies, recursively.  Memoize the transformations.  Failing to
-      ;; do that, we would build a huge object graph with lots of duplicates,
-      ;; which in turns prevents us from benefiting from memoization in
-      ;; 'package-derivation'.
-      (let ((p (proc p)))
-        (package
-          (inherit p)
-          (location (package-location p))
-          (inputs (map rewrite (package-inputs p)))
-          (native-inputs (map rewrite (package-native-inputs p)))
-          (propagated-inputs (map rewrite (package-propagated-inputs p)))
-          (replacement (and=> (package-replacement p) proc))))))
+      ;; If P is the result of a previous call, return it.
+      (if (assq-ref (package-properties p) mapping-property)
+          p
+
+          ;; Return a variant of P with PROC applied to P and its explicit
+          ;; dependencies, recursively.  Memoize the transformations.  Failing
+          ;; to do that, we would build a huge object graph with lots of
+          ;; duplicates, which in turns prevents us from benefiting from
+          ;; memoization in 'package-derivation'.
+          (let ((p (proc p)))
+            (package
+              (inherit p)
+              (location (package-location p))
+              (build-system (if deep?
+                                (build-system-with-package-mapping
+                                 (package-build-system p) rewrite)
+                                (package-build-system p)))
+              (inputs (map rewrite (package-inputs p)))
+              (native-inputs (map rewrite (package-native-inputs p)))
+              (propagated-inputs (map rewrite (package-propagated-inputs p)))
+              (replacement (and=> (package-replacement p) proc))
+              (properties `((,mapping-property . #t)
+                            ,@(package-properties p))))))))
 
   replace)
 
diff --git a/tests/packages.scm b/tests/packages.scm
index cbd0503733..f33332a461 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -1172,15 +1172,24 @@
   (let* ((dep       (dummy-package "chbouib"
                       (native-inputs `(("x" ,grep)))))
          (p0        (dummy-package "example"
+                      (source 77)
                       (inputs `(("foo" ,coreutils)
                                 ("bar" ,grep)
                                 ("baz" ,dep)))))
          (transform (lambda (p)
                       (package (inherit p) (source 42))))
          (rewrite   (package-mapping transform))
-         (p1        (rewrite p0)))
+         (p1        (rewrite p0))
+         (bag0      (package->bag p0))
+         (bag1      (package->bag p1)))
     (and (eq? p1 (rewrite p0))
          (eqv? 42 (package-source p1))
+
+         ;; Implicit inputs should be left unchanged (skip "source", "foo",
+         ;; "bar", and "baz" in this comparison).
+         (equal? (drop (bag-direct-inputs bag0) 4)
+                 (drop (bag-direct-inputs bag1) 4))
+
          (match (package-inputs p1)
            ((("foo" dep1) ("bar" dep2) ("baz" dep3))
             (and (eq? dep1 (rewrite coreutils))   ;memoization
@@ -1194,6 +1203,31 @@
                     (and (eq? dep (rewrite grep))
                          (package-source dep))))))))))
 
+(test-equal "package-mapping, deep"
+  '(42)
+  (let* ((p0        (dummy-package "example"
+                      (inputs `(("foo" ,coreutils)
+                                ("bar" ,grep)))))
+         (transform (lambda (p)
+                      (package (inherit p) (source 42))))
+         (rewrite   (package-mapping transform #:deep? #t))
+         (p1        (rewrite p0))
+         (bag       (package->bag p1)))
+    (and (eq? p1 (rewrite p0))
+         (match (bag-direct-inputs bag)
+           ((("source" 42) ("foo" dep1) ("bar" dep2) rest ..1)
+            (and (eq? dep1 (rewrite coreutils))   ;memoization
+                 (eq? dep2 (rewrite grep))
+                 (= 42 (package-source dep1))
+                 (= 42 (package-source dep2))
+
+                 ;; Check that implicit inputs of P0 also got rewritten.
+                 (delete-duplicates
+                  (map (match-lambda
+                         ((_ package . _)
+                          (package-source package)))
+                       rest))))))))
+
 (test-assert "package-input-rewriting"
   (let* ((dep     (dummy-package "chbouib"
                     (native-inputs `(("x" ,grep)))))
-- 
2.28.0

[bug#43578] [PATCH 2/4] packages: 'package-input-rewriting/spec' can rewrite implicit dependencies.

[Ludovic Courtès]

With this change, '--with-input', '--with-graft', etc. also apply to
implicit dependencies.  Thus, it's now possible to do:

  guix build python-itsdangerous --with-input=python-wrapper=python@2

or:

  guix build hello --with-graft=glibc=glibc@2.29

Additionally, before, implicit inputs were not rewritten, which could
lead to duplicates in the output of 'bag-transitive-inputs' (packages
that are not 'eq?' but lead to the same derivation).  This in turn would
lead to unnecessary rebuilds when using '--with-input' & co.  This
change fixes it by ensuring even implicit inputs are rewritten.

Fixes <https://bugs.gnu.org/42156>.

* guix/packages.scm (package-input-rewriting/spec): Add #:deep?
defaulting to #true, and pass it to 'package-mapping'.
[replacement-property]: New variable.
[rewrite]: Check that property and set it on the result of PROC.
[cut?]: New procedure.
* tests/packages.scm ("package-input-rewriting/spec"): Ensure implicit
inputs were unchanged.
("package-input-rewriting/spec, partial match"): Pass #:deep? #f.
("package-input-rewriting/spec, deep")
("package-input-rewriting/spec, no duplicates"): New tests.
(package/inherit): Move before use.
* tests/guix-build.sh: Add tests.
* tests/scripts-build.scm ("options->transformation, with-graft"):
Compare dependencies by package name or derivation file name.
* doc/guix.texi (Defining Packages): Adjust accordingly.
---
 doc/guix.texi           | 13 ++++----
 guix/packages.scm       | 53 +++++++++++++++++++++------------
 tests/guix-build.sh     | 11 +++++++
 tests/packages.scm      | 66 +++++++++++++++++++++++++++++++++++++++--
 tests/scripts-build.scm | 12 ++++++--
 5 files changed, 124 insertions(+), 31 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4595008c4f..e9e1d122ab 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -6270,12 +6270,13 @@ This is exactly what the @option{--with-input} command-line option does
 The following variant of @code{package-input-rewriting} can match packages to
 be replaced by name rather than by identity.
 
-@deffn {Scheme Procedure} package-input-rewriting/spec @var{replacements}
-Return a procedure that, given a package, applies the given @var{replacements} to
-all the package graph (excluding implicit inputs).  @var{replacements} is a list of
-spec/procedures pair; each spec is a package specification such as @code{"gcc"} or
-@code{"guile@@2"}, and each procedure takes a matching package and returns a
-replacement for that package.
+@deffn {Scheme Procedure} package-input-rewriting/spec @var{replacements} [#:deep? #t]
+Return a procedure that, given a package, applies the given
+@var{replacements} to all the package graph, including implicit inputs
+unless @var{deep?} is false.  @var{replacements} is a list of
+spec/procedures pair; each spec is a package specification such as
+@code{"gcc"} or @code{"guile@@2"}, and each procedure takes a matching
+package and returns a replacement for that package.
 @end deffn
 
 The example above could be rewritten this way:
diff --git a/guix/packages.scm b/guix/packages.scm
index 171fd048ef..f696945e30 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -422,6 +422,16 @@ name of its URI."
                                                        package)
                                                       16)))))
 
+(define-syntax-rule (package/inherit p overrides ...)
+  "Like (package (inherit P) OVERRIDES ...), except that the same
+transformation is done to the package replacement, if any.  P must be a bare
+identifier, and will be bound to either P or its replacement when evaluating
+OVERRIDES."
+  (let loop ((p p))
+    (package (inherit p)
+      overrides ...
+      (replacement (and=> (package-replacement p) loop)))))
+
 (define (package-upstream-name package)
   "Return the upstream name of PACKAGE, which could be different from the name
 it has in Guix."
@@ -1051,12 +1061,12 @@ package and returns its new name after rewrite."
 
   (package-mapping rewrite (cut assq <> replacements)))
 
-(define (package-input-rewriting/spec replacements)
+(define* (package-input-rewriting/spec replacements #:key (deep? #t))
   "Return a procedure that, given a package, applies the given REPLACEMENTS to
-all the package graph (excluding implicit inputs).  REPLACEMENTS is a list of
-spec/procedures pair; each spec is a package specification such as \"gcc\" or
-\"guile@2\", and each procedure takes a matching package and returns a
-replacement for that package."
+all the package graph, including implicit inputs unless DEEP? is false.
+REPLACEMENTS is a list of spec/procedures pair; each spec is a package
+specification such as \"gcc\" or \"guile@2\", and each procedure takes a
+matching package and returns a replacement for that package."
   (define table
     (fold (lambda (replacement table)
             (match replacement
@@ -1081,22 +1091,27 @@ replacement for that package."
                  (package-name package)
                  table))
 
-  (define (rewrite package)
-    (match (find-replacement package)
-      (#f package)
-      (proc (proc package))))
+  (define replacement-property
+    (gensym " package-replacement"))
 
-  (package-mapping rewrite find-replacement))
+  (define (rewrite p)
+    (if (assq-ref (package-properties p) replacement-property)
+        p
+        (match (find-replacement p)
+          (#f p)
+          (proc
+           (let ((new (proc p)))
+             ;; Mark NEW as already processed.
+             (package/inherit new
+               (properties `((,replacement-property . #t)
+                             ,@(package-properties new)))))))))
 
-(define-syntax-rule (package/inherit p overrides ...)
-  "Like (package (inherit P) OVERRIDES ...), except that the same
-transformation is done to the package replacement, if any.  P must be a bare
-identifier, and will be bound to either P or its replacement when evaluating
-OVERRIDES."
-  (let loop ((p p))
-    (package (inherit p)
-      overrides ...
-      (replacement (and=> (package-replacement p) loop)))))
+  (define (cut? p)
+    (or (assq-ref (package-properties p) replacement-property)
+        (find-replacement p)))
+
+  (package-mapping rewrite cut?
+                   #:deep? deep?))
 
 \f
 ;;;
diff --git a/tests/guix-build.sh b/tests/guix-build.sh
index 6c08857358..ec2f736ccb 100644
--- a/tests/guix-build.sh
+++ b/tests/guix-build.sh
@@ -259,6 +259,17 @@ drv1=`guix build guile -d`
 drv2=`guix build guile --with-input=gimp=ruby -d`
 test "$drv1" = "$drv2"
 
+# See <https://bugs.gnu.org/42156>.
+drv1=`guix build glib -d`
+drv2=`guix build glib -d --with-input=libreoffice=inkscape`
+test "$drv1" = "$drv2"
+
+# Rewriting implicit inputs.
+drv1=`guix build hello -d`
+drv2=`guix build hello -d --with-input=gcc=gcc-toolchain`
+test "$drv1" != "$drv2"
+guix gc -R "$drv2" | grep `guix build -d gcc-toolchain`
+
 if guix build guile --with-input=libunistring=something-really-silly
 then false; else true; fi
 
diff --git a/tests/packages.scm b/tests/packages.scm
index f33332a461..6fa4ad2f1b 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -38,6 +38,7 @@
   #:use-module (guix build-system)
   #:use-module (guix build-system trivial)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system python)
   #:use-module (guix memoization)
   #:use-module (guix profiles)
   #:use-module (guix scripts package)
@@ -45,6 +46,7 @@
   #:use-module (gnu packages base)
   #:use-module (gnu packages guile)
   #:use-module (gnu packages bootstrap)
+  #:use-module (gnu packages python)
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages xml)
   #:use-module (srfi srfi-1)
@@ -1262,7 +1264,8 @@
                               ("baz" ,dep)))))
          (rewrite (package-input-rewriting/spec
                    `(("coreutils" . ,(const sed))
-                     ("grep" . ,(const findutils)))))
+                     ("grep" . ,(const findutils)))
+                   #:deep? #f))
          (p1      (rewrite p0))
          (p2      (rewrite p0)))
     (and (not (eq? p1 p0))
@@ -1279,7 +1282,11 @@
                  (match (package-native-inputs dep3)
                    ((("x" dep))
                     (string=? (package-full-name dep)
-                              (package-full-name findutils))))))))))
+                              (package-full-name findutils)))))))
+
+         ;; Make sure implicit inputs were left unchanged.
+         (equal? (drop (bag-direct-inputs (package->bag p1)) 3)
+                 (drop (bag-direct-inputs (package->bag p0)) 3)))))
 
 (test-assert "package-input-rewriting/spec, partial match"
   (let* ((dep     (dummy-package "chbouib"
@@ -1290,7 +1297,8 @@
                               ("bar" ,dep)))))
          (rewrite (package-input-rewriting/spec
                    `(("chbouib@123" . ,(const sed)) ;not matched
-                     ("grep" . ,(const findutils)))))
+                     ("grep" . ,(const findutils)))
+                   #:deep? #f))
          (p1      (rewrite p0)))
     (and (not (eq? p1 p0))
          (string=? "example" (package-name p1))
@@ -1304,6 +1312,58 @@
                     (string=? (package-full-name dep)
                               (package-full-name findutils))))))))))
 
+(test-assert "package-input-rewriting/spec, deep"
+  (let* ((dep     (dummy-package "chbouib"))
+         (p0      (dummy-package "example"
+                    (build-system gnu-build-system)
+                    (inputs `(("dep" ,dep)))))
+         (rewrite (package-input-rewriting/spec
+                   `(("tar"  . ,(const sed))
+                     ("gzip" . ,(const findutils)))))
+         (p1      (rewrite p0))
+         (p2      (rewrite p0)))
+    (and (not (eq? p1 p0))
+         (eq? p1 p2)                              ;memoization
+         (string=? "example" (package-name p1))
+         (match (package-inputs p1)
+           ((("dep" dep1))
+            (and (string=? (package-full-name dep1)
+                           (package-full-name dep))
+                 (eq? dep1 (rewrite dep)))))      ;memoization
+
+         ;; Make sure implicit inputs were replaced.
+         (match (bag-direct-inputs (package->bag p1))
+           ((("dep" dep1) ("tar" tar) ("gzip" gzip) _ ...)
+            (and (eq? dep1 (rewrite dep))
+                 (string=? (package-full-name tar)
+                           (package-full-name sed))
+                 (string=? (package-full-name gzip)
+                           (package-full-name findutils))))))))
+
+(test-assert "package-input-rewriting/spec, no duplicates"
+  ;; Ensure that deep input rewriting does not forget implicit inputs.  Doing
+  ;; so could lead to duplicates in a package's inputs: in the example below,
+  ;; P0's transitive inputs would contain one rewritten "python" and one
+  ;; original "python".  These two "python" packages are thus not 'eq?' but
+  ;; they lower to the same derivation.  See <https://bugs.gnu.org/42156>,
+  ;; which can be reproduced by passing #:deep? #f.
+  (let* ((dep0    (dummy-package "dep0"
+                    (build-system trivial-build-system)
+                    (propagated-inputs `(("python" ,python)))))
+         (p0      (dummy-package "chbouib"
+                    (build-system python-build-system)
+                    (arguments `(#:python ,python))
+                    (inputs `(("dep0" ,dep0)))))
+         (rewrite (package-input-rewriting/spec '() #:deep? #t))
+         (p1      (rewrite p0))
+         (bag1    (package->bag p1))
+         (pythons (filter-map (match-lambda
+                                (("python" python) python)
+                                (_ #f))
+                              (bag-transitive-inputs bag1))))
+    (match (delete-duplicates pythons eq?)
+      ((p) (eq? p (rewrite python))))))
+
 (test-equal "package-patched-vulnerabilities"
   '(("CVE-2015-1234")
     ("CVE-2016-1234" "CVE-2018-4567")
diff --git a/tests/scripts-build.scm b/tests/scripts-build.scm
index 32876e956a..3491610754 100644
--- a/tests/scripts-build.scm
+++ b/tests/scripts-build.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2016, 2017, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016, 2017, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +19,7 @@
 (define-module (test-scripts-build)
   #:use-module (guix tests)
   #:use-module (guix store)
+  #:use-module (guix derivations)
   #:use-module (guix packages)
   #:use-module (guix git-download)
   #:use-module (guix scripts build)
@@ -163,11 +164,16 @@
                ((("foo" dep1) ("bar" dep2))
                 (and (string=? (package-full-name dep1)
                                (package-full-name grep))
-                     (eq? (package-replacement dep1) findutils)
+                     (string=? (package-full-name (package-replacement dep1))
+                               (package-full-name findutils))
                      (string=? (package-name dep2) "chbouib")
                      (match (package-native-inputs dep2)
                        ((("x" dep))
-                        (eq? (package-replacement dep) findutils)))))))))))
+                        (with-store store
+                          (string=? (derivation-file-name
+                                     (package-derivation store findutils))
+                                    (derivation-file-name
+                                     (package-derivation store dep))))))))))))))
 
 (test-equal "options->transformation, with-branch"
   (git-checkout (url "https://example.org")
-- 
2.28.0

[bug#43578] [PATCH 3/4] packages: 'package-mapping' correctly recurses into 'replacement'.

[Ludovic Courtès]

Previously, something like:

  guix build glib --with-graft=glibc=glibc@2.29

would produce a result showing that rewriting rules were not applied to
libx11@1.6.A (a replacement).

* guix/packages.scm (package-mapping): Call REPLACE instead of PROC to
'replacement'.
* tests/packages.scm ("package-input-rewriting/spec, graft"): New test.
---
 guix/packages.scm  |  2 +-
 tests/packages.scm | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index f696945e30..0d0d7492b6 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1037,7 +1037,7 @@ applied to implicit inputs as well."
               (inputs (map rewrite (package-inputs p)))
               (native-inputs (map rewrite (package-native-inputs p)))
               (propagated-inputs (map rewrite (package-propagated-inputs p)))
-              (replacement (and=> (package-replacement p) proc))
+              (replacement (and=> (package-replacement p) replace))
               (properties `((,mapping-property . #t)
                             ,@(package-properties p))))))))
 
diff --git a/tests/packages.scm b/tests/packages.scm
index 6fa4ad2f1b..e31dea6f72 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -1364,6 +1364,33 @@
     (match (delete-duplicates pythons eq?)
       ((p) (eq? p (rewrite python))))))
 
+(test-equal "package-input-rewriting/spec, graft"
+  (derivation-file-name (package-derivation %store sed))
+
+  ;; Make sure replacements are rewritten.
+  (let* ((dep0 (dummy-package "dep"
+                 (version "1")
+                 (build-system trivial-build-system)
+                 (inputs `(("coreutils" ,coreutils)))))
+         (dep1 (dummy-package "dep"
+                 (version "0")
+                 (build-system trivial-build-system)
+                 (replacement dep0)))
+         (p0   (dummy-package "p"
+                 (build-system trivial-build-system)
+                 (inputs `(("dep" ,dep1)))))
+         (rewrite (package-input-rewriting/spec
+                   `(("coreutils" . ,(const sed)))))
+         (p1      (rewrite p0)))
+    (match (package-inputs p1)
+      ((("dep" dep))
+       (match (package-inputs (package-replacement dep))
+         ((("coreutils" coreutils))
+          ;; COREUTILS is not 'eq?' to SED, so the most reliable way to check
+          ;; for equality is to lower to a derivation.
+          (derivation-file-name
+           (package-derivation %store coreutils))))))))
+
 (test-equal "package-patched-vulnerabilities"
   '(("CVE-2015-1234")
     ("CVE-2016-1234" "CVE-2018-4567")
-- 
2.28.0

[bug#43578] [PATCH 4/4] packages: 'package-input-rewriting' has a #:deep? parameter.

[Ludovic Courtès]

* guix/packages.scm (package-input-rewriting): Add #:deep? and pass it
to 'package-mapping'.
[replacement-property]: New variable.
[rewrite]: Check it.
[cut?]: New procedure.
* tests/packages.scm ("package-input-rewriting"): Pass #:deep? #f and
ensure implicit inputs were not rewritten.  Avoid 'eq?' comparisons.
("package-input-rewriting, deep"): New test.
* gnu/packages/guile.scm (package-for-guile-2.0, package-for-guile-3.0):
Pass #:deep? #f.
---
 doc/guix.texi          | 10 +++++-----
 gnu/packages/guile.scm |  6 ++++--
 guix/packages.scm      | 35 +++++++++++++++++++++++++----------
 tests/packages.scm     | 20 ++++++++++++++++++--
 4 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index e9e1d122ab..193529bbb1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -6236,12 +6236,12 @@ transformation is @dfn{input rewriting}, whereby the dependency tree of
 a package is rewritten by replacing specific inputs by others:
 
 @deffn {Scheme Procedure} package-input-rewriting @var{replacements} @
-           [@var{rewrite-name}]
+           [@var{rewrite-name}] [#:deep? #t]
 Return a procedure that, when passed a package, replaces its direct and
-indirect dependencies (but not its implicit inputs) according to
-@var{replacements}.  @var{replacements} is a list of package pairs; the
-first element of each pair is the package to replace, and the second one
-is the replacement.
+indirect dependencies, including implicit inputs when @var{deep?} is
+true, according to @var{replacements}.  @var{replacements} is a list of
+package pairs; the first element of each pair is the package to replace,
+and the second one is the replacement.
 
 Optionally, @var{rewrite-name} is a one-argument procedure that takes
 the name of a package and returns its new name after rewrite.
diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
index c59daeebe2..280053bf06 100644
--- a/gnu/packages/guile.scm
+++ b/gnu/packages/guile.scm
@@ -420,11 +420,13 @@ GNU@tie{}Guile.  Use the @code{(ice-9 readline)} module and call its
   ;; A procedure that rewrites the dependency tree of the given package to use
   ;; GUILE-2.0 instead of GUILE-3.0.
   (package-input-rewriting `((,guile-3.0 . ,guile-2.0))
-                           (guile-variant-package-name "guile2.0")))
+                           (guile-variant-package-name "guile2.0")
+                           #:deep? #f))
 
 (define package-for-guile-2.2
   (package-input-rewriting `((,guile-3.0 . ,guile-2.2))
-                           (guile-variant-package-name "guile2.2")))
+                           (guile-variant-package-name "guile2.2")
+                           #:deep? #f))
 
 (define-syntax define-deprecated-guile3.0-package
   (lambda (s)
diff --git a/guix/packages.scm b/guix/packages.scm
index 0d0d7492b6..4f2bb432be 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -1044,22 +1044,37 @@ applied to implicit inputs as well."
   replace)
 
 (define* (package-input-rewriting replacements
-                                  #:optional (rewrite-name identity))
+                                  #:optional (rewrite-name identity)
+                                  #:key (deep? #t))
   "Return a procedure that, when passed a package, replaces its direct and
-indirect dependencies (but not its implicit inputs) according to REPLACEMENTS.
-REPLACEMENTS is a list of package pairs; the first element of each pair is the
-package to replace, and the second one is the replacement.
+indirect dependencies, including implicit inputs when DEEP? is true, according
+to REPLACEMENTS.  REPLACEMENTS is a list of package pairs; the first element
+of each pair is the package to replace, and the second one is the replacement.
 
 Optionally, REWRITE-NAME is a one-argument procedure that takes the name of a
 package and returns its new name after rewrite."
+  (define replacement-property
+    ;; Property to tag right-hand sides in REPLACEMENTS.
+    (gensym " package-replacement"))
+
   (define (rewrite p)
-    (match (assq-ref replacements p)
-      (#f  (package
-             (inherit p)
-             (name (rewrite-name (package-name p)))))
-      (new new)))
+    (if (assq-ref (package-properties p) replacement-property)
+        p
+        (match (assq-ref replacements p)
+          (#f  (package/inherit p
+                 (name (rewrite-name (package-name p)))))
+          (new (if deep?
+                   (package/inherit new
+                     (properties `((,replacement-property . #t)
+                                   ,@(package-properties new))))
+                   new)))))
 
-  (package-mapping rewrite (cut assq <> replacements)))
+  (define (cut? p)
+    (or (assq-ref (package-properties p) replacement-property)
+        (assq-ref replacements p)))
+
+  (package-mapping rewrite cut?
+                   #:deep? deep?))
 
 (define* (package-input-rewriting/spec replacements #:key (deep? #t))
   "Return a procedure that, given a package, applies the given REPLACEMENTS to
diff --git a/tests/packages.scm b/tests/packages.scm
index e31dea6f72..af8941c2e2 100644
--- a/tests/packages.scm
+++ b/tests/packages.scm
@@ -1239,7 +1239,8 @@
                               ("baz" ,dep)))))
          (rewrite (package-input-rewriting `((,coreutils . ,sed)
                                              (,grep . ,findutils))
-                                           (cut string-append "r-" <>)))
+                                           (cut string-append "r-" <>)
+                                           #:deep? #f))
          (p1      (rewrite p0))
          (p2      (rewrite p0)))
     (and (not (eq? p1 p0))
@@ -1253,7 +1254,22 @@
                  (eq? dep3 (rewrite dep))         ;memoization
                  (match (package-native-inputs dep3)
                    ((("x" dep))
-                    (eq? dep findutils)))))))))
+                    (eq? dep findutils))))))
+
+         ;; Make sure implicit inputs were left unchanged.
+         (equal? (drop (bag-direct-inputs (package->bag p1)) 3)
+                 (drop (bag-direct-inputs (package->bag p0)) 3)))))
+
+(test-eq "package-input-rewriting, deep"
+  (derivation-file-name (package-derivation %store sed))
+  (let* ((p0      (dummy-package "chbouib"
+                    (build-system python-build-system)
+                    (arguments `(#:python ,python))))
+         (rewrite (package-input-rewriting `((,python . ,sed))))
+         (p1      (rewrite p0)))
+    (match (bag-direct-inputs (package->bag p1))
+      ((("python" python) _ ...)
+       (derivation-file-name (package-derivation %store python))))))
 
 (test-assert "package-input-rewriting/spec"
   (let* ((dep     (dummy-package "chbouib"
-- 
2.28.0

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[zimoun]

Hi,

On Wed, 23 Sep 2020 at 18:23, Ludovic Courtès <ludo@gnu.org> wrote:

> Concretely, the following commands had no effect thus far:
>
>   guix build python-itsdangerous --with-input=python=python2
>   guix build hello --with-input=gcc=gcc-toolchain@10
>
> In both cases, this is because the input we want to change is
> an implicit input.  This patch set fixes that, and it fixes
> <https://issues.guix.gnu.org/42156> as a side effect.

Awesome!

Therefore, 'package-with-explicit-python' & co. are becoming obsolete
(or almost), right?


> This opens new possibilities.  ‘--with-input=python=python2’ is one
> of them, but ‘--with-input=gcc=gcc-toolchain@10’ is not (it fails
> to build for obscure reasons that I’ll fix in ‘core-updates’, and
> it rebuilds the world anyway, which is not practical).  Another

Rebuilding the world, maybe. :-)  It is interesting in the HPC context
where one would like use an "optimized" compiler, isn't?


Thank you.  I will give it a try for my use cases. :-)

All the best,
simon

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[Ludovic Courtès]

Hi!

zimoun <zimon.toutoune@gmail.com> skribis:

> On Wed, 23 Sep 2020 at 18:23, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Concretely, the following commands had no effect thus far:
>>
>>   guix build python-itsdangerous --with-input=python=python2
>>   guix build hello --with-input=gcc=gcc-toolchain@10
>>
>> In both cases, this is because the input we want to change is
>> an implicit input.  This patch set fixes that, and it fixes
>> <https://issues.guix.gnu.org/42156> as a side effect.
>
> Awesome!
>
> Therefore, 'package-with-explicit-python' & co. are becoming obsolete
> (or almost), right?

Good question, we’d have to check on a case-by-case basis.
‘package-input-rewriting’ is coarser-grain: it can potentially rewrite
‘python’ dependencies deeper in the graph than
‘package-with-explicit-python’.

>> This opens new possibilities.  ‘--with-input=python=python2’ is one
>> of them, but ‘--with-input=gcc=gcc-toolchain@10’ is not (it fails
>> to build for obscure reasons that I’ll fix in ‘core-updates’, and
>> it rebuilds the world anyway, which is not practical).  Another
>
> Rebuilding the world, maybe. :-)  It is interesting in the HPC context
> where one would like use an "optimized" compiler, isn't?

Like I wrote, ‘--with-input=gcc=gcc-toolchain@10’ (or similar) isn’t
practical: you’d have to rebuild the world.

What I envision for the use case where you want to build a specific
package set with a different toolchain is to have a
‘--with-toolchain=PACKAGE=TOOLCHAIN’ option.  That would rebuild PACKAGE
with TOOLCHAIN.  Then it would either rebuild all its dependents (as per
‘--with-input’) or graft the rebuilt package (as per ‘--with-graft’).
The latter may not always be a viable option though, so I don’t know.

In fact I think it would be nice if the graft vs. rebuild choice could
be made independently for all the transformation options.

> Thank you.  I will give it a try for my use cases. :-)

Awesome, let me know how it goes!

Thank you,
Ludo’.

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[Efraim Flashner]

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

On Wed, Sep 23, 2020 at 10:51:21PM +0200, Ludovic Courtès wrote:
> Hi!
> 
> zimoun <zimon.toutoune@gmail.com> skribis:
> 
> > On Wed, 23 Sep 2020 at 18:23, Ludovic Courtès <ludo@gnu.org> wrote:
> >
> >> Concretely, the following commands had no effect thus far:
> >>
> >>   guix build python-itsdangerous --with-input=python=python2
> >>   guix build hello --with-input=gcc=gcc-toolchain@10
> >>
> >> In both cases, this is because the input we want to change is
> >> an implicit input.  This patch set fixes that, and it fixes
> >> <https://issues.guix.gnu.org/42156> as a side effect.
> >
> > Awesome!
> >
> > Therefore, 'package-with-explicit-python' & co. are becoming obsolete
> > (or almost), right?
> 
> Good question, we’d have to check on a case-by-case basis.
> ‘package-input-rewriting’ is coarser-grain: it can potentially rewrite
> ‘python’ dependencies deeper in the graph than
> ‘package-with-explicit-python’.
> 

IIRC part of the reason for python-minimal was to build tcl/tk and xorg.
We don't really want to rebuild deep enough to rebuild xorg with
python2. I'm struggling to find a use-case but going as deep as sqlite
shouldn't be a problem.

> >> This opens new possibilities.  ‘--with-input=python=python2’ is one
> >> of them, but ‘--with-input=gcc=gcc-toolchain@10’ is not (it fails
> >> to build for obscure reasons that I’ll fix in ‘core-updates’, and
> >> it rebuilds the world anyway, which is not practical).  Another
> >
> > Rebuilding the world, maybe. :-)  It is interesting in the HPC context
> > where one would like use an "optimized" compiler, isn't?
> 
> Like I wrote, ‘--with-input=gcc=gcc-toolchain@10’ (or similar) isn’t
> practical: you’d have to rebuild the world.
> 
> What I envision for the use case where you want to build a specific
> package set with a different toolchain is to have a
> ‘--with-toolchain=PACKAGE=TOOLCHAIN’ option.  That would rebuild PACKAGE
> with TOOLCHAIN.  Then it would either rebuild all its dependents (as per
> ‘--with-input’) or graft the rebuilt package (as per ‘--with-graft’).
> The latter may not always be a viable option though, so I don’t know.
> 
> In fact I think it would be nice if the graft vs. rebuild choice could
> be made independently for all the transformation options.
> 
> > Thank you.  I will give it a try for my use cases. :-)
> 
> Awesome, let me know how it goes!
> 
> Thank you,
> Ludo’.
> 
> 
> 

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[zimoun]

Hi Ludo,

On Wed, 23 Sep 2020 at 18:12, Ludovic Courtès <ludo@gnu.org> wrote:

> Concretely, the following commands had no effect thus far:
>
>   guix build python-itsdangerous --with-input=python=python2
>   guix build hello --with-input=gcc=gcc-toolchain@10
>
> In both cases, this is because the input we want to change is
> an implicit input.  This patch set fixes that, and it fixes
> <https://issues.guix.gnu.org/42156> as a side effect.

Maybe I am doing wrong and I miss something.

For example, I want to build the package ’emacs-helm’ –which does not
(directly) depends on the package ’emacs’– using the package
’emacs-next’ (changing the Emacs VM from 27 to 28 in this case).

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build emacs emacs-next
/gnu/store/7kr0pg7gwhc31q4iq5vbnm5n99srhp84-emacs-next-28.0.50.1-0.2ea3466
/gnu/store/q3c6y4ccj3li5gfdcbyz24n466fnipp1-emacs-27.1

$ cat /tmp/manif.scm
(specifications->manifest '("emacs-helm"))

$ ./pre-inst-env guix build -m /tmp/manif.scm
/gnu/store/ka9lph0hpzaky0sa52zf09469apkhb68-emacs-helm-3.6.5

$ ./pre-inst-env guix build -m /tmp/manif.scm --with-input=emacs=emacs-next
/gnu/store/ka9lph0hpzaky0sa52zf09469apkhb68-emacs-helm-3.6.5
--8<---------------cut here---------------end--------------->8---

I miss why the hash is the same.  I was expecting a different one, as
with your ’hello’ example (that I reproduce exactly).  What do I miss?

Note it is the same with:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build emacs-helm --with-input=emacs=emacs-next
/gnu/store/ka9lph0hpzaky0sa52zf09469apkhb68-emacs-helm-3.6.5
--8<---------------cut here---------------end--------------->8---


Well, I am trying to provide an example to [1] because your patch set
supersedes it, somehow.

[1] <http://issues.guix.gnu.org/41732#7>


(I have not yet tried the build transformation at the manifest level.)

All the best,
simon

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[Ludovic Courtès]

Hi zimoun,

zimoun <zimon.toutoune@gmail.com> skribis:

> I miss why the hash is the same.  I was expecting a different one, as
> with your ’hello’ example (that I reproduce exactly).  What do I miss?
>
> Note it is the same with:
>
> $ ./pre-inst-env guix build emacs-helm --with-input=emacs=emacs-next
> /gnu/store/ka9lph0hpzaky0sa52zf09469apkhb68-emacs-helm-3.6.5

That’s because ‘emacs-helm’ depends on ‘emacs-minimal’, not ‘emacs’:

--8<---------------cut here---------------start------------->8---
$ guix graph --path -t bag emacs-helm emacs
guix graph: error: no path from 'emacs-helm@3.6.5' to 'emacs@27.1'
$ guix graph --path -t bag emacs-helm emacs-minimal 
emacs-helm@3.6.5
emacs-minimal@27.1
--8<---------------cut here---------------end--------------->8---

Does it work for you if you do ‘--with-input=emacs-minimal=emacs-next’?

HTH,
Ludo’.

[bug#43578] [PATCH 0/4] Rewriting implicit inputs with 'package-input-rewriting' & co.

[zimoun]

On Sat, 26 Sep 2020 at 15:53, Ludovic Courtès <ludo@gnu.org> wrote:

> Does it work for you if you do ‘--with-input=emacs-minimal=emacs-next’?

Ahah!  Stupid me! :-)
Yes it works for me.  And it does what I wanted when discussing the
lengthy and controversial package parameters.  Thank you!

Cheers,
simon