From c61ccd65c5a41727eb6ab49cb8d0c36e81bff047 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Fri, 10 Jan 2025 17:44:22 -0800 Subject: [PATCH 01/29] feat: support items.revision --- src/config.rs | 6 +++++ src/operations/install.rs | 5 +++- src/operations/update.rs | 11 ++++---- src/utils.rs | 55 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/src/config.rs b/src/config.rs index 1adeec8..a508986 100644 --- a/src/config.rs +++ b/src/config.rs @@ -29,11 +29,13 @@ pub struct ConfigItem { pub supported_systems: Option>, #[serde(rename = "theme-file-extension")] pub theme_file_extension: Option, + pub revision: Option, } impl fmt::Display for ConfigItem { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let hook = self.hook.clone().unwrap_or_default(); + let revision = self.revision.clone().unwrap_or_default(); let default_supported_systems = vec![SchemeSystem::default()]; let system_text = self .supported_systems @@ -52,6 +54,9 @@ impl fmt::Display for ConfigItem { if !hook.is_empty() { writeln!(f, "hook = \"{}\"", hook)?; } + if !revision.is_empty() { + writeln!(f, "revision = \"{}\"", revision)?; + } writeln!(f, "supported-systems = [{}]", system_text)?; write!(f, "themes-dir = \"{}\"", self.themes_dir) } @@ -108,6 +113,7 @@ impl Config { hook: Some(BASE16_SHELL_HOOK.to_string()), supported_systems: Some(vec![SchemeSystem::Base16]), // DEFAULT_SCHEME_SYSTEM theme_file_extension: None, + revision: None, }; // Add default `item` if no items exist diff --git a/src/operations/install.rs b/src/operations/install.rs index b5c8c8e..5977a9b 100644 --- a/src/operations/install.rs +++ b/src/operations/install.rs @@ -11,10 +11,11 @@ fn install_git_url( data_item_path: &Path, item_name: &str, item_git_url: &str, + revision: Option<&str>, is_quiet: bool, ) -> Result<()> { if !data_item_path.is_dir() { - git_clone(item_git_url, data_item_path)?; + git_clone(item_git_url, data_item_path, revision)?; if !is_quiet { println!("{} installed", item_name); @@ -86,6 +87,7 @@ pub fn install(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<( &data_item_path, item.name.as_str(), item.path.as_str(), + item.revision.as_deref(), is_quiet, )?, Err(_) => install_dir(&data_item_path, item.name.as_str(), &item_path, is_quiet)?, @@ -98,6 +100,7 @@ pub fn install(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<( &schemes_repo_path, SCHEMES_REPO_NAME, SCHEMES_REPO_URL, + None, is_quiet, )?; diff --git a/src/operations/update.rs b/src/operations/update.rs index 0ea4852..f6356d3 100644 --- a/src/operations/update.rs +++ b/src/operations/update.rs @@ -1,16 +1,16 @@ use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_URL}; -use crate::utils::{git_diff, git_pull}; +use crate::utils::{git_diff, git_update}; use crate::{config::Config, constants::REPO_NAME}; use anyhow::{Context, Result}; use std::path::Path; -fn update_item(item_name: &str, item_url: &str, item_path: &Path, is_quiet: bool) -> Result<()> { +fn update_item(item_name: &str, item_url: &str, item_path: &Path, revision: Option<&str>, is_quiet: bool) -> Result<()> { if item_path.is_dir() { let is_diff = git_diff(item_path)?; if !is_diff { - git_pull(item_path) - .with_context(|| format!("Error pulling {} from {}", item_name, item_url))?; + git_update(item_path, item_url, revision) + .with_context(|| format!("Error updating {} to {}@{}", item_name, item_url, revision.unwrap_or("main")))?; if !is_quiet { println!("{} up to date", item_name); @@ -36,7 +36,7 @@ pub fn update(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<() for item in items { let item_path = hooks_path.join(&item.name); - update_item(item.name.as_str(), item.path.as_str(), &item_path, is_quiet)?; + update_item(item.name.as_str(), item.path.as_str(), &item_path, item.revision.as_deref(), is_quiet)?; } let schemes_repo_path = hooks_path.join(SCHEMES_REPO_NAME); @@ -45,6 +45,7 @@ pub fn update(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<() SCHEMES_REPO_NAME, SCHEMES_REPO_URL, &schemes_repo_path, + None, is_quiet, )?; diff --git a/src/utils.rs b/src/utils.rs index 75986dd..2ba40c1 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -41,7 +41,7 @@ pub fn get_shell_command_from_string(config_path: &Path, command: &str) -> Resul shell_words::split(&full_command).map_err(anyhow::Error::new) } -pub fn git_clone(repo_url: &str, target_dir: &Path) -> Result<()> { +pub fn git_clone(repo_url: &str, target_dir: &Path, revision: Option<&str>) -> Result<()> { if target_dir.exists() { return Err(anyhow!( "Error cloning {}. Target directory '{}' already exists", @@ -59,6 +59,17 @@ pub fn git_clone(repo_url: &str, target_dir: &Path) -> Result<()> { .status() .with_context(|| format!("Failed to clone repository from {}", repo_url))?; + let revision_str = revision.unwrap_or("main") + let command = format!("git reset --hard \"{}\"", revision_str); + let command_vec = shell_words::split(command.as_str()).map_err(anyhow::Error::new)?; + + Command::new(&command_vec[0]) + .args(&command_vec[1..]) + .current_dir(repo_url) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to checkout revision {}", revision_str))?; + Ok(()) } @@ -87,6 +98,48 @@ pub fn git_pull(repo_path: &Path) -> Result<()> { } } +pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> Result<()> { + + if !repo_path.is_dir() { + return Err(anyhow!( + "Error with updating. {} is not a directory", + repo_path.display() + )); + } + + let command = format!("git remote set-url origin \"{}\"", repo_url); + let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; + + let remote = Command::new(&command_vec[0]) + .args(&command_vec[1..]) + .current_dir(repo_path) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; + + if !remote.success() { + return Err(anyhow!("Error with setting remote URL to \"{}\"", repo_url)); + } + + let revision_str = revision.unwrap_or("main"); + let command = format!("git reset --hard \"{}\"", revision_str); + let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; + + let revision = Command::new(&command_vec[0]) + .args(&command_vec[1..]) + .current_dir(repo_path) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; + + if !revision.success() { + return Err(anyhow!("Error with checking out revision \"{}\"", revision_str)); + } + + Ok(()) + +} + pub fn git_diff(target_dir: &Path) -> Result { let command = "git status --porcelain"; let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; From f0807230069264c779c6f847584c9d42fe729e9b Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Fri, 10 Jan 2025 18:21:18 -0800 Subject: [PATCH 02/29] feat: understand all sorts of revisions --- src/utils.rs | 72 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 20 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 2ba40c1..2cee53c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -59,18 +59,9 @@ pub fn git_clone(repo_url: &str, target_dir: &Path, revision: Option<&str>) -> R .status() .with_context(|| format!("Failed to clone repository from {}", repo_url))?; - let revision_str = revision.unwrap_or("main") - let command = format!("git reset --hard \"{}\"", revision_str); - let command_vec = shell_words::split(command.as_str()).map_err(anyhow::Error::new)?; - Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_url) - .stdout(Stdio::null()) - .status() - .with_context(|| format!("Failed to checkout revision {}", revision_str))?; - - Ok(()) + let revision_str = revision.unwrap_or("main"); + return git_to_revision(target_dir, revision_str) } pub fn git_pull(repo_path: &Path) -> Result<()> { @@ -99,7 +90,6 @@ pub fn git_pull(repo_path: &Path) -> Result<()> { } pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> Result<()> { - if !repo_path.is_dir() { return Err(anyhow!( "Error with updating. {} is not a directory", @@ -108,7 +98,7 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R } let command = format!("git remote set-url origin \"{}\"", repo_url); - let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; let remote = Command::new(&command_vec[0]) .args(&command_vec[1..]) @@ -121,23 +111,65 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R return Err(anyhow!("Error with setting remote URL to \"{}\"", repo_url)); } + let revision_str = revision.unwrap_or("main"); - let command = format!("git reset --hard \"{}\"", revision_str); - let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; + return git_to_revision(repo_path, revision_str) + +} + +fn git_to_revision(repo_path: &Path, revision: &str) -> Result<()> { + println!("{}", repo_path.display()); - let revision = Command::new(&command_vec[0]) + let command = format!("git fetch origin \"{}\"", revision); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + + let fetch = Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) .stdout(Stdio::null()) .status() - .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; + .with_context(|| format!("fetch: Failed to execute process in {}", repo_path.display()))?; - if !revision.success() { - return Err(anyhow!("Error with checking out revision \"{}\"", revision_str)); + if !fetch.success() { + return Err(anyhow!("Error with fetching \"{}\"", revision)); } - Ok(()) + // Normalize the revision into the SHA. This way we can support all sorts of revisions, from + // branches, tags, SHAs, etc. + let command = format!("git rev-parse \"origin/{}\"", revision); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + + let parse_out = Command::new(&command_vec[0]) + .args(&command_vec[1..]) + .current_dir(repo_path) + .output() + .with_context(|| format!("rev-parse: Failed to execute process in {}", repo_path.display()))?; + + if !parse_out.status.success() { + return Err(anyhow!("Error resolving revision \"{}\"", revision)); + } + + let stdout = String::from_utf8_lossy(&parse_out.stdout); + + let commit_sha = match stdout.lines().next() { + Some(sha) => sha, + None => return Err(anyhow!("Error resolving revision \"{}\"", revision)) + }; + let command = format!("git checkout \"{}\"", commit_sha); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + + let checkout = Command::new(&command_vec[0]) + .args(&command_vec[1..]) + .current_dir(repo_path) + .status() + .with_context(|| format!("checkout: Failed to execute process in {}", repo_path.display()))?; + + if !checkout.success() { + return Err(anyhow!("Error checking out revision \"{}\"", commit_sha)); + } + + Ok(()) } pub fn git_diff(target_dir: &Path) -> Result { From b0c6c5ad0fdac65048d64bad03fbf03293e22fbb Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Fri, 10 Jan 2025 18:51:20 -0800 Subject: [PATCH 03/29] fix: use spec-0.11 as schemes revision value --- src/constants.rs | 1 + src/operations/install.rs | 4 ++-- src/operations/update.rs | 4 ++-- src/utils.rs | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/constants.rs b/src/constants.rs index 77f962a..ccc79d6 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -7,3 +7,4 @@ pub const CUSTOM_SCHEMES_DIR_NAME: &str = "custom-schemes"; pub const CURRENT_SCHEME_FILE_NAME: &str = "current_scheme"; pub const DEFAULT_SCHEME_SYSTEM: &str = "base16"; pub const SCHEME_EXTENSION: &str = "yaml"; +pub const SCHEMES_REPO_REVISION: &str = "spec-0.11"; diff --git a/src/operations/install.rs b/src/operations/install.rs index 5977a9b..a5cb872 100644 --- a/src/operations/install.rs +++ b/src/operations/install.rs @@ -1,5 +1,5 @@ use crate::config::Config; -use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_URL}; +use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_REVISION, SCHEMES_REPO_URL}; use crate::utils::git_clone; use anyhow::{anyhow, Result}; use std::fs::{remove_file as remove_symlink, symlink_metadata}; @@ -100,7 +100,7 @@ pub fn install(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<( &schemes_repo_path, SCHEMES_REPO_NAME, SCHEMES_REPO_URL, - None, + Some(SCHEMES_REPO_REVISION), is_quiet, )?; diff --git a/src/operations/update.rs b/src/operations/update.rs index f6356d3..46b96ac 100644 --- a/src/operations/update.rs +++ b/src/operations/update.rs @@ -1,4 +1,4 @@ -use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_URL}; +use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_URL, SCHEMES_REPO_REVISION}; use crate::utils::{git_diff, git_update}; use crate::{config::Config, constants::REPO_NAME}; use anyhow::{Context, Result}; @@ -45,7 +45,7 @@ pub fn update(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<() SCHEMES_REPO_NAME, SCHEMES_REPO_URL, &schemes_repo_path, - None, + Some(SCHEMES_REPO_REVISION), is_quiet, )?; diff --git a/src/utils.rs b/src/utils.rs index 2cee53c..8c7b48e 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -156,7 +156,7 @@ fn git_to_revision(repo_path: &Path, revision: &str) -> Result<()> { None => return Err(anyhow!("Error resolving revision \"{}\"", revision)) }; - let command = format!("git checkout \"{}\"", commit_sha); + let command = format!("git -c advice.detachedHead=false checkout \"{}\"", commit_sha); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; let checkout = Command::new(&command_vec[0]) From 6aa0291fe0b19c631ed2c18be32546e9ffc20d41 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sat, 11 Jan 2025 15:02:34 -0800 Subject: [PATCH 04/29] wip --- Cargo.lock | 3 ++- Cargo.toml | 1 + src/utils.rs | 53 ++++++++++++++++++++++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4fc1506..5475938 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "adler2" @@ -1467,6 +1467,7 @@ dependencies = [ "clap_complete", "hex_color", "home", + "rand", "serde", "serde_yaml", "shell-words", diff --git a/Cargo.toml b/Cargo.toml index c889c54..4a5e59f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,3 +27,4 @@ toml = "0.8.19" url = "2.5.4" xdg = "2.5.2" home = "0.5.11" +rand = "0.8.5" diff --git a/src/utils.rs b/src/utils.rs index 8c7b48e..40b6272 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -2,6 +2,7 @@ use crate::config::{Config, ConfigItem, DEFAULT_CONFIG_SHELL}; use crate::constants::{REPO_NAME, SCHEME_EXTENSION}; use anyhow::{anyhow, Context, Result}; use home::home_dir; +use rand::Rng; use std::fs::{self, File}; use std::io::Write; use std::path::{Path, PathBuf}; @@ -90,6 +91,7 @@ pub fn git_pull(repo_path: &Path) -> Result<()> { } pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> Result<()> { + if !repo_path.is_dir() { return Err(anyhow!( "Error with updating. {} is not a directory", @@ -97,34 +99,38 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R )); } - let command = format!("git remote set-url origin \"{}\"", repo_url); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + let remote_name = random_remote_name(); - let remote = Command::new(&command_vec[0]) - .args(&command_vec[1..]) + let remote = Command::new("git") + .args(vec!["remote", "add", &remote_name, repo_url]) .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; if !remote.success() { - return Err(anyhow!("Error with setting remote URL to \"{}\"", repo_url)); + return Err(anyhow!("Error with adding \"{}\" as remote", repo_url)); } - let revision_str = revision.unwrap_or("main"); - return git_to_revision(repo_path, revision_str) + let res = git_to_revision(repo_path, &remote_name,revision_str); + if let Err(e) = res { + return Err(e); + } else { + Ok(()) + } } -fn git_to_revision(repo_path: &Path, revision: &str) -> Result<()> { - println!("{}", repo_path.display()); - - let command = format!("git fetch origin \"{}\"", revision); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; +fn random_remote_name() -> String { + let mut rng = rand::thread_rng(); + let random_number: u32 = rng.gen(); + format!("tinty-remote-{}", random_number) +} - let fetch = Command::new(&command_vec[0]) - .args(&command_vec[1..]) +fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { + let fetch = Command::new("git") + .args(vec!["fetch", remote_name, revision]) .current_dir(repo_path) .stdout(Stdio::null()) .status() @@ -172,6 +178,25 @@ fn git_to_revision(repo_path: &Path, revision: &str) -> Result<()> { Ok(()) } +fn git_current_revision(repo_path: &Path) -> Result { + let parse_out = Command::new("git") + .args(vec!["rev-parse", "--abbrev-ref", "HEAD"]) + .current_dir(repo_path) + .output() + .with_context(|| format!("rev-parse: Failed to execute process in {}", repo_path.display()))?; + + if !parse_out.status.success() { + return Err(anyhow!("Error getting current SHA of \"{}\"", repo_path.display())); + } + + let stdout = String::from_utf8_lossy(&parse_out.stdout); + + return match stdout.lines().next() { + Some(sha) => Ok(sha.to_string()), + None => Err(anyhow!("Error getting current SHA of \"{}\"", repo_path.display())), + }; +} + pub fn git_diff(target_dir: &Path) -> Result { let command = "git status --porcelain"; let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; From fcda1620758d34bde05cdfab9c2fd6a5049df0a5 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sat, 11 Jan 2025 16:07:20 -0800 Subject: [PATCH 05/29] feat: atomic updates --- src/utils.rs | 105 +++++++++++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 40b6272..d9b86c8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -62,7 +62,7 @@ pub fn git_clone(repo_url: &str, target_dir: &Path, revision: Option<&str>) -> R let revision_str = revision.unwrap_or("main"); - return git_to_revision(target_dir, revision_str) + return git_to_revision(target_dir, "origin", revision_str) } pub fn git_pull(repo_path: &Path) -> Result<()> { @@ -99,29 +99,65 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R )); } - let remote_name = random_remote_name(); + // To make this operation atomic, we'll satisfy the remote & revision in this sequence: + // 1.) add the remote URL as a new temporary remote. + // 2.) check if the revision exists in the temporary remote. + // 3.) checkout the revision from temporary remote + // 4.) On success: + // 4.1) replace the origin remote URL + // 4.2) remove the temporary remote + // 5.) On error, remove temporary remote + // + // Note that this sequence works even if the directory is already on that remote & revision. + // + let tmp_remote_name = random_remote_name(); + + let command = format!("git remote add \"{}\" \"{}\"", tmp_remote_name, repo_url); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - let remote = Command::new("git") - .args(vec!["remote", "add", &remote_name, repo_url]) + // Create a temporary remote + Command::new(&command_vec[0]) + .args(&command_vec[1..]) .current_dir(repo_path) .stdout(Stdio::null()) .status() - .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; - - if !remote.success() { - return Err(anyhow!("Error with adding \"{}\" as remote", repo_url)); - } + .with_context(|| format!("Error with adding {} as a remote named {} in {}", repo_url,tmp_remote_name, repo_path.display()))?; + // Attempt to switch to the revision on temporary remote let revision_str = revision.unwrap_or("main"); - let res = git_to_revision(repo_path, &remote_name,revision_str); + let res = git_to_revision(repo_path, &tmp_remote_name, revision_str); if let Err(e) = res { + // Failed to switch to the desired revision. Cleanup! + Command::new("git") + .current_dir(repo_path) + .args(vec!["remote", "rm", &tmp_remote_name]) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; return Err(e); - } else { - Ok(()) } + + let command = format!("git remote set-url origin \"{}\"", repo_url); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + + // Success! Cleanup: update the origin remote to remote URL & delete temporary remote. + Command::new(&command_vec[0]) + .current_dir(repo_path) + .args(&command_vec[1..]) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to set origin remote to {} in {}", repo_url, repo_path.display()))?; + Command::new("git") + .current_dir(repo_path) + .args(vec!["remote", "rm", &tmp_remote_name]) + .stdout(Stdio::null()) + .status() + .with_context(|| format!("Failed to remove temporary remote {} in {}", tmp_remote_name, repo_path.display()))?; + return Ok(()) } + fn random_remote_name() -> String { let mut rng = rand::thread_rng(); let random_number: u32 = rng.gen(); @@ -129,8 +165,12 @@ fn random_remote_name() -> String { } fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { - let fetch = Command::new("git") - .args(vec!["fetch", remote_name, revision]) + + let command = format!("git fetch \"{}\" \"{}\"", remote_name, revision); + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + + let fetch = Command::new(&command_vec[0]) + .args(&command_vec[1..]) .current_dir(repo_path) .stdout(Stdio::null()) .status() @@ -142,61 +182,34 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul // Normalize the revision into the SHA. This way we can support all sorts of revisions, from // branches, tags, SHAs, etc. - let command = format!("git rev-parse \"origin/{}\"", revision); + let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; let parse_out = Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) .output() - .with_context(|| format!("rev-parse: Failed to execute process in {}", repo_path.display()))?; - - if !parse_out.status.success() { - return Err(anyhow!("Error resolving revision \"{}\"", revision)); - } + .with_context(|| format!("Unable to parse revision {} in {}", revision, repo_path.display()))?; let stdout = String::from_utf8_lossy(&parse_out.stdout); let commit_sha = match stdout.lines().next() { Some(sha) => sha, - None => return Err(anyhow!("Error resolving revision \"{}\"", revision)) + None => return Err(anyhow!("Unable to parse revision {} in {}", revision, repo_path.display())) }; let command = format!("git -c advice.detachedHead=false checkout \"{}\"", commit_sha); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - let checkout = Command::new(&command_vec[0]) + Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) .status() - .with_context(|| format!("checkout: Failed to execute process in {}", repo_path.display()))?; - - if !checkout.success() { - return Err(anyhow!("Error checking out revision \"{}\"", commit_sha)); - } + .with_context(|| format!("Failed to checkout SHA {} in {}", commit_sha, repo_path.display()))?; Ok(()) } -fn git_current_revision(repo_path: &Path) -> Result { - let parse_out = Command::new("git") - .args(vec!["rev-parse", "--abbrev-ref", "HEAD"]) - .current_dir(repo_path) - .output() - .with_context(|| format!("rev-parse: Failed to execute process in {}", repo_path.display()))?; - - if !parse_out.status.success() { - return Err(anyhow!("Error getting current SHA of \"{}\"", repo_path.display())); - } - - let stdout = String::from_utf8_lossy(&parse_out.stdout); - - return match stdout.lines().next() { - Some(sha) => Ok(sha.to_string()), - None => Err(anyhow!("Error getting current SHA of \"{}\"", repo_path.display())), - }; -} - pub fn git_diff(target_dir: &Path) -> Result { let command = "git status --porcelain"; let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; From 9d5597b48778a0f651e207df36e82eeee18302b7 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sat, 11 Jan 2025 16:32:05 -0800 Subject: [PATCH 06/29] chore: remove git_pull, now unused --- src/utils.rs | 94 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 55 insertions(+), 39 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index d9b86c8..e38573b 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -60,38 +60,11 @@ pub fn git_clone(repo_url: &str, target_dir: &Path, revision: Option<&str>) -> R .status() .with_context(|| format!("Failed to clone repository from {}", repo_url))?; - let revision_str = revision.unwrap_or("main"); - return git_to_revision(target_dir, "origin", revision_str) -} - -pub fn git_pull(repo_path: &Path) -> Result<()> { - if !repo_path.is_dir() { - return Err(anyhow!( - "Error with git pull. {} is not a directory", - repo_path.display() - )); - } - - let command = "git pull"; - let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; - - let status = Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_path) - .stdout(Stdio::null()) - .status() - .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; - - if status.success() { - Ok(()) - } else { - Err(anyhow!("Error wth git pull in {}", repo_path.display())) - } + return git_to_revision(target_dir, "origin", revision_str); } pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> Result<()> { - if !repo_path.is_dir() { return Err(anyhow!( "Error with updating. {} is not a directory", @@ -121,7 +94,14 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R .current_dir(repo_path) .stdout(Stdio::null()) .status() - .with_context(|| format!("Error with adding {} as a remote named {} in {}", repo_url,tmp_remote_name, repo_path.display()))?; + .with_context(|| { + format!( + "Error with adding {} as a remote named {} in {}", + repo_url, + tmp_remote_name, + repo_path.display() + ) + })?; // Attempt to switch to the revision on temporary remote let revision_str = revision.unwrap_or("main"); @@ -147,17 +127,28 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R .args(&command_vec[1..]) .stdout(Stdio::null()) .status() - .with_context(|| format!("Failed to set origin remote to {} in {}", repo_url, repo_path.display()))?; + .with_context(|| { + format!( + "Failed to set origin remote to {} in {}", + repo_url, + repo_path.display() + ) + })?; Command::new("git") .current_dir(repo_path) .args(vec!["remote", "rm", &tmp_remote_name]) .stdout(Stdio::null()) .status() - .with_context(|| format!("Failed to remove temporary remote {} in {}", tmp_remote_name, repo_path.display()))?; - return Ok(()) + .with_context(|| { + format!( + "Failed to remove temporary remote {} in {}", + tmp_remote_name, + repo_path.display() + ) + })?; + return Ok(()); } - fn random_remote_name() -> String { let mut rng = rand::thread_rng(); let random_number: u32 = rng.gen(); @@ -165,7 +156,6 @@ fn random_remote_name() -> String { } fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { - let command = format!("git fetch \"{}\" \"{}\"", remote_name, revision); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; @@ -174,7 +164,12 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul .current_dir(repo_path) .stdout(Stdio::null()) .status() - .with_context(|| format!("fetch: Failed to execute process in {}", repo_path.display()))?; + .with_context(|| { + format!( + "fetch: Failed to execute process in {}", + repo_path.display() + ) + })?; if !fetch.success() { return Err(anyhow!("Error with fetching \"{}\"", revision)); @@ -189,23 +184,44 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul .args(&command_vec[1..]) .current_dir(repo_path) .output() - .with_context(|| format!("Unable to parse revision {} in {}", revision, repo_path.display()))?; + .with_context(|| { + format!( + "Unable to parse revision {} in {}", + revision, + repo_path.display() + ) + })?; let stdout = String::from_utf8_lossy(&parse_out.stdout); let commit_sha = match stdout.lines().next() { Some(sha) => sha, - None => return Err(anyhow!("Unable to parse revision {} in {}", revision, repo_path.display())) + None => { + return Err(anyhow!( + "Unable to parse revision {} in {}", + revision, + repo_path.display() + )) + } }; - let command = format!("git -c advice.detachedHead=false checkout \"{}\"", commit_sha); + let command = format!( + "git -c advice.detachedHead=false checkout \"{}\"", + commit_sha + ); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) .status() - .with_context(|| format!("Failed to checkout SHA {} in {}", commit_sha, repo_path.display()))?; + .with_context(|| { + format!( + "Failed to checkout SHA {} in {}", + commit_sha, + repo_path.display() + ) + })?; Ok(()) } From 91f5701a0c48b07e73ad9a0efa7794eb49bb9e39 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sat, 11 Jan 2025 22:30:40 -0800 Subject: [PATCH 07/29] style: format --- src/operations/update.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/operations/update.rs b/src/operations/update.rs index 46b96ac..6262d61 100644 --- a/src/operations/update.rs +++ b/src/operations/update.rs @@ -1,16 +1,28 @@ -use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_URL, SCHEMES_REPO_REVISION}; +use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_REVISION, SCHEMES_REPO_URL}; use crate::utils::{git_diff, git_update}; use crate::{config::Config, constants::REPO_NAME}; use anyhow::{Context, Result}; use std::path::Path; -fn update_item(item_name: &str, item_url: &str, item_path: &Path, revision: Option<&str>, is_quiet: bool) -> Result<()> { +fn update_item( + item_name: &str, + item_url: &str, + item_path: &Path, + revision: Option<&str>, + is_quiet: bool, +) -> Result<()> { if item_path.is_dir() { let is_diff = git_diff(item_path)?; if !is_diff { - git_update(item_path, item_url, revision) - .with_context(|| format!("Error updating {} to {}@{}", item_name, item_url, revision.unwrap_or("main")))?; + git_update(item_path, item_url, revision).with_context(|| { + format!( + "Error updating {} to {}@{}", + item_name, + item_url, + revision.unwrap_or("main") + ) + })?; if !is_quiet { println!("{} up to date", item_name); @@ -36,7 +48,13 @@ pub fn update(config_path: &Path, data_path: &Path, is_quiet: bool) -> Result<() for item in items { let item_path = hooks_path.join(&item.name); - update_item(item.name.as_str(), item.path.as_str(), &item_path, item.revision.as_deref(), is_quiet)?; + update_item( + item.name.as_str(), + item.path.as_str(), + &item_path, + item.revision.as_deref(), + is_quiet, + )?; } let schemes_repo_path = hooks_path.join(SCHEMES_REPO_NAME); From 624299f656dad25e0bbca852c20d0ee9138675c6 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sat, 11 Jan 2025 22:55:26 -0800 Subject: [PATCH 08/29] fix: pass --quiet flags to certain git commands --- src/utils.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index e38573b..05b87a6 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -156,25 +156,22 @@ fn random_remote_name() -> String { } fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { - let command = format!("git fetch \"{}\" \"{}\"", remote_name, revision); + let command = format!("git fetch --quiet \"{}\" \"{}\"", remote_name, revision); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - let fetch = Command::new(&command_vec[0]) + Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| { format!( - "fetch: Failed to execute process in {}", + "Error with fetching revision {} in {}", + revision, repo_path.display() ) })?; - if !fetch.success() { - return Err(anyhow!("Error with fetching \"{}\"", revision)); - } - // Normalize the revision into the SHA. This way we can support all sorts of revisions, from // branches, tags, SHAs, etc. let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); @@ -206,13 +203,14 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul }; let command = format!( - "git -c advice.detachedHead=false checkout \"{}\"", + "git -c advice.detachedHead=false checkout --quiet \"{}\"", commit_sha ); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; Command::new(&command_vec[0]) .args(&command_vec[1..]) + .stdout(Stdio::null()) .current_dir(repo_path) .status() .with_context(|| { From 1f02fb7bc66c4bbd8536ca13970a9f006be09a44 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 10:09:52 -0800 Subject: [PATCH 09/29] style: call .args(...) before .current_dir(...) for continuity --- src/utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 05b87a6..74705fb 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -110,8 +110,8 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R if let Err(e) = res { // Failed to switch to the desired revision. Cleanup! Command::new("git") - .current_dir(repo_path) .args(vec!["remote", "rm", &tmp_remote_name]) + .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; @@ -123,8 +123,8 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R // Success! Cleanup: update the origin remote to remote URL & delete temporary remote. Command::new(&command_vec[0]) - .current_dir(repo_path) .args(&command_vec[1..]) + .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| { @@ -135,8 +135,8 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R ) })?; Command::new("git") - .current_dir(repo_path) .args(vec!["remote", "rm", &tmp_remote_name]) + .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| { From efaee76fce9a3a0cc13af15db88f6fe1cfe63488 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 22:45:10 -0800 Subject: [PATCH 10/29] wip: resolve SHA1 without rev-parse --- Cargo.lock | 1 + Cargo.toml | 1 + src/utils.rs | 141 +++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 140 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5475938..e8d0a86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1468,6 +1468,7 @@ dependencies = [ "hex_color", "home", "rand", + "regex", "serde", "serde_yaml", "shell-words", diff --git a/Cargo.toml b/Cargo.toml index 4a5e59f..c6e2a3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,3 +28,4 @@ url = "2.5.4" xdg = "2.5.2" home = "0.5.11" rand = "0.8.5" +regex = "1.7" diff --git a/src/utils.rs b/src/utils.rs index 74705fb..95b08b7 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,10 +1,11 @@ use crate::config::{Config, ConfigItem, DEFAULT_CONFIG_SHELL}; use crate::constants::{REPO_NAME, SCHEME_EXTENSION}; -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, Context, Error, Result}; use home::home_dir; use rand::Rng; +use regex::bytes::Regex; use std::fs::{self, File}; -use std::io::Write; +use std::io::{BufRead, BufReader, Write}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str; @@ -136,7 +137,6 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R })?; Command::new("git") .args(vec!["remote", "rm", &tmp_remote_name]) - .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| { @@ -155,6 +155,140 @@ fn random_remote_name() -> String { format!("tinty-remote-{}", random_number) } +// Resolvees the SHA1 of revision at remote_name. +// revision can be a tag, a branch, or a commit SHA1. +fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result { + // 1.) Check if its a tag. + let expected_tag_ref = format!("refs/tags/{}", revision); + let mut command = safe_command( + format!( + "git ls-remote --quiet --tags \"{}\" \"{}\"", + remote_name, expected_tag_ref + ), + repo_path, + )?; + let mut child = command + .stderr(Stdio::null()) + .stdout(Stdio::piped()) + .spawn() + .with_context(|| format!("Failed to list remote tags from {}", remote_name))?; + let stdout = child.stdout.take().expect("failed to capture stdout"); + let reader = BufReader::new(stdout); + + for line in reader.lines() { + match line { + Ok(line) => { + let parts: Vec<&str> = line.split("\t").collect(); + if parts.len() != 2 { + return Err(anyhow!( + "malformed ls-remote result. Expected tab-delimited tuple, found {} parts", + parts.len() + )); + } + // To hedge against non-exact matches, we'll compare the ref field with + // what we'd expect an exact match would look i.e. refs/tags/ + if parts[1] == expected_tag_ref { + // Found the tag. Return the SHA1 + return Ok(parts[0].to_string()); + } + } + Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), + } + } + + child.wait()?; + + // 2.) Check if its a branch + let expected_branch_ref = format!("refs/heads/{}", revision); + let mut command = safe_command( + format!( + "git ls-remote --quiet --branches \"{}\" \"{}\"", + remote_name, expected_tag_ref + ), + repo_path, + )?; + let mut child = command + .stdout(Stdio::piped()) + .spawn() + .with_context(|| format!("Failed to list branches tags from {}", remote_name))?; + let stdout = child.stdout.take().expect("failed to capture stdout"); + let reader = BufReader::new(stdout); + + for line in reader.lines() { + match line { + Ok(line) => { + let parts: Vec<&str> = line.split("\t").collect(); + if parts.len() != 2 { + return Err(anyhow!( + "malformed ls-remote result. Expected tab-delimited tuple, found {} parts", + parts.len() + )); + } + // To hedge against non-exact matches, we'll compare the ref field with + // what we'd expect an exact match would look i.e. refs/heads/ + if parts[1] == expected_branch_ref { + // Found the tag. Return the SHA1 + return Ok(parts[0].to_string()); + } + } + Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), + } + } + + child.wait()?; + + // We are here because revision isn't a tag or a branch. + // First, we'll check if revision itself *could* be a SHA1. + // If it doesn't look like one, we'll return early. + let pattern = r"^[0-9a-f]{1,40}$"; + let re = Regex::new(pattern).expect("Invalid regex"); + if !re.is_match(revision.as_bytes()) { + return Err(anyhow!("cannot resolve {} into a Git SHA1", revision)); + } + + // 3.) Check if any branch in remote contains the SHA1: + // It seems that the only way to do this is to list the branches that contain the SHA1 + // and check if it belongs in the remote. + let remote_branch_prefix = format!("remotes/{}", remote_name); + let mut command = safe_command( + format!("git branch -a --contains \"{}\"", revision), + repo_path, + )?; + let mut child = command.stdout(Stdio::piped()).spawn().with_context(|| { + format!( + "Failed to find branches containing commit {} from {}", + revision, remote_name + ) + })?; + let stdout = child.stdout.take().expect("failed to capture stdout"); + let reader = BufReader::new(stdout); + + for line in reader.lines() { + match line { + Ok(line) => { + if line.starts_with(&remote_branch_prefix) { + // Found a branch1 + return Ok(revision.to_string()); + } + } + Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), + } + } + + return Err(anyhow!( + "cannot find revision {} in remote {}", + revision, + remote_name + )); +} + +fn safe_command(command: String, cwd: &Path) -> Result { + let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; + let mut command = Command::new(&command_vec[0]); + command.args(&command_vec[1..]).current_dir(cwd); + Ok(command) +} + fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { let command = format!("git fetch --quiet \"{}\" \"{}\"", remote_name, revision); let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; @@ -180,6 +314,7 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul let parse_out = Command::new(&command_vec[0]) .args(&command_vec[1..]) .current_dir(repo_path) + .stderr(Stdio::null()) .output() .with_context(|| { format!( From ecf2341704e66a91c2917d1365008ba32f13f0bc Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 22:45:52 -0800 Subject: [PATCH 11/29] fix: use Cargo.lock version 3 --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index e8d0a86..2d94c47 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 4 +version = 3 [[package]] name = "adler2" From 4948e60a3b56fd56bda07ad7a3aeaca598b3add0 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 22:51:42 -0800 Subject: [PATCH 12/29] fix: stop using rev-parse --- src/utils.rs | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 95b08b7..f88ec89 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -306,36 +306,8 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul ) })?; - // Normalize the revision into the SHA. This way we can support all sorts of revisions, from - // branches, tags, SHAs, etc. - let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - - let parse_out = Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_path) - .stderr(Stdio::null()) - .output() - .with_context(|| { - format!( - "Unable to parse revision {} in {}", - revision, - repo_path.display() - ) - })?; - - let stdout = String::from_utf8_lossy(&parse_out.stdout); - - let commit_sha = match stdout.lines().next() { - Some(sha) => sha, - None => { - return Err(anyhow!( - "Unable to parse revision {} in {}", - revision, - repo_path.display() - )) - } - }; + // Normalize the revision into the SHA. let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); + let commit_sha = git_resolve_revision(repo_path, remote_name, revision)?; let command = format!( "git -c advice.detachedHead=false checkout --quiet \"{}\"", From 58da50e68201f71359cd3e149a8aafb070af7dbc Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 23:02:16 -0800 Subject: [PATCH 13/29] wip: specify cargo-deny as a dependency --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index c6e2a3b..d5e0931 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,3 +29,4 @@ xdg = "2.5.2" home = "0.5.11" rand = "0.8.5" regex = "1.7" +cargo-deny = "0.16.3" From a722083d55129ee4208a82d31435faa782a26fa6 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Sun, 12 Jan 2025 23:48:03 -0800 Subject: [PATCH 14/29] fix: stdout handling --- src/utils.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index f88ec89..ac09bd2 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -104,7 +104,7 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R ) })?; - // Attempt to switch to the revision on temporary remote + println!("remote name: {}", tmp_remote_name); let revision_str = revision.unwrap_or("main"); let res = git_to_revision(repo_path, &tmp_remote_name, revision_str); @@ -137,6 +137,7 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R })?; Command::new("git") .args(vec!["remote", "rm", &tmp_remote_name]) + .current_dir(repo_path) .stdout(Stdio::null()) .status() .with_context(|| { @@ -171,7 +172,8 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> .stderr(Stdio::null()) .stdout(Stdio::piped()) .spawn() - .with_context(|| format!("Failed to list remote tags from {}", remote_name))?; + .with_context(|| format!("Failed to spawn"))?; + let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); @@ -195,22 +197,25 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), } } + child + .wait() + .with_context(|| format!("Failed to list remote tags from {}", remote_name))?; - child.wait()?; // 2.) Check if its a branch let expected_branch_ref = format!("refs/heads/{}", revision); let mut command = safe_command( format!( "git ls-remote --quiet --branches \"{}\" \"{}\"", - remote_name, expected_tag_ref + remote_name, expected_branch_ref ), repo_path, )?; let mut child = command .stdout(Stdio::piped()) .spawn() - .with_context(|| format!("Failed to list branches tags from {}", remote_name))?; + .with_context(|| format!("Failed to spawn"))?; + let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); @@ -235,7 +240,9 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> } } - child.wait()?; + child + .wait() + .with_context(|| format!("Failed to list branches tags from {}", remote_name))?; // We are here because revision isn't a tag or a branch. // First, we'll check if revision itself *could* be a SHA1. From df2cc192a3f9e1e7365838397485e32f2a4fd092 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 00:11:42 -0800 Subject: [PATCH 15/29] fix: fix commit sha1 resolution --- src/utils.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index ac09bd2..595e611 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -104,7 +104,6 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R ) })?; - println!("remote name: {}", tmp_remote_name); let revision_str = revision.unwrap_or("main"); let res = git_to_revision(repo_path, &tmp_remote_name, revision_str); @@ -201,7 +200,6 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> .wait() .with_context(|| format!("Failed to list remote tags from {}", remote_name))?; - // 2.) Check if its a branch let expected_branch_ref = format!("refs/heads/{}", revision); let mut command = safe_command( @@ -253,12 +251,17 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> return Err(anyhow!("cannot resolve {} into a Git SHA1", revision)); } + safe_command(format!("git fetch --quiet \"{}\"", remote_name), repo_path)? + .stdout(Stdio::null()) + .status() + .with_context(|| format!("unable to fetch objects from remote {}", remote_name))?; + // 3.) Check if any branch in remote contains the SHA1: // It seems that the only way to do this is to list the branches that contain the SHA1 // and check if it belongs in the remote. - let remote_branch_prefix = format!("remotes/{}", remote_name); + let remote_branch_prefix = format!("refs/remotes/{}/", remote_name); let mut command = safe_command( - format!("git branch -a --contains \"{}\"", revision), + format!("git branch --format=\"%(refname)\" -a --contains \"{}\"", revision), repo_path, )?; let mut child = command.stdout(Stdio::piped()).spawn().with_context(|| { @@ -273,8 +276,8 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> for line in reader.lines() { match line { Ok(line) => { - if line.starts_with(&remote_branch_prefix) { - // Found a branch1 + if line.clone().starts_with(&remote_branch_prefix) { + // Found a branch return Ok(revision.to_string()); } } @@ -282,6 +285,13 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> } } + child.wait().with_context(|| { + format!( + "Failed to list branches from {} containing SHA1 {}", + remote_name, revision + ) + })?; + return Err(anyhow!( "cannot find revision {} in remote {}", revision, From a009f9e954309f6424f95ce873749e20996afeed Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 00:27:18 -0800 Subject: [PATCH 16/29] chore: use rust 1.84.0 in cargo deny check --- .github/workflows/lint.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 87bc134..68e2173 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -47,6 +47,8 @@ jobs: - uses: actions-rust-lang/setup-rust-toolchain@v1 - uses: EmbarkStudios/cargo-deny-action@v1 + with: + rust-version: "1.84.0" audit: runs-on: ubuntu-latest From 2a3ab5caea62b305c1daf22f8d58fc8ecc007bde Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 00:31:00 -0800 Subject: [PATCH 17/29] style: rustfmt --- src/utils.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index 595e611..79a9555 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -261,7 +261,10 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> // and check if it belongs in the remote. let remote_branch_prefix = format!("refs/remotes/{}/", remote_name); let mut command = safe_command( - format!("git branch --format=\"%(refname)\" -a --contains \"{}\"", revision), + format!( + "git branch --format=\"%(refname)\" -a --contains \"{}\"", + revision + ), repo_path, )?; let mut child = command.stdout(Stdio::piped()).spawn().with_context(|| { From 308a284cb31a3d45e2cd3e194a309914e2bb9030 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 13:00:02 -0800 Subject: [PATCH 18/29] fix(cargo): remove cargo-deny as dependency, doesn't really work that way --- Cargo.lock | 2 +- Cargo.toml | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d94c47..e8d0a86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "adler2" diff --git a/Cargo.toml b/Cargo.toml index d5e0931..c6e2a3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,4 +29,3 @@ xdg = "2.5.2" home = "0.5.11" rand = "0.8.5" regex = "1.7" -cargo-deny = "0.16.3" From 1c1a0e1e7f25c940645af30d94f58976a244cc68 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 13:00:03 -0800 Subject: [PATCH 19/29] test: test install w/ revision --- tests/cli_install_subcommand_tests.rs | 183 ++++++++++++++++++++++++++ 1 file changed, 183 insertions(+) diff --git a/tests/cli_install_subcommand_tests.rs b/tests/cli_install_subcommand_tests.rs index bcc7c79..a584ebf 100644 --- a/tests/cli_install_subcommand_tests.rs +++ b/tests/cli_install_subcommand_tests.rs @@ -1,5 +1,7 @@ mod utils; +use std::{process::Command, str}; + use crate::utils::{setup, write_to_file}; use anyhow::Result; @@ -164,3 +166,184 @@ fn test_cli_install_subcommand_with_setup_quiet_flag() -> Result<()> { cleanup()?; Ok(()) } + +#[test] +fn test_cli_install_subcommand_with_tag_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, repo_path, command_vec, cleanup) = + setup("test_cli_install_subcommand_with_tag_revision", "install")?; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "tinty-test-tag-01" +"##; + write_to_file(&config_path, config_content)?; + + let mut repo_path = repo_path.clone(); + repo_path.push("repos"); + repo_path.push("tinted-jqp"); + + // --- + // Act + // --- + let (_, _) = utils::run_command(command_vec).unwrap(); + + let output = Command::new("git") + .current_dir(repo_path) + .args(vec!["rev-parse", "--verify", "HEAD"]) + .output() + .expect("Failed to execute git rev-parse --verify HEAD"); + + let stdout = str::from_utf8(&output.stdout).expect("Not valid UTF-8"); + + // ------ + // Assert + // ------ + let expected_revision = "b6c6a7803c2669022167c9cfc5efb3dc3928507d"; + let has_match = stdout.lines().any(|line| line == expected_revision); + cleanup()?; + assert!( + has_match == true, + "Expected revision {} not found", + expected_revision, + ); + + Ok(()) +} + +#[test] +fn test_cli_install_subcommand_with_branch_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, repo_path, command_vec, cleanup) = setup( + "test_cli_install_subcommand_with_branch_revision", + "install", + )?; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "tinty-test-01" +"##; + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + let (_, _) = utils::run_command(command_vec).unwrap(); + + let mut repo_path = repo_path.clone(); + repo_path.push("repos"); + repo_path.push("tinted-jqp"); + + let output = Command::new("git") + .current_dir(repo_path) + .args(vec!["rev-parse", "--verify", "HEAD"]) + .output() + .expect("Failed to execute git rev-parse --verify HEAD"); + + let stdout = str::from_utf8(&output.stdout).expect("Not valid UTF-8"); + + // ------ + // Assert + // ------ + let expected_revision = "43b36ed5eadad59a5027e442330d2485b8607b34"; + let has_match = stdout.lines().any(|line| line == expected_revision); + cleanup()?; + assert!( + has_match == true, + "Expected revision {} not found", + expected_revision, + ); + + Ok(()) +} + +#[test] +fn test_cli_install_subcommand_with_commit_sha1_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, repo_path, command_vec, cleanup) = setup( + "test_cli_install_subcommand_with_commit_sha1_revision", + "install", + )?; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "f998d17414a7218904bb5b4fdada5daa2b2d9d5e" +"##; + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + let (_, _) = utils::run_command(command_vec).unwrap(); + + let mut repo_path = repo_path.clone(); + repo_path.push("repos"); + repo_path.push("tinted-jqp"); + + let output = Command::new("git") + .current_dir(repo_path) + .args(vec!["rev-parse", "--verify", "HEAD"]) + .output() + .expect("Failed to execute git rev-parse --verify HEAD"); + + let stdout = str::from_utf8(&output.stdout).expect("Not valid UTF-8"); + + // ------ + // Assert + // ------ + // This SHA1 is only reachable through the tinted-test-01 branch, but is not the tip of that + // branch. + let expected_revision = "f998d17414a7218904bb5b4fdada5daa2b2d9d5e"; + let has_match = stdout.lines().any(|line| line == expected_revision); + cleanup()?; + assert!( + has_match == true, + "Expected revision {} not found", + expected_revision, + ); + + Ok(()) +} + +#[test] +fn test_cli_install_subcommand_with_non_existent_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, repo_path, command_vec, cleanup) = setup( + "test_cli_install_subcommand_with_non_existent_revision", + "install", + )?; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "invalid-revision" +"##; + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + let (_, stderr) = utils::run_command(command_vec).unwrap(); + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + stderr.contains("cannot resolve invalid-revision"), + "Expected revision not found", + ); + + Ok(()) +} From a9bd7f2399b6e679e52b502332a9efe250dec9fd Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 13:00:04 -0800 Subject: [PATCH 20/29] fix: remove repo path on failed install --- src/utils.rs | 9 ++++++++- tests/cli_install_subcommand_tests.rs | 13 +++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index 79a9555..0edc941 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -62,7 +62,14 @@ pub fn git_clone(repo_url: &str, target_dir: &Path, revision: Option<&str>) -> R .with_context(|| format!("Failed to clone repository from {}", repo_url))?; let revision_str = revision.unwrap_or("main"); - return git_to_revision(target_dir, "origin", revision_str); + let result = git_to_revision(target_dir, "origin", revision_str); + if let Err(e) = result { + // Cleanup! If we cannot checkout the revision, remove the directory. + fs::remove_dir_all(target_dir) + .with_context(|| format!("Failed to remove directory {}", target_dir.display()))?; + return Err(e); + } + Ok(()) } pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> Result<()> { diff --git a/tests/cli_install_subcommand_tests.rs b/tests/cli_install_subcommand_tests.rs index a584ebf..a7b54d7 100644 --- a/tests/cli_install_subcommand_tests.rs +++ b/tests/cli_install_subcommand_tests.rs @@ -331,6 +331,10 @@ revision = "invalid-revision" "##; write_to_file(&config_path, config_content)?; + let mut repo_path = repo_path.clone(); + repo_path.push("repos"); + repo_path.push("tinted-jqp"); + // --- // Act // --- @@ -339,11 +343,20 @@ revision = "invalid-revision" // ------ // Assert // ------ + let path_exists = repo_path.exists(); cleanup()?; assert!( stderr.contains("cannot resolve invalid-revision"), "Expected revision not found", ); + assert!( + !path_exists, + "Expected repo path {} to not exist", + repo_path.display(), + ); + + + Ok(()) } From 7a2b14bb2ab74c8bf3d7f43110ca84dd82581282 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 13:55:09 -0800 Subject: [PATCH 21/29] test: test update & new remote and/or new revision --- tests/cli_update_subcommand_tests.rs | 186 +++++++++++++++++++++++++++ 1 file changed, 186 insertions(+) diff --git a/tests/cli_update_subcommand_tests.rs b/tests/cli_update_subcommand_tests.rs index cc7dfe0..ac418c7 100644 --- a/tests/cli_update_subcommand_tests.rs +++ b/tests/cli_update_subcommand_tests.rs @@ -1,7 +1,10 @@ mod utils; +use std::process::Command; + use crate::utils::{setup, REPO_NAME}; use anyhow::Result; +use utils::write_to_file; #[test] fn test_cli_update_subcommand_without_setup() -> Result<()> { @@ -93,3 +96,186 @@ fn test_cli_update_subcommand_with_setup_quiet_flag() -> Result<()> { Ok(()) } + +#[test] +fn test_cli_update_subcommand_with_new_remote() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, data_path, command_vec, cleanup) = + setup("test_cli_update_subcommand_with_new_remote", "update")?; + let expected_output = "tinted-jqp up to date"; + + let config_content = r##"[[items]] +path = "https://github.com/bezhermoso/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + utils::run_install_command(&config_path, &data_path)?; + + // Replace the remote with a new one + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + write_to_file(&config_path, config_content)?; + let (stdout, _) = utils::run_command(command_vec).unwrap(); + + let mut data_path = data_path.clone(); + data_path.push("repos"); + data_path.push("tinted-jqp"); + + let output = Command::new("git") + .args(vec!["remote", "get-url", "origin"]) + .current_dir(&data_path) + .output()?; + + let remote_stdout = String::from_utf8(output.stdout)?; + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + stdout.contains(expected_output), + "stdout does not contain the expected output" + ); + let remote_url = "https://github.com/tinted-theming/tinted-jqp"; + let has_match = remote_stdout.lines().any(|line| line == remote_url); + + assert!(has_match, "Expected origin remote to point to URL {}", remote_url); + + Ok(()) +} + +#[test] +fn test_cli_update_subcommand_with_new_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, data_path, command_vec, cleanup) = + setup("test_cli_update_subcommand_with_new_revision", "update")?; + let expected_output = "tinted-jqp up to date"; + + + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- utils::run_install_command(&config_path, &data_path)?; + + // Replace the remote with a new one + utils::run_install_command(&config_path, &data_path)?; + + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "tinty-test-tag-01" +"##; + write_to_file(&config_path, config_content)?; + let (stdout, _) = utils::run_command(command_vec).unwrap(); + + let mut data_path = data_path.clone(); + data_path.push("repos"); + data_path.push("tinted-jqp"); + + println!("repo_path: {}, exists?: {}", data_path.display(), data_path.exists()); + + let output = Command::new("git") + .args(vec!["rev-parse", "--verify", "HEAD"]) + .current_dir(&data_path) + .output()?; + + let rev_parse_out = String::from_utf8(output.stdout)?; + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + stdout.contains(expected_output), + "stdout does not contain expected output" + ); + let expected_revision = "b6c6a7803c2669022167c9cfc5efb3dc3928507d"; + let has_match = rev_parse_out.lines().any(|line| line == expected_revision); + + assert!(has_match, "Expected revision {}", expected_revision); + + Ok(()) +} + +#[test] +fn test_cli_update_subcommand_with_new_remote_but_invalid_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, data_path, command_vec, cleanup) = + setup("test_cli_update_subcommand_with_new_remote_but_invalid_revision", "update")?; + let expected_output = "tinted-jqp up to date"; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + utils::run_install_command(&config_path, &data_path)?; + + // Replace the remote with a new one + let config_content = r##"[[items]] +path = "https://github.com/bezhermoso/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "invalid-revision" +"##; + write_to_file(&config_path, config_content)?; + let (stdout, stderr) = utils::run_command(command_vec).unwrap(); + + let mut data_path = data_path.clone(); + data_path.push("repos"); + data_path.push("tinted-jqp"); + + let output = Command::new("git") + .args(vec!["remote", "get-url", "origin"]) + .current_dir(&data_path) + .output()?; + + let remote_out = String::from_utf8(output.stdout)?; + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + !stdout.contains(expected_output), + "stdout contains unexpected output" + ); + assert!( + stderr.contains("cannot resolve invalid-revision"), + "stderr does not contain the expected output" + ); + let expected_remote_url = "https://github.com/tinted-theming/tinted-jqp"; + let has_match = remote_out.lines().any(|line| line == expected_remote_url); + + assert!(has_match, "Expected remote URL {}", expected_remote_url); + + Ok(()) +} From 98e67568822585efbc5fa2833a4b5ffbc636b37d Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 14:35:17 -0800 Subject: [PATCH 22/29] style: rustfmt --- tests/cli_install_subcommand_tests.rs | 2 -- tests/cli_update_subcommand_tests.rs | 19 ++++++++++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/cli_install_subcommand_tests.rs b/tests/cli_install_subcommand_tests.rs index a7b54d7..b8682b0 100644 --- a/tests/cli_install_subcommand_tests.rs +++ b/tests/cli_install_subcommand_tests.rs @@ -356,7 +356,5 @@ revision = "invalid-revision" repo_path.display(), ); - - Ok(()) } diff --git a/tests/cli_update_subcommand_tests.rs b/tests/cli_update_subcommand_tests.rs index ac418c7..564f597 100644 --- a/tests/cli_update_subcommand_tests.rs +++ b/tests/cli_update_subcommand_tests.rs @@ -150,7 +150,11 @@ themes-dir = "themes" let remote_url = "https://github.com/tinted-theming/tinted-jqp"; let has_match = remote_stdout.lines().any(|line| line == remote_url); - assert!(has_match, "Expected origin remote to point to URL {}", remote_url); + assert!( + has_match, + "Expected origin remote to point to URL {}", + remote_url + ); Ok(()) } @@ -164,7 +168,6 @@ fn test_cli_update_subcommand_with_new_revision() -> Result<()> { setup("test_cli_update_subcommand_with_new_revision", "update")?; let expected_output = "tinted-jqp up to date"; - let config_content = r##"[[items]] path = "https://github.com/tinted-theming/tinted-jqp" name = "tinted-jqp" @@ -193,7 +196,11 @@ revision = "tinty-test-tag-01" data_path.push("repos"); data_path.push("tinted-jqp"); - println!("repo_path: {}, exists?: {}", data_path.display(), data_path.exists()); + println!( + "repo_path: {}, exists?: {}", + data_path.display(), + data_path.exists() + ); let output = Command::new("git") .args(vec!["rev-parse", "--verify", "HEAD"]) @@ -223,8 +230,10 @@ fn test_cli_update_subcommand_with_new_remote_but_invalid_revision() -> Result<( // ------- // Arrange // ------- - let (config_path, data_path, command_vec, cleanup) = - setup("test_cli_update_subcommand_with_new_remote_but_invalid_revision", "update")?; + let (config_path, data_path, command_vec, cleanup) = setup( + "test_cli_update_subcommand_with_new_remote_but_invalid_revision", + "update", + )?; let expected_output = "tinted-jqp up to date"; let config_content = r##"[[items]] path = "https://github.com/tinted-theming/tinted-jqp" From 37d62d2c77474818326a95f113304fd1202482df Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 16:39:04 -0800 Subject: [PATCH 23/29] chore: use safe_command wrapper --- src/utils.rs | 140 ++++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 74 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 0edc941..bdb0689 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -93,57 +93,52 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R // let tmp_remote_name = random_remote_name(); - let command = format!("git remote add \"{}\" \"{}\"", tmp_remote_name, repo_url); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - // Create a temporary remote - Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_path) - .stdout(Stdio::null()) - .status() - .with_context(|| { - format!( - "Error with adding {} as a remote named {} in {}", - repo_url, - tmp_remote_name, - repo_path.display() - ) - })?; + safe_command( + format!("git remote add \"{}\" \"{}\"", tmp_remote_name, repo_url), + repo_path, + )? + .current_dir(repo_path) + .stdout(Stdio::null()) + .status() + .with_context(|| { + format!( + "Error with adding {} as a remote named {} in {}", + repo_url, + tmp_remote_name, + repo_path.display() + ) + })?; let revision_str = revision.unwrap_or("main"); let res = git_to_revision(repo_path, &tmp_remote_name, revision_str); if let Err(e) = res { // Failed to switch to the desired revision. Cleanup! - Command::new("git") - .args(vec!["remote", "rm", &tmp_remote_name]) - .current_dir(repo_path) + safe_command(format!("git remote rm \"{}\"", &tmp_remote_name), repo_path)? .stdout(Stdio::null()) .status() - .with_context(|| format!("Failed to execute process in {}", repo_path.display()))?; + .with_context(|| { + format!( + "Failed to remove temporary remote {} in {}", + tmp_remote_name, + repo_path.display() + ) + })?; return Err(e); } - let command = format!("git remote set-url origin \"{}\"", repo_url); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - - // Success! Cleanup: update the origin remote to remote URL & delete temporary remote. - Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_path) - .stdout(Stdio::null()) - .status() - .with_context(|| { - format!( - "Failed to set origin remote to {} in {}", - repo_url, - repo_path.display() - ) - })?; - Command::new("git") - .args(vec!["remote", "rm", &tmp_remote_name]) - .current_dir(repo_path) + safe_command(format!("git remote set-url origin \"{}\"", repo_url), repo_path)? + .stdout(Stdio::null()) + .status() + .with_context(|| { + format!( + "Failed to set origin remote to {} in {}", + repo_url, + repo_path.display() + ) + })?; + safe_command(format!("git remote rm \"{}\"", tmp_remote_name), repo_path)? .stdout(Stdio::null()) .status() .with_context(|| { @@ -282,7 +277,6 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> })?; let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); - for line in reader.lines() { match line { Ok(line) => { @@ -317,43 +311,41 @@ fn safe_command(command: String, cwd: &Path) -> Result { } fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Result<()> { - let command = format!("git fetch --quiet \"{}\" \"{}\"", remote_name, revision); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - - Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(repo_path) - .stdout(Stdio::null()) - .status() - .with_context(|| { - format!( - "Error with fetching revision {} in {}", - revision, - repo_path.display() - ) - })?; + // Download the object from the remote + safe_command( + format!("git fetch --quiet \"{}\" \"{}\"", remote_name, revision), + repo_path, + )? + .status() + .with_context(|| { + format!( + "Error with fetching revision {} in {}", + revision, + repo_path.display() + ) + })?; - // Normalize the revision into the SHA. let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); + // Normalize the revision into the SHA. + // let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); let commit_sha = git_resolve_revision(repo_path, remote_name, revision)?; - let command = format!( - "git -c advice.detachedHead=false checkout --quiet \"{}\"", - commit_sha - ); - let command_vec = shell_words::split(&command).map_err(anyhow::Error::new)?; - - Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .stdout(Stdio::null()) - .current_dir(repo_path) - .status() - .with_context(|| { - format!( - "Failed to checkout SHA {} in {}", - commit_sha, - repo_path.display() - ) - })?; + safe_command( + format!( + "git -c advice.detachedHead=false checkout --quiet \"{}\"", + commit_sha + ), + repo_path, + )? + .stdout(Stdio::null()) + .current_dir(repo_path) + .status() + .with_context(|| { + format!( + "Failed to checkout SHA {} in {}", + commit_sha, + repo_path.display() + ) + })?; Ok(()) } From fd4bef1f713cf4429524d484e1eacb5a9ff43958 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 16:39:50 -0800 Subject: [PATCH 24/29] style: remove comment-out code --- src/utils.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils.rs b/src/utils.rs index bdb0689..2c116a4 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -326,7 +326,6 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul })?; // Normalize the revision into the SHA. - // let command = format!("git rev-parse \"{}/{}\"", remote_name, revision); let commit_sha = git_resolve_revision(repo_path, remote_name, revision)?; safe_command( From 2da77cbb225efee3643b8db63c26570dbfd442a8 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 17:04:32 -0800 Subject: [PATCH 25/29] fix: use git diff-index to check if working directory is clean, including staged changes --- src/operations/update.rs | 4 ++-- src/utils.rs | 30 ++++++++---------------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/operations/update.rs b/src/operations/update.rs index 6262d61..50cffab 100644 --- a/src/operations/update.rs +++ b/src/operations/update.rs @@ -1,5 +1,5 @@ use crate::constants::{REPO_DIR, SCHEMES_REPO_NAME, SCHEMES_REPO_REVISION, SCHEMES_REPO_URL}; -use crate::utils::{git_diff, git_update}; +use crate::utils::{git_is_working_dir_clean, git_update}; use crate::{config::Config, constants::REPO_NAME}; use anyhow::{Context, Result}; use std::path::Path; @@ -12,7 +12,7 @@ fn update_item( is_quiet: bool, ) -> Result<()> { if item_path.is_dir() { - let is_diff = git_diff(item_path)?; + let is_diff = git_is_working_dir_clean(item_path)?; if !is_diff { git_update(item_path, item_url, revision).with_context(|| { diff --git a/src/utils.rs b/src/utils.rs index 2c116a4..477061d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -349,30 +349,16 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul Ok(()) } -pub fn git_diff(target_dir: &Path) -> Result { - let command = "git status --porcelain"; - let command_vec = shell_words::split(command).map_err(anyhow::Error::new)?; - let output = Command::new(&command_vec[0]) - .args(&command_vec[1..]) - .current_dir(target_dir) - .output() - .with_context(|| format!("Failed to execute process in {}", target_dir.display()))?; - let stdout = str::from_utf8(&output.stdout).expect("Not valid UTF-8"); - - // If there is no output, then there is no diff - if stdout.is_empty() { - return Ok(false); - } +pub fn git_is_working_dir_clean(target_dir: &Path) -> Result { - // Iterate over the lines and check for changes that should be considered a diff - // Don't consider untracked files a diff - let has_diff = stdout.lines().any(|line| { - let status_code = &line[..2]; - // Status codes: M = modified, A = added, ?? = untracked - status_code != "??" - }); + // We use the Git plumbing diff-index command to tell us of files that has changed, + // both staged and unstaged. + let status = safe_command("git diff-index --quiet HEAD --".to_string(), target_dir)? + .status() + .with_context(|| format!("Failed to execute process in {}", target_dir.display()))?; - Ok(has_diff) + // With the --quiet flag, it will return a 0 exit-code if no files has changed. + Ok(!status.success()) } pub fn create_theme_filename_without_extension(item: &ConfigItem) -> Result { From b74bc2b307ecffcb3354c5e847ff1bcf03295de2 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 17:43:21 -0800 Subject: [PATCH 26/29] style: terser iteration of stdout --- src/utils.rs | 87 ++++++++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 50 deletions(-) diff --git a/src/utils.rs b/src/utils.rs index 477061d..bfc92b8 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -128,7 +128,10 @@ pub fn git_update(repo_path: &Path, repo_url: &str, revision: Option<&str>) -> R return Err(e); } - safe_command(format!("git remote set-url origin \"{}\"", repo_url), repo_path)? + safe_command( + format!("git remote set-url origin \"{}\"", repo_url), + repo_path, + )? .stdout(Stdio::null()) .status() .with_context(|| { @@ -178,26 +181,19 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); - for line in reader.lines() { - match line { - Ok(line) => { - let parts: Vec<&str> = line.split("\t").collect(); - if parts.len() != 2 { - return Err(anyhow!( - "malformed ls-remote result. Expected tab-delimited tuple, found {} parts", - parts.len() - )); - } - // To hedge against non-exact matches, we'll compare the ref field with - // what we'd expect an exact match would look i.e. refs/tags/ - if parts[1] == expected_tag_ref { - // Found the tag. Return the SHA1 - return Ok(parts[0].to_string()); - } - } - Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), - } + if let Some(parts) = reader + .lines() + .filter_map(|line| line.ok()) + .map(|line| line.split("\t").map(String::from).collect::>()) + .filter(|parts| parts.len() == 2) + .find(|parts| parts[1] == expected_tag_ref) + { + // we found a tag that matches + child.kill()?; // Abort the child process. + child.wait()?; // Cleanup + return Ok(parts[0].to_string()); // Return early. } + child .wait() .with_context(|| format!("Failed to list remote tags from {}", remote_name))?; @@ -219,25 +215,17 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); - for line in reader.lines() { - match line { - Ok(line) => { - let parts: Vec<&str> = line.split("\t").collect(); - if parts.len() != 2 { - return Err(anyhow!( - "malformed ls-remote result. Expected tab-delimited tuple, found {} parts", - parts.len() - )); - } - // To hedge against non-exact matches, we'll compare the ref field with - // what we'd expect an exact match would look i.e. refs/heads/ - if parts[1] == expected_branch_ref { - // Found the tag. Return the SHA1 - return Ok(parts[0].to_string()); - } - } - Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), - } + if let Some(parts) = reader + .lines() + .filter_map(|line| line.ok()) + .map(|line| line.split("\t").map(String::from).collect::>()) + .filter(|parts| parts.len() == 2) + .find(|parts| parts[1] == expected_branch_ref) + { + // we found a branch that matches. + child.kill()?; // Abort the child process. + child.wait()?; // Cleanup + return Ok(parts[0].to_string()); // Return early. } child @@ -275,18 +263,18 @@ fn git_resolve_revision(repo_path: &Path, remote_name: &str, revision: &str) -> revision, remote_name ) })?; + let stdout = child.stdout.take().expect("failed to capture stdout"); let reader = BufReader::new(stdout); - for line in reader.lines() { - match line { - Ok(line) => { - if line.clone().starts_with(&remote_branch_prefix) { - // Found a branch - return Ok(revision.to_string()); - } - } - Err(e) => return Err(anyhow!("failed to capture lines: {}", e)), - } + if let Some(_) = reader + .lines() + .filter_map(|line| line.ok()) + .find(|line| line.clone().starts_with(&remote_branch_prefix)) + { + // we found a remote ref that contains the commit sha + child.kill()?; // Abort the child process. + child.wait()?; // Cleanup + return Ok(revision.to_string()); // Return early. } child.wait().with_context(|| { @@ -350,7 +338,6 @@ fn git_to_revision(repo_path: &Path, remote_name: &str, revision: &str) -> Resul } pub fn git_is_working_dir_clean(target_dir: &Path) -> Result { - // We use the Git plumbing diff-index command to tell us of files that has changed, // both staged and unstaged. let status = safe_command("git diff-index --quiet HEAD --".to_string(), target_dir)? From f1d348e738ed8ca0196a11b43def132021218260 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Mon, 13 Jan 2025 19:29:54 -0800 Subject: [PATCH 27/29] test: improve tests to properly test for exclusivity of revisions --- tests/cli_update_subcommand_tests.rs | 140 ++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 4 deletions(-) diff --git a/tests/cli_update_subcommand_tests.rs b/tests/cli_update_subcommand_tests.rs index 564f597..66d0bcf 100644 --- a/tests/cli_update_subcommand_tests.rs +++ b/tests/cli_update_subcommand_tests.rs @@ -226,12 +226,12 @@ revision = "tinty-test-tag-01" } #[test] -fn test_cli_update_subcommand_with_new_remote_but_invalid_revision() -> Result<()> { +fn test_cli_update_subcommand_with_new_remote_but_invalid_tag_revision() -> Result<()> { // ------- // Arrange // ------- let (config_path, data_path, command_vec, cleanup) = setup( - "test_cli_update_subcommand_with_new_remote_but_invalid_revision", + "test_cli_update_subcommand_with_new_remote_but_invalid_tag_revision", "update", )?; let expected_output = "tinted-jqp up to date"; @@ -249,11 +249,142 @@ themes-dir = "themes" utils::run_install_command(&config_path, &data_path)?; // Replace the remote with a new one + // tinty-test-tag-01 exist in tinted-theming but not on this one. let config_content = r##"[[items]] path = "https://github.com/bezhermoso/tinted-jqp" name = "tinted-jqp" themes-dir = "themes" -revision = "invalid-revision" +revision = "tinty-test-tag-01" +"##; + write_to_file(&config_path, config_content)?; + let (stdout, stderr) = utils::run_command(command_vec).unwrap(); + + let mut data_path = data_path.clone(); + data_path.push("repos"); + data_path.push("tinted-jqp"); + + let output = Command::new("git") + .args(vec!["remote", "get-url", "origin"]) + .current_dir(&data_path) + .output()?; + + let remote_out = String::from_utf8(output.stdout)?; + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + !stdout.contains(expected_output), + "stdout contains unexpected output" + ); + assert!( + stderr.contains("cannot resolve tinty-test-tag-01"), + "stderr does not contain the expected output" + ); + let expected_remote_url = "https://github.com/tinted-theming/tinted-jqp"; + let has_match = remote_out.lines().any(|line| line == expected_remote_url); + + assert!(has_match, "Expected remote URL {}", expected_remote_url); + + Ok(()) +} + +#[test] +fn test_cli_update_subcommand_with_new_remote_but_invalid_branch_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, data_path, command_vec, cleanup) = setup( + "test_cli_update_subcommand_with_new_remote_but_invalid_branch_revision", + "update", + )?; + let expected_output = "tinted-jqp up to date"; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + utils::run_install_command(&config_path, &data_path)?; + + // Replace the remote with a new one + // tinty-test-01 exist in tinted-theming but not on this one. + let config_content = r##"[[items]] +path = "https://github.com/bezhermoso/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "tinty-test-01" +"##; + write_to_file(&config_path, config_content)?; + let (stdout, stderr) = utils::run_command(command_vec).unwrap(); + + let mut data_path = data_path.clone(); + data_path.push("repos"); + data_path.push("tinted-jqp"); + + let output = Command::new("git") + .args(vec!["remote", "get-url", "origin"]) + .current_dir(&data_path) + .output()?; + + let remote_out = String::from_utf8(output.stdout)?; + + // ------ + // Assert + // ------ + cleanup()?; + assert!( + !stdout.contains(expected_output), + "stdout contains unexpected output" + ); + assert!( + stderr.contains("cannot resolve tinty-test-01"), + "stderr does not contain the expected output" + ); + let expected_remote_url = "https://github.com/tinted-theming/tinted-jqp"; + let has_match = remote_out.lines().any(|line| line == expected_remote_url); + + assert!(has_match, "Expected remote URL {}", expected_remote_url); + + Ok(()) +} + +#[test] +fn test_cli_update_subcommand_with_new_remote_but_invalid_commit_sha1_revision() -> Result<()> { + // ------- + // Arrange + // ------- + let (config_path, data_path, command_vec, cleanup) = setup( + "test_cli_update_subcommand_with_new_remote_but_commit_sha1_revision", + "update", + )?; + let expected_output = "tinted-jqp up to date"; + let config_content = r##"[[items]] +path = "https://github.com/tinted-theming/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +"##; + + write_to_file(&config_path, config_content)?; + + // --- + // Act + // --- + utils::run_install_command(&config_path, &data_path)?; + + // Replace the remote with a new one + // This commit SHA only exist in tinted-theming but not on this one. + let config_content = r##"[[items]] +path = "https://github.com/bezhermoso/tinted-jqp" +name = "tinted-jqp" +themes-dir = "themes" +revision = "43b36ed5eadad59a5027e442330d2485b8607b34" "##; write_to_file(&config_path, config_content)?; let (stdout, stderr) = utils::run_command(command_vec).unwrap(); @@ -269,6 +400,7 @@ revision = "invalid-revision" let remote_out = String::from_utf8(output.stdout)?; + print!("{}", stdout); // ------ // Assert // ------ @@ -278,7 +410,7 @@ revision = "invalid-revision" "stdout contains unexpected output" ); assert!( - stderr.contains("cannot resolve invalid-revision"), + stderr.contains("cannot find revision 43b36ed5eadad59a5027e442330d2485b8607b34"), "stderr does not contain the expected output" ); let expected_remote_url = "https://github.com/tinted-theming/tinted-jqp"; From 4f920049587f7f55917194a1f7ec3dcf76f8d051 Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Tue, 14 Jan 2025 00:13:56 -0800 Subject: [PATCH 28/29] style: is_diff => is_clean --- src/operations/update.rs | 4 ++-- src/utils.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/operations/update.rs b/src/operations/update.rs index 50cffab..f5e4c0e 100644 --- a/src/operations/update.rs +++ b/src/operations/update.rs @@ -12,9 +12,9 @@ fn update_item( is_quiet: bool, ) -> Result<()> { if item_path.is_dir() { - let is_diff = git_is_working_dir_clean(item_path)?; + let is_clean = git_is_working_dir_clean(item_path)?; - if !is_diff { + if is_clean { git_update(item_path, item_url, revision).with_context(|| { format!( "Error updating {} to {}@{}", diff --git a/src/utils.rs b/src/utils.rs index bfc92b8..9b9ffee 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -345,7 +345,7 @@ pub fn git_is_working_dir_clean(target_dir: &Path) -> Result { .with_context(|| format!("Failed to execute process in {}", target_dir.display()))?; // With the --quiet flag, it will return a 0 exit-code if no files has changed. - Ok(!status.success()) + Ok(status.success()) } pub fn create_theme_filename_without_extension(item: &ConfigItem) -> Result { From 809d242ba3d15fc76d90a665f889b490810a2e3f Mon Sep 17 00:00:00 2001 From: Bez Hermoso Date: Tue, 14 Jan 2025 07:36:29 -0800 Subject: [PATCH 29/29] doc: update README & CHANGELOG --- CHANGELOG.md | 9 +++++++++ README.md | 1 + 2 files changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23d0063..b56557f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## Unreleased +### Added + +- Support `[[items]] revision` property that accepts a tag, branch, or commit SHA1. + +### Changed + +- `tinty update` now keeps the items' `origin` remote URLs up-to-date. +- The item repositories are now checked out to a specific commit SHA1 in "detached HEAD" mode. + ### Fixed - Fix bug where period preceeding extension is still added to generated files even when an extension doesn't exist diff --git a/README.md b/README.md index fd59fa8..309422a 100644 --- a/README.md +++ b/README.md @@ -260,6 +260,7 @@ themes across different applications seamlessly. |------------------------|----------|----------|---------------------------------------------------------------|---------|--------------------------------------------| | `name` | `string` | Required | A unique name for the item being configured. | - | `name = "vim"` | | `path` | `string` | Required | The file system path or URL to the theme template repository. Paths beginning with `~/` map to home dir. | - | `path = "https://github.com/tinted-tmux"` | +| `revision` | `string` | Optional | The Git revision to use.
Accepts a branch name, a tag, or a commit SHA1 | `main` | `revision = "1.2.0"` | | `themes-dir` | `string` | Required | The directory within the repository where theme files are located. | - | `themes-dir = "colors"` | | `hook` | `string` | Optional | A command to be executed after the theme is applied. Useful for reloading configurations. `%f` template variable maps to the path of the applied theme file. | None | `hook = "source ~/.vimrc"` | | `theme-file-extension` | `string` | Optional | Define a custom theme file extension that isn't `/\.*$/`. Tinty looks for themes named `base16-uwunicorn.*` (for example), but when the theme file isn't structured that way, this option can help specify the pattern. | - | `theme-file-extension = ".module.css"` |