Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions git_xet/src/app/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Option<String>> {
let reader = BufReader::new(Cursor::new(env_list));
Expand Down
4 changes: 2 additions & 2 deletions git_xet/src/app/uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion git_xet/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions git_xet/src/auth/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)]
Expand Down
1 change: 0 additions & 1 deletion git_xet/src/auth/ssh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ pub struct SSHCredentialHelper {
}

impl SSHCredentialHelper {
#[allow(clippy::new_ret_no_self)]
pub fn new(remote_url: &GitUrl, operation: Operation) -> Arc<Self> {
Arc::new(Self {
remote_url: remote_url.clone(),
Expand Down
8 changes: 4 additions & 4 deletions git_xet/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::io::Error>,
},
Expand Down Expand Up @@ -49,8 +49,8 @@ pub enum GitXetError {
pub type Result<T> = std::result::Result<T, GitXetError>;

impl GitXetError {
pub(crate) fn git_cmd_failed(e: impl Display, source: Option<std::io::Error>) -> GitXetError {
GitXetError::GitCommandFailed {
pub(crate) fn cmd_failed(e: impl Display, source: Option<std::io::Error>) -> GitXetError {
GitXetError::CommandFailed {
reason: e.to_string(),
source,
}
Expand Down
2 changes: 1 addition & 1 deletion git_xet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion git_xet/src/test_utils/temp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion git_xet/src/test_utils/test_repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions git_xet/src/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod process_wrapping;
Original file line number Diff line number Diff line change
Expand Up @@ -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<P, S1, I, S2>(working_dir: P, git_command: S1, args: I) -> Result<()>
where
P: AsRef<Path>,
Expand All @@ -21,7 +22,27 @@ where
S2: AsRef<OsStr>,
{
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<S1, P, I, S2>(program: S1, working_dir: P, args: I) -> Result<()>
where
S1: AsRef<OsStr>,
P: AsRef<Path>,
I: IntoIterator<Item = S2>,
S2: AsRef<OsStr>,
{
let mut command = Command::new(program);
command.current_dir(working_dir).args(args);

CapturedCommand::new(command)?.wait()
}
Expand Down Expand Up @@ -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<S1, P, I, S2>(
program: S1,
working_dir: P,
args: I,
) -> Result<CapturedCommand>
where
S1: AsRef<OsStr>,
P: AsRef<Path>,
I: IntoIterator<Item = S2>,
S2: AsRef<OsStr>,
{
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 {
Expand All @@ -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)),
})?,
})
}
Expand All @@ -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()?;
Expand All @@ -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<u8>, Vec<u8>)> {
let ret = self.child_process.wait_with_output()?;

Expand All @@ -117,11 +176,62 @@ impl CapturedCommand {
_ => {
let stdout = std::str::from_utf8(&ret.stdout).unwrap_or("<Binary Data>").trim();
let stderr = std::str::from_utf8(&ret.stderr).unwrap_or("<Binary Data>").trim();
Err(GitXetError::git_cmd_failed(
Err(GitXetError::cmd_failed(
format!("err_code = {:?}, stdout = \"{}\", stderr = \"{}\"", ret.status.code(), stdout, stderr),
None,
))
},
}
}
}

#[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(())
}
}
2 changes: 0 additions & 2 deletions hub_client/src/auth/basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use super::CredentialHelper;
pub struct NoopCredentialHelper {}

impl NoopCredentialHelper {
#[allow(clippy::new_ret_no_self)]
pub fn new() -> Arc<Self> {
Arc::new(Self {})
}
Expand All @@ -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<Self> {
Arc::new(Self {
hf_token,
Expand Down