diff --git a/git_xet/src/app/install.rs b/git_xet/src/app/install.rs index 0514deea..cb4853f0 100644 --- a/git_xet/src/app/install.rs +++ b/git_xet/src/app/install.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use crate::app::Command::Transfer; use crate::constants::{GIT_LFS_CUSTOM_TRANSFER_AGENT_NAME, GIT_LFS_CUSTOM_TRANSFER_AGENT_PROGRAM}; use crate::errors::{GitXetError, Result}; -use crate::git_process_wrapping::run_git_captured; +use crate::utils::process_wrapping::run_git_captured; #[derive(Default)] pub(crate) enum ConfigLocation { @@ -102,9 +102,9 @@ pub mod tests { use serial_test::serial; use super::{global, local}; - use crate::git_process_wrapping::run_git_captured_with_input_and_output; use crate::git_repo::GitRepo; use crate::test_utils::TestRepo; + use crate::utils::process_wrapping::run_git_captured_with_input_and_output; pub fn get_lfs_env(env_list: &[u8], key: &str) -> Result> { let reader = BufReader::new(Cursor::new(env_list)); diff --git a/git_xet/src/app/uninstall.rs b/git_xet/src/app/uninstall.rs index 29f16bd4..0f3c3245 100644 --- a/git_xet/src/app/uninstall.rs +++ b/git_xet/src/app/uninstall.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use super::install::ConfigLocation; use crate::constants::GIT_LFS_CUSTOM_TRANSFER_AGENT_NAME; use crate::errors::Result; -use crate::git_process_wrapping::run_git_captured; +use crate::utils::process_wrapping::run_git_captured; // Remove git-xet registration from the system Git config. pub fn system() -> Result<()> { @@ -64,8 +64,8 @@ mod tests { use super::{all, local}; use crate::app::install; use crate::app::install::tests::get_lfs_env; - use crate::git_process_wrapping::run_git_captured_with_input_and_output; use crate::test_utils::TestRepo; + use crate::utils::process_wrapping::run_git_captured_with_input_and_output; #[test] #[serial(env_var_write_read)] diff --git a/git_xet/src/auth.rs b/git_xet/src/auth.rs index c17bac44..c8f88a46 100644 --- a/git_xet/src/auth.rs +++ b/git_xet/src/auth.rs @@ -186,9 +186,9 @@ mod test_cred_helpers { use super::get_credential; use crate::constants::HF_TOKEN_ENV; - use crate::git_process_wrapping::run_git_captured_with_input_and_output; use crate::git_repo::GitRepo; use crate::test_utils::TestRepo; + use crate::utils::process_wrapping::run_git_captured_with_input_and_output; #[test] #[serial(env_var_write_read)] diff --git a/git_xet/src/auth/git.rs b/git_xet/src/auth/git.rs index 9ac92123..72b886c2 100644 --- a/git_xet/src/auth/git.rs +++ b/git_xet/src/auth/git.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use hub_client::BearerCredentialHelper; use crate::errors::{GitXetError, Result}; -use crate::git_process_wrapping::run_git_captured_with_input_and_output; +use crate::utils::process_wrapping::run_git_captured_with_input_and_output; // This implements the mechanism to get credential stored in configured git credential helpers, including // git-credential-cache, git-credential-store, git-credential-libsecret (Linux), git-credential-osxkeychain (macOS), @@ -88,9 +88,9 @@ mod test_cred_helpers { use serial_test::serial; use super::GitCredentialHelper; - use crate::git_process_wrapping::run_git_captured_with_input_and_output; use crate::git_url::GitUrl; use crate::test_utils::TestRepo; + use crate::utils::process_wrapping::run_git_captured_with_input_and_output; #[test] #[serial(env_var_write_read)] diff --git a/git_xet/src/auth/ssh.rs b/git_xet/src/auth/ssh.rs index 7f4433c8..e672248f 100644 --- a/git_xet/src/auth/ssh.rs +++ b/git_xet/src/auth/ssh.rs @@ -36,7 +36,6 @@ pub struct SSHCredentialHelper { } impl SSHCredentialHelper { - #[allow(clippy::new_ret_no_self)] pub fn new(remote_url: &GitUrl, operation: Operation) -> Arc { Arc::new(Self { remote_url: remote_url.clone(), diff --git a/git_xet/src/errors.rs b/git_xet/src/errors.rs index 67d840a8..287e0798 100644 --- a/git_xet/src/errors.rs +++ b/git_xet/src/errors.rs @@ -9,8 +9,8 @@ use crate::lfs_agent_protocol::GitLFSProtocolError; #[derive(Error, Debug)] pub enum GitXetError { - #[error("Git command failed: {reason}, {source:?}")] - GitCommandFailed { + #[error("Command failed: {reason}, {source:?}")] + CommandFailed { reason: String, source: Option, }, @@ -49,8 +49,8 @@ pub enum GitXetError { pub type Result = std::result::Result; impl GitXetError { - pub(crate) fn git_cmd_failed(e: impl Display, source: Option) -> GitXetError { - GitXetError::GitCommandFailed { + pub(crate) fn cmd_failed(e: impl Display, source: Option) -> GitXetError { + GitXetError::CommandFailed { reason: e.to_string(), source, } diff --git a/git_xet/src/lib.rs b/git_xet/src/lib.rs index 4a9917cd..42c9c344 100644 --- a/git_xet/src/lib.rs +++ b/git_xet/src/lib.rs @@ -2,9 +2,9 @@ pub mod app; mod auth; mod constants; mod errors; -mod git_process_wrapping; mod git_repo; mod git_url; mod lfs_agent_protocol; mod test_utils; mod token_refresher; +mod utils; diff --git a/git_xet/src/test_utils/temp.rs b/git_xet/src/test_utils/temp.rs index d10b730b..0f2f05c2 100644 --- a/git_xet/src/test_utils/temp.rs +++ b/git_xet/src/test_utils/temp.rs @@ -6,7 +6,7 @@ use anyhow::Result; use tempfile::{TempDir, tempdir}; use utils::EnvVarGuard; -use crate::git_process_wrapping::run_git_captured; +use crate::utils::process_wrapping::run_git_captured; // A test utility to create a temporary HOME environment, this sets up a clean environment for // git operations which depend heavily on a global config file. diff --git a/git_xet/src/test_utils/test_repo.rs b/git_xet/src/test_utils/test_repo.rs index 0bfb96bd..857febc2 100644 --- a/git_xet/src/test_utils/test_repo.rs +++ b/git_xet/src/test_utils/test_repo.rs @@ -4,8 +4,8 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use tempfile::TempDir; -use crate::git_process_wrapping::run_git_captured; use crate::test_utils::TempHome; +use crate::utils::process_wrapping::run_git_captured; // A test utility to create a repo at a temporary directory with a clean global config. // It allows performing a varies of git operations in this repo. diff --git a/git_xet/src/utils/mod.rs b/git_xet/src/utils/mod.rs new file mode 100644 index 00000000..6acb3f2e --- /dev/null +++ b/git_xet/src/utils/mod.rs @@ -0,0 +1 @@ +pub mod process_wrapping; diff --git a/git_xet/src/git_process_wrapping.rs b/git_xet/src/utils/process_wrapping.rs similarity index 53% rename from git_xet/src/git_process_wrapping.rs rename to git_xet/src/utils/process_wrapping.rs index fe22f068..43eff67d 100644 --- a/git_xet/src/git_process_wrapping.rs +++ b/git_xet/src/utils/process_wrapping.rs @@ -5,14 +5,15 @@ use std::process::{Child, ChildStdin, Command, Stdio}; use crate::constants::GIT_EXECUTABLE; use crate::errors::{GitXetError, Result}; -// This mod implements utilities to invoke Git commands through child processes from the `git` program. +// This mod implements utilities to invoke commands through child processes from any program, and +// exposes special helpers to invoke git commands. /// Run a Git command as a child process by setting the current working directory to `working_dir`. /// This function doesn't allow the parent process to send data to the child. /// /// Return `Ok(())` if the Git command finishes correctly and the child's stdout and stderr are ignored; /// Return the underlying I/O error if the child process spawning or waiting fails; otherwise, the captured -/// stdout and stderr of the child are wrapped in an `Err(GitXetError::GitCommandFailed(_))` and returned. +/// stdout and stderr of the child are wrapped in an `Err(GitXetError::CommandFailed(_))` and returned. pub fn run_git_captured(working_dir: P, git_command: S1, args: I) -> Result<()> where P: AsRef, @@ -21,7 +22,27 @@ where S2: AsRef, { let mut command = Command::new(GIT_EXECUTABLE); - command.current_dir(working_dir).arg(git_command.as_ref()).args(args); + command.current_dir(working_dir).arg(git_command).args(args); + + CapturedCommand::new(command)?.wait() +} + +/// Run a command as a child process by setting the current working directory to `working_dir`. +/// This function doesn't allow the parent process to send data to the child. +/// +/// Return `Ok(())` if the command finishes correctly and the child's stdout and stderr are ignored; +/// Return the underlying I/O error if the child process spawning or waiting fails; otherwise, the captured +/// stdout and stderr of the child are wrapped in an `Err(GitXetError::CommandFailed(_))` and returned. +#[allow(dead_code)] +pub fn run_program_captured(program: S1, working_dir: P, args: I) -> Result<()> +where + S1: AsRef, + P: AsRef, + I: IntoIterator, + S2: AsRef, +{ + let mut command = Command::new(program); + command.current_dir(working_dir).args(args); CapturedCommand::new(command)?.wait() } @@ -60,6 +81,41 @@ where CapturedCommand::new_with_piped_stdin(command) } +/// Run a command as a child process by setting the current working directory to `working_dir`. +/// This function allows the parent process to send data to the child through the piped stdin. +/// +/// Return `Ok(CapturedCommand)` if the command process spawns correctly; otherwise, the underlying I/O +/// error is returned. +/// +/// # Examples +/// ```ignore +/// let mut cmd = run_program_captured_with_input_and_output("tee", path, &["dump.txt"])?; +/// +/// { +/// let mut writer = cmd.stdin()?; +/// write!(writer, "some_data")?; +/// } +/// +/// let (response, _err) = cmd.wait_with_output()?; +/// ``` +#[allow(dead_code)] +pub fn run_program_captured_with_input_and_output( + program: S1, + working_dir: P, + args: I, +) -> Result +where + S1: AsRef, + P: AsRef, + I: IntoIterator, + S2: AsRef, +{ + let mut command = Command::new(program); + command.current_dir(working_dir).args(args); + + CapturedCommand::new_with_piped_stdin(command) +} + // This struct wraps inside a spawned child process, whose stdout and stderr is piped // to the parent process instead of pointing at the terminal. pub struct CapturedCommand { @@ -74,8 +130,11 @@ impl CapturedCommand { // From past experience, if the "git" program is not found the underlying error // only says "Not Found" and is not very helpful to identify the cause. We thus // capture this error and make the message more explicit. - std::io::ErrorKind::NotFound => GitXetError::git_cmd_failed(r#"program "git" not found"#, Some(e)), - _ => GitXetError::git_cmd_failed("internal", Some(e)), + std::io::ErrorKind::NotFound => GitXetError::cmd_failed( + format!(r#"program "{}" not found"#, command.get_program().display()), + Some(e), + ), + _ => GitXetError::cmd_failed("internal", Some(e)), })?, }) } @@ -98,7 +157,7 @@ impl CapturedCommand { } /// Synchronously wait for the child to exit completely, returning `Ok(())` if the child exits with status code 0; - /// otherwise, return the captured output wrapped in an `Err(GitXetError::GitCommandFailed(_))`. + /// otherwise, return the captured output wrapped in an `Err(GitXetError::CommandFailed(_))`. pub fn wait(self) -> Result<()> { // ignores output let _ = self.wait_with_output()?; @@ -108,7 +167,7 @@ impl CapturedCommand { /// Synchronously wait for the child to exit and collect all remaining output on the stdout/stderr handles, /// returning a tuple of captured output if the child exits with status code 0; otherwise, return the captured - /// output wrapped in an `Err(GitXetError::GitCommandFailed(_))`. + /// output wrapped in an `Err(GitXetError::CommandFailed(_))`. pub fn wait_with_output(self) -> Result<(Vec, Vec)> { let ret = self.child_process.wait_with_output()?; @@ -117,7 +176,7 @@ impl CapturedCommand { _ => { let stdout = std::str::from_utf8(&ret.stdout).unwrap_or("").trim(); let stderr = std::str::from_utf8(&ret.stderr).unwrap_or("").trim(); - Err(GitXetError::git_cmd_failed( + Err(GitXetError::cmd_failed( format!("err_code = {:?}, stdout = \"{}\", stderr = \"{}\"", ret.status.code(), stdout, stderr), None, )) @@ -125,3 +184,54 @@ impl CapturedCommand { } } } + +#[cfg(test)] +mod tests { + use std::io::Write; + + use anyhow::Result; + + use super::*; + + #[test] + fn test_run_program_captured() -> Result<()> { + #[cfg(unix)] + run_program_captured("sh", std::env::current_dir()?, &["-c", "echo hello"])?; + #[cfg(windows)] + run_program_captured("cmd", std::env::current_dir()?, &["/C", "echo hello"])?; + + Ok(()) + } + + #[test] + fn test_program_captured_with_input_and_output() -> Result<()> { + let mut cmd = if cfg!(windows) { + run_program_captured_with_input_and_output("cmd", std::env::current_dir()?, &["/C", "more"])? + } else { + run_program_captured_with_input_and_output("sh", std::env::current_dir()?, &["-c", "cat"])? + }; + + { + let mut writer = cmd.stdin()?; + write!(writer, "hello")?; + } + + let (response, _err) = cmd.wait_with_output()?; + #[cfg(unix)] + assert_eq!(response, "hello".as_bytes()); + #[cfg(windows)] + assert_eq!(response, "hello\r\n".as_bytes()); + + Ok(()) + } + + #[test] + fn test_error_on_get_stdin_without_captured() -> Result<()> { + let command = Command::new(if cfg!(windows) { "cmd" } else { "sh" }); + + let mut command = CapturedCommand::new(command)?; + assert!(command.stdin().is_err()); + + Ok(()) + } +} diff --git a/hub_client/src/auth/basics.rs b/hub_client/src/auth/basics.rs index fa3776ad..4e33307c 100644 --- a/hub_client/src/auth/basics.rs +++ b/hub_client/src/auth/basics.rs @@ -9,7 +9,6 @@ use super::CredentialHelper; pub struct NoopCredentialHelper {} impl NoopCredentialHelper { - #[allow(clippy::new_ret_no_self)] pub fn new() -> Arc { Arc::new(Self {}) } @@ -33,7 +32,6 @@ pub struct BearerCredentialHelper { } impl BearerCredentialHelper { - #[allow(clippy::new_ret_no_self)] pub fn new(hf_token: String, whoami: &'static str) -> Arc { Arc::new(Self { hf_token,