Skip to content

Commit

Permalink
fix(windows): transpiler cache and other test fixes (#8471)
Browse files Browse the repository at this point in the history
* umask

* process args

* update reportError.test.ts

* file exists

* transpiler cache

* back to const

* remove failing comments

* [autofix.ci] apply automated fixes

* update comment

* debug assert and remmove branch

* oops

* escape

* path sep

* seekTo

* disable

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <[email protected]>
  • Loading branch information
3 people authored Jan 29, 2024
1 parent 1e9b44a commit c538bf8
Show file tree
Hide file tree
Showing 24 changed files with 106 additions and 49 deletions.
26 changes: 16 additions & 10 deletions src/bun.js/RuntimeTranspilerCache.zig
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,16 @@ pub const RuntimeTranspilerCache = struct {
break :brk metadata_buf[0..metadata_stream.pos];
};

const vecs: []const std.os.iovec_const = if (output_bytes.len > 0)
const vecs: []const bun.PlatformIOVecConst = if (output_bytes.len > 0)
&.{
.{ .iov_base = metadata_bytes.ptr, .iov_len = metadata_bytes.len },
.{ .iov_base = output_bytes.ptr, .iov_len = output_bytes.len },
.{ .iov_base = sourcemap.ptr, .iov_len = sourcemap.len },
bun.platformIOVecConstCreate(metadata_bytes),
bun.platformIOVecConstCreate(output_bytes),
bun.platformIOVecConstCreate(sourcemap),
}
else
&.{
.{ .iov_base = metadata_bytes.ptr, .iov_len = metadata_bytes.len },
.{ .iov_base = sourcemap.ptr, .iov_len = sourcemap.len },
bun.platformIOVecConstCreate(metadata_bytes),
bun.platformIOVecConstCreate(sourcemap),
};

var position: isize = 0;
Expand All @@ -228,8 +228,13 @@ pub const RuntimeTranspilerCache = struct {
if (bun.Environment.allow_assert) {
var total: usize = 0;
for (vecs) |v| {
std.debug.assert(v.iov_len > 0);
total += v.iov_len;
if (comptime bun.Environment.isWindows) {
std.debug.assert(v.len > 0);
total += v.len;
} else {
std.debug.assert(v.iov_len > 0);
total += v.iov_len;
}
}
std.debug.assert(end_position == total);
}
Expand All @@ -246,7 +251,7 @@ pub const RuntimeTranspilerCache = struct {
}
}

try tmpfile.finish(destination_path.sliceAssumeZ());
try tmpfile.finish(@ptrCast(std.fs.path.basename(destination_path.slice())));
}

