Skip to content

Commit cc8f859

Browse files
feat: improve recursive environment glob diagnostics (Fixes #427) (#495)
## Summary - warn immediately when `environmentDirectories` contains recursive `**` path segments - report exact pattern names and elapsed time for slow configure glob expansions - recommend non-recursive container-directory patterns in the JSON-RPC documentation - support recursive globstar detection through brace expansion without flagging `foo**bar` - add direct coverage for classification and directory-only expansion ## Validation - `cargo test -p pet-fs` - `cargo test -p pet configure` - `.\scripts\rust-precommit.ps1` Fixes #427 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 09609a4 commit cc8f859

3 files changed

Lines changed: 114 additions & 29 deletions

File tree

crates/pet-fs/src/glob.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ pub fn is_glob_pattern(path: &str) -> bool {
1919
path.contains(GLOB_METACHARACTERS) || has_brace_pattern(path)
2020
}
2121

22+
/// Returns true when a glob can traverse an unbounded number of path components.
23+
pub fn is_recursive_glob_pattern(path: &str) -> bool {
24+
expand_braces(path)
25+
.iter()
26+
.any(|pattern| pattern.split(['/', '\\']).any(|segment| segment == "**"))
27+
}
28+
2229
/// Checks if a string contains a valid brace expansion pattern `{a,b}`.
2330
/// Requires an opening `{`, at least one `,`, and a closing `}`.
2431
fn has_brace_pattern(path: &str) -> bool {
@@ -202,6 +209,16 @@ mod tests {
202209
assert!(is_glob_pattern("*.txt"));
203210
}
204211

212+
#[test]
213+
fn test_is_recursive_glob_pattern() {
214+
assert!(is_recursive_glob_pattern("**/.venv"));
215+
assert!(is_recursive_glob_pattern("/home/user/**/venv"));
216+
assert!(!is_recursive_glob_pattern(".venv"));
217+
assert!(!is_recursive_glob_pattern("*/.venv"));
218+
assert!(!is_recursive_glob_pattern("foo**bar/.venv"));
219+
assert!(is_recursive_glob_pattern("C:\\workspace\\**\\.venv"));
220+
assert!(is_recursive_glob_pattern("{foo,**}/.venv"));
221+
}
205222
#[test]
206223
fn test_is_glob_pattern_with_question_mark() {
207224
assert!(is_glob_pattern("/home/user/file?.txt"));

crates/pet/src/jsonrpc.rs

Lines changed: 92 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use pet_core::{
2121
Configuration, Locator, RefreshStatePersistence, RefreshStateSyncScope,
2222
};
2323
use pet_env_var_path::get_search_paths_from_env_variables;
24-
use pet_fs::glob::expand_glob_patterns;
24+
use pet_fs::glob::{expand_glob_pattern, expand_glob_patterns, is_recursive_glob_pattern};
2525
use pet_fs::path::norm_case;
2626
use pet_jsonrpc::{
2727
send_error, send_reply,
@@ -567,6 +567,47 @@ pub struct ConfigureOptions {
567567
/// The client has a 30-second timeout for configure requests.
568568
const GLOB_EXPANSION_WARN_THRESHOLD: Duration = Duration::from_secs(5);
569569

570+
fn expand_configure_directory_patterns(kind: &str, patterns: Vec<PathBuf>) -> Vec<PathBuf> {
571+
patterns
572+
.into_iter()
573+
.flat_map(|pattern| {
574+
let start = Instant::now();
575+
let expanded = expand_glob_pattern(&pattern.to_string_lossy());
576+
let elapsed = start.elapsed();
577+
trace!(
578+
"Expanded {} pattern '{}' in {:?}",
579+
kind,
580+
pattern.display(),
581+
elapsed
582+
);
583+
if elapsed >= GLOB_EXPANSION_WARN_THRESHOLD {
584+
warn!(
585+
"Expanding {} pattern '{}' took {:?}, this may cause client timeouts",
586+
kind,
587+
pattern.display(),
588+
elapsed
589+
);
590+
}
591+
expanded
592+
})
593+
.filter(|path| path.is_dir())
594+
.collect()
595+
}
596+
597+
fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator<Item = &PathBuf> {
598+
patterns
599+
.iter()
600+
.filter(|pattern| is_recursive_glob_pattern(&pattern.to_string_lossy()))
601+
}
602+
603+
fn warn_for_recursive_environment_patterns(patterns: &[PathBuf]) {
604+
for pattern in recursive_environment_patterns(patterns) {
605+
warn!(
606+
"Recursive environmentDirectories pattern '{}' can make configure slow; prefer non-recursive container-directory patterns such as '<root>/envs' or '<root>/*/envs'",
607+
pattern.display()
608+
);
609+
}
610+
}
570611
pub fn handle_configure(context: Arc<Context>, id: u32, params: Value) {
571612
match serde_json::from_value::<ConfigureOptions>(params.clone()) {
572613
Ok(mut configure_options) => {
@@ -575,38 +616,27 @@ pub fn handle_configure(context: Arc<Context>, id: u32, params: Value) {
575616
thread::spawn(move || {
576617
let now = Instant::now();
577618

619+
// Warn before any expansion so a slow workspace pattern cannot delay
620+
// the actionable environmentDirectories diagnostic.
621+
if let Some(patterns) = configure_options.environment_directories.as_deref() {
622+
warn_for_recursive_environment_patterns(patterns);
623+
}
624+
578625
// Expand glob patterns before acquiring the write lock so we
579626
// don't block readers/writers while traversing the filesystem.
580627
let workspace_directories =
581-
configure_options.workspace_directories.take().map(|dirs| {
582-
let start = Instant::now();
583-
let result: Vec<PathBuf> = expand_glob_patterns(&dirs)
584-
.into_iter()
585-
.filter(|p| p.is_dir())
586-
.collect();
587-
trace!(
588-
"Expanded workspace directory patterns ({:?}) in {:?}",
589-
dirs,
590-
start.elapsed()
591-
);
592-
result
593-
});
628+
configure_options
629+
.workspace_directories
630+
.take()
631+
.map(|patterns| {
632+
expand_configure_directory_patterns("workspaceDirectories", patterns)
633+
});
594634
let environment_directories =
595635
configure_options
596636
.environment_directories
597637
.take()
598-
.map(|dirs| {
599-
let start = Instant::now();
600-
let result: Vec<PathBuf> = expand_glob_patterns(&dirs)
601-
.into_iter()
602-
.filter(|p| p.is_dir())
603-
.collect();
604-
trace!(
605-
"Expanded environment directory patterns ({:?}) in {:?}",
606-
dirs,
607-
start.elapsed()
608-
);
609-
result
638+
.map(|patterns| {
639+
expand_configure_directory_patterns("environmentDirectories", patterns)
610640
});
611641
let glob_elapsed = now.elapsed();
612642
trace!("Glob expansion completed in {:?}", glob_elapsed);
@@ -1412,6 +1442,42 @@ mod tests {
14121442
use std::sync::{mpsc, Barrier, Mutex};
14131443
use std::thread;
14141444

1445+
#[test]
1446+
fn recursive_environment_pattern_filter_only_returns_recursive_globs() {
1447+
let patterns = vec![
1448+
PathBuf::from(".venv"),
1449+
PathBuf::from("*/.venv"),
1450+
PathBuf::from("**/.venv"),
1451+
PathBuf::from("/workspace/**/venv"),
1452+
];
1453+
1454+
assert_eq!(
1455+
recursive_environment_patterns(&patterns)
1456+
.cloned()
1457+
.collect::<Vec<_>>(),
1458+
vec![
1459+
PathBuf::from("**/.venv"),
1460+
PathBuf::from("/workspace/**/venv")
1461+
]
1462+
);
1463+
}
1464+
1465+
#[test]
1466+
fn configure_pattern_expansion_filters_non_directories() {
1467+
let temp = tempfile::tempdir().unwrap();
1468+
let directory = temp.path().join("env");
1469+
let file = temp.path().join("python.exe");
1470+
std::fs::create_dir(&directory).unwrap();
1471+
std::fs::write(&file, "").unwrap();
1472+
1473+
assert_eq!(
1474+
expand_configure_directory_patterns(
1475+
"environmentDirectories",
1476+
vec![temp.path().join("*")],
1477+
),
1478+
vec![directory]
1479+
);
1480+
}
14151481
#[derive(Default)]
14161482
struct RecordingReporter {
14171483
environments: Mutex<Vec<PythonEnvironment>>,

docs/JSONRPC.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ interface ConfigureParams {
7272
* E.g. `workspace folders` in vscode.
7373
*
7474
* If not provided, then environments such as poetry, pipenv, and the like will not be reported.
75-
* This is because poetry, pipenv, and the like are project specific enviornents.
75+
* This is because poetry, pipenv, and the like are project-specific environments.
7676
*
77-
* Glob patterns are supported (e.g., "/home/user/projects/*", "**/.venv").
77+
* Glob patterns are supported (e.g., `/home/user/projects/*`). Avoid recursive `**` patterns when a single-level pattern is sufficient.
7878
*/
7979
workspaceDirectories?: string[];
8080
/**
@@ -83,7 +83,9 @@ interface ConfigureParams {
8383
*
8484
* Useful for VS Code so users can configure where they store virtual environments.
8585
*
86-
* Glob patterns are supported (e.g., "/home/user/envs/*", "/home/user/*/venv").
86+
* Values identify directories that contain environments. Glob patterns are supported (e.g., `<root>/envs`, `<root>/*/envs`).
87+
* Avoid recursive patterns such as `<root>/**/envs`: they can traverse large directory trees,
88+
* delay configure responses, and trigger client timeouts.
8789
*/
8890
environmentDirectories?: string[];
8991
/**

0 commit comments

Comments
 (0)