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 <me@waleedkhan.name>
This commit is contained in:
Matt Kulukundis 2024-06-15 16:50:37 -04:00 committed by Matt Fowles Kulukundis
parent 4d08c2cce8
commit 8aa71f58f3
14 changed files with 364 additions and 73 deletions

View file

@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### New features ### 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 * Show paths to config files when configuration errors occur
* `jj fix` now supports configuring the default revset for `-s` using the * `jj fix` now supports configuring the default revset for `-s` using the

View file

@ -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 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, base_ignores,
fsmonitor_kind: self.settings.fsmonitor_kind()?, fsmonitor_settings: self.settings.fsmonitor_settings()?,
progress: progress.as_ref().map(|x| x as _), progress: progress.as_ref().map(|x| x as _),
max_new_file_size: self.settings.max_new_file_size()?, max_new_file_size: self.settings.max_new_file_size()?,
})?; })?;

View file

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

View file

@ -69,7 +69,7 @@ pub(crate) fn cmd_untrack(
// untracked because they're not ignored. // 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, base_ignores,
fsmonitor_kind: command.settings().fsmonitor_kind()?, fsmonitor_settings: command.settings().fsmonitor_settings()?,
progress: None, progress: None,
max_new_file_size: command.settings().max_new_file_size()?, max_new_file_size: command.settings().max_new_file_size()?,
})?; })?;

View file

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

View file

@ -6,7 +6,7 @@ use std::sync::Arc;
use futures::StreamExt; use futures::StreamExt;
use jj_lib::backend::MergedTreeId; use jj_lib::backend::MergedTreeId;
use jj_lib::fsmonitor::FsmonitorKind; use jj_lib::fsmonitor::FsmonitorSettings;
use jj_lib::gitignore::GitIgnoreFile; use jj_lib::gitignore::GitIgnoreFile;
use jj_lib::local_working_copy::{TreeState, TreeStateError}; use jj_lib::local_working_copy::{TreeState, TreeStateError};
use jj_lib::matchers::Matcher; 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); .unwrap_or(diff_wc.right_tree_state);
output_tree_state.snapshot(SnapshotOptions { output_tree_state.snapshot(SnapshotOptions {
base_ignores, base_ignores,
fsmonitor_kind: FsmonitorKind::None, fsmonitor_settings: FsmonitorSettings::None,
progress: None, progress: None,
max_new_file_size: u64::MAX, max_new_file_size: u64::MAX,
})?; })?;

View file

