From 567468ce72a3c7c1644cb061f9ed0d396118b2fb Mon Sep 17 00:00:00 2001 From: Ivan Smirnov Date: Fri, 11 Jul 2025 16:01:54 +0100 Subject: [PATCH] More efficient cache-key globbing + support parent paths in globs (#13469) ## Summary (Related PR: #13438 - would be nice to have it merged as well since it touches on the same globwalker code) There's a few issues with `cache-key` globs, which this PR attempts to address: - As of the current state, parent or absolute paths are not allowed, which is not obvious and is not documented. E.g., cache-key paths of the form `{file = "../dep/**"}` will be essentially ignored. - Absolute glob patterns also don't work (funnily enough, there's logic in `globwalk` itself that attempts to address it in [`globwalk::glob_builder()`](https://github.com/Gilnaa/globwalk/blob/8973fa2bc560be54c91448131238fa50d56ee121/src/lib.rs#L415), which serves as inspiration to some parts of this PR). - The reason for parent paths being ignored is the way globwalker is currently being triggered in `uv-cache-info`: the base directory is being walked over completely and each entry is then being matched to one of the provided match patterns. - This may also end up being very inefficient if you have a huge root folder with thousands of files: if your match patterns are `a/b/*.rs` and `a/c/*.py` then instead of walking over the root directory, you can just walk over `a/b` and `a/c` and match the relevant patterns there. - Why supporting parent paths may be important to the point of being a blocker: in large codebases with python projects depending on other local non-python projects (e.g. rust crates), cache-keys can be very useful to track dependency on the source code of the latter (e.g. `cache-keys = [{ file = "../../crates/some-dep/**" }]`. - TLDR: parent/absolute cache-key globs don't work, glob walk can be slow. ## Solution - In this PR, user-provided glob patterns are first clustered (LCP-style) into pattern groups with longest common path prefix; each of these groups can then be walked over separately. - Pattern groups do not overlap, so we would never walk over the same directory twice (unless there's symlinks pointing to same folders). - Paths are not canonicalized nor virtually normalized (which is impossible on Unix without FS access), so the method is symlink-safe (i.e. we don't treat `a/b/..` as `a`) and should work fine with #13438. - Because of LCP logic, the minimal amount of directory space will be traversed to cover all patterns. - Absolute glob patterns will now work. - Parent-relative glob patterns will now work. - Glob walking will be more efficient in some cases. ## Possible improvements - Efficiency can be further greatly improved if we limit max depth for globwalk. Currently, a simple ".toml" will deep-traverse the whole folder. Essentially, max depth can be always set to either N or infinity. If a pattern at a pivot node contains `**`, we collect all children nodes from the subtree into the same group and don't limit max depth; otherwise, we set max depth to the length of the glob pattern. This wouldn't change correctness though and can we done separately if needed. - If this is considered important enough, docs can be updated to indicate that parent and absolute globs are supported (and symlinks are resolved, if the relevant PR is algo merged in). ## Test Plan - Glob splitting and clustering tests are included in the PR. - Relative and absolute glob cache-keys were tested in an actual codebase. --- crates/uv-cache-info/src/cache_info.rs | 50 ++-- crates/uv-cache-info/src/glob.rs | 318 +++++++++++++++++++++++++ crates/uv-cache-info/src/lib.rs | 1 + 3 files changed, 347 insertions(+), 22 deletions(-) create mode 100644 crates/uv-cache-info/src/glob.rs diff --git a/crates/uv-cache-info/src/cache_info.rs b/crates/uv-cache-info/src/cache_info.rs index ce98cc513..27a98ab54 100644 --- a/crates/uv-cache-info/src/cache_info.rs +++ b/crates/uv-cache-info/src/cache_info.rs @@ -7,6 +7,7 @@ use serde::Deserialize; use tracing::{debug, warn}; use crate::git_info::{Commit, Tags}; +use crate::glob::cluster_globs; use crate::timestamp::Timestamp; #[derive(Debug, thiserror::Error)] @@ -212,34 +213,39 @@ impl CacheInfo { } } - // If we have any globs, process them in a single pass. + // If we have any globs, first cluster them using LCP and then do a single pass on each group. if !globs.is_empty() { - let walker = globwalk::GlobWalkerBuilder::from_patterns(directory, &globs) + for (glob_base, glob_patterns) in cluster_globs(&globs) { + let walker = globwalk::GlobWalkerBuilder::from_patterns( + directory.join(glob_base), + &glob_patterns, + ) .file_type(globwalk::FileType::FILE | globwalk::FileType::SYMLINK) .build()?; - for entry in walker { - let entry = match entry { - Ok(entry) => entry, - Err(err) => { - warn!("Failed to read glob entry: {err}"); + for entry in walker { + let entry = match entry { + Ok(entry) => entry, + Err(err) => { + warn!("Failed to read glob entry: {err}"); + continue; + } + }; + let metadata = match entry.metadata() { + Ok(metadata) => metadata, + Err(err) => { + warn!("Failed to read metadata for glob entry: {err}"); + continue; + } + }; + if !metadata.is_file() { + warn!( + "Expected file for cache key, but found directory: `{}`", + entry.path().display() + ); continue; } - }; - let metadata = match entry.metadata() { - Ok(metadata) => metadata, - Err(err) => { - warn!("Failed to read metadata for glob entry: {err}"); - continue; - } - }; - if !metadata.is_file() { - warn!( - "Expected file for cache key, but found directory: `{}`", - entry.path().display() - ); - continue; + timestamp = max(timestamp, Some(Timestamp::from_metadata(&metadata))); } - timestamp = max(timestamp, Some(Timestamp::from_metadata(&metadata))); } } diff --git a/crates/uv-cache-info/src/glob.rs b/crates/uv-cache-info/src/glob.rs new file mode 100644 index 000000000..e9c85897f --- /dev/null +++ b/crates/uv-cache-info/src/glob.rs @@ -0,0 +1,318 @@ +use std::{ + collections::BTreeMap, + path::{Component, Components, Path, PathBuf}, +}; + +/// Check if a component of the path looks like it may be a glob pattern. +/// +/// Note: this function is being used when splitting a glob pattern into a long possible +/// base and the glob remainder (scanning through components until we hit the first component +/// for which this function returns true). It is acceptable for this function to return +/// false positives (e.g. patterns like 'foo[bar' or 'foo{bar') in which case correctness +/// will not be affected but efficiency might be (because we'll traverse more than we should), +/// however it should not return false negatives. +fn is_glob_like(part: Component) -> bool { + matches!(part, Component::Normal(_)) + && part.as_os_str().to_str().is_some_and(|part| { + ["*", "{", "}", "?", "[", "]"] + .into_iter() + .any(|c| part.contains(c)) + }) +} + +#[derive(Debug, Default, Clone, PartialEq, Eq)] +struct GlobParts { + base: PathBuf, + pattern: PathBuf, +} + +/// Split a glob into longest possible base + shortest possible glob pattern. +fn split_glob(pattern: impl AsRef) -> GlobParts { + let pattern: &Path = pattern.as_ref().as_ref(); + + let mut glob = GlobParts::default(); + let mut globbing = false; + let mut last = None; + + for part in pattern.components() { + if let Some(last) = last { + if last != Component::CurDir { + if globbing { + glob.pattern.push(last); + } else { + glob.base.push(last); + } + } + } + if !globbing { + globbing = is_glob_like(part); + } + // we don't know if this part is the last one, defer handling it by one iteration + last = Some(part); + } + + if let Some(last) = last { + // defer handling the last component to prevent draining entire pattern into base + if globbing || matches!(last, Component::Normal(_)) { + glob.pattern.push(last); + } else { + glob.base.push(last); + } + } + glob +} + +/// Classic trie with edges being path components and values being glob patterns. +#[derive(Default)] +struct Trie<'a> { + children: BTreeMap, Trie<'a>>, + patterns: Vec<&'a Path>, +} + +impl<'a> Trie<'a> { + fn insert(&mut self, mut components: Components<'a>, pattern: &'a Path) { + if let Some(part) = components.next() { + self.children + .entry(part) + .or_default() + .insert(components, pattern); + } else { + self.patterns.push(pattern); + } + } + + #[allow(clippy::needless_pass_by_value)] + fn collect_patterns( + &self, + pattern_prefix: PathBuf, + group_prefix: PathBuf, + patterns: &mut Vec, + groups: &mut Vec<(PathBuf, Vec)>, + ) { + // collect all patterns beneath and including this node + for pattern in &self.patterns { + patterns.push(pattern_prefix.join(pattern)); + } + for (part, child) in &self.children { + if let Component::Normal(_) = part { + // for normal components, collect all descendant patterns ('normal' edges only) + child.collect_patterns( + pattern_prefix.join(part), + group_prefix.join(part), + patterns, + groups, + ); + } else { + // for non-normal component edges, kick off separate group collection at this node + child.collect_groups(group_prefix.join(part), groups); + } + } + } + + #[allow(clippy::needless_pass_by_value)] + fn collect_groups(&self, prefix: PathBuf, groups: &mut Vec<(PathBuf, Vec)>) { + // LCP-style grouping of patterns + if self.patterns.is_empty() { + // no patterns in this node; child nodes can form independent groups + for (part, child) in &self.children { + child.collect_groups(prefix.join(part), groups); + } + } else { + // pivot point, we've hit a pattern node; we have to stop here and form a group + let mut group = Vec::new(); + self.collect_patterns(PathBuf::new(), prefix.clone(), &mut group, groups); + groups.push((prefix, group)); + } + } +} + +/// Given a collection of globs, cluster them into (base, globs) groups so that: +/// - base doesn't contain any glob symbols +/// - each directory would only be walked at most once +/// - base of each group is the longest common prefix of globs in the group +pub(crate) fn cluster_globs(patterns: &[impl AsRef]) -> Vec<(PathBuf, Vec)> { + // split all globs into base/pattern + let globs: Vec<_> = patterns.iter().map(split_glob).collect(); + + // construct a path trie out of all split globs + let mut trie = Trie::default(); + for glob in &globs { + trie.insert(glob.base.components(), &glob.pattern); + } + + // run LCP-style aggregation of patterns in the trie into groups + let mut groups = Vec::new(); + trie.collect_groups(PathBuf::new(), &mut groups); + + // finally, convert resulting patterns to strings + groups + .into_iter() + .map(|(base, patterns)| { + ( + base, + patterns + .iter() + // NOTE: this unwrap is ok because input patterns are valid utf-8 + .map(|p| p.to_str().unwrap().to_owned()) + .collect(), + ) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::{GlobParts, cluster_globs, split_glob}; + + fn windowsify(path: &str) -> String { + if cfg!(windows) { + path.replace('/', "\\") + } else { + path.to_owned() + } + } + + #[test] + fn test_split_glob() { + #[track_caller] + fn check(input: &str, base: &str, pattern: &str) { + let result = split_glob(input); + let expected = GlobParts { + base: base.into(), + pattern: pattern.into(), + }; + assert_eq!(result, expected, "{input:?} != {base:?} + {pattern:?}"); + } + + check("", "", ""); + check("a", "", "a"); + check("a/b", "a", "b"); + check("a/b/", "a", "b"); + check("a/.//b/", "a", "b"); + check("./a/b/c", "a/b", "c"); + check("c/d/*", "c/d", "*"); + check("c/d/*/../*", "c/d", "*/../*"); + check("a/?b/c", "a", "?b/c"); + check("/a/b/*", "/a/b", "*"); + check("../x/*", "../x", "*"); + check("a/{b,c}/d", "a", "{b,c}/d"); + check("a/[bc]/d", "a", "[bc]/d"); + check("*", "", "*"); + check("*/*", "", "*/*"); + check("..", "..", ""); + check("/", "/", ""); + } + + #[test] + fn test_cluster_globs() { + #[track_caller] + fn check(input: &[&str], expected: &[(&str, &[&str])]) { + let input = input.iter().map(|s| windowsify(s)).collect::>(); + + let mut result_sorted = cluster_globs(&input); + for (_, patterns) in &mut result_sorted { + patterns.sort_unstable(); + } + result_sorted.sort_unstable(); + + let mut expected_sorted = Vec::new(); + for (base, patterns) in expected { + let mut patterns_sorted = Vec::new(); + for pattern in *patterns { + patterns_sorted.push(windowsify(pattern)); + } + patterns_sorted.sort_unstable(); + expected_sorted.push((windowsify(base).into(), patterns_sorted)); + } + expected_sorted.sort_unstable(); + + assert_eq!( + result_sorted, expected_sorted, + "{input:?} != {expected_sorted:?} (got: {result_sorted:?})" + ); + } + + check(&["a/b/*", "a/c/*"], &[("a/b", &["*"]), ("a/c", &["*"])]); + check(&["./a/b/*", "a/c/*"], &[("a/b", &["*"]), ("a/c", &["*"])]); + check(&["/a/b/*", "/a/c/*"], &[("/a/b", &["*"]), ("/a/c", &["*"])]); + check( + &["../a/b/*", "../a/c/*"], + &[("../a/b", &["*"]), ("../a/c", &["*"])], + ); + check(&["x/*", "y/*"], &[("x", &["*"]), ("y", &["*"])]); + check(&[], &[]); + check( + &["./*", "a/*", "../foo/*.png"], + &[("", &["*", "a/*"]), ("../foo", &["*.png"])], + ); + check( + &[ + "?", + "/foo/?", + "/foo/bar/*", + "../bar/*.png", + "../bar/../baz/*.jpg", + ], + &[ + ("", &["?"]), + ("/foo", &["?", "bar/*"]), + ("../bar", &["*.png"]), + ("../bar/../baz", &["*.jpg"]), + ], + ); + check(&["/abs/path/*"], &[("/abs/path", &["*"])]); + check(&["/abs/*", "rel/*"], &[("/abs", &["*"]), ("rel", &["*"])]); + check(&["a/{b,c}/*", "a/d?/*"], &[("a", &["{b,c}/*", "d?/*"])]); + check( + &[ + "../shared/a/[abc].png", + "../shared/a/b/*", + "../shared/b/c/?x/d", + "docs/important/*.{doc,xls}", + "docs/important/very/*", + ], + &[ + ("../shared/a", &["[abc].png", "b/*"]), + ("../shared/b/c", &["?x/d"]), + ("docs/important", &["*.{doc,xls}", "very/*"]), + ], + ); + check(&["file.txt"], &[("", &["file.txt"])]); + check(&["/"], &[("/", &[""])]); + check(&[".."], &[("..", &[""])]); + check( + &["file1.txt", "file2.txt"], + &[("", &["file1.txt", "file2.txt"])], + ); + check( + &["a/file1.txt", "a/file2.txt"], + &[("a", &["file1.txt", "file2.txt"])], + ); + check( + &["*", "a/b/*", "a/../c/*.jpg", "a/../c/*.png", "/a/*", "/b/*"], + &[ + ("", &["*", "a/b/*"]), + ("a/../c", &["*.jpg", "*.png"]), + ("/a", &["*"]), + ("/b", &["*"]), + ], + ); + + if cfg!(windows) { + check( + &[ + r"\\foo\bar\shared/a/[abc].png", + r"\\foo\bar\shared/a/b/*", + r"\\foo\bar/shared/b/c/?x/d", + r"D:\docs\important/*.{doc,xls}", + r"D:\docs/important/very/*", + ], + &[ + (r"\\foo\bar\shared\a", &["[abc].png", r"b\*"]), + (r"\\foo\bar\shared\b\c", &[r"?x\d"]), + (r"D:\docs\important", &["*.{doc,xls}", r"very\*"]), + ], + ); + } + } +} diff --git a/crates/uv-cache-info/src/lib.rs b/crates/uv-cache-info/src/lib.rs index 286411f68..092d40652 100644 --- a/crates/uv-cache-info/src/lib.rs +++ b/crates/uv-cache-info/src/lib.rs @@ -3,4 +3,5 @@ pub use crate::timestamp::*; mod cache_info; mod git_info; +mod glob; mod timestamp;