diff --git a/crates/turborepo-lockfiles/src/berry/mod.rs b/crates/turborepo-lockfiles/src/berry/mod.rs index 3713bf5c0d936..c6417a22a3157 100644 --- a/crates/turborepo-lockfiles/src/berry/mod.rs +++ b/crates/turborepo-lockfiles/src/berry/mod.rs @@ -774,6 +774,7 @@ mod test { &lockfile, "apps/docs", HashMap::from_iter(vec![("lodash".into(), "^4.17.21".into())]), + false, ) .unwrap(); @@ -883,7 +884,7 @@ mod test { .map(|(k, v)| (k.to_string(), v.to_string())) .collect(); - let closure = transitive_closure(&lockfile, "packages/ui", unresolved_deps).unwrap(); + let closure = transitive_closure(&lockfile, "packages/ui", unresolved_deps, false).unwrap(); assert!(closure.contains(&Package { key: "ajv@npm:8.11.2".into(), @@ -1063,6 +1064,7 @@ mod test { )] .into_iter() .collect(), + false, ) .unwrap(); diff --git a/crates/turborepo-lockfiles/src/lib.rs b/crates/turborepo-lockfiles/src/lib.rs index e62d0b35e32ff..9434bc1b1a93c 100644 --- a/crates/turborepo-lockfiles/src/lib.rs +++ b/crates/turborepo-lockfiles/src/lib.rs @@ -69,11 +69,17 @@ pub trait Lockfile: Send + Sync + Any + std::fmt::Debug { pub fn all_transitive_closures( lockfile: &L, workspaces: HashMap>, + ignore_missing_packages: bool, ) -> Result>, Error> { workspaces .into_par_iter() .map(|(workspace, unresolved_deps)| { - let closure = transitive_closure(lockfile, &workspace, unresolved_deps)?; + let closure = transitive_closure( + lockfile, + &workspace, + unresolved_deps, + ignore_missing_packages, + )?; Ok((workspace, closure)) }) .collect() @@ -85,6 +91,7 @@ pub fn transitive_closure( lockfile: &L, workspace_path: &str, unresolved_deps: HashMap, + ignore_missing_packages: bool, ) -> Result, Error> { let mut transitive_deps = HashSet::new(); transitive_closure_helper( @@ -92,6 +99,7 @@ pub fn transitive_closure( workspace_path, unresolved_deps, &mut transitive_deps, + ignore_missing_packages, )?; Ok(transitive_deps) @@ -102,9 +110,16 @@ fn transitive_closure_helper( workspace_path: &str, unresolved_deps: HashMap>, resolved_deps: &mut HashSet, + ignore_missing_packages: bool, ) -> Result<(), Error> { for (name, specifier) in unresolved_deps { - let pkg = lockfile.resolve_package(workspace_path, &name, specifier.as_ref())?; + let pkg = match lockfile.resolve_package(workspace_path, &name, specifier.as_ref()) { + Ok(pkg) => pkg, + Err(Error::MissingWorkspace(_)) if ignore_missing_packages => { + continue; + } + Err(e) => return Err(e), + }; match pkg { None => { @@ -117,7 +132,15 @@ fn transitive_closure_helper( let all_deps = lockfile.all_dependencies(&pkg.key)?; resolved_deps.insert(pkg); if let Some(deps) = all_deps { - transitive_closure_helper(lockfile, workspace_path, deps, resolved_deps)?; + // we've already found one unresolved dependency, so we can't ignore its set of + // dependencies. + transitive_closure_helper( + lockfile, + workspace_path, + deps, + resolved_deps, + false, + )?; } } } diff --git a/crates/turborepo-lockfiles/src/npm.rs b/crates/turborepo-lockfiles/src/npm.rs index bc7f2f7daecef..191550f608135 100644 --- a/crates/turborepo-lockfiles/src/npm.rs +++ b/crates/turborepo-lockfiles/src/npm.rs @@ -440,6 +440,7 @@ mod test { ] .into_iter() .collect(), + false, )?; assert!(closures.get("packages/a").unwrap().contains(&Package { key: "node_modules/eslint-plugin-turbo".into(), diff --git a/crates/turborepo-lockfiles/src/pnpm/data.rs b/crates/turborepo-lockfiles/src/pnpm/data.rs index 9ff2cfd07184f..ea028cf0eabe0 100644 --- a/crates/turborepo-lockfiles/src/pnpm/data.rs +++ b/crates/turborepo-lockfiles/src/pnpm/data.rs @@ -966,6 +966,7 @@ mod tests { )] .into_iter() .collect(), + false, ) .unwrap(); @@ -1037,6 +1038,7 @@ c: )] .into_iter() .collect(), + false, ) .unwrap(); @@ -1113,7 +1115,7 @@ c: .map(|(k, v)| (k.to_owned(), v.to_owned())) .collect(), ); - let mut closures: Vec<_> = crate::all_transitive_closures(&lockfile, workspaces) + let mut closures: Vec<_> = crate::all_transitive_closures(&lockfile, workspaces, false) .unwrap() .into_iter() .map(|(k, v)| (k, v.into_iter().sorted().collect::>())) diff --git a/crates/turborepo-repository/src/package_graph/builder.rs b/crates/turborepo-repository/src/package_graph/builder.rs index d4a3ea7ac3f32..1bf6caa82cbd4 100644 --- a/crates/turborepo-repository/src/package_graph/builder.rs +++ b/crates/turborepo-repository/src/package_graph/builder.rs @@ -507,9 +507,12 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedLockfile, T> { return Ok(()); }; + // We cannot ignore missing packages in this context, it would indicate a + // malformed or stale lockfile. let mut closures = turborepo_lockfiles::all_transitive_closures( lockfile, self.all_external_dependencies()?, + false, )?; for (_, entry) in self.workspaces.iter_mut() { entry.transitive_dependencies = closures.remove(&entry.unix_dir_str()?); diff --git a/crates/turborepo-repository/src/package_graph/mod.rs b/crates/turborepo-repository/src/package_graph/mod.rs index 674e03a5686a9..eb8a902adcf3e 100644 --- a/crates/turborepo-repository/src/package_graph/mod.rs +++ b/crates/turborepo-repository/src/package_graph/mod.rs @@ -345,7 +345,12 @@ impl PackageGraph { }) .collect::>>(); - let closures = turborepo_lockfiles::all_transitive_closures(previous, external_deps)?; + // We're comparing to a previous lockfile, it's possible that a package was + // added and thus won't exist in the previous lockfile. In that case, + // we're fine to ignore it. Assuming there is not a commit with a stale + // lockfile, the same commit should add the package, so it will get + // picked up as changed. + let closures = turborepo_lockfiles::all_transitive_closures(previous, external_deps, true)?; let global_change = current.global_change(previous); diff --git a/turborepo-tests/integration/tests/lockfile-aware-caching/new-package.t b/turborepo-tests/integration/tests/lockfile-aware-caching/new-package.t new file mode 100644 index 0000000000000..f23135e461c62 --- /dev/null +++ b/turborepo-tests/integration/tests/lockfile-aware-caching/new-package.t @@ -0,0 +1,22 @@ +Setup + $ . ${TESTDIR}/../../../helpers/setup.sh + $ . ${TESTDIR}/setup.sh $(pwd) pnpm + +Add new package with an external dependency + $ mkdir -p apps/c + $ echo '{"name":"c", "dependencies": {"has-symbols": "^1.0.3"}}' > apps/c/package.json + +Update lockfile + $ pnpm i --frozen-lockfile=false > /dev/null + +Now build and verify that only the new package is in scope +Note that we need --skip-infer because we've now installed a local +turbo in this repo +Note that we need to disable path conversion because on windows, git bash considers +'//' to be an escape sequence translating to '/'. + $ MSYS_NO_PATHCONV=1 ${TURBO} --skip-infer build -F '[HEAD]' -F '!//' --dry=json | jq '.packages' + [ + "c" + ] + +