search

FredrikNoren / ungit

The easiest way to use git. On any platform. Anywhere.
MIT License
10.44k stars 637 forks source link

Node watch #1313

Closed wmertens closed 4 years ago

wmertens commented 4 years ago

Use the node-watch drop-in replacement to provide recursive watching on Linux

Fixes #1164

campersau commented 4 years ago

For me this is currently crashing the server on windows because the filenames are now absolute and not relative as before: https://github.com/FredrikNoren/ungit/blob/9133ebd68476abc7a12510df7d5537c435256e08/source/git-api.js#L87

Looks like we don't have a UI test with gitignore files...

Here is the full stack trace:

2020-04-21T16:49:00.712Z - error: RangeError: path should be a `path.relative()`d string, but got "D:\dev_projects\ungit\.git\index.lock"
    at throwError (D:\dev_projects\ungit\node_modules\ignore\index.js:337:9)
    at checkPath (D:\dev_projects\ungit\node_modules\ignore\index.js:356:12)
    at Ignore._test (D:\dev_projects\ungit\node_modules\ignore\index.js:473:5)
    at Ignore.ignores (D:\dev_projects\ungit\node_modules\ignore\index.js:512:17)
    at path (D:\dev_projects\ungit\node_modules\ignore\index.js:516:26)
    at Array.filter (<anonymous>)
    at Ignore.filter (D:\dev_projects\ungit\node_modules\ignore\index.js:520:29)
    at isFileWatched (D:\dev_projects\ungit\source\git-api.js:87:26)
    at Watcher.watcher.on (D:\dev_projects\ungit\source\git-api.js:63:15)
    at Watcher.emit (events.js:198:13)
    at D:\dev_projects\ungit\node_modules\node-watch\lib\watch.js:305:19
    at D:\dev_projects\ungit\node_modules\node-watch\lib\watch.js:39:7
    at D:\dev_projects\ungit\node_modules\node-watch\lib\watch.js:303:7
    at D:\dev_projects\ungit\node_modules\node-watch\lib\watch.js:109:10
    at Array.forEach (<anonymous>)
    at Timeout.handle [as _onTimeout] (D:\dev_projects\ungit\node_modules\node-watch\lib\watch.js:104:24)
Stopped keeping ungit alive
wmertens commented 4 years ago

3 options:

I'll look into 3 first

wmertens commented 4 years ago

I picked option 2 - does it work for you now?

campersau commented 4 years ago

Yes, it works now!

campersau commented 4 years ago

Thanks!

ylecuyer commented 4 years ago

Looks like this PR was merged too fast. I have the following error when opening a big project:

> ungit@1.5.6 start /home/ylecuyer/Projects/ungit
> node ./bin/ungit

## Ungit started ##

Took 346ms to start server.
Navigate to http://localhost:8448/#/repository?path=%2Fhome%2Fylecuyer%2FProjects%2Fbigproject
2020-04-24T07:34:11.470Z - error: Error: ENOSPC: System limit for number of file watchers reached, watch '/home/ylecuyer/Projects/bigproject/tmp/cache/assets/test/sprockets/v3.0/T9'
    at FSWatcher.start (internal/fs/watchers.js:165:26)
    at Object.watch (fs.js:1258:11)
    at /home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/watch.js:368:22
    at hasNativeRecursive (/home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/has-native-recursive.js:61:12)
    at Watcher.watchDirectory (/home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/watch.js:357:3)
    at /home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/watch.js:383:14
    at /home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/watch.js:151:35
    at Array.forEach (<anonymous>)
    at /home/ylecuyer/Projects/ungit/node_modules/node-watch/lib/watch.js:149:13
    at FSReqWrap.args [as oncomplete] (fs.js:140:20)
Stopped keeping ungit alive

IMO we should use .gitignore to feed ignored files t the watcher, or fallback to legacy watcher when the new watcher can't start

And we should add a big repo to the test suite to be sure it keeps on working cause we only have tiny repos in the test suite atm

campersau commented 4 years ago

Thanks for testing @ylecuyer ! Yes I agree, but feeding in .gitignore is currently not possible, see https://github.com/yuanchuan/node-watch/issues/93 So I am going to revert it later today.

wmertens commented 4 years ago

@ylecuyer so I'll get recursive filtering into node-watch, and it should also not die on that error, just show a message.

How can I show a message in the browser? So when the resursive watcher runs out of nodes, it shows that + a link to instructions for increasing the limit.

🤔 node-watch should also somehow keep watching with the watches it has - better make it do width-first recursion, and ignore-but-notify this particular error.