working copy: add snapshot() function to the backend trait

This includes documenting the new function and the other types moved
to the `working_copy` module.

cli/src/cli_util.rs

@@ -42,8 +42,7 @@ use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::hex_util::to_reverse_hex;
 use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::local_working_copy::{
-    CheckoutStats, LocalWorkingCopy, LockedLocalWorkingCopy, ResetError, SnapshotError,
-    SnapshotOptions, WorkingCopyStateError,
+    CheckoutStats, LocalWorkingCopy, LockedLocalWorkingCopy, ResetError, WorkingCopyStateError,
 };
 use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
@@ -63,7 +62,7 @@ use jj_lib::revset::{
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
 use jj_lib::transaction::Transaction;
 use jj_lib::tree::TreeMergeError;
-use jj_lib::working_copy::{LockedWorkingCopy, WorkingCopy};
+use jj_lib::working_copy::{LockedWorkingCopy, SnapshotError, SnapshotOptions, WorkingCopy};
 use jj_lib::workspace::{
     LockedWorkspace, Workspace, WorkspaceInitError, WorkspaceLoadError, WorkspaceLoader,
 };

cli/src/commands/mod.rs

@@ -35,7 +35,6 @@ use jj_lib::backend::{CommitId, ObjectId, TreeValue};
 use jj_lib::commit::Commit;
 use jj_lib::dag_walk::topo_order_reverse;
 use jj_lib::git_backend::GitBackend;
-use jj_lib::local_working_copy::SnapshotOptions;
 use jj_lib::matchers::EverythingMatcher;
 use jj_lib::merge::Merge;
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
@@ -48,7 +47,7 @@ use jj_lib::revset_graph::{
 };
 use jj_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser};
 use jj_lib::settings::UserSettings;
-use jj_lib::working_copy::LockedWorkingCopy;
+use jj_lib::working_copy::{LockedWorkingCopy, SnapshotOptions};
 use jj_lib::workspace::Workspace;
 use jj_lib::{conflicts, file_util, revset};
 use maplit::{hashmap, hashset};

cli/src/merge_tools/external.rs

@@ -10,13 +10,14 @@ use itertools::Itertools;
 use jj_lib::backend::{FileId, MergedTreeId, TreeValue};
 use jj_lib::conflicts::{self, materialize_merge_result};
 use jj_lib::gitignore::GitIgnoreFile;
-use jj_lib::local_working_copy::{CheckoutError, SnapshotOptions, TreeState, TreeStateError};
+use jj_lib::local_working_copy::{CheckoutError, TreeState, TreeStateError};
 use jj_lib::matchers::Matcher;
 use jj_lib::merge::Merge;
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
 use jj_lib::repo_path::RepoPath;
 use jj_lib::settings::UserSettings;
 use jj_lib::store::Store;
+use jj_lib::working_copy::SnapshotOptions;
 use regex::{Captures, Regex};
 use tempfile::TempDir;
 use thiserror::Error;

cli/src/merge_tools/mod.rs

@@ -21,11 +21,11 @@ use config::ConfigError;
 use jj_lib::backend::MergedTreeId;
 use jj_lib::conflicts::extract_as_single_hunk;
 use jj_lib::gitignore::GitIgnoreFile;
-use jj_lib::local_working_copy::SnapshotError;
 use jj_lib::matchers::Matcher;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
+use jj_lib::working_copy::SnapshotError;
 use thiserror::Error;
 
 use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError};

lib/src/local_working_copy.rs

@@ -17,7 +17,6 @@
 use std::any::Any;
 use std::collections::{BTreeMap, HashSet};
 use std::error::Error;
-use std::ffi::OsString;
 use std::fs;
 use std::fs::{File, Metadata, OpenOptions};
 use std::io::{Read, Write};
