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

> Ulf Herrman <striness@tilde.club> skribis:
>
>> -(define (build-system-with-package-mapping bs rewrite)
>> +(define (build-system-with-package-mapping bs rewrite-input rewrite-argument)
>>    "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."
>> @@ -1442,9 +1442,10 @@ (define (build-system-with-package-mapping bs rewrite)
>>      (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-inputs (map rewrite-input (bag-build-inputs lowered)))
>> +        (host-inputs (map rewrite-input (bag-host-inputs lowered)))
>> +        (target-inputs (map rewrite-input (bag-target-inputs lowered)))
>> +        (arguments (map rewrite-argument (bag-arguments lowered))))))
>
> Aah, now I understand.  :-)
>
> It’s indeed the case that arguments can capture references to packages
> that won’t be caught by ‘package-mapping’.  For instance, if you write:
>
>   (package
>     …
>     (arguments (list … #~(whatever #$coreutils))))
>
> … then ‘coreutils’ here cannot be replaced.
>
> To address this, the recommendation is to always add dependencies to
> input fields and to use self-references within arguments.  The example
> above should be written like this:
>
>   (package
>     …
>     (arguments
>      (list … #~(whatever #$(this-package-input "coreutils")))))
>
> It’s just a recommendation and one can perfectly ignore it, and I
> suppose that was the impetus for this patch.

That and a growing thirst for a nuclear option for package rewriting
brought about by trying to debug deep transformations while
simultaneously experimenting with rewriting a manifest of around 270
packages.

There are really several distinct issues at play here:
1. The case of #:qtbase and #:guile being invisible to package-mapping.
   This is what I first noticed, and cannot be fixed without modifying
   the build systems.  This is what prompted looking for packages in
   package and bag arguments, and recursing into lists therein (just in
   case an argument took a list of packages).
2. The (perceived) case of packages hiding inside arguments.  In
   hindsight, this was probably actually (3) causing this, though it's
   hard to tell because I discovered it last.  I attempted to resolve
   this by recursing through <gexp>s and <gexp-input>s.
3. `this-package' referring to the inherited package in thunked fields,
   rather than the package inheriting them.  This is what prompted the
   use of package-{inputs,arguments,etc}-with-package.

(1) could be resolved in several ways.  I'm partial to looking for
arguments whose values are packages and transforming them (when #:deep?
#t is specified), and adjusting the build systems to make that work
consistently, which is what I've done.

(2) is probably not an issue, though it occurs to me that the technique
of recursively searching through arguments looking for packages could be
used to implement a sort of automated "transformability" check, which
could help a lot when trying to debug transformations.

(3) is a major issue; the entire strategy of using `this-package-input'
to enable transformations breaks because of it.  My fix works for me at
least, though it requires exposing additional low-level procedures and
transformation authors using them.  I'll open another bug about it, as
requested.

> This is one of the things discussed while designing this change:
>
>   https://guix.gnu.org/en/blog/2021/the-big-change/
>   (search for “self-referential records”)
>
>   https://issues.guix.gnu.org/49169
>
> My take was and still is that it’s an acceptable limitation.  Packagers
> need to follow the guideline above if they want proper support for
> rewriting, ‘guix graph’, and other tools.
>
> WDYT?

I think it's probably reasonable, though I would like it if there were
tooling in place to detect cases where said guidelines are not
followed, so as to aid in debugging and verifying that a transformation
worked as intended.

- Ulf