-
Notifications
You must be signed in to change notification settings - Fork 991
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
watchman & fb-watchman - doc error, impl error, misunderstanding? #564
Comments
Thanks for the bug report! Could you provide the JSON output of running |
Please also include the watchman query that is being run? It sounds like the relative root parameter may be missing? |
(or the opposite; maybe you don't want it to have a relative root set in that case?) |
Thanks! A couple questions based on your questions (hope this isn't WAY too much):
|
Sid, you asked about the 'watch-project' call. The input was |
Maybe a separate question that might clear some of this up: what's the difference between 'watch' and 'subscribe'? The first seems to be 'enable the watching mechanism' and the second seems to be 'tell me about the changes that you see with what happened while watching'. Is that right? (I'm trying to understand why both are needed - doesn't saying "watch" or "watch project" imply that you are interested in being told about them? Sorry for my confusion on this! If there's a better place to ask, I can take my questions there. |
re: terminology: The A So in our model/terminology the Are you saying that sane wasn't behaving so you tried tweaking it, or that you tried tweaking it and then hit the error you're reporting in this issue? |
Wez, thanks for the explanation! That's very helpful. What I'm doing is based on a request from Stef Penner (I'm also at LinkedIn, working on a team concerned with developer productivity). Our primary Ember application is large, and uses ember-cli and broccoli to build/serve it (this is all in a dev environment). Ultimately when served, everything is under a single root directory with a .watchmanconfig in that directory. Watchman is being used to watch for source changes to cause rebuild+serve. Broccoli ends up creating a ton of 'sane' instances (WatchmanWatcher) for various directories in the tree, and each of those is creating a watchman.client. So we're ending up with hundreds of connections from our process to watchman, ultimately all coming from under the same root dir (at least that's how it appears from my watching the log). I was trying to create a 'WatchmanClient' in the 'sane' project that would wrap a singleton watchman.Client, and have all the WatchmanWatcher instances use that, as opposed to doing a single 'watch-project' and a lot of subscribes (per discussion with Stef). In looking at the WatchmanWatcher code, it never seems to do anything that is outside of the original directory root that was passed to it (not the project root, which for us is the repo root dir, but what I called the watch root). So it appeared that since watch-project returns the "project" root + offset to the requested root dir, I could remove references to the project root entirely from the WW code. That's where I lost putting the 'relative_root' attribute on the subscribe options. I'm still unclear - how would I know that 'relative_root' should be used in the subscribe, since it's not in the 'subscribe' description? I had assumed (apparently wrongly) that 'watch-project' would then imply that any subscribes done after that would be from the project root. That's my confusion. It's actually preferable to me that they are from the 'watch root' for each subscribe, since we don't appear to need the project root info at all in responding to the subscription messages. Sorry this is so long! And again, I do appreciate the help. Suggestions are definitely encouraged! |
Oh, one last thing: the WatchmanWatcher code never actually does a 'query' command. It just does watch or watch-project, relevant stuff to get things set up, then a 'subscribe' and listens for file change/add/delete events. Also, my above comment about watch-project should read "I had assumed (apparently wrongly) that 'watch-project' would then imply that any subscribes done after that would return the offset from the project root as the affected file (rather than offset from the 'watch root'). That's my confusion." Finally, just FYI: the way I've thought of the organization is that the 'project' (i.e. repo) root is highest in the tree, and that the 'roots' (what I called 'watch roots') are below that. It sounds like that's backwards to what you're using. Right? If so, then we're establishing a single 'root' using 'watch-project', and each WW corresponds to what you call a project underneath that root. |
Thanks for the info; that helps set context and clarify what's going on. Yeah, having one connection per subscription is overkill! The watchman protocol is pretty stateless; none of the commands implicitly impact how subsequent commands are interpreted. There's a little bit of magic regarding the encoding of the transport, but semantically, commands you executed earlier don't implicitly change how we parse subsequent commands. Side effects of previous commands impact whether a subsequent command will succeed, but they won't change how we plan to execute the command. With that in mind, using Re: knowing about What I'd recommend for your use case, ignoring how sane does things for the moment and focusing purely on the watchman interaction, is:
When you get a subscription notification from watchman it will be tagged with the name (eg: it will have its If you'd prefer that watchman gives you all your results relative to the
FWIW, I'm not sure that you can use sane without using relative_root as sane is implemented today. |
I'd like to ask something more complicated, but for now - if I'm really interested for a single subscribe in the data under some directory below the 'watch-project' root, can I just subscribe to that by setting the second item in the subscribe array to that dir, and leaving out the relative_root param? Simple example hierarchy:
Let's say broccoli-sane-watcher says it wants to put a sane WatchmanWatcher on 'styles'. It appears from my tests that both will return a "subscription" result with "app.css" as the path if that file changes, which is relative to the styles dir. Is there a reason to use relative_path in this scenario, or ever? |
Aah, I understand. It's because of some legacy broken behavior that we fairly recently fixed up (e02729e) and that we don't see often due to some restrictive configuration at FB (briefly: we turn on a The So in your example above, your
So: best practice is to do what sane is already doing; subscribe to the root and specify the relative_root parameter by feeding in the result from watch-project! |
Aha! That's it - that's exactly what I was looking for. Thank you!! I'll do things the way sane is doing them now, though might be able to clean things up slightly there (I need to look at it again). All the other points you made earlier (separate subscription IDs and stuff) are what I was already intending to do, so I think we're good. Thank you again, Wez! |
In fb-watchman’s README.md is the comment “change notifications are relative to the project root”. It appears this is for both watch and watch-project. However, when I run tests where I do a ‘watch-project’ on a dir 2 levels below the project root (what we seem to be calling the “watch root”), then change a file below that somewhere, the value returned is relative to the watch root, not the project root.
For example, I have ember project test123, in which the directory 'app' exists. If I do a 'watch-project' on 'app', then change the file app/styles/app.css, the change notification I get back is for file "styles/app.css", not "app/styles/app.css".
Is the documentation wrong, or is the implementation wrong? Or am I misunderstanding something?
The text was updated successfully, but these errors were encountered: