From d85e66bbb4ff71f49d91f2012505a1495692990b Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 21 Aug 2024 15:33:24 +0900
Subject: [PATCH] copies: turn add_records() into non-stream API,
 block_on_stream() by caller

This is simpler, and I think it's generally better to not spawn executor in
library code.
---
 cli/src/commands/diff.rs      | 22 ++++++++++++----------
 cli/src/diff_util.rs          | 11 ++++++-----
 lib/src/copies.rs             | 10 ++++------
 lib/tests/test_merged_tree.rs | 20 +++++++++-----------
 4 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs
index f601d1fa6..069f4fbe9 100644
--- a/cli/src/commands/diff.rs
+++ b/cli/src/commands/diff.rs
@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use futures::executor::block_on_stream;
 use itertools::Itertools;
 use jj_lib::copies::CopyRecords;
 use jj_lib::repo::Repo;
@@ -76,11 +77,11 @@ pub(crate) fn cmd_diff(
         from_tree = from.tree()?;
         to_tree = to.tree()?;
 
-        copy_records.add_records(workspace_command.repo().store().get_copy_records(
-            None,
-            from.id(),
-            to.id(),
-        )?)?;
+        let stream = workspace_command
+            .repo()
+            .store()
+            .get_copy_records(None, from.id(), to.id())?;
+        copy_records.add_records(block_on_stream(stream))?;
     } else {
         let to = resolve_revision(&args.revision)?;
         let parents: Vec<_> = to.parents().try_collect()?;
@@ -88,11 +89,12 @@ pub(crate) fn cmd_diff(
         to_tree = to.tree()?;
 
         for p in &parents {
-            copy_records.add_records(workspace_command.repo().store().get_copy_records(
-                None,
-                p.id(),
-                to.id(),
-            )?)?;
+            let stream =
+                workspace_command
+                    .repo()
+                    .store()
+                    .get_copy_records(None, p.id(), to.id())?;
+            copy_records.add_records(block_on_stream(stream))?;
         }
     }
 
diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs
index 1b935cb80..af95a84fd 100644
--- a/cli/src/diff_util.rs
+++ b/cli/src/diff_util.rs
@@ -19,6 +19,7 @@ use std::ops::Range;
 use std::path::{Path, PathBuf};
 use std::{io, mem};
 
+use futures::executor::block_on_stream;
 use futures::stream::BoxStream;
 use futures::StreamExt;
 use itertools::Itertools;
@@ -369,11 +370,11 @@ impl<'a> DiffRenderer<'a> {
         let to_tree = commit.tree()?;
         let mut copy_records = CopyRecords::default();
         for parent_id in commit.parent_ids() {
-            copy_records.add_records(self.repo.store().get_copy_records(
-                None,
-                parent_id,
-                commit.id(),
-            )?)?;
+            let stream = self
+                .repo
+                .store()
+                .get_copy_records(None, parent_id, commit.id())?;
+            copy_records.add_records(block_on_stream(stream))?;
         }
         self.show_diff(
             ui,
diff --git a/lib/src/copies.rs b/lib/src/copies.rs
index 604623d66..ba2333ce0 100644
--- a/lib/src/copies.rs
+++ b/lib/src/copies.rs
@@ -18,8 +18,6 @@ use std::collections::HashMap;
 use std::pin::Pin;
 use std::task::{Context, Poll};
 
-use futures::executor::block_on_stream;
-use futures::stream::BoxStream;
 use futures::Stream;
 
 use crate::backend::{BackendResult, CopyRecord};
@@ -37,13 +35,13 @@ pub struct CopyRecords {
 }
 
 impl CopyRecords {
-    /// Adds information about a stream of CopyRecords to `self`.  A target with
-    /// multiple conflicts is discarded and treated as not having an origin.
+    /// Adds information about `CopyRecord`s to `self`. A target with multiple
+    /// conflicts is discarded and treated as not having an origin.
     pub fn add_records(
         &mut self,
-        stream: BoxStream<BackendResult<CopyRecord>>,
+        copy_records: impl IntoIterator<Item = BackendResult<CopyRecord>>,
     ) -> BackendResult<()> {
-        for record in block_on_stream(stream) {
+        for record in copy_records {
             let r = record?;
             let value = self
                 .targets
diff --git a/lib/tests/test_merged_tree.rs b/lib/tests/test_merged_tree.rs
index 4ea39585c..77b4d0776 100644
--- a/lib/tests/test_merged_tree.rs
+++ b/lib/tests/test_merged_tree.rs
@@ -803,17 +803,15 @@ fn test_diff_resolved() {
 fn create_copy_records(paths: &[(&RepoPath, &RepoPath)]) -> CopyRecords {
     let mut copy_records = CopyRecords::default();
     copy_records
-        .add_records(Box::pin(futures::stream::iter(paths.iter().map(
-            |&(source, target)| {
-                Ok(CopyRecord {
-                    source: source.to_owned(),
-                    target: target.to_owned(),
-                    target_commit: CommitId::new(vec![]),
-                    source_commit: CommitId::new(vec![]),
-                    source_file: FileId::new(vec![]),
-                })
-            },
-        ))))
+        .add_records(paths.iter().map(|&(source, target)| {
+            Ok(CopyRecord {
+                source: source.to_owned(),
+                target: target.to_owned(),
+                target_commit: CommitId::new(vec![]),
+                source_commit: CommitId::new(vec![]),
+                source_file: FileId::new(vec![]),
+            })
+        }))
         .unwrap();
     copy_records
 }