From f9c4ee1c7243813a11e009a88d9ad8a68f41eb71 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Tue, 10 Dec 2024 17:10:31 +0900 Subject: [PATCH] working_copy: plumbing to propagate untracked paths to caller --- cli/examples/custom-working-copy/main.rs | 6 +++- cli/src/cli_util.rs | 3 +- cli/src/commands/file/track.rs | 3 +- cli/src/commands/file/untrack.rs | 3 +- lib/src/local_working_copy.rs | 28 +++++++++++++++---- lib/src/working_copy.rs | 27 ++++++++++++++++-- lib/tests/test_local_working_copy.rs | 9 +++--- .../test_local_working_copy_concurrent.rs | 2 +- lib/testutils/src/lib.rs | 10 ++++--- 9 files changed, 70 insertions(+), 21 deletions(-) diff --git a/cli/examples/custom-working-copy/main.rs b/cli/examples/custom-working-copy/main.rs index 7ddd5011d3..f0830a32c7 100644 --- a/cli/examples/custom-working-copy/main.rs +++ b/cli/examples/custom-working-copy/main.rs @@ -41,6 +41,7 @@ use jj_lib::working_copy::LockedWorkingCopy; use jj_lib::working_copy::ResetError; use jj_lib::working_copy::SnapshotError; use jj_lib::working_copy::SnapshotOptions; +use jj_lib::working_copy::SnapshotStats; use jj_lib::working_copy::WorkingCopy; use jj_lib::working_copy::WorkingCopyFactory; use jj_lib::working_copy::WorkingCopyStateError; @@ -233,7 +234,10 @@ impl LockedWorkingCopy for LockedConflictsWorkingCopy { self.inner.old_tree_id() } - fn snapshot(&mut self, options: &SnapshotOptions) -> Result { + fn snapshot( + &mut self, + options: &SnapshotOptions, + ) -> Result<(MergedTreeId, SnapshotStats), SnapshotError> { let options = SnapshotOptions { base_ignores: options.base_ignores.chain("", "/.conflicts".as_bytes())?, ..options.clone() diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 358b2e539d..7a2c3e6ed4 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1806,7 +1806,8 @@ 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 + // TODO: print stats + let (new_tree_id, _stats) = locked_ws .locked_wc() .snapshot(&SnapshotOptions { base_ignores, diff --git a/cli/src/commands/file/track.rs b/cli/src/commands/file/track.rs index e5d272902a..8f5d944069 100644 --- a/cli/src/commands/file/track.rs +++ b/cli/src/commands/file/track.rs @@ -52,7 +52,8 @@ pub(crate) fn cmd_file_track( let mut tx = workspace_command.start_transaction().into_inner(); let base_ignores = workspace_command.base_ignores()?; let (mut locked_ws, _wc_commit) = workspace_command.start_working_copy_mutation()?; - locked_ws.locked_wc().snapshot(&SnapshotOptions { + // TODO: print stats + let (_tree_id, _stats) = locked_ws.locked_wc().snapshot(&SnapshotOptions { base_ignores, fsmonitor_settings: command.settings().fsmonitor_settings()?, progress: None, diff --git a/cli/src/commands/file/untrack.rs b/cli/src/commands/file/untrack.rs index f2cff45fb0..743ab6417a 100644 --- a/cli/src/commands/file/untrack.rs +++ b/cli/src/commands/file/untrack.rs @@ -76,7 +76,8 @@ 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 { + // TODO: print stats + let (wc_tree_id, _stats) = locked_ws.locked_wc().snapshot(&SnapshotOptions { base_ignores, fsmonitor_settings: command.settings().fsmonitor_settings()?, progress: None, diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 177176cd74..c7fd2349db 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -108,6 +108,8 @@ use crate::working_copy::ResetError; use crate::working_copy::SnapshotError; use crate::working_copy::SnapshotOptions; use crate::working_copy::SnapshotProgress; +use crate::working_copy::SnapshotStats; +use crate::working_copy::UntrackedReason; use crate::working_copy::WorkingCopy; use crate::working_copy::WorkingCopyFactory; use crate::working_copy::WorkingCopyStateError; @@ -908,7 +910,10 @@ impl TreeState { /// Look for changes to the working copy. If there are any changes, create /// a new tree from it. #[instrument(skip_all)] - pub fn snapshot(&mut self, options: &SnapshotOptions) -> Result { + pub fn snapshot( + &mut self, + options: &SnapshotOptions, + ) -> Result<(bool, SnapshotStats), SnapshotError> { let &SnapshotOptions { ref base_ignores, ref fsmonitor_settings, @@ -935,11 +940,12 @@ impl TreeState { if matcher.visit(RepoPath::root()).is_nothing() { // No need to load the current tree, set up channels, etc. self.watchman_clock = watchman_clock; - return Ok(is_dirty); + return Ok((is_dirty, SnapshotStats::default())); } let (tree_entries_tx, tree_entries_rx) = channel(); let (file_states_tx, file_states_rx) = channel(); + let (untracked_paths_tx, untracked_paths_rx) = channel(); let (deleted_files_tx, deleted_files_rx) = channel(); trace_span!("traverse filesystem").in_scope(|| -> Result<(), SnapshotError> { @@ -951,6 +957,7 @@ impl TreeState { // Move tx sides so they'll be dropped at the end of the scope. tree_entries_tx, file_states_tx, + untracked_paths_tx, deleted_files_tx, error: OnceLock::new(), progress, @@ -972,6 +979,9 @@ impl TreeState { snapshotter.into_result() })?; + let stats = SnapshotStats { + untracked_paths: untracked_paths_rx.into_iter().collect(), + }; let mut tree_builder = MergedTreeBuilder::new(self.tree_id.clone()); trace_span!("process tree entries").in_scope(|| { for (path, tree_values) in &tree_entries_rx { @@ -1011,7 +1021,7 @@ impl TreeState { assert_eq!(state_paths, tree_paths); } self.watchman_clock = watchman_clock; - Ok(is_dirty) + Ok((is_dirty, stats)) } #[instrument(skip_all)] @@ -1087,6 +1097,8 @@ struct FileSnapshotter<'a> { start_tracking_matcher: &'a dyn Matcher, tree_entries_tx: Sender<(RepoPathBuf, MergedTreeValue)>, file_states_tx: Sender<(RepoPathBuf, FileState)>, + #[allow(unused)] // TODO + untracked_paths_tx: Sender<(RepoPathBuf, UntrackedReason)>, deleted_files_tx: Sender, error: OnceLock, progress: Option<&'a SnapshotProgress<'a>>, @@ -2150,7 +2162,10 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { &self.old_tree_id } - fn snapshot(&mut self, options: &SnapshotOptions) -> Result { + fn snapshot( + &mut self, + options: &SnapshotOptions, + ) -> Result<(MergedTreeId, SnapshotStats), SnapshotError> { let tree_state = self .wc .tree_state_mut() @@ -2158,8 +2173,9 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { 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()) + let (is_dirty, stats) = tree_state.snapshot(options)?; + self.tree_state_dirty |= is_dirty; + Ok((tree_state.current_tree_id().clone(), stats)) } fn check_out( diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 4f04190268..8efea0bc5d 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -16,6 +16,7 @@ //! default local-disk implementation. use std::any::Any; +use std::collections::BTreeMap; use std::ffi::OsString; use std::path::PathBuf; use std::sync::Arc; @@ -113,8 +114,11 @@ 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; + /// Snapshot the working copy. Returns the tree id and stats. + fn snapshot( + &mut self, + options: &SnapshotOptions, + ) -> Result<(MergedTreeId, SnapshotStats), SnapshotError>; /// Check out the specified commit in the working copy. fn check_out( @@ -249,6 +253,25 @@ impl SnapshotOptions<'_> { /// A callback for getting progress updates. pub type SnapshotProgress<'a> = dyn Fn(&RepoPath) + 'a + Sync; +/// Stats about a snapshot operation on a working copy. +#[derive(Clone, Debug, Default)] +pub struct SnapshotStats { + /// List of new (previously untracked) files which are still untracked. + pub untracked_paths: BTreeMap, +} + +/// Reason why the new path isn't tracked. +#[derive(Clone, Debug)] +pub enum UntrackedReason { + /// File was larger than the specified maximum file size. + FileTooLarge { + /// Actual size of the large file. + size: u64, + /// Maximum allowed size. + max_size: u64, + }, +} + /// Options used when checking out a tree in the working copy. #[derive(Clone)] pub struct CheckoutOptions { diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index 610de0ed96..67cc017f95 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -939,7 +939,7 @@ fn test_snapshot_racy_timestamps() { .workspace .start_working_copy_mutation() .unwrap(); - let new_tree_id = locked_ws + let (new_tree_id, _stats) = locked_ws .locked_wc() .snapshot(&SnapshotOptions::empty_for_test()) .unwrap(); @@ -973,7 +973,7 @@ fn test_snapshot_special_file() { // Snapshot the working copy with the socket file let mut locked_ws = ws.start_working_copy_mutation().unwrap(); - let tree_id = locked_ws + let (tree_id, _stats) = locked_ws .locked_wc() .snapshot(&SnapshotOptions::empty_for_test()) .unwrap(); @@ -2052,7 +2052,7 @@ fn test_fsmonitor() { .iter() .map(|p| p.to_fs_path_unchecked(Path::new(""))) .collect(); - locked_ws + let (tree_id, _stats) = locked_ws .locked_wc() .snapshot(&SnapshotOptions { fsmonitor_settings: FsmonitorSettings::Test { @@ -2060,7 +2060,8 @@ fn test_fsmonitor() { }, ..SnapshotOptions::empty_for_test() }) - .unwrap() + .unwrap(); + tree_id }; { diff --git a/lib/tests/test_local_working_copy_concurrent.rs b/lib/tests/test_local_working_copy_concurrent.rs index a89cb2bf92..db5713acf4 100644 --- a/lib/tests/test_local_working_copy_concurrent.rs +++ b/lib/tests/test_local_working_copy_concurrent.rs @@ -169,7 +169,7 @@ fn test_checkout_parallel() { // &CheckoutOptions::empty_for_test()) should never produce a // different tree. let mut locked_ws = workspace.start_working_copy_mutation().unwrap(); - let new_tree_id = locked_ws + let (new_tree_id, _stats) = locked_ws .locked_wc() .snapshot(&SnapshotOptions::empty_for_test()) .unwrap(); diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index c05e5405d3..2dadf568e6 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -60,6 +60,7 @@ use jj_lib::tree::Tree; use jj_lib::tree_builder::TreeBuilder; use jj_lib::working_copy::SnapshotError; use jj_lib::working_copy::SnapshotOptions; +use jj_lib::working_copy::SnapshotStats; use jj_lib::workspace::Workspace; use pollster::FutureExt; use tempfile::TempDir; @@ -302,17 +303,18 @@ impl TestWorkspace { pub fn snapshot_with_options( &mut self, options: &SnapshotOptions, - ) -> Result { + ) -> Result<(MergedTree, SnapshotStats), SnapshotError> { let mut locked_ws = self.workspace.start_working_copy_mutation().unwrap(); - let tree_id = locked_ws.locked_wc().snapshot(options)?; + let (tree_id, stats) = 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()) + Ok((self.repo.store().get_root_tree(&tree_id).unwrap(), stats)) } /// Like `snapshot_with_option()` but with default options pub fn snapshot(&mut self) -> Result { - self.snapshot_with_options(&SnapshotOptions::empty_for_test()) + let (tree_id, _stats) = self.snapshot_with_options(&SnapshotOptions::empty_for_test())?; + Ok(tree_id) } }