From 9c2850f6d921343f6ecc23f37980babcfe8b51bc Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Tue, 16 May 2023 14:00:47 -0700 Subject: [PATCH] Ensure exit status is checked for git operations --- crates/turborepo-scm/src/hash_object.rs | 47 ++++++++++------------- crates/turborepo-scm/src/lib.rs | 50 ++++++++++++++++++++++++- crates/turborepo-scm/src/ls_tree.rs | 35 +++++++---------- crates/turborepo-scm/src/status.rs | 36 ++++++++---------- 4 files changed, 97 insertions(+), 71 deletions(-) diff --git a/crates/turborepo-scm/src/hash_object.rs b/crates/turborepo-scm/src/hash_object.rs index f0797a731c38d..f91fb1b205a3d 100644 --- a/crates/turborepo-scm/src/hash_object.rs +++ b/crates/turborepo-scm/src/hash_object.rs @@ -8,7 +8,7 @@ use std::{ use nom::{Finish, IResult}; use turbopath::{AbsoluteSystemPathBuf, RelativeUnixPathBuf}; -use crate::{package_deps::GitHashes, Error}; +use crate::{package_deps::GitHashes, read_git_error, wait_for_success, Error}; pub(crate) fn hash_objects( pkg_path: &AbsoluteSystemPathBuf, @@ -26,33 +26,26 @@ pub(crate) fn hash_objects( .stderr(Stdio::piped()) .stdin(Stdio::piped()) .spawn()?; - { - let stdout = git - .stdout - .as_mut() - .ok_or_else(|| Error::git_error("failed to get stdout for git hash-object"))?; - // We take, rather than borrow, stdin so that we can drop it and force the - // underlying file descriptor to close, signalling the end of input. - let stdin: std::process::ChildStdin = git - .stdin - .take() - .ok_or_else(|| Error::git_error("failed to get stdin for git hash-object"))?; - let mut stderr = git - .stderr - .take() - .ok_or_else(|| Error::git_error("failed to get stderr for git hash-object"))?; - let result = read_object_hashes(stdout, stdin, &to_hash, pkg_prefix, hashes); - if let Err(err) = result { - let mut buf = String::new(); - let bytes_read = stderr.read_to_string(&mut buf)?; - if bytes_read > 0 { - // something failed with git, report that error - return Err(Error::git_error(buf)); - } - return Err(err); - } + + let stdout = git + .stdout + .as_mut() + .ok_or_else(|| Error::git_error("failed to get stdout for git hash-object"))?; + // We take, rather than borrow, stdin so that we can drop it and force the + // underlying file descriptor to close, signalling the end of input. + let stdin: std::process::ChildStdin = git + .stdin + .take() + .ok_or_else(|| Error::git_error("failed to get stdin for git hash-object"))?; + let mut stderr = git + .stderr + .take() + .ok_or_else(|| Error::git_error("failed to get stderr for git hash-object"))?; + let result = read_object_hashes(stdout, stdin, &to_hash, pkg_prefix, hashes); + if let Err(err) = result { + return Err(read_git_error(&mut stderr).unwrap_or(err)); } - git.wait()?; + wait_for_success(git, &mut stderr, "git hash-object", pkg_path)?; Ok(()) } diff --git a/crates/turborepo-scm/src/lib.rs b/crates/turborepo-scm/src/lib.rs index dbe37200b9d4b..d06fc08b4b01e 100644 --- a/crates/turborepo-scm/src/lib.rs +++ b/crates/turborepo-scm/src/lib.rs @@ -2,10 +2,14 @@ #![feature(provide_any)] #![feature(assert_matches)] -use std::backtrace::{self, Backtrace}; +use std::{ + backtrace::{self, Backtrace}, + io::Read, + process::Child, +}; use thiserror::Error; -use turbopath::PathError; +use turbopath::{AbsoluteSystemPath, PathError}; pub mod git; mod hash_object; @@ -36,4 +40,46 @@ impl Error { pub(crate) fn git_error(s: impl Into) -> Self { Error::Git(s.into(), Backtrace::capture()) } + + fn from_git_exit_code(cmd: &str, path: &AbsoluteSystemPath, exit_code: Option) -> Error { + let s = format!( + "'{}' in {} exited with status code {}", + cmd, + path.to_string(), + exit_code + .map(|code| code.to_string()) + .unwrap_or("unknown".to_string()) + ); + Error::Git(s, Backtrace::capture()) + } +} + +pub(crate) fn read_git_error(stderr: &mut R) -> Option { + let mut buf = String::new(); + if let Ok(bytes_read) = stderr.read_to_string(&mut buf) { + if bytes_read > 0 { + // something failed with git, report that error + Some(Error::git_error(buf)) + } else { + None + } + } else { + None + } +} + +pub(crate) fn wait_for_success( + mut child: Child, + stderr: &mut R, + command: &str, + root_path: impl AsRef, +) -> Result<(), Error> { + let exit_status = child.wait()?; + if !exit_status.success() { + Err(read_git_error(stderr).unwrap_or_else(|| { + Error::from_git_exit_code(command, root_path.as_ref(), exit_status.code()) + })) + } else { + Ok(()) + } } diff --git a/crates/turborepo-scm/src/ls_tree.rs b/crates/turborepo-scm/src/ls_tree.rs index 51fb9626e3e3a..670b64b8f8a3e 100644 --- a/crates/turborepo-scm/src/ls_tree.rs +++ b/crates/turborepo-scm/src/ls_tree.rs @@ -6,7 +6,7 @@ use std::{ use nom::Finish; use turbopath::{AbsoluteSystemPathBuf, RelativeUnixPathBuf}; -use crate::{package_deps::GitHashes, Error}; +use crate::{package_deps::GitHashes, read_git_error, wait_for_success, Error}; pub fn git_ls_tree(root_path: &AbsoluteSystemPathBuf) -> Result { let mut hashes = GitHashes::new(); @@ -16,27 +16,20 @@ pub fn git_ls_tree(root_path: &AbsoluteSystemPathBuf) -> Result 0 { - // something failed with git, report that error - return Err(Error::git_error(buf)); - } - } - result?; + + let stdout = git + .stdout + .as_mut() + .ok_or_else(|| Error::git_error("failed to get stdout for git ls-tree"))?; + let mut stderr = git + .stderr + .take() + .ok_or_else(|| Error::git_error("failed to get stderr for git ls-tree"))?; + let result = read_ls_tree(stdout, &mut hashes); + if let Err(err) = result { + return Err(read_git_error(&mut stderr).unwrap_or(err)); } - git.wait()?; + wait_for_success(git, &mut stderr, "git ls-tree", root_path)?; Ok(hashes) } diff --git a/crates/turborepo-scm/src/status.rs b/crates/turborepo-scm/src/status.rs index f7110c90e0cea..b7c88042afc35 100644 --- a/crates/turborepo-scm/src/status.rs +++ b/crates/turborepo-scm/src/status.rs @@ -6,7 +6,7 @@ use std::{ use nom::Finish; use turbopath::{AbsoluteSystemPathBuf, RelativeUnixPathBuf}; -use crate::{package_deps::GitHashes, Error}; +use crate::{package_deps::GitHashes, read_git_error, wait_for_success, Error}; pub(crate) fn append_git_status( root_path: &AbsoluteSystemPathBuf, @@ -26,27 +26,21 @@ pub(crate) fn append_git_status( .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn()?; - let to_hash = { - let stdout = git - .stdout - .as_mut() - .ok_or_else(|| Error::git_error("failed to get stdout for git status"))?; - let mut stderr = git - .stderr - .take() - .ok_or_else(|| Error::git_error("failed to get stderr for git status"))?; - let result = read_status(stdout, pkg_prefix, hashes); - if result.is_err() { - let mut buf = String::new(); - let bytes_read = stderr.read_to_string(&mut buf)?; - if bytes_read > 0 { - // something failed with git, report that error - return Err(Error::git_error(buf)); - } - } - result? + + let stdout = git + .stdout + .as_mut() + .ok_or_else(|| Error::git_error("failed to get stdout for git status"))?; + let mut stderr = git + .stderr + .take() + .ok_or_else(|| Error::git_error("failed to get stderr for git status"))?; + let result = read_status(stdout, pkg_prefix, hashes); + let to_hash = match result { + Err(err) => return Err(read_git_error(&mut stderr).unwrap_or(err)), + Ok(to_hash) => to_hash, }; - git.wait()?; + wait_for_success(git, &mut stderr, "git status", &root_path)?; Ok(to_hash) }