Subject: bug#65665: [PATCH] Really get all the implicit inputs.

From: Ulf Herrman <[email-masked]>
Date: Fri, 06 Oct 2023 02:37:26 -0500

I've also been using this patch, which rebinds `this-package' within
package variants to refer instead to the variant rather than the
original.  When I tried applying a deep transformation to icecat, it
failed to reach icecat-minimal or something like that.  Or maybe it was
some input of icecat - might have been my local version of mesa that had
breakage.  Anyway, I've since forgotten exactly what happened and why (I
didn't actually write the commit message until quite some time later),
but I do know that this patch fixed it.

I might try to rediscover what the problem was later, but thought it
would be good to make you aware of this so as to hopefully consolidate
world rebuilds.

- Ulf

From 06245c37fc0ee21144279682223dff01f3e6dbf4 Mon Sep 17 00:00:00 2001
Message-Id: <[email-masked]>
From: Ulf Herrman <[email-masked]>
Date: Wed, 4 Oct 2023 03:17:25 -0500
Subject: [PATCH] guix: packages: rebind `this-package' for package variants.

Background: there is a `this-package' identifier that can be used within any
of the thunked fields of <package>.  This is implemented by passing the
package in question as an argument to the "thunk" that is called to get the
field value.  `this-package' is used to implement macros like
`this-package-input'.  Consequently, there is a subtle difference between a
package that inherits another package's thunked field and one that wraps it:
in the former, the `this-package' of the thunked field will refer to the
inheriting package, while in the latter case it will refer to the original
package.

Use of `this-package' tends to cause lots of subtle and hard-to-debug breakage
when package transformations are used.  In `package-mapping', it makes more
sense to have `this-package' refer to the transformed package (that is, the
package after being transformed by the user-provided procedure and having all
of its applicable inputs transformed, recursively), since otherwise
untransformed inputs may be reintroduced via `this-package'.

There's still one place where `this-package' can cause problems, though, and
that's in the user-specified transformation procedure itself.  Whether it's
appropriate to have the thunks of the original refer to the original package
or the transformed package is only really known by the author of the
transformation.  We therefore expose procedures for invoking the field thunks
with arbitrary packages.

In my experience, this has led to far more consistent and predictable
package-transforming.

If we really need the ability to refer to the lexically enclosing package
instead of "whichever package ends up using this field definition" - which
seems to be unnecessary at present, since the overwhelming majority of uses of
'this-package' are in the context of 'this-package-input', etc - we could
introduce a 'this-literal-package' identifier.  Or we could rename all current
uses of 'this-package' to use 'this-package-variant' or something like that,
though that sounds like more work.

* guix/packages.scm (package-inputs-with-package,
  package-native-inputs-with-package, package-propagated-inputs-with-package,
  package-arguments-with-package):
  new procedures.
  (package-mapping): use them.
---
 guix/packages.scm | 61 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 47 insertions(+), 14 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index 1334def142..288867b911 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -138,6 +138,11 @@ (define-module (guix packages)
             package-transitive-propagated-inputs
             package-transitive-native-search-paths
             package-transitive-supported-systems
+            package-inputs-with-package
+            package-native-inputs-with-package
+            package-propagated-inputs-with-package
+            package-arguments-with-package
+
             package-mapping
             package-input-rewriting
             package-input-rewriting/spec
@@ -1451,6 +1456,31 @@ (define (build-system-with-package-mapping bs rewrite-input rewrite-argument)
     (inherit bs)
     (lower lower*)))
 
+(define (package-inputs-with-package p0 p)
+  (match p0
+    (($ <package> _ _ _ _ args-proc
+                   inputs-proc
+                   propagated-inputs-proc
+                   native-inputs-proc)
+     (inputs-proc p))))
+
+(define (package-native-inputs-with-package p0 p)
+  (match p0
+    (($ <package> _ _ _ _ _ _ _
+                   native-inputs-proc)
+     (native-inputs-proc p))))
+
+(define (package-propagated-inputs-with-package p0 p)
+  (match p0
+    (($ <package> _ _ _ _ _ _
+                   propagated-inputs-proc)
+     (propagated-inputs-proc p))))
+
+(define (package-arguments-with-package p0 p)
+  (match p0
+    (($ <package> _ _ _ _ arguments-proc)
+     (arguments-proc p))))
+
 (define* (package-mapping proc #:optional (cut? (const #f))
                           #:key deep?)
   "Return a procedure that, given a package, applies PROC to all the packages
@@ -1525,22 +1555,25 @@ (define* (package-mapping proc #:optional (cut? (const #f))
                                     rewrite-input
                                     rewrite-argument)
                                    (package-build-system p)))
-                 (inputs (map rewrite-input (package-inputs p)))
-                 (native-inputs (map rewrite-input (package-native-inputs p)))
+                 (inputs (map rewrite-input
+                              (package-inputs-with-package p this-package)))
+                 (native-inputs (map rewrite-input
+                                     (package-native-inputs-with-package
+                                      p this-package)))
                  (propagated-inputs
-                  (map rewrite-input (package-propagated-inputs p)))
+                  (map rewrite-input
+                       (package-propagated-inputs-with-package
+                        p this-package)))
                  (arguments
-                  (match p
-                       (($ <package> _ _ _ _ args-proc)
-                        ;; If we let ARGS-PROC be passed its original package,
-                        ;; we somehow end up in an infinite (or maybe just
-                        ;; exponential? Never seen it end...) loop.  Should
-                        ;; probably figure out what's causing that at some
-                        ;; point.
-                        (let ((args (args-proc this-package)))
-                          (if deep?
-                              (map rewrite-argument args)
-                              args)))))
+                  ;; If we let ARGS-PROC be passed its original package,
+                  ;; we somehow end up in an infinite (or maybe just
+                  ;; exponential? Never seen it end...) loop.  Should
+                  ;; probably figure out what's causing that at some
+                  ;; point.
+                  (if deep?
+                      (map rewrite-argument (package-arguments-with-package
+                                             p this-package))
+                      (package-arguments-with-package p this-package)))
                  (replacement (and=> (package-replacement p) replace))
                  (properties `((,mapping-property . #t)
                                ,@(package-properties p)))))))))

-- 
2.40.1