improving project build speed

Author: perryqh

Cargo.lock

@@ -10,10 +10,11 @@ debug = true
 clap = { version = "4.5.20", features = ["derive"] }
 clap_derive = "4.5.18"
 error-stack = "0.5.0"
-glob-match = "0.2.1"
+fast-glob = "0.4.0"
 ignore = "0.4.23"
 itertools = "0.13.0"
 path-clean = "1.0.1"
+lazy_static = "1.5.0"
 rayon = "1.10.0"
 regex = "1.11.1"
 serde = { version = "1.0.214", features = ["derive"] }

Cargo.toml

@@ -10,7 +10,7 @@ pub mod tests {
 
     use tempfile::tempdir;
 
-    use crate::{ownership::Ownership, project::Project};
+    use crate::{ownership::Ownership, project_builder::ProjectBuilder};
 
     macro_rules! ownership {
         ($($test_files:expr),+) => {{
@@ -110,13 +110,15 @@ pub mod tests {
         let config = serde_yaml::from_reader(config_file)?;
 
         let codeowners_file_path = &test_config.temp_dir_path.join(".github/CODEOWNERS");
-        let project = Project::build(&test_config.temp_dir_path, codeowners_file_path, &config)?;
+        let mut builder = ProjectBuilder::new(&config, test_config.temp_dir_path.clone(), codeowners_file_path.clone());
+        let project = builder.build()?;
         let ownership = Ownership::build(project);
         if test_config.generate_codeowners {
             std::fs::write(codeowners_file_path, ownership.generate_file())?;
         }
         // rebuild project to ensure new codeowners file is read
-        let project = Project::build(&test_config.temp_dir_path, codeowners_file_path, &config)?;
+        let mut builder = ProjectBuilder::new(&config, test_config.temp_dir_path.clone(), codeowners_file_path.clone());
+        let project = builder.build()?;
         Ok(Ownership::build(project))
     }
 

src/common_test.rs

@@ -1,6 +1,6 @@
 use ownership::{FileOwner, Ownership};
 
-use crate::project::Project;
+use crate::project_builder::ProjectBuilder;
 use clap::{Parser, Subcommand};
 use core::fmt;
 use error_stack::{Context, Result, ResultExt};
@@ -15,6 +15,7 @@ mod common_test;
 mod config;
 mod ownership;
 mod project;
+mod project_builder;
 
 #[derive(Subcommand, Debug)]
 enum Command {
@@ -114,7 +115,9 @@ fn cli() -> Result<(), Error> {
 
     let config = serde_yaml::from_reader(config_file).change_context(Error::Io)?;
 
-    let ownership = Ownership::build(Project::build(&project_root, &codeowners_file_path, &config).change_context(Error::Io)?);
+    let mut builder = ProjectBuilder::new(&config, project_root, codeowners_file_path.clone());
+    let project = builder.build().change_context(Error::Io)?;
+    let ownership = Ownership::build(project);
 
     match args.command {
         Command::Validate => ownership.validate().change_context(Error::ValidationFailed)?,

src/main.rs

@@ -1,4 +1,4 @@
-use glob_match::glob_match;
+use fast_glob::glob_match;
 use std::{
     collections::HashMap,
     fmt::{self, Display},

src/ownership/mapper.rs

@@ -2,21 +2,11 @@ use core::fmt;
 use std::{
     collections::HashMap,
     fmt::Display,
-    fs::File,
-    io::BufRead,
     path::{Path, PathBuf},
 };
 
 use error_stack::{Context, Result, ResultExt};
 
-use ignore::WalkBuilder;
-use rayon::prelude::{IntoParallelIterator, ParallelIterator};
-use regex::Regex;
-use tracing::{info, instrument};
-
-use crate::config::Config;
-use glob_match::glob_match;
-
 pub struct Project {
     pub base_path: PathBuf,
     pub files: Vec<ProjectFile>,
@@ -33,7 +23,7 @@ pub struct VendoredGem {
     pub name: String,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct ProjectFile {
     pub owner: Option<String>,
     pub path: PathBuf,
@@ -49,6 +39,7 @@ pub struct Team {
     pub avoid_ownership: bool,
 }
 
+#[derive(Clone, Debug)]
 pub struct Package {
     pub path: PathBuf,
     pub package_type: PackageType,
@@ -73,7 +64,7 @@ impl DirectoryCodeownersFile {
     }
 }
 
-#[derive(PartialEq, Eq, Debug)]
+#[derive(PartialEq, Eq, Debug, Clone)]
 pub enum PackageType {
     Ruby,
     Javascript,
@@ -85,7 +76,7 @@ impl Display for PackageType {
     }
 }
 
-mod deserializers {
+pub mod deserializers {
     use serde::Deserialize;
 
     #[derive(Deserialize)]
@@ -155,113 +146,6 @@ impl fmt::Display for Error {
 impl Context for Error {}
 
 impl Project {
-    #[instrument(level = "debug", skip_all)]
-    pub fn build(base_path: &Path, codeowners_file_path: &Path, config: &Config) -> Result<Self, Error> {
-        info!(base_path = base_path.to_str(), "scanning project");
-
-        let mut owned_file_paths: Vec<PathBuf> = Vec::new();
-        let mut packages: Vec<Package> = Vec::new();
-        let mut teams: Vec<Team> = Vec::new();
-        let mut vendored_gems: Vec<VendoredGem> = Vec::new();
-        let mut directory_codeowner_files: Vec<DirectoryCodeownersFile> = Vec::new();
-
-        let mut builder = WalkBuilder::new(base_path);
-        builder.hidden(false);
-        let walkdir = builder.build();
-
-        for entry in walkdir {
-            let entry = entry.change_context(Error::Io)?;
-
-            let absolute_path = entry.path();
-            let relative_path = absolute_path.strip_prefix(base_path).change_context(Error::Io)?.to_owned();
-
-            if entry.file_type().unwrap().is_dir() {
-                if relative_path.parent() == Some(Path::new(&config.vendored_gems_path)) {
-                    let file_name = relative_path.file_name().expect("expected a file_name");
-                    vendored_gems.push(VendoredGem {
-                        path: absolute_path.to_owned(),
-                        name: file_name.to_string_lossy().to_string(),
-                    })
-                }
-
-                continue;
-            }
-
-            let file_name = relative_path.file_name().expect("expected a file_name");
-
-            if file_name.eq_ignore_ascii_case("package.yml") && matches_globs(relative_path.parent().unwrap(), &config.ruby_package_paths) {
-                if let Some(owner) = ruby_package_owner(absolute_path)? {
-                    packages.push(Package {
-                        path: relative_path.clone(),
-                        owner,
-                        package_type: PackageType::Ruby,
-                    })
-                }
-            }
-
-            if file_name.eq_ignore_ascii_case("package.json")
-                && matches_globs(relative_path.parent().unwrap(), &config.javascript_package_paths)
-            {
-                if let Some(owner) = javascript_package_owner(absolute_path)? {
-                    packages.push(Package {
-                        path: relative_path.clone(),
-                        owner,
-                        package_type: PackageType::Javascript,
-                    })
-                }
-            }
-
-            if file_name.eq_ignore_ascii_case(".codeowner") {
-                let owner = std::fs::read_to_string(absolute_path).change_context(Error::Io)?;
-                let owner = owner.trim().to_owned();
-
-                let relative_path = relative_path.to_owned();
-                directory_codeowner_files.push(DirectoryCodeownersFile {
-                    path: relative_path,
-                    owner,
-                })
-            }
-
-            if matches_globs(&relative_path, &config.team_file_glob) {
-                let file = File::open(absolute_path).change_context(Error::Io)?;
-                let deserializer: deserializers::Team = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?;
-
-                teams.push(Team {
-                    path: absolute_path.to_owned(),
-                    name: deserializer.name,
-                    github_team: deserializer.github.team,
-                    owned_globs: deserializer.owned_globs,
-                    owned_gems: deserializer.ruby.map(|ruby| ruby.owned_gems).unwrap_or_default(),
-                    avoid_ownership: deserializer.github.do_not_add_to_codeowners_file,
-                })
-            }
-
-            if matches_globs(&relative_path, &config.owned_globs) && !matches_globs(&relative_path, &config.unowned_globs) {
-                owned_file_paths.push(absolute_path.to_owned())
-            }
-        }
-
-        info!(
-            owned_file_paths = owned_file_paths.len(),
-            packages = packages.len(),
-            teams = teams.len(),
-            vendored_gems = vendored_gems.len(),
-            "finished scanning project",
-        );
-
-        let owned_files = owned_files(owned_file_paths);
-
-        Ok(Project {
-            base_path: base_path.to_owned(),
-            files: owned_files,
-            vendored_gems,
-            teams,
-            packages,
-            codeowners_file_path: codeowners_file_path.to_path_buf(),
-            directory_codeowner_files,
-        })
-    }
-
     pub fn get_codeowners_file(&self) -> Result<String, Error> {
         let codeowners_file: String = if self.codeowners_file_path.exists() {
             std::fs::read_to_string(&self.codeowners_file_path).change_context(Error::Io)?
@@ -301,78 +185,3 @@ impl Project {
         result
     }
 }
-
-#[instrument(level = "debug", skip_all)]
-fn owned_files(owned_file_paths: Vec<PathBuf>) -> Vec<ProjectFile> {
-    let regexp = Regex::new(r#"^(?:#|//) @team (.*)$"#).expect("error compiling regular expression");
-
-    info!("opening files to read ownership annotation");
-
-    owned_file_paths
-        .into_par_iter()
-        .map(|path| {
-            let file = File::open(&path).unwrap_or_else(|_| panic!("Couldn't open {}", path.to_string_lossy()));
-            let first_line = std::io::BufReader::new(file).lines().next().transpose();
-            let first_line = first_line.expect("error reading first line");
-
-            if first_line.is_none() {
-                return ProjectFile { path, owner: None };
-            }
-
-            if let Some(first_line) = first_line {
-                let capture = regexp.captures(&first_line);
-
-                if let Some(capture) = capture {
-                    let first_capture = capture.get(1);
-
-                    if let Some(first_capture) = first_capture {
-                        return ProjectFile {
-                            path,
-                            owner: Some(first_capture.as_str().to_string()),
-                        };
-                    }
-                }
-            }
-
-            ProjectFile { path, owner: None }
-        })
-        .collect()
-}
-
-fn ruby_package_owner(path: &Path) -> Result<Option<String>, Error> {
-    let file = File::open(path).change_context(Error::Io)?;
-    let deserializer: deserializers::RubyPackage = serde_yaml::from_reader(file).change_context(Error::SerdeYaml)?;
-
-    Ok(deserializer.owner)
-}
-
-fn javascript_package_owner(path: &Path) -> Result<Option<String>, Error> {
-    let file = File::open(path).change_context(Error::Io)?;
-    let deserializer: deserializers::JavascriptPackage = serde_json::from_reader(file).change_context(Error::SerdeJson)?;
-
-    Ok(deserializer.metadata.and_then(|metadata| metadata.owner))
-}
-
-fn matches_globs(path: &Path, globs: &[String]) -> bool {
-    globs.iter().any(|glob| glob_match(glob, path.to_str().unwrap()))
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    const OWNED_GLOB: &str = "{app,components,config,frontend,lib,packs,spec,danger,script}/**/*.{rb,arb,erb,rake,js,jsx,ts,tsx}";
-
-    #[test]
-    fn test_matches_globs() {
-        // should fail because hidden directories are ignored by glob patterns unless explicitly included
-        assert!(matches_globs(Path::new("script/.eslintrc.js"), &[OWNED_GLOB.to_string()]));
-    }
-
-    #[test]
-    fn test_glob_match() {
-        // Exposes bug in glob-match https://github.com/devongovett/glob-match/issues/9
-        // should fail because hidden directories are ignored by glob patterns unless explicitly included
-        assert!(glob_match(OWNED_GLOB, "script/.eslintrc.js"));
-    }
-}