Repository: rust-lang/mdBook (language: rust)

Issue:
Watch triggers multiple times for non-markdown on APFS (macOS)
### Problem

Editing non-`.md` files in the book's source folder triggers multiple rebuilds when using `mdbook serve`. The amount of rebuilds is not constant, I've seen it vary from 2 to over 20 times. Editing markdown files triggers a single rebuild.


### Steps

1. `cd $(mktemp -d)`
2. `mdbook init </dev/null`
3. `echo "export let lorem = 'ipsum';" > src/test.ts`
4. `echo -e '```\n{{#include test.ts}}\n```' >> src/chapter_1.md`
5. `mdbook serve`
6. in a second terminal: `touch src/test.ts`

<img width="1512" alt="terminal showing over a dozen rebuilds" src="https://github.com/rust-lang/mdBook/assets/821510/3d925d81-8c34-4b76-8fc9-a70472aede0d">


### Possible Solution(s)

Switching `notify` from `macos_fsevent` to `macos_kqueue` seems to fix the issue, triggering only a single rebuild whenever the `src/test.ts` file changes.



### Notes

This happens on an APFS drive on macOS. It doesn't happen on ext4 in a linux VM.

I'm not sure why this happens. A simple test using the following code doesn't trigger the multitude of events, leading me to think the cause lies within mdbook and not in notify.

```rust
use std::{path::Path, thread::sleep, time::Duration};

use notify::RecursiveMode;

fn main() {
    let mut debouncer =
        notify_debouncer_mini::new_debouncer(Duration::from_secs(1), None, |event| {
            println!("event {:#?}", event);
        })
        .unwrap();

    let watcher = debouncer.watcher();

    watcher
        .watch(Path::new("."), RecursiveMode::Recursive)
        .unwrap();

    loop {
        sleep(Duration::from_millis(500));
    }
}
```

### Version

```text
mdbook v0.4.32 (also happens on v0.4.31, earlier versions not tested)
```



Discussion / hints:
Hey, I think I found something.
It seems like the copy_files_except_ext on line 631 is causing the file to be picked up as changed.

<img width="1164" alt="image" src="https://github.com/rust-lang/mdBook/assets/4487942/35ece585-f4ed-48f6-a8d8-6cee188b3ff7">

If you comment this line out it only gets picked up once.
I did a little bit of investigation which I posted at https://github.com/notify-rs/notify/issues/465#issuecomment-1657261035. This issue is roughly:

1. mdBook detects an "extra" file is modified.
2. mdBook deletes the output directory
3. mdBook regenerates the book, and calls `fs::copy` on the "extra" file into the output directory.
4. notify is triggering an event for the `fs::copy` with flags `INODE_META_MOD | ITEM_MODIFIED | IS_FILE | ITEM_CLONED`.
5. mdBook thinks the file is modified, and loops back to step 1.

We can't ignore events with those flags, because that's the exact same set of flags that macOS generates if you modify the already cloned file.

Some rough ideas for different options:
* On macOS, don't use `fs::copy`, but instead manually copy the contents of the file.  This uses more disk space, but probably not harmful.
    * Hard links might also work, but copy is probably more reliable.
* Have an option to use a different notify watcher, like `PollWatcher`. This could help with issues like #2102, but there isn't a convenient place to put user-specific configurations, and the `PollWatcher` performance could be noticeably bad.
* Switch notify to the kqueue backend. 

I have been testing the last option, and it seems to work more reliably. It might be a risky change, but we'll see if anyone runs into any issues.


Fix patch:
diff --git a/Cargo.toml b/Cargo.toml
index 92e50017f5..51eea5e45e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -37,7 +37,7 @@ toml = "0.5.11" # Do not update, see https://github.com/rust-lang/mdBook/issues/
 topological-sort = "0.2.2"
 
 # Watch feature
-notify = { version = "6.0.1", optional = true, features = ["macos_kqueue"] }
+notify = { version = "6.0.1", optional = true }
 notify-debouncer-mini = { version = "0.3.0", optional = true }
 ignore = { version = "0.4.20", optional = true }
 
diff --git a/src/utils/fs.rs b/src/utils/fs.rs
index 8ad5aad8bb..1c3132162b 100644
--- a/src/utils/fs.rs
+++ b/src/utils/fs.rs
@@ -166,7 +166,7 @@ pub fn copy_files_except_ext(
                         .expect("a file should have a file name...")
                 )
             );
-            fs::copy(
+            copy(
                 entry.path(),
                 &to.join(
                     entry
@@ -180,6 +180,62 @@ pub fn copy_files_except_ext(
     Ok(())
 }
 
+/// Copies a file.
+fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
+    let from = from.as_ref();
+    let to = to.as_ref();
+    return copy_inner(from, to)
+        .with_context(|| format!("failed to copy `{}` to `{}`", from.display(), to.display()));
+
+    // This is a workaround for an issue with the macOS file watcher.
+    // Rust's `std::fs::copy` function uses `fclonefileat`, which creates
+    // clones on APFS. Unfortunately fs events seem to trigger on both
+    // sides of the clone, and there doesn't seem to be a way to differentiate
+    // which side it is.
+    // https://github.com/notify-rs/notify/issues/465#issuecomment-1657261035
+    // contains more information.
+    //
+    // This is essentially a copy of the simple copy code path in Rust's
+    // standard library.
+    #[cfg(target_os = "macos")]
+    fn copy_inner(from: &Path, to: &Path) -> Result<()> {
+        use std::fs::OpenOptions;
+        use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
+
+        let mut reader = File::open(from)?;
+        let metadata = reader.metadata()?;
+        if !metadata.is_file() {
+            anyhow::bail!(
+                "expected a file, `{}` appears to be {:?}",
+                from.display(),
+                metadata.file_type()
+            );
+        }
+        let perm = metadata.permissions();
+        let mut writer = OpenOptions::new()
+            .mode(perm.mode())
+            .write(true)
+            .create(true)
+            .truncate(true)
+            .open(to)?;
+        let writer_metadata = writer.metadata()?;
+        if writer_metadata.is_file() {
+            // Set the correct file permissions, in case the file already existed.
+            // Don't set the permissions on already existing non-files like
+            // pipes/FIFOs or device nodes.
+            writer.set_permissions(perm)?;
+        }
+        std::io::copy(&mut reader, &mut writer)?;
+        Ok(())
+    }
+
+    #[cfg(not(target_os = "macos"))]
+    fn copy_inner(from: &Path, to: &Path) -> Result<()> {
+        fs::copy(from, to)?;
+        Ok(())
+    }
+}
+
 pub fn get_404_output_file(input_404: &Option<String>) -> String {
     input_404
         .as_ref()


Test patch: