Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions lib/src/fsmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions lib/src/local_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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` \
Expand Down
6 changes: 3 additions & 3 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/src/working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions lib/tests/test_local_working_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down