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

watchman query fail #302

Closed
Piasy opened this issue Dec 23, 2016 · 14 comments
Closed

watchman query fail #302

Piasy opened this issue Dec 23, 2016 · 14 comments

Comments

@Piasy
Copy link
Contributor

Piasy commented Dec 23, 2016

My project dir is a child of git root dir, so when I run watchman watch-project it returns:

{
    "version": "4.7.0",
    "watch": "/Users/piasy/src/Yolo/piasyRepo/YOLO-Android",
    "watcher": "fsevents",
    "relative_path": "YOLO-Android-Client"
}

And when buckw query /Users/piasy/src/Yolo/piasyRepo/YOLO-Android/YOLO-Android-Client, it fails:

{"version":"4.7.0","error":"unable to resolve root /Users/piasy/src/Yolo/piasyRepo/YOLO-Android/YOLO-Android-Client: directory /Users/piasy/src/Yolo/piasyRepo/YOLO-Android/YOLO-Android-Client is not watched"}

Hope buckw take care of relative_path.

@kageiit
Copy link
Contributor

kageiit commented Dec 23, 2016

It cannot because buckw has no way to know what the project root is. Is your buckw at the project root but your project is in a sub directory? Ideally buckw should reside side by side to the root build.gradle. Can you add some info on what the exact project setup looks like so we can look at possible solutions?

@Piasy
Copy link
Contributor Author

Piasy commented Dec 23, 2016

Project root is the git repo root, I mean.

My project looks like below:

screen shot 2016-12-24 at 07 41 41

And I have make buckw work manually, by replace query from $WORKING_DIR into query from $PROJECT_DIR, where it is the parent of $WORKING_DIR.

I think the result of watch-project is useful.

@wez
Copy link

wez commented Jan 25, 2017

Hey folks, I'm the watchman guy. I've just had a user report a problem in facebook/watchman#426 (which was formerly #255 for you) and I suspect it is due to this line:

okbuck/buckw

Line 62 in cb912a9

watchman watch-project $WORKING_DIR >/dev/null 2>&1

There are a couple of problems here:

  1. Using watch-project allows watchman to consolidate your watch with a pre-existing watch to reduce overall resource consumption. It is the responsibility of the caller to feed the relative_path into any queries it generates, otherwise the results will be wrong
  2. Some environments may have configured watchman to prohibit watching certain directories and the watch-project call can fail.
  3. There is no error handling in the buckw script and any errors are suppressed, leading to increased head scratching and support burden

I'd recommend that you fixup the script to properly consume the results from watch-project, or alternatively to use watchman watch instead of watchman watch-project. Doing so will fix 1. but will break 2.

@kageiit
Copy link
Contributor

kageiit commented Jan 25, 2017

@wez Thanks for the insights. The only other watch that happens at the same root is the watch setup by the buck build that follows the watch in the buckw script. Since we only watch very few categories of files by default, the performance impact may not be as bad if we setup a separate watch and not use watch-project. Will investigate more into this.

Regarding error handling, we do perform that in

okbuck/buckw

Line 135 in cb912a9

TO_CLEAN=$(getToClean | jsonq '"\n".join(obj["files"])')
i.e if the watchman query is not returning expected results, we fallback to other mechanisms. @vanniktech You may want to update your wrapper script to check if you are seeing the same.

@wez
Copy link

wez commented Jan 25, 2017

You need to keep in mind that watchman is a shared resource; there are other tools that may establish watches and may influence the results of the watch-project call depending on which one runs first.

You may also want to try dropping in an empty .watchmanconfig file at the location you consider to be the root to hint to watchman that you'd prefer it to aggregate at that location.

I'd be happy to help review and advise on any changes around this!

@kageiit
Copy link
Contributor

kageiit commented Jan 25, 2017

We do have a .watchmanconfig that we use with contents like

{
    "ignore_dirs": [
        "buck-out",
        ".idea",
        ".buckd",
        "build",
        ".gradle",
        ".swp",
        "infer-out",
        "java"
    ],
    "fsevents_latency": 0.05,
    "suppress_recrawl_warnings": true
}

Is there anything in particular we should pay attention to in terms of configuration?

@wez
Copy link

wez commented Jan 25, 2017

is that .watchmanconfig deployed to $WORKING_DIR? I don't see that in the screenshot that @Piasy included earlier in this thread. @vanniktech: do you have a /Users/nik/tmp/okbuck/.watchmanconfig file?

re: the contents, the ignore_dirs is a list of dirs; that .swp entry looks like it might be trying to match a vim .swp file, so that doesn't seem effective in this context. On macOS the first 8 entries in ignore_dirs can be filtered out for us in fseventsd, so you want to prioritize the most noisy directories first; buck-out is a good choice to list first. The other things in there seem ok.

@wez
Copy link

wez commented Jan 25, 2017

hijacking this thread for general feedback on the watchman usage in the wrapper; these are just some micro-optimization perf nits:

  • Prefer match over imatch. Watchman will automatically use case insensitive matching if the underlying filesystem requires it. Case sensitive matches and globs are faster to execute because they transform O(n) directory walks to O(1) lookups in some code paths. This is mostly moot on macOS but nicer for Linux users.
  • Prefer ["suffix", "sq"] over ["imatch", "**/*.sq", "wholename"] because it only needs to compare 3 characters rather than compute the entire path name and match the whole thing; wholename requires eagerly computing the path to a matched node and this can be a perf hit if you have a large tree where many of the files are not matched by your filter.
  • Prefer ["name", "lint.xml"] over ["imatch", "**/lint.xml", "wholename"], as it is similarly less expensive to compute

@vanniktech
Copy link

@kageiit I am using the latest version of buckw

@wez I do not have a file named .watchmanconfig

@wez
Copy link

wez commented Jan 26, 2017

@vanniktech try these steps:

watchman watch-del-all
touch /Users/nik/tmp/okbuck/.watchmanconfig
<then run the ok buck wrapper>

@vanniktech
Copy link

@wez this does work. Thank you so much.

@kageiit also updated my original ticket with the working solution so if anyone stumbles upon this they know how to fix it.

@kageiit
Copy link
Contributor

kageiit commented Jan 26, 2017

@vanniktech thanks!

I am going to cleanup the wrapper with @wez 's suggestions and okbuck will automatically include a .watchmanconfig when creating the wrapper if you do not have one. Ill add more info to the wiki once this is available

@vanniktech
Copy link

Okay cool. Looking forward to the changes, thank you everyone once again.

@kageiit
Copy link
Contributor

kageiit commented Jan 27, 2017

@wez I have cleaned up the watchman queries thanks to your suggestions. Please leave any feedback you have on #359

@hanjunx
Copy link

hanjunx commented Feb 2, 2017 via email

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

No branches or pull requests

5 participants