fsmonitor: allow core.fsmonitor = "none" to disable

When doing things like testing snapshot performance differences, this allows you to turn off the monitor, no matter what the enabled user or repository configuration has, e.g. jj st --config-toml='core.fsmonitor="none"' Signed-off-by: Austin Seipp <[email protected]>
jj-vcs · Feb 21, 2024 · 6c31bab · 6c31bab
1 parent 79518ea
commit 6c31bab
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 13 deletions.
diff --git a/lib/src/fsmonitor.rs b/lib/src/fsmonitor.rs
@@ -24,6 +24,7 @@ use std::path::PathBuf;
 use std::str::FromStr;
 
 /// The recognized kinds of filesystem monitors.
+#[derive(Eq, PartialEq)]
 pub enum FsmonitorKind {
     /// The Watchman filesystem monitor (https://facebook.github.io/watchman/).
     Watchman,
@@ -34,6 +35,13 @@ pub enum FsmonitorKind {
         /// reporting.
         changed_files: Vec<PathBuf>,
     },
+
+    /// No filesystem monitor. This is the default if nothing is configured, but
+    /// also makes it possible to turn off the monitor on a case-by-case basis
+    /// when the user gives an option like
+    /// `--config-toml='core.fsmonitor="none"'`; useful when e.g. when doing
+    /// analysis of snapshot performance.
+    None,
 }
 
 impl FromStr for FsmonitorKind {
@@ -45,6 +53,7 @@ impl FromStr for FsmonitorKind {
             "test" => Err(config::ConfigError::Message(
                 "cannot use test fsmonitor in real repository".to_string(),
             )),
+            "none" => Ok(Self::None),
             other => Err(config::ConfigError::Message(format!(
                 "unknown fsmonitor kind: {}",
                 other

diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs
@@ -744,7 +744,7 @@ impl TreeState {
 
         let sparse_matcher = self.sparse_matcher();
 
-        let fsmonitor_clock_needs_save = fsmonitor_kind.is_some();
+        let fsmonitor_clock_needs_save = fsmonitor_kind != FsmonitorKind::None;
         let mut is_dirty = fsmonitor_clock_needs_save;
         let FsmonitorMatcher {
             matcher: fsmonitor_matcher,
@@ -1017,21 +1017,21 @@ impl TreeState {
     #[instrument(skip_all)]
     fn make_fsmonitor_matcher(
         &self,
-        fsmonitor_kind: Option<FsmonitorKind>,
+        fsmonitor_kind: FsmonitorKind,
     ) -> Result<FsmonitorMatcher, SnapshotError> {
         let (watchman_clock, changed_files) = match fsmonitor_kind {
-            None => (None, None),
-            Some(FsmonitorKind::Test { changed_files }) => (None, Some(changed_files)),
+            FsmonitorKind::None => (None, None),
+            FsmonitorKind::Test { changed_files } => (None, Some(changed_files)),
             #[cfg(feature = "watchman")]
-            Some(FsmonitorKind::Watchman) => match self.query_watchman() {
+            FsmonitorKind::Watchman => match self.query_watchman() {
                 Ok((watchman_clock, changed_files)) => (Some(watchman_clock.into()), changed_files),
                 Err(err) => {
                     tracing::warn!(?err, "Failed to query filesystem monitor");
                     (None, None)
                 }
             },
             #[cfg(not(feature = "watchman"))]
-            Some(FsmonitorKind::Watchman) => {
+            FsmonitorKind::Watchman => {
                 return Err(SnapshotError::Other {
                     message: "Failed to query the filesystem monitor".to_string(),
                     err: "Cannot query Watchman because jj was not compiled with the `watchman` \

diff --git a/lib/src/settings.rs b/lib/src/settings.rs
@@ -164,10 +164,10 @@ impl UserSettings {
         self.config.get_string("user.email").unwrap_or_default()
     }
 
-    pub fn fsmonitor_kind(&self) -> Result<Option<FsmonitorKind>, config::ConfigError> {
+    pub fn fsmonitor_kind(&self) -> Result<FsmonitorKind, config::ConfigError> {
         match self.config.get_string("core.fsmonitor") {
-            Ok(fsmonitor_kind) => Ok(Some(fsmonitor_kind.parse()?)),
-            Err(config::ConfigError::NotFound(_)) => Ok(None),
+            Ok(fsmonitor_kind) => Ok(fsmonitor_kind.parse()?),
+            Err(config::ConfigError::NotFound(_)) => Ok(FsmonitorKind::None),
             Err(err) => Err(err),
         }
     }

diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs
@@ -188,7 +188,7 @@ pub struct SnapshotOptions<'a> {
     /// 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>,
+    pub fsmonitor_kind: 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
@@ -204,7 +204,7 @@ impl SnapshotOptions<'_> {
     pub fn empty_for_test() -> Self {
         SnapshotOptions {
             base_ignores: GitIgnoreFile::empty(),
-            fsmonitor_kind: None,
+            fsmonitor_kind: FsmonitorKind::None,
             progress: None,
             max_new_file_size: u64::MAX,
         }

diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs
@@ -976,9 +976,9 @@ fn test_fsmonitor() {
         locked_ws
             .locked_wc()
             .snapshot(SnapshotOptions {
-                fsmonitor_kind: Some(FsmonitorKind::Test {
+                fsmonitor_kind: FsmonitorKind::Test {
                     changed_files: fs_paths,
-                }),
+                },
                 ..SnapshotOptions::empty_for_test()
             })
             .unwrap()