Merge pull request rust-lang#2325 from ehuss/poll-watcher

Add a poll-based file watcher.

Cargo.toml

@@ -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 }
@@ -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"]
 

guide/src/cli/arg-watcher.md

@@ -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)

guide/src/cli/serve.md

@@ -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

guide/src/cli/watch.md

@@ -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
 

src/cmd/command_prelude.rs

@@ -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;
+    }
 }
 
 impl CommandExt for Command {

src/cmd/serve.rs

@@ -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};
@@ -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();
@@ -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();
 

src/cmd/watch.rs

@@ -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 {
@@ -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") {
@@ -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),
     }
 }
 
@@ -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])
-    }
-}