diff --git a/crates/prek/src/cli/run/filter.rs b/crates/prek/src/cli/run/filter.rs index ebe743357..eff63b2bf 100644 --- a/crates/prek/src/cli/run/filter.rs +++ b/crates/prek/src/cli/run/filter.rs @@ -1,12 +1,14 @@ use std::cell::OnceCell; +use std::ffi::OsStr; use std::ops::ControlFlow; use std::path::{Path, PathBuf}; +use std::sync::Arc; use anyhow::{Context, Result}; use itertools::{Either, Itertools}; use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use prek_identify::{TagSet, tags_from_path}; -use rustc_hash::FxHashSet; +use rustc_hash::{FxHashMap, FxHashSet}; use tracing::{debug, error, instrument}; use crate::config::{FilePattern, Stage}; @@ -145,16 +147,12 @@ impl<'a> ProjectFile<'a> { #[derive(Default)] pub(crate) struct FileTagCache<'a> { - paths: Vec<&'a Path>, + paths: &'a [PathBuf], tags_by_file: Vec>>, } impl<'a> FileTagCache<'a> { - pub(crate) fn from_paths(paths: I) -> Self - where - I: IntoIterator, - { - let paths = paths.into_iter().collect::>(); + pub(crate) fn from_paths(paths: &'a [PathBuf]) -> Self { let tags_by_file = (0..paths.len()).map(|_| OnceCell::new()).collect(); Self { paths, @@ -165,7 +163,7 @@ impl<'a> FileTagCache<'a> { pub(crate) fn tags(&self, file_idx: usize) -> Option<&TagSet> { self.tags_by_file[file_idx] .get_or_init(|| { - let path = self.paths[file_idx]; + let path = &self.paths[file_idx]; match tags_from_path(path) { Ok(tags) => Some(tags), Err(err) => { @@ -183,61 +181,20 @@ pub(crate) struct ProjectFiles<'a> { } impl<'a> ProjectFiles<'a> { - /// Create project-owned files after applying the project's relative path and include/exclude patterns. - /// `filenames` are paths relative to the workspace root. - pub(crate) fn for_project( - filenames: I, - project: &Project, - consumed_files: Option<&FxHashSet<&'a Path>>, - newly_consumed_files: Option<&mut FxHashSet<&'a Path>>, - ) -> Self - where - I: Iterator + Send, - { - let relative_path = project.relative_path(); - let files_capacity = if relative_path.as_os_str().is_empty() { - filenames.size_hint().0 - } else { - 0 - }; - let mut files = Vec::with_capacity(files_capacity); - Self::visit_for_project( - filenames, - project, - consumed_files, - newly_consumed_files, - |file| { - files.push(file); - ControlFlow::Continue(()) - }, - ); - - Self { files } + fn with_capacity(capacity: usize) -> Self { + Self { + files: Vec::with_capacity(capacity), + } } - /// Mark files owned by this project without collecting or visiting them. - pub(crate) fn consume_for_project( - filenames: I, - project: &Project, - consumed_files: Option<&FxHashSet<&'a Path>>, - newly_consumed_files: &mut FxHashSet<&'a Path>, - ) where - I: Iterator + Send, - { - Self::visit_for_project( - filenames, - project, - consumed_files, - Some(newly_consumed_files), - |_| ControlFlow::Continue(()), - ); + fn push(&mut self, file_idx: usize, hook_path: &'a Path) { + self.files.push(ProjectFile::new(file_idx, hook_path)); } /// Visit project-owned files without collecting them. /// - /// This shares the same ownership, orphan-project, and project-level filtering rules as - /// `for_project`, but lets callers that only need a boolean result avoid allocating a - /// `Vec`. Return [`ControlFlow::Break`] from `visit` to stop calling the visitor. + /// This applies the same ownership, orphan-project, and project-level filtering rules as the + /// run file index. Return [`ControlFlow::Break`] from `visit` to stop calling the visitor. /// Orphan projects still finish marking owned files as consumed before returning. pub(crate) fn visit_for_project( filenames: I, @@ -309,6 +266,10 @@ impl<'a> ProjectFiles<'a> { self.files.len() } + pub(crate) fn iter(&self) -> impl Iterator> { + self.files.iter() + } + /// Filter filenames by file patterns and tags for a specific hook. #[instrument(level = "trace", skip_all, fields(hook = ?hook.id))] pub(crate) fn matching_filenames( @@ -338,6 +299,127 @@ impl<'a> ProjectFiles<'a> { } } +#[derive(Default)] +struct ProjectPathNode<'a> { + project_idx: Option, + children: FxHashMap<&'a OsStr, ProjectPathNode<'a>>, +} + +impl<'a> ProjectPathNode<'a> { + fn insert(&mut self, path: &'a Path, project_idx: usize) { + let mut node = self; + for component in path.components() { + node = node.children.entry(component.as_os_str()).or_default(); + } + let previous = node.project_idx.replace(project_idx); + debug_assert!(previous.is_none()); + } + + fn matching_projects(&self, path: &Path, matches: &mut Vec) { + matches.clear(); + + let mut node = self; + if let Some(project_idx) = node.project_idx { + matches.push(project_idx); + } + if node.children.is_empty() { + return; + } + for component in path.components() { + let Some(child) = node.children.get(component.as_os_str()) else { + break; + }; + node = child; + if let Some(project_idx) = node.project_idx { + matches.push(project_idx); + } + if node.children.is_empty() { + break; + } + } + } +} + +/// Project-relative views of the run input, built once and shared by hook setup and execution. +pub(crate) struct RunFileIndex<'a> { + projects: Vec>, + tag_cache: FileTagCache<'a>, +} + +impl<'a> RunFileIndex<'a> { + pub(crate) fn new(input: &'a RunInput, projects: &[Arc]) -> Self { + let RunInput::Files(filenames) = input else { + return Self { + projects: Vec::new(), + tag_cache: FileTagCache::default(), + }; + }; + + debug_assert!( + projects + .iter() + .enumerate() + .all(|(idx, project)| project.idx() == idx), + "workspace projects must be indexed in storage order" + ); + + let mut project_tree = ProjectPathNode::default(); + let project_filters = projects + .iter() + .map(|project| { + project_tree.insert(project.relative_path(), project.idx()); + FilenameFilter::new( + project.config().files.as_ref(), + project.config().exclude.as_ref(), + ) + }) + .collect::>(); + let mut project_files = projects + .iter() + .map(|project| { + ProjectFiles::with_capacity(if project.is_root() { + filenames.len() + } else { + 0 + }) + }) + .collect::>(); + + let mut matching_projects = Vec::new(); + for (file_idx, filename) in filenames.iter().enumerate() { + project_tree.matching_projects(filename, &mut matching_projects); + + // The tree yields ancestors from root to leaf. Apply ownership from the most + // specific project upwards, stopping once an orphan project consumes the file. + for &project_idx in matching_projects.iter().rev() { + let project = &projects[project_idx]; + let hook_path = filename + .strip_prefix(project.relative_path()) + .expect("matched project path must be a file prefix"); + if project_filters[project_idx].matches(hook_path) { + project_files[project_idx].push(file_idx, hook_path); + } + if project.config().orphan.unwrap_or(false) { + break; + } + } + } + + Self { + projects: project_files, + tag_cache: FileTagCache::from_paths(filenames), + } + } + + pub(crate) fn project_files(&self, project: &Project) -> &ProjectFiles<'a> { + &self.projects[project.idx()] + } + + pub(crate) fn tag_cache(&self) -> &FileTagCache<'a> { + &self.tag_cache + } +} + #[derive(Default)] pub(crate) struct CollectOptions { pub(crate) input_mode: RunInputMode, @@ -625,4 +707,36 @@ mod tests { assert!(!filter.matches(path)); } + + #[test] + fn project_path_tree_matches_nested_ancestors() { + let project_paths = [ + PathBuf::new(), + PathBuf::from("src"), + PathBuf::from("src/backend"), + ]; + let mut tree = ProjectPathNode::default(); + for (idx, path) in project_paths.iter().enumerate() { + tree.insert(path, idx); + } + + let mut matches = Vec::new(); + tree.matching_projects(Path::new("src/backend/lib.rs"), &mut matches); + + assert_eq!(matches, vec![0, 1, 2]); + } + + #[test] + fn project_path_tree_respects_component_boundaries() { + let project_paths = [PathBuf::new(), PathBuf::from("src")]; + let mut tree = ProjectPathNode::default(); + for (idx, path) in project_paths.iter().enumerate() { + tree.insert(path, idx); + } + + let mut matches = Vec::new(); + tree.matching_projects(Path::new("src-tools/main.rs"), &mut matches); + + assert_eq!(matches, vec![0]); + } } diff --git a/crates/prek/src/cli/run/mod.rs b/crates/prek/src/cli/run/mod.rs index b5179da88..20567b900 100644 --- a/crates/prek/src/cli/run/mod.rs +++ b/crates/prek/src/cli/run/mod.rs @@ -1,6 +1,6 @@ pub(crate) use filter::{ - CollectOptions, FileTagCache, FileTagFilter, HookFileFilter, ProjectFiles, RunInput, - collect_run_input, + CollectOptions, FileTagCache, FileTagFilter, HookFileFilter, ProjectFiles, RunFileIndex, + RunInput, collect_run_input, }; pub(crate) use install::{InstallCache, install_hooks}; pub(crate) use reporter::{HookRunReporter, project_status_marker}; diff --git a/crates/prek/src/cli/run/run.rs b/crates/prek/src/cli/run/run.rs index d1f09669f..55419ed95 100644 --- a/crates/prek/src/cli/run/run.rs +++ b/crates/prek/src/cli/run/run.rs @@ -1,6 +1,5 @@ use std::fmt::Write as _; use std::io::Write as _; -use std::ops::ControlFlow; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::sync::{Arc, LazyLock}; @@ -12,7 +11,7 @@ use owo_colors::OwoColorize; use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use prek_consts::{PRE_COMMIT_CONFIG_YAML, PREK_TOML}; use prek_identify::{TagSet, tags_from_path}; -use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; +use rustc_hash::{FxBuildHasher, FxHashMap}; use tracing::{debug, error, trace}; use unicode_width::UnicodeWidthStr; @@ -23,7 +22,7 @@ use crate::cli::run::install::{InstallCache, install_hooks}; use crate::cli::run::keeper::WorkTreeKeeper; use crate::cli::run::{ CollectOptions, FileTagCache, GroupFilters, HookFileFilter, HookRunReporter, ProjectFiles, - RunInput, Selectors, collect_run_input, project_status_marker, + RunFileIndex, RunInput, Selectors, collect_run_input, project_status_marker, }; use crate::cli::{ExitStatus, RunExtraArgs}; use crate::config::{PassFilenames, Stage}; @@ -210,17 +209,13 @@ pub(crate) async fn run( ) })?; - let tag_cache = if let RunInput::Files(files) = &input { - FileTagCache::from_paths(files.iter().map(PathBuf::as_path)) - } else { - FileTagCache::default() - }; + let file_index = RunFileIndex::new(&input, workspace.all_projects()); let installed_hooks = ensure_hooks_installed( store, printer, &workspace, &input, - &tag_cache, + &file_index, &filtered_hooks, ) .await?; @@ -228,7 +223,7 @@ pub(crate) async fn run( run_hooks( &workspace, &input, - &tag_cache, + &file_index, &installed_hooks, store, show_diff_on_failure, @@ -332,7 +327,7 @@ async fn ensure_hooks_installed<'paths>( printer: Printer, workspace: &Workspace, input: &'paths RunInput, - tag_cache: &FileTagCache<'paths>, + file_index: &RunFileIndex<'paths>, hooks: &[Arc], ) -> Result> { let env_hooks = hooks @@ -364,7 +359,7 @@ async fn ensure_hooks_installed<'paths>( } let hooks_to_install = - select_hooks_to_install(workspace, input, tag_cache, &missing_env_hooks)?; + select_hooks_to_install(workspace, input, file_index, &missing_env_hooks)?; if !hooks_to_install.is_empty() { let reporter = HookInstallReporter::new(printer); let installed_hooks = @@ -394,7 +389,7 @@ async fn ensure_hooks_installed<'paths>( fn select_hooks_to_install<'paths>( workspace: &Workspace, input: &'paths RunInput, - tag_cache: &FileTagCache<'paths>, + file_index: &RunFileIndex<'paths>, hooks: &[Arc], ) -> Result>> { #[allow(clippy::mutable_key_type)] @@ -407,72 +402,28 @@ fn select_hooks_to_install<'paths>( .push(hook.clone()); } - let mut hooks_to_install = Vec::new(); - let mut consumed_files = FxHashSet::default(); + let mut hooks_to_install = Vec::with_capacity(hooks.len()); + let tag_cache = file_index.tag_cache(); for project in workspace.all_projects() { match input { - RunInput::Files(files) => { - let mut project_consumed_files = FxHashSet::default(); - let Some(hooks) = project_to_hooks.remove(project.as_ref()) else { - ProjectFiles::consume_for_project( - files.iter(), - project, - Some(&consumed_files), - &mut project_consumed_files, - ); - consumed_files.extend(project_consumed_files); + RunInput::Files(_) => { + let Some(mut hooks) = project_to_hooks.remove(project.as_ref()) else { continue; }; - let mut candidates = hooks - .into_iter() - .map(|hook| { - let matches = hook.always_run; - (hook, matches) - }) - .collect::>(); - let mut remaining = candidates.iter().filter(|(_, matches)| !*matches).count(); - - ProjectFiles::visit_for_project( - files.iter(), - project, - Some(&consumed_files), - Some(&mut project_consumed_files), - |project_file| { - for (hook, matches) in &mut candidates { - if *matches { - continue; - } - - let hook_filter = HookFileFilter::new(hook); - if hook_filter.matches_project_file(&project_file, tag_cache) { - *matches = true; - remaining -= 1; - } - } - - if remaining == 0 { - return ControlFlow::Break(()); - } - - ControlFlow::Continue(()) - }, - ); - consumed_files.extend(project_consumed_files); - - for (hook, matches) in candidates { - if matches { - hooks_to_install.push(hook); - } - } + let project_files = file_index.project_files(project); + hooks.retain(|hook| { + hook.always_run || project_files.has_matching_file(hook, tag_cache) + }); + hooks_to_install.extend(hooks); } RunInput::MessageFile(_) => { let Some(hooks) = project_to_hooks.remove(project.as_ref()) else { continue; }; - let project_input = ProjectHookInput::new(input, project, None, None)?; + let project_input = ProjectHookInput::new(input, project, file_index)?; for hook in hooks { if hook.always_run || project_input.matches_hook(&hook, tag_cache) { hooks_to_install.push(hook); @@ -494,7 +445,7 @@ fn hook_key(hook: &Hook) -> (usize, usize) { async fn run_hooks<'paths>( workspace: &Workspace, input: &'paths RunInput, - tag_cache: &FileTagCache<'paths>, + file_index: &RunFileIndex<'paths>, hooks: &[InstalledHook], store: &Store, show_diff_on_failure: bool, @@ -527,23 +478,13 @@ async fn run_hooks<'paths>( show_project_headers, printer, ); - let mut consumed_files = FxHashSet::default(); for projects in ProjectDepthGroups::new(workspace.all_projects()) { let clean_baseline = worktree_cleaned && !session.file_modified; - let mut level_consumed_files = FxHashSet::default(); let mut project_runs = Vec::new(); for project in projects { let Some(mut hooks) = project_to_hooks.remove(project.as_ref()) else { - if let RunInput::Files(files) = input { - ProjectFiles::consume_for_project( - files.iter(), - project, - Some(&consumed_files), - &mut level_consumed_files, - ); - } continue; }; @@ -560,24 +501,19 @@ async fn run_hooks<'paths>( }); } + if project_runs.is_empty() { + continue; + } + let project_results = session - .run_project_level( - project_runs, - input, - &consumed_files, - tag_cache, - clean_baseline, - ) + .run_project_level(project_runs, input, file_index, clean_baseline) .await?; let mut stop_after_level = false; for project_result in project_results { - level_consumed_files.extend(project_result.consumed_files.iter().copied()); stop_after_level |= session.finish_project_run(project_result, show_project_headers)?; } - consumed_files.extend(level_consumed_files); - if stop_after_level { break; } @@ -623,14 +559,13 @@ struct ProjectRun<'project> { groups: Vec>, } -struct ProjectRunResult<'project, 'paths> { +struct ProjectRunResult<'project> { project: &'project Project, groups: Vec, - consumed_files: FxHashSet<&'paths Path>, stop_after_level: bool, } -impl ProjectRunResult<'_, '_> { +impl ProjectRunResult<'_> { fn failed(&self) -> bool { self.groups.iter().any(ProjectGroupRunResult::failed) } @@ -725,10 +660,9 @@ impl<'a> HookRunSession<'a> { &self, project_runs: Vec>, input: &'paths RunInput, - consumed_files: &FxHashSet<&'paths Path>, - tag_cache: &FileTagCache<'paths>, + file_index: &RunFileIndex<'paths>, clean_baseline: bool, - ) -> Result>> { + ) -> Result>> { let semaphore = Rc::new(Semaphore::new(*HOOK_CONCURRENCY)); let mut runs = FuturesUnordered::new(); for (idx, project_run) in project_runs.into_iter().enumerate() { @@ -736,14 +670,7 @@ impl<'a> HookRunSession<'a> { runs.push(async move { let project = project_run.project; let result = self - .run_project( - project_run, - input, - consumed_files, - tag_cache, - clean_baseline, - semaphore, - ) + .run_project(project_run, input, file_index, clean_baseline, semaphore) .await; if let Ok(result) = &result { self.reporter.on_project_complete(project, result.failed()); @@ -765,18 +692,11 @@ impl<'a> HookRunSession<'a> { &self, project_run: ProjectRun<'project>, input: &'paths RunInput, - consumed_files: &FxHashSet<&'paths Path>, - tag_cache: &FileTagCache<'paths>, + file_index: &RunFileIndex<'paths>, clean_baseline: bool, semaphore: Rc, - ) -> Result> { - let mut project_consumed_files = FxHashSet::default(); - let project_input = ProjectHookInput::new( - input, - project_run.project, - Some(consumed_files), - Some(&mut project_consumed_files), - )?; + ) -> Result> { + let project_input = ProjectHookInput::new(input, project_run.project, file_index)?; trace!( "Files for project `{}` after filtered: {}", project_run.project, @@ -806,7 +726,7 @@ impl<'a> HookRunSession<'a> { .run_priority_group( group_hooks, &project_input, - tag_cache, + file_index.tag_cache(), Rc::clone(&semaphore), ) .await?; @@ -833,7 +753,6 @@ impl<'a> HookRunSession<'a> { Ok(ProjectRunResult { project: project_run.project, groups, - consumed_files: project_consumed_files, stop_after_level, }) } @@ -841,7 +760,7 @@ impl<'a> HookRunSession<'a> { async fn run_priority_group( &self, group_hooks: Vec, - project_input: &ProjectHookInput<'_>, + project_input: &ProjectHookInput<'_, '_>, tag_cache: &FileTagCache<'_>, semaphore: Rc, ) -> Result> { @@ -889,7 +808,7 @@ impl<'a> HookRunSession<'a> { fn finish_project_run( &mut self, - project_result: ProjectRunResult<'_, '_>, + project_result: ProjectRunResult<'_>, show_project_headers: bool, ) -> Result { self.render_project_header( @@ -1144,28 +1063,22 @@ impl Iterator for PriorityGroups { } } -enum ProjectHookInput<'a> { - Files(ProjectFiles<'a>), +enum ProjectHookInput<'index, 'paths> { + Files(&'index ProjectFiles<'paths>), MessageFile { hook_arg: PathBuf, tags: Option, }, } -impl<'a> ProjectHookInput<'a> { +impl<'index, 'paths> ProjectHookInput<'index, 'paths> { fn new( - input: &'a RunInput, + input: &'paths RunInput, project: &Project, - consumed_files: Option<&FxHashSet<&'a Path>>, - newly_consumed_files: Option<&mut FxHashSet<&'a Path>>, + file_index: &'index RunFileIndex<'paths>, ) -> Result { match input { - RunInput::Files(files) => Ok(Self::Files(ProjectFiles::for_project( - files.iter(), - project, - consumed_files, - newly_consumed_files, - ))), + RunInput::Files(_) => Ok(Self::Files(file_index.project_files(project))), RunInput::MessageFile(path) => { let tags = match tags_from_path(path) { Ok(tags) => Some(tags), @@ -1189,7 +1102,11 @@ impl<'a> ProjectHookInput<'a> { } } - fn run_input_for_hook(&self, hook: &Hook, tag_cache: &FileTagCache<'a>) -> HookRunInput<'a> { + fn run_input_for_hook( + &self, + hook: &Hook, + tag_cache: &FileTagCache<'paths>, + ) -> HookRunInput<'paths> { match self { Self::Files(project_files) => match hook.pass_filenames { // Always-run hooks without filename arguments run regardless of file matches. @@ -1216,7 +1133,7 @@ impl<'a> ProjectHookInput<'a> { } } - fn matches_hook(&self, hook: &Hook, tag_cache: &FileTagCache<'a>) -> bool { + fn matches_hook(&self, hook: &Hook, tag_cache: &FileTagCache<'paths>) -> bool { match self { Self::Files(project_files) => project_files.has_matching_file(hook, tag_cache), Self::MessageFile { hook_arg, tags } => { @@ -1398,7 +1315,7 @@ impl RunResult { async fn run_hook( hook: InstalledHook, - project_input: &ProjectHookInput<'_>, + project_input: &ProjectHookInput<'_, '_>, tag_cache: &FileTagCache<'_>, store: &Store, dry_run: bool, diff --git a/crates/prek/src/hooks/meta_hooks.rs b/crates/prek/src/hooks/meta_hooks.rs index b6b3953e3..4f0dae438 100644 --- a/crates/prek/src/hooks/meta_hooks.rs +++ b/crates/prek/src/hooks/meta_hooks.rs @@ -1,6 +1,6 @@ use std::io::Write; use std::ops::ControlFlow; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::str::FromStr; use anyhow::{Context, Result}; @@ -124,7 +124,7 @@ pub(crate) async fn check_hooks_apply( let mut code = 0; let mut output = Vec::new(); - let tag_cache = FileTagCache::from_paths(input.iter().map(PathBuf::as_path)); + let tag_cache = FileTagCache::from_paths(&input); for project in projects { let project_hooks = project @@ -271,7 +271,7 @@ pub(crate) async fn check_useless_excludes( let mut code = 0; let mut output = Vec::new(); - let tag_cache = FileTagCache::from_paths(input_workspace.iter().map(PathBuf::as_path)); + let tag_cache = FileTagCache::from_paths(&input_workspace); for project in projects { let config = project.config();