Skip to content

Commit 379326a

Browse files
branchseerclaude
andcommitted
refactor(cache): unify glob resolution into generic ResolvedGlob type
Replace the negative-glob-specific `ResolvedNegativeGlob` struct, `resolve_negative_globs()`, and `is_excluded()` with a single generic `ResolvedGlob` type that provides both `walk()` (for positive globs) and `matches()` (for negative globs). The partition + path_clean logic now lives in one place (`ResolvedGlob::new`). Also normalize fspy-tracked paths in `PostRunFingerprint::create` so stored keys are always clean (no `..` components), making cache miss messages deterministic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a841ee6 commit 379326a

File tree

3 files changed

+80
-82
lines changed

3 files changed

+80
-82
lines changed

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

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@ use vite_path::{AbsolutePath, RelativePathBuf};
1717
use vite_str::Str;
1818
use vite_task_graph::config::ResolvedInputConfig;
1919

20-
use super::{
21-
glob_inputs::{is_excluded, resolve_negative_globs},
22-
spawn::PathRead,
23-
};
20+
use super::{glob_inputs::ResolvedGlob, spawn::PathRead};
2421
use crate::collections::HashMap;
2522

2623
/// Post-run fingerprint capturing file state after execution.
@@ -90,26 +87,38 @@ impl PostRunFingerprint {
9087
return Ok(Self { inferred_inputs: HashMap::default() });
9188
}
9289

93-
// Resolve negative globs relative to the package directory (glob_base),
94-
// handling `..` components so patterns like `!../shared/dist/**` work.
95-
let resolved_negatives = resolve_negative_globs(&input_config.negative_globs, glob_base)?;
90+
let negatives: Vec<ResolvedGlob> = input_config
91+
.negative_globs
92+
.iter()
93+
.map(|p| ResolvedGlob::new(p.as_str(), glob_base))
94+
.collect::<anyhow::Result<_>>()?;
9695

