notify icon indicating copy to clipboard operation
notify copied to clipboard

Notify does not handle the new `IsFile` event on macOS.

Open wdanilo opened this issue 2 years ago • 7 comments

System details

What you did (as detailed as you can)

I'm using cargo-watch 8.1.1 (because 8.3.0 does not work on new macOS). Recently, our build script, which uses cargo-watch under the hood, started looping indefinitely. After debugging it turned out, that when using std::fs::copy("a.txt", "b.txt") on macOS, a file system event IsFile is sometimes emitted on the source file ("a.txt"). The term "sometimes" is better explained here. Our build script is copying some files to target directory, which causes the sources to emit this event, which causes cargo-watch to re-run the whole operation again. The cargo-watch issue, where the authors asked me to post issue here can be found here: https://github.com/watchexec/cargo-watch/issues/242

wdanilo avatar Jan 23 '23 01:01 wdanilo

So from what I've read across all 4 issues, the COW operation emits an Event for the source with a flag IsFile set. And we should ignore events with that flag ? Does the target also receive an event ? I don't own any mac hardware. I also guess you're using the fsevent backend, not kqueue.

0xpr03 avatar Jan 23 '23 11:01 0xpr03

@0xpr03 thanks for the reply. Yes, the target also receives the event "sometimes". However, I have not debugged deeply enough to be able define what "sometimes" means here. I believe that you should ignore this event, yes. Regarding whether I'm using queue or fsevents, TBH, I have no idea. Googling told me only that these two systems are available on macOS, but I didn't find a way to check which one is being used under the hood. If you want me to provide any logs / run some command/scripts, I'd be happy to. Does it answer your question?

wdanilo avatar Jan 23 '23 11:01 wdanilo

It has to be kqueue, based on watchexec/cargo-watch. Simply pushing out a version that ignores this, without confirming it doesn't break anything is something I wouldn't want to do, neither for v4 nor v5. I also need confirmation we will receive any event at all for such COW operations, if we ignore the source event. Otherwise some people might rather receive any event at all (*). I could create a branch with that fix, and you'd have to test out a custom build of notify (and possibly watchexec+cargo-watch).

(*) For example rebuilding everything once stuff changes with a debouncer, so they don't care about specific events, but at least one has to be emitted.

0xpr03 avatar Jan 23 '23 11:01 0xpr03

@0xpr03 instead of ignoring this event we can as well just handle it and libraries using notify can pattern match on it and ignore it by themselves. I believe this is a way more generic and safe approach. What do you think about it?

wdanilo avatar Jan 24 '23 12:01 wdanilo

@0xpr03 Hi :) Do you think that something could be done regarding this issue? Because of it, cargo-watch is not usable on M1 and M2 Macs right now, which I believe renders it as a pretty important issue. What do you think? :)

wdanilo avatar Feb 19 '23 02:02 wdanilo

I'm open for PRs. I don't own any macos hardware, so I can't develop this.

0xpr03 avatar Feb 19 '23 14:02 0xpr03

I've been trying to debug this and have not been able to make reasonable progress. From the best as I can tell, there are events that can happen to a cloned file (created with fclonefileat) that cannot be distinguished between "this is a real event" and "this is caused by creating the clone". For example:

  • IS_FILE | ITEM_CLONED — This is triggered by fclonefileat on the source. This is ok, since this already gets filtered out by notify.
  • ITEM_CREATED | INODE_META_MOD | ITEM_MODIFIED | IS_FILE | ITEM_CLONED — This is triggered on the source by fclonefileat, presumably just to let you know that the file was cloned.
  • INODE_META_MOD | ITEM_MODIFIED | IS_FILE | ITEM_CLONED — This is triggered when modifying the source of an already cloned file.
  • INODE_META_MOD | ITEM_MODIFIED | IS_FILE | ITEM_CLONED — This is triggered when running fclonefileat on an already cloned file.

I don't think it would be correct to ignore events with ITEM_CLONED, and thus I have no idea how to fix this (other than switching to PollWatcher or something else). I would almost venture to say that this is a bug in macOS.

Here is a demo program. It has an "input" directory, and it copies files from there to an "output" directory whenever the input changes. Running this essentially loops an infinite number of times, since calling fs::copy is being treated as a modification of the source file.

use std::error::Error;
use std::path::Path;
use std::sync::mpsc::channel;
use std::time::Duration;

fn main() -> Result<(), Box<dyn Error>> {
    if Path::new("output").exists() {
        std::fs::remove_dir_all("output")?;
    }
    if Path::new("input").exists() {
        std::fs::remove_dir_all("input")?;
    }
    std::fs::create_dir("output")?;
    std::fs::create_dir("input")?;
    std::fs::write("input/abc", "test")?;

    std::thread::spawn(|| watch(Path::new("input"), true).unwrap());
    // std::thread::spawn(|| watch(Path::new("output"), false).unwrap());
    std::thread::sleep(Duration::from_secs(1));
    eprintln!("modfying file");
    std::fs::write("input/abc", "test 2")?;
    std::thread::park();
    Ok(())
}

fn watch(path: &Path, do_copy: bool) -> Result<(), Box<dyn Error>> {
    use notify::RecursiveMode::*;

    let (tx, rx) = channel();
    let mut debouncer = notify_debouncer_mini::new_debouncer(Duration::from_secs(1), None, tx)?;
    let watcher = debouncer.watcher();
    watcher.watch(path, Recursive)?;
    println!("Listening for changes... {path:?}");
    loop {
        match rx.recv().unwrap() {
            Ok(event) => {
                eprintln!("got event {event:?}",);
                if do_copy {
                    // This simulates what happens when rebuilding the output.
                    // Remove these two lines to prevent an infinite loop. 
                    // However, it will still loop twice (once for the modification, and once for the copy).
                    std::fs::remove_dir_all("output")?;
                    std::fs::create_dir("output")?;
                    for entry in std::fs::read_dir(path)? {
                        let entry = entry?;
                        eprintln!("copy {:?}", entry.path());
                        std::fs::copy(
                            entry.path(),
                            Path::new("output").join(entry.path().file_name().unwrap()),
                        )?;
                    }
                }
            }
            Err(e) => {
                eprintln!("failed {e:?}");
            }
        }
    }
}

ehuss avatar Jul 30 '23 20:07 ehuss