Skip to content
Closed
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
61b7dd7
fix: handle PathError gracefully in add_files_from_stdout
devin-ai-integration[bot] Apr 30, 2025
91441b5
fix: format warn! macro according to Rust style guidelines
devin-ai-integration[bot] Apr 30, 2025
30bb433
test: add test coverage for handling unancorable paths
devin-ai-integration[bot] Apr 30, 2025
29eed78
style: fix formatting in test code
devin-ai-integration[bot] Apr 30, 2025
656c20f
Update crates/turborepo-scm/src/git.rs
anthonyshew Apr 30, 2025
517a5d3
fix: bubble up errors from add_files_from_stdout and handle them in c…
devin-ai-integration[bot] Apr 30, 2025
39bb312
style: fix formatting issues to pass Rust lints
devin-ai-integration[bot] Apr 30, 2025
2bbcf47
fix: use GitRefNotFound instead of non-existent Other variant
devin-ai-integration[bot] Apr 30, 2025
0b6951e
fix: handle path errors with warnings instead of bubbling up errors
devin-ai-integration[bot] Apr 30, 2025
aa8f5e9
fix: bubble up errors from add_files_from_stdout and handle them in c…
devin-ai-integration[bot] Apr 30, 2025
b91af4f
fix: return error directly instead of wrapping in Error::Path
devin-ai-integration[bot] Apr 30, 2025
7970096
style: fix formatting issues to pass Rust lints
devin-ai-integration[bot] Apr 30, 2025
9e52ecf
fix: log warnings for problematic paths instead of returning errors
devin-ai-integration[bot] Apr 30, 2025
91bf779
test: update test_add_files_from_stdout_with_unancorable_path to expe…
devin-ai-integration[bot] Apr 30, 2025
225d993
fix: format warning macro to pass Rust lints
devin-ai-integration[bot] Apr 30, 2025
2bc0866
fix: address PR feedback from chris-olszewski
devin-ai-integration[bot] May 1, 2025
7762841
style: format warning macro to match rustfmt style
devin-ai-integration[bot] May 1, 2025
c5bed4e
fix: fix type mismatches in change_detector.rs
devin-ai-integration[bot] May 1, 2025
6d59b8c
fix: remove ? operator to fix mismatched types in change_detector.rs
devin-ai-integration[bot] May 1, 2025
a085d64
test: update test to expect success instead of error
devin-ai-integration[bot] May 1, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions crates/turborepo-scm/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,17 @@ impl GitRepo {
let stdout = String::from_utf8(stdout).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still panic if there's a non-UTF8 path in the output.

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) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bubble up this error so it reaches the caller and they can decide what to do with it. In our case when this error is eventually returned in change_detector.rs we should catch it and behave as if every package changed since we cannot identify which exact packages changed. The warning you added should be moved there with an updated message.

warn!(
"Skipping file that could not be anchored to turbo root: {} ({})",
line, err
);
}
}
}
}

Expand Down Expand Up @@ -1312,4 +1319,33 @@ 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/\\321\\216.spec.ts";
let stdout = problematic_path.as_bytes().to_vec();

git_repo.add_files_from_stdout(&mut files, &turbo_root_path, stdout);

assert!(files.is_empty());

Ok(())
}
}
Loading