From cfcdd7186558fefbf9610bcaa825825f88b9a5e5 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 26 Oct 2023 22:54:09 -0700
Subject: [PATCH] backend: make `read_conflict` synchronous again

This avoids https://github.com/rust-lang/futures-rs/issues/2090. I
don't think we need to worry about reading legacy conflicts
asynchronously - async is really only useful for Google's backend
right now, and we don't use the legacy format at Google. In
particular, I don't want `MergedTree::value()` to have to be async.
---
 cli/examples/custom-backend/main.rs |  4 ++--
 lib/src/backend.rs                  |  4 +++-
 lib/src/git_backend.rs              | 27 +++++++++++++--------------
 lib/src/local_backend.rs            |  2 +-
 lib/src/store.rs                    |  2 +-
 lib/testutils/src/test_backend.rs   |  2 +-
 6 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/cli/examples/custom-backend/main.rs b/cli/examples/custom-backend/main.rs
index b79a0f941..dddb71586 100644
--- a/cli/examples/custom-backend/main.rs
+++ b/cli/examples/custom-backend/main.rs
@@ -141,8 +141,8 @@ impl Backend for JitBackend {
         self.inner.write_tree(path, contents)
     }
 
-    async fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
-        self.inner.read_conflict(path, id).await
+    fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
+        self.inner.read_conflict(path, id)
     }
 
     fn write_conflict(&self, path: &RepoPath, contents: &Conflict) -> BackendResult<ConflictId> {
diff --git a/lib/src/backend.rs b/lib/src/backend.rs
index e059546e6..991277f18 100644
--- a/lib/src/backend.rs
+++ b/lib/src/backend.rs
@@ -498,7 +498,9 @@ pub trait Backend: Send + Sync + Debug {
 
     fn write_tree(&self, path: &RepoPath, contents: &Tree) -> BackendResult<TreeId>;
 
-    async fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict>;
+    // Not async because it would force `MergedTree::value()` to be async. We don't
+    // need this to be async anyway because it's only used by legacy repos.
+    fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict>;
 
     fn write_conflict(&self, path: &RepoPath, contents: &Conflict) -> BackendResult<ConflictId>;
 
diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs
index cff5e2ca1..b928afded 100644
--- a/lib/src/git_backend.rs
+++ b/lib/src/git_backend.rs
@@ -275,6 +275,16 @@ impl GitBackend {
         }
         self.save_extra_metadata_table(mut_table, &table_lock)
     }
+
+    fn read_file_sync(&self, id: &FileId) -> BackendResult<Box<dyn Read>> {
+        let git_blob_id = validate_git_object_id(id)?;
+        let locked_repo = self.repo.lock().unwrap();
+        let blob = locked_repo
+            .find_blob(git_blob_id)
+            .map_err(|err| map_not_found_err(err, id))?;
+        let content = blob.content().to_owned();
+        Ok(Box::new(Cursor::new(content)))
+    }
 }
 
 fn commit_from_git_without_root_parent(
@@ -529,13 +539,7 @@ impl Backend for GitBackend {
     }
 
     async fn read_file(&self, _path: &RepoPath, id: &FileId) -> BackendResult<Box<dyn Read>> {
-        let git_blob_id = validate_git_object_id(id)?;
-        let locked_repo = self.repo.lock().unwrap();
-        let blob = locked_repo
-            .find_blob(git_blob_id)
-            .map_err(|err| map_not_found_err(err, id))?;
-        let content = blob.content().to_owned();
-        Ok(Box::new(Cursor::new(content)))
+        self.read_file_sync(id)
     }
 
     fn write_file(&self, _path: &RepoPath, contents: &mut dyn Read) -> BackendResult<FileId> {
@@ -673,13 +677,8 @@ impl Backend for GitBackend {
         Ok(TreeId::from_bytes(oid.as_bytes()))
     }
 
-    async fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
-        let mut file = self
-            .read_file(
-                &RepoPath::from_internal_string("unused"),
-                &FileId::new(id.to_bytes()),
-            )
-            .await?;
+    fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
+        let mut file = self.read_file_sync(&FileId::new(id.to_bytes()))?;
         let mut data = String::new();
         file.read_to_string(&mut data)
             .map_err(|err| BackendError::ReadObject {
diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs
index b1c87c268..06facf56f 100644
--- a/lib/src/local_backend.rs
+++ b/lib/src/local_backend.rs
@@ -221,7 +221,7 @@ impl Backend for LocalBackend {
         Ok(id)
     }
 
-    async fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
+    fn read_conflict(&self, _path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
         let path = self.conflict_path(id);
         let buf = fs::read(path).map_err(|err| map_not_found_err(err, id))?;
 
diff --git a/lib/src/store.rs b/lib/src/store.rs
index 705a79940..2e20fccf7 100644
--- a/lib/src/store.rs
+++ b/lib/src/store.rs
@@ -229,7 +229,7 @@ impl Store {
         path: &RepoPath,
         id: &ConflictId,
     ) -> BackendResult<MergedTreeValue> {
-        let backend_conflict = block_on(self.backend.read_conflict(path, id))?;
+        let backend_conflict = self.backend.read_conflict(path, id)?;
         Ok(Merge::from_backend_conflict(backend_conflict))
     }
 
diff --git a/lib/testutils/src/test_backend.rs b/lib/testutils/src/test_backend.rs
index 9bd7be0bd..fcc88b411 100644
--- a/lib/testutils/src/test_backend.rs
+++ b/lib/testutils/src/test_backend.rs
@@ -224,7 +224,7 @@ impl Backend for TestBackend {
         Ok(id)
     }
 
-    async fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
+    fn read_conflict(&self, path: &RepoPath, id: &ConflictId) -> BackendResult<Conflict> {
         match self
             .locked_data()
             .conflicts