[bug#68266] [PATCH 7/7] packages: rust: Memoize make-rust-sysroot results.

[Ludovic Courtès]

Hello,

Christopher Baines <mail@cbaines.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Christopher Baines <mail@cbaines.net> skribis:

[...]

>>>  (define*-public (make-rust-sysroot target)
>>> -  (let ((base-rust rust))
>>> +  (make-rust-sysroot/implementation target rust))
>>> +
>>> +(define make-rust-sysroot/implementation
>>> +  (mlambda (target base-rust)
>>>      (package
>>>        (inherit base-rust)
>>>        (name (string-append "rust-sysroot-for-" target))
>>
>> We should avoid using ‘mlambda’ (without ‘q’) with packages as it leads
>> to deep object comparisons.  That’s why for packages we typically
>> always have one-argument (mlambdaq (package) …).
>>
>> But since ‘base-rust’ wasn’t a parameter before, let’s keep it simple
>> (‘diff --ignore-space-change’):
>
> ...
>
>> WDYT?
>
> Yeah, that does look good. I pushed my earlier version of this patch
> this morning though.
>
> I did have a look at trying to adapt the changes to fit in (guix
> build-system cargo) instead, as I noticed that seemed to be a pattern
> elsewhere, but I think there's something weird going on with the use of
> make-rust-sysroot there since default-rust-sysroot takes an argument,

What matters is that ‘make-rust-sysroot’ takes a single argument,
‘target’, so we can safely write:

  (define make-rust-sysroot
    (mlambda (target)
      …))

I think it’s important to not bring a deep <package> comparison because
this kind of cost is then hard to pinpoint.

Perhaps you can adjust ‘make-rust-sysroot’ along these lines?

Thanks,
Ludo’.