working_copy: pass SnapshotOptions by reference

Though SnapshotOptions can be cheaply cloned, it doesn't make much sense that
snapshot() consumes a settings-like object.

cli/examples/custom-working-copy/main.rs

@@ -232,9 +232,12 @@ impl LockedWorkingCopy for LockedConflictsWorkingCopy {
         self.inner.old_tree_id()
     }
 
-    fn snapshot(&mut self, mut options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
-        options.base_ignores = options.base_ignores.chain("", "/.conflicts".as_bytes())?;
-        self.inner.snapshot(options)
+    fn snapshot(&mut self, options: &SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
+        let options = SnapshotOptions {
+            base_ignores: options.base_ignores.chain("", "/.conflicts".as_bytes())?,
+            ..options.clone()
+        };
+        self.inner.snapshot(&options)
     }
 
     fn check_out(&mut self, commit: &Commit) -> Result<CheckoutStats, CheckoutError> {

cli/src/cli_util.rs

@@ -1367,7 +1367,7 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \
             };
         self.user_repo = ReadonlyUserRepo::new(repo);
         let progress = crate::progress::snapshot_progress(ui);
-        let new_tree_id = locked_ws.locked_wc().snapshot(SnapshotOptions {
+        let new_tree_id = locked_ws.locked_wc().snapshot(&SnapshotOptions {
             base_ignores,
             fsmonitor_settings,
             progress: progress.as_ref().map(|x| x as _),

cli/src/commands/file/untrack.rs

@@ -68,7 +68,7 @@ pub(crate) fn cmd_file_untrack(
     locked_ws.locked_wc().reset(&new_commit)?;
     // Commit the working copy again so we can inform the user if paths couldn't be
     // untracked because they're not ignored.
-    let wc_tree_id = locked_ws.locked_wc().snapshot(SnapshotOptions {
+    let wc_tree_id = locked_ws.locked_wc().snapshot(&SnapshotOptions {
         base_ignores,
         fsmonitor_settings: command.settings().fsmonitor_settings()?,
         progress: None,

cli/src/merge_tools/diff_working_copies.rs

@@ -282,7 +282,7 @@ diff editing in mind and be a little inaccurate.
         let mut output_tree_state = diff_wc
             .output_tree_state
             .unwrap_or(diff_wc.right_tree_state);
-        output_tree_state.snapshot(SnapshotOptions {
+        output_tree_state.snapshot(&SnapshotOptions {
             base_ignores,
             fsmonitor_settings: FsmonitorSettings::None,
             progress: None,

lib/src/local_working_copy.rs

@@ -792,7 +792,7 @@ impl TreeState {
     /// Look for changes to the working copy. If there are any changes, create
     /// a new tree from it and return it, and also update the dirstate on disk.
     #[instrument(skip_all)]
-    pub fn snapshot(&mut self, options: SnapshotOptions) -> Result<bool, SnapshotError> {
+    pub fn snapshot(&mut self, options: &SnapshotOptions) -> Result<bool, SnapshotError> {
         let SnapshotOptions {
             base_ignores,
             fsmonitor_settings,
@@ -802,7 +802,7 @@ impl TreeState {
 
         let sparse_matcher = self.sparse_matcher();
 
-        let fsmonitor_clock_needs_save = fsmonitor_settings != FsmonitorSettings::None;
+        let fsmonitor_clock_needs_save = *fsmonitor_settings != FsmonitorSettings::None;
         let mut is_dirty = fsmonitor_clock_needs_save;
         let FsmonitorMatcher {
             matcher: fsmonitor_matcher,
@@ -829,7 +829,7 @@ impl TreeState {
             let directory_to_visit = DirectoryToVisit {
                 dir: RepoPathBuf::root(),
                 disk_dir: self.working_copy_path.clone(),
-                git_ignore: base_ignores,
+                git_ignore: base_ignores.clone(),
                 file_states: self.file_states.all(),
             };
             self.visit_directory(
@@ -839,8 +839,8 @@ impl TreeState {
                 file_states_tx,
                 present_files_tx,
                 directory_to_visit,
-                progress,
-                max_new_file_size,
+                *progress,
+                *max_new_file_size,
             )
         })?;
 
@@ -1075,13 +1075,13 @@ impl TreeState {
     #[instrument(skip_all)]
     fn make_fsmonitor_matcher(
         &self,
-        fsmonitor_settings: FsmonitorSettings,
+        fsmonitor_settings: &FsmonitorSettings,
     ) -> Result<FsmonitorMatcher, SnapshotError> {
         let (watchman_clock, changed_files) = match fsmonitor_settings {
             FsmonitorSettings::None => (None, None),
-            FsmonitorSettings::Test { changed_files } => (None, Some(changed_files)),
+            FsmonitorSettings::Test { changed_files } => (None, Some(changed_files.clone())),
             #[cfg(feature = "watchman")]
-            FsmonitorSettings::Watchman(config) => match self.query_watchman(&config) {
+            FsmonitorSettings::Watchman(config) => match self.query_watchman(config) {
                 Ok((watchman_clock, changed_files)) => (Some(watchman_clock.into()), changed_files),
                 Err(err) => {
                     tracing::warn!(?err, "Failed to query filesystem monitor");
@@ -1830,7 +1830,7 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy {
         &self.old_tree_id
     }
 
-    fn snapshot(&mut self, options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
+    fn snapshot(&mut self, options: &SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
         let tree_state = self
             .wc
             .tree_state_mut()

lib/src/working_copy.rs

@@ -100,7 +100,7 @@ pub trait LockedWorkingCopy {
     fn old_tree_id(&self) -> &MergedTreeId;
 
     /// Snapshot the working copy and return the tree id.
-    fn snapshot(&mut self, options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError>;
+    fn snapshot(&mut self, options: &SnapshotOptions) -> Result<MergedTreeId, SnapshotError>;
 
     /// Check out the specified commit in the working copy.
     fn check_out(&mut self, commit: &Commit) -> Result<CheckoutStats, CheckoutError>;

lib/tests/test_local_working_copy.rs

@@ -756,7 +756,7 @@ fn test_snapshot_racy_timestamps() {
             .unwrap();
         let new_tree_id = locked_ws
             .locked_wc()
-            .snapshot(SnapshotOptions::empty_for_test())
+            .snapshot(&SnapshotOptions::empty_for_test())
             .unwrap();
         assert_ne!(new_tree_id, previous_tree_id);
         previous_tree_id = new_tree_id;
@@ -790,7 +790,7 @@ fn test_snapshot_special_file() {
     let mut locked_ws = ws.start_working_copy_mutation().unwrap();
     let tree_id = locked_ws
         .locked_wc()
-        .snapshot(SnapshotOptions::empty_for_test())
+        .snapshot(&SnapshotOptions::empty_for_test())
         .unwrap();
     locked_ws.finish(OperationId::from_hex("abc123")).unwrap();
     let tree = store.get_root_tree(&tree_id).unwrap();
@@ -1204,7 +1204,7 @@ fn test_fsmonitor() {
         let fs_paths = paths.iter().map(|p| p.to_fs_path(Path::new(""))).collect();
         locked_ws
             .locked_wc()
-            .snapshot(SnapshotOptions {
+            .snapshot(&SnapshotOptions {
                 fsmonitor_settings: FsmonitorSettings::Test {
                     changed_files: fs_paths,
                 },
@@ -1283,16 +1283,16 @@ fn test_snapshot_max_new_file_size() {
         ..SnapshotOptions::empty_for_test()
     };
     test_workspace
-        .snapshot_with_options(options.clone())
+        .snapshot_with_options(&options)
         .expect("files exactly matching the size limit should succeed");
     std::fs::write(small_path.to_fs_path(&workspace_root), vec![0; limit + 1]).unwrap();
     test_workspace
-        .snapshot_with_options(options.clone())
+        .snapshot_with_options(&options)
         .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; limit + 1]).unwrap();
     let err = test_workspace
-        .snapshot_with_options(options.clone())
+        .snapshot_with_options(&options)
         .expect_err("new files beyond the size limit should fail");
     assert!(
         matches!(err, SnapshotError::NewFileTooLarge { .. }),

lib/tests/test_local_working_copy_concurrent.rs

@@ -138,7 +138,7 @@ fn test_checkout_parallel() {
                 let mut locked_ws = workspace.start_working_copy_mutation().unwrap();
                 let new_tree_id = locked_ws
                     .locked_wc()
-                    .snapshot(SnapshotOptions::empty_for_test())
+                    .snapshot(&SnapshotOptions::empty_for_test())
                     .unwrap();
                 assert!(tree_ids.contains(&new_tree_id));
             });

lib/testutils/src/lib.rs

@@ -267,7 +267,7 @@ impl TestWorkspace {
     /// new operation).
     pub fn snapshot_with_options(
         &mut self,
-        options: SnapshotOptions,
+        options: &SnapshotOptions,
     ) -> Result<MergedTree, SnapshotError> {
         let mut locked_ws = self.workspace.start_working_copy_mutation().unwrap();
         let tree_id = locked_ws.locked_wc().snapshot(options)?;
@@ -278,7 +278,7 @@ impl TestWorkspace {
 
     /// Like `snapshot_with_option()` but with default options
     pub fn snapshot(&mut self) -> Result<MergedTree, SnapshotError> {
-        self.snapshot_with_options(SnapshotOptions::empty_for_test())
+        self.snapshot_with_options(&SnapshotOptions::empty_for_test())
     }
 }