From b22d8fefd9555ad01c43b35e1804238678bcb58f Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sun, 25 Aug 2024 22:02:40 -0700
Subject: [PATCH] local_working_copy: pass max file size to snapshot directly

We were passing the max file size to snapshot to
`WorkingCopy::snapshot()` via `UserSettings`. It's simpler and more
flexible to set it  on `SnapshotOptions` instead.
---
 lib/src/working_copy.rs              |  1 +
 lib/tests/test_local_working_copy.rs | 27 ++++++++++++---------------
 lib/testutils/src/lib.rs             | 17 ++++++++++-------
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs
index 1ea3107d26..ad6a4e696e 100644
--- a/lib/src/working_copy.rs
+++ b/lib/src/working_copy.rs
@@ -180,6 +180,7 @@ pub enum SnapshotError {
 
 /// Options used when snapshotting the working copy. Some of them may be ignored
 /// by some `WorkingCopy` implementations.
+#[derive(Clone)]
 pub struct SnapshotOptions<'a> {
     /// The `.gitignore`s to use while snapshotting. The typically come from the
     /// user's configured patterns combined with per-repo patterns.
diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs
index c15ee8d1cf..0d2383bf9f 100644
--- a/lib/tests/test_local_working_copy.rs
+++ b/lib/tests/test_local_working_copy.rs
@@ -1271,31 +1271,28 @@ fn test_fsmonitor() {
 
 #[test]
 fn test_snapshot_max_new_file_size() {
-    let settings = UserSettings::from_config(
-        testutils::base_config()
-            .add_source(config::File::from_str(
-                "snapshot.max-new-file-size = \"1KiB\"",
-                config::FileFormat::Toml,
-            ))
-            .build()
-            .unwrap(),
-    );
+    let settings = testutils::user_settings();
     let mut test_workspace = TestWorkspace::init(&settings);
     let workspace_root = test_workspace.workspace.workspace_root().clone();
     let small_path = RepoPath::from_internal_string("small");
     let large_path = RepoPath::from_internal_string("large");
-    std::fs::write(small_path.to_fs_path(&workspace_root), vec![0; 1024]).unwrap();
+    let limit: usize = 1024;
+    std::fs::write(small_path.to_fs_path(&workspace_root), vec![0; limit]).unwrap();
+    let options = SnapshotOptions {
+        max_new_file_size: limit as u64,
+        ..SnapshotOptions::empty_for_test()
+    };
     test_workspace
-        .snapshot()
+        .snapshot_with_options(options.clone())
         .expect("files exactly matching the size limit should succeed");
-    std::fs::write(small_path.to_fs_path(&workspace_root), vec![0; 1024 + 1]).unwrap();
+    std::fs::write(small_path.to_fs_path(&workspace_root), vec![0; limit + 1]).unwrap();
     test_workspace
-        .snapshot()
+        .snapshot_with_options(options.clone())
         .expect("existing files may grow beyond the size limit");
     // A new file of 1KiB + 1 bytes should fail
-    std::fs::write(large_path.to_fs_path(&workspace_root), vec![0; 1024 + 1]).unwrap();
+    std::fs::write(large_path.to_fs_path(&workspace_root), vec![0; limit + 1]).unwrap();
     let err = test_workspace
-        .snapshot()
+        .snapshot_with_options(options.clone())
         .expect_err("new files beyond the size limit should fail");
     assert!(
         matches!(err, SnapshotError::NewFileTooLarge { .. }),
diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs
index 803d60999d..c9952a5a69 100644
--- a/lib/testutils/src/lib.rs
+++ b/lib/testutils/src/lib.rs
@@ -205,7 +205,6 @@ pub struct TestWorkspace {
     temp_dir: TempDir,
     pub workspace: Workspace,
     pub repo: Arc<ReadonlyRepo>,
-    settings: UserSettings,
 }
 
 impl TestWorkspace {
@@ -243,7 +242,6 @@ impl TestWorkspace {
             temp_dir,
             workspace,
             repo,
-            settings: settings.clone(),
         }
     }
 
@@ -254,16 +252,21 @@ impl TestWorkspace {
     /// Snapshots the working copy and returns the tree. Updates the working
     /// copy state on disk, but does not update the working-copy commit (no
     /// new operation).
-    pub fn snapshot(&mut self) -> Result<MergedTree, SnapshotError> {
+    pub fn snapshot_with_options(
+        &mut self,
+        options: SnapshotOptions,
+    ) -> Result<MergedTree, SnapshotError> {
         let mut locked_ws = self.workspace.start_working_copy_mutation().unwrap();
-        let tree_id = locked_ws.locked_wc().snapshot(SnapshotOptions {
-            max_new_file_size: self.settings.max_new_file_size().unwrap(),
-            ..SnapshotOptions::empty_for_test()
-        })?;
+        let tree_id = locked_ws.locked_wc().snapshot(options)?;
         // arbitrary operation id
         locked_ws.finish(self.repo.op_id().clone()).unwrap();
         Ok(self.repo.store().get_root_tree(&tree_id).unwrap())
     }
+
+    /// Like `snapshot_with_option()` but with default options
+    pub fn snapshot(&mut self) -> Result<MergedTree, SnapshotError> {
+        self.snapshot_with_options(SnapshotOptions::empty_for_test())
+    }
 }
 
 pub fn load_repo_at_head(settings: &UserSettings, repo_path: &Path) -> Arc<ReadonlyRepo> {