From 426949073f265d23d5ee49a5c27aa507309e0168 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Tue, 28 May 2024 21:48:04 -0700 Subject: [PATCH 1/3] node:fs/promises.cp should make path if dest does not exist --- src/bun.js/node/node_fs.zig | 7 ++++++- test/js/node/fs/fs.test.ts | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 07853d9d73be3f..90e42da241e6f7 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -6627,7 +6627,12 @@ pub const NodeFS = struct { mode_ |= C.darwin.COPYFILE_EXCL; } - return ret.errnoSysP(C.copyfile(src, dest, null, mode_), .copyfile, src) orelse ret.success; + const first_try = ret.errnoSysP(C.copyfile(src, dest, null, mode_), .copyfile, src) orelse return ret.success; + if (first_try == .err and first_try.err.errno == @intFromEnum(C.E.NOENT)) { + bun.makePath(std.fs.cwd(), bun.path.dirname(dest, .auto)) catch {}; + return ret.errnoSysP(C.copyfile(src, dest, null, mode_), .copyfile, src) orelse ret.success; + } + return first_try; } if (Environment.isLinux) { diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index b91a53334321af..61f567695ffe95 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -3216,3 +3216,18 @@ it("promises.fdatasync with a bad fd should include that in the error thrown", a } expect.unreachable(); }); + +it("promises.cp should work even if dest does not exist", async () => { + const x_dir = tmpdirSync(); + let src = "package-lock.json"; + let folder = "folder-not-exist"; + let dst = join(folder, src); + + src = join(x_dir, src); + folder = join(x_dir, folder); + dst = join(x_dir, dst); + + await promises.writeFile(src, "A".repeat(131073)); + await promises.rm(folder, { recursive: true, force: true }); + await promises.cp(src, dst); +}); From 9faeb805bd0b4a61614a99918a201799276af2f4 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Wed, 29 May 2024 17:54:39 -0700 Subject: [PATCH 2/3] add windows support --- src/bun.js/node/node_fs.zig | 52 ++++++++++++++--------------------- src/bun.js/node/types.zig | 8 ++++++ src/bun.zig | 10 +++++++ src/resolver/resolve_path.zig | 11 ++++++++ 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig index 90e42da241e6f7..4f0d29e66b5204 100644 --- a/src/bun.js/node/node_fs.zig +++ b/src/bun.js/node/node_fs.zig @@ -6779,30 +6779,30 @@ pub const NodeFS = struct { } if (Environment.isWindows) { + const src_enoent_maybe = ret.initErrWithP(.ENOENT, .copyfile, this.osPathIntoSyncErrorBuf(src)); + const dst_enoent_maybe = ret.initErrWithP(.ENOENT, .copyfile, this.osPathIntoSyncErrorBuf(dest)); const stat_ = reuse_stat orelse switch (windows.GetFileAttributesW(src)) { - windows.INVALID_FILE_ATTRIBUTES => return .{ .err = .{ - .errno = @intFromEnum(C.SystemErrno.ENOENT), - .syscall = .copyfile, - .path = this.osPathIntoSyncErrorBuf(src), - } }, + windows.INVALID_FILE_ATTRIBUTES => return ret.errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(src)).?, else => |result| result, }; if (stat_ & windows.FILE_ATTRIBUTE_REPARSE_POINT == 0) { if (windows.CopyFileW(src, dest, @intFromBool(mode.shouldntOverwrite())) == 0) { - const err = windows.GetLastError(); - const errpath = switch (err) { - .FILE_EXISTS, .ALREADY_EXISTS => dest, - else => src, - }; - return shouldIgnoreEbusy( - args.src, - args.dest, - Maybe(Return.CopyFile).errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(errpath)) orelse .{ .err = .{ - .errno = @intFromEnum(C.SystemErrno.ENOENT), - .syscall = .copyfile, - .path = this.osPathIntoSyncErrorBuf(src), - } }, - ); + var err = windows.GetLastError(); + var errpath: bun.OSPathSliceZ = undefined; + switch (err) { + .FILE_EXISTS, .ALREADY_EXISTS => errpath = dest, + .PATH_NOT_FOUND => { + bun.makePathW(std.fs.cwd(), bun.path.dirnameW(dest)) catch {}; + const second_try = windows.CopyFileW(src, dest, @intFromBool(mode.shouldntOverwrite())); + if (second_try > 0) return ret.success; + err = windows.GetLastError(); + errpath = dest; + if (err == .FILE_EXISTS or err == .ALREADY_EXISTS) errpath = src; + }, + else => errpath = src, + } + const result = ret.errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest)) orelse src_enoent_maybe; + return shouldIgnoreEbusy(args.src, args.dest, result); } return ret.success; } else { @@ -6813,24 +6813,14 @@ pub const NodeFS = struct { var wbuf: bun.WPathBuffer = undefined; const len = bun.windows.GetFinalPathNameByHandleW(handle.cast(), &wbuf, wbuf.len, 0); if (len == 0) { - return Maybe(Return.CopyFile).errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest)) orelse .{ .err = .{ - .errno = @intFromEnum(C.SystemErrno.ENOENT), - .syscall = .copyfile, - .path = this.osPathIntoSyncErrorBuf(dest), - } }; + return ret.errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest)) orelse dst_enoent_maybe; } const flags = if (stat_ & windows.FILE_ATTRIBUTE_DIRECTORY != 0) std.os.windows.SYMBOLIC_LINK_FLAG_DIRECTORY else 0; if (windows.CreateSymbolicLinkW(dest, wbuf[0..len :0], flags) == 0) { - return Maybe(Return.CopyFile).errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest)) orelse .{ - .err = .{ - .errno = @intFromEnum(C.SystemErrno.ENOENT), - .syscall = .copyfile, - .path = this.osPathIntoSyncErrorBuf(dest), - }, - }; + return ret.errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest)) orelse dst_enoent_maybe; } return ret.success; } diff --git a/src/bun.js/node/types.zig b/src/bun.js/node/types.zig index c94293e46f109b..e721ab341e8963 100644 --- a/src/bun.js/node/types.zig +++ b/src/bun.js/node/types.zig @@ -169,6 +169,14 @@ pub fn Maybe(comptime ReturnTypeT: type, comptime ErrorTypeT: type) type { return .{ .err = e }; } + pub inline fn initErrWithP(e: C.SystemErrno, syscall: Syscall.Tag, path: anytype) Maybe(ReturnType, ErrorType) { + return .{ .err = .{ + .errno = @intFromEnum(e), + .syscall = syscall, + .path = path, + } }; + } + pub inline fn asErr(this: *const @This()) ?ErrorType { if (this.* == .err) return this.err; return null; diff --git a/src/bun.zig b/src/bun.zig index 6a31ebe3566a51..f1c01aa7fed493 100644 --- a/src/bun.zig +++ b/src/bun.zig @@ -2539,6 +2539,16 @@ pub fn makePath(dir: std.fs.Dir, sub_path: []const u8) !void { } } +/// Like std.fs.Dir.makePath except instead of infinite looping on dangling +/// symlink, it deletes the symlink and tries again. +pub fn makePathW(dir: std.fs.Dir, sub_path: []const u16) !void { + // was going to copy/paste makePath and use all W versions but they didn't all exist + // and this buffer was needed anyway + var buf: PathBuffer = undefined; + const buf_len = simdutf.convert.utf16.to.utf8.le(sub_path, &buf); + return makePath(dir, buf[0..buf_len]); +} + pub const Async = @import("async"); /// This is a helper for writing path string literals that are compatible with Windows. diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig index f1e384582b0ac9..7c706ed9cd1061 100644 --- a/src/resolver/resolve_path.zig +++ b/src/resolver/resolve_path.zig @@ -489,6 +489,17 @@ pub fn dirname(str: []const u8, comptime platform: Platform) []const u8 { } } +pub fn dirnameW(str: []const u16) []const u16 { + const separator = lastIndexOfSeparatorWindowsT(u16, str) orelse { + // return disk designator instead + if (str.len < 2) return &.{}; + if (!(str[1] == ':')) return &.{}; + if (!bun.path.isDriveLetterT(u16, str[0])) return &.{}; + return str[0..2]; + }; + return str[0..separator]; +} + threadlocal var relative_from_buf: bun.PathBuffer = undefined; threadlocal var relative_to_buf: bun.PathBuffer = undefined; From 48e76e7df2cf572645d426aac1cc6bf239317151 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Wed, 29 May 2024 17:54:56 -0700 Subject: [PATCH 3/3] test the text was written too --- test/js/node/fs/fs.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts index 61f567695ffe95..9c19ee7fba66b6 100644 --- a/test/js/node/fs/fs.test.ts +++ b/test/js/node/fs/fs.test.ts @@ -3219,6 +3219,7 @@ it("promises.fdatasync with a bad fd should include that in the error thrown", a it("promises.cp should work even if dest does not exist", async () => { const x_dir = tmpdirSync(); + const text_expected = "A".repeat(131073); let src = "package-lock.json"; let folder = "folder-not-exist"; let dst = join(folder, src); @@ -3227,7 +3228,10 @@ it("promises.cp should work even if dest does not exist", async () => { folder = join(x_dir, folder); dst = join(x_dir, dst); - await promises.writeFile(src, "A".repeat(131073)); + await promises.writeFile(src, text_expected); await promises.rm(folder, { recursive: true, force: true }); await promises.cp(src, dst); + + const text_actual = await Bun.file(dst).text(); + expect(text_actual).toBe(text_expected); });