Merge pull request #2036 from zed-industries/spurious-modified-buffers

Fix buffers appearing as modified when guest joined after buffer had been saved

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,33 @@ 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_saved_version =
+                    host_buffer.read_with(host_cx, |b, _| b.saved_version().clone());
+                let guest_saved_version =
+                    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 = 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 = 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 = 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 = guest_buffer.read_with(client_cx, |b, _| b.has_conflict());
+                assert_eq!(guest_has_conflict, host_has_conflict);
             }
         }
     }
@@ -638,14 +670,7 @@ async fn randomly_mutate_git(client: &mut TestClient, rng: &Mutex<StdRng>) {
         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 +694,63 @@ async fn randomly_mutate_git(client: &mut TestClient, rng: &Mutex<StdRng>) {
 }
 
 async fn randomly_mutate_fs(client: &mut TestClient, rng: &Mutex<StdRng>) {
-    let is_dir = rng.lock().gen::<bool>();
-    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::<bool>();
     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 +1216,15 @@ fn gen_file_name(rng: &Mutex<StdRng>) -> String {
     }
     name
 }
+
+async fn child_file_paths(client: &TestClient, dir_path: &Path) -> Vec<PathBuf> {
+    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
+}

crates/language/src/buffer.rs

@@ -385,6 +385,12 @@ 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
+            .ok_or_else(|| anyhow!("invalid saved_mtime"))?
+            .into();
         Ok(this)
     }
 
@@ -395,6 +401,9 @@ 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()),
         }
     }
 
@@ -549,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<Arc<Language>>, cx: &mut ModelContext<Self>) {
         self.syntax_map.lock().clear();
         self.language = language;

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 {

crates/rpc/proto/zed.proto

@@ -975,6 +975,9 @@ message BufferState {
     string base_text = 3;
     optional string diff_base = 4;
     LineEnding line_ending = 5;
+    repeated VectorClockEntry saved_version = 6;
+    string saved_version_fingerprint = 7;
+    Timestamp saved_mtime = 8;
 }
 
 message BufferChunk {

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;