[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#68266] [PATCH 7/7] packages: rust: Memoize make-rust-sysroot result
From: |
Efraim Flashner |
Subject: |
[bug#68266] [PATCH 7/7] packages: rust: Memoize make-rust-sysroot results. |
Date: |
Sat, 13 Jan 2024 18:15:49 +0200 |
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
signature.asc
Description: PGP signature
- [bug#68266] [PATCH 0/7] Memoize packages associated with cross building., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 1/7] gnu: Memozise make-ld-wrapper results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 2/7] gnu: Memozise cross-binutils results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 3/7] gnu: Memozise cross-gcc results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 5/7] gnu: Memozise cross-mig results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 4/7] gnu: Memozise cross-kernel-headers results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 7/7] packages: rust: Memoize make-rust-sysroot results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 6/7] gnu: Memozise cross-libc results., Christopher Baines, 2024/01/05
- [bug#68266] [PATCH 1/7] gnu: Memozise make-ld-wrapper results., Ludovic Courtès, 2024/01/08
[bug#68266] [PATCH v2] guix: store: Add report-object-cache-duplication., Christopher Baines, 2024/01/10