Optionally following symlinks

[stuhood]

Hey folks! Have been happily using 5.0.0.pre2 for a few months now.

Our use of the notify crate is symlink aware, so before running read_link, we ensure that we use this crate's API to watch it. The behavior of read_link is to return the destination of a symlink, even if it is dead, and we rely on that in order to track the existence of the symlink and watch it in case it changes.

But on linux, the notify crate's inotify implementation currently doesn't set IN_DONT_FOLLOW:

    IN_DONT_FOLLOW (since Linux 2.6.15) 

Don't dereference pathname if it is a symbolic link. 

... which means that the attempt to watch a dead symlink fails.

---

To work around this, I have a small patch to the crate that currently just... unconditionally sets IN_DONT_FOLLOW (which is clearly not landable as is), and then another landable patch to avoid stating things unnecessarily: #256. But I'd like to help ensure that this can be supported upstream, and would be interested in suggestions for how it could be included into the API as an optional toggle.

A global toggle per-watcher would be sufficient for our usecase, but it's also possible that some callers would prefer this on a watch-by-watch basis. Alternatively, perhaps a Watcher::symlink_watch method that was the spiritual equivalent of https://doc.rust-lang.org/std/path/struct.Path.html#method.symlink_metadata... ie, not reading/watching through a symlink if it existed?

[0xpr03]

Coming back to this: I think we want some toggle at first and land some defense against dead symlinks. In light of #291 I think we should rather use ignore-sets to let people "disable" symlinks per-path. Everything else is probably too much overhead and possibly confusing.

[stuhood]

Coming back to this: I think we want some toggle at first and land some defense against dead symlinks. In light of #291 I think we should rather use ignore-sets to let people "disable" symlinks per-path. Everything else is probably too much overhead and possibly confusing.

#291 seems like it would primarily affect the recursive watch API: with regard to watching individual/single paths, having per-path flexibility would be preferable I think.

I suspect that given how different their settings and behavior are, that rather than a single watch function with a RecursiveMode enum, there should be two functions. Maybe something like:

fn watch_single<P: ..>(&mut self, path: P, options: SingleWatchOptions) -> ..;
fn watch_recursive<P: ..>(&mut self, path: P, options: RecursiveWatchOptions) -> ..;

...which is on some level is akin to 1) adding more options/settings to RecursiveMode and 2) renaming it. But these methods could seemingly be added backwards compatibly.

---

At a high level, the difference between these usecases is sortof like the difference between std::io::File (single watch) and something like glob or ignore (multi-path/recursive watch).

[tdyas]

I am one of the maintainers of the https://github.com/pantsbuild/pants repository. This issue was originally reported by @stuhood who is also a maintainer of that project. I'd like to help resolve this issue so we can stop using an out-of-date, forked copy of notify in that project.

What are the current thoughts on an appropriate API for disabling following symlinks?

[vpzomtrrfrt]

Was this issue resolved by #635?