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

node:fs/promises.cp should make path if dest does not exist #11433

Merged
merged 3 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 27 additions & 32 deletions src/bun.js/node/node_fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
nektro marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down Expand Up @@ -6774,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 {
Expand All @@ -6808,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;
}
Expand Down
8 changes: 8 additions & 0 deletions src/bun.js/node/types.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 10 additions & 0 deletions src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions src/resolver/resolve_path.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 19 additions & 0 deletions test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3216,3 +3216,22 @@ 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();
const text_expected = "A".repeat(131073);
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, text_expected);
await promises.rm(folder, { recursive: true, force: true });
await promises.cp(src, dst);
nektro marked this conversation as resolved.
Show resolved Hide resolved

const text_actual = await Bun.file(dst).text();
expect(text_actual).toBe(text_expected);
});
Loading