diff --git a/Cargo.lock b/Cargo.lock index f97d18a..4a62460 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,7 +179,7 @@ checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" [[package]] name = "codeowners" -version = "0.2.5" +version = "0.2.6" dependencies = [ "assert_cmd", "clap", diff --git a/Cargo.toml b/Cargo.toml index f61c3da..3637047 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.2.5" +version = "0.2.6" edition = "2024" [profile.release] diff --git a/dev/run_benchmarks_for_file.sh b/dev/run_benchmarks_for_file.sh new file mode 100755 index 0000000..bb60317 --- /dev/null +++ b/dev/run_benchmarks_for_file.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +mkdir -p tmp + +# Check if the file exists before removing it +if [ -f "tmp/codeowners_for_file_benchmarks.md" ]; then + rm tmp/codeowners_for_file_benchmarks.md +fi + +echo "To run these benchmarks on your application, you can place this repo next to your rails application and run /usr/bin/env bash ../rubyatscale/codeowners-rs/dev/run_benchmarks_for_file.sh " >> tmp/codeowners_for_file_benchmarks.md + +hyperfine --warmup=2 --runs=3 --export-markdown tmp/codeowners_for_file_benchmarks.md \ + "../rubyatscale/codeowners-rs/target/release/codeowners for-file \"$1\"" \ + "bin/codeowners for_file \"$1\"" \ + "bin/codeownership for_file \"$1\"" \ No newline at end of file diff --git a/dev/run_benchmarks.sh b/dev/run_benchmarks_for_gv.sh old mode 100644 new mode 100755 similarity index 59% rename from dev/run_benchmarks.sh rename to dev/run_benchmarks_for_gv.sh index 106cf1f..3892238 --- a/dev/run_benchmarks.sh +++ b/dev/run_benchmarks_for_gv.sh @@ -2,11 +2,12 @@ # Check if the file exists before removing it if [ -f "tmp/codeowners_benchmarks.md" ]; then - rm tmp/codeowners_benchmarks.md + rm tmp/codeowners_benchmarks_gv.md fi -echo "To run these benchmarks on your application, you can place this repo next to your rails application and run /usr/bin/env bash ../rubyatscale/codeowners-rs/dev/run_benchmarks.sh from the root of your application" >> tmp/codeowners_benchmarks.md +echo "To run these benchmarks on your application, you can place this repo next to your rails application and run /usr/bin/env bash ../rubyatscale/codeowners-rs/dev/run_benchmarks_for_gv.sh from the root of your application" >> tmp/codeowners_benchmarks_gv.md -hyperfine --warmup=2 --runs=3 --export-markdown tmp/codeowners_benchmarks.md \ +hyperfine --warmup=2 --runs=3 --export-markdown tmp/codeowners_benchmarks_gv.md \ '../rubyatscale/codeowners-rs/target/release/codeowners gv' \ - 'bin/codeowners-rs gv' \ No newline at end of file + 'bin/codeowners validate' \ + 'bin/codeownership validate' \ No newline at end of file diff --git a/src/bin/compare_for_file.rs b/src/bin/compare_for_file.rs new file mode 100644 index 0000000..7bce8ca --- /dev/null +++ b/src/bin/compare_for_file.rs @@ -0,0 +1,179 @@ +// This is a tool to compare the output of the original codeowners CLI with the optimized version. +// It's useful for verifying that the optimized version is correct. +// +// It's not used in CI, but it's useful for debugging. +// +// To run it, use `cargo run --bin compare_for_file ` +// +// It will compare the output of the original codeowners CLI with the optimized version for all files in the project. + +use std::{ + fs::File, + io::{self, Write}, + path::{Path, PathBuf}, + process::Command, +}; + +use codeowners::config::Config as OwnershipConfig; +use codeowners::ownership::{FileOwner, for_file_fast}; +use codeowners::runner::{RunConfig, Runner}; +use ignore::WalkBuilder; + +fn main() { + let project_root = std::env::args().nth(1).expect("usage: compare_for_file "); + let project_root = PathBuf::from(project_root); + if !project_root.is_absolute() { + eprintln!("Project root must be absolute"); + std::process::exit(2); + } + + let codeowners_file_path = project_root.join(".github/CODEOWNERS"); + let config_path = project_root.join("config/code_ownership.yml"); + + let run_config = RunConfig { + project_root: project_root.clone(), + codeowners_file_path, + config_path: config_path.clone(), + no_cache: false, + }; + + // Build the original, accurate-but-slower runner once + let runner = match Runner::new(&run_config) { + Ok(r) => r, + Err(e) => { + eprintln!("Failed to initialize Runner: {}", e); + std::process::exit(1); + } + }; + + // Load config once for the optimized path + let config_file = match File::open(&config_path) { + Ok(f) => f, + Err(e) => { + eprintln!("Can't open config file {}: {}", config_path.display(), e); + std::process::exit(1); + } + }; + let optimized_config: OwnershipConfig = match serde_yaml::from_reader(config_file) { + Ok(c) => c, + Err(e) => { + eprintln!("Can't parse config file {}: {}", config_path.display(), e); + std::process::exit(1); + } + }; + + let mut total_files: usize = 0; + let mut diff_count: usize = 0; + + // Prefer tracked files from git; fall back to walking the FS if git is unavailable + let tracked_files_output = Command::new("git").arg("-C").arg(&project_root).arg("ls-files").arg("-z").output(); + + match tracked_files_output { + Ok(output) if output.status.success() => { + let bytes = output.stdout; + for rel in bytes.split(|b| *b == 0u8) { + if rel.is_empty() { + continue; + } + let rel_str = match std::str::from_utf8(rel) { + Ok(s) => s, + Err(_) => continue, + }; + let abs_path = project_root.join(rel_str); + // Only process regular files that currently exist + if !abs_path.is_file() { + continue; + } + + total_files += 1; + let original = run_original(&runner, &abs_path); + let optimized = run_optimized(&project_root, &optimized_config, &abs_path); + + if original != optimized { + diff_count += 1; + println!("\n==== {} ====", abs_path.display()); + println!("ORIGINAL:\n{}", original); + println!("OPTIMIZED:\n{}", optimized); + let _ = io::stdout().flush(); + } + + if total_files % 1000 == 0 { + eprintln!("Processed {} files... diffs so far: {}", total_files, diff_count); + } + } + } + _ => { + eprintln!("git ls-files failed; falling back to filesystem walk (untracked files may be included)"); + let walker = WalkBuilder::new(&project_root) + .hidden(false) + .git_ignore(true) + .git_exclude(true) + .follow_links(false) + .build(); + + for result in walker { + let entry = match result { + Ok(e) => e, + Err(err) => { + eprintln!("walk error: {}", err); + continue; + } + }; + if !entry.file_type().map(|t| t.is_file()).unwrap_or(false) { + continue; + } + let path = entry.path(); + total_files += 1; + + let original = run_original(&runner, path); + let optimized = run_optimized(&project_root, &optimized_config, path); + + if original != optimized { + diff_count += 1; + println!("\n==== {} ====", path.display()); + println!("ORIGINAL:\n{}", original); + println!("OPTIMIZED:\n{}", optimized); + let _ = io::stdout().flush(); + } + + if total_files % 1000 == 0 { + eprintln!("Processed {} files... diffs so far: {}", total_files, diff_count); + } + } + } + } + + println!("Checked {} files. Diffs: {}", total_files, diff_count); + if diff_count > 0 { + std::process::exit(3); + } +} + +fn run_original(runner: &Runner, file_path: &Path) -> String { + let result = runner.for_file(&file_path.to_string_lossy()); + if !result.validation_errors.is_empty() { + return result.validation_errors.join("\n"); + } + if !result.io_errors.is_empty() { + return format!("IO_ERROR: {}", result.io_errors.join(" | ")); + } + result.info_messages.join("\n") +} + +fn run_optimized(project_root: &Path, config: &OwnershipConfig, file_path: &Path) -> String { + let owners: Vec = match for_file_fast::find_file_owners(project_root, config, file_path) { + Ok(v) => v, + Err(e) => return format!("IO_ERROR: {}", e), + }; + match owners.len() { + 0 => format!("{}", FileOwner::default()), + 1 => format!("{}", owners[0]), + _ => { + let mut lines = vec!["Error: file is owned by multiple teams!".to_string()]; + for owner in owners { + lines.push(format!("\n{}", owner)); + } + lines.join("\n") + } + } +} diff --git a/src/ownership.rs b/src/ownership.rs index 67e3e71..7d91d95 100644 --- a/src/ownership.rs +++ b/src/ownership.rs @@ -11,6 +11,7 @@ use tracing::{info, instrument}; mod file_generator; mod file_owner_finder; +pub mod for_file_fast; pub(crate) mod mapper; pub(crate) mod parser; mod validator; @@ -32,7 +33,7 @@ use self::{ pub struct Ownership { project: Arc, } - +#[derive(Debug)] pub struct FileOwner { pub team: Team, pub team_config_file_path: String, diff --git a/src/ownership/for_file_fast.rs b/src/ownership/for_file_fast.rs new file mode 100644 index 0000000..97babc7 --- /dev/null +++ b/src/ownership/for_file_fast.rs @@ -0,0 +1,424 @@ +use std::{ + collections::{HashMap, HashSet}, + fs, + path::Path, +}; + +use fast_glob::glob_match; +use glob::glob; + +use crate::{config::Config, project::Team, project_file_builder::build_project_file_without_cache}; + +use super::{FileOwner, mapper::Source}; + +pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path) -> Result, String> { + let absolute_file_path = if file_path.is_absolute() { + file_path.to_path_buf() + } else { + project_root.join(file_path) + }; + let relative_file_path = absolute_file_path + .strip_prefix(project_root) + .unwrap_or(&absolute_file_path) + .to_path_buf(); + + let teams = load_teams(project_root, &config.team_file_glob)?; + let teams_by_name = build_teams_by_name_map(&teams); + + let mut sources_by_team: HashMap> = HashMap::new(); + + if let Some(team_name) = read_top_of_file_team(&absolute_file_path) { + // Only consider top-of-file annotations for files included by config.owned_globs and not excluded by config.unowned_globs + if let Some(rel_str) = relative_file_path.to_str() { + let is_config_owned = glob_list_matches(rel_str, &config.owned_globs); + let is_config_unowned = glob_list_matches(rel_str, &config.unowned_globs); + if is_config_owned && !is_config_unowned { + if let Some(team) = teams_by_name.get(&team_name) { + sources_by_team.entry(team.name.clone()).or_default().push(Source::TeamFile); + } + } + } + } + + if let Some((owner_team_name, dir_source)) = most_specific_directory_owner(project_root, &relative_file_path, &teams_by_name) { + sources_by_team.entry(owner_team_name).or_default().push(dir_source); + } + + if let Some((owner_team_name, package_source)) = nearest_package_owner(project_root, &relative_file_path, config, &teams_by_name) { + sources_by_team.entry(owner_team_name).or_default().push(package_source); + } + + if let Some((owner_team_name, gem_source)) = vendored_gem_owner(&relative_file_path, config, &teams) { + sources_by_team.entry(owner_team_name).or_default().push(gem_source); + } + + if let Some(rel_str) = relative_file_path.to_str() { + for team in &teams { + let subtracts: HashSet<&str> = team.subtracted_globs.iter().map(|s| s.as_str()).collect(); + for owned_glob in &team.owned_globs { + if glob_match(owned_glob, rel_str) && !subtracts.iter().any(|sub| glob_match(sub, rel_str)) { + sources_by_team + .entry(team.name.clone()) + .or_default() + .push(Source::TeamGlob(owned_glob.clone())); + } + } + } + } + + for team in &teams { + let team_rel = team.path.strip_prefix(project_root).unwrap_or(&team.path).to_path_buf(); + if team_rel == relative_file_path { + sources_by_team.entry(team.name.clone()).or_default().push(Source::TeamYml); + } + } + + let mut file_owners: Vec = Vec::new(); + for (team_name, sources) in sources_by_team.into_iter() { + if let Some(team) = teams_by_name.get(&team_name) { + let relative_team_yml_path = team + .path + .strip_prefix(project_root) + .unwrap_or(&team.path) + .to_string_lossy() + .to_string(); + file_owners.push(FileOwner { + team: team.clone(), + team_config_file_path: relative_team_yml_path, + sources, + }); + } + } + + // TODO: remove this once we've verified the fast path is working + // This is simply matching the order of behavior of the original codeowners CLI + if file_owners.len() > 1 { + file_owners.sort_by(|a, b| { + let priority_a = a.sources.iter().map(source_priority).min().unwrap_or(u8::MAX); + let priority_b = b.sources.iter().map(source_priority).min().unwrap_or(u8::MAX); + priority_a.cmp(&priority_b).then_with(|| a.team.name.cmp(&b.team.name)) + }); + } + + Ok(file_owners) +} + +fn build_teams_by_name_map(teams: &[Team]) -> HashMap { + let mut map = HashMap::with_capacity(teams.len() * 2); + for team in teams { + map.insert(team.name.clone(), team.clone()); + map.insert(team.github_team.clone(), team.clone()); + } + map +} + +fn load_teams(project_root: &Path, team_file_globs: &[String]) -> std::result::Result, String> { + let mut teams: Vec = Vec::new(); + for glob_str in team_file_globs { + let absolute_glob = project_root.join(glob_str).to_string_lossy().into_owned(); + let paths = glob(&absolute_glob).map_err(|e| e.to_string())?; + for path in paths.flatten() { + match Team::from_team_file_path(path.clone()) { + Ok(team) => teams.push(team), + Err(e) => { + eprintln!("Error parsing team file: {}, path: {}", e, path.display()); + continue; + } + } + } + } + Ok(teams) +} + +// no regex: parse cheaply with ASCII-aware checks + +fn read_top_of_file_team(path: &Path) -> Option { + let project_file = build_project_file_without_cache(&path.to_path_buf()); + if let Some(owner) = project_file.owner { + return Some(owner); + } + + None +} + +fn most_specific_directory_owner( + project_root: &Path, + relative_file_path: &Path, + teams_by_name: &HashMap, +) -> Option<(String, Source)> { + let mut current = project_root.join(relative_file_path); + let mut best: Option<(String, Source)> = None; + loop { + if !current.pop() { + break; + } + let codeowner_path = current.join(".codeowner"); + if let Ok(owner_str) = fs::read_to_string(&codeowner_path) { + let owner = owner_str.trim(); + if let Some(team) = teams_by_name.get(owner) { + let relative_dir = current + .strip_prefix(project_root) + .unwrap_or(current.as_path()) + .to_string_lossy() + .to_string(); + let candidate = (team.name.clone(), Source::Directory(relative_dir)); + match &best { + None => best = Some(candidate), + Some((_, existing_source)) => { + if candidate.1.len() > existing_source.len() { + best = Some(candidate); + } + } + } + } + } + if current == project_root { + break; + } + } + best +} + +fn nearest_package_owner( + project_root: &Path, + relative_file_path: &Path, + config: &Config, + teams_by_name: &HashMap, +) -> Option<(String, Source)> { + let mut current = project_root.join(relative_file_path); + loop { + if !current.pop() { + break; + } + let parent_rel = current.strip_prefix(project_root).unwrap_or(current.as_path()); + if let Some(rel_str) = parent_rel.to_str() { + if glob_list_matches(rel_str, &config.ruby_package_paths) { + let pkg_yml = current.join("package.yml"); + if pkg_yml.exists() { + if let Ok(owner) = read_ruby_package_owner(&pkg_yml) { + if let Some(team) = teams_by_name.get(&owner) { + let package_path = parent_rel.join("package.yml"); + let package_glob = format!("{rel_str}/**/**"); + return Some(( + team.name.clone(), + Source::Package(package_path.to_string_lossy().to_string(), package_glob), + )); + } + } + } + } + if glob_list_matches(rel_str, &config.javascript_package_paths) { + let pkg_json = current.join("package.json"); + if pkg_json.exists() { + if let Ok(owner) = read_js_package_owner(&pkg_json) { + if let Some(team) = teams_by_name.get(&owner) { + let package_path = parent_rel.join("package.json"); + let package_glob = format!("{rel_str}/**/**"); + return Some(( + team.name.clone(), + Source::Package(package_path.to_string_lossy().to_string(), package_glob), + )); + } + } + } + } + } + if current == project_root { + break; + } + } + None +} + +// removed: use `Source::len()` instead + +fn glob_list_matches(path: &str, globs: &[String]) -> bool { + globs.iter().any(|g| glob_match(g, path)) +} + +fn read_ruby_package_owner(path: &Path) -> std::result::Result { + let file = std::fs::File::open(path).map_err(|e| e.to_string())?; + let deserializer: crate::project::deserializers::RubyPackage = serde_yaml::from_reader(file).map_err(|e| e.to_string())?; + deserializer.owner.ok_or_else(|| "Missing owner".to_string()) +} + +fn read_js_package_owner(path: &Path) -> std::result::Result { + let file = std::fs::File::open(path).map_err(|e| e.to_string())?; + let deserializer: crate::project::deserializers::JavascriptPackage = serde_json::from_reader(file).map_err(|e| e.to_string())?; + deserializer + .metadata + .and_then(|m| m.owner) + .ok_or_else(|| "Missing owner".to_string()) +} + +fn vendored_gem_owner(relative_file_path: &Path, config: &Config, teams: &[Team]) -> Option<(String, Source)> { + use std::path::Component; + let mut comps = relative_file_path.components(); + let first = comps.next()?; + let second = comps.next()?; + let first_str = match first { + Component::Normal(s) => s.to_string_lossy(), + _ => return None, + }; + if first_str != config.vendored_gems_path { + return None; + } + let gem_name = match second { + Component::Normal(s) => s.to_string_lossy().to_string(), + _ => return None, + }; + for team in teams { + if team.owned_gems.iter().any(|g| g == &gem_name) { + return Some((team.name.clone(), Source::TeamGem)); + } + } + None +} + +fn source_priority(source: &Source) -> u8 { + match source { + // Highest confidence first + Source::TeamFile => 0, + Source::Directory(_) => 1, + Source::Package(_, _) => 2, + Source::TeamGlob(_) => 3, + Source::TeamGem => 4, + Source::TeamYml => 5, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::project::Team; + use std::collections::HashMap; + use tempfile::tempdir; + + fn build_config_for_temp(frontend_glob: &str, ruby_glob: &str, vendored_path: &str) -> crate::config::Config { + crate::config::Config { + owned_globs: vec!["**/*".to_string()], + ruby_package_paths: vec![ruby_glob.to_string()], + javascript_package_paths: vec![frontend_glob.to_string()], + team_file_glob: vec!["config/teams/**/*.yml".to_string()], + unowned_globs: vec![], + vendored_gems_path: vendored_path.to_string(), + cache_directory: "tmp/cache/codeowners".to_string(), + } + } + + fn team_named(name: &str) -> Team { + Team { + path: Path::new("config/teams/foo.yml").to_path_buf(), + name: name.to_string(), + github_team: format!("@{}Team", name), + owned_globs: vec![], + subtracted_globs: vec![], + owned_gems: vec![], + avoid_ownership: false, + } + } + + #[test] + fn test_read_top_of_file_team_parses_at_and_colon_forms() { + let td = tempdir().unwrap(); + + // @team form + let file_at = td.path().join("at_form.rb"); + std::fs::write(&file_at, "# @team Payroll\nputs 'x'\n").unwrap(); + assert_eq!(read_top_of_file_team(&file_at), Some("Payroll".to_string())); + } + + #[test] + fn test_most_specific_directory_owner_prefers_deeper() { + let td = tempdir().unwrap(); + let project_root = td.path(); + + // Build directories + let deep_dir = project_root.join("a/b/c"); + std::fs::create_dir_all(&deep_dir).unwrap(); + let mid_dir = project_root.join("a/b"); + let top_dir = project_root.join("a"); + + // Write .codeowner files + std::fs::write(top_dir.join(".codeowner"), "TopTeam").unwrap(); + std::fs::write(mid_dir.join(".codeowner"), "MidTeam").unwrap(); + std::fs::write(deep_dir.join(".codeowner"), "DeepTeam").unwrap(); + + // Build teams_by_name + let mut tbn: HashMap = HashMap::new(); + for name in ["TopTeam", "MidTeam", "DeepTeam"] { + let t = team_named(name); + tbn.insert(t.name.clone(), t); + } + + let rel_file = Path::new("a/b/c/file.rb"); + let result = most_specific_directory_owner(project_root, rel_file, &tbn).unwrap(); + match result.1 { + Source::Directory(path) => { + assert!(path.ends_with("a/b/c"), "expected deepest directory, got {}", path); + } + _ => panic!("expected Directory source"), + } + assert_eq!(result.0, "DeepTeam"); + } + + #[test] + fn test_nearest_package_owner_ruby_and_js() { + let td = tempdir().unwrap(); + let project_root = td.path(); + let config = build_config_for_temp("frontend/**/*", "packs/**/*", "vendored"); + + // Ruby package + let ruby_pkg = project_root.join("packs/payroll"); + std::fs::create_dir_all(&ruby_pkg).unwrap(); + std::fs::write(ruby_pkg.join("package.yml"), "---\nowner: Payroll\n").unwrap(); + + // JS package + let js_pkg = project_root.join("frontend/flow"); + std::fs::create_dir_all(&js_pkg).unwrap(); + std::fs::write(js_pkg.join("package.json"), r#"{"metadata": {"owner": "UX"}}"#).unwrap(); + + // Teams map + let mut tbn: HashMap = HashMap::new(); + for name in ["Payroll", "UX"] { + let t = team_named(name); + tbn.insert(t.name.clone(), t); + } + + // Ruby nearest + let rel_ruby = Path::new("packs/payroll/app/models/thing.rb"); + let ruby_owner = nearest_package_owner(project_root, rel_ruby, &config, &tbn).unwrap(); + assert_eq!(ruby_owner.0, "Payroll"); + match ruby_owner.1 { + Source::Package(pkg_path, glob) => { + assert!(pkg_path.ends_with("packs/payroll/package.yml")); + assert_eq!(glob, "packs/payroll/**/**"); + } + _ => panic!("expected Package source for ruby"), + } + + // JS nearest + let rel_js = Path::new("frontend/flow/src/index.ts"); + let js_owner = nearest_package_owner(project_root, rel_js, &config, &tbn).unwrap(); + assert_eq!(js_owner.0, "UX"); + match js_owner.1 { + Source::Package(pkg_path, glob) => { + assert!(pkg_path.ends_with("frontend/flow/package.json")); + assert_eq!(glob, "frontend/flow/**/**"); + } + _ => panic!("expected Package source for js"), + } + } + + #[test] + fn test_vendored_gem_owner() { + let config = build_config_for_temp("frontend/**/*", "packs/**/*", "vendored"); + let mut teams: Vec = vec![team_named("Payroll")]; + teams[0].owned_gems = vec!["awesome_gem".to_string()]; + + let path = Path::new("vendored/awesome_gem/lib/a.rb"); + let result = vendored_gem_owner(path, &config, &teams).unwrap(); + assert_eq!(result.0, "Payroll"); + matches!(result.1, Source::TeamGem); + } +} diff --git a/src/project_file_builder.rs b/src/project_file_builder.rs index 5ac06df..ba4c0ed 100644 --- a/src/project_file_builder.rs +++ b/src/project_file_builder.rs @@ -47,7 +47,7 @@ impl<'a> ProjectFileBuilder<'a> { } } -fn build_project_file_without_cache(path: &PathBuf) -> ProjectFile { +pub(crate) fn build_project_file_without_cache(path: &PathBuf) -> ProjectFile { let content = match std::fs::read_to_string(path) { Ok(content) => content, Err(_) => { diff --git a/src/runner.rs b/src/runner.rs index ef53829..bfbe2fb 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -40,7 +40,8 @@ pub fn for_file(run_config: &RunConfig, file_path: &str, fast: bool) -> RunResul if fast { for_file_from_codeowners(run_config, file_path) } else { - run_with_runner(run_config, |runner| runner.for_file(file_path)) + //run_with_runner(run_config, |runner| runner.for_file(file_path)) + for_file_optimized(run_config, file_path) } } @@ -84,6 +85,58 @@ pub fn team_for_file_from_codeowners(run_config: &RunConfig, file_path: &str) -> .map_err(|e| Error::Io(e.to_string()))?) } +pub fn team_for_file(run_config: &RunConfig, file_path: &str) -> Result, Error> { + let config = config_from_path(&run_config.config_path)?; + use crate::ownership::for_file_fast::find_file_owners; + let owners = find_file_owners(&run_config.project_root, &config, std::path::Path::new(file_path)).map_err(Error::Io)?; + + Ok(owners.first().map(|fo| fo.team.clone())) +} + +// (imports below intentionally trimmed after refactor) + +fn for_file_optimized(run_config: &RunConfig, file_path: &str) -> RunResult { + let config = match config_from_path(&run_config.config_path) { + Ok(c) => c, + Err(err) => { + return RunResult { + io_errors: vec![err.to_string()], + ..Default::default() + }; + } + }; + + use crate::ownership::for_file_fast::find_file_owners; + let file_owners = match find_file_owners(&run_config.project_root, &config, std::path::Path::new(file_path)) { + Ok(v) => v, + Err(err) => { + return RunResult { + io_errors: vec![err], + ..Default::default() + }; + } + }; + + let info_messages: Vec = match file_owners.len() { + 0 => vec![format!("{}", FileOwner::default())], + 1 => vec![format!("{}", file_owners[0])], + _ => { + let mut error_messages = vec!["Error: file is owned by multiple teams!".to_string()]; + for file_owner in file_owners { + error_messages.push(format!("\n{}", file_owner)); + } + return RunResult { + validation_errors: error_messages, + ..Default::default() + }; + } + }; + RunResult { + info_messages, + ..Default::default() + } +} + pub fn for_team(run_config: &RunConfig, team_name: &str) -> RunResult { run_with_runner(run_config, |runner| runner.for_team(team_name)) } @@ -200,6 +253,7 @@ impl Runner { }, } } + pub fn generate(&self, git_stage: bool) -> RunResult { let content = self.ownership.generate_file(); if let Some(parent) = &self.run_config.codeowners_file_path.parent() { @@ -235,6 +289,8 @@ impl Runner { .output(); } + // TODO: remove this once we've verified the fast path is working + #[allow(dead_code)] pub fn for_file(&self, file_path: &str) -> RunResult { let relative_file_path = Path::new(file_path) .strip_prefix(&self.run_config.project_root) diff --git a/tests/fixtures/valid_project/.github/CODEOWNERS b/tests/fixtures/valid_project/.github/CODEOWNERS index 09a9fd7..6df553a 100644 --- a/tests/fixtures/valid_project/.github/CODEOWNERS +++ b/tests/fixtures/valid_project/.github/CODEOWNERS @@ -30,6 +30,8 @@ # Team YML ownership /config/teams/payments.yml @PaymentsTeam /config/teams/payroll.yml @PayrollTeam +/config/teams/ux.yml @UX # Team owned gems /gems/payroll_calculator/**/** @PayrollTeam +/gems/pets/**/** @UX diff --git a/tests/fixtures/valid_project/config/code_ownership.yml b/tests/fixtures/valid_project/config/code_ownership.yml index fa56397..05bc791 100644 --- a/tests/fixtures/valid_project/config/code_ownership.yml +++ b/tests/fixtures/valid_project/config/code_ownership.yml @@ -1,10 +1,10 @@ owned_globs: - - "**/*.{rb,tsx}" + - "{gems,config,javascript,ruby,components}/**/*.{rb,tsx}" ruby_package_paths: - ruby/packages/**/* javascript_package_paths: - javascript/packages/** team_file_glob: - config/teams/**/*.yml -vendored_gems_path: gems +unbuilt_gems_path: gems unowned_globs: diff --git a/tests/fixtures/valid_project/config/teams/ux.yml b/tests/fixtures/valid_project/config/teams/ux.yml new file mode 100644 index 0000000..d4675fa --- /dev/null +++ b/tests/fixtures/valid_project/config/teams/ux.yml @@ -0,0 +1,6 @@ +name: UX +github: + team: '@UX' +ruby: + owned_gems: + - pets \ No newline at end of file diff --git a/tests/fixtures/valid_project/gems/pets/dog.rb b/tests/fixtures/valid_project/gems/pets/dog.rb new file mode 100644 index 0000000..0518b82 --- /dev/null +++ b/tests/fixtures/valid_project/gems/pets/dog.rb @@ -0,0 +1,5 @@ +# generated +# next line should be ignored +# @team Payments + +class Dog; end diff --git a/tests/fixtures/valid_project/should_be_ignored/an_ignored_file.rb b/tests/fixtures/valid_project/should_be_ignored/an_ignored_file.rb new file mode 100644 index 0000000..24ad7ca --- /dev/null +++ b/tests/fixtures/valid_project/should_be_ignored/an_ignored_file.rb @@ -0,0 +1,7 @@ +# @team Payments + +class AnIgnoredFile + def initialize + @name = "AnIgnoredFile" + end +end \ No newline at end of file diff --git a/tests/valid_project_test.rs b/tests/valid_project_test.rs index 6ee2caa..d8b4e81 100644 --- a/tests/valid_project_test.rs +++ b/tests/valid_project_test.rs @@ -97,6 +97,26 @@ fn test_fast_for_file() -> Result<(), Box> { Ok(()) } +#[test] +fn test_fast_for_file_with_ignored_file() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/valid_project") + .arg("--no-cache") + .arg("for-file") + .arg("should_be_ignored/an_ignored_file.rb") + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Team: Unowned + Github Team: Unowned + Team YML: + Description: + - Unowned + "})); + Ok(()) +} + #[test] fn test_fast_for_file_full_path() -> Result<(), Box> { let project_root = Path::new("tests/fixtures/valid_project"); @@ -118,6 +138,26 @@ fn test_fast_for_file_full_path() -> Result<(), Box> { Ok(()) } +#[test] +fn test_for_file_with_components() -> Result<(), Box> { + Command::cargo_bin("codeowners")? + .arg("--project-root") + .arg("tests/fixtures/valid_project") + .arg("--no-cache") + .arg("for-file") + .arg("gems/pets/dog.rb") + .assert() + .success() + .stdout(predicate::eq(indoc! {" + Team: UX + Github Team: @UX + Team YML: config/teams/ux.yml + Description: + - Owner specified in Team YML's `owned_gems` + "})); + Ok(()) +} + #[test] fn test_for_file_same_team_multiple_ownerships() -> Result<(), Box> { Command::cargo_bin("codeowners")?