[windows] fix(fs.watch) (#8746)

* clean fixes

* [autofix.ci] apply automated fixes

* more tests passing

* EPERM instead of AccessDenied

* [autofix.ci] apply automated fixes

* fix test on windows

* cleanup

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
Ciro Spaciari
2024-02-07 20:44:07 -03:00
committed by GitHub
parent b61282e290
commit 5df59cb02b
4 changed files with 67 additions and 49 deletions

View File

@@ -1,6 +1,7 @@
const std = @import("std");
const bun = @import("root").bun;
const uv = bun.windows.libuv;
const windows = bun.windows;
const uv = windows.libuv;
const Path = @import("../../resolver/resolve_path.zig");
const Fs = @import("../../fs.zig");
const Mutex = @import("../../lock.zig").Lock;
@@ -48,7 +49,8 @@ pub const PathWatcherManager = struct {
errdefer bun.default_allocator.free(cloned_path);
const dir = bun.openDirAbsolute(cloned_path[0..cloned_path.len]) catch |err| {
if (err == error.ENOENT) {
log("openDirAbsolute({s}) err {}", .{ cloned_path, err });
if (err == error.ENOTDIR) {
const file = try bun.openFileZ(cloned_path, .{ .mode = .read_only });
const result = PathInfo{
.fd = bun.toFD(file.handle),
@@ -87,7 +89,6 @@ pub const PathWatcherManager = struct {
.watcher_count = 0,
});
errdefer this.destroy();
return this;
}
@@ -242,36 +243,36 @@ pub const PathWatcher = struct {
return;
}
const path = if (filename) |file| file[0..bun.len(file) :0] else this.path.path;
const path = if (filename) |file| file[0..bun.len(file) :0] else return;
// if we are watching a file we already have the file info
const path_info = if (this.path.is_file) this.path else brk: {
// we need the absolute path to get the file info
var buf: [bun.MAX_PATH_BYTES + 1]u8 = undefined;
var parts = [_]string{ this.path.path, path };
@memcpy(buf[0..this.path.path.len], this.path.path);
buf[this.path.path.len] = std.fs.path.sep;
const cwd_z = buf[0 .. this.path.path.len + 1];
var joined_buf: [bun.MAX_PATH_BYTES + 1]u8 = undefined;
const file_path = Path.joinAbsStringBuf(
cwd_z,
&joined_buf,
&parts,
.windows,
);
// we need the absolute path to get the file info
var buf: [bun.MAX_PATH_BYTES + 1]u8 = undefined;
var parts = [_]string{path};
const cwd = Path.dirname(this.path.path, .windows);
@memcpy(buf[0..cwd.len], cwd);
buf[cwd.len] = std.fs.path.sep;
joined_buf[file_path.len] = 0;
const file_path_z = joined_buf[0..file_path.len :0];
break :brk manager._fdFromAbsolutePathZ(file_path_z) catch return;
};
var joined_buf: [bun.MAX_PATH_BYTES + 1]u8 = undefined;
const file_path = Path.joinAbsStringBuf(
buf[0 .. cwd.len + 1],
&joined_buf,
&parts,
.windows,
);
joined_buf[file_path.len] = 0;
const file_path_z = joined_buf[0..file_path.len :0];
const path_info = manager._fdFromAbsolutePathZ(file_path_z) catch return;
defer manager._decrementPathRef(path);
defer {
if (!this.path.is_file) {
manager._decrementPathRef(path_info.path);
}
}
defer this.flush();
// events always use the relative path
if (events & uv.UV_RENAME != 0) {
this.emit(path, path_info.hash, timestamp, path_info.is_file, .rename);
}
if (events & uv.UV_CHANGE != 0) {
this.emit(path, path_info.hash, timestamp, path_info.is_file, .change);
}
this.emit(path, path_info.hash, timestamp, path_info.is_file, if (events & uv.UV_RENAME != 0) .rename else .change);
}
pub fn init(manager: *PathWatcherManager, path: PathWatcherManager.PathInfo, recursive: bool, callback: Callback, updateEndCallback: UpdateEndCallback, ctx: ?*anyopaque) !*PathWatcher {
@@ -290,8 +291,17 @@ pub const PathWatcher = struct {
return error.FailedToInitializeFSEvent;
}
this.handle.data = this;
const event_path = brk: {
var outbuf: [bun.MAX_PATH_BYTES]u8 = undefined;
const size = bun.sys.readlink(path.path, &outbuf).unwrap() catch break :brk path.path;
if (size >= bun.MAX_PATH_BYTES) break :brk path.path;
outbuf[size] = 0;
break :brk outbuf[0..size];
};
// UV_FS_EVENT_RECURSIVE only works for Windows and OSX
if (uv.uv_fs_event_start(&this.handle, PathWatcher.uvEventCallback, path.path.ptr, if (recursive) uv.UV_FS_EVENT_RECURSIVE else 0) != 0) {
if (uv.uv_fs_event_start(&this.handle, PathWatcher.uvEventCallback, event_path.ptr, if (recursive) uv.UV_FS_EVENT_RECURSIVE else 0) != 0) {
return error.FailedToStartFSEvent;
}
// we handle this in node_fs_watcher

View File

@@ -1201,6 +1201,10 @@ pub fn send(fd: bun.FileDescriptor, buf: []const u8, flag: u32) Maybe(usize) {
}
pub fn readlink(in: [:0]const u8, buf: []u8) Maybe(usize) {
if (comptime Environment.isWindows) {
return sys_uv.readlink(in, buf);
}
while (true) {
const rc = sys.readlink(in, buf.ptr, buf.len);

View File

@@ -77,6 +77,7 @@ pub fn chmod(file_path: [:0]const u8, flags: bun.Mode) Maybe(void) {
assertIsValidWindowsPath(u8, file_path);
var req: uv.fs_t = uv.fs_t.uninitialized;
defer req.deinit();
const rc = uv.uv_fs_chmod(uv.Loop.get(), &req, file_path.ptr, flags, null);
log("uv chmod({s}, {d}) = {d}", .{ file_path, flags, rc.int() });
@@ -265,7 +266,7 @@ pub fn fdatasync(fd: FileDescriptor) Maybe(void) {
log("uv fdatasync({}) = {d}", .{ uv_fd, rc.int() });
return if (rc.errno()) |errno|
.{ .err = .{ .errno = errno, .syscall = .fstat, .fd = fd, .from_libuv = true } }
.{ .err = .{ .errno = errno, .syscall = .fdatasync, .fd = fd, .from_libuv = true } }
else
.{ .result = {} };
}
@@ -278,7 +279,7 @@ pub fn fsync(fd: FileDescriptor) Maybe(void) {
log("uv fsync({d}) = {d}", .{ uv_fd, rc.int() });
return if (rc.errno()) |errno|
.{ .err = .{ .errno = errno, .syscall = .fstat, .fd = fd, .from_libuv = true } }
.{ .err = .{ .errno = errno, .syscall = .fsync, .fd = fd, .from_libuv = true } }
else
.{ .result = {} };
}
@@ -304,7 +305,7 @@ pub fn lstat(path: [:0]const u8) Maybe(bun.Stat) {
log("uv lstat({s}) = {d}", .{ path, rc.int() });
return if (rc.errno()) |errno|
.{ .err = .{ .errno = errno, .syscall = .fstat, .from_libuv = true } }
.{ .err = .{ .errno = errno, .syscall = .lstat, .from_libuv = true } }
else
.{ .result = req.statbuf };
}

View File

@@ -25,6 +25,8 @@ const testDir = tempDirWithFiles("watch", {
[encodingFileName]: "hello",
});
const isWindows = process.platform === "win32";
describe("fs.watch", () => {
test("non-persistent watcher should not block the event loop", done => {
try {
@@ -74,7 +76,7 @@ describe("fs.watch", () => {
watcher.on("change", (event, filename) => {
count++;
try {
expect(event).toBe("rename");
expect(["rename", "change"]).toContain(event);
expect(["new-file.txt", "new-folder.txt"]).toContain(filename);
if (count >= 2) {
watcher.close();
@@ -114,7 +116,7 @@ describe("fs.watch", () => {
if (basename === "subfolder") return;
count++;
try {
expect(event).toBe("rename");
expect(["rename", "change"]).toContain(event);
expect(["new-file.txt", "new-folder.txt"]).toContain(basename);
if (count >= 2) {
watcher.close();
@@ -145,7 +147,7 @@ describe("fs.watch", () => {
let err: Error | undefined = undefined;
const watcher = fs.watch(testsubdir, function (event, filename) {
try {
expect(event).toBe("rename");
expect(["rename", "change"]).toContain(event);
expect(filename).toBe("deleted.txt");
} catch (e: any) {
err = e;
@@ -405,28 +407,29 @@ describe("fs.watch", () => {
expect(promise).resolves.toBe("change");
});
test("should throw if no permission to watch the directory", async () => {
// on windows 0o200 will be readable (match nodejs behavior)
test.skipIf(isWindows)("should throw if no permission to watch the directory", async () => {
const filepath = path.join(testDir, "permission-dir");
fs.mkdirSync(filepath, { recursive: true });
await fs.promises.chmod(filepath, 0o200);
fs.chmodSync(filepath, 0o200);
try {
const watcher = fs.watch(filepath);
watcher.close();
expect("unreacheable").toBe(false);
expect.unreachable();
} catch (err: any) {
expect(err.message.indexOf("AccessDenied") !== -1).toBeTrue();
}
});
test("should throw if no permission to watch the file", async () => {
const filepath = path.join(testDir, "permission-file");
fs.writeFileSync(filepath, "hello.txt");
await fs.promises.chmod(filepath, 0o200);
test.skipIf(isWindows)("should throw if no permission to watch the file", async () => {
const filepath = path.join(testDir, "permission-file.txt");
fs.writeFileSync(filepath, "hello.txt");
fs.chmodSync(filepath, 0o200);
try {
const watcher = fs.watch(filepath);
watcher.close();
expect("unreacheable").toBe(false);
expect.unreachable();
} catch (err: any) {
expect(err.message.indexOf("AccessDenied") !== -1).toBeTrue();
}
@@ -455,7 +458,7 @@ describe("fs.promises.watch", () => {
for await (const event of watcher) {
count++;
try {
expect(event.eventType).toBe("rename");
expect(["rename", "change"]).toContain(event.eventType);
expect(["new-file.txt", "new-folder.txt"]).toContain(event.filename);
if (count >= 2) {
@@ -502,7 +505,7 @@ describe("fs.promises.watch", () => {
count++;
try {
expect(event.eventType).toBe("rename");
expect(["rename", "change"]).toContain(event.eventType);
expect(["new-file.txt", "new-folder.txt"]).toContain(basename);
if (count >= 2) {
@@ -577,7 +580,7 @@ describe("fs.promises.watch", () => {
return event.eventType;
}
} catch {
expect("unreacheable").toBe(false);
expect.unreachable();
} finally {
clearInterval(interval);
}
@@ -605,7 +608,7 @@ describe("fs.promises.watch", () => {
return event.eventType;
}
} catch {
expect("unreacheable").toBe(false);
expect.unreachable();
} finally {
clearInterval(interval);
}
@@ -628,7 +631,7 @@ describe("fs.promises.watch", () => {
return event.eventType;
}
} catch (e: any) {
expect("unreacheable").toBe(false);
expect.unreachable();
} finally {
clearInterval(interval);
}