Follow symlinks outside of watched folder #232

ArjhanToteck · 2024-03-07T02:24:47Z

Solves #231 and updates the README to match. Next.js depends on this package, and this would make development much nicer (vercel/next.js#53175).

linux-foundation-easycla · 2024-03-07T02:24:51Z

The committers listed above are authorized under a signed CLA.

✅ login: ArjhanToteck (5b22101, 409ef51, c54fa9f, a5f52e9, d8e305f, c54bf67, 0cd204b, 289ce5a)

alexander-akait · 2024-03-07T11:28:09Z

Looks like tests are failed, can you fix it?

Solves webpack#231 and updates the README to match.

ArjhanToteck · 2024-03-08T03:20:11Z

Still got a lot of remote environments failing the tests, but the machines I've tested on have been passing 100% of the unit tests reliably. I'll see if I can reproduce the failures somehow, unless it's not necessary as the required ones are passing.

ArjhanToteck · 2024-03-10T00:49:46Z

It's always the polling servers failing and I don't know why.

alexander-akait · 2024-03-11T11:41:24Z


         should detect a change to the direct symlink:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/vsts/work/1/s/test/Watchpack.js)
      at Test.Runnable._timeoutError (/home/vsts/work/1/s/node_modules/mocha/lib/runnable.js:440:10)
      at Timeout.<anonymous> (/home/vsts/work/1/s/node_modules/mocha/lib/runnable.js:251:24)
      at listOnTimeout (internal/timers.js:554:17)
      at processTimers (internal/timers.js:497:7)

  2) Watchpack
       "before each" hook:

      Uncaught AssertionError: expected Array [] to equal Array [ '/home/vsts/work/1/s/test/fixtures/link2' ] (at length, A has 0 and B has 1)
      + expected - actual

      -[]
      +[
      +  "/home/vsts/work/1/s/test/fixtures/link2"
      +]
      
      at Assertion.fail (/home/vsts/work/1/s/node_modules/should/lib/assertion.js:92:17)
      at Assertion.value (/home/vsts/work/1/s/node_modules/should/lib/assertion.js:164:19)
      at /home/vsts/work/1/s/test/Watchpack.js:1290:37
      at Watchpack.<anonymous> (/home/vsts/work/1/s/test/Watchpack.js:1261:6)
      at Watchpack.emit (events.js:314:20)
      at Watchpack._onTimeout (/home/vsts/work/1/s/lib/watchpack.js:9:22455)
      at listOnTimeout (internal/timers.js:554:17)
      at processTimers (internal/timers.js:497:7)

alexander-akait · 2024-03-11T11:41:44Z

Will it help?

ArjhanToteck · 2024-03-11T18:00:25Z

Well, I've already seen the logs, I'm just wondering if you know whats different about the polling servers vs the normal ones, as only the polling servers are reliably failing these tests.

alexander-akait · 2024-03-12T14:40:06Z

I did some changes, please rebase

codecov · 2024-03-12T15:16:08Z

Codecov Report

Attention: Patch coverage is 61.29032% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.15%. Comparing base (585ebf9) to head (d8e305f).

Files	Patch %	Lines
lib/DirectoryWatcher.js	61.29%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #232      +/-   ##
==========================================
- Coverage   91.55%   90.15%   -1.41%     
==========================================
  Files           6        6              
  Lines        1042     1056      +14     
  Branches      250      256       +6     
==========================================
- Hits          954      952       -2     
- Misses         88      104      +16