pub fn load(
Expand Down Expand Up @@ -481,6 +486,7 @@ pub const RuntimeTranspilerCache = struct {

const file = cache_fd.asFile();
const metadata_bytes = try file.preadAll(&metadata_bytes_buf, 0);
if (comptime bun.Environment.isWindows) try file.seekTo(0);
var metadata_stream = std.io.fixedBufferStream(metadata_bytes_buf[0..metadata_bytes]);

var entry = Entry{
Expand Down Expand Up @@ -539,7 +545,7 @@ pub const RuntimeTranspilerCache = struct {
const cache_dir_fd = brk: {
if (std.fs.path.dirname(cache_file_path)) |dirname| {
const dir = try std.fs.cwd().makeOpenPath(dirname, .{ .access_sub_paths = true });
break :brk bun.toFD(dir.fd);
break :brk bun.toLibUVOwnedFD(dir.fd);
}

break :brk bun.toFD(std.fs.cwd().fd);
Expand Down
9 changes: 4 additions & 5 deletions src/bun.js/bindings/BunProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
#include <uv.h>
#include <io.h>
#include <fcntl.h>
// Using the same typedef and define for `mode_t` and `umask` as node on windows.
// https://github.com/nodejs/node/blob/ad5e2dab4c8306183685973387829c2f69e793da/src/node_process_methods.cc#L29
#define umask _umask
typedef int mode_t;
#endif
#include "JSNextTickQueue.h"
#include "ProcessBindingUV.h"
Expand Down Expand Up @@ -342,8 +346,6 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionDlopen,
JSC_DEFINE_HOST_FUNCTION(Process_functionUmask,
(JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))
{
#if !OS(WINDOWS)

if (callFrame->argumentCount() == 0 || callFrame->argument(0).isUndefined()) {
mode_t currentMask = umask(0);
umask(currentMask);
Expand Down Expand Up @@ -376,9 +378,6 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionUmask,
}

return JSC::JSValue::encode(JSC::jsNumber(umask(newUmask)));
#else
return JSC::JSValue::encode(JSC::jsNumber(0));
#endif
}

extern "C" uint64_t Bun__readOriginTimer(void*);
Expand Down
7 changes: 1 addition & 6 deletions src/bun.js/webcore/blob.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2900,14 +2900,9 @@ pub const Blob = struct {
return JSValue.jsBoolean(true);
}

if (comptime Environment.isWindows) {
this.globalThis.throwTODO("exists is not implemented on Windows");
return JSValue.jsUndefined();
}

// We say regular files and pipes exist.
// This is mostly meant for "Can we use this in new Response(file)?"
return JSValue.jsBoolean(bun.isRegularFile(store.data.file.mode) or std.os.S.ISFIFO(store.data.file.mode));
return JSValue.jsBoolean(bun.isRegularFile(store.data.file.mode) or bun.C.S.ISFIFO(store.data.file.mode));
}

// This mostly means 'can it be read?'
Expand Down
15 changes: 15 additions & 0 deletions src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ pub const PlatformIOVec = if (Environment.isWindows)
else
std.os.iovec;

pub const PlatformIOVecConst = if (Environment.isWindows)
windows.libuv.uv_buf_t
else
std.os.iovec_const;

pub fn platformIOVecCreate(input: []const u8) PlatformIOVec {
if (Environment.isWindows) return windows.libuv.uv_buf_t.init(input);
if (Environment.allow_assert) {
Expand All @@ -140,6 +145,16 @@ pub fn platformIOVecCreate(input: []const u8) PlatformIOVec {
return .{ .iov_len = @intCast(input.len), .iov_base = @constCast(input.ptr) };
}

pub fn platformIOVecConstCreate(input: []const u8) PlatformIOVecConst {
if (Environment.isWindows) return windows.libuv.uv_buf_t.init(input);
if (Environment.allow_assert) {
if (input.len > @as(usize, std.math.maxInt(u32))) {
Output.debugWarn("call to bun.PlatformIOVecConst.init with length larger than u32, this will overflow on windows", .{});
}
}
return .{ .iov_len = @intCast(input.len), .iov_base = input.ptr };
}

pub fn platformIOVecToSlice(iovec: PlatformIOVec) []u8 {
if (Environment.isWindows) return windows.libuv.uv_buf_t.slice(iovec);
return iovec.base[0..iovec.len];
Expand Down
17 changes: 15 additions & 2 deletions src/cli.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,8 @@ pub const Command = struct {

const script_name_to_search = ctx.args.entry_points[0];

var absolute_script_path: ?string = null;

var file_path = script_name_to_search;
const file_: anyerror!std.fs.File = brk: {
if (std.fs.path.isAbsoluteWindows(script_name_to_search)) {
Expand All @@ -1787,6 +1789,7 @@ pub const Command = struct {
if (comptime Environment.isWindows) {
resolved = resolve_path.normalizeString(resolved, true, .windows);
}
absolute_script_path = resolved;
break :brk bun.openFile(
resolved,
.{ .mode = .read_only },
Expand Down Expand Up @@ -1822,15 +1825,25 @@ pub const Command = struct {
Global.configureAllocator(.{ .long_running = true });

// the case where this doesn't work is if the script name on disk doesn't end with a known JS-like file extension
const absolute_script_path = bun.getFdPath(file.handle, &script_name_buf) catch return false;
absolute_script_path = absolute_script_path orelse brk: {
if (comptime !Environment.isWindows) break :brk bun.getFdPath(file.handle, &script_name_buf) catch return false;

var fd_path_buf: bun.PathBuffer = undefined;
const path = bun.getFdPath(file.handle, &fd_path_buf) catch return false;
break :brk resolve_path.normalizeString(
resolve_path.PosixToWinNormalizer.resolveCWDWithExternalBufZ(&script_name_buf, path) catch @panic("Could not resolve path"),
true,
.windows,
);
};

if (!ctx.debug.loaded_bunfig) {
bun.CLI.Arguments.loadConfigPath(ctx.allocator, true, "bunfig.toml", ctx, .RunCommand) catch {};
}

BunJS.Run.boot(
ctx.*,
absolute_script_path,
absolute_script_path.?,
) catch |err| {
if (Output.enable_ansi_colors) {
ctx.log.printForLogLevelWithEnableAnsiColors(Output.errorWriter(), true) catch {};
Expand Down
2 changes: 2 additions & 0 deletions src/cli/create_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ pub const CreateCommand = struct {
package_json_contents = try MutableString.init(ctx.allocator, size);
package_json_contents.list.expandToCapacity();

const prev_file_pos = if (comptime Environment.isWindows) try pkg.getPos() else 0;
_ = pkg.preadAll(package_json_contents.list.items, 0) catch |err| {
package_json_file = null;

Expand All @@ -565,6 +566,7 @@ pub const CreateCommand = struct {
Output.prettyErrorln("Error reading package.json: <r><red>{s}", .{@errorName(err)});
break :read_package_json;
};
if (comptime Environment.isWindows) try pkg.seekTo(prev_file_pos);
// The printer doesn't truncate, so we must do so manually
std.os.ftruncate(pkg.handle, 0) catch {};

Expand Down
2 changes: 2 additions & 0 deletions src/cli/init_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,12 @@ pub const InitCommand = struct {
package_json_contents = try MutableString.init(alloc, size);
package_json_contents.list.expandToCapacity();

const prev_file_pos = if (comptime Environment.isWindows) try pkg.getPos() else 0;
_ = pkg.preadAll(package_json_contents.list.items, 0) catch {
package_json_file = null;
break :read_package_json;
};
if (comptime Environment.isWindows) try pkg.seekTo(prev_file_pos);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/cli/install_completions_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,10 @@ pub const InstallCompletionsCommand = struct {
0,
) catch break :brk true;

if (comptime Environment.isWindows) {
try dot_zshrc.seekTo(0);
}

const contents = buf[0..read];

// Do they possibly have it in the file already?
Expand Down
2 changes: 1 addition & 1 deletion src/darwin_c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,8 @@ pub const preallocate_length = std.math.maxInt(u51);
pub const Mode = std.os.mode_t;

pub const E = std.os.E;
pub const S = std.os.S;
pub fn getErrno(rc: anytype) E {
return std.c.getErrno(rc);
}

pub extern "c" fn umask(Mode) Mode;
1 change: 0 additions & 1 deletion src/feature_flags.zig
Original file line number Diff line number Diff line change
Expand Up @@ -177,5 +177,4 @@ pub const concurrent_transpiler = !env.isWindows;
// https://github.com/oven-sh/bun/issues/5426#issuecomment-1813865316
pub const disable_auto_js_to_ts_in_node_modules = true;

// TODO: implement the IO for rtc for windows
pub const runtime_transpiler_cache = !env.isWindows;
4 changes: 4 additions & 0 deletions src/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1133,10 +1133,12 @@ pub const FileSystem = struct {
while (true) {

// We use pread to ensure if the file handle was open, it doesn't seek from the last position
const prev_file_pos = if (comptime Environment.isWindows) try file.getPos() else 0;
const read_count = file.preadAll(shared_buffer.list.items[offset..], offset) catch |err| {
fs.readFileError(path, err);
return err;
};
if (comptime Environment.isWindows) try file.seekTo(prev_file_pos);
shared_buffer.list.items = shared_buffer.list.items[0 .. read_count + offset];
file_contents = shared_buffer.list.items;
debug("pread({d}, {d}) = {d}", .{ file.handle, size, read_count });
Expand Down Expand Up @@ -1179,10 +1181,12 @@ pub const FileSystem = struct {
// stick a zero at the end
buf[size] = 0;

const prev_file_pos = if (comptime Environment.isWindows) try file.getPos() else 0;
const read_count = file.preadAll(buf, 0) catch |err| {
fs.readFileError(path, err);
return err;
};
if (comptime Environment.isWindows) try file.seekTo(prev_file_pos);
file_contents = buf[0..read_count];
debug("pread({d}, {d}) = {d}", .{ file.handle, size, read_count });

Expand Down
3 changes: 3 additions & 0 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6582,6 +6582,7 @@ pub const PackageManager = struct {
current_package_json_buf,
0,
);
if (comptime Environment.isWindows) try manager.root_package_json_file.seekTo(0);

const package_json_source = logger.Source.initPathString(
package_json_cwd,
Expand Down Expand Up @@ -6757,6 +6758,7 @@ pub const PackageManager = struct {
current_package_json_buf,
0,
);
if (comptime Environment.isWindows) try manager.root_package_json_file.seekTo(0);

const package_json_source = logger.Source.initPathString(
package_json_cwd,
Expand Down Expand Up @@ -7462,6 +7464,7 @@ pub const PackageManager = struct {
current_package_json_buf,
0,
);
if (comptime Environment.isWindows) try manager.root_package_json_file.seekTo(0);

const package_json_source = logger.Source.initPathString(
package_json_cwd,
Expand Down
1 change: 1 addition & 0 deletions src/linux_c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,7 @@ pub const IFF_LOOPBACK = net_c.IFF_LOOPBACK;

pub const Mode = u32;
pub const E = std.os.E;
pub const S = std.os.S;

pub extern "c" fn umask(Mode) Mode;

Expand Down
5 changes: 4 additions & 1 deletion src/sys.zig
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,10 @@ pub fn writev(fd: bun.FileDescriptor, buffers: []std.os.iovec) Maybe(usize) {
}
}

pub fn pwritev(fd: bun.FileDescriptor, buffers: []const std.os.iovec_const, position: isize) Maybe(usize) {
pub fn pwritev(fd: bun.FileDescriptor, buffers: []const bun.PlatformIOVecConst, position: isize) Maybe(usize) {
if (comptime Environment.isWindows) {
return sys_uv.pwritev(fd, buffers, position);
}
if (comptime Environment.isMac) {
const rc = pwritev_sym(fd.cast(), buffers.ptr, @as(i32, @intCast(buffers.len)), position);
if (comptime Environment.allow_assert)
Expand Down
2 changes: 1 addition & 1 deletion src/sys_uv.zig
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ pub fn preadv(fd: FileDescriptor, bufs: []const bun.PlatformIOVec, position: i64
}
}

pub fn pwritev(fd: FileDescriptor, bufs: []const bun.PlatformIOVec, position: i64) Maybe(usize) {
pub fn pwritev(fd: FileDescriptor, bufs: []const bun.PlatformIOVecConst, position: i64) Maybe(usize) {
const uv_fd = bun.uvfdcast(fd);
comptime std.debug.assert(bun.PlatformIOVec == uv.uv_buf_t);

Expand Down
4 changes: 2 additions & 2 deletions src/tmp.zig
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub const Tmpfile = struct {
if (comptime allow_tmpfile) {
switch (bun.sys.openat(destination_dir, ".", O.WRONLY | O.TMPFILE | O.CLOEXEC, perm)) {
.result => |fd| {
tmpfile.fd = fd;
tmpfile.fd = bun.toLibUVOwnedFD(fd);
break :open;
},
.err => |err| {
Expand All @@ -43,7 +43,7 @@ pub const Tmpfile = struct {
}

tmpfile.fd = switch (bun.sys.openat(destination_dir, tmpfilename, O.CREAT | O.CLOEXEC | O.WRONLY, perm)) {
.result => |fd| fd,
.result => |fd| bun.toLibUVOwnedFD(fd),
.err => |err| return .{ .err = err },
};
break :open;
Expand Down
6 changes: 6 additions & 0 deletions src/windows_c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,12 @@ pub fn renameAtW(
new_path_w: []const u16,
replace_if_exists: bool,
) Maybe(void) {
if (comptime bun.Environment.allow_assert) {
// if the directories are the same and the destination path is absolute, the old path name is kept
if (old_dir_fd == new_dir_fd) {
std.debug.assert(!std.fs.path.isAbsoluteWindowsWTF16(new_path_w));
}
}
const src_fd = switch (bun.sys.ntCreateFile(
old_dir_fd,
old_path_w,
Expand Down
3 changes: 2 additions & 1 deletion test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import assert from "assert";
import dedent from "dedent";
import { sep } from "path";
import { itBundled, testForFile } from "./expectBundled";
var { describe, test, expect } = testForFile(import.meta.path);

Expand Down Expand Up @@ -36,7 +37,7 @@ describe("bundler", () => {
},
target: "bun",
run: {
stdout: "a/b",
stdout: `a${sep}b`,
},
});
itBundled("edgecase/ImportStarFunction", {
Expand Down
7 changes: 3 additions & 4 deletions test/cli/run/run-process-env.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @known-failing-on-windows: 1 failing
import { describe, expect, test } from "bun:test";
import { bunExe, bunRunAsScript, tempDirWithFiles } from "harness";

Expand All @@ -7,7 +6,7 @@ describe("process.env", () => {
const scriptName = "start:dev";

const dir = tempDirWithFiles("processenv", {
"package.json": `{'scripts': {'${scriptName}': '${bunExe()} run index.ts'}}`,
"package.json": JSON.stringify({ "scripts": { [`${scriptName}`]: `${bunExe()} run index.ts` } }),
"index.ts": "console.log(process.env.npm_lifecycle_event);",
});

Expand All @@ -18,9 +17,9 @@ describe("process.env", () => {
// https://github.com/oven-sh/bun/issues/3589
test("npm_lifecycle_event should have the value of the last call", () => {
const dir = tempDirWithFiles("processenv_ls_call", {
"package.json": `{"scripts": { "first": "${bunExe()} run --cwd lsc second" } }`,
"package.json": JSON.stringify({ scripts: { first: `${bunExe()} run --cwd lsc second` } }),
"lsc": {
"package.json": `{"scripts": { "second": "${bunExe()} run index.ts" } }`,
"package.json": JSON.stringify({ scripts: { second: `${bunExe()} run index.ts` } }),
"index.ts": "console.log(process.env.npm_lifecycle_event);",
},
});
Expand Down
1 change: 0 additions & 1 deletion test/cli/run/transpiler-cache.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @known-failing-on-windows: 1 failing
import assert from "assert";
import { Subprocess } from "bun";
import { beforeEach, describe, expect, test } from "bun:test";
Expand Down
Loading

0 comments on commit c538bf8

Please sign in to comment.