From 03c02a34dc946cd09b17859db4ea7fefa2a6bf52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Mon, 25 Mar 2024 20:10:49 +0100 Subject: [PATCH 1/5] Get default push remote from configuration --- asyncgit/src/sync/remotes/mod.rs | 99 ++++++++++++++++++++++++++++++-- src/popups/push.rs | 7 ++- 2 files changed, 99 insertions(+), 7 deletions(-) diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index 3c68dcec7a..c0e39dfaaa 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -51,6 +51,65 @@ pub fn get_default_remote(repo_path: &RepoPath) -> Result { get_default_remote_in_repo(&repo) } +/// Gets the current branch the user is on. +/// Returns none if they are not on a branch +/// and Err if there was a problem finding the branch +fn get_current_branch( + repo: &Repository, +) -> Result> { + for b in repo.branches(None)? { + let branch = b?.0; + if branch.is_head() { + return Ok(Some(branch)); + } + } + Ok(None) +} + +/// Tries to find the default repo to push to based on configuration. +/// +/// > remote.pushDefault +/// > +/// > The remote to push to by default. Overrides branch..remote for all branches, and is +/// > overridden by branch..pushRemote for specific branches. +/// +/// [git-config-remote-push-default]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault +/// +/// Falls back to `get_default_remote_in_repo`. +pub fn get_default_remote_for_push( + repo_path: &RepoPath, +) -> Result { + let repo = repo(repo_path)?; + get_default_remote_for_push_in_repo(&repo) +} + +pub(crate) fn get_default_remote_for_push_in_repo( + repo: &Repository, +) -> Result { + scope_time!("get_default_remote_for_push_in_repo"); + + let config = repo.config()?; + + let branch = get_current_branch(repo)?; + + if let Some(branch) = branch { + let remote_name = bytes2string(branch.name_bytes()?)?; + + let entry_name = + format!("branch.{}.pushRemote", &remote_name); + + if let Ok(entry) = config.get_entry(&entry_name) { + return bytes2string(entry.value_bytes()); + } + + if let Ok(entry) = config.get_entry("remote.pushDefault") { + return bytes2string(entry.value_bytes()); + } + } + + get_default_remote_in_repo(repo) +} + /// see `get_default_remote` pub(crate) fn get_default_remote_in_repo( repo: &Repository, @@ -272,7 +331,7 @@ mod tests { fn test_default_remote_inconclusive() { let (remote_dir, _remote) = repo_init().unwrap(); let remote_path = remote_dir.path().to_str().unwrap(); - let (repo_dir, _repo) = repo_clone(remote_path).unwrap(); + let (repo_dir, repo) = repo_clone(remote_path).unwrap(); let repo_path: &RepoPath = &repo_dir .into_path() .as_os_str() @@ -299,9 +358,39 @@ mod tests { ] ); - let res = - get_default_remote_in_repo(&repo(repo_path).unwrap()); - assert_eq!(res.is_err(), true); - assert!(matches!(res, Err(Error::NoDefaultRemoteFound))); + let default_push_remote = + get_default_remote_for_push_in_repo(&repo); + + assert!(matches!( + default_push_remote, + Err(Error::NoDefaultRemoteFound) + )); + + let mut config = repo.config().unwrap(); + + config + .set_str("remote.pushDefault", "defaultreporemote") + .unwrap(); + + let default_push_remote = + get_default_remote_for_push_in_repo(&repo); + + assert!( + matches!(default_push_remote, Ok(remote_name) if remote_name == "defaultreporemote") + ); + + config + .set_str( + "branch.master.pushRemote", + "defaultbranchremote", + ) + .unwrap(); + + let default_push_remote = + get_default_remote_for_push_in_repo(&repo); + + assert!( + matches!(default_push_remote, Ok(remote_name) if remote_name == "defaultbranchremote") + ); } } diff --git a/src/popups/push.rs b/src/popups/push.rs index a192e2eac2..8891c12edf 100644 --- a/src/popups/push.rs +++ b/src/popups/push.rs @@ -16,7 +16,9 @@ use asyncgit::{ extract_username_password, need_username_password, BasicAuthCredential, }, - get_branch_remote, get_default_remote, RepoPathRef, + get_branch_remote, + remotes::get_default_remote_for_push, + RepoPathRef, }, AsyncGitNotification, AsyncPush, PushRequest, PushType, RemoteProgress, RemoteProgressState, @@ -132,7 +134,8 @@ impl PushPopup { remote } else { log::info!("push: branch '{}' has no upstream - looking up default remote",self.branch); - let remote = get_default_remote(&self.repo.borrow())?; + let remote = + get_default_remote_for_push(&self.repo.borrow())?; log::info!( "push: branch '{}' to remote '{}'", self.branch, From d3d187af3c34fe8c80b4a5cd711496d777eaad33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 28 Mar 2024 20:02:00 +0100 Subject: [PATCH 2/5] Respect branch..remote for remote to push to --- asyncgit/src/sync/remotes/mod.rs | 45 +++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index c0e39dfaaa..df63b3ccaf 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -70,10 +70,20 @@ fn get_current_branch( /// /// > remote.pushDefault /// > -/// > The remote to push to by default. Overrides branch..remote for all branches, and is -/// > overridden by branch..pushRemote for specific branches. +/// > The remote to push to by default. Overrides `branch..remote` for all branches, and is +/// > overridden by `branch..pushRemote` for specific branches. +/// +/// > branch..remote +/// > +/// > When on branch ``, it tells `git fetch` and `git push` which remote to fetch from or +/// > push to. The remote to push to may be overridden with `remote.pushDefault` (for all +/// > branches). The remote to push to, for the current branch, may be further overridden by +/// > `branch..pushRemote`. If no remote is configured, or if you are not on any branch and +/// > there is more than one remote defined in the repository, it defaults to `origin` for fetching +/// > and `remote.pushDefault` for pushing. /// /// [git-config-remote-push-default]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-remotepushDefault +/// [git-config-branch-name-remote]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-branchltnamegtremote /// /// Falls back to `get_default_remote_in_repo`. pub fn get_default_remote_for_push( @@ -105,6 +115,12 @@ pub(crate) fn get_default_remote_for_push_in_repo( if let Ok(entry) = config.get_entry("remote.pushDefault") { return bytes2string(entry.value_bytes()); } + + let entry_name = format!("branch.{}.remote", &remote_name); + + if let Ok(entry) = config.get_entry(&entry_name) { + return bytes2string(entry.value_bytes()); + } } get_default_remote_in_repo(repo) @@ -328,7 +344,7 @@ mod tests { } #[test] - fn test_default_remote_inconclusive() { + fn test_default_remote_for_push() { let (remote_dir, _remote) = repo_init().unwrap(); let remote_path = remote_dir.path().to_str().unwrap(); let (repo_dir, repo) = repo_clone(remote_path).unwrap(); @@ -358,8 +374,7 @@ mod tests { ] ); - let default_push_remote = - get_default_remote_for_push_in_repo(&repo); + let default_push_remote = get_default_remote_in_repo(&repo); assert!(matches!( default_push_remote, @@ -369,28 +384,34 @@ mod tests { let mut config = repo.config().unwrap(); config - .set_str("remote.pushDefault", "defaultreporemote") + .set_str("branch.master.remote", "branchremote") .unwrap(); let default_push_remote = get_default_remote_for_push_in_repo(&repo); assert!( - matches!(default_push_remote, Ok(remote_name) if remote_name == "defaultreporemote") + matches!(default_push_remote, Ok(remote_name) if remote_name == "branchremote") + ); + + config.set_str("remote.pushDefault", "pushdefault").unwrap(); + + let default_push_remote = + get_default_remote_for_push_in_repo(&repo); + + assert!( + matches!(default_push_remote, Ok(remote_name) if remote_name == "pushdefault") ); config - .set_str( - "branch.master.pushRemote", - "defaultbranchremote", - ) + .set_str("branch.master.pushRemote", "branchpushremote") .unwrap(); let default_push_remote = get_default_remote_for_push_in_repo(&repo); assert!( - matches!(default_push_remote, Ok(remote_name) if remote_name == "defaultbranchremote") + matches!(default_push_remote, Ok(remote_name) if remote_name == "branchpushremote") ); } } From 34e7b6f9408eab6ce9e356563c46967259212584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Wed, 3 Apr 2024 10:59:23 +0200 Subject: [PATCH 3/5] Use default push remote for push --- asyncgit/src/sync/branch/mod.rs | 14 ++++----- asyncgit/src/sync/cred.rs | 51 ++++++++++++++++++++++++++++++- asyncgit/src/sync/mod.rs | 4 +-- asyncgit/src/sync/remotes/push.rs | 4 +-- src/popups/push.rs | 14 ++++----- src/tabs/status.rs | 23 +++++++++++--- 6 files changed, 86 insertions(+), 24 deletions(-) diff --git a/asyncgit/src/sync/branch/mod.rs b/asyncgit/src/sync/branch/mod.rs index 4dbe89aa3a..02645cf5fa 100644 --- a/asyncgit/src/sync/branch/mod.rs +++ b/asyncgit/src/sync/branch/mod.rs @@ -5,13 +5,13 @@ pub mod merge_ff; pub mod merge_rebase; pub mod rename; -use super::{ - remotes::get_default_remote_in_repo, utils::bytes2string, - RepoPath, -}; +use super::{utils::bytes2string, RepoPath}; use crate::{ error::{Error, Result}, - sync::{repository::repo, utils::get_head_repo, CommitId}, + sync::{ + remotes::get_default_remote_for_push_in_repo, + repository::repo, utils::get_head_repo, CommitId, + }, }; use git2::{Branch, BranchType, Repository}; use scopetime::scope_time; @@ -209,7 +209,7 @@ pub struct BranchCompare { } /// -pub(crate) fn branch_set_upstream( +pub(crate) fn branch_set_upstream_after_push( repo: &Repository, branch_name: &str, ) -> Result<()> { @@ -219,7 +219,7 @@ pub(crate) fn branch_set_upstream( repo.find_branch(branch_name, BranchType::Local)?; if branch.upstream().is_err() { - let remote = get_default_remote_in_repo(repo)?; + let remote = get_default_remote_for_push_in_repo(repo)?; let upstream_name = format!("{remote}/{branch_name}"); branch.set_upstream(Some(upstream_name.as_str()))?; } diff --git a/asyncgit/src/sync/cred.rs b/asyncgit/src/sync/cred.rs index 3f908a6352..54eb2cce94 100644 --- a/asyncgit/src/sync/cred.rs +++ b/asyncgit/src/sync/cred.rs @@ -1,7 +1,12 @@ //! credentials git helper use super::{ - remotes::get_default_remote_in_repo, repository::repo, RepoPath, + remotes::{ + get_default_remote_for_push_in_repo, + get_default_remote_in_repo, + }, + repository::repo, + RepoPath, }; use crate::error::{Error, Result}; use git2::CredentialHelper; @@ -43,6 +48,22 @@ pub fn need_username_password(repo_path: &RepoPath) -> Result { Ok(is_http) } +/// know if username and password are needed for this url +pub fn need_username_password_for_push( + repo_path: &RepoPath, +) -> Result { + let repo = repo(repo_path)?; + let remote = repo + .find_remote(&get_default_remote_for_push_in_repo(&repo)?)?; + let url = remote + .pushurl() + .or_else(|| remote.url()) + .ok_or(Error::UnknownRemote)? + .to_owned(); + let is_http = url.starts_with("http"); + Ok(is_http) +} + /// extract username and password pub fn extract_username_password( repo_path: &RepoPath, @@ -71,6 +92,34 @@ pub fn extract_username_password( }) } +/// extract username and password +pub fn extract_username_password_for_push( + repo_path: &RepoPath, +) -> Result { + let repo = repo(repo_path)?; + let url = repo + .find_remote(&get_default_remote_for_push_in_repo(&repo)?)? + .url() + .ok_or(Error::UnknownRemote)? + .to_owned(); + let mut helper = CredentialHelper::new(&url); + + //TODO: look at Cred::credential_helper, + //if the username is in the url we need to set it here, + //I dont think `config` will pick it up + + if let Ok(config) = repo.config() { + helper.config(&config); + } + + Ok(match helper.execute() { + Some((username, password)) => { + BasicAuthCredential::new(Some(username), Some(password)) + } + None => extract_cred_from_url(&url), + }) +} + /// extract credentials from url pub fn extract_cred_from_url(url: &str) -> BasicAuthCredential { url::Url::parse(url).map_or_else( diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index fd9392c92e..f5334ba43a 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -79,8 +79,8 @@ pub use merge::{ }; pub use rebase::rebase_branch; pub use remotes::{ - get_default_remote, get_remotes, push::AsyncProgress, - tags::PushTagsProgress, + get_default_remote, get_default_remote_for_push, get_remotes, + push::AsyncProgress, tags::PushTagsProgress, }; pub(crate) use repository::repo; pub use repository::{RepoPath, RepoPathRef}; diff --git a/asyncgit/src/sync/remotes/push.rs b/asyncgit/src/sync/remotes/push.rs index 37cdd4a9ca..dab50f79ef 100644 --- a/asyncgit/src/sync/remotes/push.rs +++ b/asyncgit/src/sync/remotes/push.rs @@ -2,7 +2,7 @@ use crate::{ error::{Error, Result}, progress::ProgressPercent, sync::{ - branch::branch_set_upstream, + branch::branch_set_upstream_after_push, cred::BasicAuthCredential, remotes::{proxy_auto, Callbacks}, repository::repo, @@ -176,7 +176,7 @@ pub fn push_raw( } if !delete { - branch_set_upstream(&repo, branch)?; + branch_set_upstream_after_push(&repo, branch)?; } Ok(()) diff --git a/src/popups/push.rs b/src/popups/push.rs index 8891c12edf..82787cce30 100644 --- a/src/popups/push.rs +++ b/src/popups/push.rs @@ -13,8 +13,8 @@ use anyhow::Result; use asyncgit::{ sync::{ cred::{ - extract_username_password, need_username_password, - BasicAuthCredential, + extract_username_password_for_push, + need_username_password_for_push, BasicAuthCredential, }, get_branch_remote, remotes::get_default_remote_for_push, @@ -106,11 +106,11 @@ impl PushPopup { self.show()?; - if need_username_password(&self.repo.borrow())? { - let cred = extract_username_password(&self.repo.borrow()) - .unwrap_or_else(|_| { - BasicAuthCredential::new(None, None) - }); + if need_username_password_for_push(&self.repo.borrow())? { + let cred = extract_username_password_for_push( + &self.repo.borrow(), + ) + .unwrap_or_else(|_| BasicAuthCredential::new(None, None)); if cred.is_complete() { self.push_to_remote(Some(cred), force) } else { diff --git a/src/tabs/status.rs b/src/tabs/status.rs index 67605f8e84..feaf62bd49 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -57,6 +57,11 @@ enum DiffTarget { WorkingDir, } +struct RemoteStatus { + has_remotes: bool, + has_remote_for_push: bool, +} + pub struct Status { repo: RepoPathRef, visible: bool, @@ -65,8 +70,8 @@ pub struct Status { index: ChangesComponent, index_wd: ChangesComponent, diff: DiffComponent, + remotes: RemoteStatus, git_diff: AsyncDiff, - has_remotes: bool, git_state: RepoState, git_status_workdir: AsyncStatus, git_status_stage: AsyncStatus, @@ -155,7 +160,10 @@ impl Status { Self { queue: env.queue.clone(), visible: true, - has_remotes: false, + remotes: RemoteStatus { + has_remotes: false, + has_remote_for_push: false, + }, git_state: RepoState::Clean, focus: Focus::WorkDir, diff_target: DiffTarget::WorkingDir, @@ -407,9 +415,14 @@ impl Status { } fn check_remotes(&mut self) { - self.has_remotes = + self.remotes.has_remotes = sync::get_default_remote(&self.repo.borrow().clone()) .is_ok(); + self.remotes.has_remote_for_push = + sync::get_default_remote_for_push( + &self.repo.borrow().clone(), + ) + .is_ok(); } /// @@ -600,11 +613,11 @@ impl Status { .as_ref() .map_or(true, |state| state.ahead > 0); - is_ahead && self.has_remotes + is_ahead && self.remotes.has_remote_for_push } const fn can_pull(&self) -> bool { - self.has_remotes && self.git_branch_state.is_some() + self.remotes.has_remotes && self.git_branch_state.is_some() } fn can_abort_merge(&self) -> bool { From a6bc709ab76b4a56d8a517646c8fac253e01c9df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Wed, 10 Apr 2024 20:07:01 +0200 Subject: [PATCH 4/5] Split test, restoring previous test --- asyncgit/src/sync/remotes/mod.rs | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/asyncgit/src/sync/remotes/mod.rs b/asyncgit/src/sync/remotes/mod.rs index df63b3ccaf..a271efc016 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -344,10 +344,10 @@ mod tests { } #[test] - fn test_default_remote_for_push() { + fn test_default_remote_inconclusive() { let (remote_dir, _remote) = repo_init().unwrap(); let remote_path = remote_dir.path().to_str().unwrap(); - let (repo_dir, repo) = repo_clone(remote_path).unwrap(); + let (repo_dir, _repo) = repo_clone(remote_path).unwrap(); let repo_path: &RepoPath = &repo_dir .into_path() .as_os_str() @@ -374,12 +374,36 @@ mod tests { ] ); - let default_push_remote = get_default_remote_in_repo(&repo); + let default_remote = + get_default_remote_in_repo(&repo(repo_path).unwrap()); assert!(matches!( - default_push_remote, + default_remote, Err(Error::NoDefaultRemoteFound) )); + } + + #[test] + fn test_default_remote_for_push() { + let (remote_dir, _remote) = repo_init().unwrap(); + let remote_path = remote_dir.path().to_str().unwrap(); + let (repo_dir, repo) = repo_clone(remote_path).unwrap(); + let repo_path: &RepoPath = &repo_dir + .into_path() + .as_os_str() + .to_str() + .unwrap() + .into(); + + debug_cmd_print( + repo_path, + "git remote rename origin alternate", + ); + + debug_cmd_print( + repo_path, + &format!("git remote add someremote {remote_path}")[..], + ); let mut config = repo.config().unwrap(); From a638e0dfdb9573306480dbf85f76bc2429115f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Thu, 11 Apr 2024 15:48:08 +0200 Subject: [PATCH 5/5] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4284f56d..99e6e20fb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * support `core.commitChar` filtering [[@concelare](https://github.com/concelare)] ([#2136](https://github.com/extrawurst/gitui/issues/2136)) * allow reset in branch popup ([#2170](https://github.com/extrawurst/gitui/issues/2170)) * support ssh commit signing (when `user.signingKey` and `gpg.format = ssh` of gitconfig are set; ssh-agent isn't yet supported) [[@yanganto](https://github.com/yanganto)] ([#1149](https://github.com/extrawurst/gitui/issues/1149)) +* respect configuration for remote when pushing [[@cruessler](https://github.com/cruessler)] ### Changed * Make info and error message popups scrollable [[@MichaelAug](https://github.com/MichaelAug)] ([#1138](https://github.com/extrawurst/gitui/issues/1138))