Async file copying on windows #8649
Conversation
src/bun.js/node/node_fs.zig
Outdated
| else | ||
| 0; | ||
| if (windows.CreateSymbolicLinkW(dest, wbuf[0..len :0], flags) == 0) { | ||
| if (Maybe(Return.CopyFile).errnoSysP(0, .copyfile, this.osPathIntoSyncErrorBuf(dest))) |e| { |
There was a problem hiding this comment.
i think you can pass the rc of the api call into this function, instead of 0. it will return null on success. this will eliminate the "should be unreachable" thing.
There was a problem hiding this comment.
passing rc or 0 doesn't make a difference because rc is known to be zero. The main issue is that constructing the error object may fail, for example if we detect that the status code is actually a success.
I've gone ahead and changed these to instead construct errors with a generic ENOENT code, so that if these do ever get hit the error is at least somewhat helpful.
| @@ -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 { | |||
There was a problem hiding this comment.
this is fine but later this would be legendary to make into a generic wrapper for
"wrap any function with type"
There was a problem hiding this comment.
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 🥲
There was a problem hiding this comment.
see std.meta.ArgsTuple
it’s like variadics but worse
src/resolver/resolve_path.zig
Outdated
| return struct { | ||
| allow_above_root: bool = false, | ||
| separator: T = std.fs.path.sep, | ||
| isSeparator: fn (T) bool = makeIsSepAnyT(T), |
There was a problem hiding this comment.
i wish this could be a comptime field but i suspect it isnt so shrimple to do so is it?
There was a problem hiding this comment.
which field do you think should be comptime?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
|
❌ @gvilums 4 files with test failures on bun-darwin-aarch64:
|
|
❌ @gvilums 3 files with test failures on linux-x64: |
|
❌ @gvilums 3 files with test failures on linux-x64-baseline: |
|
❌ @gvilums 4 files with test failures on bun-darwin-x64:
|
❌🪟 @gvilums, there are 11 test regressions on Windows x86_64
|
|
awesome |
What does this PR do?
This PR implements
fs.promises.cpon windows, and fixes some errors with symlinks withfs.cpSync.How did you verify your code works?
Tests in
cp.test.ts