Skip to content

Commit e4244ee

Browse files
committed
verifying new == old
1 parent 0c79763 commit e4244ee

File tree

8 files changed

+243
-19
lines changed

8 files changed

+243
-19
lines changed

src/bin/compare_for_file.rs

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
use std::{
2+
fs::File,
3+
io::{self, Write},
4+
path::{Path, PathBuf},
5+
process::Command,
6+
};
7+
8+
use codeowners::config::Config as OwnershipConfig;
9+
use codeowners::runner::{RunConfig, Runner};
10+
use codeowners::ownership::{fast, FileOwner};
11+
use ignore::WalkBuilder;
12+
use serde_yaml;
13+
14+
fn main() {
15+
let project_root = std::env::args()
16+
.nth(1)
17+
.expect("usage: compare_for_file <absolute_project_root>");
18+
let project_root = PathBuf::from(project_root);
19+
if !project_root.is_absolute() {
20+
eprintln!("Project root must be absolute");
21+
std::process::exit(2);
22+
}
23+
24+
let codeowners_file_path = project_root.join(".github/CODEOWNERS");
25+
let config_path = project_root.join("config/code_ownership.yml");
26+
27+
let run_config = RunConfig {
28+
project_root: project_root.clone(),
29+
codeowners_file_path,
30+
config_path: config_path.clone(),
31+
no_cache: false,
32+
};
33+
34+
// Build the original, accurate-but-slower runner once
35+
let runner = match Runner::new(&run_config) {
36+
Ok(r) => r,
37+
Err(e) => {
38+
eprintln!("Failed to initialize Runner: {}", e);
39+
std::process::exit(1);
40+
}
41+
};
42+
43+
// Load config once for the optimized path
44+
let config_file = match File::open(&config_path) {
45+
Ok(f) => f,
46+
Err(e) => {
47+
eprintln!("Can't open config file {}: {}", config_path.display(), e);
48+
std::process::exit(1);
49+
}
50+
};
51+
let optimized_config: OwnershipConfig = match serde_yaml::from_reader(config_file) {
52+
Ok(c) => c,
53+
Err(e) => {
54+
eprintln!("Can't parse config file {}: {}", config_path.display(), e);
55+
std::process::exit(1);
56+
}
57+
};
58+
59+
let mut total_files: usize = 0;
60+
let mut diff_count: usize = 0;
61+
62+
// Prefer tracked files from git; fall back to walking the FS if git is unavailable
63+
let tracked_files_output = Command::new("git")
64+
.arg("-C")
65+
.arg(&project_root)
66+
.arg("ls-files")
67+
.arg("-z")
68+
.output();
69+
70+
match tracked_files_output {
71+
Ok(output) if output.status.success() => {
72+
let bytes = output.stdout;
73+
for rel in bytes.split(|b| *b == 0u8) {
74+
if rel.is_empty() { continue; }
75+
let rel_str = match std::str::from_utf8(rel) { Ok(s) => s, Err(_) => continue };
76+
let abs_path = project_root.join(rel_str);
77+
// Only process regular files that currently exist
78+
if !abs_path.is_file() { continue; }
79+
80+
total_files += 1;
81+
let original = run_original(&runner, &abs_path);
82+
let optimized = run_optimized(&project_root, &optimized_config, &abs_path);
83+
84+
if original != optimized {
85+
diff_count += 1;
86+
println!("\n==== {} ====", abs_path.display());
87+
println!("ORIGINAL:\n{}", original);
88+
println!("OPTIMIZED:\n{}", optimized);
89+
let _ = io::stdout().flush();
90+
}
91+
92+
if total_files % 1000 == 0 {
93+
eprintln!(
94+
"Processed {} files... diffs so far: {}",
95+
total_files, diff_count
96+
);
97+
}
98+
}
99+
}
100+
_ => {
101+
eprintln!("git ls-files failed; falling back to filesystem walk (untracked files may be included)");
102+
let walker = WalkBuilder::new(&project_root)
103+
.hidden(false)
104+
.git_ignore(true)
105+
.git_exclude(true)
106+
.follow_links(false)
107+
.build();
108+
109+
for result in walker {
110+
let entry = match result {
111+
Ok(e) => e,
112+
Err(err) => {
113+
eprintln!("walk error: {}", err);
114+
continue;
115+
}
116+
};
117+
if !entry.file_type().map(|t| t.is_file()).unwrap_or(false) {
118+
continue;
119+
}
120+
let path = entry.path();
121+
total_files += 1;
122+
123+
let original = run_original(&runner, path);
124+
let optimized = run_optimized(&project_root, &optimized_config, path);
125+
126+
if original != optimized {
127+
diff_count += 1;
128+
println!("\n==== {} ====", path.display());
129+
println!("ORIGINAL:\n{}", original);
130+
println!("OPTIMIZED:\n{}", optimized);
131+
let _ = io::stdout().flush();
132+
}
133+
134+
if total_files % 1000 == 0 {
135+
eprintln!(
136+
"Processed {} files... diffs so far: {}",
137+
total_files, diff_count
138+
);
139+
}
140+
}
141+
}
142+
}
143+
144+
println!("Checked {} files. Diffs: {}", total_files, diff_count);
145+
if diff_count > 0 {
146+
std::process::exit(3);
147+
}
148+
}
149+
150+
fn run_original(runner: &Runner, file_path: &Path) -> String {
151+
let result = runner.for_file(&file_path.to_string_lossy());
152+
if !result.validation_errors.is_empty() {
153+
return result.validation_errors.join("\n");
154+
}
155+
if !result.io_errors.is_empty() {
156+
return format!("IO_ERROR: {}", result.io_errors.join(" | "));
157+
}
158+
result.info_messages.join("\n")
159+
}
160+
161+
fn run_optimized(project_root: &Path, config: &OwnershipConfig, file_path: &Path) -> String {
162+
let owners: Vec<FileOwner> = match fast::find_file_owners(project_root, config, file_path) {
163+
Ok(v) => v,
164+
Err(e) => return format!("IO_ERROR: {}", e),
165+
};
166+
match owners.len() {
167+
0 => format!("{}", FileOwner::default()),
168+
1 => format!("{}", owners[0]),
169+
_ => {
170+
let mut lines = vec!["Error: file is owned by multiple teams!".to_string()];
171+
for owner in owners {
172+
lines.push(format!("\n{}", owner));
173+
}
174+
lines.join("\n")
175+
}
176+
}
177+
}
178+
179+

