Handle new packages in lockfile comparisons

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();
 

crates/turborepo-lockfiles/src/lib.rs

@@ -69,11 +69,17 @@ pub trait Lockfile: Send + Sync + Any + std::fmt::Debug {
 pub fn all_transitive_closures<L: Lockfile + ?Sized>(
     lockfile: &L,
     workspaces: HashMap<String, HashMap<String, String>>,
+    ignore_missing_packages: bool,
 ) -> Result<HashMap<String, HashSet<Package>>, 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,13 +91,15 @@ pub fn transitive_closure<L: Lockfile + ?Sized>(
     lockfile: &L,
     workspace_path: &str,
     unresolved_deps: HashMap<String, String>,
+    ignore_missing_packages: bool,
 ) -> Result<HashSet<Package>, Error> {
     let mut transitive_deps = HashSet::new();
     transitive_closure_helper(
         lockfile,
         workspace_path,
         unresolved_deps,
         &mut transitive_deps,
+        ignore_missing_packages,
     )?;
 
     Ok(transitive_deps)
@@ -102,9 +110,16 @@ fn transitive_closure_helper<L: Lockfile + ?Sized>(
     workspace_path: &str,
     unresolved_deps: HashMap<String, impl AsRef<str>>,
     resolved_deps: &mut HashSet<Package>,
+    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<L: Lockfile + ?Sized>(
                 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,
+                    )?;
                 }
             }
         }

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(),

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::<Vec<_>>()))

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()?);

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

@@ -345,7 +345,12 @@ impl PackageGraph {
             })
             .collect::<HashMap<_, HashMap<_, _>>>();
 
-        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);
 

turborepo-tests/integration/tests/lockfile-aware-caching/new-package.t

@@ -0,0 +1,20 @@
+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 > /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
+  $ ${TURBO} --skip-infer build -F '[HEAD]' -F '!//' --dry=json | jq '.packages' 
+  [
+    "c"
+  ]
+
+