diff --git a/crates/spk-build/src/build/binary.rs b/crates/spk-build/src/build/binary.rs index 229e8d871..2312c83ad 100644 --- a/crates/spk-build/src/build/binary.rs +++ b/crates/spk-build/src/build/binary.rs @@ -836,6 +836,7 @@ fn split_manifest_by_component( if component .files .matches(node.path.to_path("/"), node.entry.is_dir()) + .map_err(|err| Error::String(err.to_string()))? { let is_new_file = seen.insert(node.path.to_owned()); if matches!(component.file_match_mode, ComponentFileMatchMode::All) || is_new_file { diff --git a/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher.rs b/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher.rs index 5723d74ba..eb5471b70 100644 --- a/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher.rs +++ b/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher.rs @@ -2,6 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 // https://github.com/spkenv/spk +use std::sync::{Arc, Mutex}; + use serde::{Deserialize, Serialize}; use super::{Error, Result}; @@ -15,7 +17,7 @@ mod file_matcher_test; #[derive(Clone)] pub struct FileMatcher { rules: Vec, - gitignore: ignore::gitignore::Gitignore, + gitignore: Arc>>>, } impl std::fmt::Debug for FileMatcher { @@ -32,7 +34,7 @@ impl Default for FileMatcher { fn default() -> Self { Self { rules: Vec::new(), - gitignore: ignore::gitignore::Gitignore::empty(), + gitignore: Arc::new(Mutex::new(None)), } } } @@ -76,15 +78,7 @@ impl FileMatcher { I: Into, { let rules: Vec<_> = rules.into_iter().map(Into::into).collect(); - let mut builder = ignore::gitignore::GitignoreBuilder::new("/"); - for rule in rules.iter() { - builder - .add_line(None, rule) - .map_err(|err| Error::String(format!("Invalid file pattern '{rule}': {err:?}")))?; - } - let gitignore = builder - .build() - .map_err(|err| Error::String(format!("Failed to compile file patterns: {err:?}")))?; + let gitignore = Arc::new(Mutex::new(None)); Ok(Self { rules, gitignore }) } @@ -99,11 +93,50 @@ impl FileMatcher { &self.rules } - /// Reports true if the given path matches a rule in this set - pub fn matches>(&self, path: P, is_dir: bool) -> bool { - self.gitignore - .matched_path_or_any_parents(path, is_dir) - .is_ignore() + /// Reports true if the given path matches a rule in this set. + /// + /// The first call to this will construct an internal gitignore + /// matching object and cache it for subsequent calls. + pub fn matches>(&self, path: P, is_dir: bool) -> Result { + if self.rules.is_empty() { + // Short-circuit if there are no rules to match against. + // See `all()` method for making a FileMatcher that will + // match all paths. + return Ok(false); + } + + let mut matcher = self + .gitignore + .lock() + .map_err(|err| Error::String(err.to_string()))?; + + if matcher.is_none() { + let mut builder = ignore::gitignore::GitignoreBuilder::new("/"); + for rule in self.rules.iter() { + builder.add_line(None, rule).map_err(|err| { + let error = format!("Invalid file pattern '{rule}': {err:?}"); + *matcher = Some(Err(Error::String(error.clone()))); + Error::String(error) + })?; + } + let gitignore = builder.build().map_err(|err| { + let error = format!("Failed to compile file patterns: {err:?}"); + *matcher = Some(Err(Error::String(error.clone()))); + Error::String(error) + })?; + *matcher = Some(Ok(gitignore)); + } + + if let Some(ref gitignore_matcher) = *matcher { + match gitignore_matcher { + Ok(m) => Ok(m.matched_path_or_any_parents(path, is_dir).is_ignore()), + Err(err) => Err(Error::String(err.to_string())), + } + } else { + Err(Error::String( + "Unable to construct a gitignore matching object for file matcher".to_string(), + )) + } } } diff --git a/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher_test.rs b/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher_test.rs index 876899758..762dfe3e5 100644 --- a/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher_test.rs +++ b/crates/spk-schema/crates/foundation/src/spec_ops/file_matcher_test.rs @@ -19,5 +19,8 @@ fn test_file_matcher_matching( // we're not really testing gitignore here, just that the // semantics of our function works as expected let matcher = FileMatcher::new(patterns.iter().map(|s| s.to_string())).unwrap(); - assert_eq!(matcher.matches(path, path.ends_with('/')), should_match); + assert_eq!( + matcher.matches(path, path.ends_with('/')).unwrap(), + should_match + ); }