src/ownership.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ mod file_owner_finder;
1414
pub(crate) mod mapper;
1515
pub(crate) mod parser;
1616
mod validator;
17-
pub(crate) mod fast;
17+
pub mod fast;
1818

1919
use crate::{
2020
ownership::mapper::DirectoryMapper,

src/ownership/fast.rs

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::{config::Config, project::Team};
99

1010
use super::{FileOwner, mapper::Source};
1111

12-
pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path) -> Vec<FileOwner> {
12+
pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path) -> Result<Vec<FileOwner>, String> {
1313
let absolute_file_path = if file_path.is_absolute() {
1414
file_path.to_path_buf()
1515
} else {
@@ -22,15 +22,22 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path)
2222

2323
let teams = match load_teams(project_root, &config.team_file_glob) {
2424
Ok(t) => t,
25-
Err(_) => return vec![],
25+
Err(e) => return Err(e),
2626
};
2727
let teams_by_name = build_teams_by_name_map(&teams);
2828

2929
let mut sources_by_team: HashMap<String, Vec<Source>> = HashMap::new();
3030

3131
if let Some(team_name) = read_top_of_file_team(&absolute_file_path) {
32-
if let Some(team) = teams_by_name.get(&team_name) {
33-
sources_by_team.entry(team.name.clone()).or_default().push(Source::TeamFile);
32+
// Only consider top-of-file annotations for files included by config.owned_globs and not excluded by config.unowned_globs
33+
if let Some(rel_str) = relative_file_path.to_str() {
34+
let is_config_owned = glob_list_matches(rel_str, &config.owned_globs);
35+
let is_config_unowned = glob_list_matches(rel_str, &config.unowned_globs);
36+
if is_config_owned && !is_config_unowned {
37+
if let Some(team) = teams_by_name.get(&team_name) {
38+
sources_by_team.entry(team.name.clone()).or_default().push(Source::TeamFile);
39+
}
40+
}
3441
}
3542
}
3643

@@ -106,7 +113,7 @@ pub fn find_file_owners(project_root: &Path, config: &Config, file_path: &Path)
106113
});
107114
}
108115

109-
file_owners
116+
Ok(file_owners)
110117
}
111118

112119
fn build_teams_by_name_map(teams: &[Team]) -> HashMap<String, Team> {
@@ -137,23 +144,25 @@ fn load_teams(project_root: &Path, team_file_globs: &[String]) -> std::result::R
137144
}
138145

