feat: add an option to monitor the filesystem asynchronously

- make an internal set of watchman extensions until the client api gets
  updates with triggers
- add a config option to enable using triggers in watchman

Co-authored-by: Waleed Khan <[email protected]>

CHANGELOG.md

@@ -25,6 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### New features
 
+* Support background filesystem monitoring via watchman triggers enabled with
+  the `core.watchman.register_trigger = true` config.
+
 * Show paths to config files when configuration errors occur
 
 * `jj fix` now supports configuring the default revset for `-s` using the

cli/src/cli_util.rs

@@ -1222,7 +1222,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
         let progress = crate::progress::snapshot_progress(ui);
         let new_tree_id = locked_ws.locked_wc().snapshot(SnapshotOptions {
             base_ignores,
-            fsmonitor_kind: self.settings.fsmonitor_kind()?,
+            fsmonitor_settings: self.settings.fsmonitor_settings()?,
             progress: progress.as_ref().map(|x| x as _),
             max_new_file_size: self.settings.max_new_file_size()?,
         })?;

cli/src/commands/debug.rs

@@ -19,6 +19,7 @@ use std::io::Write as _;
 use clap::Subcommand;
 use jj_lib::backend::TreeId;
 use jj_lib::default_index::{AsCompositeIndex as _, DefaultIndexStore, DefaultReadonlyIndex};
+use jj_lib::fsmonitor::{FsmonitorSettings, WatchmanConfig};
 use jj_lib::local_working_copy::LocalWorkingCopy;
 use jj_lib::merged_tree::MergedTree;
 use jj_lib::object_id::ObjectId;
