diff options
author | Christoph Rüßler <christoph.ruessler@mailbox.org> | 2024-05-16 12:03:55 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-16 12:03:55 +0200 |
commit | a92be3be9d07283373052b34faa589eee39dc068 (patch) | |
tree | 76d0d1f32da0bcb47391a7be658ba83f528a3a4c | |
parent | 7651fdba89fb5bdfaaf9b0a29a7a6ae25c8cbaca (diff) |
Get default fetch remote from configuration (#2204)
fixes #1093
-rw-r--r-- | CHANGELOG.md | 3 | ||||
-rw-r--r-- | asyncgit/src/sync/cred.rs | 51 | ||||
-rw-r--r-- | asyncgit/src/sync/mod.rs | 5 | ||||
-rw-r--r-- | asyncgit/src/sync/remotes/mod.rs | 78 | ||||
-rw-r--r-- | src/popups/pull.rs | 21 | ||||
-rw-r--r-- | src/tabs/status.rs | 27 |
6 files changed, 162 insertions, 23 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 271cfcfe..d8537f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixes +* respect configuration for remote when fetching (also applies to pulling) [[@cruessler](https://github.com/cruessler)] ([#1093](https://github.com/extrawurst/gitui/issues/1093)) + ## [0.26.0+1] - 2024-04-14 **0.26.1** diff --git a/asyncgit/src/sync/cred.rs b/asyncgit/src/sync/cred.rs index 54eb2cce..eb1ef7fc 100644 --- a/asyncgit/src/sync/cred.rs +++ b/asyncgit/src/sync/cred.rs @@ -2,6 +2,7 @@ use super::{ remotes::{ + get_default_remote_for_fetch_in_repo, get_default_remote_for_push_in_repo, get_default_remote_in_repo, }, @@ -49,6 +50,26 @@ pub fn need_username_password(repo_path: &RepoPath) -> Result<bool> { } /// know if username and password are needed for this url +/// TODO: Very similar to `need_username_password_for_fetch`. Can be refactored. See also +/// `need_username_password`. +pub fn need_username_password_for_fetch( + repo_path: &RepoPath, +) -> Result<bool> { + let repo = repo(repo_path)?; + let remote = repo + .find_remote(&get_default_remote_for_fetch_in_repo(&repo)?)?; + let url = remote + .url() + .or_else(|| remote.url()) + .ok_or(Error::UnknownRemote)? + .to_owned(); + let is_http = url.starts_with("http"); + Ok(is_http) +} + +/// know if username and password are needed for this url +/// TODO: Very similar to `need_username_password_for_fetch`. Can be refactored. See also +/// `need_username_password`. pub fn need_username_password_for_push( repo_path: &RepoPath, ) -> Result<bool> { @@ -93,6 +114,36 @@ pub fn extract_username_password( } /// extract username and password +/// TODO: Very similar to `extract_username_password_for_fetch`. Can be refactored. +pub fn extract_username_password_for_fetch( + repo_path: &RepoPath, +) -> Result<BasicAuthCredential> { + let repo = repo(repo_path)?; + let url = repo + .find_remote(&get_default_remote_for_fetch_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 username and password +/// TODO: Very similar to `extract_username_password_for_fetch`. Can be refactored. pub fn extract_username_password_for_push( repo_path: &RepoPath, ) -> Result<BasicAuthCredential> { diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index f5334ba4..4312ff7f 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -79,8 +79,9 @@ pub use merge::{ }; pub use rebase::rebase_branch; pub use remotes::{ - get_default_remote, get_default_remote_for_push, get_remotes, - push::AsyncProgress, tags::PushTagsProgress, + get_default_remote, get_default_remote_for_fetch, + 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/mod.rs b/asyncgit/src/sync/remotes/mod.rs index a271efc0..a97ac1ad 100644 --- a/asyncgit/src/sync/remotes/mod.rs +++ b/asyncgit/src/sync/remotes/mod.rs @@ -66,6 +66,47 @@ fn get_current_branch( Ok(None) } +/// Tries to find the default repo to fetch from based on configuration. +/// +/// > branch.<name>.remote +/// > +/// > When on branch `<name>`, it tells `git fetch` and `git push` which remote to fetch from or +/// > push to. [...] 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 [...]. +/// +/// [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_fetch( + repo_path: &RepoPath, +) -> Result<String> { + let repo = repo(repo_path)?; + get_default_remote_for_fetch_in_repo(&repo) +} + +// TODO: Very similar to `get_default_remote_for_push_in_repo`. Can probably be refactored. +pub(crate) fn get_default_remote_for_fetch_in_repo( + repo: &Repository, +) -> Result<String> { + scope_time!("get_default_remote_for_fetch_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.{}.remote", &remote_name); + + if let Ok(entry) = config.get_entry(&entry_name) { + return bytes2string(entry.value_bytes()); + } + } + + get_default_remote_in_repo(repo) +} + /// Tries to find the default repo to push to based on configuration. /// /// > remote.pushDefault @@ -93,6 +134,7 @@ pub fn get_default_remote_for_push( get_default_remote_for_push_in_repo(&repo) } +// TODO: Very similar to `get_default_remote_for_fetch_in_repo`. Can probably be refactored. pub(crate) fn get_default_remote_for_push_in_repo( repo: &Repository, ) -> Result<String> { @@ -384,6 +426,42 @@ mod tests { } #[test] + fn test_default_remote_for_fetch() { + 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(); + + config + .set_str("branch.master.remote", "branchremote") + .unwrap(); + + let default_fetch_remote = + get_default_remote_for_fetch_in_repo(&repo); + + assert!( + matches!(default_fetch_remote, Ok(remote_name) if remote_name == "branchremote") + ); + } + + #[test] fn test_default_remote_for_push() { let (remote_dir, _remote) = repo_init().unwrap(); let remote_path = remote_dir.path().to_str().unwrap(); diff --git a/src/popups/pull.rs b/src/popups/pull.rs index 58c720a5..44549fd2 100644 --- a/src/popups/pull.rs +++ b/src/popups/pull.rs @@ -15,10 +15,11 @@ use asyncgit::{ sync::{ self, cred::{ - extract_username_password, need_username_password, - BasicAuthCredential, + extract_username_password_for_fetch, + need_username_password_for_fetch, BasicAuthCredential, }, - get_default_remote, RepoPathRef, + remotes::get_default_remote_for_fetch, + RepoPathRef, }, AsyncGitNotification, AsyncPull, FetchRequest, RemoteProgress, }; @@ -69,11 +70,11 @@ impl PullPopup { pub fn fetch(&mut self, branch: String) -> Result<()> { self.branch = branch; 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_fetch(&self.repo.borrow())? { + let cred = extract_username_password_for_fetch( + &self.repo.borrow(), + ) + .unwrap_or_else(|_| BasicAuthCredential::new(None, None)); if cred.is_complete() { self.fetch_from_remote(Some(cred)) } else { @@ -92,7 +93,9 @@ impl PullPopup { self.pending = true; self.progress = None; self.git_fetch.request(FetchRequest { - remote: get_default_remote(&self.repo.borrow())?, + remote: get_default_remote_for_fetch( + &self.repo.borrow(), + )?, branch: self.branch.clone(), basic_credential: cred, })?; diff --git a/src/tabs/status.rs b/src/tabs/status.rs index feaf62bd..4c21eaeb 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -58,7 +58,7 @@ enum DiffTarget { } struct RemoteStatus { - has_remotes: bool, + has_remote_for_fetch: bool, has_remote_for_push: bool, } @@ -161,7 +161,7 @@ impl Status { queue: env.queue.clone(), visible: true, remotes: RemoteStatus { - has_remotes: false, + has_remote_for_fetch: false, has_remote_for_push: false, }, git_state: RepoState::Clean, @@ -415,9 +415,11 @@ impl Status { } fn check_remotes(&mut self) { - self.remotes.has_remotes = - sync::get_default_remote(&self.repo.borrow().clone()) - .is_ok(); + self.remotes.has_remote_for_fetch = + sync::get_default_remote_for_fetch( + &self.repo.borrow().clone(), + ) + .is_ok(); self.remotes.has_remote_for_push = sync::get_default_remote_for_push( &self.repo.borrow().clone(), @@ -580,7 +582,7 @@ impl Status { } fn fetch(&self) { - if self.can_pull() { + if self.can_fetch() { self.queue.push(InternalEvent::FetchRemotes); } } @@ -616,8 +618,9 @@ impl Status { is_ahead && self.remotes.has_remote_for_push } - const fn can_pull(&self) -> bool { - self.remotes.has_remotes && self.git_branch_state.is_some() + const fn can_fetch(&self) -> bool { + self.remotes.has_remote_for_fetch + && self.git_branch_state.is_some() } fn can_abort_merge(&self) -> bool { @@ -754,12 +757,12 @@ impl Component for Status { out.push(CommandInfo::new( strings::commands::status_fetch(&self.key_config), - self.can_pull(), + self.can_fetch(), !focus_on_diff, )); out.push(CommandInfo::new( strings::commands::status_pull(&self.key_config), - self.can_pull(), + self.can_fetch(), !focus_on_diff, )); @@ -881,13 +884,13 @@ impl Component for Status { Ok(EventState::Consumed) } else if key_match(k, self.key_config.keys.fetch) && !self.is_focus_on_diff() - && self.can_pull() + && self.can_fetch() { self.fetch(); Ok(EventState::Consumed) } else if key_match(k, self.key_config.keys.pull) && !self.is_focus_on_diff() - && self.can_pull() + && self.can_fetch() { self.pull(); Ok(EventState::Consumed) |