Detailed changes
@@ -1024,7 +1024,7 @@ impl GitGraph {
}
}
}
- RepositoryEvent::BranchChanged | RepositoryEvent::MergeHeadsChanged => {
+ RepositoryEvent::BranchChanged => {
self.pending_select_sha = None;
// Only invalidate if we scanned atleast once,
// meaning we are not inside the initial repo loading state
@@ -3174,12 +3174,6 @@ mod tests {
.any(|event| matches!(event, RepositoryEvent::BranchChanged)),
"initial repository scan should emit BranchChanged"
);
- assert!(
- observed_repository_events
- .iter()
- .any(|event| matches!(event, RepositoryEvent::MergeHeadsChanged)),
- "initial repository scan should emit MergeHeadsChanged"
- );
let commit_count_after = repository.read_with(cx, |repo, _| {
repo.get_graph_data(crate::LogSource::default(), crate::LogOrder::default())
.map(|data| data.commit_data.len())
@@ -769,9 +769,7 @@ impl GitPanel {
move |this, _git_store, event, window, cx| match event {
GitStoreEvent::RepositoryUpdated(
_,
- RepositoryEvent::StatusesChanged
- | RepositoryEvent::BranchChanged
- | RepositoryEvent::MergeHeadsChanged,
+ RepositoryEvent::StatusesChanged | RepositoryEvent::BranchChanged,
true,
)
| GitStoreEvent::RepositoryAdded
@@ -72,7 +72,7 @@ use std::{
},
time::Instant,
};
-use sum_tree::{Edit, SumTree, TreeSet};
+use sum_tree::{Edit, SumTree, TreeMap};
use task::Shell;
use text::{Bias, BufferId};
use util::{
@@ -251,9 +251,8 @@ pub struct RepositoryId(pub u64);
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct MergeDetails {
- pub conflicted_paths: TreeSet<RepoPath>,
+ pub merge_heads_by_conflicted_path: TreeMap<RepoPath, Vec<Option<SharedString>>>,
pub message: Option<SharedString>,
- pub heads: Vec<Option<SharedString>>,
}
#[derive(Clone)]
@@ -407,7 +406,6 @@ pub enum GitGraphEvent {
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum RepositoryEvent {
StatusesChanged,
- MergeHeadsChanged,
BranchChanged,
StashEntriesChanged,
PendingOpsChanged { pending_ops: SumTree<PendingOps> },
@@ -3511,9 +3509,9 @@ impl RepositorySnapshot {
removed_statuses: Default::default(),
current_merge_conflicts: self
.merge
- .conflicted_paths
+ .merge_heads_by_conflicted_path
.iter()
- .map(|repo_path| repo_path.to_proto())
+ .map(|(repo_path, _)| repo_path.to_proto())
.collect(),
merge_message: self.merge.message.as_ref().map(|msg| msg.to_string()),
project_id,
@@ -3582,9 +3580,9 @@ impl RepositorySnapshot {
removed_statuses,
current_merge_conflicts: self
.merge
- .conflicted_paths
+ .merge_heads_by_conflicted_path
.iter()
- .map(|path| path.to_proto())
+ .map(|(path, _)| path.to_proto())
.collect(),
merge_message: self.merge.message.as_ref().map(|msg| msg.to_string()),
project_id,
@@ -3640,12 +3638,16 @@ impl RepositorySnapshot {
}
pub fn had_conflict_on_last_merge_head_change(&self, repo_path: &RepoPath) -> bool {
- self.merge.conflicted_paths.contains(repo_path)
+ self.merge
+ .merge_heads_by_conflicted_path
+ .contains_key(repo_path)
}
pub fn has_conflict(&self, repo_path: &RepoPath) -> bool {
- let had_conflict_on_last_merge_head_change =
- self.merge.conflicted_paths.contains(repo_path);
+ let had_conflict_on_last_merge_head_change = self
+ .merge
+ .merge_heads_by_conflicted_path
+ .contains_key(repo_path);
let has_conflict_currently = self
.status_for_path(repo_path)
.is_some_and(|entry| entry.status.is_conflicted());
@@ -3684,13 +3686,13 @@ pub fn proto_to_stash(entry: &proto::StashEntry) -> Result<StashEntry> {
}
impl MergeDetails {
- async fn load(
+ async fn update(
+ &mut self,
backend: &Arc<dyn GitRepository>,
- status: &SumTree<StatusEntry>,
- prev_snapshot: &RepositorySnapshot,
- ) -> Result<(MergeDetails, bool)> {
+ current_conflicted_paths: Vec<RepoPath>,
+ ) -> Result<bool> {
log::debug!("load merge details");
- let message = backend.merge_message().await;
+ self.message = backend.merge_message().await.map(SharedString::from);
let heads = backend
.revparse_batch(vec![
"MERGE_HEAD".into(),
@@ -3705,44 +3707,31 @@ impl MergeDetails {
.into_iter()
.map(|opt| opt.map(SharedString::from))
.collect::<Vec<_>>();
- let merge_heads_changed = heads != prev_snapshot.merge.heads;
- let conflicted_paths = if merge_heads_changed {
- let current_conflicted_paths = TreeSet::from_ordered_entries(
- status
- .iter()
- .filter(|entry| entry.status.is_conflicted())
- .map(|entry| entry.repo_path.clone()),
- );
- // It can happen that we run a scan while a lengthy merge is in progress
- // that will eventually result in conflicts, but before those conflicts
- // are reported by `git status`. Since for the moment we only care about
- // the merge heads state for the purposes of tracking conflicts, don't update
- // this state until we see some conflicts.
- if heads.iter().any(Option::is_some)
- && !prev_snapshot.merge.heads.iter().any(Option::is_some)
- && current_conflicted_paths.is_empty()
- {
- log::debug!("not updating merge heads because no conflicts found");
- return Ok((
- MergeDetails {
- message: message.map(SharedString::from),
- ..prev_snapshot.merge.clone()
- },
- false,
- ));
+ let mut conflicts_changed = false;
+
+ // Record the merge state for newly conflicted paths
+ for path in ¤t_conflicted_paths {
+ if self.merge_heads_by_conflicted_path.get(&path).is_none() {
+ conflicts_changed = true;
+ self.merge_heads_by_conflicted_path
+ .insert(path.clone(), heads.clone());
}
+ }
- current_conflicted_paths
- } else {
- prev_snapshot.merge.conflicted_paths.clone()
- };
- let details = MergeDetails {
- conflicted_paths,
- message: message.map(SharedString::from),
- heads,
- };
- Ok((details, merge_heads_changed))
+ // Clear state for paths that are no longer conflicted and for which the merge heads have changed
+ self.merge_heads_by_conflicted_path
+ .retain(|path, old_merge_heads| {
+ let keep = current_conflicted_paths.contains(path)
+ || (old_merge_heads == &heads
+ && old_merge_heads.iter().any(|head| head.is_some()));
+ if !keep {
+ conflicts_changed = true;
+ }
+ keep
+ });
+
+ Ok(conflicts_changed)
}
}
@@ -3798,7 +3787,7 @@ impl Repository {
.shared();
cx.subscribe_self(move |this, event: &RepositoryEvent, _| match event {
- RepositoryEvent::BranchChanged | RepositoryEvent::MergeHeadsChanged => {
+ RepositoryEvent::BranchChanged => {
if this.scan_id > 1 {
this.initial_graph_data.clear();
}
@@ -6004,12 +5993,6 @@ impl Repository {
update: proto::UpdateRepository,
cx: &mut Context<Self>,
) -> Result<()> {
- let conflicted_paths = TreeSet::from_ordered_entries(
- update
- .current_merge_conflicts
- .into_iter()
- .filter_map(|path| RepoPath::from_proto(&path).log_err()),
- );
let new_branch = update.branch_summary.as_ref().map(proto_to_branch);
let new_head_commit = update
.head_commit_details
@@ -6021,7 +6004,17 @@ impl Repository {
self.snapshot.branch = new_branch;
self.snapshot.head_commit = new_head_commit;
- self.snapshot.merge.conflicted_paths = conflicted_paths;
+ // We don't store any merge head state for downstream projects; the upstream
+ // will track it and we will just get the updated conflicts
+ let new_merge_heads = TreeMap::from_ordered_entries(
+ update
+ .current_merge_conflicts
+ .into_iter()
+ .filter_map(|path| Some((RepoPath::from_proto(&path).ok()?, vec![]))),
+ );
+ let conflicts_changed =
+ self.snapshot.merge.merge_heads_by_conflicted_path != new_merge_heads;
+ self.snapshot.merge.merge_heads_by_conflicted_path = new_merge_heads;
self.snapshot.merge.message = update.merge_message.map(SharedString::from);
let new_stash_entries = GitStash {
entries: update
@@ -6054,7 +6047,7 @@ impl Repository {
}),
)
.collect::<Vec<_>>();
- if !edits.is_empty() {
+ if conflicts_changed || !edits.is_empty() {
cx.emit(RepositoryEvent::StatusesChanged);
}
self.snapshot.statuses_by_path.edit(edits, ());
@@ -6141,17 +6134,16 @@ impl Repository {
let RepositoryState::Local(LocalRepositoryState { backend, .. }) = state else {
bail!("not a local repository")
};
- let (snapshot, events) = this
- .update(&mut cx, |this, _| {
- this.paths_needing_status_update.clear();
- compute_snapshot(
- this.id,
- this.work_directory_abs_path.clone(),
- this.snapshot.clone(),
- backend.clone(),
- )
- })
- .await?;
+ let compute_snapshot = this.update(&mut cx, |this, _| {
+ this.paths_needing_status_update.clear();
+ compute_snapshot(
+ this.id,
+ this.work_directory_abs_path.clone(),
+ this.snapshot.clone(),
+ backend.clone(),
+ )
+ });
+ let (snapshot, events) = cx.background_spawn(compute_snapshot).await?;
this.update(&mut cx, |this, cx| {
this.snapshot = snapshot.clone();
this.clear_pending_ops(cx);
@@ -6759,25 +6751,24 @@ async fn compute_snapshot(
)])
.await?;
let stash_entries = backend.stash_entries().await?;
+ let mut conflicted_paths = Vec::new();
let statuses_by_path = SumTree::from_iter(
- statuses
- .entries
- .iter()
- .map(|(repo_path, status)| StatusEntry {
+ statuses.entries.iter().map(|(repo_path, status)| {
+ if status.is_conflicted() {
+ conflicted_paths.push(repo_path.clone());
+ }
+ StatusEntry {
repo_path: repo_path.clone(),
status: *status,
- }),
+ }
+ }),
(),
);
- let (merge_details, merge_heads_changed) =
- MergeDetails::load(&backend, &statuses_by_path, &prev_snapshot).await?;
- log::debug!("new merge details (changed={merge_heads_changed:?}): {merge_details:?}");
-
- if merge_heads_changed {
- events.push(RepositoryEvent::MergeHeadsChanged);
- }
+ let mut merge_details = prev_snapshot.merge;
+ let conflicts_changed = merge_details.update(&backend, conflicted_paths).await?;
+ log::debug!("new merge details: {merge_details:?}");
- if statuses_by_path != prev_snapshot.statuses_by_path {
+ if conflicts_changed || statuses_by_path != prev_snapshot.statuses_by_path {
events.push(RepositoryEvent::StatusesChanged)
}
@@ -336,7 +336,7 @@ mod conflict_set_tests {
second_head: UnmergedStatusCode::Updated,
},
);
- // Cause the repository to emit MergeHeadsChanged.
+ // Cause the repository to update cached conflicts
state.refs.insert("MERGE_HEAD".into(), "123".into())
})
.unwrap();
@@ -461,6 +461,168 @@ mod conflict_set_tests {
assert_eq!(conflict_range, Point::new(1, 0)..Point::new(6, 0));
});
}
+
+ #[gpui::test]
+ async fn test_conflict_updates_with_delayed_merge_head_conflicts(
+ executor: BackgroundExecutor,
+ cx: &mut TestAppContext,
+ ) {
+ zlog::init_test();
+ cx.update(|cx| {
+ settings::init(cx);
+ });
+
+ let initial_text = "
+ one
+ two
+ three
+ four
+ "
+ .unindent();
+
+ let conflicted_text = "
+ one
+ <<<<<<< HEAD
+ two
+ =======
+ TWO
+ >>>>>>> branch
+ three
+ four
+ "
+ .unindent();
+
+ let resolved_text = "
+ one
+ TWO
+ three
+ four
+ "
+ .unindent();
+
+ let fs = FakeFs::new(executor);
+ fs.insert_tree(
+ path!("/project"),
+ json!({
+ ".git": {},
+ "a.txt": initial_text,
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+ let (git_store, buffer) = project.update(cx, |project, cx| {
+ (
+ project.git_store().clone(),
+ project.open_local_buffer(path!("/project/a.txt"), cx),
+ )
+ });
+ let buffer = buffer.await.unwrap();
+ let conflict_set = git_store.update(cx, |git_store, cx| {
+ git_store.open_conflict_set(buffer.clone(), cx)
+ });
+
+ let (events_tx, events_rx) = mpsc::channel::<ConflictSetUpdate>();
+ let _conflict_set_subscription = cx.update(|cx| {
+ cx.subscribe(&conflict_set, move |_, event, _| {
+ events_tx.send(event.clone()).ok();
+ })
+ });
+
+ cx.run_until_parked();
+ events_rx
+ .try_recv()
+ .expect_err("conflict set should start empty");
+
+ fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+ state.refs.insert("MERGE_HEAD".into(), "123".into())
+ })
+ .unwrap();
+
+ cx.run_until_parked();
+ events_rx
+ .try_recv()
+ .expect_err("merge head without conflicted paths should not publish conflicts");
+ conflict_set.update(cx, |conflict_set, _| {
+ assert!(!conflict_set.has_conflict);
+ assert_eq!(conflict_set.snapshot.conflicts.len(), 0);
+ });
+
+ buffer.update(cx, |buffer, cx| {
+ buffer.set_text(conflicted_text.clone(), cx);
+ });
+ fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+ state.unmerged_paths.insert(
+ repo_path("a.txt"),
+ UnmergedStatus {
+ first_head: UnmergedStatusCode::Updated,
+ second_head: UnmergedStatusCode::Updated,
+ },
+ );
+ })
+ .unwrap();
+
+ cx.run_until_parked();
+ let update = events_rx
+ .try_recv()
+ .expect("conflicts should appear once conflicted paths are visible");
+ assert_eq!(update.old_range, 0..0);
+ assert_eq!(update.new_range, 0..1);
+ conflict_set.update(cx, |conflict_set, cx| {
+ assert!(conflict_set.has_conflict);
+ let conflict_range = conflict_set.snapshot().conflicts[0]
+ .range
+ .to_point(buffer.read(cx));
+ assert_eq!(conflict_range, Point::new(1, 0)..Point::new(6, 0));
+ });
+
+ buffer.update(cx, |buffer, cx| {
+ buffer.set_text(resolved_text.clone(), cx);
+ });
+
+ cx.run_until_parked();
+ let update = events_rx
+ .try_recv()
+ .expect("resolved buffer text should clear visible conflict markers");
+ assert_eq!(update.old_range, 0..1);
+ assert_eq!(update.new_range, 0..0);
+ conflict_set.update(cx, |conflict_set, _| {
+ assert!(conflict_set.has_conflict);
+ assert_eq!(conflict_set.snapshot.conflicts.len(), 0);
+ });
+
+ fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+ state.refs.insert("MERGE_HEAD".into(), "456".into());
+ })
+ .unwrap();
+
+ cx.run_until_parked();
+ events_rx.try_recv().expect_err(
+ "merge-head change without unmerged-path changes should not emit marker updates",
+ );
+ conflict_set.update(cx, |conflict_set, _| {
+ assert!(conflict_set.has_conflict);
+ assert_eq!(conflict_set.snapshot.conflicts.len(), 0);
+ });
+
+ fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+ state.unmerged_paths.remove(&repo_path("a.txt"));
+ state.refs.remove("MERGE_HEAD");
+ })
+ .unwrap();
+
+ cx.run_until_parked();
+ let update = events_rx.try_recv().expect(
+ "status catch-up should emit a no-op update when clearing stale conflict state",
+ );
+ assert_eq!(update.old_range, 0..0);
+ assert_eq!(update.new_range, 0..0);
+ assert!(update.buffer_range.is_none());
+ conflict_set.update(cx, |conflict_set, _| {
+ assert!(!conflict_set.has_conflict);
+ assert_eq!(conflict_set.snapshot.conflicts.len(), 0);
+ });
+ }
}
mod git_traversal {
@@ -10409,10 +10409,7 @@ async fn test_ignored_dirs_events(cx: &mut gpui::TestAppContext) {
assert_eq!(
repository_updates.lock().drain(..).collect::<Vec<_>>(),
- vec![
- RepositoryEvent::StatusesChanged,
- RepositoryEvent::MergeHeadsChanged,
- ],
+ vec![RepositoryEvent::StatusesChanged,],
"Initial worktree scan should produce a repo update event"
);
assert_eq!(
@@ -10579,7 +10576,6 @@ async fn test_odd_events_for_ignored_dirs(
assert_eq!(
repository_updates.lock().drain(..).collect::<Vec<_>>(),
vec![
- RepositoryEvent::MergeHeadsChanged,
RepositoryEvent::BranchChanged,
RepositoryEvent::StatusesChanged,
RepositoryEvent::StatusesChanged,
@@ -53,6 +53,10 @@ impl<K: Clone + Ord, V: Clone> TreeMap<K, V> {
self.0.is_empty()
}
+ pub fn contains_key(&self, key: &K) -> bool {
+ self.get(key).is_some()
+ }
+
pub fn get(&self, key: &K) -> Option<&V> {
let (.., item) = self
.0