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

Async file copying on windows #8649

Merged
merged 12 commits into from
Feb 4, 2024
Merged

Async file copying on windows #8649

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9902059
rework and simplify file opening in windows
gvilums Feb 2, 2024
aa2e622
fix tests
gvilums Feb 2, 2024
405d44b
symlink tests pass
gvilums Feb 2, 2024
44861a1
update error handling
gvilums Feb 2, 2024
93688df
remove outdated normalization check
gvilums Feb 2, 2024
0b89c49
fix mac build
gvilums Feb 2, 2024
e608070
apply suggested fixes
gvilums Feb 2, 2024
b32a5ef
fix path tests
gvilums Feb 3, 2024
c19be55
remove debug print
gvilums Feb 3, 2024
948665a
Merge remote-tracking branch 'origin/main' into georgijs/cpwin
gvilums Feb 3, 2024
08c460c
fix windows compile
gvilums Feb 3, 2024
e1bceb9
Merge remote-tracking branch 'origin/main' into georgijs/cpwin
gvilums Feb 4, 2024
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
4 changes: 2 additions & 2 deletions src/StandaloneModuleGraph.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ pub const StandaloneModuleGraph = struct {
Global.exit(1);
};

const file = bun.sys.ntCreateFile(
const file = bun.sys.openFileAtWindows(
bun.invalid_fd,
out,
// access_mask
Expand Down Expand Up @@ -716,7 +716,7 @@ pub const StandaloneModuleGraph = struct {
var nt_path_buf: bun.WPathBuffer = undefined;
const nt_path = bun.strings.addNTPathPrefix(&nt_path_buf, image_path);

return bun.sys.ntCreateFile(
return bun.sys.openFileAtWindows(
bun.invalid_fd,
nt_path,
// access_mask
Expand Down
8 changes: 6 additions & 2 deletions src/bun.js/node/dir_iterator.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,12 @@ pub fn NewIterator(comptime use_windows_ospath: bool) type {

const kind = blk: {
const attrs = dir_info.FileAttributes;
if (attrs & w.FILE_ATTRIBUTE_DIRECTORY != 0) break :blk Entry.Kind.directory;
if (attrs & w.FILE_ATTRIBUTE_REPARSE_POINT != 0) break :blk Entry.Kind.sym_link;
const isdir = attrs & w.FILE_ATTRIBUTE_DIRECTORY != 0;
const islink = attrs & w.FILE_ATTRIBUTE_REPARSE_POINT != 0;
// on windows symlinks can be directories, too. We prioritize the
// "sym_link" kind over the "directory" kind
if (islink) break :blk Entry.Kind.sym_link;
if (isdir) break :blk Entry.Kind.directory;
break :blk Entry.Kind.file;
};

Expand Down
232 changes: 153 additions & 79 deletions src/bun.js/node/node_fs.zig
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/cli/run_command.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1377,7 +1377,7 @@ pub const RunCommand = struct {
if (Environment.allow_assert) {
std.debug.assert(std.fs.path.isAbsoluteWindowsWTF16(path_to_use));
}
const handle = (bun.sys.ntCreateFile(
const handle = (bun.sys.openFileAtWindows(
bun.invalid_fd, // absolute path is given
path_to_use,
w.STANDARD_RIGHTS_READ | w.FILE_READ_DATA | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE,
Expand Down
139 changes: 103 additions & 36 deletions src/resolver/resolve_path.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,15 @@ pub fn normalizeStringGeneric(
) []u8 {
return normalizeStringGenericT(u8, path_, buf, allow_above_root, separator, isSeparator, preserve_trailing_slash);
}

fn separatorAdapter(comptime T: type, func: anytype) fn (T) bool {
Copy link
Member

Choose a reason for hiding this comment

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

this is fine but later this would be legendary to make into a generic wrapper for

"wrap any function with type"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I agree, could probably live in the bun namespace. Only problem is that zig doesn't support variadics, so it can't be fully generalized 🥲

Copy link
Collaborator

Choose a reason for hiding this comment

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

see std.meta.ArgsTuple

it’s like variadics but worse

return struct {
fn call(char: T) bool {
return func(T, char);
}
}.call;
}

pub fn normalizeStringGenericT(
comptime T: type,
path_: []const T,
Expand All @@ -632,7 +641,34 @@ pub fn normalizeStringGenericT(
comptime isSeparatorT: anytype,
comptime preserve_trailing_slash: bool,
) []T {
const isWindows, const sep_str = comptime .{ separator == std.fs.path.sep_windows, &[_]u8{separator} };
return normalizeStringGenericTZ(T, path_, buf, .{
.allow_above_root = allow_above_root,
.separator = separator,
.isSeparator = separatorAdapter(T, isSeparatorT),
.preserve_trailing_slash = preserve_trailing_slash,
.zero_terminate = false,
.add_nt_prefix = false,
});
}

pub fn NormalizeOptions(comptime T: type) type {
return struct {
allow_above_root: bool = false,
separator: T = std.fs.path.sep,
isSeparator: fn (T) bool = makeIsSepAnyT(T),
Copy link
Member

Choose a reason for hiding this comment

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

i wish this could be a comptime field but i suspect it isnt so shrimple to do so is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which field do you think should be comptime?

Copy link
Member

Choose a reason for hiding this comment

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

isSeparator. if i remember correctly it was previously a comptime function. i might be misremembering. what is significant is that i am less certain this function will be inlined. in the previous code (if im right and it used comptime function), then it is almost certain that LLVM inlined a function for this == thing

Copy link
Contributor Author

@gvilums gvilums Feb 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

because the entire struct is only ever used in a comptime context I think this should be basically equivalent to passing a comptime parameter (correct me if I'm wrong on that one)

preserve_trailing_slash: bool = false,
zero_terminate: bool = false,
add_nt_prefix: bool = false,
};
}

pub fn normalizeStringGenericTZ(
comptime T: type,
path_: []const T,
buf: []T,
comptime options: NormalizeOptions(T),
) if (options.zero_terminate) [:0]T else []T {
const isWindows, const sep_str = comptime .{ options.separator == std.fs.path.sep_windows, &[_]u8{options.separator} };

if (isWindows and bun.Environment.isDebug) {
// this is here to catch a potential mistake by the caller
Expand All @@ -644,64 +680,83 @@ pub fn normalizeStringGenericT(

var buf_i: usize = 0;
var dotdot: usize = 0;
var path_begin: usize = 0;

const volLen, const indexOfThirdUNCSlash = if (isWindows and !allow_above_root)
const volLen, const indexOfThirdUNCSlash = if (isWindows and !options.allow_above_root)
windowsVolumeNameLenT(T, path_)
else
.{ 0, 0 };

if (isWindows and !allow_above_root) {
if (isWindows and !options.allow_above_root) {
if (volLen > 0) {
if (options.add_nt_prefix) {
@memcpy(buf[buf_i .. buf_i + 4], &comptime strings.literalBuf(T, "\\??\\"));
buf_i += 4;
}
if (path_[1] != ':') {
// UNC paths
buf[0..2].* = comptime strings.literalBuf(T, sep_str ++ sep_str);
@memcpy(buf[2 .. indexOfThirdUNCSlash + 1], path_[2 .. indexOfThirdUNCSlash + 1]);
buf[indexOfThirdUNCSlash] = separator;
@memcpy(buf[buf_i .. buf_i + 2], &comptime strings.literalBuf(T, sep_str ++ sep_str));
@memcpy(buf[buf_i + 2 .. buf_i + indexOfThirdUNCSlash + 1], path_[2 .. indexOfThirdUNCSlash + 1]);
buf[buf_i + indexOfThirdUNCSlash] = options.separator;
@memcpy(
buf[indexOfThirdUNCSlash + 1 .. volLen],
buf[buf_i + indexOfThirdUNCSlash + 1 .. buf_i + volLen],
path_[indexOfThirdUNCSlash + 1 .. volLen],
);
buf[volLen] = separator;
buf_i = volLen + 1;
buf[buf_i + volLen] = options.separator;
buf_i += volLen + 1;
path_begin = volLen + 1;

// it is just a volume name
if (buf_i >= path_.len)
return buf[0..buf_i];
// if (buf_i >= path_.len) {
// if (options.zero_terminate) {
// buf[buf_i] = 0;
// return buf[0..buf_i :0];
// } else {
// return buf[0..buf_i];
// }
// }
} else {
// drive letter
buf[0] = path_[0];
buf[1] = ':';
buf_i = 2;
buf[buf_i] = path_[0];
buf[buf_i + 1] = ':';
buf_i += 2;
dotdot = buf_i;
path_begin = 2;
}
} else if (path_.len > 0 and isSeparatorT(T, path_[0])) {
buf[buf_i] = separator;
} else if (path_.len > 0 and options.isSeparator(path_[0])) {
buf[buf_i] = options.separator;
buf_i += 1;
dotdot = 1;
path_begin = 1;
}
}
if (isWindows and allow_above_root) {
if (isWindows and options.allow_above_root) {
if (path_.len >= 2 and path_[1] == ':') {
buf[0] = path_[0];
buf[1] = ':';
buf_i = 2;
if (options.add_nt_prefix) {
@memcpy(buf[buf_i .. buf_i + 4], &comptime strings.literalBuf(T, "\\??\\"));
buf_i += 4;
}
buf[buf_i] = path_[0];
buf[buf_i + 1] = ':';
buf_i += 2;
dotdot = buf_i;
path_begin = 2;
}
}

var r: usize = 0;
var path, const buf_start = if (isWindows)
.{ path_[buf_i..], buf_i }
.{ path_[path_begin..], buf_i }
else
.{ path_, 0 };

const n = path.len;

if (isWindows and (allow_above_root or volLen > 0)) {
if (isWindows and (options.allow_above_root or volLen > 0)) {
// consume leading slashes on windows
if (r < n and isSeparatorT(T, path[r])) {
if (r < n and options.isSeparator(path[r])) {
r += 1;
buf[buf_i] = separator;
buf[buf_i] = options.separator;
buf_i += 1;
}
}
Expand All @@ -710,26 +765,26 @@ pub fn normalizeStringGenericT(
// empty path element
// or
// . element
if (isSeparatorT(T, path[r])) {
if (options.isSeparator(path[r])) {
r += 1;
continue;
}

if (path[r] == '.' and (r + 1 == n or isSeparatorT(T, path[r + 1]))) {
if (path[r] == '.' and (r + 1 == n or options.isSeparator(path[r + 1]))) {
// skipping two is a windows-specific bugfix
r += 1;
continue;
}

if (@"is .. with type"(T, path[r..]) and (r + 2 == n or isSeparatorT(T, path[r + 2]))) {
if (@"is .. with type"(T, path[r..]) and (r + 2 == n or options.isSeparator(path[r + 2]))) {
r += 2;
// .. element: remove to last separator
if (buf_i > dotdot) {
buf_i -= 1;
while (buf_i > dotdot and !isSeparatorT(T, buf[buf_i])) {
while (buf_i > dotdot and !options.isSeparator(buf[buf_i])) {
buf_i -= 1;
}
} else if (allow_above_root) {
} else if (options.allow_above_root) {
if (buf_i > buf_start) {
buf[buf_i..][0..3].* = comptime strings.literalBuf(T, sep_str ++ "..");
buf_i += 3;
Expand All @@ -745,22 +800,22 @@ pub fn normalizeStringGenericT(

// real path element.
// add slash if needed
if (buf_i != buf_start and !isSeparatorT(T, buf[buf_i - 1])) {
buf[buf_i] = separator;
if (buf_i != buf_start and !options.isSeparator(buf[buf_i - 1])) {
buf[buf_i] = options.separator;
buf_i += 1;
}

const from = r;
while (r < n and !isSeparatorT(T, path[r])) : (r += 1) {}
while (r < n and !options.isSeparator(path[r])) : (r += 1) {}
const count = r - from;
@memcpy(buf[buf_i..][0..count], path[from..][0..count]);
buf_i += count;
}

if (preserve_trailing_slash) {
if (options.preserve_trailing_slash) {
// Was there a trailing slash? Let's keep it.
if (buf_i > 0 and path_[path_.len - 1] == separator and buf[buf_i] != separator) {
buf[buf_i] = separator;
if (buf_i > 0 and path_[path_.len - 1] == options.separator and buf[buf_i] != options.separator) {
buf[buf_i] = options.separator;
buf_i += 1;
}
}
Expand All @@ -772,7 +827,11 @@ pub fn normalizeStringGenericT(
buf_i += 1;
}

const result = buf[0..buf_i];
if (options.zero_terminate) {
buf[buf_i] = 0;
}

const result = if (options.zero_terminate) buf[0..buf_i :0] else buf[0..buf_i];

if (bun.Environment.allow_assert and isWindows) {
std.debug.assert(!strings.hasPrefixComptimeType(T, result, comptime strings.literal(T, "\\:\\")));
Expand Down Expand Up @@ -1414,6 +1473,14 @@ pub fn isSepAnyT(comptime T: type, char: anytype) bool {
return @call(.always_inline, isSepPosixT, .{ T, char }) or @call(.always_inline, isSepWin32T, .{ T, char });
}

pub fn makeIsSepAnyT(comptime T: type) fn (char: T) bool {
return struct {
pub fn call(char: T) bool {
return @call(.always_inline, isSepAnyT, .{ T, char });
}
}.call;
}

pub fn lastIndexOfSeparatorWindows(slice: []const u8) ?usize {
return lastIndexOfSeparatorWindowsT(u8, slice);
}
Expand Down
Loading
Loading