diff --git a/crates/pet-fs/src/glob.rs b/crates/pet-fs/src/glob.rs index e4b9de1f..41d6fba1 100644 --- a/crates/pet-fs/src/glob.rs +++ b/crates/pet-fs/src/glob.rs @@ -19,6 +19,13 @@ pub fn is_glob_pattern(path: &str) -> bool { path.contains(GLOB_METACHARACTERS) || has_brace_pattern(path) } +/// Returns true when a glob can traverse an unbounded number of path components. +pub fn is_recursive_glob_pattern(path: &str) -> bool { + expand_braces(path) + .iter() + .any(|pattern| pattern.split(['/', '\\']).any(|segment| segment == "**")) +} + /// Checks if a string contains a valid brace expansion pattern `{a,b}`. /// Requires an opening `{`, at least one `,`, and a closing `}`. fn has_brace_pattern(path: &str) -> bool { @@ -202,6 +209,16 @@ mod tests { assert!(is_glob_pattern("*.txt")); } + #[test] + fn test_is_recursive_glob_pattern() { + assert!(is_recursive_glob_pattern("**/.venv")); + assert!(is_recursive_glob_pattern("/home/user/**/venv")); + assert!(!is_recursive_glob_pattern(".venv")); + assert!(!is_recursive_glob_pattern("*/.venv")); + assert!(!is_recursive_glob_pattern("foo**bar/.venv")); + assert!(is_recursive_glob_pattern("C:\\workspace\\**\\.venv")); + assert!(is_recursive_glob_pattern("{foo,**}/.venv")); + } #[test] fn test_is_glob_pattern_with_question_mark() { assert!(is_glob_pattern("/home/user/file?.txt")); diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 489062ce..8ff6cd6b 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -21,7 +21,7 @@ use pet_core::{ Configuration, Locator, RefreshStatePersistence, RefreshStateSyncScope, }; use pet_env_var_path::get_search_paths_from_env_variables; -use pet_fs::glob::expand_glob_patterns; +use pet_fs::glob::{expand_glob_pattern, expand_glob_patterns, is_recursive_glob_pattern}; use pet_fs::path::norm_case; use pet_jsonrpc::{ send_error, send_reply, @@ -567,6 +567,47 @@ pub struct ConfigureOptions { /// The client has a 30-second timeout for configure requests. const GLOB_EXPANSION_WARN_THRESHOLD: Duration = Duration::from_secs(5); +fn expand_configure_directory_patterns(kind: &str, patterns: Vec) -> Vec { + patterns + .into_iter() + .flat_map(|pattern| { + let start = Instant::now(); + let expanded = expand_glob_pattern(&pattern.to_string_lossy()); + let elapsed = start.elapsed(); + trace!( + "Expanded {} pattern '{}' in {:?}", + kind, + pattern.display(), + elapsed + ); + if elapsed >= GLOB_EXPANSION_WARN_THRESHOLD { + warn!( + "Expanding {} pattern '{}' took {:?}, this may cause client timeouts", + kind, + pattern.display(), + elapsed + ); + } + expanded + }) + .filter(|path| path.is_dir()) + .collect() +} + +fn recursive_environment_patterns(patterns: &[PathBuf]) -> impl Iterator { + patterns + .iter() + .filter(|pattern| is_recursive_glob_pattern(&pattern.to_string_lossy())) +} + +fn warn_for_recursive_environment_patterns(patterns: &[PathBuf]) { + for pattern in recursive_environment_patterns(patterns) { + warn!( + "Recursive environmentDirectories pattern '{}' can make configure slow; prefer non-recursive container-directory patterns such as '/envs' or '/*/envs'", + pattern.display() + ); + } +} pub fn handle_configure(context: Arc, id: u32, params: Value) { match serde_json::from_value::(params.clone()) { Ok(mut configure_options) => { @@ -575,38 +616,27 @@ pub fn handle_configure(context: Arc, id: u32, params: Value) { thread::spawn(move || { let now = Instant::now(); + // Warn before any expansion so a slow workspace pattern cannot delay + // the actionable environmentDirectories diagnostic. + if let Some(patterns) = configure_options.environment_directories.as_deref() { + warn_for_recursive_environment_patterns(patterns); + } + // Expand glob patterns before acquiring the write lock so we // don't block readers/writers while traversing the filesystem. let workspace_directories = - configure_options.workspace_directories.take().map(|dirs| { - let start = Instant::now(); - let result: Vec = expand_glob_patterns(&dirs) - .into_iter() - .filter(|p| p.is_dir()) - .collect(); - trace!( - "Expanded workspace directory patterns ({:?}) in {:?}", - dirs, - start.elapsed() - ); - result - }); + configure_options + .workspace_directories + .take() + .map(|patterns| { + expand_configure_directory_patterns("workspaceDirectories", patterns) + }); let environment_directories = configure_options .environment_directories .take() - .map(|dirs| { - let start = Instant::now(); - let result: Vec = expand_glob_patterns(&dirs) - .into_iter() - .filter(|p| p.is_dir()) - .collect(); - trace!( - "Expanded environment directory patterns ({:?}) in {:?}", - dirs, - start.elapsed() - ); - result + .map(|patterns| { + expand_configure_directory_patterns("environmentDirectories", patterns) }); let glob_elapsed = now.elapsed(); trace!("Glob expansion completed in {:?}", glob_elapsed); @@ -1412,6 +1442,42 @@ mod tests { use std::sync::{mpsc, Barrier, Mutex}; use std::thread; + #[test] + fn recursive_environment_pattern_filter_only_returns_recursive_globs() { + let patterns = vec![ + PathBuf::from(".venv"), + PathBuf::from("*/.venv"), + PathBuf::from("**/.venv"), + PathBuf::from("/workspace/**/venv"), + ]; + + assert_eq!( + recursive_environment_patterns(&patterns) + .cloned() + .collect::>(), + vec![ + PathBuf::from("**/.venv"), + PathBuf::from("/workspace/**/venv") + ] + ); + } + + #[test] + fn configure_pattern_expansion_filters_non_directories() { + let temp = tempfile::tempdir().unwrap(); + let directory = temp.path().join("env"); + let file = temp.path().join("python.exe"); + std::fs::create_dir(&directory).unwrap(); + std::fs::write(&file, "").unwrap(); + + assert_eq!( + expand_configure_directory_patterns( + "environmentDirectories", + vec![temp.path().join("*")], + ), + vec![directory] + ); + } #[derive(Default)] struct RecordingReporter { environments: Mutex>, diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index fca5a403..ddc84f80 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -72,9 +72,9 @@ interface ConfigureParams { * E.g. `workspace folders` in vscode. * * If not provided, then environments such as poetry, pipenv, and the like will not be reported. - * This is because poetry, pipenv, and the like are project specific enviornents. + * This is because poetry, pipenv, and the like are project-specific environments. * - * Glob patterns are supported (e.g., "/home/user/projects/*", "**/.venv"). + * Glob patterns are supported (e.g., `/home/user/projects/*`). Avoid recursive `**` patterns when a single-level pattern is sufficient. */ workspaceDirectories?: string[]; /** @@ -83,7 +83,9 @@ interface ConfigureParams { * * Useful for VS Code so users can configure where they store virtual environments. * - * Glob patterns are supported (e.g., "/home/user/envs/*", "/home/user/*/venv"). + * Values identify directories that contain environments. Glob patterns are supported (e.g., `/envs`, `/*/envs`). + * Avoid recursive patterns such as `/**/envs`: they can traverse large directory trees, + * delay configure responses, and trigger client timeouts. */ environmentDirectories?: string[]; /**