@@ -59,7 +58,9 @@ use crate::repo_path::{FsPathParseError, RepoPath, RepoPathComponent, RepoPathJo
 use crate::settings::HumanByteSize;
 use crate::store::Store;
 use crate::tree::Tree;
-use crate::working_copy::{LockedWorkingCopy, WorkingCopy};
+use crate::working_copy::{
+    LockedWorkingCopy, SnapshotError, SnapshotOptions, SnapshotProgress, WorkingCopy,
+};
 
 #[cfg(unix)]
 type FileExecutableFlag = bool;
@@ -309,28 +310,6 @@ pub struct CheckoutStats {
     pub skipped_files: u32,
 }
 
-#[derive(Debug, Error)]
-pub enum SnapshotError {
-    #[error("Working copy path {} is not valid UTF-8", path.to_string_lossy())]
-    InvalidUtf8Path { path: OsString },
-    #[error("Symlink {path} target is not valid UTF-8")]
-    InvalidUtf8SymlinkTarget { path: PathBuf, target: PathBuf },
-    #[error("Internal backend error: {0}")]
-    InternalBackendError(#[from] BackendError),
-    #[error("New file {path} of size ~{size} exceeds snapshot.max-new-file-size ({max_size})")]
-    NewFileTooLarge {
-        path: PathBuf,
-        size: HumanByteSize,
-        max_size: HumanByteSize,
-    },
-    #[error("{message}: {err:?}")]
-    Other {
-        message: String,
-        #[source]
-        err: Box<dyn std::error::Error + Send + Sync>,
-    },
-}
-
 #[derive(Debug, Error)]
 pub enum CheckoutError {
     // The current working-copy commit was deleted, maybe by an overly aggressive GC that happened
@@ -362,24 +341,6 @@ impl CheckoutError {
     }
 }
 
-pub struct SnapshotOptions<'a> {
-    pub base_ignores: Arc<GitIgnoreFile>,
-    pub fsmonitor_kind: Option<FsmonitorKind>,
-    pub progress: Option<&'a SnapshotProgress<'a>>,
-    pub max_new_file_size: u64,
-}
-
-impl SnapshotOptions<'_> {
-    pub fn empty_for_test() -> Self {
-        SnapshotOptions {
-            base_ignores: GitIgnoreFile::empty(),
-            fsmonitor_kind: None,
-            progress: None,
-            max_new_file_size: u64::MAX,
-        }
-    }
-}
-
 struct FsmonitorMatcher {
     matcher: Option<Box<dyn Matcher>>,
     watchman_clock: Option<crate::protos::working_copy::WatchmanClock>,
@@ -1592,6 +1553,18 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy {
     fn old_tree_id(&self) -> &MergedTreeId {
         &self.old_tree_id
     }
+
+    fn snapshot(&mut self, options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
+        let tree_state = self
+            .wc
+            .tree_state_mut()
+            .map_err(|err| SnapshotError::Other {
+                message: "Failed to read the working copy state".to_string(),
+                err: err.into(),
+            })?;
+        self.tree_state_dirty |= tree_state.snapshot(options)?;
+        Ok(tree_state.current_tree_id().clone())
+    }
 }
 
 impl LockedLocalWorkingCopy {
@@ -1607,21 +1580,6 @@ impl LockedLocalWorkingCopy {
         Ok(())
     }
 
-    // The base_ignores are passed in here rather than being set on the TreeState
-    // because the TreeState may be long-lived if the library is used in a
-    // long-lived process.
-    pub fn snapshot(&mut self, options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError> {
-        let tree_state = self
-            .wc
-            .tree_state_mut()
-            .map_err(|err| SnapshotError::Other {
-                message: "Failed to read the working copy state".to_string(),
-                err: err.into(),
-            })?;
-        self.tree_state_dirty |= tree_state.snapshot(options)?;
-        Ok(tree_state.current_tree_id().clone())
-    }
-
     pub fn check_out(&mut self, new_tree: &MergedTree) -> Result<CheckoutStats, CheckoutError> {
         // TODO: Write a "pending_checkout" file with the new TreeId so we can
         // continue an interrupted update if we find such a file.
@@ -1694,5 +1652,3 @@ impl LockedLocalWorkingCopy {
         Ok(self.wc)
     }
 }
-
-pub type SnapshotProgress<'a> = dyn Fn(&RepoPath) + 'a + Sync;

lib/src/working_copy.rs

@@ -16,10 +16,18 @@
 //! default local-disk implementation.
 
 use std::any::Any;
-use std::path::Path;
+use std::ffi::OsString;
+use std::path::{Path, PathBuf};
+use std::sync::Arc;
 
-use crate::backend::MergedTreeId;
+use thiserror::Error;
+
+use crate::backend::{BackendError, MergedTreeId};
+use crate::fsmonitor::FsmonitorKind;
+use crate::gitignore::GitIgnoreFile;
 use crate::op_store::{OperationId, WorkspaceId};
+use crate::repo_path::RepoPath;
+use crate::settings::HumanByteSize;
 
 /// The trait all working-copy implementations must implement.
 pub trait WorkingCopy {
@@ -50,4 +58,88 @@ pub trait LockedWorkingCopy {
 
     /// The tree at the time the lock was taken
     fn old_tree_id(&self) -> &MergedTreeId;
+
+    /// Snapshot the working copy and return the tree id.
+    fn snapshot(&mut self, options: SnapshotOptions) -> Result<MergedTreeId, SnapshotError>;
 }
+
+/// An error while snapshotting the working copy.
+#[derive(Debug, Error)]
+pub enum SnapshotError {
+    /// A path in the working copy was not valid UTF-8.
+    #[error("Working copy path {} is not valid UTF-8", path.to_string_lossy())]
+    InvalidUtf8Path {
+        /// The path with invalid UTF-8.
+        path: OsString,
+    },
+    /// A symlink target in the working copy was not valid UTF-8.
+    #[error("Symlink {path} target is not valid UTF-8")]
+    InvalidUtf8SymlinkTarget {
+        /// The path of the symlink that has a target that's not valid UTF-8.
+        /// This path itself is valid UTF-8.
+        path: PathBuf,
+        /// The symlink target with invalid UTF-8.
+        target: PathBuf,
+    },
+    /// Reading or writing from the commit backend failed.
+    #[error("Internal backend error: {0}")]
+    InternalBackendError(#[from] BackendError),
+    /// A file was larger than the specified maximum file size for new
+    /// (previously untracked) files.
+    #[error("New file {path} of size ~{size} exceeds snapshot.max-new-file-size ({max_size})")]
+    NewFileTooLarge {
+        /// The path of the large file.
+        path: PathBuf,
+        /// The size of the large file.
+        size: HumanByteSize,
+        /// The maximum allowed size.
+        max_size: HumanByteSize,
+    },
+    /// Some other error happened while snapshotting the working copy.
+    #[error("{message}: {err:?}")]
+    Other {
+        /// Error message.
+        message: String,
+        /// The underlying error.
+        #[source]
+        err: Box<dyn std::error::Error + Send + Sync>,
+    },
+}
+
+/// Options used when snapshotting the working copy. Some of them may be ignored
+/// by some `WorkingCopy` implementations.
+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.
+    // The base_ignores are passed in here rather than being set on the TreeState
+    // because the TreeState may be long-lived if the library is used in a
+    // long-lived process.
+    pub base_ignores: Arc<GitIgnoreFile>,
+    /// The fsmonitor (e.g. Watchman) to use, if any.
+    // TODO: Should we make this a field on `LocalWorkingCopy` instead since it's quite specific to
+    // that implementation?
+    pub fsmonitor_kind: Option<FsmonitorKind>,
+    /// A callback for the UI to display progress.
+    pub progress: Option<&'a SnapshotProgress<'a>>,
+    /// The size of the largest file that should be allowed to become tracked
+    /// (already tracked files are always snapshotted). If there are larger
+    /// files in the working copy, then `LockedWorkingCopy::snapshot()` may
+    /// (depending on implementation)
+    /// return `SnapshotError::NewFileTooLarge`.
+    pub max_new_file_size: u64,
+}
+
+impl SnapshotOptions<'_> {
+    /// Create an instance for use in tests.
+    pub fn empty_for_test() -> Self {
+        SnapshotOptions {
+            base_ignores: GitIgnoreFile::empty(),
+            fsmonitor_kind: None,
+            progress: None,
+            max_new_file_size: u64::MAX,
+        }
+    }
+}
+
+/// A callback for getting progress updates.
+pub type SnapshotProgress<'a> = dyn Fn(&RepoPath) + 'a + Sync;

lib/tests/test_local_working_copy.rs

@@ -27,13 +27,14 @@ use std::sync::Arc;
 use itertools::Itertools;
 use jj_lib::backend::{MergedTreeId, ObjectId, TreeId, TreeValue};
 use jj_lib::fsmonitor::FsmonitorKind;
-use jj_lib::local_working_copy::{CheckoutStats, LocalWorkingCopy, SnapshotError, SnapshotOptions};
+use jj_lib::local_working_copy::{CheckoutStats, LocalWorkingCopy};
 use jj_lib::merge::Merge;
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
 use jj_lib::op_store::{OperationId, WorkspaceId};
 use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use jj_lib::settings::UserSettings;
+use jj_lib::working_copy::{LockedWorkingCopy, SnapshotError, SnapshotOptions};
 use jj_lib::workspace::LockedWorkspace;
 use test_case::test_case;
 use testutils::{create_tree, write_random_commit, TestRepoBackend, TestWorkspace};

lib/tests/test_local_working_copy_concurrent.rs

@@ -16,9 +16,10 @@ use std::cmp::max;
 use std::thread;
 
 use assert_matches::assert_matches;
-use jj_lib::local_working_copy::{CheckoutError, SnapshotOptions};
+use jj_lib::local_working_copy::CheckoutError;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
+use jj_lib::working_copy::{LockedWorkingCopy, SnapshotOptions};
 use jj_lib::workspace::Workspace;
 use testutils::{create_tree, write_working_copy_file, TestRepo, TestWorkspace};
 

lib/testutils/src/lib.rs

@@ -23,7 +23,6 @@ use jj_lib::commit::Commit;
 use jj_lib::commit_builder::CommitBuilder;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::local_backend::LocalBackend;
-use jj_lib::local_working_copy::{SnapshotError, SnapshotOptions};
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader, StoreFactories};
 use jj_lib::repo_path::RepoPath;
@@ -33,6 +32,7 @@ use jj_lib::store::Store;
 use jj_lib::transaction::Transaction;
 use jj_lib::tree::Tree;
 use jj_lib::tree_builder::TreeBuilder;
+use jj_lib::working_copy::{LockedWorkingCopy, SnapshotError, SnapshotOptions};
 use jj_lib::workspace::Workspace;
 use tempfile::TempDir;