-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core): optimize daemon output glob matching #27775
Changes from 5 commits
15f848d
d931e75
818fbd5
1c3b07d
f601907
c453042
e70bbbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
pub mod cache; | ||
pub mod expand_outputs; | ||
pub mod file_ops; | ||
pub mod cache; | ||
pub mod validate_outputs; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
use itertools::Itertools; | ||
use regex::Regex; | ||
|
||
use crate::native::glob::{contains_glob_pattern, glob_transform::partition_glob}; | ||
|
||
const ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES: [&str; 2] = ["!{workspaceRoot}", "{workspaceRoot}"]; | ||
|
||
fn is_missing_prefix(output: &str) -> bool { | ||
let re = Regex::new(r"^!?\{[\s\S]+\}").expect("Output pattern regex should compile"); | ||
|
||
!re.is_match(output) | ||
} | ||
|
||
#[napi] | ||
pub fn validate_outputs(outputs: Vec<String>) -> anyhow::Result<()> { | ||
let (missing_prefix, workspace_globs) = outputs.iter().fold( | ||
(vec![], vec![]), | ||
|(mut missing_prefix, mut workspace_globs), output| { | ||
if is_missing_prefix(output) { | ||
missing_prefix.push(output); | ||
} | ||
|
||
for prefix in ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES.iter() { | ||
if let Some(trimmed) = output.strip_prefix(prefix) { | ||
if contains_glob_pattern(&trimmed) { | ||
let (root, _) = partition_glob(&trimmed); | ||
if root.is_empty() { | ||
workspace_globs.push(output); | ||
} | ||
} | ||
} | ||
} | ||
|
||
(missing_prefix, workspace_globs) | ||
}, | ||
); | ||
|
||
if workspace_globs.is_empty() && missing_prefix.is_empty() { | ||
return Ok(()); | ||
} | ||
|
||
let mut error_message = String::new(); | ||
if !missing_prefix.is_empty() { | ||
error_message.push_str(&format!( | ||
"The following outputs are invalid: \n - {}\n\nRun `nx repair` to fix this.", | ||
missing_prefix.iter().join("\n - ") | ||
)); | ||
} | ||
if !workspace_globs.is_empty() { | ||
error_message.push_str(&format!( | ||
"The following outputs are defined by a glob pattern from the workspace root: \n - {}\n\nThese can be slow, replace them with a more specific pattern.", | ||
workspace_globs.iter().join("\n - ") | ||
)); | ||
} | ||
|
||
Err(anyhow::anyhow!(error_message)) | ||
} | ||
|
||
#[napi] | ||
pub fn get_transformable_outputs(outputs: Vec<String>) -> Vec<String> { | ||
outputs | ||
.into_iter() | ||
.filter(|output| is_missing_prefix(output)) | ||
.collect() | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::is_missing_prefix; | ||
|
||
#[test] | ||
fn test_is_missing_prefix() { | ||
assert!(is_missing_prefix("dist")); | ||
assert!(is_missing_prefix("!dist")); | ||
assert!(!is_missing_prefix("{workspaceRoot}/dist")); | ||
assert!(!is_missing_prefix("!{workspaceRoot}/dist")); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,14 +3,15 @@ use crate::native::glob::glob_parser::parse_glob; | |||||
use itertools::Itertools; | ||||||
use std::collections::HashSet; | ||||||
|
||||||
use super::contains_glob_pattern; | ||||||
|
||||||
#[derive(Debug)] | ||||||
enum GlobType { | ||||||
Negative(String), | ||||||
Positive(String), | ||||||
} | ||||||
|
||||||
pub fn convert_glob(glob: &str) -> anyhow::Result<Vec<String>> { | ||||||
let (negated, parsed) = parse_glob(glob)?; | ||||||
fn convert_glob_segments(negated: bool, parsed: Vec<Vec<GlobGroup>>) -> Vec<String> { | ||||||
let mut built_segments: Vec<Vec<GlobType>> = Vec::new(); | ||||||
for (index, glob_segment) in parsed.iter().enumerate() { | ||||||
let is_last = index == parsed.len() - 1; | ||||||
|
@@ -58,7 +59,12 @@ pub fn convert_glob(glob: &str) -> anyhow::Result<Vec<String>> { | |||||
.into_iter() | ||||||
.collect::<Vec<_>>(); | ||||||
globs.sort(); | ||||||
Ok(globs) | ||||||
globs | ||||||
} | ||||||
|
||||||
pub fn convert_glob(glob: &str) -> anyhow::Result<Vec<String>> { | ||||||
let (negated, parsed) = parse_glob(glob)?; | ||||||
Ok(convert_glob_segments(negated, parsed)) | ||||||
} | ||||||
|
||||||
fn build_segment( | ||||||
|
@@ -107,6 +113,33 @@ fn build_segment( | |||||
} | ||||||
} | ||||||
|
||||||
pub fn partition_glob(glob: &str) -> (String, Vec<String>) { | ||||||
let (negated, groups) = parse_glob(glob).unwrap(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a
Suggested change
|
||||||
// Partition glob into leading directories and patterns that should be matched | ||||||
let mut leading_dir_segments: Vec<String> = vec![]; | ||||||
let mut pattern_segments = vec![]; | ||||||
groups | ||||||
.into_iter() | ||||||
.filter(|group| !group.is_empty()) | ||||||
.for_each(|group| match &group[0] { | ||||||
GlobGroup::NonSpecial(value) => { | ||||||
if !contains_glob_pattern(&value) && pattern_segments.is_empty() { | ||||||
leading_dir_segments.push(value.to_string()); | ||||||
} else { | ||||||
pattern_segments.push(group); | ||||||
} | ||||||
} | ||||||
_ => { | ||||||
pattern_segments.push(group); | ||||||
} | ||||||
}); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could use |
||||||
|
||||||
( | ||||||
leading_dir_segments.join("/"), | ||||||
convert_glob_segments(negated, pattern_segments), | ||||||
) | ||||||
} | ||||||
|
||||||
#[cfg(test)] | ||||||
mod test { | ||||||
use super::convert_glob; | ||||||
|
@@ -233,4 +266,25 @@ mod test { | |||||
] | ||||||
); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn should_partition_glob_with_leading_dirs() { | ||||||
let (leading_dirs, globs) = super::partition_glob("dist/app/**/!(README|LICENSE).(js|ts)"); | ||||||
assert_eq!(leading_dirs, "dist/app"); | ||||||
assert_eq!(globs, ["!**/{README,LICENSE}.{js,ts}", "**/*.{js,ts}",]); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn should_partition_glob_with_leading_dirs_and_simple_patterns() { | ||||||
let (leading_dirs, globs) = super::partition_glob("dist/app/**/*.css"); | ||||||
assert_eq!(leading_dirs, "dist/app"); | ||||||
assert_eq!(globs, ["**/*.css"]); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn should_partition_glob_with_leading_dirs_and_no_patterns() { | ||||||
let (leading_dirs, globs) = super::partition_glob("dist/app/"); | ||||||
assert_eq!(leading_dirs, "dist/app"); | ||||||
assert_eq!(globs, [] as [String; 0]); | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please test with something like... This should be |
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the multi-threaded walker below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do this some other time.