Skip to content

Commit 8ee958b

Browse files
branchseerclaude
andcommitted
refactor(cache): deduplicate PreRunFingerprint creation and add glob base tests
- Extract PreRunFingerprint::from_cache_metadata() to avoid code duplication in try_hit() and update() methods - Rename base_dir parameter to workspace_root for clarity - Replace expect() calls with proper anyhow errors in spawn.rs - Add e2e tests for glob base directory behavior: - Verify globs are relative to package directory, not task cwd - Test root package and subpackage glob resolution - Test that custom cwd doesn't affect glob base Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 26715e8 commit 8ee958b

20 files changed

Lines changed: 519 additions & 52 deletions

crates/vite_task/src/session/cache/mod.rs

Lines changed: 43 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,40 @@ pub struct PreRunFingerprint {
3636
pub globbed_inputs: BTreeMap<RelativePathBuf, u64>,
3737
}
3838

39+
impl PreRunFingerprint {
40+
/// Build a pre-run fingerprint from cache metadata and globbed inputs.
41+
///
42+
/// # Arguments
43+
/// * `cache_metadata` - Cache metadata from plan stage (contains absolute glob_base)
44+
/// * `globbed_inputs` - Hashes of explicit input files computed from positive globs
45+
/// * `workspace_root` - Workspace root for converting absolute glob_base to relative
46+
fn from_cache_metadata(
47+
cache_metadata: &CacheMetadata,
48+
globbed_inputs: BTreeMap<RelativePathBuf, u64>,
49+
workspace_root: &AbsolutePath,
50+
) -> anyhow::Result<Self> {
51+
// Convert absolute glob_base to relative for cache key
52+
let glob_base = cache_metadata
53+
.glob_base
54+
.strip_prefix(workspace_root)
55+
.map_err(|e| anyhow::anyhow!("failed to strip prefix from glob_base: {e}"))?
56+
.ok_or_else(|| {
57+
anyhow::anyhow!(
58+
"glob_base {:?} is not inside workspace {:?}",
59+
cache_metadata.glob_base,
60+
workspace_root
61+
)
62+
})?;
63+
64+
Ok(Self {
65+
spawn_fingerprint: cache_metadata.spawn_fingerprint.clone(),
66+
input_config: cache_metadata.input_config.clone(),
67+
glob_base,
68+
globbed_inputs,
69+
})
70+
}
71+
}
72+
3973
/// Command cache value, for validating post-run fingerprint after the spawn fingerprint is matched,
4074
/// and replaying the std outputs if validated.
4175
#[derive(Debug, Encode, Decode, Serialize)]
@@ -140,44 +174,26 @@ impl ExecutionCache {
140174
/// # Arguments
141175
/// * `cache_metadata` - Cache metadata from plan stage
142176
/// * `globbed_inputs` - Hashes of explicit input files computed from positive globs
143-
/// * `base_dir` - Workspace root for validating post-run fingerprint
177+
/// * `workspace_root` - Workspace root for converting paths and validating fingerprints
144178
pub async fn try_hit(
145179
&self,
146180
cache_metadata: &CacheMetadata,
147181
globbed_inputs: BTreeMap<RelativePathBuf, u64>,
148-
base_dir: &AbsolutePath,
182+
workspace_root: &AbsolutePath,
149183
) -> anyhow::Result<Result<CommandCacheValue, CacheMiss>> {
150184
let spawn_fingerprint = &cache_metadata.spawn_fingerprint;
151185
let execution_cache_key = &cache_metadata.execution_cache_key;
152186
let input_config = &cache_metadata.input_config;
153187

154-
// Convert absolute glob_base to relative for cache key
155-
let glob_base = cache_metadata
156-
.glob_base
157-
.strip_prefix(base_dir)
158-
.map_err(|e| anyhow::anyhow!("failed to strip prefix from glob_base: {e}"))?
159-
.ok_or_else(|| {
160-
anyhow::anyhow!(
161-
"glob_base {:?} is not inside workspace {:?}",
162-
cache_metadata.glob_base,
163-
base_dir
164-
)
165-
})?;
166-
167-
// Build pre-run fingerprint combining spawn fingerprint, input config, and globbed inputs
168-
let pre_run_fingerprint = PreRunFingerprint {
169-
spawn_fingerprint: spawn_fingerprint.clone(),
170-
input_config: input_config.clone(),
171-
glob_base,
172-
globbed_inputs,
173-
};
188+
let pre_run_fingerprint =
189+
PreRunFingerprint::from_cache_metadata(cache_metadata, globbed_inputs, workspace_root)?;
174190

175191
// Try to directly find the cache by pre-run fingerprint first
176192
if let Some(cache_value) = self.get_by_pre_run_fingerprint(&pre_run_fingerprint).await? {
177193
// Validate post-run fingerprint (inferred inputs) only if auto inference is enabled
178194
if input_config.includes_auto {
179195
if let Some(post_run_fingerprint_mismatch) =
180-
cache_value.post_run_fingerprint.validate(base_dir)?
196+
cache_value.post_run_fingerprint.validate(workspace_root)?
181197
{
182198
// Found the cache with the same pre-run fingerprint, but the post-run fingerprint mismatches
183199
return Ok(Err(CacheMiss::FingerprintMismatch(
@@ -217,39 +233,20 @@ impl ExecutionCache {
217233
/// # Arguments
218234
/// * `cache_metadata` - Cache metadata from plan stage
219235
/// * `globbed_inputs` - Hashes of explicit input files computed from positive globs
220-
/// * `base_dir` - Workspace root for converting absolute paths to relative
236+
/// * `workspace_root` - Workspace root for converting absolute paths to relative
221237
/// * `cache_value` - The cache value to store (outputs and post-run fingerprint)
222238
pub async fn update(
223239
&self,
224240
cache_metadata: &CacheMetadata,
225241
globbed_inputs: BTreeMap<RelativePathBuf, u64>,
226-
base_dir: &AbsolutePath,
242+
workspace_root: &AbsolutePath,
227243
cache_value: CommandCacheValue,
228244
) -> anyhow::Result<()> {
229245
let spawn_fingerprint = &cache_metadata.spawn_fingerprint;
230246
let execution_cache_key = &cache_metadata.execution_cache_key;
231-
let input_config = &cache_metadata.input_config;
232247

233-
// Convert absolute glob_base to relative for cache key
234-
let glob_base = cache_metadata
235-
.glob_base
236-
.strip_prefix(base_dir)
237-
.map_err(|e| anyhow::anyhow!("failed to strip prefix from glob_base: {e}"))?
238-
.ok_or_else(|| {
239-
anyhow::anyhow!(
240-
"glob_base {:?} is not inside workspace {:?}",
241-
cache_metadata.glob_base,
242-
base_dir
243-
)
244-
})?;
245-
246-
// Build pre-run fingerprint combining spawn fingerprint, input config, and globbed inputs
247-
let pre_run_fingerprint = PreRunFingerprint {
248-
spawn_fingerprint: spawn_fingerprint.clone(),
249-
input_config: input_config.clone(),
250-
glob_base,
251-
globbed_inputs,
252-
};
248+
let pre_run_fingerprint =
249+
PreRunFingerprint::from_cache_metadata(cache_metadata, globbed_inputs, workspace_root)?;
253250

254251
self.upsert_pre_run_fingerprint_cache(&pre_run_fingerprint, &cache_value).await?;
255252
self.upsert_execution_key_to_fingerprint(execution_cache_key, spawn_fingerprint).await?;

crates/vite_task/src/session/execute/spawn.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,18 +154,22 @@ where
154154
}
155155
}
156156

157-
let termination = match tracking_state {
158-
TrackingState::FspyEnabled(tracked_child) => Some(tracked_child.wait_handle.await?),
157+
// Wait for process termination and process path accesses if fspy was enabled
158+
let (termination, path_accesses) = match tracking_state {
159+
TrackingState::FspyEnabled(tracked_child) => {
160+
let termination = tracked_child.wait_handle.await?;
161+
// path_accesses must be Some when fspy is enabled (they're set together)
162+
let path_accesses = path_accesses.ok_or_else(|| {
163+
anyhow::anyhow!("internal error: fspy enabled but path_accesses is None")
164+
})?;
165+
(termination, path_accesses)
166+
}
159167
TrackingState::FspyDisabled(mut tokio_child) => {
160168
let exit_status = tokio_child.wait().await?;
161169
return Ok(SpawnResult { exit_status, duration: start.elapsed() });
162170
}
163171
};
164172
let duration = start.elapsed();
165-
166-
// Process path accesses from fspy (only when fspy was enabled)
167-
let termination = termination.expect("fspy enabled but no termination");
168-
let path_accesses = path_accesses.expect("fspy enabled but no path_accesses");
169173
let path_reads = &mut path_accesses.path_reads;
170174
let path_writes = &mut path_accesses.path_writes;
171175

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const rootOther = 'initial';
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "glob-base-test",
3+
"private": true
4+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const other = 'initial';
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "sub-pkg",
3+
"private": true
4+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const sub = 'initial';
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"tasks": {
3+
"sub-glob-test": {
4+
"command": "print-file src/sub.ts",
5+
"inputs": ["src/**/*.ts"],
6+
"cache": true
7+
},
8+
"sub-glob-with-cwd": {
9+
"command": "print-file sub.ts",
10+
"cwd": "src",
11+
"inputs": ["src/**/*.ts"],
12+
"cache": true
13+
}
14+
}
15+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
packages:
2+
- 'packages/*'
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# Test glob base directory behavior
2+
# Globs are relative to PACKAGE directory, NOT task cwd
3+
4+
# 1. Root package - glob matches files in root's src/
5+
[[e2e]]
6+
name = "root glob - matches src files"
7+
steps = [
8+
"vite run root-glob-test",
9+
# Modify matched file
10+
"replace-file-content src/root.ts initial modified",
11+
# Cache miss: file matches glob
12+
"vite run root-glob-test",
13+
]
14+
15+
[[e2e]]
16+
name = "root glob - ignores other directory"
17+
steps = [
18+
"vite run root-glob-test",
19+
# Modify file outside glob pattern
20+
"replace-file-content other/other.ts initial modified",
21+
# Cache hit: file doesn't match glob
22+
"vite run root-glob-test",
23+
]
24+
25+
[[e2e]]
26+
name = "root glob - ignores subpackage files"
27+
steps = [
28+
"vite run root-glob-test",
29+
# Modify file in subpackage - should NOT match root's src/**
30+
"replace-file-content packages/sub-pkg/src/sub.ts initial modified",
31+
# Cache hit: subpackage files don't match root's glob
32+
"vite run root-glob-test",
33+
]
34+
35+
# 2. Root package with custom cwd - glob still relative to package root
36+
[[e2e]]
37+
name = "root glob with cwd - glob relative to package not cwd"
38+
steps = [
39+
"vite run root-glob-with-cwd",
40+
# Modify file - glob is src/** relative to package root
41+
"replace-file-content src/root.ts initial modified",
42+
# Cache miss: file matches glob (relative to package, not cwd)
43+
"vite run root-glob-with-cwd",
44+
]
45+
46+
# 3. Subpackage - glob matches files in subpackage's src/
47+
[[e2e]]
48+
name = "subpackage glob - matches own src files"
49+
steps = [
50+
"vite run sub-pkg#sub-glob-test",
51+
# Modify matched file in subpackage
52+
"replace-file-content packages/sub-pkg/src/sub.ts initial modified",
53+
# Cache miss: file matches subpackage's glob
54+
"vite run sub-pkg#sub-glob-test",
55+
]
56+
57+
[[e2e]]
58+
name = "subpackage glob - ignores other directory in subpackage"
59+
steps = [
60+
"vite run sub-pkg#sub-glob-test",
61+
# Modify file outside glob pattern
62+
"replace-file-content packages/sub-pkg/other/other.ts initial modified",
63+
# Cache hit: file doesn't match glob
64+
"vite run sub-pkg#sub-glob-test",
65+
]
66+
67+
[[e2e]]
68+
name = "subpackage glob - ignores root src files"
69+
steps = [
70+
"vite run sub-pkg#sub-glob-test",
71+
# Modify file in root - should NOT match subpackage's src/**
72+
"replace-file-content src/root.ts initial modified",
73+
# Cache hit: root files don't match subpackage's glob
74+
"vite run sub-pkg#sub-glob-test",
75+
]
76+
77+
# 4. Subpackage with custom cwd - glob still relative to subpackage root
78+
[[e2e]]
79+
name = "subpackage glob with cwd - glob relative to package not cwd"
80+
steps = [
81+
"vite run sub-pkg#sub-glob-with-cwd",
82+
# Modify file - glob is src/** relative to subpackage root
83+
"replace-file-content packages/sub-pkg/src/sub.ts initial modified",
84+
# Cache miss: file matches glob (relative to subpackage, not cwd)
85+
"vite run sub-pkg#sub-glob-with-cwd",
86+
]

0 commit comments

Comments
 (0)