-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Make Watchman a non-hard dependency #1644
Comments
It does not seem to be limited to windows, I get the same message on OS X 10.10.5 |
Updated the title to reflect that. |
Why is it required for a static build that does not |
Good question. At the moment, I think the answer may be "because nobody has taken the taken the time to isolate the parts of the code that depend on Watchman". It probably would be interesting to do so, as having a small dependency footprint is a desirable property. |
I think that's the bigger/more surprising thing. It's odd to require watchman for e.g. CI environments where:
|
Yep, I agree with you. Like I said over here:
We'd definitely welcome a pull-request that addressed this. |
Retitled this to reflect the desired solution, so removing the "docs" label, although we'd still welcome clearer docs in the meantime. |
Would this be considered rather simple to do, or are there a some gotchas involved? I have some free time and could try my best to provide a pull request. If it's going to be a bit complicated though I would probably just let somebody else with more knowledge of the internals take a shot at it. |
@ryancole: I don't think it is likely to be particularly difficult, but few things are ever as easy as you think they will... If you wanted to look at the code to gauge the complexity of it I'd suggest starting here: relay/packages/relay-compiler/bin/RelayCompilerBin.js Lines 45 to 55 in 8f719e1
That's the watch expression that the current system passes through to Watchman. To break the dependency, we need to implement simple code that knows how to identify all the files matched by that description. Doesn't look too bad, to be honest. |
👍 but I am wondering what is the suggested way around this now. Are people just installing watchman as part of the CI build? |
Yeah. It requires using a |
I'm checking in the built graphql files. It's not pretty, but way prettier than adding watchman to CI. |
The main issue from what I can see is in this line:
Code excerpt: async parseEverything(parserName: string): Promise<void> {
if (this.parsers[parserName]) {
// no need to parse
return;
}
const parserConfig = this.parserConfigs[parserName];
this.parsers[parserName] = parserConfig.getParser(parserConfig.baseDir);
// here \/
const files = await RelayCodegenWatcher.queryFiles(
parserConfig.baseDir,
parserConfig.watchmanExpression,
parserConfig.getFileFilter
? parserConfig.getFileFilter(parserConfig.baseDir)
: anyFileFilter,
);
this.parseFileChanges(parserName, files);
} Watchman is being used to return all files that relay-compiler needs to parse. This makes it that, even if no |
@sibelius I use the following in my circle.yml:
|
thanks to @eddies, we got this working on Travis by adding: before_install:
# dirty ugly watchman hack https://github.com/facebook/relay/issues/1644#issuecomment-315998313
- if [[ ! -e watchman ]]; then git clone https://github.com/facebook/watchman.git && cd watchman/ && git checkout v4.7.0 && ./autogen.sh && ./configure && make && sudo make install; fi
- sudo sysctl fs.inotify.max_user_watches=524288
- sudo sysctl -p
- cd ~/build/$TRAVIS_REPO_SLUG
# end dirty watchman hack |
Any suggestions? Why this issue is closed? |
It was resolved. You don't need Watchman any more unless you're watching. |
@taion Unless we have src as current dir. |
I'm still hitting the watchman dependency when running
However, if I add the
everything works. Why is watchman still enabled by default even when not using |
Not sure where a good place for this would be in the docs – probably the compiler section.
See #1626 for @unirey hitting this issue.
The text was updated successfully, but these errors were encountered: