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.

Sign up for GitHub

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

Jump to bottom

fs: add recursive watch for linux #45098

Merged
merged 38 commits into from
Oct 31, 2022
Merged

fs: add recursive watch for linux #45098

merged 38 commits into from
Oct 31, 2022

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Oct 20, 2022 •
edited
Loading

Fixes #36005

List

CC @nodejs/fs

@anonrig anonrig added fs Issues and PRs related to the fs subsystem / file system. notable-change PRs with changes that should be highlighted in changelogs. labels Oct 20, 2022
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Oct 20, 2022
kibertoad
kibertoad reviewed Oct 20, 2022
View reviewed changes
lib/fs.js Show resolved Hide resolved
aduh95
aduh95 reviewed Oct 20, 2022
View reviewed changes
lib/internal/fs/linux_recursive_watcher.js Outdated Show resolved Hide resolved
lib/internal/fs/linux_recursive_watcher.js Outdated Show resolved Hide resolved
lib/internal/fs/linux_recursive_watcher.js Outdated Show resolved Hide resolved
@anonrig anonrig force-pushed the fs/add-recursive-watch branch 6 times, most recently from 7275124 to a7b310d Compare October 21, 2022 23:24
@anonrig anonrig marked this pull request as ready for review October 21, 2022 23:30
@anonrig anonrig requested review from targos and removed request for kibertoad October 21, 2022 23:46
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 21, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 21, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
aduh95
aduh95 reviewed Oct 22, 2022
View reviewed changes
lib/internal/fs/linux_watcher.js Outdated Show resolved Hide resolved
lib/internal/fs/linux_watcher.js Outdated Show resolved Hide resolved
lib/internal/fs/linux_watcher.js Outdated Show resolved Hide resolved
mcollina
mcollina requested changes Oct 22, 2022
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This module uses a mix of callbacks and promises. I would prefer if it only relied on callbacks as it would be a callback API. Maybe I'm a bit too concerned about the intertwining of our two sets of APIs.

This also needs support for the promises API.

