From 467e5691b968134e8d29ee6c4a2ad2c73fcf134c Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 16 Jan 2023 16:22:47 +0100 Subject: [PATCH 1/6] Include saved mtime and fingerprint when serializing buffers This still doesn't include: - An assertion in the randomized test to ensure buffers are not spuriously marked as modified - Sending an update when synchronizing buffers after a reconnection --- crates/language/src/buffer.rs | 7 +++++++ crates/rpc/proto/zed.proto | 2 ++ 2 files changed, 9 insertions(+) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 9ceb6b32a9..b92f0a7c3e 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -385,6 +385,11 @@ impl Buffer { rpc::proto::LineEnding::from_i32(message.line_ending) .ok_or_else(|| anyhow!("missing line_ending"))?, )); + this.saved_version_fingerprint = message.saved_version_fingerprint; + this.saved_mtime = message + .saved_mtime + .ok_or_else(|| anyhow!("invalid saved_mtime"))? + .into(); Ok(this) } @@ -395,6 +400,8 @@ impl Buffer { base_text: self.base_text().to_string(), diff_base: self.diff_base.as_ref().map(|h| h.to_string()), line_ending: proto::serialize_line_ending(self.line_ending()) as i32, + saved_version_fingerprint: self.saved_version_fingerprint.clone(), + saved_mtime: Some(self.saved_mtime.into()), } } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 8626806abe..86494df05b 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -975,6 +975,8 @@ message BufferState { string base_text = 3; optional string diff_base = 4; LineEnding line_ending = 5; + string saved_version_fingerprint = 6; + Timestamp saved_mtime = 7; } message BufferChunk { From 2cd9db1cfeb4dd787d88a806aca1301d69060136 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 17 Jan 2023 16:20:09 +0100 Subject: [PATCH 2/6] Ensure `Buffer::{is_dirty,has_conflict}` converge in randomized test --- .../src/tests/randomized_integration_tests.rs | 99 ++++++++++++++----- 1 file changed, 77 insertions(+), 22 deletions(-) diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 11c62005c2..8517385798 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -10,7 +10,7 @@ use collections::BTreeMap; use fs::{FakeFs, Fs as _}; use futures::StreamExt as _; use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; -use language::{range_to_lsp, FakeLspAdapter, Language, LanguageConfig, PointUtf16}; +use language::{range_to_lsp, FakeLspAdapter, Language, LanguageConfig, PointUtf16, Rope}; use lsp::FakeLanguageServer; use parking_lot::Mutex; use project::{search::SearchQuery, Project}; @@ -19,7 +19,12 @@ use rand::{ prelude::*, }; use settings::Settings; -use std::{env, ffi::OsStr, path::PathBuf, sync::Arc}; +use std::{ + env, + ffi::OsStr, + path::{Path, PathBuf}, + sync::Arc, +}; #[gpui::test(iterations = 100)] async fn test_random_collaboration( @@ -398,6 +403,14 @@ async fn test_random_collaboration( let guest_diff_base = guest_buffer .read_with(client_cx, |b, _| b.diff_base().map(ToString::to_string)); assert_eq!(guest_diff_base, host_diff_base); + + let host_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); + let guest_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); + assert_eq!(guest_is_dirty, host_is_dirty); + + let host_has_conflict = host_buffer.read_with(host_cx, |b, _| b.has_conflict()); + let guest_has_conflict = host_buffer.read_with(host_cx, |b, _| b.has_conflict()); + assert_eq!(guest_has_conflict, host_has_conflict); } } } @@ -638,14 +651,7 @@ async fn randomly_mutate_git(client: &mut TestClient, rng: &Mutex) { client.fs.create_dir(&git_dir_path).await.unwrap(); } - let mut child_paths = client.fs.read_dir(&dir_path).await.unwrap(); - let mut child_file_paths = Vec::new(); - while let Some(child_path) = child_paths.next().await { - let child_path = child_path.unwrap(); - if client.fs.is_file(&child_path).await { - child_file_paths.push(child_path); - } - } + let mut child_file_paths = child_file_paths(client, &dir_path).await; let count = rng.lock().gen_range(0..=child_file_paths.len()); child_file_paths.shuffle(&mut *rng.lock()); child_file_paths.truncate(count); @@ -669,26 +675,63 @@ async fn randomly_mutate_git(client: &mut TestClient, rng: &Mutex) { } async fn randomly_mutate_fs(client: &mut TestClient, rng: &Mutex) { - let is_dir = rng.lock().gen::(); - let mut new_path = client + let parent_dir_path = client .fs .directories() .await .choose(&mut *rng.lock()) .unwrap() .clone(); - new_path.push(gen_file_name(rng)); + + let is_dir = rng.lock().gen::(); if is_dir { - log::info!("{}: creating local dir at {:?}", client.username, new_path); - client.fs.create_dir(&new_path).await.unwrap(); + let mut dir_path = parent_dir_path.clone(); + dir_path.push(gen_file_name(rng)); + log::info!("{}: creating local dir at {:?}", client.username, dir_path); + client.fs.create_dir(&dir_path).await.unwrap(); } else { - new_path.set_extension("rs"); - log::info!("{}: creating local file at {:?}", client.username, new_path); - client - .fs - .create_file(&new_path, Default::default()) - .await - .unwrap(); + let child_file_paths = child_file_paths(client, &parent_dir_path).await; + let create_new_file = child_file_paths.is_empty() || rng.lock().gen(); + let text = Alphanumeric.sample_string(&mut *rng.lock(), 16); + if create_new_file { + let mut file_path = parent_dir_path.clone(); + file_path.push(gen_file_name(rng)); + file_path.set_extension("rs"); + log::info!( + "{}: creating local file at {:?}", + client.username, + file_path + ); + client + .fs + .create_file(&file_path, Default::default()) + .await + .unwrap(); + log::info!( + "{}: setting local file {:?} text to {:?}", + client.username, + file_path, + text + ); + client + .fs + .save(&file_path, &Rope::from(text.as_str()), fs::LineEnding::Unix) + .await + .unwrap(); + } else { + let file_path = child_file_paths.choose(&mut *rng.lock()).unwrap(); + log::info!( + "{}: setting local file {:?} text to {:?}", + client.username, + file_path, + text + ); + client + .fs + .save(file_path, &Rope::from(text.as_str()), fs::LineEnding::Unix) + .await + .unwrap(); + } } } @@ -1154,3 +1197,15 @@ fn gen_file_name(rng: &Mutex) -> String { } name } + +async fn child_file_paths(client: &TestClient, dir_path: &Path) -> Vec { + let mut child_paths = client.fs.read_dir(dir_path).await.unwrap(); + let mut child_file_paths = Vec::new(); + while let Some(child_path) = child_paths.next().await { + let child_path = child_path.unwrap(); + if client.fs.is_file(&child_path).await { + child_file_paths.push(child_path); + } + } + child_file_paths +} From bb200aa082f7bb760463fd55eebb562dc86d1601 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 17 Jan 2023 16:20:50 +0100 Subject: [PATCH 3/6] Relay saved version metadata to ensure buffers modified state converges --- crates/language/src/buffer.rs | 10 ++++++++++ crates/project/src/project.rs | 13 +++++++++++++ crates/rpc/proto/zed.proto | 5 +++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index b92f0a7c3e..0e546d4ba0 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -385,6 +385,7 @@ impl Buffer { rpc::proto::LineEnding::from_i32(message.line_ending) .ok_or_else(|| anyhow!("missing line_ending"))?, )); + this.saved_version = proto::deserialize_version(message.saved_version); this.saved_version_fingerprint = message.saved_version_fingerprint; this.saved_mtime = message .saved_mtime @@ -400,6 +401,7 @@ impl Buffer { base_text: self.base_text().to_string(), diff_base: self.diff_base.as_ref().map(|h| h.to_string()), line_ending: proto::serialize_line_ending(self.line_ending()) as i32, + saved_version: proto::serialize_version(&self.saved_version), saved_version_fingerprint: self.saved_version_fingerprint.clone(), saved_mtime: Some(self.saved_mtime.into()), } @@ -556,6 +558,14 @@ impl Buffer { &self.saved_version } + pub fn saved_version_fingerprint(&self) -> &str { + &self.saved_version_fingerprint + } + + pub fn saved_mtime(&self) -> SystemTime { + self.saved_mtime + } + pub fn set_language(&mut self, language: Option>, cx: &mut ModelContext) { self.syntax_map.lock().clear(); self.language = language; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 478271118c..d3b3c11f2e 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -5209,6 +5209,19 @@ impl Project { }) .log_err(); + client + .send(proto::BufferReloaded { + project_id, + buffer_id, + version: language::proto::serialize_version(buffer.saved_version()), + mtime: Some(buffer.saved_mtime().into()), + fingerprint: buffer.saved_version_fingerprint().into(), + line_ending: language::proto::serialize_line_ending( + buffer.line_ending(), + ) as i32, + }) + .log_err(); + cx.background() .spawn( async move { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 86494df05b..ba481ce45b 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -975,8 +975,9 @@ message BufferState { string base_text = 3; optional string diff_base = 4; LineEnding line_ending = 5; - string saved_version_fingerprint = 6; - Timestamp saved_mtime = 7; + repeated VectorClockEntry saved_version = 6; + string saved_version_fingerprint = 7; + Timestamp saved_mtime = 8; } message BufferChunk { From fcf97ab41eabfa6719138a368e0d0301e3daa030 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 17 Jan 2023 16:21:20 +0100 Subject: [PATCH 4/6] Bump protocol version --- crates/rpc/src/rpc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index 39cdea7165..b05bc17906 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 45; +pub const PROTOCOL_VERSION: u32 = 46; From cc788dc5f74164a9bab5eeefed015e7be165157f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 17 Jan 2023 16:46:06 +0100 Subject: [PATCH 5/6] Verify `saved_version`, `saved_version_fingerprint` and `saved_mtime` --- .../src/tests/randomized_integration_tests.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 8517385798..34346e2128 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -404,6 +404,25 @@ async fn test_random_collaboration( .read_with(client_cx, |b, _| b.diff_base().map(ToString::to_string)); assert_eq!(guest_diff_base, host_diff_base); + let host_saved_version = + host_buffer.read_with(host_cx, |b, _| b.saved_version().clone()); + let guest_saved_version = + host_buffer.read_with(host_cx, |b, _| b.saved_version().clone()); + assert_eq!(guest_saved_version, host_saved_version); + + let host_saved_version_fingerprint = host_buffer + .read_with(host_cx, |b, _| b.saved_version_fingerprint().to_string()); + let guest_saved_version_fingerprint = host_buffer + .read_with(host_cx, |b, _| b.saved_version_fingerprint().to_string()); + assert_eq!( + guest_saved_version_fingerprint, + host_saved_version_fingerprint + ); + + let host_saved_mtime = host_buffer.read_with(host_cx, |b, _| b.saved_mtime()); + let guest_saved_mtime = host_buffer.read_with(host_cx, |b, _| b.saved_mtime()); + assert_eq!(guest_saved_mtime, host_saved_mtime); + let host_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); let guest_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); assert_eq!(guest_is_dirty, host_is_dirty); From dc88a67f502dc77273b0170d7358ce7c45f79453 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 17 Jan 2023 18:09:45 +0100 Subject: [PATCH 6/6] Fix assertions --- .../collab/src/tests/randomized_integration_tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 34346e2128..ed8f984d27 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -407,28 +407,28 @@ async fn test_random_collaboration( let host_saved_version = host_buffer.read_with(host_cx, |b, _| b.saved_version().clone()); let guest_saved_version = - host_buffer.read_with(host_cx, |b, _| b.saved_version().clone()); + guest_buffer.read_with(client_cx, |b, _| b.saved_version().clone()); assert_eq!(guest_saved_version, host_saved_version); let host_saved_version_fingerprint = host_buffer .read_with(host_cx, |b, _| b.saved_version_fingerprint().to_string()); - let guest_saved_version_fingerprint = host_buffer - .read_with(host_cx, |b, _| b.saved_version_fingerprint().to_string()); + let guest_saved_version_fingerprint = guest_buffer + .read_with(client_cx, |b, _| b.saved_version_fingerprint().to_string()); assert_eq!( guest_saved_version_fingerprint, host_saved_version_fingerprint ); let host_saved_mtime = host_buffer.read_with(host_cx, |b, _| b.saved_mtime()); - let guest_saved_mtime = host_buffer.read_with(host_cx, |b, _| b.saved_mtime()); + let guest_saved_mtime = guest_buffer.read_with(client_cx, |b, _| b.saved_mtime()); assert_eq!(guest_saved_mtime, host_saved_mtime); let host_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); - let guest_is_dirty = host_buffer.read_with(host_cx, |b, _| b.is_dirty()); + let guest_is_dirty = guest_buffer.read_with(client_cx, |b, _| b.is_dirty()); assert_eq!(guest_is_dirty, host_is_dirty); let host_has_conflict = host_buffer.read_with(host_cx, |b, _| b.has_conflict()); - let guest_has_conflict = host_buffer.read_with(host_cx, |b, _| b.has_conflict()); + let guest_has_conflict = guest_buffer.read_with(client_cx, |b, _| b.has_conflict()); assert_eq!(guest_has_conflict, host_has_conflict); } }