@ -800,6 +800,10 @@ To configure the Watchman filesystem monitor, set
`core.fsmonitor = "watchman"`. Ensure that you have [installed the Watchman `core.fsmonitor = "watchman"`. Ensure that you have [installed the Watchman
executable on your system](https://facebook.github.io/watchman/docs/install). 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_snapshot_trigger = true`.
You can check whether Watchman is enabled and whether it is installed correctly You can check whether Watchman is enabled and whether it is installed correctly
using `jj debug watchman status`. using `jj debug watchman status`.

View file

@ -21,13 +21,23 @@
#![warn(missing_docs)] #![warn(missing_docs)]
use std::path::PathBuf; 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. /// The recognized kinds of filesystem monitors.
#[derive(Eq, PartialEq, Clone, Debug)] #[derive(Eq, PartialEq, Clone, Debug)]
pub enum FsmonitorKind { pub enum FsmonitorSettings {
/// The Watchman filesystem monitor (<https://facebook.github.io/watchman/>). /// The Watchman filesystem monitor (<https://facebook.github.io/watchman/>).
Watchman, Watchman(WatchmanConfig),
/// Only used in tests. /// Only used in tests.
Test { Test {
@ -44,20 +54,27 @@ pub enum FsmonitorKind {
None, None,
} }
impl FromStr for FsmonitorKind { impl FsmonitorSettings {
type Err = config::ConfigError; /// Creates an `FsmonitorSettings` from a `config`.
pub fn from_config(config: &Config) -> Result<FsmonitorSettings, ConfigError> {
fn from_str(s: &str) -> Result<Self, Self::Err> { match config.get_string("core.fsmonitor") {
match s { Ok(s) => match s.as_str() {
"watchman" => Ok(Self::Watchman), "watchman" => Ok(Self::Watchman(WatchmanConfig {
"test" => Err(config::ConfigError::Message( register_trigger: config
.get_bool("core.watchman.register_snapshot_trigger")
.optional()?
.unwrap_or_default(),
})),
"test" => Err(ConfigError::Message(
"cannot use test fsmonitor in real repository".to_string(), "cannot use test fsmonitor in real repository".to_string(),
)), )),
"none" => Ok(Self::None), "none" => Ok(Self::None),
other => Err(config::ConfigError::Message(format!( other => Err(ConfigError::Message(format!(
"unknown fsmonitor kind: {}", "unknown fsmonitor kind: {other}",
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, 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 /// Represents an instance in time from the perspective of the filesystem
/// monitor. /// monitor.
/// ///
@ -139,6 +160,9 @@ pub mod watchman {
#[error("Failed to query Watchman")] #[error("Failed to query Watchman")]
WatchmanQueryError(#[source] watchman_client::Error), WatchmanQueryError(#[source] watchman_client::Error),
#[error("Failed to register Watchman trigger")]
WatchmanTriggerError(#[source] watchman_client::Error),
} }
/// Handle to the underlying Watchman instance. /// Handle to the underlying Watchman instance.
@ -153,7 +177,10 @@ pub mod watchman {
/// copy to build up its in-memory representation of the /// copy to build up its in-memory representation of the
/// filesystem, which may take some time. /// filesystem, which may take some time.
#[instrument] #[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..."); info!("Initializing Watchman filesystem monitor...");
let connector = watchman_client::Connector::new(); let connector = watchman_client::Connector::new();
let client = connector let client = connector
@ -166,10 +193,20 @@ pub mod watchman {
.resolve_root(working_copy_root) .resolve_root(working_copy_root)
.await .await
.map_err(Error::ResolveRootError)?; .map_err(Error::ResolveRootError)?;
Ok(Fsmonitor {
let monitor = Fsmonitor {
client, client,
resolved_root, 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. /// Query for changed files since the previous point in time.
@ -184,24 +221,6 @@ pub mod watchman {
) -> Result<(Clock, Option<Vec<PathBuf>>), Error> { ) -> Result<(Clock, Option<Vec<PathBuf>>), Error> {
// TODO: might be better to specify query options by caller, but we // TODO: might be better to specify query options by caller, but we
// shouldn't expose the underlying watchman API too much. // 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..."); info!("Querying Watchman for changed files...");
let QueryResult { let QueryResult {
version: _, version: _,
@ -219,7 +238,7 @@ pub mod watchman {
&self.resolved_root, &self.resolved_root,
QueryRequestCommon { QueryRequestCommon {
since: previous_clock.map(|Clock(clock)| clock), since: previous_clock.map(|Clock(clock)| clock),
expression: Some(expression), expression: Some(self.build_exclude_expr()),
..Default::default() ..Default::default()
}, },
) )
@ -242,5 +261,73 @@ pub mod watchman {
Ok((clock, Some(paths))) 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)))
}
} }
} }

View file

@ -0,0 +1,186 @@
// Copyright 2024 The Jujutsu Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// TODO: remove this file after watchman adopts and releases it.
// https://github.com/facebook/watchman/pull/1221
#![allow(missing_docs)]
use std::path::PathBuf;
use serde::{Deserialize, Serialize, Serializer};
use watchman_client::expr::Expr;
use watchman_client::{Client, Error, ResolvedRoot};
/// Registers a trigger.
pub async fn register_trigger(
client: &Client,
root: &ResolvedRoot,
request: TriggerRequest,
) -> Result<TriggerResponse, Error> {
let response: TriggerResponse = client
.generic_request(TriggerCommand(
"trigger",
root.project_root().to_path_buf(),
request,
))
.await?;
Ok(response)
}
/// Removes a registered trigger.
pub async fn remove_trigger(
client: &Client,
root: &ResolvedRoot,
name: &str,
) -> Result<TriggerDelResponse, Error> {
let response: TriggerDelResponse = client
.generic_request(TriggerDelCommand(
"trigger-del",
root.project_root().to_path_buf(),
name.into(),
))
.await?;
Ok(response)
}
/// Lists registered triggers.
pub async fn list_triggers(
client: &Client,
root: &ResolvedRoot,
) -> Result<TriggerListResponse, Error> {
let response: TriggerListResponse = client
.generic_request(TriggerListCommand(
"trigger-list",
root.project_root().to_path_buf(),
))
.await?;
Ok(response)
}
/// The `trigger` command request.
///
/// The fields are explained in detail here:
/// <https://facebook.github.io/watchman/docs/cmd/trigger#extended-syntax>
#[derive(Deserialize, Serialize, Default, Clone, Debug)]
pub struct TriggerRequest {
/// Defines the name of the trigger.
pub name: String,
/// Specifies the command to invoke.
pub command: Vec<String>,
/// It true, matching files (up to system limits) will be added to the
/// command's execution args.
#[serde(default, skip_serializing_if = "is_false")]
pub append_files: bool,
/// Specifies the expression used to filter candidate matches.
#[serde(skip_serializing_if = "Option::is_none", skip_deserializing)]
pub expression: Option<Expr>,
/// Configure the way `stdin` is configured for the executed trigger.
#[serde(
default,
skip_serializing_if = "TriggerStdinConfig::is_devnull",
serialize_with = "TriggerStdinConfig::serialize",
skip_deserializing
)]
pub stdin: TriggerStdinConfig,
/// Specifies a file to write the output stream to. Prefix with `>` to
/// overwrite and `>>` to append.
#[serde(skip_serializing_if = "Option::is_none")]
pub stdout: Option<String>,
/// Specifies a file to write the error stream to. Prefix with `>` to
/// overwrite and `>>` to append.
#[serde(skip_serializing_if = "Option::is_none")]
pub stderr: Option<String>,
/// Specifies a limit on the number of files reported on stdin when stdin is
/// set to hold the set of matched files.
#[serde(skip_serializing_if = "Option::is_none")]
pub max_files_stdin: Option<u64>,
/// Specifies the working directory that will be set prior to spawning the
/// process. The default is to set the working directory to the watched
/// root. The value of this property is a string that will be interpreted
/// relative to the watched root.
#[serde(skip_serializing_if = "Option::is_none")]
pub chdir: Option<String>,
}
#[derive(Clone, Debug)]
pub enum TriggerStdinConfig {
DevNull,
FieldNames(Vec<String>),
NamePerLine,
}
impl Default for TriggerStdinConfig {
fn default() -> Self {
Self::DevNull
}
}
impl TriggerStdinConfig {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
match self {
Self::DevNull => serializer.serialize_str("/dev/null"),
Self::FieldNames(names) => serializer.collect_seq(names.iter()),
Self::NamePerLine => serializer.serialize_str("NAME_PER_LINE"),
}
}
fn is_devnull(&self) -> bool {
matches!(self, Self::DevNull)
}
}
#[derive(Serialize, Clone, Debug)]
pub struct TriggerCommand(pub &'static str, pub PathBuf, pub TriggerRequest);
#[derive(Deserialize, Debug)]
pub struct TriggerResponse {
pub version: String,
pub disposition: String,
pub triggerid: String,
}
#[derive(Serialize, Clone, Debug)]
pub struct TriggerDelCommand(pub &'static str, pub PathBuf, pub String);
#[derive(Deserialize, Debug)]
pub struct TriggerDelResponse {
pub version: String,
pub deleted: bool,
pub trigger: String,
}
#[derive(Serialize, Clone, Debug)]
pub struct TriggerListCommand(pub &'static str, pub PathBuf);
#[derive(Deserialize, Debug)]
pub struct TriggerListResponse {
pub version: String,
pub triggers: Vec<TriggerRequest>,
}
#[allow(clippy::trivially_copy_pass_by_ref)]
fn is_false(v: &bool) -> bool {
!*v
}

View file

@ -44,6 +44,8 @@ pub mod fileset;
mod fileset_parser; mod fileset_parser;
pub mod fmt_util; pub mod fmt_util;
pub mod fsmonitor; pub mod fsmonitor;
#[cfg(feature = "watchman")]
pub mod fsmonitor_watchman_extensions;
#[cfg(feature = "git")] #[cfg(feature = "git")]
pub mod git; pub mod git;
#[cfg(feature = "git")] #[cfg(feature = "git")]

View file

@ -49,7 +49,7 @@ use crate::conflicts::{self, materialize_tree_value, MaterializedTreeValue};
use crate::file_util::{check_symlink_support, try_symlink}; use crate::file_util::{check_symlink_support, try_symlink};
#[cfg(feature = "watchman")] #[cfg(feature = "watchman")]
use crate::fsmonitor::watchman; use crate::fsmonitor::watchman;
use crate::fsmonitor::FsmonitorKind; use crate::fsmonitor::{FsmonitorSettings, WatchmanConfig};
use crate::gitignore::GitIgnoreFile; use crate::gitignore::GitIgnoreFile;
use crate::lock::FileLock; use crate::lock::FileLock;
use crate::matchers::{ use crate::matchers::{
@ -726,8 +726,9 @@ impl TreeState {
#[instrument(skip(self))] #[instrument(skip(self))]
pub async fn query_watchman( pub async fn query_watchman(
&self, &self,
config: &WatchmanConfig,
) -> Result<(watchman::Clock, Option<Vec<PathBuf>>), TreeStateError> { ) -> Result<(watchman::Clock, Option<Vec<PathBuf>>), TreeStateError> {
let fsmonitor = watchman::Fsmonitor::init(&self.working_copy_path) let fsmonitor = watchman::Fsmonitor::init(&self.working_copy_path, config)
.await .await
.map_err(|err| TreeStateError::Fsmonitor(Box::new(err)))?; .map_err(|err| TreeStateError::Fsmonitor(Box::new(err)))?;
let previous_clock = self.watchman_clock.clone().map(watchman::Clock::from); let previous_clock = self.watchman_clock.clone().map(watchman::Clock::from);
@ -744,19 +745,19 @@ impl TreeState {
pub fn snapshot(&mut self, options: SnapshotOptions) -> Result<bool, SnapshotError> { pub fn snapshot(&mut self, options: SnapshotOptions) -> Result<bool, SnapshotError> {
let SnapshotOptions { let SnapshotOptions {
base_ignores, base_ignores,
fsmonitor_kind, fsmonitor_settings,
progress, progress,
max_new_file_size, max_new_file_size,
} = options; } = options;
let sparse_matcher = self.sparse_matcher(); let sparse_matcher = self.sparse_matcher();
let fsmonitor_clock_needs_save = fsmonitor_kind != FsmonitorKind::None; let fsmonitor_clock_needs_save = fsmonitor_settings != FsmonitorSettings::None;
let mut is_dirty = fsmonitor_clock_needs_save; let mut is_dirty = fsmonitor_clock_needs_save;
let FsmonitorMatcher { let FsmonitorMatcher {
matcher: fsmonitor_matcher, matcher: fsmonitor_matcher,
watchman_clock, watchman_clock,
} = self.make_fsmonitor_matcher(fsmonitor_kind)?; } = self.make_fsmonitor_matcher(fsmonitor_settings)?;
let fsmonitor_matcher = match fsmonitor_matcher.as_ref() { let fsmonitor_matcher = match fsmonitor_matcher.as_ref() {
None => &EverythingMatcher, None => &EverythingMatcher,
Some(fsmonitor_matcher) => fsmonitor_matcher.as_ref(), Some(fsmonitor_matcher) => fsmonitor_matcher.as_ref(),
@ -1024,13 +1025,13 @@ impl TreeState {
#[instrument(skip_all)] #[instrument(skip_all)]
fn make_fsmonitor_matcher( fn make_fsmonitor_matcher(
&self, &self,
fsmonitor_kind: FsmonitorKind, fsmonitor_settings: FsmonitorSettings,
) -> Result<FsmonitorMatcher, SnapshotError> { ) -> Result<FsmonitorMatcher, SnapshotError> {
let (watchman_clock, changed_files) = match fsmonitor_kind { let (watchman_clock, changed_files) = match fsmonitor_settings {
FsmonitorKind::None => (None, None), FsmonitorSettings::None => (None, None),
FsmonitorKind::Test { changed_files } => (None, Some(changed_files)), FsmonitorSettings::Test { changed_files } => (None, Some(changed_files)),
#[cfg(feature = "watchman")] #[cfg(feature = "watchman")]
FsmonitorKind::Watchman => match self.query_watchman() { FsmonitorSettings::Watchman(config) => match self.query_watchman(&config) {
Ok((watchman_clock, changed_files)) => (Some(watchman_clock.into()), changed_files), Ok((watchman_clock, changed_files)) => (Some(watchman_clock.into()), changed_files),
Err(err) => { Err(err) => {
tracing::warn!(?err, "Failed to query filesystem monitor"); tracing::warn!(?err, "Failed to query filesystem monitor");
@ -1038,7 +1039,7 @@ impl TreeState {
} }
}, },
#[cfg(not(feature = "watchman"))] #[cfg(not(feature = "watchman"))]
FsmonitorKind::Watchman => { FsmonitorSettings::Watchman(_) => {
return Err(SnapshotError::Other { return Err(SnapshotError::Other {
message: "Failed to query the filesystem monitor".to_string(), message: "Failed to query the filesystem monitor".to_string(),
err: "Cannot query Watchman because jj was not compiled with the `watchman` \ err: "Cannot query Watchman because jj was not compiled with the `watchman` \
@ -1686,9 +1687,10 @@ impl LocalWorkingCopy {
#[cfg(feature = "watchman")] #[cfg(feature = "watchman")]
pub fn query_watchman( pub fn query_watchman(
&self, &self,
config: &WatchmanConfig,
) -> Result<(watchman::Clock, Option<Vec<PathBuf>>), WorkingCopyStateError> { ) -> Result<(watchman::Clock, Option<Vec<PathBuf>>), WorkingCopyStateError> {
self.tree_state()? self.tree_state()?
.query_watchman() .query_watchman(config)
.map_err(|err| WorkingCopyStateError { .map_err(|err| WorkingCopyStateError {
message: "Failed to query watchman".to_string(), message: "Failed to query watchman".to_string(),
err: err.into(), err: err.into(),

View file

@ -23,7 +23,7 @@ use rand_chacha::ChaCha20Rng;
use crate::backend::{ChangeId, Commit, Signature, Timestamp}; use crate::backend::{ChangeId, Commit, Signature, Timestamp};
use crate::fmt_util::binary_prefix; use crate::fmt_util::binary_prefix;
use crate::fsmonitor::FsmonitorKind; use crate::fsmonitor::FsmonitorSettings;
use crate::signing::SignBehavior; use crate::signing::SignBehavior;
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
@ -164,12 +164,8 @@ impl UserSettings {
self.config.get_string("user.email").unwrap_or_default() self.config.get_string("user.email").unwrap_or_default()
} }
pub fn fsmonitor_kind(&self) -> Result<FsmonitorKind, config::ConfigError> { pub fn fsmonitor_settings(&self) -> Result<FsmonitorSettings, config::ConfigError> {
match self.config.get_string("core.fsmonitor") { FsmonitorSettings::from_config(&self.config)
Ok(fsmonitor_kind) => Ok(fsmonitor_kind.parse()?),
Err(config::ConfigError::NotFound(_)) => Ok(FsmonitorKind::None),
Err(err) => Err(err),
}
} }
// Must not be changed to avoid git pushing older commits with no set email // Must not be changed to avoid git pushing older commits with no set email

View file

@ -24,7 +24,7 @@ use thiserror::Error;
use crate::backend::{BackendError, MergedTreeId}; use crate::backend::{BackendError, MergedTreeId};
use crate::commit::Commit; use crate::commit::Commit;
use crate::fsmonitor::FsmonitorKind; use crate::fsmonitor::FsmonitorSettings;
use crate::gitignore::{GitIgnoreError, GitIgnoreFile}; use crate::gitignore::{GitIgnoreError, GitIgnoreFile};
use crate::op_store::{OperationId, WorkspaceId}; use crate::op_store::{OperationId, WorkspaceId};
use crate::repo_path::{RepoPath, RepoPathBuf}; use crate::repo_path::{RepoPath, RepoPathBuf};
@ -189,7 +189,7 @@ pub struct SnapshotOptions<'a> {
/// The fsmonitor (e.g. Watchman) to use, if any. /// 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 // TODO: Should we make this a field on `LocalWorkingCopy` instead since it's quite specific to
// that implementation? // that implementation?
pub fsmonitor_kind: FsmonitorKind, pub fsmonitor_settings: FsmonitorSettings,
/// A callback for the UI to display progress. /// A callback for the UI to display progress.
pub progress: Option<&'a SnapshotProgress<'a>>, pub progress: Option<&'a SnapshotProgress<'a>>,
/// The size of the largest file that should be allowed to become tracked /// The size of the largest file that should be allowed to become tracked
@ -205,7 +205,7 @@ impl SnapshotOptions<'_> {
pub fn empty_for_test() -> Self { pub fn empty_for_test() -> Self {
SnapshotOptions { SnapshotOptions {
base_ignores: GitIgnoreFile::empty(), base_ignores: GitIgnoreFile::empty(),
fsmonitor_kind: FsmonitorKind::None, fsmonitor_settings: FsmonitorSettings::None,
progress: None, progress: None,
max_new_file_size: u64::MAX, max_new_file_size: u64::MAX,
} }

View file

@ -23,7 +23,7 @@ use indoc::indoc;
use itertools::Itertools; use itertools::Itertools;
use jj_lib::backend::{MergedTreeId, TreeId, TreeValue}; use jj_lib::backend::{MergedTreeId, TreeId, TreeValue};
use jj_lib::file_util::{check_symlink_support, try_symlink}; use jj_lib::file_util::{check_symlink_support, try_symlink};
use jj_lib::fsmonitor::FsmonitorKind; use jj_lib::fsmonitor::FsmonitorSettings;
use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::local_working_copy::LocalWorkingCopy;
use jj_lib::merge::{Merge, MergedTreeValue}; use jj_lib::merge::{Merge, MergedTreeValue};
use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
@ -1183,7 +1183,7 @@ fn test_fsmonitor() {
locked_ws locked_ws
.locked_wc() .locked_wc()
.snapshot(SnapshotOptions { .snapshot(SnapshotOptions {
fsmonitor_kind: FsmonitorKind::Test { fsmonitor_settings: FsmonitorSettings::Test {
changed_files: fs_paths, changed_files: fs_paths,
}, },
..SnapshotOptions::empty_for_test() ..SnapshotOptions::empty_for_test()