fix(core): handle invalid group glob groups (#21027)

This commit is contained in:
Jonathan Cammisuli 2024-01-15 14:34:46 -05:00 committed by GitHub
parent c7c5a6d058
commit c10af4d9d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 184 additions and 13 deletions

View File

@ -172,7 +172,7 @@ Here's how we could define that, starting from our default situation:
{
"namedInputs": {
"default": ["{projectRoot}/**/*"],
"production": ["default", "!{projectRoot}/**/?(*.)+spec.ts"]
"production": ["default", "!{projectRoot}/**/?(*.)+(spec|test).ts"]
},
"targetDefaults": {
"build": {

View File

@ -4,6 +4,7 @@ import { NxJsonConfiguration } from '../config/nx-json';
import { createTaskGraph } from '../tasks-runner/create-task-graph';
import { NativeTaskHasherImpl } from './native-task-hasher-impl';
import { ProjectGraphBuilder } from '../project-graph/project-graph-builder';
import { testOnlyTransferFileMap } from '../native';
describe('native task hasher', () => {
let tempFs: TempFs;
@ -653,4 +654,41 @@ describe('native task hasher', () => {
}
`);
});
/**
* commented out to show how to debug issues with hashing
*
*
*
* gather the project graph + task graph with `nx run project:target --graph=graph.json`
* gather the file-map.json from `.nx/cache/file-map.json`
* gather the nx.json file
*/
// it('should test client workspaces', async () => {
// let nxJson = require('nx.json');
// let graphs = require('graph.json');
// let projectGraph = graphs.graph;
// let taskGraph = graphs.tasks;
//
// let files = require('file-map.json');
// let projectFiles = files.fileMap.projectFileMap;
// let nonProjectFiles = files.fileMap.nonProjectFiles;
//
// let transferred = testOnlyTransferFileMap(projectFiles, nonProjectFiles);
//
// let hasher = new NativeTaskHasherImpl(
// '',
// nxJson,
// projectGraph,
// transferred,
// { selectivelyHashTsConfig: false }
// );
//
// const hashes = await hasher.hashTasks(
// Object.values(taskGraph.tasks),
// taskGraph,
// {}
// );
// console.dir(hashes, { depth: null });
// });
});

View File

@ -359,4 +359,20 @@ mod test {
assert!(glob_set.is_match("packages/package-c/package.json"));
assert!(!glob_set.is_match("packages/package-a/package.json"));
}
#[test]
fn should_handle_invalid_group_globs() {
let glob_set = build_glob_set(&[
"libs/**/*",
"!libs/**/?(*.)+spec.ts?(.snap)",
"!libs/tsconfig.spec.json",
"!libs/jest.config.ts",
"!libs/.eslintrc.json",
"!libs/**/test-setup.ts",
])
.unwrap();
assert!(glob_set.is_match("libs/src/index.ts"));
assert!(!glob_set.is_match("libs/src/index.spec.ts"));
}
}

View File

@ -8,6 +8,24 @@ use nom::sequence::{preceded, terminated};
use nom::{Finish, IResult};
use std::borrow::Cow;
/// Consumes special characters if they are not part of a group, otherwise returns an error
/// Example:
/// - ?snap -> snap
/// - +snap -> snap
/// - @snap -> snap
///
fn special_char_with_no_group(input: &str) -> IResult<&str, &str, VerboseError<&str>> {
context("special_char_with_no_group", |input| {
// check the input if it has ?,+, or @
let (alt_input, _) = alt((tag("?"), tag("+"), tag("@")))(input)?;
// check the remaining input from the previous parser to see if the next character is (
// if it is, then we know that the special character is part of a group, and we can return an Err here
let _ = is_not("(")(alt_input)?;
// consume the special character and return the rest of the alt input
Ok((alt_input, ""))
})(input)
}
fn simple_group(input: &str) -> IResult<&str, GlobGroup, VerboseError<&str>> {
context(
"simple_group",
@ -121,8 +139,14 @@ fn parse_segment(input: &str) -> IResult<&str, Vec<GlobGroup>, VerboseError<&str
context(
"parse_segment",
many_till(
context(
"glob_group",
context("glob_group", |input| {
// check if the special character is part of a group
let group_input = match special_char_with_no_group(input) {
// if there was no (, then we know that the special character is not part of a group, we can return this input
Ok((no_group_input, _)) => no_group_input,
// otherwise, there was a ( after the special character, so we need to parse the original input
Err(_) => input,
};
alt((
simple_group,
zero_or_more_group,
@ -134,8 +158,8 @@ fn parse_segment(input: &str) -> IResult<&str, Vec<GlobGroup>, VerboseError<&str
negated_group,
brace_group_with_empty_item,
non_special_character,
)),
),
))(group_input)
}),
eof,
),
)(input)
@ -176,7 +200,29 @@ pub fn parse_glob(input: &str) -> anyhow::Result<(bool, Vec<Vec<GlobGroup>>)> {
mod test {
use crate::native::glob::glob_group::GlobGroup;
use crate::native::glob::glob_parser::parse_glob;
use crate::native::glob::glob_parser::{parse_glob, special_char_with_no_group};
#[test]
fn invalid_groups() {
let result = special_char_with_no_group("?snap").unwrap();
assert_eq!(result, ("snap", ""));
// assert_eq!(result, ("?", "snap"));
let result = parse_glob("libs/?(*.)+spec.ts?(.snap)").unwrap();
assert_eq!(
result,
(
false,
vec![
vec![GlobGroup::NonSpecial("libs".into())],
vec![
GlobGroup::ZeroOrOne("*.".into()),
GlobGroup::NonSpecial("spec.ts".into()),
GlobGroup::ZeroOrOne(".snap".into())
]
]
)
);
}
#[test]
fn should_parse_globs() {

View File

@ -198,4 +198,39 @@ mod test {
]
);
}
#[test]
fn should_convert_globs_with_invalid_groups() {
let globs = convert_glob("libs/**/?(*.)+spec.ts?(.snap)").unwrap();
assert_eq!(
globs,
[
"libs/**/*.spec.ts",
"libs/**/*.spec.ts.snap",
"libs/**/spec.ts",
"libs/**/spec.ts.snap"
]
);
let globs = convert_glob("libs/**/?(*.)@spec.ts?(.snap)").unwrap();
assert_eq!(
globs,
[
"libs/**/*.spec.ts",
"libs/**/*.spec.ts.snap",
"libs/**/spec.ts",
"libs/**/spec.ts.snap"
]
);
let globs = convert_glob("libs/**/?(*.)?spec.ts?(.snap)").unwrap();
assert_eq!(
globs,
[
"libs/**/*.spec.ts",
"libs/**/*.spec.ts.snap",
"libs/**/spec.ts",
"libs/**/spec.ts.snap"
]
);
}
}

View File

@ -139,6 +139,7 @@ export interface FileMap {
projectFileMap: ProjectFiles
nonProjectFiles: Array<FileData>
}
export function testOnlyTransferFileMap(projectFiles: Record<string, Array<FileData>>, nonProjectFiles: Array<FileData>): NxWorkspaceFilesExternals
export class ImportResult {
file: string
sourceProject: string

View File

@ -246,7 +246,7 @@ if (!nativeBinding) {
throw new Error(`Failed to load native binding`)
}
const { expandOutputs, getFilesForOutputs, remove, copy, hashArray, hashFile, ImportResult, findImports, transferProjectGraph, HashPlanner, TaskHasher, EventType, Watcher, WorkspaceContext, WorkspaceErrors } = nativeBinding
const { expandOutputs, getFilesForOutputs, remove, copy, hashArray, hashFile, ImportResult, findImports, transferProjectGraph, HashPlanner, TaskHasher, EventType, Watcher, WorkspaceContext, WorkspaceErrors, testOnlyTransferFileMap } = nativeBinding
module.exports.expandOutputs = expandOutputs
module.exports.getFilesForOutputs = getFilesForOutputs
@ -263,3 +263,4 @@ module.exports.EventType = EventType
module.exports.Watcher = Watcher
module.exports.WorkspaceContext = WorkspaceContext
module.exports.WorkspaceErrors = WorkspaceErrors
module.exports.testOnlyTransferFileMap = testOnlyTransferFileMap

View File

@ -81,6 +81,13 @@ where
}
}
/// Enable logging for the native module
/// You can set log levels and different logs by setting the `NX_NATIVE_LOGGING` environment variable
/// Examples:
/// - `NX_NATIVE_LOGGING=trace|warn|debug|error|info` - enable all logs for all crates and modules
/// - `NX_NATIVE_LOGGING=nx=trace` - enable all logs for the `nx` (this) crate
/// - `NX_NATIVE_LOGGING=nx::native::tasks::hashers::hash_project_files=trace` - enable all logs for the `hash_project_files` module
/// - `NX_NATIVE_LOGGING=[{project_name=project}]` - enable logs that contain the project in its span
pub(crate) fn enable_logger() {
let env_filter =
EnvFilter::try_from_env("NX_NATIVE_LOGGING").unwrap_or_else(|_| EnvFilter::new("ERROR"));

View File

@ -1,9 +1,11 @@
use crate::native::project_graph::types::ProjectGraph;
use napi::bindgen_prelude::External;
use crate::native::project_graph::types::ProjectGraph;
#[napi]
/// Transfer the project graph from the JS world to the Rust world, so that we can pass the project graph via memory quicker
/// This wont be needed once the project graph is created in Rust
pub fn transfer_project_graph(project_graph: ProjectGraph) -> External<ProjectGraph> {
External::new(project_graph)
}

View File

@ -1,7 +1,7 @@
use std::collections::HashMap;
use anyhow::*;
use tracing::trace;
use tracing::{trace, trace_span};
use crate::native::glob::build_glob_set;
use crate::native::types::FileData;
@ -12,7 +12,9 @@ pub fn hash_project_files(
file_sets: &[String],
project_file_map: &HashMap<String, Vec<FileData>>,
) -> Result<String> {
let _span = trace_span!("hash_project_files", project_name).entered();
let collected_files = collect_files(project_name, project_root, file_sets, project_file_map)?;
trace!("collected_files: {:?}", collected_files.len());
let mut hasher = xxhash_rust::xxh3::Xxh3::new();
for file in collected_files {
hasher.update(file.hash.as_bytes());
@ -39,11 +41,12 @@ fn collect_files<'a>(
.collect::<Vec<_>>();
let now = std::time::Instant::now();
let glob_set = build_glob_set(&globs)?;
trace!("build_glob_set for {}: {:?}", project_name, now.elapsed());
trace!("build_glob_set for {:?}", now.elapsed());
project_file_map.get(project_name).map_or_else(
|| Err(anyhow!("project {} not found", project_name)),
|files| {
trace!("files: {:?}", files.len());
let now = std::time::Instant::now();
let hashes = files
.iter()

View File

@ -2,7 +2,7 @@ use napi::bindgen_prelude::External;
use std::collections::HashMap;
use crate::native::hasher::hash;
use crate::native::utils::{Normalize, path::get_child_files};
use crate::native::utils::{path::get_child_files, Normalize};
use rayon::prelude::*;
use std::ops::Deref;
use std::path::{Path, PathBuf};
@ -302,7 +302,6 @@ impl WorkspaceContext {
#[napi]
pub fn get_files_in_directory(&self, directory: String) -> Vec<String> {
get_child_files(&directory, self.files_worker
.get_files())
get_child_files(&directory, self.files_worker.get_files())
}
}

View File

@ -1,3 +1,8 @@
use crate::native::types::FileData;
use crate::native::workspace::types::NxWorkspaceFilesExternals;
use napi::bindgen_prelude::External;
use std::collections::HashMap;
pub mod config_files;
pub mod context;
mod errors;
@ -5,3 +10,21 @@ mod files_archive;
mod files_hashing;
pub mod types;
pub mod workspace_files;
#[napi]
// should only be used in tests to transfer the file map from the JS world to the Rust world
pub fn __test_only_transfer_file_map(
project_files: HashMap<String, Vec<FileData>>,
non_project_files: Vec<FileData>,
) -> NxWorkspaceFilesExternals {
let all_workspace_files = project_files
.iter()
.flat_map(|(_, files)| files.clone())
.chain(non_project_files.clone())
.collect::<Vec<FileData>>();
NxWorkspaceFilesExternals {
project_files: External::new(project_files),
global_files: External::new(non_project_files),
all_workspace_files: External::new(all_workspace_files),
}
}