Flag	Coverage Δ
integration	`90.15% <61.29%> (-1.41%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alexander-akait · 2024-03-12T15:18:38Z

Weird... Need to to tests...

ArjhanToteck · 2024-03-12T16:10:56Z

Ok so the issue is with the WATCHPACK_POLLING env variable. I guess that's what was different about those "polling" servers. I'm guessing the initial parameter in doScan is probably what's screwing things up, I'll look into fixing it.

alexander-akait · 2024-03-12T16:17:22Z

Feel free to feedback

alexander-akait · 2024-03-15T17:25:27Z

lib/DirectoryWatcher.js

@@ -106,13 +106,14 @@ class DirectoryWatcher extends EventEmitter {
 						}
 					}
 				};
+				this.watcher.on("change", this.onWatchEvent.bind(this));


I am afraid about perf here...

Didn't end up being necessary anyway, the new solution is simpler

ArjhanToteck · 2024-03-16T02:38:35Z

Tests are stuck again, should work this time tho

ArjhanToteck · 2024-03-17T19:57:07Z

The only thing now is that Watchpack Errors will print for a moment when a followed symlink is invalid, which is the case for a few of the tests, however this seems like it should be the correct behavior anyway, unless we don't want to warn users at all when an invalid symlink is watched.

ArjhanToteck · 2024-03-17T20:54:36Z

Ok looks like Windows is still failing when polling actually

alexander-akait · 2024-03-18T10:23:36Z

lib/DirectoryWatcher.js

+							this.watcherManager.options.followSymlinks
+						) {
+							try {
+								symlinkStats = fs.statSync(itemPath);


Do we need to do it in the sync way?

It should now be able to watch symlinks that lead to outside of the watched folder when polling

alexander-akait · 2024-07-31T04:54:56Z

/cc @sokra Can you review it?

ArjhanToteck · 2024-08-01T19:24:52Z

there was a change i had made at one point that had fixed polling on the ubuntu servers, i probably shouldn't have force pushed, i just didn't want to have 1000 commits lol. Let me see if I can figure out what it had been before a code review, the one thing I can't figure out is why it doesnt work on windows.
Also, there's something wrong with the macos test servers, they aren't installing or running node.js properly at all.

alexander-akait · 2024-08-13T11:50:59Z

Looks like two tests are broken on all OS...

Follow symlinks

409ef51

Solves webpack#231 and updates the README to match.

ArjhanToteck force-pushed the main branch from 1168d2a to 409ef51 Compare March 8, 2024 02:15

Added test to detect symlinked file change outside watched directory

289ce5a

ArjhanToteck changed the title ~~Follow symlinks~~ Follow symlinks outside of watched folder Mar 8, 2024

Using both lstat and stat in onWatchEvent was unnecessary

5b22101

ArjhanToteck force-pushed the main branch from 3b12542 to fa80ba6 Compare March 10, 2024 00:35

ArjhanToteck force-pushed the main branch from fa80ba6 to 101507e Compare March 11, 2024 18:10

calling both lstat and stat isnt needed in doScan

c54fa9f

ArjhanToteck force-pushed the main branch from 101507e to c54fa9f Compare March 11, 2024 18:29

wondering if its just slow

0cd204b

ArjhanToteck added 2 commits March 12, 2024 08:57

Merge branch 'webpack:main' into main

a5f52e9

Update Watchpack.js

c54bf67

ArjhanToteck force-pushed the main branch 3 times, most recently from fb9accd to fa835be Compare March 12, 2024 17:50

alexander-akait reviewed Mar 15, 2024

View reviewed changes

ArjhanToteck force-pushed the main branch from 6072f3b to b403508 Compare March 16, 2024 02:10

ArjhanToteck force-pushed the main branch 2 times, most recently from 41032a3 to 8aafd4a Compare March 17, 2024 19:53

alexander-akait reviewed Mar 18, 2024

View reviewed changes

ArjhanToteck force-pushed the main branch from 696a67a to ef86439 Compare May 19, 2024 01:05

ArjhanToteck force-pushed the main branch from ef86439 to 8e28857 Compare July 27, 2024 05:36

Fixed for polling servers

d8e305f

It should now be able to watch symlinks that lead to outside of the watched folder when polling

ArjhanToteck force-pushed the main branch from 8e28857 to d8e305f Compare July 27, 2024 05:40

alexander-akait requested a review from sokra July 31, 2024 04:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow symlinks outside of watched folder #232

Follow symlinks outside of watched folder #232

ArjhanToteck commented Mar 7, 2024

linux-foundation-easycla bot commented Mar 7, 2024 •

edited

Loading

alexander-akait commented Mar 7, 2024

ArjhanToteck commented Mar 8, 2024 •

edited

Loading

ArjhanToteck commented Mar 10, 2024

alexander-akait commented Mar 11, 2024

alexander-akait commented Mar 11, 2024

ArjhanToteck commented Mar 11, 2024

alexander-akait commented Mar 12, 2024

codecov bot commented Mar 12, 2024 •

edited

Loading

alexander-akait commented Mar 12, 2024

ArjhanToteck commented Mar 12, 2024

alexander-akait commented Mar 12, 2024

alexander-akait Mar 15, 2024

ArjhanToteck Mar 16, 2024

ArjhanToteck commented Mar 16, 2024

ArjhanToteck commented Mar 17, 2024

ArjhanToteck commented Mar 17, 2024

alexander-akait Mar 18, 2024

alexander-akait commented Jul 31, 2024

ArjhanToteck commented Aug 1, 2024

alexander-akait commented Aug 13, 2024

Follow symlinks outside of watched folder #232

Are you sure you want to change the base?

Follow symlinks outside of watched folder #232

Conversation

ArjhanToteck commented Mar 7, 2024

linux-foundation-easycla bot commented Mar 7, 2024 • edited Loading

alexander-akait commented Mar 7, 2024

ArjhanToteck commented Mar 8, 2024 • edited Loading

ArjhanToteck commented Mar 10, 2024

alexander-akait commented Mar 11, 2024

alexander-akait commented Mar 11, 2024

ArjhanToteck commented Mar 11, 2024

alexander-akait commented Mar 12, 2024

codecov bot commented Mar 12, 2024 • edited Loading

Codecov Report

alexander-akait commented Mar 12, 2024

ArjhanToteck commented Mar 12, 2024

alexander-akait commented Mar 12, 2024

alexander-akait Mar 15, 2024

Choose a reason for hiding this comment

ArjhanToteck Mar 16, 2024

Choose a reason for hiding this comment

ArjhanToteck commented Mar 16, 2024

ArjhanToteck commented Mar 17, 2024

ArjhanToteck commented Mar 17, 2024

alexander-akait Mar 18, 2024

Choose a reason for hiding this comment

alexander-akait commented Jul 31, 2024

ArjhanToteck commented Aug 1, 2024

alexander-akait commented Aug 13, 2024

linux-foundation-easycla bot commented Mar 7, 2024 •

edited

Loading

ArjhanToteck commented Mar 8, 2024 •

edited

Loading

codecov bot commented Mar 12, 2024 •

edited

Loading