RafaelGSS added a commit that referenced this pull request Nov 10, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
b322bf5 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 10, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
35f6ec0 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 10, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
ad196fe 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 11, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
3a85432 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 11, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
facfd90 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 11, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
0237cff 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* test_runner:
  * support function mocking (Colin Ihrig) [#45326](#45326)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
RafaelGSS added a commit that referenced this pull request Nov 14, 2022
@RafaelGSS
2022-11-14, Version 19.1.0 (Current)
3770d3a 
Notable changes:

* deps:
  * update ICU to 72.1 (Michaël Zasso) [#45068](#45068)
* doc:
  * add lukekarrys to collaborators (Luke Karrys) [#45180](#45180)
  * add anonrig to collaborators (Yagiz Nizipli) [#45002](#45002)
* fs:
  * (SEMVER-MINOR) fs: add recursive watch to linux (Yagiz Nizipli) [#45098](#45098)
* lib:
  * drop fetch experimental warning (Matteo Collina) [#45287](#45287)
* test_runner:
  * support function mocking (Colin Ihrig) [#45326](#45326)
* util
  * (SEMVER-MINOR) add MIME utilities (Bradley Farias) [#21128](#21128)
  * improve textdecoder decode performance (Yagiz Nizipli) [#45294](#45294)

PR-URL: #45269
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@anonrig @danielleadams
fs: add recursive watch to linux
08d8f2e 
PR-URL: #45098
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams added a commit that referenced this pull request Dec 30, 2022
@danielleadams
2023-01-03, Version 18.13.0 'Hydrogen' (LTS)
3607ac1 
Notable changes:

Add support for externally shared js builtins:

By default Node.js is built so that all dependencies are bundled into the
Node.js binary itself. Some Node.js distributions prefer to manage dependencies
externally. There are existing build options that allow dependencies with
native code to be externalized. This commit adds additional options so that
dependencies with JavaScript code (including WASM) can also be externalized.
This addition does not affect binaries shipped by the Node.js project but
will allow other distributions to externalize additional dependencies when
needed.

Contributed by Michael Dawson in #44376

fs.watch recursive support on Linux:

`fs.watch` supports recursive watch using the `recursive: true` option.

```js
const watcher = fs.watch(testDirectory, { recursive: true });
watcher.on('change', function(event, filename) {
});
```

Contributed by Yagiz Nizipli in #45098

Support function mocking on Node.js test runner:

The `node:test` module supports mocking during testing via a top-level `mock`
object.

```js
test('spies on an object method', (t) => {
  const number = {
    value: 5,
    add(a) {
      return this.value + a;
    },
  };
  t.mock.method(number, 'add');

  assert.strictEqual(number.add(3), 8);
  assert.strictEqual(number.add.mock.calls.length, 1);
});
```

Contributed by Colin Ihrig in #45326

Other notable changes:

buffer:
  * (SEMVER-MINOR) introduce File (Khafra) #45139
build:
  * disable v8 snapshot compression by default (Joyee Cheung) #45716
crypto:
  * update root certificates (Luigi Pinca) #45490
deps:
  * update ICU to 72.1 (Michaël Zasso) #45068
doc:
  * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697
  * add Rafael to the tsc (Michael Dawson) #45691
  * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576
  * add lukekarrys to collaborators (Luke Karrys) #45180
  * add anonrig to collaborators (Yagiz Nizipli) #45002
  * deprecate url.parse() (Rich Trott) #44919
lib:
  * drop fetch experimental warning (Matteo Collina) #45287
net:
  * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731
* src:
  * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639
  * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869
test_runner:
  * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792
  * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792
tls:
  * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935
  * remove trustcor root ca certificates (Ben Noordhuis) #45776
tools:
  * update certdata.txt (Luigi Pinca) #45490
util:
  * add fast path for utf8 encoding (Yagiz Nizipli) #45412
  * improve textdecoder decode performance (Yagiz Nizipli) #45294
  * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128

PR-URL: TBD
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
Merged
danielleadams added a commit that referenced this pull request Dec 30, 2022
@danielleadams
2023-01-03, Version 18.13.0 'Hydrogen' (LTS)
e2b8a60 
Notable changes:

Add support for externally shared js builtins:

By default Node.js is built so that all dependencies are bundled into the
Node.js binary itself. Some Node.js distributions prefer to manage dependencies
externally. There are existing build options that allow dependencies with
native code to be externalized. This commit adds additional options so that
dependencies with JavaScript code (including WASM) can also be externalized.
This addition does not affect binaries shipped by the Node.js project but
will allow other distributions to externalize additional dependencies when
needed.

Contributed by Michael Dawson in #44376

fs.watch recursive support on Linux:

`fs.watch` supports recursive watch using the `recursive: true` option.

```js
const watcher = fs.watch(testDirectory, { recursive: true });
watcher.on('change', function(event, filename) {
});
```

Contributed by Yagiz Nizipli in #45098

Support function mocking on Node.js test runner:

The `node:test` module supports mocking during testing via a top-level `mock`
object.

```js
test('spies on an object method', (t) => {
  const number = {
    value: 5,
    add(a) {
      return this.value + a;
    },
  };
  t.mock.method(number, 'add');

  assert.strictEqual(number.add(3), 8);
  assert.strictEqual(number.add.mock.calls.length, 1);
});
```

Contributed by Colin Ihrig in #45326

Other notable changes:

buffer:
  * (SEMVER-MINOR) introduce File (Khafra) #45139
build:
  * disable v8 snapshot compression by default (Joyee Cheung) #45716
crypto:
  * update root certificates (Luigi Pinca) #45490
deps:
  * update ICU to 72.1 (Michaël Zasso) #45068
doc:
  * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697
  * add Rafael to the tsc (Michael Dawson) #45691
  * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576
  * add lukekarrys to collaborators (Luke Karrys) #45180
  * add anonrig to collaborators (Yagiz Nizipli) #45002
  * deprecate url.parse() (Rich Trott) #44919
lib:
  * drop fetch experimental warning (Matteo Collina) #45287
net:
  * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731
* src:
  * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639
  * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869
test_runner:
  * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792
  * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792
tls:
  * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935
  * remove trustcor root ca certificates (Ben Noordhuis) #45776
tools:
  * update certdata.txt (Luigi Pinca) #45490
util:
  * add fast path for utf8 encoding (Yagiz Nizipli) #45412
  * improve textdecoder decode performance (Yagiz Nizipli) #45294
  * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128

PR-URL: #46025
@danielleadams
Copy link
Contributor

Hi @anonrig - this did not land cleanly on v18.x. When landing there were variables used that were not defined in v18.x-staging branch's version of the file. Could you open a backport PR? Thank you.

@danielleadams danielleadams added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Dec 30, 2022
danielleadams added a commit that referenced this pull request Dec 30, 2022
@danielleadams
2023-01-03, Version 18.13.0 'Hydrogen' (LTS)
cabeade 
Notable changes:

Add support for externally shared js builtins:

By default Node.js is built so that all dependencies are bundled into the
Node.js binary itself. Some Node.js distributions prefer to manage dependencies
externally. There are existing build options that allow dependencies with
native code to be externalized. This commit adds additional options so that
dependencies with JavaScript code (including WASM) can also be externalized.
This addition does not affect binaries shipped by the Node.js project but
will allow other distributions to externalize additional dependencies when
needed.

Contributed by Michael Dawson in #44376

fs.watch recursive support on Linux:

`fs.watch` supports recursive watch using the `recursive: true` option.

```js
const watcher = fs.watch(testDirectory, { recursive: true });
watcher.on('change', function(event, filename) {
});
```

Contributed by Yagiz Nizipli in #45098

Support function mocking on Node.js test runner:

The `node:test` module supports mocking during testing via a top-level `mock`
object.

```js
test('spies on an object method', (t) => {
  const number = {
    value: 5,
    add(a) {
      return this.value + a;
    },
  };
  t.mock.method(number, 'add');

  assert.strictEqual(number.add(3), 8);
  assert.strictEqual(number.add.mock.calls.length, 1);
});
```

Contributed by Colin Ihrig in #45326

Other notable changes:

buffer:
  * (SEMVER-MINOR) introduce File (Khafra) #45139
build:
  * disable v8 snapshot compression by default (Joyee Cheung) #45716
crypto:
  * update root certificates (Luigi Pinca) #45490
deps:
  * update ICU to 72.1 (Michaël Zasso) #45068
doc:
  * add doc-only deprecation for headers/trailers setters (Rich Trott) #45697
  * add Rafael to the tsc (Michael Dawson) #45691
  * deprecate use of invalid ports in `url.parse` (Antoine du Hamel) #45576
  * add lukekarrys to collaborators (Luke Karrys) #45180
  * add anonrig to collaborators (Yagiz Nizipli) #45002
  * deprecate url.parse() (Rich Trott) #44919
lib:
  * drop fetch experimental warning (Matteo Collina) #45287
net:
  * (SEMVER-MINOR) add autoSelectFamily and autoSelectFamilyAttemptTimeout options (Paolo Insogna) #44731
* src:
  * (SEMVER-MINOR) add uvwasi version (Jithil P Ponnan) #45639
  * (SEMVER-MINOR) add initial shadow realm support (Chengzhong Wu) #42869
test_runner:
  * (SEMVER-MINOR) add t.after() hook (Colin Ihrig) #45792
  * (SEMVER-MINOR) don't use a symbol for runHook() (Colin Ihrig) #45792
tls:
  * (SEMVER-MINOR) add "ca" property to certificate object (Ben Noordhuis) #44935
  * remove trustcor root ca certificates (Ben Noordhuis) #45776
tools:
  * update certdata.txt (Luigi Pinca) #45490
util:
  * add fast path for utf8 encoding (Yagiz Nizipli) #45412
  * improve textdecoder decode performance (Yagiz Nizipli) #45294
  * (SEMVER-MINOR) add MIME utilities (#21128) (Bradley Farias) #21128

PR-URL: #46025
@anonrig
Copy link
Member Author

anonrig commented Jan 3, 2023

@danielleadams It seems this pull request is in the v18.x-staging branch: https://github.com/nodejs/node/blob/v18.13.0-proposal/lib/internal/fs/recursive_watch.js

@anonrig anonrig removed the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Jan 3, 2023
@torchiaf torchiaf mentioned this pull request Jan 12, 2023
Closed
@juanarbol
Copy link
Member

@danielleadams It seems this pull request is in the v18.x-staging branch: https://github.com/nodejs/node/blob/v18.13.0-proposal/lib/internal/fs/recursive_watch.js

That link is a 404; marking this is a "backport-requested-v18.x" again.

@juanarbol juanarbol added the backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. label Jan 26, 2023
@anonrig
Copy link
Member Author

anonrig commented Jan 26, 2023

This pull request is already backported @juanarbol

Referencing: https://github.com/nodejs/node/blob/v18.x-staging/lib/internal/fs/recursive_watch.js

@aduh95
Copy link
Contributor

aduh95 commented Jan 27, 2023

That's not correct @anonrig, the pull request was not backported, it never showed on the v18.x changelog. The file you linked was added with c2f0377 (which is a backport for #45265), but all the other changes in this PR still need to be backported manually (docs and tests).

@nascarsayan nascarsayan mentioned this pull request Mar 12, 2023
Closed
@schlichtanders schlichtanders mentioned this pull request Oct 9, 2023
Closed
@raphaelboukara
Copy link

Hi :)
Does this fix will be backported to v18.x ?

@mcollina
Copy link
Member

mcollina commented Jan 15, 2024 •
edited
Loading

I don't expect this to be backported, because there are issues with this and Node.js v18 is in maintenance.

See #48437 for more details.

@richardlau richardlau added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. and removed backport-requested-v18.x PRs awaiting manual backport to the v18.x-staging branch. labels Jan 15, 2024
@arenekosreal arenekosreal mentioned this pull request Feb 18, 2024
Closed
@kirjavascript kirjavascript mentioned this pull request Apr 18, 2024
Merged
@RedYetiDev RedYetiDev mentioned this pull request Oct 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott approved these changes

@kibertoad kibertoad kibertoad left review comments

@targos targos targos left review comments

@cagataycali cagataycali cagataycali left review comments

@aduh95 aduh95 aduh95 approved these changes

@V-for-Vasili V-for-Vasili V-for-Vasili left review comments

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@MoLow MoLow MoLow approved these changes

@bnoordhuis bnoordhuis Awaiting requested review from bnoordhuis

@cjihrig cjihrig Awaiting requested review from cjihrig

@richardlau richardlau Awaiting requested review from richardlau

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Recursive fs.watch for linux