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.

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

document expected behavior for symlinks #18

Open
skybrian opened this issue Oct 29, 2015 · 8 comments
Open

document expected behavior for symlinks #18

skybrian opened this issue Oct 29, 2015 · 8 comments
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@skybrian
Copy link

It's unclear what's supposed to happen for symlinks. We don't need to implement full symlink support, but we should say what happens when they exist.

  • Is a symlink a file? A directory? Does this change depending on the symlink's target?
  • Do we detect when the symlink changes? When its target changes?
  • What happens when the symlink points outside the directory being watched?
@nex3 nex3 added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Oct 29, 2015
@nex3
Copy link
Member

nex3 commented Oct 29, 2015

Looking into this led me to realize that the behavior for symlinks isn't great, nor is it consistent. The polling watcher treats them effectively the same as their targets, which is probably the most useful option. But I believe the native Directory.watch doesn't do this on any platform, and only Linux even supports an option to enable it, which means we'll have to polyfill.

@skybrian
Copy link
Author

A couple more annoying issues:

  • A symlink may point to a different filesystem where native watching works differently or not at all. There may be no alternative to polling to detect changes.
  • A symlink may be broken (doesn't point anywhere at all). Does it exist as far as the watcher is concerned?

@nex3
Copy link
Member

nex3 commented Oct 29, 2015

A symlink may point to a different filesystem where native watching works differently or not at all. There may be no alternative to polling to detect changes.

I don't know if we have a reliable way to detect this. If we do, we could have the polyfill fall back to a polling watcher, but that may be pretty difficult to implement—it's a lot easier if all the event sources are FileSystemEvents rather than WatchEvents so that we can uniformly apply the same logic.

In general, our policy up to this point for non-watchable filesystems has just been to require that the caller (and transitively the user) explicitly specify the polling watcher. This is probably the easiest solution for non-watchable subdirectories as well, at least for now.

A symlink may be broken (doesn't point anywhere at all). Does it exist as far as the watcher is concerned?

Right now the polling watcher just treats them as though they don't exist. Generally we only emit events for files, which broken links sort of are and sort of aren't, so it could go either way, and I don't have a strong preference. On one hand not emitting events means choosing to hide information we have; on the other I'd imagine almost all actual uses are going to assume we're emitting events for real files, won't test with broken links, and will end up breaking if they come up in practice.

What's your preference?

@nex3
Copy link
Member

nex3 commented Oct 30, 2015

Just for my own documentation, I was playing around with how symlinks work with FSEvents on OS X, and I learned an unpleasant fact: it's possible to create a valid symlink without triggering any events at all, which means we won't even know to create a watcher for the resolved path. To do so, just create a broken symlink in the watched directory. From then on, many changes to that symlink—including creating its target file or even retargeting it to a real file—will not be reported. Removing it will, but by that point it's not particularly useful.

Unfortunately this seems to be a fundamental flaw in FSEvents itself, so we can't fix it in dart:io. It's not even the sort of thing we can polyfill since there's no way to tell when it's happening. Hopefully it won't come up much in practice, but we should clearly document it as a potential pitfall.

@skybrian
Copy link
Author

If the Analyzer or a text editor is the main use case, readable files and directories are what we care about and hiding broken links or other unusual filesystem objects seems better than reporting them.

But then there's the question of how to handle a transition - if a symlink used to work, but the file it points to is deleted or replaced with something that's not a regular file, it seems useful to send a "delete" event. Also nice to report "add" for the opposite transition, but it may not be possible.

Other unpleasant edge cases, beyond symlinks:

  • replace a file with a directory. It's not a file anymore, so is that a modify or delete? (The comment on ChangeEvent.MODIFY says "the contents of a file have changed.")
  • replace the top-level file or directory you're watching.
  • something exists, but you don't have permission to read it. Or the permissions change.

@nex3
Copy link
Member

nex3 commented Oct 30, 2015

But then there's the question of how to handle a transition - if a symlink used to work, but the file it points to is deleted or replaced with something that's not a regular file, it seems useful to send a "delete" event. Also nice to report "add" for the opposite transition, but it may not be possible.

If we treat broken links as non-existent, we'll definitely report delete and add events when they're broken or fixed, respectively (except on OS X I guess).

replace a file with a directory. It's not a file anymore, so is that a modify or delete? (The comment on ChangeEvent.MODIFY says "the contents of a file have changed.")

Today we report a DELETE, which I think is correct.

replace the top-level file or directory you're watching.

This just closes the stream. It's pretty easy to just open a new one afterwards.

something exists, but you don't have permission to read it. Or the permissions change.

For a directory, we pretend it doesn't exist. For a file, we leave it up to the user to handle gracefully.

nex3 added a commit that referenced this issue Nov 16, 2015
So far this only covers the Linux and polling watchers.

See #18

R=rnystrom@google.com

Review URL: https://codereview.chromium.org//1406223012 .
@bergwerf
Copy link

I built a simple tool that watches a directory and applies formatting when a file changes (dartfmt, gofmt, astyle, etc.). You can view it here: https://github.com/molview/molviewfmt. Now I noticed it often doesn't work with Dart anymore because the files in my lib directory are also symlinked in many other folders. This causes the watcher to emit an file changed event for tool/packages/my_package/src/my_file.dart but not for lib/src/my_file.dart. This is a severe issue because I built my formatter so that it parses the root .gitignore and ignores file changes that are ignored here (initially to prevent the formatting of the entire node_modules directory in npm projects when formatting all files in the directory at once). This means I cannot use this tool anymore to watch Dart files (because of the symlinks). I would like to disable following symlinks in this case or at least following symlinks that point to a directory in the watching directory. Is this possible? I could not find an option in the docs.

@nex3
Copy link
Member

nex3 commented May 10, 2016

Currently the watcher package is not symlink-aware at all, and the behavior follows whatever the native behavior of Directory.watch() is on whatever OS you're using. I have some work to update that, but I don't anticipate having time to finish it any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants