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.

Sign up for GitHub

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

Add a poll-based file watcher. #2325

Merged
merged 3 commits into from
May 13, 2024
Merged

Add a poll-based file watcher. #2325

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3e85da
Add a poll-based file watcher.
ehuss Feb 24, 2024
46d57bc
Add some more context on why `--watcher=native` may not be desirable.
ehuss May 13, 2024
0752fa4
Fix `--watcher` markdown header.
ehuss May 13, 2024
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
3 changes: 2 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ notify = { version = "6.1.1", optional = true }
notify-debouncer-mini = { version = "0.4.1", optional = true }
ignore = { version = "0.4.20", optional = true }
pathdiff = { version = "0.2.1", optional = true }
walkdir = { version = "2.3.3", optional = true }

# Serve feature
futures-util = { version = "0.3.28", optional = true }
Expand All @@ -61,7 +62,7 @@ walkdir = "2.3.3"

[features]
default = ["watch", "serve", "search"]
watch = ["dep:notify", "dep:notify-debouncer-mini", "dep:ignore", "dep:pathdiff"]
watch = ["dep:notify", "dep:notify-debouncer-mini", "dep:ignore", "dep:pathdiff", "dep:walkdir"]
serve = ["dep:futures-util", "dep:tokio", "dep:warp"]
search = ["dep:elasticlunr-rs", "dep:ammonia"]

Expand Down
7 changes: 7 additions & 0 deletions guide/src/cli/arg-watcher.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#### `--watcher`

There are different backends used to determine when a file has changed.

* `poll` (default) --- Checks for file modifications by scanning the filesystem every second.
* `native` --- Uses the native operating system facilities to receive notifications when files change.
This can have less constant overhead, but may not be as reliable as the `poll` based watcher. See these issues for more information: [#383](https://github.com/rust-lang/mdBook/issues/383) [#1441](https://github.com/rust-lang/mdBook/issues/1441) [#1707](https://github.com/rust-lang/mdBook/issues/1707) [#2035](https://github.com/rust-lang/mdBook/issues/2035) [#2102](https://github.com/rust-lang/mdBook/issues/2102)
2 changes: 2 additions & 0 deletions guide/src/cli/serve.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ book. Relative paths are interpreted relative to the book's root directory. If
not specified it will default to the value of the `build.build-dir` key in
`book.toml`, or to `./book`.

{{#include arg-watcher.md}}

#### Specify exclude patterns

The `serve` command will not automatically trigger a build for files listed in
Expand Down
1 change: 1 addition & 0 deletions guide/src/cli/watch.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ book. Relative paths are interpreted relative to the book's root directory. If
not specified it will default to the value of the `build.build-dir` key in
`book.toml`, or to `./book`.

{{#include arg-watcher.md}}

#### Specify exclude patterns

Expand Down
13 changes: 13 additions & 0 deletions src/cmd/command_prelude.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ pub trait CommandExt: Sized {
fn arg_open(self) -> Self {
self._arg(arg!(-o --open "Opens the compiled book in a web browser"))
}

fn arg_watcher(self) -> Self {
#[cfg(feature = "watch")]
return self._arg(
Arg::new("watcher")
.long("watcher")
.value_parser(["poll", "native"])
.default_value("poll")
.help("The filesystem watching technique"),
);
#[cfg(not(feature = "watch"))]
return self;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is a little weird to me. Wouldn't we want to feature gate the entire arg_watcher function, instead of the body?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would require the callers to have to do some pretty awkward #[cfg] statements to conditionally determine if the method is available (since #[cfg] doesn't work well with builder patterns).

}
}

impl CommandExt for Command {
Expand Down
25 changes: 7 additions & 18 deletions src/cmd/serve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use clap::builder::NonEmptyStringValueParser;
use futures_util::sink::SinkExt;
use futures_util::StreamExt;
use mdbook::errors::*;
use mdbook::utils;
use mdbook::utils::fs::get_404_output_file;
use mdbook::MDBook;
use std::net::{SocketAddr, ToSocketAddrs};
Expand Down Expand Up @@ -43,12 +42,13 @@ pub fn make_subcommand() -> Command {
.help("Port to use for HTTP connections"),
)
.arg_open()
.arg_watcher()
}

// Serve command implementation
pub fn execute(args: &ArgMatches) -> Result<()> {
let book_dir = get_book_dir(args);
let mut book = MDBook::load(book_dir)?;
let mut book = MDBook::load(&book_dir)?;

let port = args.get_one::<String>("port").unwrap();
let hostname = args.get_one::<String>("hostname").unwrap();
Expand Down Expand Up @@ -97,23 +97,12 @@ pub fn execute(args: &ArgMatches) -> Result<()> {
}

#[cfg(feature = "watch")]
watch::trigger_on_change(&book, move |paths, book_dir| {
info!("Files changed: {:?}", paths);
info!("Building book...");

// FIXME: This area is really ugly because we need to re-set livereload :(
let result = MDBook::load(book_dir).and_then(|mut b| {
update_config(&mut b);
b.build()
});

if let Err(e) = result {
error!("Unable to load the book");
utils::log_backtrace(&e);
} else {
{
let watcher = watch::WatcherKind::from_str(args.get_one::<String>("watcher").unwrap());
watch::rebuild_on_change(watcher, &book_dir, &update_config, &move || {
let _ = tx.send(Message::text("reload"));
}
});
});
}

let _ = thread_handle.join();

Expand Down
211 changes: 31 additions & 180 deletions src/cmd/watch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
use super::command_prelude::*;
use crate::{get_book_dir, open};
use ignore::gitignore::Gitignore;
use mdbook::errors::Result;
use mdbook::utils;
use mdbook::MDBook;
use pathdiff::diff_paths;
use std::path::{Path, PathBuf};
use std::sync::mpsc::channel;
use std::thread::sleep;
use std::time::Duration;

mod native;
mod poller;

// Create clap subcommand arguments
pub fn make_subcommand() -> Command {
Expand All @@ -17,12 +14,28 @@ pub fn make_subcommand() -> Command {
.arg_dest_dir()
.arg_root_dir()
.arg_open()
.arg_watcher()
}

pub enum WatcherKind {
Poll,
Native,
}

impl WatcherKind {
pub fn from_str(s: &str) -> WatcherKind {
match s {
"poll" => WatcherKind::Poll,
"native" => WatcherKind::Native,
_ => panic!("unsupported watcher {s}"),
}
}
}

// Watch command implementation
pub fn execute(args: &ArgMatches) -> Result<()> {
let book_dir = get_book_dir(args);
let mut book = MDBook::load(book_dir)?;
let mut book = MDBook::load(&book_dir)?;

let update_config = |book: &mut MDBook| {
if let Some(dest_dir) = args.get_one::<PathBuf>("dest-dir") {
Expand All @@ -41,42 +54,21 @@ pub fn execute(args: &ArgMatches) -> Result<()> {
open(path);
}

trigger_on_change(&book, |paths, book_dir| {
info!("Files changed: {:?}\nBuilding book...\n", paths);
let result = MDBook::load(book_dir).and_then(|mut b| {
update_config(&mut b);
b.build()
});

if let Err(e) = result {
error!("Unable to build the book");
utils::log_backtrace(&e);
}
});
let watcher = WatcherKind::from_str(args.get_one::<String>("watcher").unwrap());
rebuild_on_change(watcher, &book_dir, &update_config, &|| {});

Ok(())
}

fn remove_ignored_files(book_root: &Path, paths: &[PathBuf]) -> Vec<PathBuf> {
if paths.is_empty() {
return vec![];
}

match find_gitignore(book_root) {
Some(gitignore_path) => {
let (ignore, err) = Gitignore::new(&gitignore_path);
if let Some(err) = err {
warn!(
"error reading gitignore `{}`: {err}",
gitignore_path.display()
);
}
filter_ignored_files(ignore, paths)
}
None => {
// There is no .gitignore file.
paths.iter().map(|path| path.to_path_buf()).collect()
}
pub fn rebuild_on_change(
kind: WatcherKind,
book_dir: &Path,
update_config: &dyn Fn(&mut MDBook),
post_build: &dyn Fn(),
) {
match kind {
WatcherKind::Poll => self::poller::rebuild_on_change(book_dir, update_config, post_build),
WatcherKind::Native => self::native::rebuild_on_change(book_dir, update_config, post_build),
}
}

Expand All @@ -86,144 +78,3 @@ fn find_gitignore(book_root: &Path) -> Option<PathBuf> {
.map(|p| p.join(".gitignore"))
.find(|p| p.exists())
}

// Note: The usage of `canonicalize` may encounter occasional failures on the Windows platform, presenting a potential risk.
// For more details, refer to [Pull Request #2229](https://github.com/rust-lang/mdBook/pull/2229#discussion_r1408665981).
fn filter_ignored_files(ignore: Gitignore, paths: &[PathBuf]) -> Vec<PathBuf> {
let ignore_root = ignore
.path()
.canonicalize()
.expect("ignore root canonicalize error");

paths
.iter()
.filter(|path| {
let relative_path =
diff_paths(path, &ignore_root).expect("One of the paths should be an absolute");
!ignore
.matched_path_or_any_parents(&relative_path, relative_path.is_dir())
.is_ignore()
})
.map(|path| path.to_path_buf())
.collect()
}

/// Calls the closure when a book source file is changed, blocking indefinitely.
pub fn trigger_on_change<F>(book: &MDBook, closure: F)
where
F: Fn(Vec<PathBuf>, &Path),
{
use notify::RecursiveMode::*;

// Create a channel to receive the events.
let (tx, rx) = channel();

let mut debouncer = match notify_debouncer_mini::new_debouncer(Duration::from_secs(1), tx) {
Ok(d) => d,
Err(e) => {
error!("Error while trying to watch the files:\n\n\t{:?}", e);
std::process::exit(1)
}
};
let watcher = debouncer.watcher();

// Add the source directory to the watcher
if let Err(e) = watcher.watch(&book.source_dir(), Recursive) {
error!("Error while watching {:?}:\n {:?}", book.source_dir(), e);
std::process::exit(1);
};

let _ = watcher.watch(&book.theme_dir(), Recursive);

// Add the book.toml file to the watcher if it exists
let _ = watcher.watch(&book.root.join("book.toml"), NonRecursive);

for dir in &book.config.build.extra_watch_dirs {
let path = book.root.join(dir);
let canonical_path = path.canonicalize().unwrap_or_else(|e| {
error!("Error while watching extra directory {path:?}:\n {e}");
std::process::exit(1);
});

if let Err(e) = watcher.watch(&canonical_path, Recursive) {
error!(
"Error while watching extra directory {:?}:\n {:?}",
canonical_path, e
);
std::process::exit(1);
}
}

info!("Listening for changes...");

loop {
let first_event = rx.recv().unwrap();
sleep(Duration::from_millis(50));
let other_events = rx.try_iter();

let all_events = std::iter::once(first_event).chain(other_events);

let paths: Vec<_> = all_events
.filter_map(|event| match event {
Ok(events) => Some(events),
Err(error) => {
log::warn!("error while watching for changes: {error}");
None
}
})
.flatten()
.map(|event| event.path)
.collect();

// If we are watching files outside the current repository (via extra-watch-dirs), then they are definitionally
// ignored by gitignore. So we handle this case by including such files into the watched paths list.
let any_external_paths = paths.iter().filter(|p| !p.starts_with(&book.root)).cloned();
let mut paths = remove_ignored_files(&book.root, &paths[..]);
paths.extend(any_external_paths);

if !paths.is_empty() {
closure(paths, &book.root);
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use ignore::gitignore::GitignoreBuilder;
use std::env;

#[test]
fn test_filter_ignored_files() {
let current_dir = env::current_dir().unwrap();

let ignore = GitignoreBuilder::new(&current_dir)
.add_line(None, "*.html")
.unwrap()
.build()
.unwrap();
let should_remain = current_dir.join("record.text");
let should_filter = current_dir.join("index.html");

let remain = filter_ignored_files(ignore, &[should_remain.clone(), should_filter]);
assert_eq!(remain, vec![should_remain])
}

#[test]
fn filter_ignored_files_should_handle_parent_dir() {
let current_dir = env::current_dir().unwrap();

let ignore = GitignoreBuilder::new(&current_dir)
.add_line(None, "*.html")
.unwrap()
.build()
.unwrap();

let parent_dir = current_dir.join("..");
let should_remain = parent_dir.join("record.text");
let should_filter = parent_dir.join("index.html");

let remain = filter_ignored_files(ignore, &[should_remain.clone(), should_filter]);
assert_eq!(remain, vec![should_remain])
}
}
Loading
Loading