139146
lazy_static! {
140-
static ref TOP_OF_FILE_TEAM_AT_REGEX: Regex = Regex::new(r#"^(?:#|//)\s*@team\s+(.+)$"#)
141-
.expect("error compiling regular expression");
142-
static ref TOP_OF_FILE_TEAM_COLON_REGEX: Regex = Regex::new(r#"(?i)^(?:#|//)\s*team\s*:\s*(.+)$"#)
143-
.expect("error compiling regular expression");
147+
static ref TOP_OF_FILE_TEAM_AT_REGEX: Option<Regex> = Regex::new(r#"^(?:#|//)\s*@team\s+(.+)$"#).ok();
148+
static ref TOP_OF_FILE_TEAM_COLON_REGEX: Option<Regex> = Regex::new(r#"(?i)^(?:#|//)\s*team\s*:\s*(.+)$"#).ok();
144149
}
145150

146151
fn read_top_of_file_team(path: &Path) -> Option<String> {
147152
let content = fs::read_to_string(path).ok()?;
148153
for line in content.lines().take(15) {
149-
if let Some(cap) = TOP_OF_FILE_TEAM_AT_REGEX.captures(line) {
150-
if let Some(m) = cap.get(1) {
151-
return Some(m.as_str().to_string());
154+
if let Some(re) = &*TOP_OF_FILE_TEAM_AT_REGEX {
155+
if let Some(cap) = re.captures(line) {
156+
if let Some(m) = cap.get(1) {
157+
return Some(m.as_str().to_string());
158+
}
152159
}
153160
}
154-
if let Some(cap) = TOP_OF_FILE_TEAM_COLON_REGEX.captures(line) {
155-
if let Some(m) = cap.get(1) {
156-
return Some(m.as_str().to_string());
161+
if let Some(re) = &*TOP_OF_FILE_TEAM_COLON_REGEX {
162+
if let Some(cap) = re.captures(line) {
163+
if let Some(m) = cap.get(1) {
164+
return Some(m.as_str().to_string());
165+
}
157166
}
158167
}
159168
let trimmed = line.trim_start();

src/project_builder.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,10 @@ impl<'a> ProjectBuilder<'a> {
216216
}
217217

218218
fn matches_globs(path: &Path, globs: &[String]) -> bool {
219-
globs.iter().any(|glob| glob_match(glob, path.to_str().unwrap()))
219+
match path.to_str() {
220+
Some(s) => globs.iter().any(|glob| glob_match(glob, s)),
221+
None => false,
222+
}
220223
}
221224

222225
fn ruby_package_owner(path: &Path) -> Result<Option<String>, Error> {

src/runner.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub fn for_file(run_config: &RunConfig, file_path: &str, fast: bool) -> RunResul
3939
if fast {
4040
for_file_from_codeowners(run_config, file_path)
4141
} else {
42+
// run_with_runner(run_config, |runner| runner.for_file(file_path))
4243
for_file_optimized(run_config, file_path)
4344
}
4445
}
@@ -97,7 +98,12 @@ fn for_file_optimized(run_config: &RunConfig, file_path: &str) -> RunResult {
9798
};
9899

99100
use crate::ownership::fast::find_file_owners;
100-
let file_owners = find_file_owners(&run_config.project_root, &config, std::path::Path::new(file_path));
101+
let file_owners = match find_file_owners(&run_config.project_root, &config, std::path::Path::new(file_path)) {
102+
Ok(v) => v,
103+
Err(err) => {
104+
return RunResult { io_errors: vec![err], ..Default::default() };
105+
}
106+
};
101107

102108
let info_messages: Vec<String> = match file_owners.len() {
103109
0 => vec![format!("{}", FileOwner::default())],

tests/fixtures/valid_project/config/code_ownership.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
owned_globs:
2-
- "**/*.{rb,tsx}"
2+
- "{gems,config,javascript,ruby}/**/*.{rb,tsx}"
33
ruby_package_paths:
44
- ruby/packages/**/*
55
javascript_package_paths:
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# @team Payments
2+
3+
class AnIgnoredFile
4+
def initialize
5+
@name = "AnIgnoredFile"
6+
end
7+
end

tests/valid_project_test.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,26 @@ fn test_fast_for_file() -> Result<(), Box<dyn Error>> {
9797
Ok(())
9898
}
9999

100+
#[test]
101+
fn test_fast_for_file_with_ignored_file() -> Result<(), Box<dyn Error>> {
102+
Command::cargo_bin("codeowners")?
103+
.arg("--project-root")
104+
.arg("tests/fixtures/valid_project")
105+
.arg("--no-cache")
106+
.arg("for-file")
107+
.arg("should_be_ignored/an_ignored_file.rb")
108+
.assert()
109+
.success()
110+
.stdout(predicate::eq(indoc! {"
111+
Team: Unowned
112+
Github Team: Unowned
113+
Team YML:
114+
Description:
115+
- Unowned
116+
"}));
117+
Ok(())
118+
}
119+
100120
#[test]
101121
fn test_fast_for_file_full_path() -> Result<(), Box<dyn Error>> {
102122
let project_root = Path::new("tests/fixtures/valid_project");

0 commit comments

Comments
 (0)