Fix/filter unnamed packages (#7280)

crates/turborepo-repository/src/lib.rs

@@ -7,4 +7,3 @@ pub mod inference;
 pub mod package_graph;
 pub mod package_json;
 pub mod package_manager;
-mod util;

crates/turborepo-repository/src/package_graph/builder.rs

@@ -281,7 +281,18 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
  }?;
 
  for (path, json) in package_jsons {
- self.add_json(path, json)?;
+ match self.add_json(path, json) {
+ Ok(()) => {}
+ Err(Error::PackageJsonMissingName(path)) => {
+ // previous implementations of turbo would silently ignore package.json files
+ // that didn't have a name field (well, actually, if two or more had the same
+ // name, it would throw a 'name clash' error, but that's a different story)
+ //
+ // let's try to match that behavior, but log a debug message
+ tracing::debug!("ignoring package.json at {} since it has no name", path);
+ }
+ Err(err) => return Err(err),
+ }
  }
 
  let Self {

crates/turborepo-repository/src/package_manager/mod.rs

@@ -26,7 +26,6 @@ use crate::{
  discovery,
  package_json::PackageJson,
  package_manager::{bun::BunDetector, npm::NpmDetector, pnpm::PnpmDetector, yarn::YarnDetector},
- util::IsLast,
 };
 
 #[derive(Debug, Deserialize)]
@@ -484,14 +483,7 @@ impl PackageManager {
  &globs.validated_exclusions,
  globwalk::WalkType::Files,
  )?;
-
- // we need to remove package.json files that are in subfolders of others so that
- // we don't yield subpackages. sort, keep track of the parent of last
- // json we encountered, and only yield it if it's not a subfolder of it
- //
- // ideally we would do this during traversal, but walkdir doesn't support
- // inorder traversal so we can't
- Ok(filter_subfolder_package_jsons(files))
+ Ok(files.into_iter())
  }
 
  pub fn lockfile_name(&self) -> &'static str {
@@ -602,56 +594,12 @@ impl PackageManager {
  }
 }
 
-fn filter_subfolder_package_jsons<T: IntoIterator<Item = AbsoluteSystemPathBuf>>(
- map: T,
-) -> impl Iterator<Item = AbsoluteSystemPathBuf> {
- let mut last_parent = None;
- map.into_iter()
- .sorted_by(|a, b| {
- // get an iterator of the components of each path, and zip them together
- let mut segments = a.components().with_last().zip(b.components().with_last());
-
- // find the first pair of components that are different, and compare them
- // if one of the segments is the last, then the other is a subfolder of it.
- // we must always yield 'file-likes' (the last segment of a path) ahead of
- // subfolders (non-last segments) so that we can guarantee we find the
- // package.json before processing its subfolders
- segments
- .find_map(|((a_last, a_cmp), (b_last, b_cmp))| {
- if a_last == b_last {
- match a_cmp.cmp(&b_cmp) {
- std::cmp::Ordering::Equal => None,
- other => Some(other),
- }
- } else if a_last {
- Some(std::cmp::Ordering::Less)
- } else {
- Some(std::cmp::Ordering::Greater)
- }
- })
- .unwrap_or(std::cmp::Ordering::Equal)
- })
- .filter(move |entry| {
- match &last_parent {
- // last_parent is the parent of the last json we yielded. if the current
- // entry is a subfolder of it, we don't want to yield it
- Some(parent) if entry.starts_with(parent) => false,
- // update last_parent to the parent of the current entry
- _ => {
- last_parent = Some(entry.parent().unwrap().to_owned());
- true
- }
- }
- })
-}
-
 #[cfg(test)]
 mod tests {
- use std::{borrow::Cow, collections::HashSet, fs::File};
+ use std::{collections::HashSet, fs::File};
 
  use pretty_assertions::assert_eq;
  use tempfile::tempdir;
- use test_case::test_case;
  use turbopath::AbsoluteSystemPathBuf;
 
  use super::*;
@@ -674,49 +622,6 @@ mod tests {
  panic!("Couldn't find Turborepo root from {}", cwd);
  }
 
- #[test_case(&[
- "/a/b/package.json",
- "/a/package.json",
- ], &[
- "/a/package.json",
- ] ; "basic")]
- #[test_case(&[
- "/a/package.json",
- "/a/b/package.json",
- ], &[
- "/a/package.json",
- ] ; "order flipped")]
- #[test_case(&[
- "/a/package.json",
- "/b/package.json",
- ], &[
- "/a/package.json",
- "/b/package.json",
- ] ; "disjoint")]
- #[test_case(&[
- "/a/package.json",
- "/z/package.json",
- "/package.json"
- ], &[
- "/package.json",
- ] ; "root")]
- fn lexicographic_file_sort(inc: &[&str], expected: &[&str]) {
- let to_path = |s: &&str| {
- AbsoluteSystemPathBuf::new(if cfg!(windows) {
- Cow::from(format!("C:/{}", s))
- } else {
- (*s).into()
- })
- .unwrap()
- };
-
- let inc = inc.into_iter().map(to_path).collect::<Vec<_>>();
- let expected = expected.into_iter().map(to_path).collect::<Vec<_>>();
- let sorted = filter_subfolder_package_jsons(inc);
- let sorted = sorted.collect::<Vec<_>>();
- assert_eq!(sorted, expected);
- }
-
  #[test]
  fn test_get_package_jsons() {
  let root = repo_root();

turborepo-tests/integration/tests/run/nested-packages.t → turborepo-tests/integration/tests/run/unnamed-packages.t

@@ -1,9 +1,8 @@
 Setup
  $ . ${TESTDIR}/../../../helpers/setup_integration_test.sh nested_packages
 
-Run a build. In this test, the fixture is set up so that the build will
-if we read nested packages, in a manner similar to the `.next` folder
-since it will not always produce a `name` field.
+Run a build. In this test, the fixture is set up so that the nested package
+which does not have a name should be ignored. We should process it but filter.
  $ ${TURBO} build
  \xe2\x80\xa2 Packages in scope: my-app (esc)
  \xe2\x80\xa2 Running build in 1 packages (esc)