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

Confusion about Remove when combined with Add vs AddRaw #387

Closed
nshalman opened this issue Aug 19, 2021 · 3 comments
Closed

Confusion about Remove when combined with Add vs AddRaw #387

nshalman opened this issue Aug 19, 2021 · 3 comments
Labels

Comments

@nshalman
Copy link
Contributor

nshalman commented Aug 19, 2021

Please describe the issue that occurred.

While porting #371 to support AddRaw from #289 I run into the following conundrum:

If I AddRaw a symlink, I can Remove that symlink from being watched just fine.
If I Add a symlink, I cannot Remove the symlink directly, but would have to resolve it first.

What should fsnotify do?

Are you able to reproduce the issue? Please provide steps to reproduce and a code sample if possible.

See the test case added in #388

@nathany
Copy link
Contributor

nathany commented Aug 19, 2021

It does seem like a bug if Remove doesn't resolve symlinks the way Add does. Is that across every OS?

I'm not sure if we should go as far as Remove and RemoveRaw APIs.

To be honest, for the simplicity of the API, I'm starting to wonder if we should've made a breaking change (major version) that pushes the responsibility for EvalSymlinks up to the caller. 🤔 Probably not, but I had that thought.

@Code0x58 What do you think?

@Code0x58
Copy link
Contributor

I added a comment on #388 (comment). I'm not sure that a major change is worth it at the moment, as removing Add/Remove or replacing them with AddRaw/(presumably)RemoveRaw - there's still hope that Add/Remove could be incrementally improved to the point that they don't come with old caveats, so could be a nice transparent upgrade in the future.

@nshalman
Copy link
Contributor Author

Fixed by #394

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants