git_ui: Fix cached entries in git panel potentially getting stale

crates/project/src/git_store.rs

@@ -5378,7 +5378,7 @@ impl Repository {
     where
         AsyncFn: AsyncFnOnce(WeakEntity<Repository>, &mut AsyncApp) -> Result<()> + 'static,
     {
-        let ids = self.new_pending_ops_for_paths(paths, git_status);
+        let ids = self.new_pending_ops_for_paths(&paths, git_status);
 
         cx.spawn(async move |this, cx| {
             let (job_status, result) = match f(this.clone(), cx).await {
@@ -5387,17 +5387,57 @@ impl Repository {
                 Err(err) => (pending_op::JobStatus::Error, Err(err)),
             };
 
-            this.update(cx, |this, _| {
-                let mut edits = Vec::with_capacity(ids.len());
+            // If the job finished successfully, we will go ahead and try immediately pulling latest git status
+            let statuses = if let pending_op::JobStatus::Finished = job_status {
+                this.update(cx, |this, _| {
+                    this.send_job(None, move |git_repo, _cx| async move {
+                        match git_repo {
+                            RepositoryState::Local { backend, .. } => {
+                                backend.status(&paths).await.ok()
+                            }
+                            // TODO decide whether we should also ping the remote for new status update.
+                            // At the same time, currently we do not clear pending ops for remote repos at all since
+                            // `compute_snapshot` runs only for local repos.
+                            RepositoryState::Remote { .. } => None,
+                        }
+                    })
+                })?
+                .await?
+            } else {
+                None
+            };
+
+            this.update(cx, |this, cx| {
+                let mut pending_ops_edits = Vec::with_capacity(ids.len());
+                let mut status_edits = Vec::with_capacity(ids.len());
                 for (id, entry) in ids {
                     if let Some(mut ops) = this.snapshot.pending_ops_for_path(&entry) {
                         if let Some(op) = ops.op_by_id_mut(id) {
                             op.job_status = job_status;
                         }
-                        edits.push(sum_tree::Edit::Insert(ops));
+                        pending_ops_edits.push(sum_tree::Edit::Insert(ops));
+                    }
+                    if let Some(statuses) = statuses.as_ref() {
+                        if let Some(status) =
+                            statuses.entries.iter().find(|(path, _)| path == &entry)
+                        {
+                            status_edits.push(sum_tree::Edit::Insert(StatusEntry {
+                                repo_path: entry,
+                                status: status.1,
+                            }));
+                        }
                     }
                 }
-                this.snapshot.pending_ops_by_path.edit(edits, ());
+                this.snapshot
+                    .pending_ops_by_path
+                    .edit(pending_ops_edits, ());
+
+                let emit_update = status_edits.len() > 0;
+                this.snapshot.statuses_by_path.edit(status_edits, ());
+
+                if emit_update {
+                    cx.emit(RepositoryEvent::StatusesChanged);
+                }
             })?;
 
             result
@@ -5406,7 +5446,7 @@ impl Repository {
 
     fn new_pending_ops_for_paths(
         &mut self,
-        paths: Vec<RepoPath>,
+        paths: &[RepoPath],
         git_status: pending_op::GitStatus,
     ) -> Vec<(PendingOpId, RepoPath)> {
         let mut edits = Vec::with_capacity(paths.len());
@@ -5415,7 +5455,7 @@ impl Repository {
             let mut ops = self
                 .snapshot
                 .pending_ops_for_path(&path)
-                .unwrap_or_else(|| PendingOps::new(&path));
+                .unwrap_or_else(|| PendingOps::new(path));
             let id = ops.max_id() + 1;
             ops.ops.push(PendingOp {
                 id,
@@ -5423,7 +5463,7 @@ impl Repository {
                 job_status: pending_op::JobStatus::Running,
             });
             edits.push(sum_tree::Edit::Insert(ops));
-            ids.push((id, path));
+            ids.push((id, path.clone()));
         }
         self.snapshot.pending_ops_by_path.edit(edits, ());
         ids

crates/project/src/project_tests.rs

@@ -8516,12 +8516,13 @@ async fn test_git_status_postprocessing(cx: &mut gpui::TestAppContext) {
 }
 
 #[track_caller]
-/// We merge lhs into rhs.
+/// We merge lhs into rhs discarding any PendingOp that is running.
 fn merge_pending_ops_snapshots(
     source: Vec<pending_op::PendingOps>,
     mut target: Vec<pending_op::PendingOps>,
 ) -> Vec<pending_op::PendingOps> {
-    for s_ops in source {
+    for mut s_ops in source {
+        s_ops.ops.retain(|op| !op.running());
         if let Some(idx) = target.iter().zip(0..).find_map(|(ops, idx)| {
             if ops.repo_path == s_ops.repo_path {
                 Some(idx)
@@ -8529,26 +8530,7 @@ fn merge_pending_ops_snapshots(
                 None
             }
         }) {
-            let t_ops = &mut target[idx];
-            for s_op in s_ops.ops {
-                if let Some(op_idx) = t_ops
-                    .ops
-                    .iter()
-                    .zip(0..)
-                    .find_map(|(op, idx)| if op.id == s_op.id { Some(idx) } else { None })
-                {
-                    let t_op = &mut t_ops.ops[op_idx];
-                    match (s_op.job_status, t_op.job_status) {
-                        (pending_op::JobStatus::Running, _) => {}
-                        (s_st, pending_op::JobStatus::Running) => t_op.job_status = s_st,
-                        (s_st, t_st) if s_st == t_st => {}
-                        _ => unreachable!(),
-                    }
-                } else {
-                    t_ops.ops.push(s_op);
-                }
-            }
-            t_ops.ops.sort_by(|l, r| l.id.cmp(&r.id));
+            target[idx].ops.append(&mut s_ops.ops);
         } else {
             target.push(s_ops);
         }
@@ -8614,8 +8596,6 @@ async fn test_repository_pending_ops_staging(
         assert!(repo.pending_ops_by_path.is_empty());
     });
 
-    let mut id = 1u16;
-
     let mut assert_stage = async |path: RepoPath, stage| {
         let git_status = if stage {
             pending_op::GitStatus::Staged
@@ -8629,14 +8609,9 @@ async fn test_repository_pending_ops_staging(
                 repo.unstage_entries(vec![path.clone()], cx)
             };
             let ops = repo.pending_ops_for_path(&path).unwrap();
-            assert_eq!(
-                ops.ops.last(),
-                Some(&pending_op::PendingOp {
-                    id: id.into(),
-                    git_status,
-                    job_status: pending_op::JobStatus::Running
-                })
-            );
+            let last = ops.ops.last().unwrap();
+            assert_eq!(last.git_status, git_status);
+            assert_eq!(last.job_status, pending_op::JobStatus::Running);
             task
         })
         .await
@@ -8644,17 +8619,10 @@ async fn test_repository_pending_ops_staging(
 
         repo.read_with(cx, |repo, _cx| {
             let ops = repo.pending_ops_for_path(&path).unwrap();
-            assert_eq!(
-                ops.ops.last(),
-                Some(&pending_op::PendingOp {
-                    id: id.into(),
-                    git_status,
-                    job_status: pending_op::JobStatus::Finished
-                })
-            );
+            let last = ops.ops.last().unwrap();
+            assert_eq!(last.git_status, git_status);
+            assert_eq!(last.job_status, pending_op::JobStatus::Finished);
         });
-
-        id += 1;
     };
 
     assert_stage(repo_path("a.txt"), true).await;
@@ -8670,33 +8638,37 @@ async fn test_repository_pending_ops_staging(
             .lock()
             .get(&worktree::PathKey(repo_path("a.txt").as_ref().clone()), ())
             .unwrap()
-            .ops,
+            .ops
+            .iter()
+            .map(
+                |&pending_op::PendingOp {
+                     git_status,
+                     job_status,
+                     ..
+                 }| (git_status, job_status)
+            )
+            .collect::<Vec<_>>(),
         vec![
-            pending_op::PendingOp {
-                id: 1u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 2u16.into(),
-                git_status: pending_op::GitStatus::Unstaged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 3u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 4u16.into(),
-                git_status: pending_op::GitStatus::Unstaged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 5u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            }
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Unstaged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Unstaged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            )
         ],
     );
 
@@ -8791,18 +8763,25 @@ async fn test_repository_pending_ops_long_running_staging(
             .lock()
             .get(&worktree::PathKey(repo_path("a.txt").as_ref().clone()), ())
             .unwrap()
-            .ops,
+            .ops
+            .iter()
+            .map(
+                |&pending_op::PendingOp {
+                     git_status,
+                     job_status,
+                     ..
+                 }| (git_status, job_status)
+            )
+            .collect::<Vec<_>>(),
         vec![
-            pending_op::PendingOp {
-                id: 1u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Skipped
-            },
-            pending_op::PendingOp {
-                id: 2u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            }
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Skipped
+            ),
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            )
         ],
     );
 
@@ -8899,37 +8878,52 @@ async fn test_repository_pending_ops_stage_all(
             .lock()
             .get(&worktree::PathKey(repo_path("a.txt").as_ref().clone()), ())
             .unwrap()
-            .ops,
+            .ops
+            .iter()
+            .map(
+                |&pending_op::PendingOp {
+                     git_status,
+                     job_status,
+                     ..
+                 }| (git_status, job_status)
+            )
+            .collect::<Vec<_>>(),
         vec![
-            pending_op::PendingOp {
-                id: 1u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 2u16.into(),
-                git_status: pending_op::GitStatus::Unstaged,
-                job_status: pending_op::JobStatus::Finished
-            },
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Unstaged,
+                pending_op::JobStatus::Finished
+            ),
         ],
     );
+
     assert_eq!(
         pending_ops_all
             .lock()
             .get(&worktree::PathKey(repo_path("b.txt").as_ref().clone()), ())
             .unwrap()
-            .ops,
+            .ops
+            .iter()
+            .map(
+                |&pending_op::PendingOp {
+                     git_status,
+                     job_status,
+                     ..
+                 }| (git_status, job_status)
+            )
+            .collect::<Vec<_>>(),
         vec![
-            pending_op::PendingOp {
-                id: 1u16.into(),
-                git_status: pending_op::GitStatus::Staged,
-                job_status: pending_op::JobStatus::Finished
-            },
-            pending_op::PendingOp {
-                id: 2u16.into(),
-                git_status: pending_op::GitStatus::Unstaged,
-                job_status: pending_op::JobStatus::Finished
-            },
+            (
+                pending_op::GitStatus::Staged,
+                pending_op::JobStatus::Finished
+            ),
+            (
+                pending_op::GitStatus::Unstaged,
+                pending_op::JobStatus::Finished
+            ),
         ],
     );