From 7bca15704b719996ba79df70678bcfac158d4a12 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 12 Mar 2025 13:39:30 -0600 Subject: [PATCH] Git on main thread (#26573) This moves spawning of the git subprocess to the main thread. We're not yet sure why, but when we spawn a process using GCD's background queues, sub-processes like git-credential-manager fail to open windows. This seems to be fixable either by using the main thread, or by using a standard background thread, but for now we use the main thread. Release Notes: - Git: Fix git-credential-manager --------- Co-authored-by: Max Brunsfeld Co-authored-by: Kirill Bulatov --- crates/collab/src/tests/integration_tests.rs | 26 +- .../remote_editing_collaboration_tests.rs | 26 +- crates/git/src/blame.rs | 26 +- crates/git/src/commit.rs | 5 +- crates/git/src/repository.rs | 1266 ++++++++++------- crates/git_ui/src/branch_picker.rs | 6 +- crates/git_ui/src/git_panel.rs | 12 +- crates/project/src/buffer_store.rs | 98 +- crates/project/src/git.rs | 129 +- .../remote_server/src/remote_editing_tests.rs | 26 +- crates/worktree/src/worktree.rs | 19 +- 11 files changed, 926 insertions(+), 713 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index d3cef14b0e..4e99b67dba 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -6770,7 +6770,7 @@ async fn test_remote_git_branches( assert_eq!(branches_b, branches_set); - cx_b.update(|cx| repo_b.read(cx).change_branch(new_branch)) + cx_b.update(|cx| repo_b.read(cx).change_branch(new_branch.to_string())) .await .unwrap() .unwrap(); @@ -6790,15 +6790,23 @@ async fn test_remote_git_branches( assert_eq!(host_branch.name, branches[2]); // Also try creating a new branch - cx_b.update(|cx| repo_b.read(cx).create_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx_b.update(|cx| { + repo_b + .read(cx) + .create_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); - cx_b.update(|cx| repo_b.read(cx).change_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx_b.update(|cx| { + repo_b + .read(cx) + .change_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); executor.run_until_parked(); diff --git a/crates/collab/src/tests/remote_editing_collaboration_tests.rs b/crates/collab/src/tests/remote_editing_collaboration_tests.rs index bb889f849c..fbc25cbf03 100644 --- a/crates/collab/src/tests/remote_editing_collaboration_tests.rs +++ b/crates/collab/src/tests/remote_editing_collaboration_tests.rs @@ -294,7 +294,7 @@ async fn test_ssh_collaboration_git_branches( assert_eq!(&branches_b, &branches_set); - cx_b.update(|cx| repo_b.read(cx).change_branch(new_branch)) + cx_b.update(|cx| repo_b.read(cx).change_branch(new_branch.to_string())) .await .unwrap() .unwrap(); @@ -316,15 +316,23 @@ async fn test_ssh_collaboration_git_branches( assert_eq!(server_branch.name, branches[2]); // Also try creating a new branch - cx_b.update(|cx| repo_b.read(cx).create_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx_b.update(|cx| { + repo_b + .read(cx) + .create_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); - cx_b.update(|cx| repo_b.read(cx).change_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx_b.update(|cx| { + repo_b + .read(cx) + .change_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); executor.run_until_parked(); diff --git a/crates/git/src/blame.rs b/crates/git/src/blame.rs index 71d61eeefc..8bdf92aa72 100644 --- a/crates/git/src/blame.rs +++ b/crates/git/src/blame.rs @@ -2,8 +2,8 @@ use crate::commit::get_messages; use crate::Oid; use anyhow::{anyhow, Context as _, Result}; use collections::{HashMap, HashSet}; +use futures::AsyncWriteExt; use serde::{Deserialize, Serialize}; -use std::io::Write; use std::process::Stdio; use std::{ops::Range, path::Path}; use text::Rope; @@ -21,14 +21,14 @@ pub struct Blame { } impl Blame { - pub fn for_path( + pub async fn for_path( git_binary: &Path, working_directory: &Path, path: &Path, content: &Rope, remote_url: Option, ) -> Result { - let output = run_git_blame(git_binary, working_directory, path, content)?; + let output = run_git_blame(git_binary, working_directory, path, content).await?; let mut entries = parse_git_blame(&output)?; entries.sort_unstable_by(|a, b| a.range.start.cmp(&b.range.start)); @@ -39,8 +39,9 @@ impl Blame { } let shas = unique_shas.into_iter().collect::>(); - let messages = - get_messages(working_directory, &shas).context("failed to get commit messages")?; + let messages = get_messages(working_directory, &shas) + .await + .context("failed to get commit messages")?; Ok(Self { entries, @@ -53,13 +54,13 @@ impl Blame { const GIT_BLAME_NO_COMMIT_ERROR: &str = "fatal: no such ref: HEAD"; const GIT_BLAME_NO_PATH: &str = "fatal: no such path"; -fn run_git_blame( +async fn run_git_blame( git_binary: &Path, working_directory: &Path, path: &Path, contents: &Rope, ) -> Result { - let child = util::command::new_std_command(git_binary) + let mut child = util::command::new_smol_command(git_binary) .current_dir(working_directory) .arg("blame") .arg("--incremental") @@ -72,18 +73,19 @@ fn run_git_blame( .spawn() .map_err(|e| anyhow!("Failed to start git blame process: {}", e))?; - let mut stdin = child + let stdin = child .stdin - .as_ref() + .as_mut() .context("failed to get pipe to stdin of git blame command")?; for chunk in contents.chunks() { - stdin.write_all(chunk.as_bytes())?; + stdin.write_all(chunk.as_bytes()).await?; } - stdin.flush()?; + stdin.flush().await?; let output = child - .wait_with_output() + .output() + .await .map_err(|e| anyhow!("Failed to read git blame output: {}", e))?; if !output.status.success() { diff --git a/crates/git/src/commit.rs b/crates/git/src/commit.rs index f32ad226af..0c2feb9af2 100644 --- a/crates/git/src/commit.rs +++ b/crates/git/src/commit.rs @@ -3,20 +3,21 @@ use anyhow::{anyhow, Result}; use collections::HashMap; use std::path::Path; -pub fn get_messages(working_directory: &Path, shas: &[Oid]) -> Result> { +pub async fn get_messages(working_directory: &Path, shas: &[Oid]) -> Result> { if shas.is_empty() { return Ok(HashMap::default()); } const MARKER: &str = ""; - let output = util::command::new_std_command("git") + let output = util::command::new_smol_command("git") .current_dir(working_directory) .arg("show") .arg("-s") .arg(format!("--format=%B{}", MARKER)) .args(shas.iter().map(ToString::to_string)) .output() + .await .map_err(|e| anyhow!("Failed to start git blame process: {}", e))?; anyhow::ensure!( diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index 31f4a5d4c4..ad0798e619 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -4,15 +4,15 @@ use crate::{blame::Blame, status::GitStatus}; use anyhow::{anyhow, Context, Result}; use askpass::{AskPassResult, AskPassSession}; use collections::{HashMap, HashSet}; -use futures::{select_biased, FutureExt as _}; +use futures::future::BoxFuture; +use futures::{select_biased, AsyncWriteExt, FutureExt as _}; use git2::BranchType; -use gpui::SharedString; +use gpui::{AppContext, AsyncApp, SharedString}; use parking_lot::Mutex; use rope::Rope; use schemars::JsonSchema; use serde::Deserialize; use std::borrow::Borrow; -use std::io::Write as _; use std::process::Stdio; use std::sync::LazyLock; use std::{ @@ -21,7 +21,7 @@ use std::{ sync::Arc, }; use sum_tree::MapSeekTarget; -use util::command::{new_smol_command, new_std_command}; +use util::command::new_smol_command; use util::ResultExt; pub const REMOTE_CANCELLED_BY_USER: &str = "Operation cancelled by user"; @@ -156,19 +156,20 @@ pub trait GitRepository: Send + Sync { /// Returns the contents of an entry in the repository's index, or None if there is no entry for the given path. /// /// Also returns `None` for symlinks. - fn load_index_text(&self, path: &RepoPath) -> Option; + fn load_index_text(&self, path: RepoPath, cx: AsyncApp) -> BoxFuture>; /// Returns the contents of an entry in the repository's HEAD, or None if HEAD does not exist or has no entry for the given path. /// /// Also returns `None` for symlinks. - fn load_committed_text(&self, path: &RepoPath) -> Option; + fn load_committed_text(&self, path: RepoPath, cx: AsyncApp) -> BoxFuture>; fn set_index_text( &self, - path: &RepoPath, + path: RepoPath, content: Option, - env: &HashMap, - ) -> anyhow::Result<()>; + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; /// Returns the URL of the remote with the given name. fn remote_url(&self, name: &str) -> Option; @@ -178,25 +179,36 @@ pub trait GitRepository: Send + Sync { fn merge_head_shas(&self) -> Vec; - /// Returns the list of git statuses, sorted by path + // Note: this method blocks the current thread! fn status(&self, path_prefixes: &[RepoPath]) -> Result; - fn branches(&self) -> Result>; - fn change_branch(&self, _: &str) -> Result<()>; - fn create_branch(&self, _: &str) -> Result<()>; - fn branch_exits(&self, _: &str) -> Result; + fn branches(&self) -> BoxFuture>>; + + fn change_branch(&self, _: String, _: AsyncApp) -> BoxFuture>; + fn create_branch(&self, _: String, _: AsyncApp) -> BoxFuture>; + + fn reset( + &self, + commit: String, + mode: ResetMode, + env: HashMap, + ) -> BoxFuture>; - fn reset(&self, commit: &str, mode: ResetMode, env: &HashMap) -> Result<()>; fn checkout_files( &self, - commit: &str, - paths: &[RepoPath], - env: &HashMap, - ) -> Result<()>; + commit: String, + paths: Vec, + env: HashMap, + ) -> BoxFuture>; - fn show(&self, commit: &str) -> Result; + fn show(&self, commit: String, cx: AsyncApp) -> BoxFuture>; - fn blame(&self, path: &Path, content: Rope) -> Result; + fn blame( + &self, + path: RepoPath, + content: Rope, + cx: AsyncApp, + ) -> BoxFuture>; /// Returns the absolute path to the repository. For worktrees, this will be the path to the /// worktree's gitdir within the main repository (typically `.git/worktrees/`). @@ -213,48 +225,67 @@ pub trait GitRepository: Send + Sync { /// Updates the index to match the worktree at the given paths. /// /// If any of the paths have been deleted from the worktree, they will be removed from the index if found there. - fn stage_paths(&self, paths: &[RepoPath], env: &HashMap) -> Result<()>; + fn stage_paths( + &self, + paths: Vec, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; /// Updates the index to match HEAD at the given paths. /// /// If any of the paths were previously staged but do not exist in HEAD, they will be removed from the index. - fn unstage_paths(&self, paths: &[RepoPath], env: &HashMap) -> Result<()>; + fn unstage_paths( + &self, + paths: Vec, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; fn commit( &self, - message: &str, - name_and_email: Option<(&str, &str)>, - env: &HashMap, - ) -> Result<()>; + message: SharedString, + name_and_email: Option<(SharedString, SharedString)>, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; fn push( &self, - branch_name: &str, - upstream_name: &str, + branch_name: String, + upstream_name: String, options: Option, askpass: AskPassSession, - env: &HashMap, - ) -> Result; + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; fn pull( &self, - branch_name: &str, - upstream_name: &str, + branch_name: String, + upstream_name: String, askpass: AskPassSession, - env: &HashMap, - ) -> Result; + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; + fn fetch( &self, askpass: AskPassSession, - env: &HashMap, - ) -> Result; + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture>; - fn get_remotes(&self, branch_name: Option<&str>) -> Result>; + fn get_remotes( + &self, + branch_name: Option, + cx: AsyncApp, + ) -> BoxFuture>>; /// returns a list of remote branches that contain HEAD - fn check_for_pushed_commit(&self) -> Result>; + fn check_for_pushed_commit(&self, cx: AsyncApp) -> BoxFuture>>; /// Run git diff - fn diff(&self, diff: DiffType) -> Result; + fn diff(&self, diff: DiffType, cx: AsyncApp) -> BoxFuture>; } pub enum DiffType { @@ -275,14 +306,14 @@ impl std::fmt::Debug for dyn GitRepository { } pub struct RealGitRepository { - pub repository: Mutex, + pub repository: Arc>, pub git_binary_path: PathBuf, } impl RealGitRepository { pub fn new(repository: git2::Repository, git_binary_path: Option) -> Self { Self { - repository: Mutex::new(repository), + repository: Arc::new(Mutex::new(repository)), git_binary_path: git_binary_path.unwrap_or_else(|| PathBuf::from("git")), } } @@ -316,162 +347,200 @@ impl GitRepository for RealGitRepository { repo.commondir().into() } - fn show(&self, commit: &str) -> Result { - let repo = self.repository.lock(); - let Ok(commit) = repo.revparse_single(commit)?.into_commit() else { - anyhow::bail!("{} is not a commit", commit); - }; - let details = CommitDetails { - sha: commit.id().to_string().into(), - message: String::from_utf8_lossy(commit.message_raw_bytes()) - .to_string() - .into(), - commit_timestamp: commit.time().seconds(), - committer_email: String::from_utf8_lossy(commit.committer().email_bytes()) - .to_string() - .into(), - committer_name: String::from_utf8_lossy(commit.committer().name_bytes()) - .to_string() - .into(), - }; - Ok(details) + fn show(&self, commit: String, cx: AsyncApp) -> BoxFuture> { + let repo = self.repository.clone(); + cx.background_spawn(async move { + let repo = repo.lock(); + let Ok(commit) = repo.revparse_single(&commit)?.into_commit() else { + anyhow::bail!("{} is not a commit", commit); + }; + let details = CommitDetails { + sha: commit.id().to_string().into(), + message: String::from_utf8_lossy(commit.message_raw_bytes()) + .to_string() + .into(), + commit_timestamp: commit.time().seconds(), + committer_email: String::from_utf8_lossy(commit.committer().email_bytes()) + .to_string() + .into(), + committer_name: String::from_utf8_lossy(commit.committer().name_bytes()) + .to_string() + .into(), + }; + Ok(details) + }) + .boxed() } - fn reset(&self, commit: &str, mode: ResetMode, env: &HashMap) -> Result<()> { - let working_directory = self.working_directory()?; + fn reset( + &self, + commit: String, + mode: ResetMode, + env: HashMap, + ) -> BoxFuture> { + async move { + let working_directory = self.working_directory(); - let mode_flag = match mode { - ResetMode::Mixed => "--mixed", - ResetMode::Soft => "--soft", - }; + let mode_flag = match mode { + ResetMode::Mixed => "--mixed", + ResetMode::Soft => "--soft", + }; - let output = new_std_command(&self.git_binary_path) - .envs(env) - .current_dir(&working_directory) - .args(["reset", mode_flag, commit]) - .output()?; - if !output.status.success() { - return Err(anyhow!( - "Failed to reset:\n{}", - String::from_utf8_lossy(&output.stderr) - )); + let output = new_smol_command(&self.git_binary_path) + .envs(env) + .current_dir(&working_directory?) + .args(["reset", mode_flag, &commit]) + .output() + .await?; + if !output.status.success() { + return Err(anyhow!( + "Failed to reset:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + Ok(()) } - Ok(()) + .boxed() } fn checkout_files( &self, - commit: &str, - paths: &[RepoPath], - env: &HashMap, - ) -> Result<()> { - if paths.is_empty() { - return Ok(()); - } - let working_directory = self.working_directory()?; + commit: String, + paths: Vec, + env: HashMap, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + async move { + if paths.is_empty() { + return Ok(()); + } - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["checkout", commit, "--"]) - .args(paths.iter().map(|path| path.as_ref())) - .output()?; - if !output.status.success() { - return Err(anyhow!( - "Failed to checkout files:\n{}", - String::from_utf8_lossy(&output.stderr) - )); + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory?) + .envs(env) + .args(["checkout", &commit, "--"]) + .args(paths.iter().map(|path| path.as_ref())) + .output() + .await?; + if !output.status.success() { + return Err(anyhow!( + "Failed to checkout files:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + Ok(()) } - Ok(()) + .boxed() } - fn load_index_text(&self, path: &RepoPath) -> Option { - fn logic(repo: &git2::Repository, path: &RepoPath) -> Result> { - const STAGE_NORMAL: i32 = 0; - let index = repo.index()?; + fn load_index_text(&self, path: RepoPath, cx: AsyncApp) -> BoxFuture> { + let repo = self.repository.clone(); + cx.background_spawn(async move { + fn logic(repo: &git2::Repository, path: &RepoPath) -> Result> { + const STAGE_NORMAL: i32 = 0; + let index = repo.index()?; - // This check is required because index.get_path() unwraps internally :( - check_path_to_repo_path_errors(path)?; + // This check is required because index.get_path() unwraps internally :( + check_path_to_repo_path_errors(path)?; - let oid = match index.get_path(path, STAGE_NORMAL) { - Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id, - _ => return Ok(None), - }; + let oid = match index.get_path(path, STAGE_NORMAL) { + Some(entry) if entry.mode != GIT_MODE_SYMLINK => entry.id, + _ => return Ok(None), + }; - let content = repo.find_blob(oid)?.content().to_owned(); - Ok(Some(String::from_utf8(content)?)) - } - - match logic(&self.repository.lock(), path) { - Ok(value) => return value, - Err(err) => log::error!("Error loading index text: {:?}", err), - } - None + let content = repo.find_blob(oid)?.content().to_owned(); + Ok(Some(String::from_utf8(content)?)) + } + match logic(&repo.lock(), &path) { + Ok(value) => return value, + Err(err) => log::error!("Error loading index text: {:?}", err), + } + None + }) + .boxed() } - fn load_committed_text(&self, path: &RepoPath) -> Option { - let repo = self.repository.lock(); - let head = repo.head().ok()?.peel_to_tree().log_err()?; - let entry = head.get_path(path).ok()?; - if entry.filemode() == i32::from(git2::FileMode::Link) { - return None; - } - let content = repo.find_blob(entry.id()).log_err()?.content().to_owned(); - let content = String::from_utf8(content).log_err()?; - Some(content) + fn load_committed_text(&self, path: RepoPath, cx: AsyncApp) -> BoxFuture> { + let repo = self.repository.clone(); + cx.background_spawn(async move { + let repo = repo.lock(); + let head = repo.head().ok()?.peel_to_tree().log_err()?; + let entry = head.get_path(&path).ok()?; + if entry.filemode() == i32::from(git2::FileMode::Link) { + return None; + } + let content = repo.find_blob(entry.id()).log_err()?.content().to_owned(); + let content = String::from_utf8(content).log_err()?; + Some(content) + }) + .boxed() } fn set_index_text( &self, - path: &RepoPath, + path: RepoPath, content: Option, - env: &HashMap, - ) -> anyhow::Result<()> { - let working_directory = self.working_directory()?; - if let Some(content) = content { - let mut child = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["hash-object", "-w", "--stdin"]) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .spawn()?; - child.stdin.take().unwrap().write_all(content.as_bytes())?; - let output = child.wait_with_output()?.stdout; - let sha = String::from_utf8(output)?; + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + let working_directory = working_directory?; + if let Some(content) = content { + let mut child = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .envs(&env) + .args(["hash-object", "-w", "--stdin"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn()?; + child + .stdin + .take() + .unwrap() + .write_all(content.as_bytes()) + .await?; + let output = child.output().await?.stdout; + let sha = String::from_utf8(output)?; - log::debug!("indexing SHA: {sha}, path {path:?}"); + log::debug!("indexing SHA: {sha}, path {path:?}"); - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["update-index", "--add", "--cacheinfo", "100644", &sha]) - .arg(path.as_ref()) - .output()?; + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .envs(env) + .args(["update-index", "--add", "--cacheinfo", "100644", &sha]) + .arg(path.as_ref()) + .output() + .await?; - if !output.status.success() { - return Err(anyhow!( - "Failed to stage:\n{}", - String::from_utf8_lossy(&output.stderr) - )); + if !output.status.success() { + return Err(anyhow!( + "Failed to stage:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + } else { + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .envs(env) + .args(["update-index", "--force-remove"]) + .arg(path.as_ref()) + .output() + .await?; + + if !output.status.success() { + return Err(anyhow!( + "Failed to unstage:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } } - } else { - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["update-index", "--force-remove"]) - .arg(path.as_ref()) - .output()?; - if !output.status.success() { - return Err(anyhow!( - "Failed to unstage:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - } - - Ok(()) + Ok(()) + }) + .boxed() } fn remote_url(&self, name: &str) -> Option { @@ -515,415 +584,477 @@ impl GitRepository for RealGitRepository { GitStatus::new(&self.git_binary_path, &working_directory, path_prefixes) } - fn branch_exits(&self, name: &str) -> Result { - let repo = self.repository.lock(); - let branch = repo.find_branch(name, BranchType::Local); - match branch { - Ok(_) => Ok(true), - Err(e) => match e.code() { - git2::ErrorCode::NotFound => Ok(false), - _ => Err(anyhow!(e)), - }, - } - } - - fn branches(&self) -> Result> { - let working_directory = self - .repository - .lock() - .workdir() - .context("failed to read git work directory")? - .to_path_buf(); - let fields = [ - "%(HEAD)", - "%(objectname)", - "%(parent)", - "%(refname)", - "%(upstream)", - "%(upstream:track)", - "%(committerdate:unix)", - "%(contents:subject)", - ] - .join("%00"); - let args = vec!["for-each-ref", "refs/heads/**/*", "--format", &fields]; - - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .args(args) - .output()?; - - if !output.status.success() { - return Err(anyhow!( - "Failed to git git branches:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - - let input = String::from_utf8_lossy(&output.stdout); - - let mut branches = parse_branch_input(&input)?; - if branches.is_empty() { - let args = vec!["symbolic-ref", "--quiet", "--short", "HEAD"]; - - let output = new_std_command(&self.git_binary_path) + fn branches(&self) -> BoxFuture>> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + async move { + let fields = [ + "%(HEAD)", + "%(objectname)", + "%(parent)", + "%(refname)", + "%(upstream)", + "%(upstream:track)", + "%(committerdate:unix)", + "%(contents:subject)", + ] + .join("%00"); + let args = vec!["for-each-ref", "refs/heads/**/*", "--format", &fields]; + let working_directory = working_directory?; + let output = new_smol_command(&git_binary_path) .current_dir(&working_directory) .args(args) - .output()?; + .output() + .await?; - // git symbolic-ref returns a non-0 exit code if HEAD points - // to something other than a branch - if output.status.success() { - let name = String::from_utf8_lossy(&output.stdout).trim().to_string(); - - branches.push(Branch { - name: name.into(), - is_head: true, - upstream: None, - most_recent_commit: None, - }); + if !output.status.success() { + return Err(anyhow!( + "Failed to git git branches:\n{}", + String::from_utf8_lossy(&output.stderr) + )); } + + let input = String::from_utf8_lossy(&output.stdout); + + let mut branches = parse_branch_input(&input)?; + if branches.is_empty() { + let args = vec!["symbolic-ref", "--quiet", "--short", "HEAD"]; + + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .args(args) + .output() + .await?; + + // git symbolic-ref returns a non-0 exit code if HEAD points + // to something other than a branch + if output.status.success() { + let name = String::from_utf8_lossy(&output.stdout).trim().to_string(); + + branches.push(Branch { + name: name.into(), + is_head: true, + upstream: None, + most_recent_commit: None, + }); + } + } + + Ok(branches) } - - Ok(branches) + .boxed() } - fn change_branch(&self, name: &str) -> Result<()> { - let repo = self.repository.lock(); - let revision = repo.find_branch(name, BranchType::Local)?; - let revision = revision.get(); - let as_tree = revision.peel_to_tree()?; - repo.checkout_tree(as_tree.as_object(), None)?; - repo.set_head( - revision - .name() - .ok_or_else(|| anyhow!("Branch name could not be retrieved"))?, - )?; - Ok(()) + fn change_branch(&self, name: String, cx: AsyncApp) -> BoxFuture> { + let repo = self.repository.clone(); + cx.background_spawn(async move { + let repo = repo.lock(); + let revision = repo.find_branch(&name, BranchType::Local)?; + let revision = revision.get(); + let as_tree = revision.peel_to_tree()?; + repo.checkout_tree(as_tree.as_object(), None)?; + repo.set_head( + revision + .name() + .ok_or_else(|| anyhow!("Branch name could not be retrieved"))?, + )?; + Ok(()) + }) + .boxed() } - fn create_branch(&self, name: &str) -> Result<()> { - let repo = self.repository.lock(); - let current_commit = repo.head()?.peel_to_commit()?; - repo.branch(name, ¤t_commit, false)?; - Ok(()) + fn create_branch(&self, name: String, cx: AsyncApp) -> BoxFuture> { + let repo = self.repository.clone(); + cx.background_spawn(async move { + let repo = repo.lock(); + let current_commit = repo.head()?.peel_to_commit()?; + repo.branch(&name, ¤t_commit, false)?; + Ok(()) + }) + .boxed() } - fn blame(&self, path: &Path, content: Rope) -> Result { - let working_directory = self - .repository - .lock() - .workdir() - .with_context(|| format!("failed to get git working directory for file {:?}", path))? - .to_path_buf(); + fn blame( + &self, + path: RepoPath, + content: Rope, + cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); const REMOTE_NAME: &str = "origin"; let remote_url = self.remote_url(REMOTE_NAME); - crate::blame::Blame::for_path( - &self.git_binary_path, - &working_directory, - path, - &content, - remote_url, - ) + cx.background_spawn(async move { + crate::blame::Blame::for_path( + &git_binary_path, + &working_directory?, + &path, + &content, + remote_url, + ) + .await + }) + .boxed() } - fn diff(&self, diff: DiffType) -> Result { - let working_directory = self.working_directory()?; - let args = match diff { - DiffType::HeadToIndex => Some("--staged"), - DiffType::HeadToWorktree => None, - }; + fn diff(&self, diff: DiffType, cx: AsyncApp) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + let args = match diff { + DiffType::HeadToIndex => Some("--staged"), + DiffType::HeadToWorktree => None, + }; - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .args(["diff"]) - .args(args) - .output()?; - - if !output.status.success() { - return Err(anyhow!( - "Failed to run git diff:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - Ok(String::from_utf8_lossy(&output.stdout).to_string()) - } - - fn stage_paths(&self, paths: &[RepoPath], env: &HashMap) -> Result<()> { - let working_directory = self.working_directory()?; - - if !paths.is_empty() { - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["update-index", "--add", "--remove", "--"]) - .args(paths.iter().map(|p| p.as_ref())) - .output()?; + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory?) + .args(["diff"]) + .args(args) + .output() + .await?; if !output.status.success() { return Err(anyhow!( - "Failed to stage paths:\n{}", + "Failed to run git diff:\n{}", String::from_utf8_lossy(&output.stderr) )); } - } - Ok(()) + Ok(String::from_utf8_lossy(&output.stdout).to_string()) + }) + .boxed() } - fn unstage_paths(&self, paths: &[RepoPath], env: &HashMap) -> Result<()> { - let working_directory = self.working_directory()?; + fn stage_paths( + &self, + paths: Vec, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + if !paths.is_empty() { + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory?) + .envs(env) + .args(["update-index", "--add", "--remove", "--"]) + .args(paths.iter().map(|p| p.as_ref())) + .output() + .await?; - if !paths.is_empty() { - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .envs(env) - .args(["reset", "--quiet", "--"]) - .args(paths.iter().map(|p| p.as_ref())) - .output()?; - - if !output.status.success() { - return Err(anyhow!( - "Failed to unstage:\n{}", - String::from_utf8_lossy(&output.stderr) - )); + if !output.status.success() { + return Err(anyhow!( + "Failed to stage paths:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } } - } - Ok(()) + Ok(()) + }) + .boxed() + } + + fn unstage_paths( + &self, + paths: Vec, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + + cx.background_spawn(async move { + if !paths.is_empty() { + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory?) + .envs(env) + .args(["reset", "--quiet", "--"]) + .args(paths.iter().map(|p| p.as_ref())) + .output() + .await?; + + if !output.status.success() { + return Err(anyhow!( + "Failed to unstage:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + } + Ok(()) + }) + .boxed() } fn commit( &self, - message: &str, - name_and_email: Option<(&str, &str)>, - env: &HashMap, - ) -> Result<()> { - let working_directory = self.working_directory()?; + message: SharedString, + name_and_email: Option<(SharedString, SharedString)>, + env: HashMap, + cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + let mut cmd = new_smol_command(&git_binary_path); + cmd.current_dir(&working_directory?) + .envs(env) + .args(["commit", "--quiet", "-m"]) + .arg(&message.to_string()) + .arg("--cleanup=strip"); - let mut cmd = new_std_command(&self.git_binary_path); - cmd.current_dir(&working_directory) - .envs(env) - .args(["commit", "--quiet", "-m"]) - .arg(message) - .arg("--cleanup=strip"); + if let Some((name, email)) = name_and_email { + cmd.arg("--author").arg(&format!("{name} <{email}>")); + } - if let Some((name, email)) = name_and_email { - cmd.arg("--author").arg(&format!("{name} <{email}>")); - } + let output = cmd.output().await?; - let output = cmd.output()?; - - if !output.status.success() { - return Err(anyhow!( - "Failed to commit:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - Ok(()) + if !output.status.success() { + return Err(anyhow!( + "Failed to commit:\n{}", + String::from_utf8_lossy(&output.stderr) + )); + } + Ok(()) + }) + .boxed() } fn push( &self, - branch_name: &str, - remote_name: &str, + branch_name: String, + remote_name: String, options: Option, ask_pass: AskPassSession, - env: &HashMap, - ) -> Result { - let working_directory = self.working_directory()?; + env: HashMap, + // note: git push *must* be started on the main thread for + // git-credentials manager to work (hence taking an AsyncApp) + _cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + async move { + let working_directory = working_directory?; - let mut command = new_smol_command("git"); - command - .envs(env) - .env("GIT_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS_REQUIRE", "force") - .current_dir(&working_directory) - .args(["push"]) - .args(options.map(|option| match option { - PushOptions::SetUpstream => "--set-upstream", - PushOptions::Force => "--force-with-lease", - })) - .arg(remote_name) - .arg(format!("{}:{}", branch_name, branch_name)) - .stdout(smol::process::Stdio::piped()) - .stderr(smol::process::Stdio::piped()); - let git_process = command.spawn()?; + let mut command = new_smol_command("git"); + command + .envs(env) + .env("GIT_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS_REQUIRE", "force") + .env("GIT_HTTP_USER_AGENT", "Zed") + .current_dir(&working_directory) + .args(["push"]) + .args(options.map(|option| match option { + PushOptions::SetUpstream => "--set-upstream", + PushOptions::Force => "--force-with-lease", + })) + .arg(remote_name) + .arg(format!("{}:{}", branch_name, branch_name)) + .stdin(smol::process::Stdio::null()) + .stdout(smol::process::Stdio::piped()) + .stderr(smol::process::Stdio::piped()); + let git_process = command.spawn()?; - run_remote_command(ask_pass, git_process) + run_remote_command(ask_pass, git_process).await + } + .boxed() } fn pull( &self, - branch_name: &str, - remote_name: &str, + branch_name: String, + remote_name: String, ask_pass: AskPassSession, - env: &HashMap, - ) -> Result { - let working_directory = self.working_directory()?; + env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + async { + let mut command = new_smol_command("git"); + command + .envs(env) + .env("GIT_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS_REQUIRE", "force") + .current_dir(&working_directory?) + .args(["pull"]) + .arg(remote_name) + .arg(branch_name) + .stdout(smol::process::Stdio::piped()) + .stderr(smol::process::Stdio::piped()); + let git_process = command.spawn()?; - let mut command = new_smol_command("git"); - command - .envs(env) - .env("GIT_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS_REQUIRE", "force") - .current_dir(&working_directory) - .args(["pull"]) - .arg(remote_name) - .arg(branch_name) - .stdout(smol::process::Stdio::piped()) - .stderr(smol::process::Stdio::piped()); - let git_process = command.spawn()?; - - run_remote_command(ask_pass, git_process) + run_remote_command(ask_pass, git_process).await + } + .boxed() } fn fetch( &self, ask_pass: AskPassSession, - env: &HashMap, - ) -> Result { - let working_directory = self.working_directory()?; + env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { + let working_directory = self.working_directory(); + async { + let mut command = new_smol_command("git"); + command + .envs(env) + .env("GIT_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS", ask_pass.script_path()) + .env("SSH_ASKPASS_REQUIRE", "force") + .current_dir(&working_directory?) + .args(["fetch", "--all"]) + .stdout(smol::process::Stdio::piped()) + .stderr(smol::process::Stdio::piped()); + let git_process = command.spawn()?; - let mut command = new_smol_command("git"); - command - .envs(env) - .env("GIT_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS", ask_pass.script_path()) - .env("SSH_ASKPASS_REQUIRE", "force") - .current_dir(&working_directory) - .args(["fetch", "--all"]) - .stdout(smol::process::Stdio::piped()) - .stderr(smol::process::Stdio::piped()); - let git_process = command.spawn()?; - - run_remote_command(ask_pass, git_process) + run_remote_command(ask_pass, git_process).await + } + .boxed() } - fn get_remotes(&self, branch_name: Option<&str>) -> Result> { - let working_directory = self.working_directory()?; + fn get_remotes( + &self, + branch_name: Option, + cx: AsyncApp, + ) -> BoxFuture>> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + let working_directory = working_directory?; + if let Some(branch_name) = branch_name { + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .args(["config", "--get"]) + .arg(format!("branch.{}.remote", branch_name)) + .output() + .await?; - if let Some(branch_name) = branch_name { - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .args(["config", "--get"]) - .arg(format!("branch.{}.remote", branch_name)) - .output()?; + if output.status.success() { + let remote_name = String::from_utf8_lossy(&output.stdout); - if output.status.success() { - let remote_name = String::from_utf8_lossy(&output.stdout); - - return Ok(vec![Remote { - name: remote_name.trim().to_string().into(), - }]); - } - } - - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .args(["remote"]) - .output()?; - - if output.status.success() { - let remote_names = String::from_utf8_lossy(&output.stdout) - .split('\n') - .filter(|name| !name.is_empty()) - .map(|name| Remote { - name: name.trim().to_string().into(), - }) - .collect(); - - return Ok(remote_names); - } else { - return Err(anyhow!( - "Failed to get remotes:\n{}", - String::from_utf8_lossy(&output.stderr) - )); - } - } - - fn check_for_pushed_commit(&self) -> Result> { - let working_directory = self.working_directory()?; - let git_cmd = |args: &[&str]| -> Result { - let output = new_std_command(&self.git_binary_path) - .current_dir(&working_directory) - .args(args) - .output()?; - if output.status.success() { - Ok(String::from_utf8(output.stdout)?) - } else { - Err(anyhow!(String::from_utf8_lossy(&output.stderr).to_string())) - } - }; - - let head = git_cmd(&["rev-parse", "HEAD"]) - .context("Failed to get HEAD")? - .trim() - .to_owned(); - - let mut remote_branches = vec![]; - let mut add_if_matching = |remote_head: &str| { - if let Ok(merge_base) = git_cmd(&["merge-base", &head, remote_head]) { - if merge_base.trim() == head { - if let Some(s) = remote_head.strip_prefix("refs/remotes/") { - remote_branches.push(s.to_owned().into()); - } + return Ok(vec![Remote { + name: remote_name.trim().to_string().into(), + }]); } } - }; - // check the main branch of each remote - let remotes = git_cmd(&["remote"]).context("Failed to get remotes")?; - for remote in remotes.lines() { - if let Ok(remote_head) = - git_cmd(&["symbolic-ref", &format!("refs/remotes/{remote}/HEAD")]) - { - add_if_matching(remote_head.trim()); + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .args(["remote"]) + .output() + .await?; + + if output.status.success() { + let remote_names = String::from_utf8_lossy(&output.stdout) + .split('\n') + .filter(|name| !name.is_empty()) + .map(|name| Remote { + name: name.trim().to_string().into(), + }) + .collect(); + + return Ok(remote_names); + } else { + return Err(anyhow!( + "Failed to get remotes:\n{}", + String::from_utf8_lossy(&output.stderr) + )); } - } + }) + .boxed() + } - // ... and the remote branch that the checked-out one is tracking - if let Ok(remote_head) = git_cmd(&["rev-parse", "--symbolic-full-name", "@{u}"]) { - add_if_matching(remote_head.trim()); - } + fn check_for_pushed_commit(&self, cx: AsyncApp) -> BoxFuture>> { + let working_directory = self.working_directory(); + let git_binary_path = self.git_binary_path.clone(); + cx.background_spawn(async move { + let working_directory = working_directory?; + let git_cmd = async |args: &[&str]| -> Result { + let output = new_smol_command(&git_binary_path) + .current_dir(&working_directory) + .args(args) + .output() + .await?; + if output.status.success() { + Ok(String::from_utf8(output.stdout)?) + } else { + Err(anyhow!(String::from_utf8_lossy(&output.stderr).to_string())) + } + }; - Ok(remote_branches) + let head = git_cmd(&["rev-parse", "HEAD"]) + .await + .context("Failed to get HEAD")? + .trim() + .to_owned(); + + let mut remote_branches = vec![]; + let mut add_if_matching = async |remote_head: &str| { + if let Ok(merge_base) = git_cmd(&["merge-base", &head, remote_head]).await { + if merge_base.trim() == head { + if let Some(s) = remote_head.strip_prefix("refs/remotes/") { + remote_branches.push(s.to_owned().into()); + } + } + } + }; + + // check the main branch of each remote + let remotes = git_cmd(&["remote"]) + .await + .context("Failed to get remotes")?; + for remote in remotes.lines() { + if let Ok(remote_head) = + git_cmd(&["symbolic-ref", &format!("refs/remotes/{remote}/HEAD")]).await + { + add_if_matching(remote_head.trim()).await; + } + } + + // ... and the remote branch that the checked-out one is tracking + if let Ok(remote_head) = git_cmd(&["rev-parse", "--symbolic-full-name", "@{u}"]).await { + add_if_matching(remote_head.trim()).await; + } + + Ok(remote_branches) + }) + .boxed() } } -fn run_remote_command( +async fn run_remote_command( mut ask_pass: AskPassSession, git_process: smol::process::Child, ) -> std::result::Result { - smol::block_on(async { - select_biased! { - result = ask_pass.run().fuse() => { - match result { - AskPassResult::CancelledByUser => { - Err(anyhow!(REMOTE_CANCELLED_BY_USER))? - } - AskPassResult::Timedout => { - Err(anyhow!("Connecting to host timed out"))? - } + select_biased! { + result = ask_pass.run().fuse() => { + match result { + AskPassResult::CancelledByUser => { + Err(anyhow!(REMOTE_CANCELLED_BY_USER))? } - } - output = git_process.output().fuse() => { - let output = output?; - if !output.status.success() { - Err(anyhow!( - "{}", - String::from_utf8_lossy(&output.stderr) - )) - } else { - Ok(RemoteCommandOutput { - stdout: String::from_utf8_lossy(&output.stdout).to_string(), - stderr: String::from_utf8_lossy(&output.stderr).to_string(), - }) + AskPassResult::Timedout => { + Err(anyhow!("Connecting to host timed out"))? } } } - }) + output = git_process.output().fuse() => { + let output = output?; + if !output.status.success() { + Err(anyhow!( + "{}", + String::from_utf8_lossy(&output.stderr) + )) + } else { + Ok(RemoteCommandOutput { + stdout: String::from_utf8_lossy(&output.stdout).to_string(), + stderr: String::from_utf8_lossy(&output.stderr).to_string(), + }) + } + } + } } #[derive(Debug, Clone)] @@ -969,36 +1100,39 @@ impl FakeGitRepositoryState { impl GitRepository for FakeGitRepository { fn reload_index(&self) {} - fn load_index_text(&self, path: &RepoPath) -> Option { + fn load_index_text(&self, path: RepoPath, _: AsyncApp) -> BoxFuture> { let state = self.state.lock(); - state.index_contents.get(path.as_ref()).cloned() + let content = state.index_contents.get(path.as_ref()).cloned(); + async { content }.boxed() } - fn load_committed_text(&self, path: &RepoPath) -> Option { + fn load_committed_text(&self, path: RepoPath, _: AsyncApp) -> BoxFuture> { let state = self.state.lock(); - state.head_contents.get(path.as_ref()).cloned() + let content = state.head_contents.get(path.as_ref()).cloned(); + async { content }.boxed() } fn set_index_text( &self, - path: &RepoPath, + path: RepoPath, content: Option, - _env: &HashMap, - ) -> anyhow::Result<()> { + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { let mut state = self.state.lock(); if let Some(message) = state.simulated_index_write_error_message.clone() { - return Err(anyhow::anyhow!(message)); + return async { Err(anyhow::anyhow!(message)) }.boxed(); } if let Some(content) = content { state.index_contents.insert(path.clone(), content); } else { - state.index_contents.remove(path); + state.index_contents.remove(&path); } state .event_emitter .try_send(state.path.clone()) .expect("Dropped repo change event"); - Ok(()) + async { Ok(()) }.boxed() } fn remote_url(&self, _name: &str) -> Option { @@ -1013,15 +1147,20 @@ impl GitRepository for FakeGitRepository { vec![] } - fn show(&self, _: &str) -> Result { + fn show(&self, _: String, _: AsyncApp) -> BoxFuture> { unimplemented!() } - fn reset(&self, _: &str, _: ResetMode, _: &HashMap) -> Result<()> { + fn reset(&self, _: String, _: ResetMode, _: HashMap) -> BoxFuture> { unimplemented!() } - fn checkout_files(&self, _: &str, _: &[RepoPath], _: &HashMap) -> Result<()> { + fn checkout_files( + &self, + _: String, + _: Vec, + _: HashMap, + ) -> BoxFuture> { unimplemented!() } @@ -1058,10 +1197,10 @@ impl GitRepository for FakeGitRepository { }) } - fn branches(&self) -> Result> { + fn branches(&self) -> BoxFuture>> { let state = self.state.lock(); let current_branch = &state.current_branch_name; - Ok(state + let result = Ok(state .branches .iter() .map(|branch_name| Branch { @@ -1070,98 +1209,119 @@ impl GitRepository for FakeGitRepository { most_recent_commit: None, upstream: None, }) - .collect()) + .collect()); + + async { result }.boxed() } - fn branch_exits(&self, name: &str) -> Result { - let state = self.state.lock(); - Ok(state.branches.contains(name)) - } - - fn change_branch(&self, name: &str) -> Result<()> { + fn change_branch(&self, name: String, _: AsyncApp) -> BoxFuture> { let mut state = self.state.lock(); state.current_branch_name = Some(name.to_owned()); state .event_emitter .try_send(state.path.clone()) .expect("Dropped repo change event"); - Ok(()) + async { Ok(()) }.boxed() } - fn create_branch(&self, name: &str) -> Result<()> { + fn create_branch(&self, name: String, _: AsyncApp) -> BoxFuture> { let mut state = self.state.lock(); state.branches.insert(name.to_owned()); state .event_emitter .try_send(state.path.clone()) .expect("Dropped repo change event"); - Ok(()) + async { Ok(()) }.boxed() } - fn blame(&self, path: &Path, _content: Rope) -> Result { + fn blame( + &self, + path: RepoPath, + _content: Rope, + _cx: AsyncApp, + ) -> BoxFuture> { let state = self.state.lock(); - state + let result = state .blames - .get(path) - .with_context(|| format!("failed to get blame for {:?}", path)) - .cloned() + .get(&path) + .with_context(|| format!("failed to get blame for {:?}", path.0)) + .cloned(); + async { result }.boxed() } - fn stage_paths(&self, _paths: &[RepoPath], _env: &HashMap) -> Result<()> { + fn stage_paths( + &self, + _paths: Vec, + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { unimplemented!() } - fn unstage_paths(&self, _paths: &[RepoPath], _env: &HashMap) -> Result<()> { + fn unstage_paths( + &self, + _paths: Vec, + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { unimplemented!() } fn commit( &self, - _message: &str, - _name_and_email: Option<(&str, &str)>, - _env: &HashMap, - ) -> Result<()> { + _message: SharedString, + _name_and_email: Option<(SharedString, SharedString)>, + _env: HashMap, + _: AsyncApp, + ) -> BoxFuture> { unimplemented!() } fn push( &self, - _branch: &str, - _remote: &str, + _branch: String, + _remote: String, _options: Option, _ask_pass: AskPassSession, - _env: &HashMap, - ) -> Result { + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { unimplemented!() } fn pull( &self, - _branch: &str, - _remote: &str, + _branch: String, + _remote: String, _ask_pass: AskPassSession, - _env: &HashMap, - ) -> Result { + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { unimplemented!() } fn fetch( &self, _ask_pass: AskPassSession, - _env: &HashMap, - ) -> Result { + _env: HashMap, + _cx: AsyncApp, + ) -> BoxFuture> { unimplemented!() } - fn get_remotes(&self, _branch: Option<&str>) -> Result> { + fn get_remotes( + &self, + _branch: Option, + _cx: AsyncApp, + ) -> BoxFuture>> { unimplemented!() } - fn check_for_pushed_commit(&self) -> Result> { + fn check_for_pushed_commit(&self, _cx: AsyncApp) -> BoxFuture>> { unimplemented!() } - fn diff(&self, _diff: DiffType) -> Result { + fn diff(&self, _diff: DiffType, _cx: AsyncApp) -> BoxFuture> { unimplemented!() } } diff --git a/crates/git_ui/src/branch_picker.rs b/crates/git_ui/src/branch_picker.rs index 1fefd6a9ac..07a6ea4db6 100644 --- a/crates/git_ui/src/branch_picker.rs +++ b/crates/git_ui/src/branch_picker.rs @@ -205,9 +205,9 @@ impl BranchListDelegate { return; }; cx.spawn(|_, cx| async move { - cx.update(|cx| repo.read(cx).create_branch(&new_branch_name))? + cx.update(|cx| repo.read(cx).create_branch(new_branch_name.to_string()))? .await??; - cx.update(|cx| repo.read(cx).change_branch(&new_branch_name))? + cx.update(|cx| repo.read(cx).change_branch(new_branch_name.to_string()))? .await??; Ok(()) }) @@ -358,7 +358,7 @@ impl PickerDelegate for BranchListDelegate { let cx = cx.to_async(); anyhow::Ok(async move { - cx.update(|cx| repo.read(cx).change_branch(&branch.name))? + cx.update(|cx| repo.read(cx).change_branch(branch.name.to_string()))? .await? }) })??; diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index 28ce505ab3..afe83b0c02 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -1501,15 +1501,17 @@ impl GitPanel { telemetry::event!("Git Uncommitted"); let confirmation = self.check_for_pushed_commits(window, cx); - let prior_head = self.load_commit_details("HEAD", cx); + let prior_head = self.load_commit_details("HEAD".to_string(), cx); let task = cx.spawn_in(window, |this, mut cx| async move { let result = maybe!(async { if let Ok(true) = confirmation.await { let prior_head = prior_head.await?; - repo.update(&mut cx, |repo, cx| repo.reset("HEAD^", ResetMode::Soft, cx))? - .await??; + repo.update(&mut cx, |repo, cx| { + repo.reset("HEAD^".to_string(), ResetMode::Soft, cx) + })? + .await??; Ok(Some(prior_head)) } else { @@ -3401,7 +3403,7 @@ impl GitPanel { fn load_commit_details( &self, - sha: &str, + sha: String, cx: &mut Context, ) -> Task> { let Some(repo) = self.active_repository.clone() else { @@ -3911,7 +3913,7 @@ impl GitPanelMessageTooltip { cx.spawn_in(window, |this, mut cx| async move { let details = git_panel .update(&mut cx, |git_panel, cx| { - git_panel.load_commit_details(&sha, cx) + git_panel.load_commit_details(sha.to_string(), cx) })? .await?; diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index f4ae0ce05a..8e50012296 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -837,52 +837,63 @@ impl LocalBufferStore { let snapshot = worktree_handle.update(&mut cx, |tree, _| tree.as_local().unwrap().snapshot())?; let diff_bases_changes_by_buffer = cx - .background_spawn(async move { - diff_state_updates - .into_iter() - .filter_map(|(buffer, path, current_index_text, current_head_text)| { - let local_repo = snapshot.local_repo_for_path(&path)?; - let relative_path = local_repo.relativize(&path).ok()?; - let index_text = if current_index_text.is_some() { - local_repo.repo().load_index_text(&relative_path) - } else { - None - }; - let head_text = if current_head_text.is_some() { - local_repo.repo().load_committed_text(&relative_path) - } else { - None - }; + .spawn(async move |cx| { + let mut results = Vec::new(); + for (buffer, path, current_index_text, current_head_text) in diff_state_updates + { + let Some(local_repo) = snapshot.local_repo_for_path(&path) else { + continue; + }; + let Some(relative_path) = local_repo.relativize(&path).ok() else { + continue; + }; + let index_text = if current_index_text.is_some() { + local_repo + .repo() + .load_index_text(relative_path.clone(), cx.clone()) + .await + } else { + None + }; + let head_text = if current_head_text.is_some() { + local_repo + .repo() + .load_committed_text(relative_path, cx.clone()) + .await + } else { + None + }; - // Avoid triggering a diff update if the base text has not changed. - if let Some((current_index, current_head)) = - current_index_text.as_ref().zip(current_head_text.as_ref()) + // Avoid triggering a diff update if the base text has not changed. + if let Some((current_index, current_head)) = + current_index_text.as_ref().zip(current_head_text.as_ref()) + { + if current_index.as_deref() == index_text.as_ref() + && current_head.as_deref() == head_text.as_ref() { - if current_index.as_deref() == index_text.as_ref() - && current_head.as_deref() == head_text.as_ref() - { - return None; - } + continue; } + } - let diff_bases_change = - match (current_index_text.is_some(), current_head_text.is_some()) { - (true, true) => Some(if index_text == head_text { - DiffBasesChange::SetBoth(head_text) - } else { - DiffBasesChange::SetEach { - index: index_text, - head: head_text, - } - }), - (true, false) => Some(DiffBasesChange::SetIndex(index_text)), - (false, true) => Some(DiffBasesChange::SetHead(head_text)), - (false, false) => None, - }; + let diff_bases_change = + match (current_index_text.is_some(), current_head_text.is_some()) { + (true, true) => Some(if index_text == head_text { + DiffBasesChange::SetBoth(head_text) + } else { + DiffBasesChange::SetEach { + index: index_text, + head: head_text, + } + }), + (true, false) => Some(DiffBasesChange::SetIndex(index_text)), + (false, true) => Some(DiffBasesChange::SetHead(head_text)), + (false, false) => None, + }; - Some((buffer, diff_bases_change)) - }) - .collect::>() + results.push((buffer, diff_bases_change)) + } + + results }) .await; @@ -1620,11 +1631,12 @@ impl BufferStore { anyhow::Ok(Some((repo, relative_path, content))) }); - cx.background_spawn(async move { + cx.spawn(|cx| async move { let Some((repo, relative_path, content)) = blame_params? else { return Ok(None); }; - repo.blame(&relative_path, content) + repo.blame(relative_path.clone(), content, cx) + .await .with_context(|| format!("Failed to blame {:?}", relative_path.0)) .map(Some) }) diff --git a/crates/project/src/git.rs b/crates/project/src/git.rs index e56c6eb998..f459e2c5c4 100644 --- a/crates/project/src/git.rs +++ b/crates/project/src/git.rs @@ -401,7 +401,7 @@ impl GitStore { if let Some((repo, path)) = this.repository_and_path_for_buffer_id(buffer_id, cx) { let recv = repo.update(cx, |repo, cx| { repo.set_index_text( - &path, + path, new_index_text.as_ref().map(|rope| rope.to_string()), cx, ) @@ -715,7 +715,7 @@ impl GitStore { repository_handle .update(&mut cx, |repository_handle, cx| { repository_handle.set_index_text( - &RepoPath::from_str(&envelope.payload.path), + RepoPath::from_str(&envelope.payload.path), envelope.payload.text, cx, ) @@ -808,7 +808,7 @@ impl GitStore { repository_handle .update(&mut cx, |repository_handle, _| { - repository_handle.create_branch(&branch_name) + repository_handle.create_branch(branch_name) })? .await??; @@ -828,7 +828,7 @@ impl GitStore { repository_handle .update(&mut cx, |repository_handle, _| { - repository_handle.change_branch(&branch_name) + repository_handle.change_branch(branch_name) })? .await??; @@ -847,7 +847,7 @@ impl GitStore { let commit = repository_handle .update(&mut cx, |repository_handle, _| { - repository_handle.show(&envelope.payload.commit) + repository_handle.show(envelope.payload.commit) })? .await??; Ok(proto::GitCommitDetails { @@ -876,7 +876,7 @@ impl GitStore { repository_handle .update(&mut cx, |repository_handle, cx| { - repository_handle.reset(&envelope.payload.commit, mode, cx) + repository_handle.reset(envelope.payload.commit, mode, cx) })? .await??; Ok(proto::Ack {}) @@ -1081,8 +1081,8 @@ impl Repository { fn send_job(&self, job: F) -> oneshot::Receiver where - F: FnOnce(GitRepo) -> Fut + 'static, - Fut: Future + Send + 'static, + F: FnOnce(GitRepo, AsyncApp) -> Fut + 'static, + Fut: Future + 'static, R: Send + 'static, { self.send_keyed_job(None, job) @@ -1090,8 +1090,8 @@ impl Repository { fn send_keyed_job(&self, key: Option, job: F) -> oneshot::Receiver where - F: FnOnce(GitRepo) -> Fut + 'static, - Fut: Future + Send + 'static, + F: FnOnce(GitRepo, AsyncApp) -> Fut + 'static, + Fut: Future + 'static, R: Send + 'static, { let (result_tx, result_rx) = futures::channel::oneshot::channel(); @@ -1100,8 +1100,8 @@ impl Repository { .unbounded_send(GitJob { key, job: Box::new(|cx: &mut AsyncApp| { - let job = job(git_repo); - cx.background_spawn(async move { + let job = job(git_repo, cx.clone()); + cx.spawn(|_| async move { let result = job.await; result_tx.send(result).ok(); }) @@ -1292,9 +1292,9 @@ impl Repository { let commit = commit.to_string(); let env = self.worktree_environment(cx); - self.send_job(|git_repo| async move { + self.send_job(|git_repo, _| async move { match git_repo { - GitRepo::Local(repo) => repo.checkout_files(&commit, &paths, &env.await), + GitRepo::Local(repo) => repo.checkout_files(commit, paths, env.await).await, GitRepo::Remote { project_id, client, @@ -1322,17 +1322,17 @@ impl Repository { pub fn reset( &self, - commit: &str, + commit: String, reset_mode: ResetMode, cx: &mut App, ) -> oneshot::Receiver> { let commit = commit.to_string(); let env = self.worktree_environment(cx); - self.send_job(|git_repo| async move { + self.send_job(|git_repo, _| async move { match git_repo { GitRepo::Local(git_repo) => { let env = env.await; - git_repo.reset(&commit, reset_mode, &env) + git_repo.reset(commit, reset_mode, env).await } GitRepo::Remote { project_id, @@ -1359,11 +1359,10 @@ impl Repository { }) } - pub fn show(&self, commit: &str) -> oneshot::Receiver> { - let commit = commit.to_string(); - self.send_job(|git_repo| async move { + pub fn show(&self, commit: String) -> oneshot::Receiver> { + self.send_job(|git_repo, cx| async move { match git_repo { - GitRepo::Local(git_repository) => git_repository.show(&commit), + GitRepo::Local(git_repository) => git_repository.show(commit, cx).await, GitRepo::Remote { project_id, client, @@ -1433,9 +1432,9 @@ impl Repository { let env = env.await; this.update(&mut cx, |this, _| { - this.send_job(|git_repo| async move { + this.send_job(|git_repo, cx| async move { match git_repo { - GitRepo::Local(repo) => repo.stage_paths(&entries, &env), + GitRepo::Local(repo) => repo.stage_paths(entries, env, cx).await, GitRepo::Remote { project_id, client, @@ -1504,9 +1503,9 @@ impl Repository { let env = env.await; this.update(&mut cx, |this, _| { - this.send_job(|git_repo| async move { + this.send_job(|git_repo, cx| async move { match git_repo { - GitRepo::Local(repo) => repo.unstage_paths(&entries, &env), + GitRepo::Local(repo) => repo.unstage_paths(entries, env, cx).await, GitRepo::Remote { project_id, client, @@ -1587,17 +1586,11 @@ impl Repository { cx: &mut App, ) -> oneshot::Receiver> { let env = self.worktree_environment(cx); - self.send_job(|git_repo| async move { + self.send_job(|git_repo, cx| async move { match git_repo { GitRepo::Local(repo) => { let env = env.await; - repo.commit( - message.as_ref(), - name_and_email - .as_ref() - .map(|(name, email)| (name.as_ref(), email.as_ref())), - &env, - ) + repo.commit(message, name_and_email, env, cx).await } GitRepo::Remote { project_id, @@ -1634,12 +1627,12 @@ impl Repository { let askpass_id = util::post_inc(&mut self.latest_askpass_id); let env = self.worktree_environment(cx); - self.send_job(move |git_repo| async move { + self.send_job(move |git_repo, cx| async move { match git_repo { GitRepo::Local(git_repository) => { let askpass = AskPassSession::new(&executor, askpass).await?; let env = env.await; - git_repository.fetch(askpass, &env) + git_repository.fetch(askpass, env, cx).await } GitRepo::Remote { project_id, @@ -1685,12 +1678,21 @@ impl Repository { let askpass_id = util::post_inc(&mut self.latest_askpass_id); let env = self.worktree_environment(cx); - self.send_job(move |git_repo| async move { + self.send_job(move |git_repo, cx| async move { match git_repo { GitRepo::Local(git_repository) => { let env = env.await; let askpass = AskPassSession::new(&executor, askpass).await?; - git_repository.push(&branch, &remote, options, askpass, &env) + git_repository + .push( + branch.to_string(), + remote.to_string(), + options, + askpass, + env, + cx, + ) + .await } GitRepo::Remote { project_id, @@ -1740,12 +1742,14 @@ impl Repository { let askpass_id = util::post_inc(&mut self.latest_askpass_id); let env = self.worktree_environment(cx); - self.send_job(move |git_repo| async move { + self.send_job(move |git_repo, cx| async move { match git_repo { GitRepo::Local(git_repository) => { let askpass = AskPassSession::new(&executor, askpass).await?; let env = env.await; - git_repository.pull(&branch, &remote, askpass, &env) + git_repository + .pull(branch.to_string(), remote.to_string(), askpass, env, cx) + .await } GitRepo::Remote { project_id, @@ -1781,18 +1785,17 @@ impl Repository { fn set_index_text( &self, - path: &RepoPath, + path: RepoPath, content: Option, cx: &mut App, ) -> oneshot::Receiver> { - let path = path.clone(); let env = self.worktree_environment(cx); self.send_keyed_job( Some(GitJobKey::WriteIndex(path.clone())), - |git_repo| async move { + |git_repo, cx| async move { match git_repo { - GitRepo::Local(repo) => repo.set_index_text(&path, content, &env.await), + GitRepo::Local(repo) => repo.set_index_text(path, content, env.await, cx).await, GitRepo::Remote { project_id, client, @@ -1819,11 +1822,9 @@ impl Repository { &self, branch_name: Option, ) -> oneshot::Receiver>> { - self.send_job(|repo| async move { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => { - git_repository.get_remotes(branch_name.as_deref()) - } + GitRepo::Local(git_repository) => git_repository.get_remotes(branch_name, cx).await, GitRepo::Remote { project_id, client, @@ -1854,9 +1855,13 @@ impl Repository { } pub fn branches(&self) -> oneshot::Receiver>> { - self.send_job(|repo| async move { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => git_repository.branches(), + GitRepo::Local(git_repository) => { + let git_repository = git_repository.clone(); + cx.background_spawn(async move { git_repository.branches().await }) + .await + } GitRepo::Remote { project_id, client, @@ -1884,9 +1889,9 @@ impl Repository { } pub fn diff(&self, diff_type: DiffType, _cx: &App) -> oneshot::Receiver> { - self.send_job(|repo| async move { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => git_repository.diff(diff_type), + GitRepo::Local(git_repository) => git_repository.diff(diff_type, cx).await, GitRepo::Remote { project_id, client, @@ -1916,11 +1921,12 @@ impl Repository { }) } - pub fn create_branch(&self, branch_name: &str) -> oneshot::Receiver> { - let branch_name = branch_name.to_owned(); - self.send_job(|repo| async move { + pub fn create_branch(&self, branch_name: String) -> oneshot::Receiver> { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => git_repository.create_branch(&branch_name), + GitRepo::Local(git_repository) => { + git_repository.create_branch(branch_name, cx).await + } GitRepo::Remote { project_id, client, @@ -1942,11 +1948,12 @@ impl Repository { }) } - pub fn change_branch(&self, branch_name: &str) -> oneshot::Receiver> { - let branch_name = branch_name.to_owned(); - self.send_job(|repo| async move { + pub fn change_branch(&self, branch_name: String) -> oneshot::Receiver> { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => git_repository.change_branch(&branch_name), + GitRepo::Local(git_repository) => { + git_repository.change_branch(branch_name, cx).await + } GitRepo::Remote { project_id, client, @@ -1969,9 +1976,9 @@ impl Repository { } pub fn check_for_pushed_commits(&self) -> oneshot::Receiver>> { - self.send_job(|repo| async move { + self.send_job(|repo, cx| async move { match repo { - GitRepo::Local(git_repository) => git_repository.check_for_pushed_commit(), + GitRepo::Local(git_repository) => git_repository.check_for_pushed_commit(cx).await, GitRepo::Remote { project_id, client, diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 0c2b700bce..d0e1722e6a 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -1361,7 +1361,7 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA assert_eq!(&remote_branches, &branches_set); - cx.update(|cx| repository.read(cx).change_branch(new_branch)) + cx.update(|cx| repository.read(cx).change_branch(new_branch.to_string())) .await .unwrap() .unwrap(); @@ -1383,15 +1383,23 @@ async fn test_remote_git_branches(cx: &mut TestAppContext, server_cx: &mut TestA assert_eq!(server_branch.name, branches[2]); // Also try creating a new branch - cx.update(|cx| repository.read(cx).create_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx.update(|cx| { + repository + .read(cx) + .create_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); - cx.update(|cx| repository.read(cx).change_branch("totally-new-branch")) - .await - .unwrap() - .unwrap(); + cx.update(|cx| { + repository + .read(cx) + .change_branch("totally-new-branch".to_string()) + }) + .await + .unwrap() + .unwrap(); cx.run_until_parked(); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index a4d36e8c90..dd7381eff1 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -1055,13 +1055,13 @@ impl Worktree { Worktree::Local(this) => { let path = Arc::from(path); let snapshot = this.snapshot(); - cx.background_spawn(async move { + cx.spawn(|cx| async move { if let Some(repo) = snapshot.repository_for_path(&path) { if let Some(repo_path) = repo.relativize(&path).log_err() { if let Some(git_repo) = snapshot.git_repositories.get(&repo.work_directory_id) { - return Ok(git_repo.repo_ptr.load_index_text(&repo_path)); + return Ok(git_repo.repo_ptr.load_index_text(repo_path, cx).await); } } } @@ -1079,13 +1079,16 @@ impl Worktree { Worktree::Local(this) => { let path = Arc::from(path); let snapshot = this.snapshot(); - cx.background_spawn(async move { + cx.spawn(|cx| async move { if let Some(repo) = snapshot.repository_for_path(&path) { if let Some(repo_path) = repo.relativize(&path).log_err() { if let Some(git_repo) = snapshot.git_repositories.get(&repo.work_directory_id) { - return Ok(git_repo.repo_ptr.load_committed_text(&repo_path)); + return Ok(git_repo + .repo_ptr + .load_committed_text(repo_path, cx) + .await); } } } @@ -5520,7 +5523,9 @@ impl BackgroundScanner { state.repository_scans.insert( path_key.clone(), self.executor.spawn(async move { - update_branches(&job_state, &mut local_repository).log_err(); + update_branches(&job_state, &mut local_repository) + .await + .log_err(); log::trace!("updating git statuses for repo {repository_name}",); let t0 = Instant::now(); @@ -5665,11 +5670,11 @@ fn send_status_update_inner( .is_ok() } -fn update_branches( +async fn update_branches( state: &Mutex, repository: &mut LocalRepositoryEntry, ) -> Result<()> { - let branches = repository.repo().branches()?; + let branches = repository.repo().branches().await?; let snapshot = state.lock().snapshot.snapshot.clone(); let mut repository = snapshot .repository(repository.work_directory.path_key())