From eedf008c809b026f876aa0ed060d981b84b48487 Mon Sep 17 00:00:00 2001 From: Ashcon Partovi Date: Fri, 2 Feb 2024 14:25:53 -0800 Subject: [PATCH 1/4] Fix formatting not always working --- .vscode/settings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index c0001c02d633b2..96f8d506a0f861 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,7 +3,7 @@ "editor.tabSize": 2, "editor.insertSpaces": true, "editor.formatOnSave": true, - "editor.formatOnSaveMode": "modificationsIfAvailable", + "editor.formatOnSaveMode": "file", // Search "search.quickOpen.includeSymbols": false, From 5934b17f0013410d3b7111ca3f98d7a79da9698b Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Fri, 2 Feb 2024 18:58:16 -0800 Subject: [PATCH 2/4] fix(windows): fix a few more tests (#8644) * fix regression tests * fix fs.test.ts bigintstats * enable transpiler cache lol * remove failing * fix filesystem router * update * fix run-unicode test * update comment * add updated snapshot * fix remaining node-module-module tests * fixup * [autofix.ci] apply automated fixes * fix tty tests --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/api/BunObject.zig | 22 ++++++- src/bun.js/api/filesystem_router.zig | 3 + src/bun.js/bindings/CommonJSModuleRecord.cpp | 3 + src/bun.js/bindings/PathInlines.h | 4 ++ src/bun.js/bindings/bindings.zig | 4 +- src/c.zig | 4 +- src/feature_flags.zig | 2 +- src/fs.zig | 9 +++ src/resolver/resolve_path.zig | 43 ++++++++++--- src/resolver/resolver.zig | 22 +++++-- src/router.zig | 62 ++++++++++++++++--- src/windows.zig | 4 ++ test/cli/install/migration/migrate.test.ts | 2 + test/cli/run/run-unicode.test.ts | 3 +- test/cli/run/transpiler-cache.test.ts | 11 ++-- .../__snapshots__/inspect-error.test.js.snap | 38 ++++++------ test/js/bun/util/filesystem_router.test.ts | 5 +- .../js/node/module/node-module-module.test.js | 24 ++++--- test/js/node/tty.test.ts | 5 +- test/regression/issue/07261.test.ts | 3 +- test/regression/issue/07500.test.ts | 6 +- 21 files changed, 197 insertions(+), 82 deletions(-) diff --git a/src/bun.js/api/BunObject.zig b/src/bun.js/api/BunObject.zig index 1781799e79efef..77453c33da7034 100644 --- a/src/bun.js/api/BunObject.zig +++ b/src/bun.js/api/BunObject.zig @@ -1042,14 +1042,30 @@ pub fn openInEditor( } pub fn getPublicPath(to: string, origin: URL, comptime Writer: type, writer: Writer) void { - return getPublicPathWithAssetPrefix(to, VirtualMachine.get().bundler.fs.top_level_dir, origin, VirtualMachine.get().bundler.options.routes.asset_prefix_path, comptime Writer, writer); + return getPublicPathWithAssetPrefix( + to, + VirtualMachine.get().bundler.fs.top_level_dir, + origin, + VirtualMachine.get().bundler.options.routes.asset_prefix_path, + comptime Writer, + writer, + .loose, + ); } -pub fn getPublicPathWithAssetPrefix(to: string, dir: string, origin: URL, asset_prefix: string, comptime Writer: type, writer: Writer) void { +pub fn getPublicPathWithAssetPrefix( + to: string, + dir: string, + origin: URL, + asset_prefix: string, + comptime Writer: type, + writer: Writer, + comptime platform: bun.path.Platform, +) void { const relative_path = if (strings.hasPrefix(to, dir)) strings.withoutTrailingSlash(to[dir.len..]) else - VirtualMachine.get().bundler.fs.relative(dir, to); + VirtualMachine.get().bundler.fs.relativePlatform(dir, to, platform); if (origin.isAbsolute()) { if (strings.hasPrefix(relative_path, "..") or strings.hasPrefix(relative_path, "./")) { writer.writeAll(origin.origin) catch return; diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index 9995a7fbdb1ecd..d2827ffb6e4811 100644 --- a/src/bun.js/api/filesystem_router.zig +++ b/src/bun.js/api/filesystem_router.zig @@ -249,6 +249,7 @@ pub const FileSystemRouter = struct { }; this.arena.deinit(); + this.router.deinit(); globalThis.allocator().destroy(this.arena); this.arena = arena; @@ -380,6 +381,7 @@ pub const FileSystemRouter = struct { dir.deref(); } + this.router.deinit(); this.arena.deinit(); } }; @@ -586,6 +588,7 @@ pub const MatchedRoute = struct { if (this.asset_prefix) |prefix| prefix.slice() else "", @TypeOf(&writer), &writer, + .posix, ); return ZigString.init(buf[0..writer.context.pos]) .withEncoding() diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index 48c03939b6dc88..57f4201dfe2e2b 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -471,6 +471,9 @@ JSC_DEFINE_HOST_FUNCTION(functionCommonJSModuleRecord_compile, (JSGlobalObject * moduleObject->sourceCode.set(vm, moduleObject, jsSourceCode); auto index = filenameString.reverseFind(PLATFORM_SEP, filenameString.length()); + // filenameString is coming from js, any separator could be used + if (index == WTF::notFound) + index = filenameString.reverseFind(NOT_PLATFORM_SEP, filenameString.length()); String dirnameString; if (index != WTF::notFound) { dirnameString = filenameString.substring(0, index); diff --git a/src/bun.js/bindings/PathInlines.h b/src/bun.js/bindings/PathInlines.h index eb8495563dda23..26c0314f9df435 100644 --- a/src/bun.js/bindings/PathInlines.h +++ b/src/bun.js/bindings/PathInlines.h @@ -4,9 +4,13 @@ #if OS(WINDOWS) #define PLATFORM_SEP_s "\\"_s #define PLATFORM_SEP '\\' +#define NOT_PLATFORM_SEP_s "/"_s +#define NOT_PLATFORM_SEP '/' #else #define PLATFORM_SEP_s "/"_s #define PLATFORM_SEP '/' +#define NOT_PLATFORM_SEP_s "\\"_s +#define NOT_PLATFORM_SEP '\\' #endif ALWAYS_INLINE bool isAbsolutePath(WTF::String input) diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 22ce9217993d8c..0b0647dfd16933 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -4031,7 +4031,7 @@ pub const JSValue = enum(JSValueReprInt) { return jsNumberFromInt32(@as(i32, @intCast(i))); } - return jsNumberFromDouble(@as(f64, @floatFromInt(@as(i52, @truncate(i))))); + return jsNumberFromDouble(@floatFromInt(i)); } pub inline fn toJS(this: JSValue, _: *const JSGlobalObject) JSValue { @@ -4047,7 +4047,7 @@ pub const JSValue = enum(JSValueReprInt) { } pub fn jsNumberFromPtrSize(i: usize) JSValue { - return jsNumberFromDouble(@as(f64, @floatFromInt(@as(i52, @intCast(@as(u51, @truncate(i))))))); + return jsNumberFromDouble(@floatFromInt(i)); } pub fn coerceDoubleTruncatingIntoInt64(this: JSValue) i64 { diff --git a/src/c.zig b/src/c.zig index 570915033a19a0..c4808c6d0e017b 100644 --- a/src/c.zig +++ b/src/c.zig @@ -375,7 +375,9 @@ pub fn getRelease(buf: []u8) []const u8 { if (err != 0) { return "unknown"; } - return bun.sliceTo(&info.version, 0); + const release = bun.sliceTo(&info.release, 0); + @memcpy(buf[0..release.len], release); + return buf[0..release.len]; } } diff --git a/src/feature_flags.zig b/src/feature_flags.zig index c458909b5294bb..2a0d41ba9785e2 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -161,4 +161,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; -pub const runtime_transpiler_cache = !env.isWindows; +pub const runtime_transpiler_cache = true; diff --git a/src/fs.zig b/src/fs.zig index f0ecdb18eb2f6d..0cb56c40046894 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -457,6 +457,15 @@ pub const FileSystem = struct { }); } + pub fn relativePlatform(_: *@This(), from: string, to: string, comptime platform: path_handler.Platform) string { + return @call(.always_inline, path_handler.relativePlatform, .{ + from, + to, + platform, + false, + }); + } + pub fn relativeTo(f: *@This(), to: string) string { return @call(.always_inline, path_handler.relative, .{ f.top_level_dir, diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig index 4a7df06662c825..2f52c1195f2335 100644 --- a/src/resolver/resolve_path.zig +++ b/src/resolver/resolve_path.zig @@ -531,7 +531,7 @@ fn windowsVolumeNameLenT(comptime T: type, path: []const T) struct { usize, usiz // with drive letter const c = path[0]; if (path[1] == ':') { - if ('a' <= c and c <= 'z' or 'A' <= c and c <= 'Z') { + if (isDriveLetterT(T, c)) { return .{ 2, 0 }; } } @@ -578,15 +578,27 @@ pub fn windowsFilesystemRoot(path: []const u8) []const u8 { return windowsFilesystemRootT(u8, path); } +pub fn isDriveLetter(c: u8) bool { + return isDriveLetterT(u8, c); +} + +pub fn isDriveLetterT(comptime T: type, c: T) bool { + return 'a' <= c and c <= 'z' or 'A' <= c and c <= 'Z'; +} + // path.relative lets you do relative across different share drives pub fn windowsFilesystemRootT(comptime T: type, path: []const T) []const T { - if (path.len < 3) + // minimum: `C:` + if (path.len < 2) return if (isSepAnyT(T, path[0])) path[0..1] else path[0..0]; // with drive letter const c = path[0]; - if (path[1] == ':' and isSepAnyT(T, path[2])) { - if ('a' <= c and c <= 'z' or 'A' <= c and c <= 'Z') { - return path[0..3]; + if (path[1] == ':') { + if (isDriveLetterT(T, c)) { + if (path.len > 2 and isSepAnyT(T, path[2])) + return path[0..3] + else + return path[0..2]; } } // UNC @@ -759,7 +771,7 @@ pub fn normalizeStringGenericT( if (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) { + if (buf_i > 0 and path_[path_.len - 1] == separator and buf[buf_i - 1] != separator) { buf[buf_i] = separator; buf_i += 1; } @@ -1004,7 +1016,7 @@ pub fn normalizeStringBuf( buf: []u8, comptime allow_above_root: bool, comptime _platform: Platform, - comptime preserve_trailing_slash: anytype, + comptime preserve_trailing_slash: bool, ) []u8 { return normalizeStringBufT(u8, str, buf, allow_above_root, _platform, preserve_trailing_slash); } @@ -1015,7 +1027,7 @@ pub fn normalizeStringBufT( buf: []T, comptime allow_above_root: bool, comptime _platform: Platform, - comptime preserve_trailing_slash: anytype, + comptime preserve_trailing_slash: bool, ) []T { const platform = comptime _platform.resolve(); @@ -2114,15 +2126,26 @@ export fn ResolvePath__joinAbsStringBufCurrentPlatformBunString( pub fn platformToPosixInPlace(comptime T: type, path_buffer: []T) void { if (std.fs.path.sep == '/') return; var idx: usize = 0; - while (std.mem.indexOfScalarPos(T, path_buffer, idx, std.fs.path.sep)) |index| : (idx = index) { + while (std.mem.indexOfScalarPos(T, path_buffer, idx, std.fs.path.sep)) |index| : (idx = index + 1) { path_buffer[index] = '/'; } } +pub fn platformToPosixBuf(comptime T: type, path: []const T, buf: []T) []T { + if (std.fs.path.sep == '/') return; + var idx: usize = 0; + while (std.mem.indexOfScalarPos(T, path, idx, std.fs.path.sep)) |index| : (idx = index + 1) { + @memcpy(buf[idx..index], path[idx..index]); + buf[index] = '/'; + } + @memcpy(buf[idx..path.len], path[idx..path.len]); + return buf[0..path.len]; +} + pub fn posixToPlatformInPlace(comptime T: type, path_buffer: []T) void { if (std.fs.path.sep == '/') return; var idx: usize = 0; - while (std.mem.indexOfScalarPos(T, path_buffer, idx, '/')) |index| : (idx = index) { + while (std.mem.indexOfScalarPos(T, path_buffer, idx, '/')) |index| : (idx = index + 1) { path_buffer[index] = std.fs.path.sep; } } diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index 448c9956335030..33d01186599e7d 100644 --- a/src/resolver/resolver.zig +++ b/src/resolver/resolver.zig @@ -3277,7 +3277,17 @@ pub const Resolver = struct { defer sliced.deinit(); const str = brk: { - if (std.fs.path.isAbsolute(sliced.slice())) break :brk sliced.slice(); + if (std.fs.path.isAbsolute(sliced.slice())) { + if (comptime Environment.isWindows) { + const dir_path_buf = bufs(.node_modules_paths_buf); + var normalizer = bun.path.PosixToWinNormalizer{}; + const normalized = normalizer.resolveCWD(sliced.slice()) catch { + @panic("Failed to get cwd for _nodeModulesPaths"); + }; + break :brk bun.path.normalizeBuf(normalized, dir_path_buf, .windows); + } + break :brk sliced.slice(); + } const dir_path_buf = bufs(.node_modules_paths_buf); break :brk bun.path.joinStringBuf(dir_path_buf, &[_]string{ r.fs.top_level_dir, sliced.slice() }, .auto); }; @@ -3292,10 +3302,10 @@ pub const Resolver = struct { const path_without_trailing_slash = strings.withoutTrailingSlash(dir_info.abs_path); const path_parts = brk: { if (path_without_trailing_slash.len == 1 and path_without_trailing_slash[0] == '/') { - break :brk [2]string{ "", "/node_modules" }; + break :brk [2]string{ "", std.fs.path.sep_str ++ "node_modules" }; } - break :brk [2]string{ path_without_trailing_slash, "/node_modules" }; + break :brk [2]string{ path_without_trailing_slash, std.fs.path.sep_str ++ "node_modules" }; }; const nodemodules_path = bun.strings.concat(stack_fallback_allocator.get(), &path_parts) catch unreachable; bun.path.posixToPlatformInPlace(u8, nodemodules_path); @@ -3305,7 +3315,7 @@ pub const Resolver = struct { } else { // does not exist const full_path = std.fs.path.resolve(r.allocator, &[1][]const u8{str}) catch unreachable; - var path = full_path; + var path = strings.withoutTrailingSlash(full_path); while (true) { const path_without_trailing_slash = strings.withoutTrailingSlash(path); @@ -3315,13 +3325,13 @@ pub const Resolver = struct { stack_fallback_allocator.get(), &[_]string{ path_without_trailing_slash, - "/node_modules", + std.fs.path.sep_str ++ "node_modules", }, ) catch unreachable, ), ) catch unreachable; - path = path[0 .. strings.lastIndexOfChar(path, '/') orelse break]; + path = path[0 .. strings.lastIndexOfChar(path, std.fs.path.sep) orelse break]; } } diff --git a/src/router.zig b/src/router.zig index 0dfde3afe892d7..de6494591a61f1 100644 --- a/src/router.zig +++ b/src/router.zig @@ -62,6 +62,14 @@ pub fn init( }; } +pub fn deinit(this: *Router) void { + if (comptime Environment.isWindows) { + for (this.routes.list.items(.filepath)) |abs_path| { + this.allocator.free(abs_path); + } + } +} + pub fn getEntryPoints(this: *const Router) ![]const string { return this.routes.list.items(.filepath); } @@ -164,7 +172,7 @@ pub const Routes = struct { .name = index.name, .path = index.abs_path.slice(), .pathname = url_path.pathname, - .basename = index.entry.base(), + .basename = index.basename, .hash = index_route_hash, .file_path = index.abs_path.slice(), .query_string = url_path.query_string, @@ -187,7 +195,7 @@ pub const Routes = struct { .name = route.name, .path = route.abs_path.slice(), .pathname = url_path.pathname, - .basename = route.entry.base(), + .basename = route.basename, .hash = route.full_hash, .file_path = route.abs_path.slice(), .query_string = url_path.query_string, @@ -459,7 +467,7 @@ const RouteLoader = struct { // length is extended by one // entry.dir is a string with a trailing slash if (comptime Environment.isDebug) { - std.debug.assert(entry.dir.ptr[base_dir.len - 1] == '/'); + std.debug.assert(bun.path.isSepAny(entry.dir[base_dir.len - 1])); } const public_dir = entry.dir.ptr[base_dir.len - 1 .. entry.dir.len]; @@ -543,10 +551,23 @@ pub const Route = struct { /// This is [inconsistent with Next.js](https://github.com/vercel/next.js/issues/21498) match_name: PathString, - entry: *Fs.FileSystem.Entry, + basename: string, full_hash: u32, param_count: u16, - abs_path: PathString, + + // On windows we need to normalize this path to have forward slashes. + // To avoid modifying memory we do not own, allocate another buffer + abs_path: if (Environment.isWindows) struct { + path: string, + + pub fn slice(this: @This()) string { + return this.path; + } + + pub fn isEmpty(this: @This()) bool { + return this.path.len == 0; + } + } else PathString, /// URL-safe path for the route's transpiled script relative to project's top level directory /// - It might not share a prefix with the absolute path due to symlinks. @@ -560,8 +581,9 @@ pub const Route = struct { pub const Ptr = TinyPtr; pub const index_route_name: string = "/"; - var route_file_buf: [bun.MAX_PATH_BYTES]u8 = undefined; - var second_route_file_buf: [bun.MAX_PATH_BYTES]u8 = undefined; + threadlocal var route_file_buf: bun.PathBuffer = undefined; + threadlocal var second_route_file_buf: bun.PathBuffer = undefined; + threadlocal var normalized_abs_path_buf: bun.windows.PathBuffer = undefined; pub const Sorter = struct { const sort_table: [std.math.maxInt(u8)]u8 = brk: { @@ -647,7 +669,7 @@ pub const Route = struct { const base = base_[0 .. base_.len - extname.len]; - const public_dir = std.mem.trim(u8, public_dir_, "/"); + const public_dir = std.mem.trim(u8, public_dir_, std.fs.path.sep_str); // this is a path like // "/pages/index.js" @@ -669,6 +691,11 @@ pub const Route = struct { buf = buf[base.len..]; bun.copy(u8, buf, extname); buf = buf[extname.len..]; + + if (comptime Environment.isWindows) { + bun.path.platformToPosixInPlace(u8, route_file_buf[0 .. @intFromPtr(buf.ptr) - @intFromPtr(&route_file_buf)]); + } + break :brk route_file_buf[0 .. @intFromPtr(buf.ptr) - @intFromPtr(&route_file_buf)]; }; @@ -755,9 +782,22 @@ pub const Route = struct { entry.abs_path = PathString.init(abs_path_str); } + const abs_path = if (comptime Environment.isWindows) + allocator.dupe(u8, bun.path.platformToPosixBuf(u8, abs_path_str, &normalized_abs_path_buf)) catch bun.outOfMemory() + else + PathString.init(abs_path_str); + + if (comptime Environment.allow_assert and Environment.isWindows) { + std.debug.assert(!strings.containsChar(name, '\\')); + std.debug.assert(!strings.containsChar(public_path, '\\')); + std.debug.assert(!strings.containsChar(match_name, '\\')); + std.debug.assert(!strings.containsChar(abs_path, '\\')); + std.debug.assert(!strings.containsChar(entry.base(), '\\')); + } + return Route{ .name = name, - .entry = entry, + .basename = entry.base(), .public_path = PathString.init(public_path), .match_name = PathString.init(match_name), .full_hash = if (is_index) @@ -766,7 +806,9 @@ pub const Route = struct { @as(u32, @truncate(bun.hash(name))), .param_count = validation_result.param_count, .kind = validation_result.kind, - .abs_path = entry.abs_path, + .abs_path = if (comptime Environment.isWindows) .{ + .path = abs_path, + } else abs_path, .has_uppercase = has_uppercase, }; } diff --git a/src/windows.zig b/src/windows.zig index be35bc0eb658c8..ccfb7112fd0fb3 100644 --- a/src/windows.zig +++ b/src/windows.zig @@ -67,6 +67,10 @@ pub const nt_object_prefix = [4]u16{ '\\', '?', '?', '\\' }; pub const nt_maxpath_prefix = [4]u16{ '\\', '\\', '?', '\\' }; const std = @import("std"); +const Environment = bun.Environment; +pub const PathBuffer = if (Environment.isWindows) bun.PathBuffer else void; +pub const WPathBuffer = if (Environment.isWindows) bun.WPathBuffer else void; + pub const HANDLE = win32.HANDLE; /// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfilevaliddata diff --git a/test/cli/install/migration/migrate.test.ts b/test/cli/install/migration/migrate.test.ts index 5af930d5dcf3d2..9c8b759f18c647 100644 --- a/test/cli/install/migration/migrate.test.ts +++ b/test/cli/install/migration/migrate.test.ts @@ -8,6 +8,8 @@ import { tmpdir } from "os"; const ROOT_TEMP_DIR = join(tmpdir(), "migrate", sep); beforeAll(() => { + // if the test was stopped early + fs.rmSync(ROOT_TEMP_DIR, { recursive: true, force: true }); fs.mkdirSync(ROOT_TEMP_DIR); }); diff --git a/test/cli/run/run-unicode.test.ts b/test/cli/run/run-unicode.test.ts index d86246beb587fc..660815850a2cf9 100644 --- a/test/cli/run/run-unicode.test.ts +++ b/test/cli/run/run-unicode.test.ts @@ -1,4 +1,3 @@ -// @known-failing-on-windows: 1 failing import { expect, test } from "bun:test"; import { mkdirSync, realpathSync } from "fs"; import { bunEnv, bunExe } from "harness"; @@ -6,7 +5,7 @@ import { tmpdir } from "os"; import { join } from "path"; test("running a weird filename works", async () => { - const troll = "💥'\"​\n"; + const troll = process.platform == "win32" ? "💥'​\\" : "💥'\"​\n"; const dir = join(realpathSync(tmpdir()), "bun-run-test" + troll); mkdirSync(dir, { recursive: true }); console.log("dir", dir); diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index 2ce052cdfb4f0a..3c4b8f417fe14d 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -1,4 +1,3 @@ -import assert from "assert"; import { Subprocess } from "bun"; import { beforeEach, describe, expect, test } from "bun:test"; import { realpathSync, chmodSync, existsSync, mkdirSync, readdirSync, rmSync, writeFileSync } from "fs"; @@ -63,7 +62,7 @@ describe("transpiler cache", () => { writeFileSync(join(temp_dir, "a.js"), dummyFile((50 * 1024 * 1.5) | 0, "1", "a")); const a = bunRun(join(temp_dir, "a.js"), env); expect(a.stdout == "a"); - assert(existsSync(cache_dir)); + expect(existsSync(cache_dir)).toBeTrue(); expect(newCacheCount()).toBe(1); const b = bunRun(join(temp_dir, "a.js"), env); expect(b.stdout == "a"); @@ -73,7 +72,7 @@ describe("transpiler cache", () => { writeFileSync(join(temp_dir, "a.js"), "//" + "a".repeat(50 * 1024 * 1.5)); const a = bunRun(join(temp_dir, "a.js"), env); expect(a.stdout == ""); - assert(existsSync(cache_dir)); + expect(existsSync(cache_dir)).toBeTrue(); expect(newCacheCount()).toBe(1); const b = bunRun(join(temp_dir, "a.js"), env); expect(b.stdout == ""); @@ -83,7 +82,7 @@ describe("transpiler cache", () => { writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024 - 1, "1", "a")); const a = bunRun(join(temp_dir, "a.js"), env); expect(a.stdout == "a"); - assert(!existsSync(cache_dir)); + expect(!existsSync(cache_dir)).toBeTrue(); }); test("it is indeed content addressable", async () => { writeFileSync(join(temp_dir, "a.js"), dummyFile(50 * 1024, "1", "b")); @@ -132,7 +131,7 @@ describe("transpiler cache", () => { await Promise.all(processes.map(x => x.exited)); - assert(!killing); + expect(!killing).toBeTrue(); remover.kill(9); @@ -177,7 +176,7 @@ describe("transpiler cache", () => { ); const a = bunRun(join(temp_dir, "a.js"), { ...env, NODE_ENV: undefined, HELLO: "1" }); expect(a.stdout == "development 1"); - assert(existsSync(cache_dir)); + expect(existsSync(cache_dir)).toBeTrue(); expect(newCacheCount()).toBe(1); const b = bunRun(join(temp_dir, "a.js"), { ...env, NODE_ENV: "production", HELLO: "5" }); expect(b.stdout == "production 5"); diff --git a/test/js/bun/util/__snapshots__/inspect-error.test.js.snap b/test/js/bun/util/__snapshots__/inspect-error.test.js.snap index 5bf59a64d0c86b..4ab0d893757a55 100644 --- a/test/js/bun/util/__snapshots__/inspect-error.test.js.snap +++ b/test/js/bun/util/__snapshots__/inspect-error.test.js.snap @@ -1,37 +1,35 @@ // Bun Snapshot v1, https://goo.gl/fbAQLP exports[`error.cause 1`] = ` -"1 | // @known-failing-on-windows: 1 failing -2 | import { test, expect } from "bun:test"; -3 | -4 | test("error.cause", () => { -5 | const err = new Error("error 1"); -6 | const err2 = new Error("error 2", { cause: err }); +"1 | import { test, expect } from "bun:test"; +2 | +3 | test("error.cause", () => { +4 | const err = new Error("error 1"); +5 | const err2 = new Error("error 2", { cause: err }); ^ error: error 2 - at [dir]/inspect-error.test.js:6:16 + at [dir]/inspect-error.test.js:5:16 -1 | // @known-failing-on-windows: 1 failing -2 | import { test, expect } from "bun:test"; -3 | -4 | test("error.cause", () => { -5 | const err = new Error("error 1"); +1 | import { test, expect } from "bun:test"; +2 | +3 | test("error.cause", () => { +4 | const err = new Error("error 1"); ^ error: error 1 - at [dir]/inspect-error.test.js:5:15 + at [dir]/inspect-error.test.js:4:15 " `; exports[`Error 1`] = ` -"10 | .replaceAll("//", "/"), -11 | ).toMatchSnapshot(); -12 | }); -13 | -14 | test("Error", () => { -15 | const err = new Error("my message"); +" 9 | .replaceAll("//", "/"), +10 | ).toMatchSnapshot(); +11 | }); +12 | +13 | test("Error", () => { +14 | const err = new Error("my message"); ^ error: my message - at [dir]/inspect-error.test.js:15:15 + at [dir]/inspect-error.test.js:14:15 " `; diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts index 2942fab439277f..a9aa29bf0f82dc 100644 --- a/test/js/bun/util/filesystem_router.test.ts +++ b/test/js/bun/util/filesystem_router.test.ts @@ -1,4 +1,3 @@ -// @known-failing-on-windows: 1 failing import { FileSystemRouter } from "bun"; import { it, expect } from "bun:test"; import path, { dirname, resolve } from "path"; @@ -18,7 +17,7 @@ function createTree(basedir: string, paths: string[]) { } var count = 0; function make(files: string[]) { - const dir = tempdir + `fs-router-test-${count++}`; + const dir = (tempdir + `fs-router-test-${count++}`).replace(/\\/g, "/"); rmSync(dir, { recursive: true, force: true, @@ -196,9 +195,7 @@ it("should support catch-all routes", () => { }); for (let fixture of ["/posts/123", "/posts/hey", "/posts/zorp", "/posts", "/index", "/posts/"]) { - console.log(`matching ${fixture}`); const match = router.match(fixture); - console.log(match); expect(match?.name).not.toBe("/posts/[...id]"); } diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index c5453e4ede099d..52c201d5c52779 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -1,6 +1,5 @@ -// @known-failing-on-windows: 1 failing import { expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, ospath } from "harness"; import { _nodeModulePaths, builtinModules, isBuiltin, wrap } from "module"; import Module from "module"; import path from "path"; @@ -37,21 +36,26 @@ test("module.Module works", () => { }); test("_nodeModulePaths() works", () => { + const root = process.platform === "win32" ? "C:\\" : "/"; expect(() => { _nodeModulePaths(); }).toThrow(); expect(_nodeModulePaths(".").length).toBeGreaterThan(0); - expect(_nodeModulePaths(".").pop()).toBe("/node_modules"); + expect(_nodeModulePaths(".").pop()).toBe(root + "node_modules"); expect(_nodeModulePaths("")).toEqual(_nodeModulePaths(".")); - expect(_nodeModulePaths("/")).toEqual(["/node_modules"]); + expect(_nodeModulePaths("/")).toEqual([root + "node_modules"]); expect(_nodeModulePaths("/a/b/c/d")).toEqual([ - "/a/b/c/d/node_modules", - "/a/b/c/node_modules", - "/a/b/node_modules", - "/a/node_modules", - "/node_modules", + ospath(root + "a/b/c/d/node_modules"), + ospath(root + "a/b/c/node_modules"), + ospath(root + "a/b/node_modules"), + ospath(root + "a/node_modules"), + ospath(root + "node_modules"), + ]); + expect(_nodeModulePaths("/a/b/../d")).toEqual([ + ospath(root + "a/d/node_modules"), + ospath(root + "a/node_modules"), + ospath(root + "node_modules"), ]); - expect(_nodeModulePaths("/a/b/../d")).toEqual(["/a/d/node_modules", "/a/node_modules", "/node_modules"]); }); test("Module.wrap", () => { diff --git a/test/js/node/tty.test.ts b/test/js/node/tty.test.ts index 75057ba123794e..41ccb8f5de69eb 100644 --- a/test/js/node/tty.test.ts +++ b/test/js/node/tty.test.ts @@ -1,4 +1,3 @@ -// @known-failing-on-windows: 1 failing import { describe, it, expect } from "bun:test"; import { WriteStream } from "node:tty"; @@ -8,7 +7,7 @@ describe("WriteStream.prototype.getColorDepth", () => { WriteStream.prototype.getColorDepth.call(undefined, { TERM_PROGRAM: "iTerm.app", }), - ).toBe(8); + ).toBe(process.platform === "win32" ? 24 : 8); }); it("iTerm modern", () => { @@ -21,6 +20,6 @@ describe("WriteStream.prototype.getColorDepth", () => { }); it("empty", () => { - expect(WriteStream.prototype.getColorDepth.call(undefined, {})).toBe(1); + expect(WriteStream.prototype.getColorDepth.call(undefined, {})).toBe(process.platform === "win32" ? 24 : 1); }); }); diff --git a/test/regression/issue/07261.test.ts b/test/regression/issue/07261.test.ts index 4a32145b8d2eed..0d522377aa4f4b 100644 --- a/test/regression/issue/07261.test.ts +++ b/test/regression/issue/07261.test.ts @@ -1,4 +1,3 @@ -// @known-failing-on-windows: 1 failing import { bunEnv, bunExe } from "harness"; import { mkdirSync, rmSync, writeFileSync, mkdtempSync } from "fs"; import { tmpdir } from "os"; @@ -15,7 +14,7 @@ it("imports tsconfig.json with abritary keys", async () => { writeFileSync(join(testDir, "tsconfig.json"), '{ "key-with-hyphen": true }'); const { exitCode } = Bun.spawnSync({ - cmd: [bunExe(), "-e", `require('${join(testDir, "tsconfig.json")}')`], + cmd: [bunExe(), "-e", `require('${join(testDir, "tsconfig.json").replace(/\\/g, "\\\\")}').compilerOptions`], env: bunEnv, stderr: "inherit", }); diff --git a/test/regression/issue/07500.test.ts b/test/regression/issue/07500.test.ts index 20ac53f761a0ec..1c88b96b2d028e 100644 --- a/test/regression/issue/07500.test.ts +++ b/test/regression/issue/07500.test.ts @@ -11,10 +11,12 @@ test("7500 - Bun.stdin.text() doesn't read all data", async () => { .split(" ") .join("\n"); await Bun.write(filename, text); + const cat = process.platform === "win32" ? "Get-Content" : "cat"; const bunCommand = `${bunExe()} ${join(import.meta.dir, "7500-repro-fixture.js")}`; - const shellCommand = `cat ${filename} | ${bunCommand}`; + const shellCommand = `${cat} ${filename} | ${bunCommand}`.replace(/\\/g, "\\\\"); - const cmd = process.platform === "win32" ? ["pwsh.exe", `-Command='${shellCommand}'`] : ["bash", "-c", shellCommand]; + const cmd = + process.platform === "win32" ? ["pwsh.exe", `-Command { '${shellCommand}' }`] : ["bash", "-c", shellCommand]; const proc = Bun.spawnSync({ cmd, stdin: "inherit", From d16ac873476f79aef717473faa7a4b418152f110 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Fri, 2 Feb 2024 23:25:07 -0800 Subject: [PATCH 3/4] Export compile_commands.json --- .vscode/c_cpp_properties.json | 1 + CMakeLists.txt | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.vscode/c_cpp_properties.json b/.vscode/c_cpp_properties.json index 5c4265d70baea9..6bed0ed4fe626f 100644 --- a/.vscode/c_cpp_properties.json +++ b/.vscode/c_cpp_properties.json @@ -3,6 +3,7 @@ { "name": "Debug", "forcedInclude": ["${workspaceFolder}/src/bun.js/bindings/root.h"], + "compileCommands": "${workspaceFolder}/build/compile_commands.json", "includePath": [ "${workspaceFolder}/build/bun-webkit/include", "${workspaceFolder}/build/codegen", diff --git a/CMakeLists.txt b/CMakeLists.txt index c23c4b2001c076..5cb68bc47a7cfc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,6 +26,9 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug") set(DEBUG ON) set(DEFAULT_ZIG_OPTIMIZE "Debug") set(bun "bun-debug") + + # COMPILE_COMMANDS + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) elseif(CMAKE_BUILD_TYPE STREQUAL "Release") set(DEBUG OFF) set(DEFAULT_ZIG_OPTIMIZE "ReleaseFast") From e0c0fe235a58e76fd6df2008a6f60cea5d89d604 Mon Sep 17 00:00:00 2001 From: Dylan Conway <35280289+dylan-conway@users.noreply.github.com> Date: Sat, 3 Feb 2024 00:35:25 -0800 Subject: [PATCH 4/4] fix(windows): fix macros (#8653) * fix macro tests * path format options * remove failing comment * fix buffer toString memcpy length --- src/bun.js/bindings/JSBuffer.cpp | 6 +-- src/bundler/entry_points.zig | 16 ++++-- src/cli/create_command.zig | 4 +- src/fmt.zig | 86 +++++++++++++++++++++++++++--- src/install/install.zig | 10 ++-- src/libarchive/libarchive.zig | 6 +-- test/transpiler/macro-test.test.ts | 2 - 7 files changed, 104 insertions(+), 26 deletions(-) diff --git a/src/bun.js/bindings/JSBuffer.cpp b/src/bun.js/bindings/JSBuffer.cpp index 1891fd1faa7aa6..ebffd1a33c0060 100644 --- a/src/bun.js/bindings/JSBuffer.cpp +++ b/src/bun.js/bindings/JSBuffer.cpp @@ -1495,10 +1495,8 @@ static inline JSC::EncodedJSValue jsBufferToString(JSC::VM& vm, JSC::JSGlobalObj return JSC::JSValue::encode(JSC::jsEmptyString(vm)); } else { auto str = String::createUninitialized(u16length, data); - // always zero out the last byte of the string incase the buffer is not a multiple of 2 - data[u16length - 1] = 0; - memcpy(data, reinterpret_cast(castedThis->typedVector() + offset), length); - return JSC::JSValue::encode(JSC::jsString(vm, WTFMove(str))); + memcpy(reinterpret_cast(data), reinterpret_cast(castedThis->typedVector() + offset), u16length * 2); + return JSC::JSValue::encode(JSC::jsString(vm, str)); } break; diff --git a/src/bundler/entry_points.zig b/src/bundler/entry_points.zig index 5b76d439e540ea..da3f548412b1d6 100644 --- a/src/bundler/entry_points.zig +++ b/src/bundler/entry_points.zig @@ -315,12 +315,20 @@ pub const MacroEntryPoint = struct { \\Bun.registerMacro({d}, Macros['{s}']); , .{ - dir_to_use, - import_path.filename, + bun.fmt.fmtPath(u8, dir_to_use, .{ + .escape_backslashes = true, + }), + bun.fmt.fmtPath(u8, import_path.filename, .{ + .escape_backslashes = true, + }), function_name, function_name, - dir_to_use, - import_path.filename, + bun.fmt.fmtPath(u8, dir_to_use, .{ + .escape_backslashes = true, + }), + bun.fmt.fmtPath(u8, import_path.filename, .{ + .escape_backslashes = true, + }), macro_id, function_name, }, diff --git a/src/cli/create_command.zig b/src/cli/create_command.zig index f51ea93f735997..b44cdf8ce588b2 100644 --- a/src/cli/create_command.zig +++ b/src/cli/create_command.zig @@ -493,7 +493,7 @@ pub const CreateCommand = struct { progress_.refresh(); - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path) }); + Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); Global.exit(1); }; }; @@ -513,7 +513,7 @@ pub const CreateCommand = struct { } CopyFile.copyFile(infile.handle, outfile.handle) catch |err| { - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path) }); + Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); Global.exit(1); }; } diff --git a/src/fmt.zig b/src/fmt.zig index 65b2b9493700da..06aa0302dfa3e1 100644 --- a/src/fmt.zig +++ b/src/fmt.zig @@ -45,33 +45,107 @@ pub fn formatUTF16Type(comptime Slice: type, slice_: Slice, writer: anytype) !vo } } +pub fn formatUTF16TypeEscapeBackslashes(comptime Slice: type, slice_: Slice, writer: anytype) !void { + var chunk = getSharedBuffer(); + + // Defensively ensure recursion doesn't cause the buffer to be overwritten in-place + shared_temp_buffer_ptr = null; + defer { + if (shared_temp_buffer_ptr) |existing| { + if (existing != chunk.ptr) { + bun.default_allocator.destroy(@as(*SharedTempBuffer, @ptrCast(chunk.ptr))); + } + } else { + shared_temp_buffer_ptr = @ptrCast(chunk.ptr); + } + } + + var slice = slice_; + + while (slice.len > 0) { + const result = strings.copyUTF16IntoUTF8(chunk, Slice, slice, true); + if (result.read == 0 or result.written == 0) + break; + + const to_write = chunk[0..result.written]; + var ptr = to_write; + while (strings.indexOfChar(ptr, '\\')) |i| { + try writer.writeAll(ptr[0 .. i + 1]); + try writer.writeAll("\\"); + ptr = ptr[i + 1 ..]; + } + try writer.writeAll(ptr); + slice = slice[result.read..]; + } +} + pub fn formatUTF16(slice_: []align(1) const u16, writer: anytype) !void { return formatUTF16Type([]align(1) const u16, slice_, writer); } pub const FormatUTF16 = struct { buf: []const u16, - pub fn format(self: @This(), comptime _: []const u8, opts: anytype, writer: anytype) !void { - _ = opts; - try formatUTF16Type([]const u16, self.buf, writer); + escape_backslashes: bool = false, + pub fn format(self: @This(), comptime _: []const u8, _: anytype, writer: anytype) !void { + if (self.escape_backslashes) { + try formatUTF16TypeEscapeBackslashes([]const u16, self.buf, writer); + } else { + try formatUTF16Type([]const u16, self.buf, writer); + } } }; pub const FormatUTF8 = struct { buf: []const u8, + escape_backslashes: bool = false, pub fn format(self: @This(), comptime _: []const u8, _: anytype, writer: anytype) !void { - try writer.writeAll(self.buf); + if (self.escape_backslashes) { + var ptr = self.buf; + while (strings.indexOfChar(ptr, '\\')) |i| { + try writer.writeAll(ptr[0 .. i + 1]); + try writer.writeAll("\\"); + ptr = ptr[i + 1 ..]; + } + try writer.writeAll(ptr); + } else { + try writer.writeAll(self.buf); + } } }; +pub const PathFormatOptions = struct { + escape_backslashes: bool = false, +}; + pub fn fmtUTF16(buf: []const u16) FormatUTF16 { return FormatUTF16{ .buf = buf }; } pub const FormatOSPath = if (Environment.isWindows) FormatUTF16 else FormatUTF8; -pub fn fmtOSPath(buf: bun.OSPathSlice) FormatOSPath { - return FormatOSPath{ .buf = buf }; +pub fn fmtOSPath(buf: bun.OSPathSlice, options: PathFormatOptions) FormatOSPath { + return FormatOSPath{ + .buf = buf, + .escape_backslashes = options.escape_backslashes, + }; +} + +pub fn fmtPath( + comptime T: type, + path: []const T, + options: PathFormatOptions, +) if (T == u8) FormatUTF8 else FormatUTF16 { + if (T == u8) { + return FormatUTF8{ + .buf = path, + .escape_backslashes = options.escape_backslashes, + }; + } + + return FormatUTF16{ + .buf = path, + .escape_backslashes = options.escape_backslashes, + }; } pub fn formatLatin1(slice_: []const u8, writer: anytype) !void { diff --git a/src/install/install.zig b/src/install/install.zig index 4fdcaa578f5f09..8c09a31752de87 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -1291,7 +1291,7 @@ pub const PackageInstall = struct { progress_.refresh(); - Output.prettyErrorln("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path) }); + Output.prettyErrorln("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); Global.crash(); }; }; @@ -1316,7 +1316,7 @@ pub const PackageInstall = struct { progress_.refresh(); - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path) }); + Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); Global.crash(); }; } else { @@ -1330,7 +1330,7 @@ pub const PackageInstall = struct { progress_.refresh(); - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path) }); + Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); Global.crash(); }; } @@ -1378,8 +1378,8 @@ pub const PackageInstall = struct { defer subdir.close(); - var buf: if (Environment.isWindows) bun.WPathBuffer else [0]u16 = undefined; - var buf2: if (Environment.isWindows) bun.WPathBuffer else [0]u16 = undefined; + var buf: bun.windows.WPathBuffer = undefined; + var buf2: bun.windows.WPathBuffer = undefined; var to_copy_buf: []u16 = undefined; var to_copy_buf2: []u16 = undefined; if (comptime Environment.isWindows) { diff --git a/src/libarchive/libarchive.zig b/src/libarchive/libarchive.zig index d5d00754e09610..542f0131750dbd 100644 --- a/src/libarchive/libarchive.zig +++ b/src/libarchive/libarchive.zig @@ -548,7 +548,7 @@ pub const Archive = struct { const path_slice: bun.OSPathSlice = pathname.ptr[0..pathname.len]; if (comptime log) { - Output.prettyln(" {}", .{bun.fmt.fmtOSPath(path_slice)}); + Output.prettyln(" {}", .{bun.fmt.fmtOSPath(path_slice, .{})}); } count += 1; @@ -699,7 +699,7 @@ pub const Archive = struct { lib.ARCHIVE_RETRY => { if (comptime log) { Output.err("libarchive error", "extracting {}, retry {d} / {d}", .{ - bun.fmt.fmtOSPath(path_slice), + bun.fmt.fmtOSPath(path_slice, .{}), retries_remaining, 5, }); @@ -709,7 +709,7 @@ pub const Archive = struct { if (comptime log) { const archive_error = std.mem.span(lib.archive_error_string(archive)); Output.err("libarchive error", "extracting {}: {s}", .{ - bun.fmt.fmtOSPath(path_slice), + bun.fmt.fmtOSPath(path_slice, .{}), archive_error, }); } diff --git a/test/transpiler/macro-test.test.ts b/test/transpiler/macro-test.test.ts index a7de24f6b15e82..401168bb109a90 100644 --- a/test/transpiler/macro-test.test.ts +++ b/test/transpiler/macro-test.test.ts @@ -1,5 +1,3 @@ -// @known-failing-on-windows: panic "TODO on Windows" - import { expect, test } from "bun:test"; import { addStrings, addStringsUTF16, escape, identity } from "./macro.ts" assert { type: "macro" }; import { escapeHTML } from "bun" assert { type: "macro" };