diff --git a/crates/turborepo-lib/src/run/scope/change_detector.rs b/crates/turborepo-lib/src/run/scope/change_detector.rs index 670b17d491ae6..212f3ace5772b 100644 --- a/crates/turborepo-lib/src/run/scope/change_detector.rs +++ b/crates/turborepo-lib/src/run/scope/change_detector.rs @@ -1,6 +1,6 @@ use std::collections::{HashMap, HashSet}; -use tracing::debug; +use tracing::{debug, warn}; use turbopath::{AbsoluteSystemPath, AnchoredSystemPathBuf}; use turborepo_repository::{ change_mapper::{ @@ -9,7 +9,7 @@ use turborepo_repository::{ }, package_graph::{PackageGraph, PackageName}, }; -use turborepo_scm::{git::InvalidRange, SCM}; +use turborepo_scm::{git::InvalidRange, Error as ScmError, SCM}; use crate::run::scope::ResolutionError; @@ -82,6 +82,30 @@ impl<'a> ScopeChangeDetector<'a> { } } +impl<'a> ScopeChangeDetector<'a> { + fn all_packages_changed_due_to_error( + &self, + from_ref: Option<&str>, + to_ref: Option<&str>, + error_message: &str, + ) -> Result, ResolutionError> { + debug!("{}, defaulting to all packages changed", error_message); + Ok(self + .pkg_graph + .packages() + .map(|(name, _)| { + ( + name.to_owned(), + PackageInclusionReason::All(AllPackageChangeReason::GitRefNotFound { + from_ref: from_ref.map(String::from), + to_ref: to_ref.map(String::from), + }), + ) + }) + .collect()) + } +} + impl<'a> GitChangeDetector for ScopeChangeDetector<'a> { /// get the actual changed packages between two git refs fn changed_packages( @@ -99,24 +123,26 @@ impl<'a> GitChangeDetector for ScopeChangeDetector<'a> { include_uncommitted, allow_unknown_objects, merge_base, - )? { - Err(InvalidRange { from_ref, to_ref }) => { - debug!("invalid ref range, defaulting to all packages changed"); - return Ok(self - .pkg_graph - .packages() - .map(|(name, _)| { - ( - name.to_owned(), - PackageInclusionReason::All(AllPackageChangeReason::GitRefNotFound { - from_ref: from_ref.clone(), - to_ref: to_ref.clone(), - }), - ) - }) - .collect()); - } + ) { Ok(changed_files) => changed_files, + Err(ScmError::Path(err, _)) => { + warn!( + "Could not process some file paths: {}. Defaulting to all packages changed.", + err + ); + return self.all_packages_changed_due_to_error( + from_ref, + to_ref, + &format!("path error: {}", err), + ); + } + Err(err) => { + return self.all_packages_changed_due_to_error( + from_ref, + to_ref, + &format!("unexpected error: {}", err), + ); + } }; let lockfile_contents = self.get_lockfile_contents(from_ref, &changed_files); diff --git a/crates/turborepo-scm/src/git.rs b/crates/turborepo-scm/src/git.rs index bbd287bf79c0d..21bb6dfeb6b0b 100644 --- a/crates/turborepo-scm/src/git.rs +++ b/crates/turborepo-scm/src/git.rs @@ -22,6 +22,16 @@ pub struct InvalidRange { pub to_ref: Option, } +impl std::fmt::Display for InvalidRange { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "Invalid git range: from_ref={:?}, to_ref={:?}", + self.from_ref, self.to_ref + ) + } +} + impl SCM { pub fn get_current_branch(&self, path: &AbsoluteSystemPath) -> Result { match self { @@ -303,7 +313,7 @@ impl GitRepo { } let output = self.execute_git_command(&args, pathspec)?; - self.add_files_from_stdout(&mut files, turbo_root, output); + self.add_files_from_stdout(&mut files, turbo_root, output)?; // We only care about non-tracked files if we haven't specified both ends up the // comparison @@ -314,11 +324,11 @@ impl GitRepo { &["ls-files", "--others", "--modified", "--exclude-standard"], pathspec, )?; - self.add_files_from_stdout(&mut files, turbo_root, ls_files_output); + self.add_files_from_stdout(&mut files, turbo_root, ls_files_output)?; // Include any files that have been staged, but not committed let diff_output = self.execute_git_command(&["diff", "--name-only", "--cached"], pathspec)?; - self.add_files_from_stdout(&mut files, turbo_root, diff_output); + self.add_files_from_stdout(&mut files, turbo_root, diff_output)?; } Ok(files) @@ -350,15 +360,23 @@ impl GitRepo { files: &mut HashSet, turbo_root: &AbsoluteSystemPath, stdout: Vec, - ) { - let stdout = String::from_utf8(stdout).unwrap(); + ) -> Result<(), Error> { + let stdout = String::from_utf8(stdout)?; for line in stdout.lines() { let path = RelativeUnixPath::new(line).unwrap(); - let anchored_to_turbo_root_file_path = self - .reanchor_path_from_git_root_to_turbo_root(turbo_root, path) - .unwrap(); - files.insert(anchored_to_turbo_root_file_path); + match self.reanchor_path_from_git_root_to_turbo_root(turbo_root, path) { + Ok(anchored_to_turbo_root_file_path) => { + files.insert(anchored_to_turbo_root_file_path); + } + Err(err) => { + warn!( + "Skipping file that could not be anchored to turbo root: {} ({})", + line, err + ); + } + } } + Ok(()) } fn reanchor_path_from_git_root_to_turbo_root( @@ -1312,4 +1330,34 @@ mod tests { assert_eq!(None, actual); } + + #[test] + fn test_add_files_from_stdout_with_unancorable_path() -> Result<(), Error> { + let (repo_root, _repo) = setup_repository(None)?; + let git_root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); + + // Create a separate directory that is not a subdirectory of git_root + let turbo_root = tempfile::tempdir()?; + let turbo_root_path = AbsoluteSystemPathBuf::try_from(turbo_root.path()).unwrap(); + + // Create a GitRepo instance directly + let git_repo = GitRepo { + root: git_root.clone(), + bin: GitRepo::find_bin()?, + }; + + // Create a HashSet to collect files + let mut files = HashSet::new(); + + // Create stdout with a path that cannot be anchored + let problematic_path = "some/path/with/special/characters/test.spec.ts"; + let stdout = problematic_path.as_bytes().to_vec(); + + let result = git_repo.add_files_from_stdout(&mut files, &turbo_root_path, stdout); + + assert!(result.is_ok()); + assert!(files.is_empty()); + + Ok(()) + } }