@@ -374,11 +375,11 @@ fn cmd_debug_watchman(
     match subcommand {
         DebugWatchmanSubcommand::Status => {
             // TODO(ilyagr): It would be nice to add colors here
-            match command.settings().fsmonitor_kind()? {
-                jj_lib::fsmonitor::FsmonitorKind::Watchman => {
+            match command.settings().fsmonitor_settings()? {
+                FsmonitorSettings::Watchman { .. } => {
                     writeln!(ui.stdout(), "Watchman is enabled via `core.fsmonitor`.")?
                 }
-                jj_lib::fsmonitor::FsmonitorKind::None => writeln!(
+                FsmonitorSettings::None => writeln!(
                     ui.stdout(),
                     "Watchman is disabled. Set `core.fsmonitor=\"watchman\"` to \
                      enable.\nAttempting to contact the `watchman` CLI regardless..."
@@ -391,20 +392,20 @@ fn cmd_debug_watchman(
                 }
             };
             let wc = check_local_disk_wc(workspace_command.working_copy().as_any())?;
-            let _ = wc.query_watchman()?;
+            let _ = wc.query_watchman(&WatchmanConfig::default())?;
             writeln!(
                 ui.stdout(),
                 "The watchman server seems to be installed and working correctly."
             )?;
         }
         DebugWatchmanSubcommand::QueryClock => {
             let wc = check_local_disk_wc(workspace_command.working_copy().as_any())?;
-            let (clock, _changed_files) = wc.query_watchman()?;
+            let (clock, _changed_files) = wc.query_watchman(&WatchmanConfig::default())?;
             writeln!(ui.stdout(), "Clock: {clock:?}")?;
         }
         DebugWatchmanSubcommand::QueryChangedFiles => {
             let wc = check_local_disk_wc(workspace_command.working_copy().as_any())?;
-            let (_clock, changed_files) = wc.query_watchman()?;
+            let (_clock, changed_files) = wc.query_watchman(&WatchmanConfig::default())?;
             writeln!(ui.stdout(), "Changed files: {changed_files:?}")?;
         }
         DebugWatchmanSubcommand::ResetClock => {

cli/src/commands/untrack.rs

@@ -69,7 +69,7 @@ pub(crate) fn cmd_untrack(
     // untracked because they're not ignored.
     let wc_tree_id = locked_ws.locked_wc().snapshot(SnapshotOptions {
         base_ignores,
-        fsmonitor_kind: command.settings().fsmonitor_kind()?,
+        fsmonitor_settings: command.settings().fsmonitor_settings()?,
         progress: None,
         max_new_file_size: command.settings().max_new_file_size()?,
     })?;

cli/src/config-schema.json

@@ -168,6 +168,16 @@
                     "type": "string",
                     "enum": ["none", "watchman"],
                     "description": "Whether to use an external filesystem monitor, useful for large repos"
+                },
+                "watchman": {
+                    "type": "object",
+                    "properties": {
+                      "register_trigger": {
+                        "type": "boolean",
+                        "default": false,
+                        "description": "Whether to use triggers to monitor for changes in the background."
+                      }
+                    }
                 }
             }
         },

cli/src/merge_tools/diff_working_copies.rs

@@ -6,7 +6,7 @@ use std::sync::Arc;
 
 use futures::StreamExt;
 use jj_lib::backend::MergedTreeId;
-use jj_lib::fsmonitor::FsmonitorKind;
+use jj_lib::fsmonitor::FsmonitorSettings;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::local_working_copy::{TreeState, TreeStateError};
 use jj_lib::matchers::Matcher;
@@ -279,7 +279,7 @@ diff editing in mind and be a little inaccurate.
             .unwrap_or(diff_wc.right_tree_state);
         output_tree_state.snapshot(SnapshotOptions {
             base_ignores,
-            fsmonitor_kind: FsmonitorKind::None,
+            fsmonitor_settings: FsmonitorSettings::None,
             progress: None,
             max_new_file_size: u64::MAX,
         })?;

docs/config.md

@@ -800,6 +800,9 @@ To configure the Watchman filesystem monitor, set
 `core.fsmonitor = "watchman"`. Ensure that you have [installed the Watchman
 executable on your system](https://facebook.github.io/watchman/docs/install).
 
+You can configure `jj` to use watchman triggers to automatically create
+snapshots on filestem changes by setting `core.watchman.register_trigger = true`.
+
 You can check whether Watchman is enabled and whether it is installed correctly
 using `jj debug watchman status`.
 

lib/src/fsmonitor.rs

@@ -21,13 +21,23 @@
 #![warn(missing_docs)]
 
 use std::path::PathBuf;
-use std::str::FromStr;
+
+use config::{Config, ConfigError};
+
+use crate::settings::ConfigResultExt;
+
+/// Config for Watchman filesystem monitor (<https://facebook.github.io/watchman/>).
+#[derive(Default, Eq, PartialEq, Clone, Debug)]
+pub struct WatchmanConfig {
+    /// Whether to use triggers to monitor for changes in the background.
+    register_trigger: bool,
+}
 
 /// The recognized kinds of filesystem monitors.
 #[derive(Eq, PartialEq, Clone, Debug)]
-pub enum FsmonitorKind {
+pub enum FsmonitorSettings {
     /// The Watchman filesystem monitor (<https://facebook.github.io/watchman/>).
-    Watchman,
+    Watchman(WatchmanConfig),
 
     /// Only used in tests.
     Test {
@@ -44,20 +54,27 @@ pub enum FsmonitorKind {
     None,
 }
 
-impl FromStr for FsmonitorKind {
-    type Err = config::ConfigError;
-
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s {
-            "watchman" => Ok(Self::Watchman),
-            "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
-            ))),
+impl FsmonitorSettings {
+    /// Creates an `FsmonitorSettings` from a `config`.
+    pub fn from_config(config: &Config) -> Result<FsmonitorSettings, ConfigError> {
+        match config.get_string("core.fsmonitor") {
+            Ok(s) => match s.as_str() {
+                "watchman" => Ok(Self::Watchman(WatchmanConfig {
+                    register_trigger: config
+                        .get_bool("core.watchman.register_trigger")
+                        .optional()?
+                        .unwrap_or_default(),
+                })),
+                "test" => Err(ConfigError::Message(
+                    "cannot use test fsmonitor in real repository".to_string(),
+                )),
+                "none" => Ok(Self::None),
+                other => Err(ConfigError::Message(format!(
+                    "unknown fsmonitor kind: {other}",
+                ))),
+            },
+            Err(ConfigError::NotFound(_)) => Ok(Self::None),
+            Err(err) => Err(err),
         }
     }
 }
@@ -77,6 +94,10 @@ pub mod watchman {
         Clock as InnerClock, ClockSpec, NameOnly, QueryRequestCommon, QueryResult,
     };
 
+    use crate::fsmonitor_watchman_extensions::{
+        list_triggers, register_trigger, remove_trigger, TriggerRequest,
+    };
+
     /// Represents an instance in time from the perspective of the filesystem
     /// monitor.
     ///
@@ -139,6 +160,9 @@ pub mod watchman {
 
         #[error("Failed to query Watchman")]
         WatchmanQueryError(#[source] watchman_client::Error),
+
+        #[error("Failed to register Watchman trigger")]
+        WatchmanTriggerError(#[source] watchman_client::Error),
     }
 
     /// Handle to the underlying Watchman instance.
@@ -153,7 +177,10 @@ pub mod watchman {
         /// copy to build up its in-memory representation of the
         /// filesystem, which may take some time.
         #[instrument]
-        pub async fn init(working_copy_path: &Path) -> Result<Self, Error> {
+        pub async fn init(
+            working_copy_path: &Path,
+            config: &super::WatchmanConfig,
+        ) -> Result<Self, Error> {
             info!("Initializing Watchman filesystem monitor...");
             let connector = watchman_client::Connector::new();
             let client = connector
@@ -166,10 +193,20 @@ pub mod watchman {
                 .resolve_root(working_copy_root)
                 .await
                 .map_err(Error::ResolveRootError)?;
-            Ok(Fsmonitor {
+
+            let monitor = Fsmonitor {
                 client,
                 resolved_root,
-            })
+            };
+
+            // Registering the trigger causes an unconditional evaluation of the query, so
+            // test if it is already registered first.
+            if !config.register_trigger {
+                monitor.unregister_trigger().await?;
+            } else if !monitor.is_trigger_registered().await? {
+                monitor.register_trigger().await?;
+            }
+            Ok(monitor)
         }
 
         /// Query for changed files since the previous point in time.
@@ -184,24 +221,6 @@ pub mod watchman {
         ) -> Result<(Clock, Option<Vec<PathBuf>>), Error> {
             // TODO: might be better to specify query options by caller, but we
             // shouldn't expose the underlying watchman API too much.
-            let exclude_dirs = [Path::new(".git"), Path::new(".jj")];
-            let excludes = itertools::chain(
-                // the directories themselves
-                [expr::Expr::Name(expr::NameTerm {
-                    paths: exclude_dirs.iter().map(|&name| name.to_owned()).collect(),
-                    wholename: true,
-                })],
-                // and all files under the directories
-                exclude_dirs.iter().map(|&name| {
-                    expr::Expr::DirName(expr::DirNameTerm {
-                        path: name.to_owned(),
-                        depth: None,
-                    })
-                }),
-            )
-            .collect();
-            let expression = expr::Expr::Not(Box::new(expr::Expr::Any(excludes)));
-
             info!("Querying Watchman for changed files...");
             let QueryResult {
                 version: _,
@@ -219,7 +238,7 @@ pub mod watchman {
                     &self.resolved_root,
                     QueryRequestCommon {
                         since: previous_clock.map(|Clock(clock)| clock),
-                        expression: Some(expression),
+                        expression: Some(self.build_exclude_expr()),
                         ..Default::default()
                     },
                 )
@@ -242,5 +261,73 @@ pub mod watchman {
                 Ok((clock, Some(paths)))
             }
         }
+
+        /// Return whether or not a trigger has been registered already.
+        #[instrument(skip(self))]
+        async fn is_trigger_registered(&self) -> Result<bool, Error> {
+            info!("Checking for an existing Watchman trigger...");
+            Ok(list_triggers(&self.client, &self.resolved_root)
+                .await
+                .map_err(Error::WatchmanTriggerError)?
+                .triggers
+                .iter()
+                .any(|t| t.name == "jj-background-monitor"))
+        }
+
+        /// Register trigger for changed files.
+        #[instrument(skip(self))]
+        async fn register_trigger(&self) -> Result<(), Error> {
+            info!("Registering Watchman trigger...");
+            register_trigger(
+                &self.client,
+                &self.resolved_root,
+                TriggerRequest {
+                    name: "jj-background-monitor".to_string(),
+                    command: vec![
+                        "jj".to_string(),
+                        "files".to_string(),
+                        "-r".to_string(),
+                        "root()".to_string(),
+                    ],
+                    expression: Some(self.build_exclude_expr()),
+                    ..Default::default()
+                },
+            )
+            .await
+            .map_err(Error::WatchmanTriggerError)?;
+            Ok(())
+        }
+
+        /// Register trigger for changed files.
+        #[instrument(skip(self))]
+        async fn unregister_trigger(&self) -> Result<(), Error> {
+            info!("Unregistering Watchman trigger...");
+            remove_trigger(&self.client, &self.resolved_root, "jj-background-monitor")
+                .await
+                .map_err(Error::WatchmanTriggerError)?;
+            Ok(())
+        }
+
+        /// Build an exclude expr for `working_copy_path`.
+        fn build_exclude_expr(&self) -> expr::Expr {
+            // TODO: consider parsing `.gitignore`.
+            let exclude_dirs = [Path::new(".git"), Path::new(".jj")];
+            let excludes = itertools::chain(
+                // the directories themselves
+                [expr::Expr::Name(expr::NameTerm {
+                    paths: exclude_dirs.iter().map(|&name| name.to_owned()).collect(),
+                    wholename: true,
+                })],
+                // and all files under the directories
+                exclude_dirs.iter().map(|&name| {
+                    expr::Expr::DirName(expr::DirNameTerm {
+                        path: name.to_owned(),
+                        depth: None,
+                    })
+                }),
+            )
+            .collect();
+            expr::Expr::Not(Box::new(expr::Expr::Any(excludes)))
+        }
     }
 }