From 16813bd29634ea928d5e475c6c0fa9b4e13bba7a Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 11:25:38 -0700 Subject: [PATCH] Split marketplace CLI operations --- codex-rs/cli/src/marketplace_cmd.rs | 153 +---------------------- codex-rs/cli/src/marketplace_cmd/ops.rs | 156 ++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 148 deletions(-) create mode 100644 codex-rs/cli/src/marketplace_cmd/ops.rs diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index ed8a0f6b77..d163abfe29 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -13,11 +13,11 @@ use codex_utils_cli::CliConfigOverrides; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::process::Command; use std::time::SystemTime; use std::time::UNIX_EPOCH; mod metadata; +mod ops; #[derive(Debug, Parser)] pub struct MarketplaceCli { @@ -126,7 +126,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { return Ok(()); } - let staging_root = marketplace_staging_root(&install_root); + let staging_root = ops::marketplace_staging_root(&install_root); fs::create_dir_all(&staging_root).with_context(|| { format!( "failed to create marketplace staging directory {}", @@ -146,7 +146,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { match &source { MarketplaceSource::LocalDirectory { path } => { - copy_dir_recursive(path, &staged_root).with_context(|| { + ops::copy_dir_recursive(path, &staged_root).with_context(|| { format!( "failed to copy marketplace source {} into {}", path.display(), @@ -155,7 +155,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { })?; } MarketplaceSource::Git { url, ref_name } => { - clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; + ops::clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; } } @@ -175,7 +175,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { source.display() ); } - replace_marketplace_root(&staged_root, &destination) + ops::replace_marketplace_root(&staged_root, &destination) .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; record_added_marketplace(&codex_home, &marketplace_name, &install_metadata)?; @@ -361,121 +361,6 @@ fn is_github_shorthand_segment(segment: &str) -> bool { .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.')) } -pub(super) fn clone_git_source( - url: &str, - ref_name: Option<&str>, - sparse_paths: &[String], - destination: &Path, -) -> Result<()> { - let destination = destination.to_string_lossy().to_string(); - if sparse_paths.is_empty() { - run_git(&["clone", url, destination.as_str()], None)?; - if let Some(ref_name) = ref_name { - run_git(&["checkout", ref_name], Some(Path::new(&destination)))?; - } - return Ok(()); - } - - run_git( - &[ - "clone", - "--filter=blob:none", - "--no-checkout", - url, - destination.as_str(), - ], - None, - )?; - let mut sparse_args = vec!["sparse-checkout", "set"]; - sparse_args.extend(sparse_paths.iter().map(String::as_str)); - let destination = Path::new(&destination); - run_git(&sparse_args, Some(destination))?; - run_git(&["checkout", ref_name.unwrap_or("HEAD")], Some(destination))?; - Ok(()) -} - -pub(super) fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<()> { - let mut command = Command::new("git"); - command.args(args); - command.env("GIT_TERMINAL_PROMPT", "0"); - if let Some(cwd) = cwd { - command.current_dir(cwd); - } - - let output = command - .output() - .with_context(|| format!("failed to run git {}", args.join(" ")))?; - if output.status.success() { - return Ok(()); - } - - let stderr = String::from_utf8_lossy(&output.stderr); - let stdout = String::from_utf8_lossy(&output.stdout); - bail!( - "git {} failed with status {}\nstdout:\n{}\nstderr:\n{}", - args.join(" "), - output.status, - stdout.trim(), - stderr.trim() - ); -} - -pub(super) fn copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { - fs::create_dir_all(target)?; - for entry in fs::read_dir(source)? { - let entry = entry?; - let source_path = entry.path(); - let target_path = target.join(entry.file_name()); - let file_type = entry.file_type()?; - - if file_type.is_dir() { - if entry.file_name().to_str() == Some(".git") { - continue; - } - copy_dir_recursive(&source_path, &target_path)?; - } else if file_type.is_file() { - fs::copy(&source_path, &target_path)?; - } else if file_type.is_symlink() { - copy_symlink_target(&source_path, &target_path)?; - } - } - Ok(()) -} - -#[cfg(unix)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - std::os::unix::fs::symlink(fs::read_link(source)?, target)?; - Ok(()) -} - -#[cfg(windows)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - let metadata = fs::metadata(source)?; - if metadata.is_dir() { - copy_dir_recursive(source, target) - } else { - fs::copy(source, target).map(|_| ()).map_err(Into::into) - } -} - -pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { - if let Some(parent) = destination.parent() { - fs::create_dir_all(parent)?; - } - if destination.exists() { - bail!( - "marketplace destination already exists: {}", - destination.display() - ); - } - - fs::rename(staged_root, destination).map_err(Into::into) -} - -pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { - install_root.join(".staging") -} - fn safe_marketplace_dir_name(marketplace_name: &str) -> Result { let safe = marketplace_name .chars() @@ -576,7 +461,6 @@ impl MarketplaceSource { mod tests { use super::*; use pretty_assertions::assert_eq; - use tempfile::TempDir; #[test] fn github_shorthand_parses_ref_suffix() { @@ -673,31 +557,4 @@ mod tests { vec!["plugins/foo", "skills/bar"] ); } - - #[test] - fn replace_marketplace_root_rejects_existing_destination() { - let temp_dir = TempDir::new().unwrap(); - let staged_root = temp_dir.path().join("staged"); - let destination = temp_dir.path().join("destination"); - fs::create_dir_all(&staged_root).unwrap(); - fs::write(staged_root.join("marker.txt"), "staged").unwrap(); - fs::create_dir_all(&destination).unwrap(); - fs::write(destination.join("marker.txt"), "installed").unwrap(); - - let err = replace_marketplace_root(&staged_root, &destination).unwrap_err(); - - assert!( - err.to_string() - .contains("marketplace destination already exists"), - "unexpected error: {err}" - ); - assert_eq!( - fs::read_to_string(staged_root.join("marker.txt")).unwrap(), - "staged" - ); - assert_eq!( - fs::read_to_string(destination.join("marker.txt")).unwrap(), - "installed" - ); - } } diff --git a/codex-rs/cli/src/marketplace_cmd/ops.rs b/codex-rs/cli/src/marketplace_cmd/ops.rs new file mode 100644 index 0000000000..1027e96fd3 --- /dev/null +++ b/codex-rs/cli/src/marketplace_cmd/ops.rs @@ -0,0 +1,156 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +pub(super) fn clone_git_source( + url: &str, + ref_name: Option<&str>, + sparse_paths: &[String], + destination: &Path, +) -> Result<()> { + let destination = destination.to_string_lossy().to_string(); + if sparse_paths.is_empty() { + run_git(&["clone", url, destination.as_str()], None)?; + if let Some(ref_name) = ref_name { + run_git(&["checkout", ref_name], Some(Path::new(&destination)))?; + } + return Ok(()); + } + + run_git( + &[ + "clone", + "--filter=blob:none", + "--no-checkout", + url, + destination.as_str(), + ], + None, + )?; + let mut sparse_args = vec!["sparse-checkout", "set"]; + sparse_args.extend(sparse_paths.iter().map(String::as_str)); + let destination = Path::new(&destination); + run_git(&sparse_args, Some(destination))?; + run_git(&["checkout", ref_name.unwrap_or("HEAD")], Some(destination))?; + Ok(()) +} + +fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<()> { + let mut command = Command::new("git"); + command.args(args); + command.env("GIT_TERMINAL_PROMPT", "0"); + if let Some(cwd) = cwd { + command.current_dir(cwd); + } + + let output = command + .output() + .with_context(|| format!("failed to run git {}", args.join(" ")))?; + if output.status.success() { + return Ok(()); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + bail!( + "git {} failed with status {}\nstdout:\n{}\nstderr:\n{}", + args.join(" "), + output.status, + stdout.trim(), + stderr.trim() + ); +} + +pub(super) fn copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { + fs::create_dir_all(target)?; + for entry in fs::read_dir(source)? { + let entry = entry?; + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + let file_type = entry.file_type()?; + + if file_type.is_dir() { + if entry.file_name().to_str() == Some(".git") { + continue; + } + copy_dir_recursive(&source_path, &target_path)?; + } else if file_type.is_file() { + fs::copy(&source_path, &target_path)?; + } else if file_type.is_symlink() { + copy_symlink_target(&source_path, &target_path)?; + } + } + Ok(()) +} + +#[cfg(unix)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + std::os::unix::fs::symlink(fs::read_link(source)?, target)?; + Ok(()) +} + +#[cfg(windows)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + let metadata = fs::metadata(source)?; + if metadata.is_dir() { + copy_dir_recursive(source, target) + } else { + fs::copy(source, target).map(|_| ()).map_err(Into::into) + } +} + +pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { + if let Some(parent) = destination.parent() { + fs::create_dir_all(parent)?; + } + if destination.exists() { + bail!( + "marketplace destination already exists: {}", + destination.display() + ); + } + + fs::rename(staged_root, destination).map_err(Into::into) +} + +pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { + install_root.join(".staging") +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + #[test] + fn replace_marketplace_root_rejects_existing_destination() { + let temp_dir = TempDir::new().unwrap(); + let staged_root = temp_dir.path().join("staged"); + let destination = temp_dir.path().join("destination"); + fs::create_dir_all(&staged_root).unwrap(); + fs::write(staged_root.join("marker.txt"), "staged").unwrap(); + fs::create_dir_all(&destination).unwrap(); + fs::write(destination.join("marker.txt"), "installed").unwrap(); + + let err = replace_marketplace_root(&staged_root, &destination).unwrap_err(); + + assert!( + err.to_string() + .contains("marketplace destination already exists"), + "unexpected error: {err}" + ); + assert_eq!( + fs::read_to_string(staged_root.join("marker.txt")).unwrap(), + "staged" + ); + assert_eq!( + fs::read_to_string(destination.join("marker.txt")).unwrap(), + "installed" + ); + } +}