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

[Efraim Flashner <efraim@flashner.co.il>]

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

On Fri, Jan 12, 2024 at 05:57:26PM +0000, Christopher Baines wrote:
> 
> Ludovic Courtès <ludo@gnu.org> writes:
> 
> > Christopher Baines <mail@cbaines.net> skribis:
> >
> >> To ensure that it just returns a single package record for some given
> >> arguments, as this helps to avoid poor performance of the store connection
> >> object cache.
> >>
> >> * gnu/packages/rust.scm (make-rust-sysroot): Move code to
> >> make-rust-sysroot/implementation.
> >> (make-rust-sysroot/implementation): New variable.
> >>
> >> Change-Id: Ibb30c7398328c87c032bb8828635a34ada935167
> >
> > [...]
> >
> >>  (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,
> but doesn't use it. Maybe once that's figured out, we can move the
> memoization there and switch to just using the target as the key.
> 
> Unfortunately I'm still waiting to see what effect this has on the data
> service processing revisions. I'm pretty sure it's going to help, but
> I'm concerned it's not going to help enough to make processing revisions
> for patches feasible again.

I looked at the build system a bit and I think it was a combination of
cargo-culting the other cross build implementations that took a target
argument for the cross-compilers and I figured that rust-sysroot would
also need one. The other bit was I think I had in mind the possibility
of choosing seemingly arbitrary targets which were supported by rust but
not known to Guix and having it possible to cross-compile to those.
Given that rust, like go, IIRC doesn't actually need a cross-compiled
compiler to build cross-compiled packages, could it be that parts of
that logic can be rewritten/simplified?


-- 
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 --]