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

[windows] Watchman freezes when a symlink is present in node_modules #390

Closed
Venryx opened this issue Nov 26, 2016 · 8 comments
Closed

[windows] Watchman freezes when a symlink is present in node_modules #390

Venryx opened this issue Nov 26, 2016 · 8 comments
Labels

Comments

@Venryx
Copy link

Venryx commented Nov 26, 2016

Watchman freezes when a symlink is present in node_modules on Windows.

Here are the two log messages before and after the point of freeze:

// this is the external, source folder where the npm module resides

2016-11-26T06:52:08,198: [client=000001BA4E1AD160:stm=000001BA504EA0A0] PERF: {"start_time": 1480171924.651, "pid": 17872, "description": "dispatch_command:watch-project", "meta": {"root": {"case_sensitive": false, "ticks": 2, "path": "C:\\Root\\Apps\\@V\\@Modules\\react-native-scrollable-tab-view\\Custom", "recrawl_count": 0, "number": 3, "watcher": "win32"}, "args": ["watch-project", "C:\\Root\\Apps\\@V\\@Modules\\react-native-scrollable-tab-view\\Custom"]}, "version": "4.7.0", "elapsed_time": 3.5470000000000002}

// freeze occurs during the process logged below, of crawling the main project folder, which contains "node_modules/<the symlink to the module above>"

2016-11-26T06:54:45,523: [io C:\Root\Apps\@V\LucidLink\Main] PERF: {"start_time": 1480171922.7149999, "pid": 17872, "description": "full-crawl", "meta": {"root": {"case_sensitive": false, "ticks": 2, "path": "C:\\Root\\Apps\\@V\\LucidLink\\Main", "recrawl_count": 0, "number": 2, "watcher": "win32"}}, "version": "4.7.0", "elapsed_time": 162.80799999999999}

The only reason it unfroze is that I deleted the symlink after 3 minutes of waiting. (I've waited much longer before and it never resolves)

I've also looked at the file-activity of the watchman process, and when it's stuck it seems to repeatedly gain and release a handle to the node_modules folder.

I've tried adding the symlink file/folder to the .watchmanconfig ignore_dirs, as well as adding it to the rn-cli.config.js getBlacklistRE function, but they seem to do nothing. I even went deep into the packager javascript code and did all sorts of modifications, including forcibly removing the symlink-scan results and ensuring there was only one root specified, but: while the console output of the packager no longer said it scanned the symlinked folder, the freeze in watchman still occurred.

Unfortunately, the problem seems to be in watchman itself rather than anything currently configurable. After about 4 hours of digging around in the JS, I figured I'd give up on the self-fixing approach, and see if I could learn anything here on the GitHub project. (and I have read through much of the discussion on GitHub and elsewhere concerning support for symlinks and/or support for Windows, but didn't find anything specifically on this issue)

I read that Windows support is not a high priority at this point, but figured it's worth putting out a mention of the issue anyway.

@Venryx
Copy link
Author

Venryx commented Nov 26, 2016

I've tried putting the symlink in a newly-made "./RNM" (react native modules) folder, and watchman gets stuck "crawling" that too (endlessly creating and releasing a handle to the RNM folder). So whatever is wrong, it happens even when there's only one symlink in an otherwise empty folder.

@wez
Copy link
Contributor

wez commented Nov 29, 2016

It's been a little while since I looked at symlink handling on windows. From what I recall, I chose to skip it in the current windows implementation because they required special privileges to create and that didn't seem to match up with the target use case: repos managed by non-privileged users.

Adding more awareness of windows symlinks means more formally moving away from the MSVCRT functions for working with the file system and (more importantly in the context of this discussion) refactoring watchman to not assume that it has POSIX filesystem APIs available.

That is something that will soon become more feasible; I have a stack of diffs that is starting to lead in that direction, but it is not a current goal among the folks working on Watchman at FB. We will happily accept contributions that help us get there; we just aren't focusing on that ourselves right now.

@wez wez changed the title Watchman freezes when a symlink is present in node_modules [windows] Watchman freezes when a symlink is present in node_modules Dec 2, 2016
@mlabrum
Copy link

mlabrum commented May 16, 2017

How hard would it be to atleast get watchman to skip over windows symlinks so that it doesn't go into a infinite loop?

I'm using this in react native, and react native will setup watchman to watch the "real" paths instead of the symlinked paths, the only issue at the moment is watchman going into a loop....

I would be willing to attempt this if I was pointed in somewhat the right direction

Cheers,

@wez
Copy link
Contributor

wez commented May 16, 2017

I'm working on a patch series that should improve things here on windows at the moment, stay tuned!

@mlabrum
Copy link

mlabrum commented May 16, 2017

Awesome! Do you have any rough timelines of when that will be out?

@wez
Copy link
Contributor

wez commented May 19, 2017

The patch series is complete and being reviewed internally; I hope to push these changes by early next week

@wez
Copy link
Contributor

wez commented Jun 2, 2017

The latest builds should have much better behavior with symlinks on windows.

https://ci.appveyor.com/api/projects/wez/watchman/artifacts/watchman.zip?branch=master&job=Environment:+WATCHMAN_WIN7_COMPAT%3D always resolves to the most recent build from master.

Could you try this out and let me know if that resolves your issue?

@wez wez added the windows label Jun 2, 2017
@wez
Copy link
Contributor

wez commented Jun 6, 2017

@mlabrum confirmed that this was resolved in his comments on #479, so I'm closing this issue out.

@wez wez closed this as completed Jun 6, 2017
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