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

fs.watch(): reports delete of file as change instead of rename #27869

Closed
bpasero opened this issue May 25, 2019 · 7 comments
Closed

fs.watch(): reports delete of file as change instead of rename #27869

bpasero opened this issue May 25, 2019 · 7 comments
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.

Comments

@bpasero
Copy link
Contributor

bpasero commented May 25, 2019

  • Version: 12.0.0
  • Platform: Darwin Benjamins-MacBook-Pro.local 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64

Given the following code:

var fs = require("fs");

fs.watch('./test.txt', (event, filename) => {
    console.log(event, filename);
})

Running below in a directory where test.txt exists and gets deleted yields different results for event in 12.x vs 10.x:

  • 12.x: change
  • 10.x: rename

This basically renders node.js 12 incapable of detecting a file delete vs change when watching it.

@bpasero bpasero changed the title fs.watch(): reports deletes as changes (regression in 12.x) fs.watch(): reports deletes as change instead of rename (regression in 12.x) May 25, 2019
@bpasero bpasero changed the title fs.watch(): reports deletes as change instead of rename (regression in 12.x) fs.watch(): reports delete of file as change instead of rename (regression in 12.x) May 25, 2019
@devsnek devsnek added the fs Issues and PRs related to the fs subsystem / file system. label May 25, 2019
@devsnek
Copy link
Member

devsnek commented May 25, 2019

there was no change in the fs_event binding logic, so perhaps this was an upstream change in libuv? @nodejs/libuv

@bnoordhuis
Copy link
Member

There's been precisely one change to fsevents.c between v10.15.3 and v12.0.0: libuv/libuv@2d2af38

@devsnek
Copy link
Member

devsnek commented May 25, 2019

@bnoordhuis for those not familiar with the libuv codebase, is that commit responsible for this behaviour change?

@bpasero bpasero changed the title fs.watch(): reports delete of file as change instead of rename (regression in 12.x) fs.watch(): reports delete of file as change instead of rename (regression for node.js > 10.x) May 25, 2019
@bpasero bpasero changed the title fs.watch(): reports delete of file as change instead of rename (regression for node.js > 10.x) fs.watch(): reports delete of file as change instead of rename May 25, 2019
@himself65
Copy link
Member

@addaleax addaleax added the libuv Issues and PRs related to the libuv dependency or the uv binding. label May 26, 2019
@bnoordhuis
Copy link
Member

@devsnek I don't know, I haven't had a chance to test. But if it is a libuv issue, it must be that commit.

If someone can try reverting that patch in master or v12.x with git apply -R --directory=deps/uv and checking if that fixes the issue, that would be great.

bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue May 27, 2019
Commit 2d2af38 ("fsevents: really watch files with fsevents on macos
10.7+") from last November introduced a regression where events that
were previously reported as UV_RENAME were now reported as UV_CHANGE.
This commit rectifies that.

Fixes: nodejs/node#27869
@bnoordhuis
Copy link
Member

libuv/libuv@2d2af38 is indeed responsible for the change in behavior. I'm working on a fix.

deepak1556 pushed a commit to electron/node that referenced this issue May 27, 2019
Commit 2d2af38 ("fsevents: really watch files with fsevents on macos
10.7+") from last November introduced a regression where events that
were previously reported as UV_RENAME were now reported as UV_CHANGE.
This commit rectifies that.

Fixes: nodejs/node#27869
@Trott Trott added the confirmed-bug Issues with confirmed bugs. label May 28, 2019
vtjnash pushed a commit to vtjnash/libuv that referenced this issue May 28, 2019
Commit 2d2af38 ("fsevents: really watch files with fsevents on macos
10.7+") from last November introduced a regression where events that
were previously reported as UV_RENAME were now reported as UV_CHANGE.
This commit rectifies that.

Fixes: nodejs/node#27869
(cherry picked from commit d649d21)
codebytere pushed a commit to electron/node that referenced this issue Jun 20, 2019
Commit 2d2af38 ("fsevents: really watch files with fsevents on macos
10.7+") from last November introduced a regression where events that
were previously reported as UV_RENAME were now reported as UV_CHANGE.
This commit rectifies that.

Fixes: nodejs/node#27869
@bnoordhuis
Copy link
Member

I've bundled the reported issues in #29460. That's the issue to track now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants