vercel · dimitropoulos · Mar 5, 2025 · Feb 13, 2025 · Feb 13, 2025 · Feb 13, 2025
@@ -15,7 +15,9 @@ use turborepo_filewatch::{
     NotifyError, OptionalWatch,
 };
 use turborepo_repository::{
-    change_mapper::{ChangeMapper, GlobalDepsPackageChangeMapper, PackageChanges},
+    change_mapper::{
+        ChangeMapper, GlobalDepsPackageChangeMapper, LockfileContents, PackageChanges,
+    },
     package_graph::{PackageGraph, PackageGraphBuilder, PackageName, WorkspacePackage},
     package_json::PackageJson,
 };
@@ -342,7 +344,8 @@ impl Subscriber {
                     continue;
                 }
 
-                let changed_packages = change_mapper.changed_packages(changed_files.clone(), None);
+                let changed_packages = change_mapper
+                    .changed_packages(changed_files.clone(), LockfileContents::Unchanged);
 
                 tracing::warn!("changed_files: {:?}", changed_files);
                 tracing::warn!("changed_packages: {:?}", changed_packages);

@@ -5,7 +5,7 @@ use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
 use turborepo_repository::{
     change_mapper::{
         AllPackageChangeReason, ChangeMapper, DefaultPackageChangeMapper, Error,
-        GlobalDepsPackageChangeMapper, PackageChanges, PackageInclusionReason,
+        GlobalDepsPackageChangeMapper, LockfileContents, PackageChanges, PackageInclusionReason,
     },
     package_graph::{PackageGraph, PackageName},
 };
@@ -52,13 +52,12 @@ impl<'a> ScopeChangeDetector<'a> {
     }
 
     /// Gets the lockfile content from SCM if it has changed.
-    /// Does *not* error if cannot get content, instead just
-    /// returns a Some(None)
-    fn get_lockfile_contents(
+    /// Does *not* error if cannot get content.
+    pub fn get_lockfile_contents(
         &self,
         from_ref: Option<&str>,
         changed_files: &HashSet<AnchoredSystemPathBuf>,
-    ) -> Option<Option<Vec<u8>>> {
+    ) -> LockfileContents {
         let lockfile_path = self
             .pkg_graph
             .package_manager()
@@ -70,23 +69,21 @@ impl<'a> ScopeChangeDetector<'a> {
             &lockfile_path,
         ) {
             debug!("lockfile did not change");
-            return None;
+            return LockfileContents::Unchanged;
         }
 
-        let lockfile_path = self
-            .pkg_graph
-            .package_manager()
-            .lockfile_path(self.turbo_root);
-
         let Ok(content) = self.scm.previous_content(from_ref, &lockfile_path) else {
-            return Some(None);
+            debug!("lockfile did change but could not get previous content");
+            return LockfileContents::UnknownChange;
         };
 
-        Some(Some(content))
+        debug!("lockfile changed, have the previous content");
+        LockfileContents::Changed(content)
     }
 }
 
 impl<'a> GitChangeDetector for ScopeChangeDetector<'a> {
+    /// get the actual changed packages between two git refs
     fn changed_packages(
         &self,
         from_ref: Option<&str>,

@@ -17,6 +17,7 @@ biome_diagnostics = { workspace = true }
 biome_json_parser = { workspace = true }
 biome_json_syntax = { workspace = true }
 
+either = { workspace = true }
 globwalk = { version = "0.1.0", path = "../turborepo-globwalk" }
 itertools = { workspace = true }
 lazy-regex = "2.5.0"

@@ -7,8 +7,8 @@ use std::{
 };
 
 pub use package::{
-    DefaultPackageChangeMapper, Error, GlobalDepsPackageChangeMapper, PackageChangeMapper,
-    PackageMapping,
+    DefaultPackageChangeMapper, DefaultPackageChangeMapperWithLockfile, Error,
+    GlobalDepsPackageChangeMapper, PackageChangeMapper, PackageMapping,
 };
 use tracing::debug;
 use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf};
@@ -29,6 +29,19 @@ pub enum LockfileChange {
     ChangedPackages(HashSet<turborepo_lockfiles::Package>),
 }
 
+/// This describes the state of a change to a lockfile.
+pub enum LockfileContents {
+    /// We know the lockfile did not change
+    Unchanged,
+    /// We know the lockfile changed but don't have the file contents of the
+    /// previous lockfile (i.e. `git status`, or perhaps a lockfile that was
+    /// deleted or otherwise inaccessible with the information we have)
+    UnknownChange,
+    /// We know the lockfile changed and have the contents of the previous
+    /// lockfile
+    Changed(Vec<u8>),
+}
+
 #[derive(Debug, PartialEq, Eq, Hash, Clone)]
 pub enum PackageInclusionReason {
     /// All the packages are invalidated
@@ -122,13 +135,7 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
     pub fn changed_packages(
         &self,
         changed_files: HashSet<AnchoredSystemPathBuf>,
-        // None - we don't know if the lockfile changed
-        //
-        // Some(None) - we know the lockfile changed, but don't know exactly why (i.e. `git status`
-        // and the lockfile is there)
-        //
-        // Some(Some(content)) - we know the lockfile changed and have the contents
-        lockfile_change: Option<Option<Vec<u8>>>,
+        lockfile_contents: LockfileContents,
     ) -> Result<PackageChanges, ChangeMapError> {
         if let Some(file) = Self::default_global_file_changed(&changed_files) {
             debug!("global file changed");
@@ -142,15 +149,16 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
         // get filtered files and add the packages that contain them
         let filtered_changed_files = self.filter_ignored_files(changed_files.iter())?;
 
+        // calculate lockfile_change here based on changed_files
         match self.get_changed_packages(filtered_changed_files.into_iter()) {
             PackageChanges::All(reason) => Ok(PackageChanges::All(reason)),
 
             PackageChanges::Some(mut changed_pkgs) => {
-                match lockfile_change {
-                    Some(Some(content)) => {
+                match lockfile_contents {
+                    LockfileContents::Changed(previous_lockfile_contents) => {
                         // if we run into issues, don't error, just assume all packages have changed
                         let Ok(lockfile_changes) =
-                            self.get_changed_packages_from_lockfile(&content)
+                            self.get_changed_packages_from_lockfile(&previous_lockfile_contents)
                         else {
                             debug!(
                                 "unable to determine lockfile changes, assuming all packages \
@@ -183,13 +191,22 @@ impl<'a, PD: PackageChangeMapper> ChangeMapper<'a, PD> {
                     }
 
                     // We don't have the actual contents, so just invalidate everything
-                    Some(None) => {
-                        debug!("no previous lockfile available, assuming all packages changed");
+                    LockfileContents::UnknownChange => {
+                        // this can happen in a blobless checkout
+                        debug!(
+                            "we know the lockfile changed but we don't have the contents so we \
+                             have to assume all packages changed and rebuild everything"
+                        );
                         Ok(PackageChanges::All(
                             AllPackageChangeReason::LockfileChangedWithoutDetails,
                         ))
                     }
-                    None => Ok(PackageChanges::Some(changed_pkgs)),
+
+                    // We don't know if the lockfile changed or not, so we can't assume anything
+                    LockfileContents::Unchanged => {
+                        debug!("the lockfile did not change");
+                        Ok(PackageChanges::Some(changed_pkgs))
+                    }
                 }
             }
         }

@@ -24,6 +24,19 @@ pub trait PackageChangeMapper {
     fn detect_package(&self, file: &AnchoredSystemPath) -> PackageMapping;
 }
 
+impl<L, R> PackageChangeMapper for either::Either<L, R>
+where
+    L: PackageChangeMapper,
+    R: PackageChangeMapper,
+{
+    fn detect_package(&self, file: &AnchoredSystemPath) -> PackageMapping {
+        match self {
+            either::Either::Left(l) => l.detect_package(file),
+            either::Either::Right(r) => r.detect_package(file),
+        }
+    }
+}
+
 /// Detects package by checking if the file is inside the package.
 ///
 /// Does *not* use the `globalDependencies` in turbo.json.
@@ -73,6 +86,43 @@ impl PackageChangeMapper for DefaultPackageChangeMapper<'_> {
     }
 }
 
+pub struct DefaultPackageChangeMapperWithLockfile<'a> {
+    base: DefaultPackageChangeMapper<'a>,
+}
+
+impl<'a> DefaultPackageChangeMapperWithLockfile<'a> {
+    pub fn new(pkg_dep_graph: &'a PackageGraph) -> Self {
+        Self {
+            base: DefaultPackageChangeMapper::new(pkg_dep_graph),
+        }
+    }
+}
+
+impl PackageChangeMapper for DefaultPackageChangeMapperWithLockfile<'_> {
+    fn detect_package(&self, path: &AnchoredSystemPath) -> PackageMapping {
+        // If we have a lockfile change, we consider this as a root package change,
+        // since there's a chance that the root package uses a workspace package
+        // dependency (this is cursed behavior but sadly possible). There's a chance
+        // that we can make this more accurate by checking which package
+        // manager, since not all package managers may permit root pulling from
+        // workspace package dependencies
+        if PackageManager::supported_managers()
+            .iter()
+            .any(|pm| pm.lockfile_name() == path.as_str())
+        {
+            PackageMapping::Package((
+                WorkspacePackage {
+                    name: PackageName::Root,
+                    path: AnchoredSystemPathBuf::from_raw("").unwrap(),
+                },
+                PackageInclusionReason::ConservativeRootLockfileChanged,
+            ))
+        } else {
+            self.base.detect_package(path)
+        }
+    }
+}
+
 #[derive(Error, Debug)]
 pub enum Error {
     #[error(transparent)]
@@ -88,7 +138,7 @@ pub enum Error {
 /// changes all packages. Since we have a list of global deps,
 /// we can check against that and avoid invalidating in unnecessary cases.
 pub struct GlobalDepsPackageChangeMapper<'a> {
-    pkg_dep_graph: &'a PackageGraph,
+    base: DefaultPackageChangeMapperWithLockfile<'a>,
     global_deps_matcher: wax::Any<'a>,
 }
 
@@ -97,36 +147,19 @@ impl<'a> GlobalDepsPackageChangeMapper<'a> {
         pkg_dep_graph: &'a PackageGraph,
         global_deps: I,
     ) -> Result<Self, Error> {
+        let base = DefaultPackageChangeMapperWithLockfile::new(pkg_dep_graph);
         let global_deps_matcher = wax::any(global_deps)?;
 
         Ok(Self {
-            pkg_dep_graph,
+            base,
             global_deps_matcher,
         })
     }
 }
 
 impl PackageChangeMapper for GlobalDepsPackageChangeMapper<'_> {
     fn detect_package(&self, path: &AnchoredSystemPath) -> PackageMapping {
-        // If we have a lockfile change, we consider this as a root package change,
-        // since there's a chance that the root package uses a workspace package
-        // dependency (this is cursed behavior but sadly possible). There's a chance
-        // that we can make this more accurate by checking which package
-        // manager, since not all package managers may permit root pulling from
-        // workspace package dependencies
-        if PackageManager::supported_managers()
-            .iter()
-            .any(|pm| pm.lockfile_name() == path.as_str())
-        {
-            return PackageMapping::Package((
-                WorkspacePackage {
-                    name: PackageName::Root,
-                    path: AnchoredSystemPathBuf::from_raw("").unwrap(),
-                },
-                PackageInclusionReason::ConservativeRootLockfileChanged,
-            ));
-        }
-        match DefaultPackageChangeMapper::new(self.pkg_dep_graph).detect_package(path) {
+        match self.base.detect_package(path) {
             // Since `DefaultPackageChangeMapper` is overly conservative, we can check here if
             // the path is actually in globalDeps and if not, return it as
             // PackageDetection::Package(WorkspacePackage::root()).
@@ -160,7 +193,8 @@ mod tests {
     use super::{DefaultPackageChangeMapper, GlobalDepsPackageChangeMapper};
     use crate::{
         change_mapper::{
-            AllPackageChangeReason, ChangeMapper, PackageChanges, PackageInclusionReason,
+            AllPackageChangeReason, ChangeMapper, LockfileContents, PackageChanges,
+            PackageInclusionReason,
         },
         discovery::{self, PackageDiscovery},
         package_graph::{PackageGraphBuilder, WorkspacePackage},
@@ -208,7 +242,7 @@ mod tests {
             [AnchoredSystemPathBuf::from_raw("README.md")?]
                 .into_iter()
                 .collect(),
-            None,
+            LockfileContents::Unchanged,
         )?;
 
         // We should return All because we don't have global deps and
@@ -228,7 +262,7 @@ mod tests {
             [AnchoredSystemPathBuf::from_raw("README.md")?]
                 .into_iter()
                 .collect(),
-            None,
+            LockfileContents::Unchanged,
         )?;
 
         // We only get a root workspace change since we have global deps specified and

@@ -37,6 +37,7 @@ impl SCM {
         }
     }
 
+    /// get the actual changed files between two git refs
     pub fn changed_files(
         &self,
         turbo_root: &AbsoluteSystemPath,
-Original file line number
+Diff line change
@@ Expand Up / @@ -37,6 +37,7 @@ impl SCM { @@
             }
         }
+        /// get the actual changed files between two git refs
         pub fn changed_files(
             &self,
             turbo_root: &AbsoluteSystemPath,
@@ Expand Down @@