Skip to content

Commit c2aa084

Browse files
wan9chiclaude
andcommitted
refactor(cache): name remote cache values by where they come from
- `PlanOptions.remote_cache_mode`: the `--remote-cache` flag. - `ResolvedRemoteCacheConfig` with `access`: resolved for a `vp run` level. - `PlanContext::resolved_remote_cache`: the level's resolved value. - `CacheConfig.remote_cache_allowed`: the task's `cache.remote`. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 20df2f1 commit c2aa084

71 files changed

Lines changed: 276 additions & 262 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/vt/src/cli/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub enum LogMode {
1919
Grouped,
2020
}
2121

22-
/// Remote cache access selected on the command line.
22+
/// Remote cache mode selected with `--remote-cache`.
2323
#[derive(Debug, Clone, Copy, PartialEq, Eq, clap::ValueEnum)]
2424
pub enum RemoteCacheMode {
2525
/// Remote caching is disabled.
@@ -261,7 +261,7 @@ impl ResolvedRunCommand {
261261
plan_options: PlanOptions {
262262
extra_args: self.additional_args.into(),
263263
cache_override,
264-
remote_cache: self.flags.remote_cache.map(Into::into),
264+
remote_cache_mode: self.flags.remote_cache.map(Into::into),
265265
concurrency_limit,
266266
parallel,
267267
fail_if_no_match,

‎crates/vt/src/session/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ impl<'a> Session<'a> {
582582
plan_options: PlanOptions {
583583
extra_args: run_command.additional_args.clone().into(),
584584
cache_override: run_command.flags.cache_override(),
585-
remote_cache: run_command.flags.remote_cache.map(Into::into),
585+
remote_cache_mode: run_command.flags.remote_cache.map(Into::into),
586586
concurrency_limit: None,
587587
parallel: false,
588588
// The selector path runs whatever the user picked interactively;

‎crates/vt_graph/src/config/mod.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl ResolvedTaskOptions {
8282
)?;
8383

8484
Some(CacheConfig {
85-
remote_cache: enabled_cache_config.remote.unwrap_or(true),
85+
remote_cache_allowed: enabled_cache_config.remote.unwrap_or(true),
8686
env_config: EnvConfig {
8787
fingerprinted_envs: enabled_cache_config
8888
.env
@@ -101,8 +101,9 @@ impl ResolvedTaskOptions {
101101

102102
#[derive(Debug, Clone, Serialize)]
103103
pub struct CacheConfig {
104-
/// Whether this task permits remote caching when local caching is enabled.
105-
pub remote_cache: bool,
104+
/// Whether the task's `cache.remote` allows remote caching. An execution uses
105+
/// the remote cache only if its `vp run` level also resolves one.
106+
pub remote_cache_allowed: bool,
106107
pub env_config: EnvConfig,
107108
pub input_config: ResolvedGlobConfig,
108109
pub output_config: ResolvedGlobConfig,

‎crates/vt_graph/src/config/user.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,8 @@ pub enum UserGlobalCacheConfig {
377377
pub struct ResolvedGlobalCacheConfig {
378378
pub scripts: bool,
379379
pub tasks: bool,
380-
/// Remote cache endpoint from `cache.remote.url`, if configured.
380+
/// Remote cache endpoint from `cache.remote.url`, if configured. Planning
381+
/// uses `VP_REMOTE_CACHE_URL` instead when it's set.
381382
pub remote_url: Option<Arc<str>>,
382383
}
383384

‎crates/vt_plan/src/cache_metadata.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use vt_str::{self, Str};
99
use wincode::{SchemaRead, SchemaWrite};
1010

1111
pub use crate::envs::EnvValueHash;
12-
use crate::{envs::EnvFingerprints, remote_cache::RemoteCacheConfig};
12+
use crate::{envs::EnvFingerprints, remote_cache::ResolvedRemoteCacheConfig};
1313

1414
/// Key to identify an execution across sessions.
1515
#[derive(Debug, SchemaWrite, SchemaRead, Serialize)]
@@ -55,8 +55,9 @@ pub struct CacheMetadata {
5555
/// Used at execution time to determine what output files to archive.
5656
pub output_config: ResolvedGlobConfig,
5757

58-
/// Remote cache access. `None` means local caching only.
59-
pub remote_cache: Option<RemoteCacheConfig>,
58+
/// Remote cache for this execution: the one resolved for its `vp run` level,
59+
/// unless the task sets `cache.remote: false`. `None` means local caching only.
60+
pub remote_cache: Option<ResolvedRemoteCacheConfig>,
6061

6162
/// The unfiltered env context for runner-aware APIs. This is the planning
6263
/// context's envs before spawn-env filtering, including command prefix envs

‎crates/vt_plan/src/context.rs‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use vt_graph::{
88
use vt_path::AbsolutePath;
99
use vt_str::Str;
1010

11-
use crate::{PlanRequestParser, path_env::prepend_path_env, remote_cache::RemoteCacheConfig};
11+
use crate::{
12+
PlanRequestParser, path_env::prepend_path_env, remote_cache::ResolvedRemoteCacheConfig,
13+
};
1214

1315
#[derive(Debug, thiserror::Error)]
1416
#[error(
@@ -65,7 +67,7 @@ pub struct PlanContext<'a> {
6567
/// Settings pass between levels only through envs. A nested level gets a
6668
/// copy of this value through [`duplicate`](Self::duplicate), but replaces
6769
/// it before any reads.
68-
remote_cache: Option<RemoteCacheConfig>,
70+
resolved_remote_cache: Option<ResolvedRemoteCacheConfig>,
6971

7072
/// The query that caused the current expansion.
7173
/// Used by the skip rule to detect and skip duplicate nested expansions.
@@ -91,7 +93,7 @@ impl<'a> PlanContext<'a> {
9193
indexed_task_graph,
9294
extra_args: Arc::default(),
9395
resolved_global_cache,
94-
remote_cache: None,
96+
resolved_remote_cache: None,
9597
parent_query,
9698
}
9799
}
@@ -166,12 +168,12 @@ impl<'a> PlanContext<'a> {
166168
self.resolved_global_cache = config;
167169
}
168170

169-
pub const fn remote_cache(&self) -> Option<&RemoteCacheConfig> {
170-
self.remote_cache.as_ref()
171+
pub const fn resolved_remote_cache(&self) -> Option<&ResolvedRemoteCacheConfig> {
172+
self.resolved_remote_cache.as_ref()
171173
}
172174

173-
pub fn set_remote_cache(&mut self, remote_cache: Option<RemoteCacheConfig>) {
174-
self.remote_cache = remote_cache;
175+
pub fn set_resolved_remote_cache(&mut self, config: Option<ResolvedRemoteCacheConfig>) {
176+
self.resolved_remote_cache = config;
175177
}
176178

177179
pub fn parent_query(&self) -> &TaskQuery {
@@ -198,7 +200,7 @@ impl<'a> PlanContext<'a> {
198200
indexed_task_graph: self.indexed_task_graph,
199201
extra_args: Arc::clone(&self.extra_args),
200202
resolved_global_cache: self.resolved_global_cache.clone(),
201-
remote_cache: self.remote_cache.clone(),
203+
resolved_remote_cache: self.resolved_remote_cache.clone(),
202204
parent_query: Arc::clone(&self.parent_query),
203205
}
204206
}

‎crates/vt_plan/src/plan.rs‎

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use crate::{
4141
CacheOverride, PlanOptions, PlanRequest, QueryPlanRequest, ScriptCommand,
4242
SyntheticPlanRequest,
4343
},
44-
remote_cache::{self, RemoteCacheConfig},
44+
remote_cache::{self, ResolvedRemoteCacheConfig},
4545
resolve_cache_with_override,
4646
};
4747

@@ -306,7 +306,7 @@ async fn plan_task_as_execution_node(
306306
&cwd,
307307
package_path,
308308
parent_cache_config,
309-
context.remote_cache(),
309+
context.resolved_remote_cache(),
310310
)?;
311311
ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(spawn_execution))
312312
}
@@ -337,7 +337,7 @@ async fn plan_task_as_execution_node(
337337
Some(task_execution_cache_key),
338338
&and_item.envs,
339339
&resolved_options,
340-
context.remote_cache(),
340+
context.resolved_remote_cache(),
341341
&script_command.envs,
342342
program_path,
343343
spawn_args,
@@ -405,7 +405,7 @@ async fn plan_task_as_execution_node(
405405
}),
406406
&BTreeMap::new(),
407407
&resolved_options,
408-
context.remote_cache(),
408+
context.resolved_remote_cache(),
409409
context.envs(),
410410
Arc::clone(&*SHELL_PROGRAM_PATH),
411411
SHELL_ARGS.iter().map(|s| Str::from(*s)).chain(std::iter::once(script)).collect(),
@@ -496,7 +496,7 @@ fn resolve_synthetic_cache_config(
496496
Some(enabled_cache_config) => {
497497
let EnabledCacheConfig { env, untracked_env, input, output, remote } =
498498
enabled_cache_config;
499-
parent_config.remote_cache &= remote.unwrap_or(true);
499+
parent_config.remote_cache_allowed &= remote.unwrap_or(true);
500500
parent_config.env_config.fingerprinted_envs.extend(env.unwrap_or_default());
501501
parent_config
502502
.env_config
@@ -559,7 +559,7 @@ pub fn plan_synthetic_request(
559559
cwd: &Arc<AbsolutePath>,
560560
package_dir: &AbsolutePath,
561561
parent_cache_config: ParentCacheConfig,
562-
remote_cache: Option<&RemoteCacheConfig>,
562+
resolved_remote_cache: Option<&ResolvedRemoteCacheConfig>,
563563
) -> Result<SpawnExecution, Error> {
564564
let SyntheticPlanRequest { program, args, cache_config, envs } = synthetic_plan_request;
565565

@@ -580,7 +580,7 @@ pub fn plan_synthetic_request(
580580
execution_cache_key,
581581
prefix_envs,
582582
&resolved_options,
583-
remote_cache,
583+
resolved_remote_cache,
584584
&envs,
585585
program_path,
586586
args,
@@ -617,7 +617,7 @@ fn plan_spawn_execution(
617617
execution_cache_key: Option<ExecutionCacheKey>,
618618
prefix_envs: &BTreeMap<EnvName<Str>, Str>,
619619
resolved_task_options: &ResolvedTaskOptions,
620-
remote_cache: Option<&RemoteCacheConfig>,
620+
resolved_remote_cache: Option<&ResolvedRemoteCacheConfig>,
621621
envs: &Arc<FxHashMap<EnvName<Arc<OsStr>>, Arc<OsStr>>>,
622622
program_path: Arc<AbsolutePath>,
623623
args: Arc<[Str]>,
@@ -691,7 +691,11 @@ fn plan_spawn_execution(
691691
execution_cache_key,
692692
input_config: cache_config.input_config.clone(),
693693
output_config: cache_config.output_config.clone(),
694-
remote_cache: if cache_config.remote_cache { remote_cache.cloned() } else { None },
694+
remote_cache: if cache_config.remote_cache_allowed {
695+
resolved_remote_cache.cloned()
696+
} else {
697+
None
698+
},
695699
unfiltered_envs: Arc::clone(envs),
696700
});
697701
}
@@ -760,16 +764,17 @@ pub async fn plan_query_request(
760764
);
761765
context.set_resolved_global_cache(final_cache);
762766
}
763-
// Resolve `context.remote_cache` for this level (see its doc for the data
764-
// flow). Write this level's `--remote-cache` flag to `VP_REMOTE_CACHE` first,
765-
// so it overrides inherited values and nested levels inherit it. Leave the
766-
// default unset so each level picks its own from the endpoint it sees.
767-
if let Some(mode) = plan_options.remote_cache {
767+
// Resolve `context.resolved_remote_cache` for this level (see its doc for
768+
// the data flow). Write this level's `--remote-cache` flag to
769+
// `VP_REMOTE_CACHE` first, so it overrides inherited values and nested
770+
// levels inherit it. Leave the default unset so each level picks its own
771+
// from the endpoint it sees.
772+
if let Some(mode) = plan_options.remote_cache_mode {
768773
context.add_envs(std::iter::once((remote_cache::MODE_ENV, mode.as_str())));
769774
}
770-
let remote_cache =
775+
let resolved_remote_cache =
771776
remote_cache::resolve(context.resolved_global_cache().remote_url.as_ref(), context.envs())?;
772-
context.set_remote_cache(remote_cache);
777+
context.set_resolved_remote_cache(resolved_remote_cache);
773778

774779
// Resolve effective concurrency for this level.
775780
//
@@ -974,7 +979,7 @@ mod tests {
974979

975980
fn parent_config(includes_auto: bool, positive_globs: &[&str]) -> CacheConfig {
976981
CacheConfig {
977-
remote_cache: true,
982+
remote_cache_allowed: true,
978983
env_config: EnvConfig {
979984
fingerprinted_envs: FxHashSet::default(),
980985
untracked_env: FxHashSet::default(),
@@ -1175,7 +1180,7 @@ mod tests {
11751180
)
11761181
.unwrap()
11771182
.unwrap();
1178-
assert!(!result.remote_cache);
1183+
assert!(!result.remote_cache_allowed);
11791184
}
11801185

11811186
#[test]

‎crates/vt_plan/src/plan_request.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ pub enum CacheOverride {
5050
pub struct PlanOptions {
5151
pub extra_args: Arc<[Str]>,
5252
pub cache_override: CacheOverride,
53-
/// Remote mode for this invocation. None inherits the planning environment.
54-
pub remote_cache: Option<crate::remote_cache::RemoteCacheMode>,
53+
/// This level's `--remote-cache` flag. `None` when the flag isn't passed,
54+
/// leaving `VP_REMOTE_CACHE` from the envs to select the mode.
55+
pub remote_cache_mode: Option<crate::remote_cache::RemoteCacheMode>,
5556
/// Per-level concurrency limit. `None` means inherit from the parent level
5657
/// (or default to [`crate::DEFAULT_CONCURRENCY_LIMIT`] at the root).
5758
pub concurrency_limit: Option<usize>,

‎crates/vt_plan/src/remote_cache.rs‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
//! Remote cache settings resolved for each invocation during planning.
1+
//! Remote cache settings: the mode requested with `--remote-cache` or
2+
//! `VP_REMOTE_CACHE`, and the access resolved from it for each `vp run` level.
23
34
use std::{ffi::OsStr, sync::Arc};
45

@@ -11,7 +12,8 @@ use crate::Error;
1112
pub(crate) const MODE_ENV: &str = "VP_REMOTE_CACHE";
1213
const URL_ENV: &str = "VP_REMOTE_CACHE_URL";
1314

14-
/// Remote access requested by an invocation. Local cache policy is independent.
15+
/// Remote cache mode requested with `--remote-cache` or `VP_REMOTE_CACHE`.
16+
/// `resolve` combines it with the endpoint into a [`ResolvedRemoteCacheConfig`].
1517
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
1618
#[serde(rename_all = "kebab-case")]
1719
pub enum RemoteCacheMode {
@@ -30,15 +32,16 @@ impl RemoteCacheMode {
3032
}
3133
}
3234

33-
/// Remote endpoint and access mode for a cacheable execution.
35+
/// Remote cache access and endpoint resolved for a `vp run` level from the
36+
/// requested mode, `VP_REMOTE_CACHE_URL`, and `cache.remote.url`.
3437
#[derive(Debug, Clone, Serialize)]
35-
pub struct RemoteCacheConfig {
36-
pub mode: RemoteCacheAccess,
38+
pub struct ResolvedRemoteCacheConfig {
39+
pub access: RemoteCacheAccess,
3740
/// Endpoint as configured. It is validated when the remote cache is used.
3841
pub url: Arc<str>,
3942
}
4043

41-
/// Access permitted when remote caching is enabled.
44+
/// Remote cache access after resolution. `off` resolves to no remote cache.
4245
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)]
4346
#[serde(rename_all = "kebab-case")]
4447
pub enum RemoteCacheAccess {
@@ -56,11 +59,11 @@ fn env_value<'a>(
5659
}
5760

5861
/// Resolves remote cache access for one `vp run` level from the envs visible at
59-
/// that level. See `PlanContext::remote_cache` for what they include.
62+
/// that level. See `PlanContext::resolved_remote_cache` for what they include.
6063
pub(crate) fn resolve(
6164
configured_url: Option<&Arc<str>>,
6265
envs: &FxHashMap<EnvName<Arc<OsStr>>, Arc<OsStr>>,
63-
) -> Result<Option<RemoteCacheConfig>, Error> {
66+
) -> Result<Option<ResolvedRemoteCacheConfig>, Error> {
6467
let mode = match env_value(envs, MODE_ENV) {
6568
None => None,
6669
Some(value) => Some(match value.to_str() {
@@ -80,10 +83,10 @@ pub(crate) fn resolve(
8083
match (mode, url) {
8184
(Some(RemoteCacheMode::Off), _) | (None, None) => Ok(None),
8285
(Some(RemoteCacheMode::Read) | None, Some(url)) => {
83-
Ok(Some(RemoteCacheConfig { url, mode: RemoteCacheAccess::Read }))
86+
Ok(Some(ResolvedRemoteCacheConfig { access: RemoteCacheAccess::Read, url }))
8487
}
8588
(Some(RemoteCacheMode::ReadWrite), Some(url)) => {
86-
Ok(Some(RemoteCacheConfig { url, mode: RemoteCacheAccess::ReadWrite }))
89+
Ok(Some(ResolvedRemoteCacheConfig { access: RemoteCacheAccess::ReadWrite, url }))
8790
}
8891
(Some(RemoteCacheMode::Read | RemoteCacheMode::ReadWrite), None) => {
8992
Err(Error::MissingRemoteCacheEndpoint)
@@ -110,7 +113,7 @@ mod tests {
110113
let resolved = resolve(None, &envs).unwrap();
111114
if cfg!(windows) {
112115
let resolved = resolved.unwrap();
113-
assert_eq!(resolved.mode, RemoteCacheAccess::ReadWrite);
116+
assert_eq!(resolved.access, RemoteCacheAccess::ReadWrite);
114117
assert_eq!(&*resolved.url, "https://cache.example");
115118
} else {
116119
assert!(resolved.is_none());

‎crates/vt_plan/tests/plan_snapshots/fixtures/additional_env/snapshots/task_graph.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ flowchart TD
2222
"resolved_options": {
2323
"cwd": "<workspace>/",
2424
"cache_config": {
25-
"remote_cache": true,
25+
"remote_cache_allowed": true,
2626
"env_config": {
2727
"fingerprinted_envs": [],
2828
"untracked_env": [
@@ -62,7 +62,7 @@ flowchart TD
6262
"resolved_options": {
6363
"cwd": "<workspace>/",
6464
"cache_config": {
65-
"remote_cache": true,
65+
"remote_cache_allowed": true,
6666
"env_config": {
6767
"fingerprinted_envs": [],
6868
"untracked_env": [

0 commit comments

Comments
 (0)