git_graph: Add remote support for commit data handler

crates/collab/src/rpc.rs

@@ -436,6 +436,7 @@ impl Server {
             .add_request_handler(forward_mutating_project_request::<proto::GitRemoveRemote>)
             .add_request_handler(forward_read_only_project_request::<proto::GitGetWorktrees>)
             .add_request_handler(forward_read_only_project_request::<proto::GitGetHeadSha>)
+            .add_request_handler(forward_read_only_project_request::<proto::GetCommitData>)
             .add_request_handler(forward_mutating_project_request::<proto::GitCreateWorktree>)
             .add_request_handler(disallow_guest_request::<proto::GitRemoveWorktree>)
             .add_request_handler(disallow_guest_request::<proto::GitRenameWorktree>)

crates/collab/tests/integration/git_tests.rs

@@ -4,12 +4,16 @@ use call::ActiveCall;
 use client::RECEIVE_TIMEOUT;
 use collections::HashMap;
 use git::{
-    repository::{RepoPath, Worktree as GitWorktree},
+    Oid,
+    repository::{CommitData, RepoPath, Worktree as GitWorktree},
     status::{DiffStat, FileStatus, StatusCode, TrackedStatus},
 };
 use git_ui::{git_panel::GitPanel, project_diff::ProjectDiff};
-use gpui::{AppContext as _, BackgroundExecutor, TestAppContext, VisualTestContext};
-use project::ProjectPath;
+use gpui::{AppContext as _, BackgroundExecutor, SharedString, TestAppContext, VisualTestContext};
+use project::{
+    ProjectPath,
+    git_store::{CommitDataState, Repository},
+};
 use serde_json::json;
 
 use util::{path, rel_path::rel_path};
@@ -91,6 +95,90 @@ fn collect_diff_stats<C: gpui::AppContext>(
     })
 }
 
