rust: honor preconfigured Cargo cache for offline builds#5634
Conversation
Rust currently overwrites CARGO_HOME for dependent builds with a package-stage directory. This prevents users from using a prepopulated Cargo cache when CARGO_NET_OFFLINE=true, since Cargo instead searches the empty stage-local cache. Honor an explicitly configured CARGO_HOME only for offline builds. Retain Spack's isolated per-package Cargo cache for the normal online case.
climbfuji
left a comment
There was a problem hiding this comment.
You said it all. Thanks for this.
alecbcs
left a comment
There was a problem hiding this comment.
I'm a little torn here between exposing this functionality as a variant on rust and allowing the environment overrides. Setting the environment variables are likely what more people who aren't in the Spack community will expect, but it could lead to difficulties debugging where folks will get two different behaviors under the same Spack package hash.
Co-authored-by: Alec Scott <hi@alecbcs.com>
Yeah. I wasn't sure what to do. This was in truth a very GPT 5.6 Terra fix. Once you get into rust land, my knowledge goes poof. If you prefer a variant, we could do that. I mean, at the moment I suppose @climbfuji and myself are two of the few people I know that do these sort of air-gapped installs. If you prefer this sort of "look at the environment for this stuff" to be under a variant, we could probably adapt to that. |
I'm fine either way as long as we can utilize our own mirror under CARGO_HOME. |
mcmehrtens
left a comment
There was a problem hiding this comment.
Thanks for proposing these changes! I'm still relatively new to how Spack does things, so take my input with a grain of salt, but I wanted to offer my thoughts anyway.
A month or so ago, I went down a rabbit-hole of trying to get Go packages to build in an air-gapped environment and ran into the exact problem this PR is addressing. There's actually an existing issue and closed PR that propose to solve this problem in Rust world (and I think could be mirrored in the Go ecosystem as well). I'd argue the proper solution to the problem addressed in this PR is a more architectural change to how Spack manages/fetches dependencies for Cargo or Go, but I don't see an issue going forward with this PR's implementation as a stopgap until those changes get a new champion to carry them through.
I thought for a while about what @alecbcs said about a variant. As a user, a variant feels like the wrong flag for this (this is a difference in build behavior, not in enabling/disabling functionalities). I agree with @alecbcs's concern that the package hash will be the same whether Cargo is using an offline cache or live registry (and could resolve to widely different versions, especially since Spack's CargoBuilder doesn't default to using --locked). That feels wrong, but to my understanding, that's already the case.
Today, by default, user A could build a Cargo package on Monday, user B could build the same Cargo package on Friday (with the same version, variants, resolved hash, etc.), and their resolved dependencies very well might be different. This feels synonymous to me with user A building a Cargo package from an offline cache, user B building the same package with the live registry, and the packages resolving to different dependencies. This problem with the hash not accounting for Cargo/Go dependencies is what would be better addressed by spack/spack#16045. In other words, introducing the ability to point at an offline cache wouldn't be, in my opinion, a regression from existing behavior.
My main concern with this PR is the scenario in which a user sets CARGO_HOME (very common) and CARGO_NET_OFFLINE for a non-Spack managed Rust project, and then having those settings unknowingly leak into Spack's configuration—although, to be fair, I'm not entirely sure what all the downsides/problems or downstream effects of this would be. I suppose it just feels un-Spack-like to be plopping Spack dependencies into a user's CARGO_HOME (and, FWIW, was why this behavior was changed to begin with).
I think a better solution is to mirror existing environment variables like SPACK_GNUPGHOME or SPACK_PYTHON and use a variable name such as SPACK_CARGO_HOME instead of simply CARGO_HOME. SPACK_CARGO_HOME clearly indicates intent to Spack in a way plain CARGO_HOME does not. CARGO_NET_OFFLINE should already get passed through to the Cargo build, and I think not introducing a Spack-prefixed equivalent for that is fine. CARGO_NET_OFFLINE seems to me like the type of setting that would be set at a system-level, not project-by-project, but I could be easily convinced otherwise for similar reasons. Regardless, as a fallback, the user can still change the value of CARGO_NET_OFFLINE at Spack runtime if so desired.
@alecbcs, @mathomp4, and @climbfuji: any thoughts on this? @alecbcs, would a change like this need any documentation or test updates?
Co-authored-by: Matthew Mehrtens <12023414+mcmehrtens@users.noreply.github.com>
|
@mcmehrtens I have no issue with moving from In a way, as you say, I'll ping @climbfuji and @AlexanderRichert-NOAA if they have thoughts/objections. But I could pretty quickly push a change on this branch if all are okay with it. |
|
@mcmehrtens I agree with your points against a variant. I also agree we need a better long term solution for the Go and Rust ecosystems. @becker33 and I have discussed this in the past for how we could only define "edge nodes" (go or rust packages that depend on a system library) and then read metadata from crates.io / pkg.go.dev for all of the packages that are purely in those languages and construct synthetic internal objects for them and put them into the dependency solver without us having to define custom package.py files for each and every one. You could then save an index of those packages from the open and then use that on an air-gapped system to truly have Spack manage all of the dependencies in these ecosystems and properly create a full mirror of all of the source. This is something we're planning in the future roadmap, but I think for now this is a good quick work around. |
Summary
Rust unconditionally sets
CARGO_HOMEto a package-stage directory for dependent builds. This overrides a user-provided, prepopulated Cargo cache, makingCARGO_NET_OFFLINE=trueineffective.This change preserves an explicitly configured
CARGO_HOMEwhen Cargo is explicitly in offline mode. Online builds retain the existing isolated per-package cache behavior.I'm going to CC in @climbfuji as he might be able to explain more.
Testing
Tested with an offline Cargo cache populated before the Spack build; Rust-dependent packages use the configured cache rather than a new stage-local directory.
Disclosure
This change was developed with assistance from OpenAI GPT-5.6 Terra and reviewed by the contributor.