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: remove race condition for recursive watch on Linux #51406

Merged
merged 8 commits into from
Jan 25, 2024
Merged

fs: remove race condition for recursive watch on Linux #51406

merged 8 commits into from
Jan 25, 2024

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented Jan 8, 2024

Fixes #48437

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jan 8, 2024
@mcollina
Copy link
Member Author

mcollina commented Jan 8, 2024

Supersedes #51049 with a complete fix.

anonrig
anonrig approved these changes Jan 8, 2024
View reviewed changes
Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
fixup
0101f44 
Signed-off-by: Matteo Collina <[email protected]>
@anonrig anonrig added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jan 8, 2024
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 8, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 8, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56714/

@mcollina
fixup
4586010 
Signed-off-by: Matteo Collina <[email protected]>
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 11, 2024
@mcollina
fixup
ff16eca 
Signed-off-by: Matteo Collina <[email protected]>
@anonrig
Copy link
Member

anonrig commented Jan 11, 2024

Linter is failing @mcollina

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 11, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56750/

@mcollina
update old tests
63d8245 
Signed-off-by: Matteo Collina <[email protected]>
@mcollina
major rewrite of Linux recursive watch
faf947d 
Signed-off-by: Matteo Collina <[email protected]>
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56819/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56837/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56844/

@mcollina
fixup
00ce833 
Signed-off-by: Matteo Collina <[email protected]>
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56846/

@mcollina
fixup
607bf16 
Signed-off-by: Matteo Collina <[email protected]>
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 22, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56883/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56912/

@mcollina
Copy link
Member Author

@anonrig @richardlau PTAL, this is good to land.

@mcollina mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 25, 2024
@nodejs-github-bot nodejs-github-bot merged commit 2791e83 into nodejs:main Jan 25, 2024
54 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 2791e83

@mcollina mcollina mentioned this pull request Jan 26, 2024
Closed
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 2, 2024
@mcollina @marco-ippolito
fs: remove race condition for recursive watch on Linux
192a3fa 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: nodejs#51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Feb 9, 2024
@mcollina @rdw-msft
fs: remove race condition for recursive watch on Linux
3cec9d8 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: nodejs#51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@MoLow MoLow mentioned this pull request Feb 13, 2024
Closed
targos pushed a commit that referenced this pull request Feb 15, 2024
@mcollina @targos
fs: remove race condition for recursive watch on Linux
15a7f21 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Feb 19, 2024
@mcollina @marco-ippolito
fs: remove race condition for recursive watch on Linux
4bb807c 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: nodejs#51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@marco-ippolito marco-ippolito mentioned this pull request Mar 1, 2024
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@mcollina @richardlau
fs: remove race condition for recursive watch on Linux
9254e60 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@mcollina @richardlau
fs: remove race condition for recursive watch on Linux
bbd1351 
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #51406
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Merged
@kylo5aby kylo5aby mentioned this pull request Mar 26, 2024
Closed
@benmccann benmccann mentioned this pull request May 20, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau left review comments

@anonrig anonrig anonrig approved these changes

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@MoLow MoLow MoLow approved these changes

@atlowChemi atlowChemi Awaiting requested review from atlowChemi

@cjihrig cjihrig Awaiting requested review from cjihrig

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. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Node 20 recursive file watching is misbehaving on Linux
6 participants
@mcollina @nodejs-github-bot @anonrig @richardlau @MoLow @marco-ippolito