+async fn load_commit_data_batch(
+    repository: &gpui::Entity<Repository>,
+    shas: &[Oid],
+    executor: &BackgroundExecutor,
+    cx: &mut TestAppContext,
+) -> HashMap<Oid, CommitData> {
+    let states = cx.update(|cx| {
+        shas.iter()
+            .map(|sha| {
+                (
+                    *sha,
+                    repository.update(cx, |repository, cx| {
+                        repository.fetch_commit_data(*sha, true, cx).clone()
+                    }),
+                )
+            })
+            .collect::<Vec<_>>()
+    });
+
+    executor.run_until_parked();
+
+    let mut commit_data = HashMap::default();
+    for (sha, state) in states {
+        let data = match state {
+            CommitDataState::Loaded(data) => data.as_ref().clone(),
+            CommitDataState::Loading(Some(shared)) => shared.await.unwrap().as_ref().clone(),
+            CommitDataState::Loading(None) => {
+                panic!("fetch_commit_data(..., true) should return an await-result state")
+            }
+        };
+        commit_data.insert(sha, data);
+    }
+
+    commit_data
+}
+
+fn assert_remote_cache_matches_local_cache(
+    local_repository: &gpui::Entity<Repository>,
+    remote_repository: &gpui::Entity<Repository>,
+    cx_local: &mut TestAppContext,
+    cx_remote: &mut TestAppContext,
+) {
+    let local_cache = cx_local.update(|cx| {
+        local_repository.update(cx, |repository, _| repository.loaded_commit_data_for_test())
+    });
+    let remote_cache = cx_remote.update(|cx| {
+        remote_repository.update(cx, |repository, _| repository.loaded_commit_data_for_test())
+    });
+
+    for (sha, remote_commit_data) in &remote_cache {
+        let local_commit_data = local_cache
+            .get(sha)
+            .unwrap_or_else(|| panic!("local cache missing commit data for {sha}"));
+        assert_eq!(
+            local_commit_data.sha, remote_commit_data.sha,
+            "local and remote cache should agree on sha for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.parents, remote_commit_data.parents,
+            "local and remote cache should agree on parents for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.author_name, remote_commit_data.author_name,
+            "local and remote cache should agree on author_name for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.author_email, remote_commit_data.author_email,
+            "local and remote cache should agree on author_email for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.commit_timestamp, remote_commit_data.commit_timestamp,
+            "local and remote cache should agree on commit_timestamp for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.subject, remote_commit_data.subject,
+            "local and remote cache should agree on subject for {sha}"
+        );
+        assert_eq!(
+            local_commit_data.message, remote_commit_data.message,
+            "local and remote cache should agree on message for {sha}"
+        );
+    }
+}
+
 #[gpui::test]
 async fn test_project_diff(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
     let mut server = TestServer::start(cx_a.background_executor.clone()).await;
@@ -480,6 +568,110 @@ async fn test_remote_git_head_sha(
     assert_eq!(remote_head_sha.unwrap(), local_head_sha);
 }
 
+#[gpui::test]
+async fn test_remote_git_commit_data_batches(
+    executor: BackgroundExecutor,
+    cx_a: &mut TestAppContext,
+    cx_b: &mut TestAppContext,
+) {
+    let mut server = TestServer::start(executor.clone()).await;
+    let client_a = server.create_client(cx_a, "user_a").await;
+    let client_b = server.create_client(cx_b, "user_b").await;
+    server
+        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+    let active_call_a = cx_a.read(ActiveCall::global);
+
+    client_a
+        .fs()
+        .insert_tree(
+            path!("/project"),
+            json!({ ".git": {}, "file.txt": "content" }),
+        )
+        .await;
+
+    let commit_shas = [
+        "0123456789abcdef0123456789abcdef01234567"
+            .parse::<Oid>()
+            .unwrap(),
+        "1111111111111111111111111111111111111111"
+            .parse::<Oid>()
+            .unwrap(),
+        "2222222222222222222222222222222222222222"
+            .parse::<Oid>()
+            .unwrap(),
+        "3333333333333333333333333333333333333333"
+            .parse::<Oid>()
+            .unwrap(),
+    ];
+
+    client_a.fs().set_commit_data(
+        Path::new(path!("/project/.git")),
+        commit_shas.iter().enumerate().map(|(index, sha)| {
+            (
+                CommitData {
+                    sha: *sha,
+                    parents: Default::default(),
+                    author_name: SharedString::from(format!("Author {index}")),
+                    author_email: SharedString::from(format!("author{index}@example.com")),
+                    commit_timestamp: 1_700_000_000 + index as i64,
+                    subject: SharedString::from(format!("Subject {index}")),
+                    message: SharedString::from(format!("Subject {index}\n\nBody {index}")),
+                },
+                false,
+            )
+        }),
+    );
+
+    let (project_a, _) = client_a.build_local_project(path!("/project"), cx_a).await;
+    executor.run_until_parked();
+
+    let repo_a = cx_a.update(|cx| project_a.read(cx).active_repository(cx).unwrap());
+
+    let primed_before = load_commit_data_batch(&repo_a, &commit_shas[..2], &executor, cx_a).await;
+    assert_eq!(
+        primed_before.len(),
+        2,
+        "host should prime two commits before sharing"
+    );
+
+    let project_id = active_call_a
+        .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
+        .await
+        .unwrap();
+    let project_b = client_b.join_remote_project(project_id, cx_b).await;
+
+    executor.run_until_parked();
+
+    let repo_b = cx_b.update(|cx| project_b.read(cx).active_repository(cx).unwrap());
+
+    let remote_batch_one =
+        load_commit_data_batch(&repo_b, &commit_shas[..3], &executor, cx_b).await;
+    assert_eq!(remote_batch_one.len(), 3);
+    for (index, sha) in commit_shas[..3].iter().enumerate() {
+        let commit_data = remote_batch_one.get(sha).unwrap();
+        assert_eq!(commit_data.sha, *sha);
+        assert_eq!(commit_data.subject.as_ref(), format!("Subject {index}"));
+        assert_eq!(
+            commit_data.message.as_ref(),
+            format!("Subject {index}\n\nBody {index}")
+        );
+    }
+
+    let primed_after = load_commit_data_batch(&repo_a, &commit_shas[2..], &executor, cx_a).await;
+    assert_eq!(
+        primed_after.len(),
+        2,
+        "host should prime remaining commits after remote fetches"
+    );
+
+    let remote_batch_two =
+        load_commit_data_batch(&repo_b, &commit_shas[1..], &executor, cx_b).await;
+    assert_eq!(remote_batch_two.len(), 3);
+
+    assert_remote_cache_matches_local_cache(&repo_a, &repo_b, cx_a, cx_b);
+}
+
 #[gpui::test]
 async fn test_linked_worktrees_sync(
     executor: BackgroundExecutor,

crates/fs/src/fake_git_repo.rs

@@ -9,7 +9,7 @@ use git::{
     Oid, RunHook,
     blame::Blame,
     repository::{
-        AskPassDelegate, Branch, CommitDataReader, CommitDetails, CommitOptions,
+        AskPassDelegate, Branch, CommitData, CommitDataReader, CommitDetails, CommitOptions,
         CreateWorktreeTarget, FetchOptions, GRAPH_CHUNK_SIZE, GitRepository,
         GitRepositoryCheckpoint, InitialGraphCommitData, LogOrder, LogSource, PushOptions, RefEdit,
         Remote, RepoPath, ResetMode, SearchCommitArgs, Worktree,
@@ -47,6 +47,12 @@ pub struct FakeCommitSnapshot {
     pub sha: String,
 }
 
+#[derive(Debug, Clone)]
+pub enum FakeCommitDataEntry {
+    Success(CommitData),
+    Fail(CommitData),
+}
+
 #[derive(Debug, Clone)]
 pub struct FakeGitRepositoryState {
     pub commit_history: Vec<FakeCommitSnapshot>,
@@ -67,6 +73,7 @@ pub struct FakeGitRepositoryState {
     pub simulated_graph_error: Option<String>,
     pub refs: HashMap<String, String>,
     pub graph_commits: Vec<Arc<InitialGraphCommitData>>,
+    pub commit_data: HashMap<Oid, FakeCommitDataEntry>,
     pub stash_entries: GitStash,
 }
 
@@ -88,6 +95,7 @@ impl FakeGitRepositoryState {
             oids: Default::default(),
             remotes: HashMap::default(),
             graph_commits: Vec::new(),
+            commit_data: Default::default(),
             commit_history: Vec::new(),
             stash_entries: Default::default(),
         }
@@ -1452,7 +1460,24 @@ impl GitRepository for FakeGitRepository {
     }
 
     fn commit_data_reader(&self) -> Result<CommitDataReader> {
-        anyhow::bail!("commit_data_reader not supported for FakeGitRepository")
+        let fs = self.fs.clone();
+        let dot_git_path = self.dot_git_path.clone();
+        let executor = self.executor.clone();
+        Ok(CommitDataReader::for_test(executor, move |sha| {
+            fs.with_git_state(&dot_git_path, false, |state| {
+                let commit = state
+                    .commit_data
+                    .get(&sha)
+                    .context(format!("graph commit data not found for {sha}"))?;
+
+                match commit {
+                    FakeCommitDataEntry::Success(data) => Ok(data.clone()),
+                    FakeCommitDataEntry::Fail(_) => {
+                        bail!("simulated commit data read failure for {sha}")
+                    }
+                }
+            })?
+        }))
     }
 
     fn update_ref(&self, ref_name: String, commit: String) -> BoxFuture<'_, Result<()>> {

crates/fs/src/fs.rs

@@ -53,10 +53,10 @@ mod fake_git_repo;
 #[cfg(feature = "test-support")]
 use collections::{BTreeMap, btree_map};
 #[cfg(feature = "test-support")]
-use fake_git_repo::FakeGitRepositoryState;
+use fake_git_repo::{FakeCommitDataEntry, FakeGitRepositoryState};
 #[cfg(feature = "test-support")]
 use git::{
-    repository::{InitialGraphCommitData, RepoPath, Worktree, repo_path},
+    repository::{CommitData, InitialGraphCommitData, RepoPath, Worktree, repo_path},
     status::{FileStatus, StatusCode, TrackedStatus, UnmergedStatus},
 };
 #[cfg(feature = "test-support")]
@@ -2212,6 +2212,29 @@ impl FakeFs {
         .unwrap();
     }
 
+    pub fn set_commit_data(
+        &self,
+        dot_git: &Path,
+        commit_data: impl IntoIterator<Item = (CommitData, bool)>,
+    ) {
+        self.with_git_state(dot_git, true, |state| {
+            state.commit_data = commit_data
+                .into_iter()
+                .map(|(data, should_fail)| {
+                    (
+                        data.sha,
+                        if should_fail {
+                            FakeCommitDataEntry::Fail(data)
+                        } else {
+                            FakeCommitDataEntry::Success(data)
+                        },
+                    )
+                })
+                .collect();
+        })
+        .unwrap();
+    }
+
     /// Put the given git repository into a state with the given status,
     /// by mutating the head, index, and unmerged state.
     pub fn set_status_for_repo(&self, dot_git: &Path, statuses: &[(&str, FileStatus)]) {