9796
let inferred_inputs = path_reads
9897
.par_iter()
99-
.filter(|(path, _)| {
100-
if resolved_negatives.is_empty() {
101-
return true;
102-
}
103-
// Convert workspace-relative path to absolute for negative glob matching.
104-
// Clean the path to resolve any `..` components from fspy-tracked paths
98+
.filter_map(|(relative_path, path_read)| {
99+
// Clean the absolute path to normalize `..` from fspy-tracked paths
105100
// (e.g., `packages/sub-pkg/../shared/dist/output.js`).
106-
let absolute = path_clean::PathClean::clean(base_dir.join(path).as_path());
107-
!is_excluded(&absolute, &resolved_negatives)
108-
})
109-
.map(|(relative_path, path_read)| {
110-
let full_path = Arc::<AbsolutePath>::from(base_dir.join(relative_path));
111-
let fingerprint = fingerprint_path(&full_path, *path_read)?;
112-
Ok((relative_path.clone(), fingerprint))
101+
let cleaned_abs =
102+
path_clean::PathClean::clean(base_dir.join(relative_path).as_path());
103+
104+
// Apply negative globs against the cleaned path
105+
if negatives.iter().any(|neg| neg.matches(&cleaned_abs)) {
106+
return None;
107+
}
108+
109+
// Derive a cleaned workspace-relative key so stored paths are normalized
110+
let clean_key = cleaned_abs
111+
.strip_prefix(base_dir.as_path())
112+
.ok()
113+
.and_then(|p| RelativePathBuf::new(p).ok())
114+
.unwrap_or_else(|| relative_path.clone());
115+
116+
let full_path = Arc::<AbsolutePath>::from(base_dir.join(&clean_key));
117+
let fingerprint = match fingerprint_path(&full_path, *path_read) {
118+
Ok(f) => f,
119+
Err(e) => return Some(Err(e)),
120+
};
121+
Some(Ok((clean_key, fingerprint)))
113122
})
114123
.collect::<anyhow::Result<HashMap<_, _>>>()?;
115124

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

Lines changed: 50 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -17,49 +17,58 @@ use vite_path::{AbsolutePath, RelativePathBuf};
1717
use vite_str::Str;
1818
use wax::{Glob, Program as _};
1919

20-
/// A negative glob pattern resolved to an absolute base directory and optional variant.
20+
/// A glob pattern resolved to an absolute base directory.
2121
///
22-
/// For example, `../shared/dist/**` relative to `/ws/packages/app` resolves to:
23-
/// - `resolved_base`: `/ws/packages/shared/dist`
22+
/// Uses [`wax::Glob::partition`] to separate the invariant prefix from the
23+
/// wildcard suffix, then resolves the prefix to an absolute path via
24+
/// [`path_clean`] (normalizing components like `..`).
25+
///
26+
/// For example, `../shared/src/**` relative to `/ws/packages/app` resolves to:
27+
/// - `resolved_base`: `/ws/packages/shared/src`
2428
/// - `variant`: `Some(Glob("**"))`
2529
#[expect(clippy::disallowed_types, reason = "path_clean returns std::path::PathBuf")]
26-
pub struct ResolvedNegativeGlob {
30+
pub struct ResolvedGlob {
2731
resolved_base: std::path::PathBuf,
2832
variant: Option<Glob<'static>>,
2933
}
3034

31-
/// Resolve negative globs relative to `base_dir`, handling `..` components.
32-
///
33-
/// Uses [`Glob::partition`] to split each pattern into an invariant base and an
34-
/// optional variant, then resolves the base with [`path_clean`] to normalize `..`.
35-
pub fn resolve_negative_globs(
36-
patterns: &std::collections::BTreeSet<Str>,
37-
base_dir: &AbsolutePath,
38-
) -> anyhow::Result<Vec<ResolvedNegativeGlob>> {
39-
patterns
40-
.iter()
41-
.map(|pattern| {
42-
let glob = Glob::new(pattern.as_str())?.into_owned();
43-
let (base_pathbuf, variant) = glob.partition();
44-
let base_str = base_pathbuf.to_str().unwrap_or(".");
45-
let resolved_base = if base_str.is_empty() {
46-
base_dir.as_path().to_path_buf()
47-
} else {
48-
base_dir.join(base_str).as_path().clean()
49-
};
50-
Ok(ResolvedNegativeGlob { resolved_base, variant: variant.map(Glob::into_owned) })
51-
})
52-
.collect()
53-
}
35+
impl ResolvedGlob {
36+
/// Resolve a glob pattern relative to `base_dir`.
37+
pub fn new(pattern: &str, base_dir: &AbsolutePath) -> anyhow::Result<Self> {
38+
let glob = Glob::new(pattern)?.into_owned();
39+
let (base_pathbuf, variant) = glob.partition();
40+
let base_str = base_pathbuf.to_str().unwrap_or(".");
41+
let resolved_base = if base_str.is_empty() {
42+
base_dir.as_path().to_path_buf()
43+
} else {
44+
base_dir.join(base_str).as_path().clean()
45+
};
46+
Ok(Self { resolved_base, variant: variant.map(Glob::into_owned) })
47+
}
48+
49+
/// Walk the filesystem and yield matching file paths.
50+
#[expect(clippy::disallowed_types, reason = "yields std::path::PathBuf from wax walker")]
51+
pub fn walk(&self) -> Box<dyn Iterator<Item = std::path::PathBuf> + '_> {
52+
match &self.variant {
53+
Some(variant_glob) => Box::new(
54+
variant_glob
55+
.walk(&self.resolved_base)
56+
.filter_map(Result::ok)
57+
.map(wax::walk::Entry::into_path),
58+
),
59+
None => Box::new(std::iter::once(self.resolved_base.clone())),
60+
}
61+
}
5462

55-
/// Check if an absolute path is excluded by any of the resolved negative globs.
56-
#[expect(clippy::disallowed_types, reason = "matching against std::path::Path from wax walker")]
57-
pub fn is_excluded(path: &std::path::Path, negatives: &[ResolvedNegativeGlob]) -> bool {
58-
negatives.iter().any(|neg| {
59-
path.strip_prefix(&neg.resolved_base).ok().is_some_and(|remainder| {
60-
neg.variant.as_ref().map_or(remainder.as_os_str().is_empty(), |v| v.is_match(remainder))
63+
/// Check if an absolute path matches this resolved glob.
64+
#[expect(clippy::disallowed_types, reason = "matching against std::path::Path")]
65+
pub fn matches(&self, path: &std::path::Path) -> bool {
66+
path.strip_prefix(&self.resolved_base).ok().is_some_and(|remainder| {
67+
self.variant
68+
.as_ref()
69+
.map_or(remainder.as_os_str().is_empty(), |v| v.is_match(remainder))
6170
})
62-
})
71+
}
6372
}
6473

6574
/// Compute globbed inputs by walking positive glob patterns and filtering with negative patterns.
@@ -88,7 +97,6 @@ pub fn is_excluded(path: &std::path::Path, negatives: &[ResolvedNegativeGlob]) -
8897
/// )?;
8998
/// // Returns: { "packages/foo/src/index.ts" => 0x1234..., ... }
9099
/// ```
91-
#[expect(clippy::disallowed_types, reason = "path_clean and wax walker return std::path types")]
92100
pub fn compute_globbed_inputs(
93101
base_dir: &AbsolutePath,
94102
workspace_root: &AbsolutePath,
@@ -100,43 +108,24 @@ pub fn compute_globbed_inputs(
100108
return Ok(BTreeMap::new());
101109
}
102110

103-
// Resolve negative globs, normalizing `..` components
104-
let resolved_negatives = resolve_negative_globs(negative_globs, base_dir)?;
111+
let negatives: Vec<ResolvedGlob> = negative_globs
112+
.iter()
113+
.map(|p| ResolvedGlob::new(p.as_str(), base_dir))
114+
.collect::<anyhow::Result<_>>()?;
105115

106116
let mut result = BTreeMap::new();
107117

108-
// Walk each positive glob pattern, resolving `..` via partition + path_clean
109118
for pattern in positive_globs {
110-
let glob = Glob::new(pattern.as_str())?.into_owned();
111-
let (base_pathbuf, variant) = glob.partition();
112-
let base_str = base_pathbuf.to_str().unwrap_or(".");
113-
let resolved_base = if base_str.is_empty() {
114-
base_dir.as_path().to_path_buf()
115-
} else {
116-
base_dir.join(base_str).as_path().clean()
117-
};
118-
119-
let entries: Box<dyn Iterator<Item = std::path::PathBuf>> = match variant {
120-
Some(variant_glob) => Box::new(
121-
variant_glob
122-
.walk(&resolved_base)
123-
.filter_map(Result::ok)
124-
.map(wax::walk::Entry::into_path),
125-
),
126-
None => {
127-
// No wildcard: exact file path
128-
Box::new(std::iter::once(resolved_base))
129-
}
130-
};
119+
let resolved = ResolvedGlob::new(pattern.as_str(), base_dir)?;
131120

132-
for absolute_path in entries {
121+
for absolute_path in resolved.walk() {
133122
// Skip non-files
134123
if !absolute_path.is_file() {
135124
continue;
136125
}
137126

138127
// Apply negative patterns
139-
if is_excluded(&absolute_path, &resolved_negatives) {
128+
if negatives.iter().any(|neg| neg.matches(&absolute_path)) {
140129
continue;
141130
}
142131

crates/vite_task_bin/tests/e2e_snapshots/fixtures/inputs-negative-glob-subpackage/snapshots/dotdot auto negative - miss on non-excluded sibling inferred file.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,6 @@ export const shared = 'initial';
1010
> replace-file-content packages/shared/src/utils.ts initial modified
1111

1212
> vp run sub-pkg#dotdot-auto-negative
13-
~/packages/sub-pkg$ print-file ../shared/src/utils.ts ../shared/dist/output.jscache miss: content of input 'packages/sub-pkg/../shared/src/utils.ts' changed, executing
13+
~/packages/sub-pkg$ print-file ../shared/src/utils.ts ../shared/dist/output.jscache miss: content of input 'packages/shared/src/utils.ts' changed, executing
1414
export const shared = 'modified';
1515
// initial shared output

0 commit comments

Comments
 (0)