Re: Transform options should error on nonexistant targets

[Ludovic Courtès]

Hi,

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

> On Wed, 18 Aug 2021 at 03:57, Ryan Prior <rprior@protonmail.com> wrote:
>> I learned today that Guix will chug happily along applying a transform to a 
>> nonexistent package.
>>
>> For example, I can run:
>> guix environment --with-latest=not-exist --ad-hoc which
>>
>> This shows no warning or errors. I think it would be beneficial to show an 
>> error & bail if the target of a transformation is a package that doesn't 
>> exist, as this is likely indicative of an error.
>
> Indeed. :-)  The issue comes from guix/transformations.scm 
> (options->transformation):
>
>               (let ((new (transform obj)))
>                 (when (eq? new obj)
>                   (warning (G_ "transformation '~a' had no effect on ~a~%")
>                            name
>                            (if (package? obj)
>                                (package-full-name obj)
>                                obj)))
>                 new)
>
>
> and the warning is not reach because guix/packages.scm (package-mapping):
>
>             (else
>              ;; 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
>                 …
>
>
> In the case of “guix build hello --with-latest=foo”, then ’proc’ has no
> effect, i.e., ’p’ is identical to ’(proc p)’ but a new package is
> allocated which leads that ’new’ and ’obj’ are not ’eq?’.

Indeed.

[...]

> +             (let ((new (proc p)))
> +               (if (eq? new p)
> +                   p
> +                   (package
> +                     (inherit new)
> +                     (location (package-location new))
> +                     (build-system (if deep?
> +                                       (build-system-with-package-mapping
> +                                        (package-build-system new) rewrite)
> +                                       (package-build-system new)))
> +                     (inputs (map rewrite (package-inputs new)))
> +                     (native-inputs (map rewrite (package-native-inputs 
> new)))
> +                     (propagated-inputs (map rewrite 
> (package-propagated-inputs new)))
> +                     (replacement (and=> (package-replacement new) replace))
> +                     (properties `((,mapping-property . #t)
> +                                   ,@(package-properties new))))))))))

Unfortunately we cannot do that: rewriting happens lazily, when the
various inputs fields (which are thunked) are accessed.  When PROC
returns P, we still need to recurse into its inputs, until CUT? says we
can stop.  (I’m surprised this change triggers only one test failure
actually.)

Does that make sense?

Thanks,
Ludo’.