diff --git a/crates/turborepo-globwalk/src/lib.rs b/crates/turborepo-globwalk/src/lib.rs index 9854ff793cba8..da7678f3dc775 100644 --- a/crates/turborepo-globwalk/src/lib.rs +++ b/crates/turborepo-globwalk/src/lib.rs @@ -27,7 +27,7 @@ pub enum WalkType { } pub use walkdir::Error as WalkDirError; -use wax::walk::Entry; +use wax::walk::{Entry, EntryResidue}; #[derive(Debug, thiserror::Error)] pub enum WalkError { @@ -282,6 +282,23 @@ pub struct GlobError { reason: String, } +#[derive(Debug, Default, Copy, Clone)] +pub struct Settings { + /// Don't recurse into a directory if it contains a `package.json` file. + /// NOTE: If globbing from the root of a workspace, this setting will cause + /// us to not recurse into the individual packages. Therefore, only use this + /// setting if you are globbing in an individual package at not at the + /// workspace root. + ignore_nested_packages: bool, +} + +impl Settings { + pub fn ignore_nested_packages(mut self) -> Self { + self.ignore_nested_packages = true; + self + } +} + /// ValidatedGlob. /// /// Represents an input string that we have either validated or @@ -340,6 +357,18 @@ impl FromStr for ValidatedGlob { } } +pub fn globwalk_with_settings( + base_path: &AbsoluteSystemPath, + include: &[ValidatedGlob], + exclude: &[ValidatedGlob], + walk_type: WalkType, + settings: Settings, +) -> Result, WalkError> { + let include = include.iter().map(|i| i.inner.clone()).collect::>(); + let exclude = exclude.iter().map(|e| e.inner.clone()).collect::>(); + globwalk_internal(base_path, &include, &exclude, walk_type, settings) +} + pub fn globwalk( base_path: &AbsoluteSystemPath, include: &[ValidatedGlob], @@ -348,7 +377,7 @@ pub fn globwalk( ) -> Result, WalkError> { let include = include.iter().map(|i| i.inner.clone()).collect::>(); let exclude = exclude.iter().map(|e| e.inner.clone()).collect::>(); - globwalk_internal(base_path, &include, &exclude, walk_type) + globwalk_internal(base_path, &include, &exclude, walk_type, Default::default()) } #[tracing::instrument] @@ -357,6 +386,7 @@ pub fn globwalk_internal( include: &[String], exclude: &[String], walk_type: WalkType, + settings: Settings, ) -> Result, WalkError> { let (base_path_new, include_paths, exclude_paths) = preprocess_paths_and_globs(base_path, include, exclude)?; @@ -376,7 +406,15 @@ pub fn globwalk_internal( // Use flat_map_iter as we only want parallelism for walking the globs and not iterating // over the results. // See https://docs.rs/rayon/latest/rayon/iter/trait.ParallelIterator.html#method.flat_map_iter - .flat_map_iter(|glob| walk_glob(walk_type, &base_path_new, ex_patterns.clone(), glob)) + .flat_map_iter(|glob| { + walk_glob( + walk_type, + &base_path_new, + ex_patterns.clone(), + glob, + settings, + ) + }) .collect() } @@ -386,16 +424,32 @@ fn walk_glob( base_path_new: &Path, ex_patterns: Vec, glob: Glob, + settings: Settings, ) -> Vec> { - glob.walk(base_path_new) + let iter = glob + .walk(base_path_new) .not(ex_patterns) .unwrap_or_else(|e| { // Per docs, only fails if exclusion list is too large, since we're using // pre-compiled globs panic!("Failed to compile exclusion globs: {}", e,) + }); + + if settings.ignore_nested_packages { + iter.filter_entry(|entry| { + let path = entry.path(); + if path.is_dir() && path != base_path_new && path.join("package.json").exists() { + return Some(EntryResidue::Tree); + } + + None }) .filter_map(|entry| visit_file(walk_type, entry)) .collect::>() + } else { + iter.filter_map(|entry| visit_file(walk_type, entry)) + .collect::>() + } } #[tracing::instrument] @@ -428,7 +482,7 @@ mod test { use crate::{ add_doublestar_to_dir, collapse_path, escape_glob_literals, fix_glob_pattern, globwalk, - ValidatedGlob, WalkError, WalkType, + Settings, ValidatedGlob, WalkError, WalkType, }; #[cfg(unix)] @@ -712,7 +766,8 @@ mod test { &["*.txt"], &[], &["/test.txt"], - &["/test.txt"] + &["/test.txt"], + Default::default() ; "hello world" )] #[test_case( @@ -721,7 +776,8 @@ mod test { &["subdir/test.txt", "test.txt"], &[], &["/subdir/test.txt", "/test.txt"], - &["/subdir/test.txt", "/test.txt"] + &["/subdir/test.txt", "/test.txt"], + Default::default() ; "bullet files" )] #[test_case(&[ @@ -754,7 +810,8 @@ mod test { "/repos/some-app/packages/colors/package.json", "/repos/some-app/packages/faker/package.json", "/repos/some-app/packages/left-pad/package.json", - ] + ], + Default::default() ; "finding workspace package.json files" )] #[test_case(&[ @@ -792,7 +849,8 @@ mod test { "/repos/some-app/packages/colors/package.json", "/repos/some-app/packages/faker/package.json", "/repos/some-app/packages/left-pad/package.json", - ] + ], + Default::default() ; "excludes unexpected workspace package.json files" )] #[test_case(&[ @@ -832,8 +890,26 @@ mod test { "/repos/some-app/packages/left-pad/package.json", "/repos/some-app/packages/xzibit/package.json", "/repos/some-app/packages/xzibit/packages/yo-dawg/package.json", - ] + ], + Default::default() ; "nested packages work")] + #[test_case(&[ + "/external/file.txt", + "/repos/some-app/package.json", + "/repos/some-app/index.js", + "/repos/some-app/just-a-dir/index.js", + "/repos/some-app/packages/xzibit/package.json", + "/repos/some-app/packages/xzibit/index.js", + "/repos/some-app/packages/colors/package.json", + "/repos/some-app/packages/colors/index.js", + ], + "/repos/some-app/", + &["**/*.js"], + &[], + &["/repos/some-app/index.js", "/repos/some-app/just-a-dir/index.js"], + &["/repos/some-app/index.js", "/repos/some-app/just-a-dir/index.js"], + Settings::default().ignore_nested_packages() + ; "ignore nested workspaces setting only matches top level package")] #[test_case(&[ "/external/file.txt", "/repos/some-app/apps/docs/package.json", @@ -871,7 +947,8 @@ mod test { "/repos/some-app/packages/left-pad/package.json", "/repos/some-app/packages/xzibit/package.json", "/repos/some-app/packages/xzibit/packages/yo-dawg/package.json", - ] + ], + Default::default() ; "includes do not override excludes")] #[test_case(&[ "/external/file.txt", @@ -920,7 +997,8 @@ mod test { "/repos/some-app/dist/js/node_modules/browserify.js", "/repos/some-app/public/dist/css/index.css", "/repos/some-app/public/dist/images/rick_astley.jpg", - ] + ], + Default::default() ; "output globbing grabs the desired content" )] #[test_case(&[ @@ -945,7 +1023,8 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] + ], + Default::default() ; "passing ** captures all children")] #[test_case(&[ "/repos/some-app/dist/index.html", @@ -970,7 +1049,8 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] + ], + Default::default() ; "passing just a directory captures no children")] #[test_case(&[ "/repos/some-app/dist/index.html", @@ -990,13 +1070,16 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] ; "redundant includes do not duplicate")] + ], + Default::default() + ; "redundant includes do not duplicate" + )] #[test_case(&[ "/repos/some-app/dist/index.html", "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ], "/repos/some-app/", &["**"], &["**"], &[ ], &[ ] ; "exclude everything, include everything")] + ], "/repos/some-app/", &["**"], &["**"], &[ ], &[ ], Default::default() ; "exclude everything, include everything")] #[test_case(&[ "/repos/some-app/dist/index.html", "/repos/some-app/dist/js/index.js", @@ -1012,7 +1095,8 @@ mod test { ], &[ "/repos/some-app/dist/index.html", - ] + ], + Default::default() ; "passing just a directory to exclude prevents capture of children")] #[test_case(&[ "/repos/some-app/dist/index.html", @@ -1028,7 +1112,8 @@ mod test { "/repos/some-app/dist/index.html", // "/repos/some-app/dist/js", ], - &["/repos/some-app/dist/index.html",] + &["/repos/some-app/dist/index.html",], + Default::default() ; "passing ** to exclude prevents capture of children")] #[test_case(&[ "/repos/some-app/dist/index.html", @@ -1040,7 +1125,8 @@ mod test { &["**"], &["./"], &[], - &[] + &[], + Default::default() ; "exclude everything with folder . applies at base path" )] #[test_case(&[ @@ -1053,7 +1139,8 @@ mod test { &["**"], &["./dist"], &["repos/some-app"], - &[] + &[], + Default::default() ; "exclude everything with traversal applies at a non-base path" )] #[test_case(&[ @@ -1066,7 +1153,8 @@ mod test { &["**"], &["dist/../"], &[], - &[] + &[], + Default::default() ; "exclude everything with folder traversal (..) applies at base path" )] #[test_case(&[ @@ -1091,7 +1179,8 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] + ], + Default::default() ; "traversal works within base path" )] #[test_case(&[ @@ -1117,7 +1206,8 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] + ], + Default::default() ; "self references work (.)" )] #[test_case(&[ @@ -1129,7 +1219,7 @@ mod test { ], "/repos/some-app/", &["*"], &[ ], &[ "/repos/some-app/dist", "/repos/some-app/package.json", - ], &["/repos/some-app/package.json"] ; "depth of 1 includes handles folders properly")] + ], &["/repos/some-app/package.json"], Default::default() ; "depth of 1 includes handles folders properly")] #[test_case(&[ "/repos/some-app/package.json", "/repos/some-app/dist/index.html", @@ -1145,7 +1235,8 @@ mod test { "/repos/some-app/dist", "/repos/some-app/package.json", ], - &["/repos/some-app/package.json"] + &["/repos/some-app/package.json"], + Default::default() ; "depth of 1 excludes prevents capturing folders")] #[test_case(&[ "/repos/some-app/dist/index.html", @@ -1170,7 +1261,8 @@ mod test { "/repos/some-app/dist/js/index.js", "/repos/some-app/dist/js/lib.js", "/repos/some-app/dist/js/node_modules/browserify.js", - ] + ], + Default::default() ; "No-trailing slash basePath works")] #[test_case(&[ "/repos/some-app/included.txt", @@ -1179,7 +1271,7 @@ mod test { "/repos/some-app/included.txt", ], &[ "/repos/some-app/included.txt", - ] ; "exclude single file")] + ], Default::default() ; "exclude single file")] #[test_case(&[ "/repos/some-app/one/included.txt", "/repos/some-app/one/two/included.txt", @@ -1203,7 +1295,7 @@ mod test { "/repos/some-app/one/included.txt", "/repos/some-app/one/two/included.txt", "/repos/some-app/one/two/three/included.txt", - ] ; "exclude nested single file")] + ], Default::default() ; "exclude nested single file")] #[test_case(&[ "/repos/some-app/one/included.txt", "/repos/some-app/one/two/included.txt", @@ -1211,7 +1303,7 @@ mod test { "/repos/some-app/one/excluded.txt", "/repos/some-app/one/two/excluded.txt", "/repos/some-app/one/two/three/excluded.txt", - ], "/repos/some-app", &["**"], &["**"], &[], &[] ; "exclude everything")] + ], "/repos/some-app", &["**"], &["**"], &[], &[], Default::default() ; "exclude everything")] #[test_case(&[ "/repos/some-app/one/included.txt", "/repos/some-app/one/two/included.txt", @@ -1219,7 +1311,7 @@ mod test { "/repos/some-app/one/excluded.txt", "/repos/some-app/one/two/excluded.txt", "/repos/some-app/one/two/three/excluded.txt", - ], "/repos/some-app", &["**"], &["**/"], &[], &[] ; "exclude everything with slash")] + ], "/repos/some-app", &["**"], &["**/"], &[], &[], Default::default() ; "exclude everything with slash")] #[test_case(&[ "/repos/some-app/foo/bar", "/repos/some-app/some-foo/bar", @@ -1234,7 +1326,8 @@ mod test { ], &[ "/repos/some-app/included", - ] + ], + Default::default() ; "exclude everything with leading star" )] #[test_case(&[ @@ -1252,7 +1345,8 @@ mod test { ], &[ "/repos/some-app/included", - ] + ], + Default::default() ; "exclude everything with trailing star" )] fn glob_walk_files( @@ -1262,6 +1356,7 @@ mod test { exclude: &[&str], expected: &[&str], expected_files: &[&str], + settings: Settings, ) { let dir = setup_files(files); let base_path = base_path.trim_start_matches('/'); @@ -1279,7 +1374,9 @@ mod test { (crate::WalkType::Files, expected_files), (crate::WalkType::All, expected), ] { - let success = super::globwalk(&path, &include, &exclude, walk_type).unwrap(); + let success = + super::globwalk_with_settings(&path, &include, &exclude, walk_type, settings) + .unwrap(); let success = success .iter() diff --git a/crates/turborepo-lib/src/boundaries.rs b/crates/turborepo-lib/src/boundaries.rs index d4ba29470d93e..b8a075a9b1aef 100644 --- a/crates/turborepo-lib/src/boundaries.rs +++ b/crates/turborepo-lib/src/boundaries.rs @@ -4,6 +4,7 @@ use std::{ }; use git2::Repository; +use globwalk::Settings; use itertools::Itertools; use miette::{Diagnostic, NamedSource, Report, SourceSpan}; use oxc_resolver::{ResolveError, Resolver}; @@ -189,7 +190,7 @@ impl Run { unresolved_external_dependencies: Option<&BTreeMap>, source_map: &SourceMap, ) -> Result<(usize, Vec), Error> { - let files = globwalk::globwalk( + let files = globwalk::globwalk_with_settings( package_root, &[ "**/*.js".parse().unwrap(), @@ -201,6 +202,7 @@ impl Run { ], &["**/node_modules/**".parse().unwrap()], globwalk::WalkType::Files, + Settings::default().ignore_nested_packages(), )?; let files_checked = files.len(); diff --git a/turborepo-tests/integration/fixtures/boundaries/package.json b/turborepo-tests/integration/fixtures/boundaries/package.json index 83520cd0fc7cb..404d3338e68cf 100644 --- a/turborepo-tests/integration/fixtures/boundaries/package.json +++ b/turborepo-tests/integration/fixtures/boundaries/package.json @@ -8,7 +8,7 @@ }, "packageManager": "npm@10.5.0", "workspaces": [ - "apps/**", - "packages/**" + "apps/*", + "packages/*" ] } diff --git a/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/index.js b/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/index.js new file mode 100644 index 0000000000000..5ccdd557de48e --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/index.js @@ -0,0 +1,2 @@ +// Invalid import but we're in a nested package so it doesn't get detected +import { walkThePlank } from "../../module-package/my-module.mjs"; diff --git a/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/package.json b/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/package.json new file mode 100644 index 0000000000000..36b2111fc8d32 --- /dev/null +++ b/turborepo-tests/integration/fixtures/boundaries/packages/another/nested-package/package.json @@ -0,0 +1,3 @@ +{ + "name": "nested-package" +}