diff --git a/CHANGELOG.md b/CHANGELOG.md index 0153a8f735..ec54b4a8df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * After commit: jump back to unstaged area [[@tommady](https://github.com/tommady)] ([#2476](https://github.com/extrawurst/gitui/issues/2476)) * The default key to close the commit error message popup is now the Escape key [[@wessamfathi](https://github.com/wessamfathi)] ([#2552](https://github.com/extrawurst/gitui/issues/2552)) * use OSC52 copying in case other methods fail [[@naseschwarz](https://github.com/naseschwarz)] ([#2366](https://github.com/gitui-org/gitui/issues/2366)) +* push: respect `branch.*.merge` when push default is upstream [[@vlad-anger](https://github.com/vlad-anger)] ([#2542](https://github.com/gitui-org/gitui/pull/2542)) +* set the terminal title to `gitui ({repo_path})` [[@acuteenvy](https://github.com/acuteenvy)] ([#2462](https://github.com/gitui-org/gitui/issues/2462)) + +### Fixes +* Resolve `core.hooksPath` relative to `GIT_WORK_TREE` [[@naseschwarz](https://github.com/naseschwarz)] ([#2571](https://github.com/gitui-org/gitui/issues/2571)) ## [0.27.0] - 2024-01-14 diff --git a/Cargo.lock b/Cargo.lock index b44241cc85..2ff6ac2c19 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -173,6 +173,7 @@ dependencies = [ "fuzzy-matcher", "git2", "git2-hooks", + "git2-testing", "gix", "invalidstring", "log", @@ -219,9 +220,9 @@ checksum = "4c7f02d4ea65f2c1853089ffd8d2787bdbc63de2f0d29dedbcf8ccdfa0ccd4cf" [[package]] name = "base64" -version = "0.21.7" +version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" [[package]] name = "base64ct" @@ -2889,14 +2890,15 @@ dependencies = [ [[package]] name = "ron" -version = "0.8.1" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b91f7eff05f748767f183df4320a63d6936e9c6107d97c9e6bdd9784f4289c94" +checksum = "63f3aa105dea217ef30d89581b65a4d527a19afc95ef5750be3890e8d3c5b837" dependencies = [ "base64", "bitflags 2.9.0", "serde", "serde_derive", + "unicode-ident", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e88a508b30..a1f93e07ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ build = "build.rs" anyhow = "1.0" asyncgit = { path = "./asyncgit", version = "0.27.0", default-features = false } backtrace = "0.3" -base64 = "0.21" +base64 = "0.22" bitflags = "2.9" bugreport = "0.5.1" bwrap = { version = "1.3", features = ["use_std"] } @@ -45,7 +45,7 @@ ratatui = { version = "0.29", default-features = false, features = [ 'serde', ] } rayon-core = "1.12" -ron = "0.8" +ron = "0.9" scopeguard = "1.2" scopetime = { path = "./scopetime", version = "0.1" } serde = "1.0" diff --git a/asyncgit/Cargo.toml b/asyncgit/Cargo.toml index 51971123b3..5b9f8675f2 100644 --- a/asyncgit/Cargo.toml +++ b/asyncgit/Cargo.toml @@ -18,7 +18,7 @@ dirs = "5.0" easy-cast = "0.5" fuzzy-matcher = "0.3" git2 = "0.20" -git2-hooks = { path = "../git2-hooks", version = ">=0.4" } +git2-hooks = { path = "../git2-hooks", version = ">=0.4", features = ["test-utils"] } gix = { version = "0.70.0", default-features = false, features = [ "max-performance", "revision", @@ -39,6 +39,7 @@ url = "2.5" [dev-dependencies] env_logger = "0.11" +git2-testing = { path = "../git2-testing" } invalidstring = { path = "../invalidstring", version = "0.1" } pretty_assertions = "1.4" serial_test = "3.2" diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs index 37fba9fa91..8bd730096e 100644 --- a/asyncgit/src/error.rs +++ b/asyncgit/src/error.rs @@ -53,6 +53,10 @@ pub enum Error { #[error("git error:{0}")] Git(#[from] git2::Error), + /// + #[error("git config error: {0}")] + GitConfig(String), + /// #[error("strip prefix error: {0}")] StripPrefix(#[from] StripPrefixError), diff --git a/asyncgit/src/sync/branch/mod.rs b/asyncgit/src/sync/branch/mod.rs index 1a67390789..93185d6f45 100644 --- a/asyncgit/src/sync/branch/mod.rs +++ b/asyncgit/src/sync/branch/mod.rs @@ -243,6 +243,25 @@ pub fn get_branch_remote( } } +/// Retrieve the upstream merge of a local `branch`, +/// configured in "branch.*.merge" +/// +/// For details check git2 `branch_upstream_merge` +pub fn get_branch_upstream_merge( + repo_path: &RepoPath, + branch: &str, +) -> Result> { + let repo = repo(repo_path)?; + let branch = repo.find_branch(branch, BranchType::Local)?; + let reference = bytes2string(branch.get().name_bytes())?; + let remote_name = repo.branch_upstream_merge(&reference).ok(); + if let Some(remote_name) = remote_name { + Ok(Some(bytes2string(remote_name.as_ref())?)) + } else { + Ok(None) + } +} + /// returns whether the pull merge strategy is set to rebase pub fn config_is_pull_rebase(repo_path: &RepoPath) -> Result { let repo = repo(repo_path)?; @@ -673,6 +692,49 @@ mod tests_branches { assert!(get_branch_remote(repo_path, "foo").is_err()); } + + #[test] + fn test_branch_no_upstream_merge_config() { + let (_r, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: &RepoPath = + &root.as_os_str().to_str().unwrap().into(); + + let upstream_merge_res = + get_branch_upstream_merge(&repo_path, "master"); + assert!( + upstream_merge_res.is_ok_and(|v| v.as_ref().is_none()) + ); + } + + #[test] + fn test_branch_with_upstream_merge_config() { + let (_r, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: &RepoPath = + &root.as_os_str().to_str().unwrap().into(); + + let branch_name = "master"; + let upstrem_merge = "refs/heads/master"; + + let mut config = repo.config().unwrap(); + config + .set_str( + &format!("branch.{branch_name}.merge"), + &upstrem_merge, + ) + .expect("fail set branch merge config"); + + let upstream_merge_res = + get_branch_upstream_merge(&repo_path, &branch_name); + assert!(upstream_merge_res + .as_ref() + .is_ok_and(|v| v.as_ref().is_some())); + assert_eq!( + &upstream_merge_res.unwrap().unwrap(), + upstrem_merge + ); + } } #[cfg(test)] diff --git a/asyncgit/src/sync/config.rs b/asyncgit/src/sync/config.rs index 60d0b409a5..17a62b6736 100644 --- a/asyncgit/src/sync/config.rs +++ b/asyncgit/src/sync/config.rs @@ -62,6 +62,52 @@ pub fn untracked_files_config_repo( Ok(ShowUntrackedFilesConfig::All) } +// see https://git-scm.com/docs/git-config#Documentation/git-config.txt-pushdefault +/// represents `push.default` git config +#[derive(PartialEq, Eq)] +pub enum PushDefaultStrategyConfig { + Nothing, + Current, + Upstream, + Simple, + Matching, +} + +impl Default for PushDefaultStrategyConfig { + fn default() -> Self { + Self::Simple + } +} + +impl<'a> TryFrom<&'a str> for PushDefaultStrategyConfig { + type Error = crate::Error; + fn try_from( + value: &'a str, + ) -> std::result::Result { + match value { + "nothing" => Ok(Self::Nothing), + "current" => Ok(Self::Current), + "upstream" | "tracking" => Ok(Self::Upstream), + "simple" => Ok(Self::Simple), + "matching" => Ok(Self::Matching), + _ => Err(crate::Error::GitConfig(format!( + "malformed value for push.default: {value}, must be one of nothing, matching, simple, upstream or current" + ))), + } + } +} + +pub fn push_default_strategy_config_repo( + repo: &Repository, +) -> Result { + (get_config_string_repo(repo, "push.default")?).map_or_else( + || Ok(PushDefaultStrategyConfig::default()), + |entry_str| { + PushDefaultStrategyConfig::try_from(entry_str.as_str()) + }, + ) +} + /// pub fn untracked_files_config( repo_path: &RepoPath, diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 3e82cf6f8d..2f978c61e2 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -76,6 +76,8 @@ pub fn hooks_prepare_commit_msg( mod tests { use super::*; use crate::sync::tests::repo_init; + use git2_hooks::test_utils::create_hook; + use git2_testing::*; #[test] fn test_post_commit_hook_reject_in_subfolder() { @@ -87,11 +89,7 @@ mod tests { exit 1 "; - git2_hooks::create_hook( - &repo, - git2_hooks::HOOK_POST_COMMIT, - hook, - ); + create_hook(&repo, git2_hooks::HOOK_POST_COMMIT, hook); let subfolder = root.join("foo/"); std::fs::create_dir_all(&subfolder).unwrap(); @@ -124,11 +122,7 @@ mod tests { exit 1 "; - git2_hooks::create_hook( - &repo, - git2_hooks::HOOK_PRE_COMMIT, - hook, - ); + create_hook(&repo, git2_hooks::HOOK_PRE_COMMIT, hook); let res = hooks_pre_commit(repo_path).unwrap(); if let HookResult::NotOk(res) = res { assert_eq!( @@ -151,11 +145,7 @@ mod tests { exit 1 "; - git2_hooks::create_hook( - &repo, - git2_hooks::HOOK_COMMIT_MSG, - hook, - ); + create_hook(&repo, git2_hooks::HOOK_COMMIT_MSG, hook); let subfolder = root.join("foo/"); std::fs::create_dir_all(&subfolder).unwrap(); @@ -174,4 +164,37 @@ mod tests { assert_eq!(msg, String::from("msg\n")); } + + #[test] + fn test_hooks_commit_msg_reject_in_hooks_folder_githooks_moved_absolute( + ) { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let mut config = repo.config().unwrap(); + + const HOOKS_DIR: &'static str = "my_hooks"; + config.set_str("core.hooksPath", HOOKS_DIR).unwrap(); + + let hook = b"#!/bin/sh + echo 'msg' > $1 + echo 'rejected' + exit 1 + "; + let hooks_folder = root.join(HOOKS_DIR); + std::fs::create_dir_all(&hooks_folder).unwrap(); + create_hook_in_path(&hooks_folder.join("commit-msg"), hook); + + let mut msg = String::from("test"); + let res = hooks_commit_msg( + &hooks_folder.to_str().unwrap().into(), + &mut msg, + ) + .unwrap(); + assert_eq!( + res, + HookResult::NotOk(String::from("rejected\n")) + ); + + assert_eq!(msg, String::from("msg\n")); + } } diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 3c4b106c3e..c52a556aad 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -39,7 +39,7 @@ pub use blame::{blame_file, BlameHunk, FileBlame}; pub use branch::{ branch_compare_upstream, checkout_branch, checkout_commit, config_is_pull_rebase, create_branch, delete_branch, - get_branch_remote, get_branches_info, + get_branch_remote, get_branch_upstream_merge, get_branches_info, merge_commit::merge_upstream_commit, merge_ff::branch_merge_upstream_fastforward, merge_rebase::merge_upstream_rebase, rename::rename_branch, diff --git a/asyncgit/src/sync/remotes/push.rs b/asyncgit/src/sync/remotes/push.rs index 97b1f7fc5f..b28b8a22d3 100644 --- a/asyncgit/src/sync/remotes/push.rs +++ b/asyncgit/src/sync/remotes/push.rs @@ -3,7 +3,12 @@ use crate::{ progress::ProgressPercent, sync::{ branch::branch_set_upstream_after_push, + config::{ + push_default_strategy_config_repo, + PushDefaultStrategyConfig, + }, cred::BasicAuthCredential, + get_branch_upstream_merge, remotes::{proxy_auto, Callbacks}, repository::repo, CommitId, RepoPath, @@ -92,7 +97,7 @@ impl AsyncProgress for ProgressNotification { } /// -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum PushType { /// Branch, @@ -145,6 +150,9 @@ pub fn push_raw( let repo = repo(repo_path)?; let mut remote = repo.find_remote(remote)?; + let push_default_strategy = + push_default_strategy_config_repo(&repo)?; + let mut options = PushOptions::new(); options.proxy_options(proxy_auto()); @@ -158,14 +166,28 @@ pub fn push_raw( (true, false) => "+", (false, false) => "", }; - let ref_type = match ref_type { + let git_ref_type = match ref_type { PushType::Branch => "heads", PushType::Tag => "tags", }; - let branch_name = - format!("{branch_modifier}refs/{ref_type}/{branch}"); - remote.push(&[branch_name.as_str()], Some(&mut options))?; + let mut push_ref = + format!("{branch_modifier}refs/{git_ref_type}/{branch}"); + + if !delete + && ref_type == PushType::Branch + && push_default_strategy + == PushDefaultStrategyConfig::Upstream + { + if let Ok(Some(branch_upstream_merge)) = + get_branch_upstream_merge(repo_path, branch) + { + push_ref.push_str(&format!(":{branch_upstream_merge}")); + } + } + + log::debug!("push to: {push_ref}"); + remote.push(&[push_ref], Some(&mut options))?; if let Some((reference, msg)) = callbacks.get_stats()?.push_rejected_msg diff --git a/git2-hooks/Cargo.toml b/git2-hooks/Cargo.toml index 3fab403cd2..3f63282ccd 100644 --- a/git2-hooks/Cargo.toml +++ b/git2-hooks/Cargo.toml @@ -12,8 +12,15 @@ license = "MIT" categories = ["development-tools"] keywords = ["git"] +[features] +default = [] + +# test-utils publishes fns that are useful for testing. These may panic on common errors. +test-utils = ["git2-testing"] + [dependencies] git2 = ">=0.17" +git2-testing = { path = "../git2-testing", optional = true } log = "0.4" shellexpand = "3.1" thiserror = "2.0" diff --git a/git2-hooks/src/hookspath.rs b/git2-hooks/src/hookspath.rs index 3648676ee2..af4f8af0a7 100644 --- a/git2-hooks/src/hookspath.rs +++ b/git2-hooks/src/hookspath.rs @@ -41,16 +41,8 @@ impl HookPaths { if let Some(config_path) = Self::config_hook_path(repo)? { let hooks_path = PathBuf::from(config_path); - let hook = hooks_path.join(hook); - - let hook = shellexpand::full( - hook.as_os_str() - .to_str() - .ok_or(HooksError::PathToString)?, - )?; - - let hook = PathBuf::from_str(hook.as_ref()) - .map_err(|_| HooksError::PathToString)?; + let hook = + Self::expand_path(&hooks_path.join(hook), &pwd)?; return Ok(Self { git: git_dir, @@ -66,6 +58,41 @@ impl HookPaths { }) } + /// Expand path according to the rule of githooks and config + /// core.hooksPath + fn expand_path(path: &Path, pwd: &Path) -> Result { + let hook_expanded = shellexpand::full( + path.as_os_str() + .to_str() + .ok_or(HooksError::PathToString)?, + )?; + let hook_expanded = PathBuf::from_str(hook_expanded.as_ref()) + .map_err(|_| HooksError::PathToString)?; + + // `man git-config`: + // + // > A relative path is taken as relative to the + // > directory where the hooks are run (see the + // > "DESCRIPTION" section of githooks[5]). + // + // `man githooks`: + // + // > Before Git invokes a hook, it changes its + // > working directory to either $GIT_DIR in a bare + // > repository or the root of the working tree in a + // > non-bare repository. + // + // I.e. relative paths in core.hooksPath in non-bare + // repositories are always relative to GIT_WORK_TREE. + Ok({ + if hook_expanded.is_absolute() { + hook_expanded + } else { + pwd.join(hook_expanded) + } + }) + } + fn config_hook_path(repo: &Repository) -> Result> { Ok(repo.config()?.get_string(CONFIG_HOOKS_PATH).ok()) } @@ -232,3 +259,35 @@ impl CommandExt for Command { self } } + +#[cfg(test)] +mod test { + use super::HookPaths; + use std::path::Path; + + #[test] + fn test_hookspath_relative() { + assert_eq!( + HookPaths::expand_path( + &Path::new("pre-commit"), + &Path::new("example_git_root"), + ) + .unwrap(), + Path::new("example_git_root").join("pre-commit") + ); + } + + #[test] + fn test_hookspath_absolute() { + let absolute_hook = + std::env::current_dir().unwrap().join("pre-commit"); + assert_eq!( + HookPaths::expand_path( + &absolute_hook, + &Path::new("example_git_root"), + ) + .unwrap(), + absolute_hook + ); + } +} diff --git a/git2-hooks/src/lib.rs b/git2-hooks/src/lib.rs index 2a458856d7..d28164ebb6 100644 --- a/git2-hooks/src/lib.rs +++ b/git2-hooks/src/lib.rs @@ -30,7 +30,7 @@ mod hookspath; use std::{ fs::File, io::{Read, Write}, - path::{Path, PathBuf}, + path::PathBuf, }; pub use error::HooksError; @@ -80,38 +80,6 @@ impl HookResult { } } -/// helper method to create git hooks programmatically (heavy used in unittests) -/// -/// # Panics -/// Panics if hook could not be created -pub fn create_hook( - r: &Repository, - hook: &str, - hook_script: &[u8], -) -> PathBuf { - let hook = HookPaths::new(r, None, hook).unwrap(); - - let path = hook.hook.clone(); - - create_hook_in_path(&hook.hook, hook_script); - - path -} - -fn create_hook_in_path(path: &Path, hook_script: &[u8]) { - File::create(path).unwrap().write_all(hook_script).unwrap(); - - #[cfg(unix)] - { - std::process::Command::new("chmod") - .arg("+x") - .arg(path) - // .current_dir(path) - .output() - .unwrap(); - } -} - /// Git hook: `commit_msg` /// /// This hook is documented here . @@ -231,10 +199,43 @@ pub fn hooks_prepare_commit_msg( Ok(res) } +#[cfg(any(feature = "test-utils", test))] +mod test_utils_priv { + use crate::hookspath::HookPaths; + use git2::Repository; + use std::path::PathBuf; + + /// helper method to create git hooks programmatically (heavy used in unittests) + /// + /// # Panics + /// Panics if hook could not be created + pub fn create_hook( + r: &Repository, + hook: &str, + hook_script: &[u8], + ) -> PathBuf { + let hook = HookPaths::new(r, None, hook).unwrap(); + + let path = hook.hook.clone(); + + git2_testing::create_hook_in_path(&hook.hook, hook_script); + + path + } +} + +#[cfg(feature = "test-utils")] +pub mod test_utils { + pub use crate::test_utils_priv::create_hook; +} + #[cfg(test)] mod tests { + use super::test_utils_priv::*; use super::*; - use git2_testing::{repo_init, repo_init_bare}; + use git2_testing::{ + create_hook_in_path, repo_init, repo_init_bare, + }; use pretty_assertions::assert_eq; use tempfile::TempDir; diff --git a/git2-testing/src/lib.rs b/git2-testing/src/lib.rs index e8ca87477d..204a6dbaab 100644 --- a/git2-testing/src/lib.rs +++ b/git2-testing/src/lib.rs @@ -1,4 +1,7 @@ use git2::Repository; +use std::fs::File; +use std::io::Write; +use std::path::Path; use tempfile::TempDir; /// initialize test repo in temp path @@ -83,3 +86,21 @@ fn sandbox_config_files() { set_search_path(ConfigLevel::ProgramData, path).unwrap(); }); } + +/// helper method to create a git hook in a custom path (used in unittests) +/// +/// # Panics +/// Panics if hook could not be created +pub fn create_hook_in_path(path: &Path, hook_script: &[u8]) { + File::create(path).unwrap().write_all(hook_script).unwrap(); + + #[cfg(unix)] + { + std::process::Command::new("chmod") + .arg("+x") + .arg(path) + // .current_dir(path) + .output() + .unwrap(); + } +} diff --git a/src/main.rs b/src/main.rs index abc7f7b464..feea894491 100644 --- a/src/main.rs +++ b/src/main.rs @@ -47,7 +47,7 @@ mod ui; mod watcher; use crate::{app::App, args::process_cmdline}; -use anyhow::{bail, Result}; +use anyhow::{anyhow, bail, Result}; use app::QuitState; use asyncgit::{ sync::{utils::repo_work_dir, RepoPath}, @@ -71,7 +71,9 @@ use spinner::Spinner; use std::{ cell::RefCell, io::{self, Stdout}, - panic, process, + panic, + path::Path, + process, time::{Duration, Instant}, }; use ui::style::Theme; @@ -142,8 +144,8 @@ fn main() -> Result<()> { set_panic_handlers()?; - let mut terminal = start_terminal(io::stdout())?; let mut repo_path = cliargs.repo_path; + let mut terminal = start_terminal(io::stdout(), &repo_path)?; let input = Input::new(); let updater = if cliargs.notify_watcher { @@ -359,8 +361,27 @@ fn select_event( Ok(ev) } -fn start_terminal(buf: Stdout) -> io::Result { - let backend = CrosstermBackend::new(buf); +fn start_terminal( + buf: Stdout, + repo_path: &RepoPath, +) -> Result { + let mut path = repo_path.gitpath().canonicalize()?; + let home = dirs::home_dir().ok_or_else(|| { + anyhow!("failed to find the home directory") + })?; + if path.starts_with(&home) { + let relative_part = path + .strip_prefix(&home) + .expect("can't fail because of the if statement"); + path = Path::new("~").join(relative_part); + } + + let mut backend = CrosstermBackend::new(buf); + backend.execute(crossterm::terminal::SetTitle(format!( + "gitui ({})", + path.display() + )))?; + let mut terminal = Terminal::new(backend)?; terminal.hide_cursor()?; terminal.clear()?;