From 0601eb00076089e7edf65233f930e6e1d3d166d1 Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Wed, 8 Oct 2025 18:00:38 -0700 Subject: [PATCH] Make `--linker=isolated` the default for `bun install` (#23311) ### What does this PR do? Makes isolated installs the default install strategy for projects with workspaces in Bun v1.3. Also fixes creating patches with `bun patch` and `--linker isolated` Fixes #22693 ### How did you verify your code works? Added tests for node_modules renaming `bun patch` with isolated install. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Claude Bot Co-authored-by: Claude --- src/StandaloneModuleGraph.zig | 4 +- src/bun.js/ModuleLoader.zig | 2 +- src/bun.js/RuntimeTranspilerCache.zig | 2 +- src/compile_target.zig | 2 +- src/fs.zig | 2 +- src/install/PackageInstaller.zig | 5 +- src/install/PackageManager.zig | 2 +- .../PackageManagerDirectories.zig | 2 +- .../PackageManager/install_with_manager.zig | 33 +- src/install/PackageManager/patchPackage.zig | 204 ++-------- src/install/extract_tarball.zig | 14 +- src/install/install.zig | 1 + src/install/isolated_install.zig | 231 ++++++++--- src/install/isolated_install/FileCopier.zig | 5 +- src/install/isolated_install/Installer.zig | 83 +++- src/install/lockfile.zig | 13 +- src/install/migration.zig | 2 +- src/install/patch_install.zig | 40 +- src/install/pnpm.zig | 2 +- src/install/repository.zig | 2 +- src/install/yarn.zig | 2 +- src/paths/Path.zig | 5 - src/semver/SemverString.zig | 13 +- src/sys.zig | 20 +- .../__snapshots__/bun-install.test.ts.snap | 19 - test/cli/install/bun-add.test.ts | 23 +- test/cli/install/bun-install-cpu-os.test.ts | 4 +- test/cli/install/bun-install-patch.test.ts | 382 +++++++++++++++++- .../bun-install-security-provider.test.ts | 4 +- test/cli/install/bun-install.test.ts | 59 ++- test/cli/install/bun-link.test.ts | 2 +- test/cli/install/bun-patch.test.ts | 2 +- test/cli/install/bun-pm.test.ts | 4 +- .../bun-update-security-provider.test.ts | 4 +- test/cli/install/bun-update.test.ts | 8 +- test/cli/install/dummy.registry.ts | 3 +- test/cli/install/isolated-install.test.ts | 136 ++++++- test/cli/install/lockfile-only.test.ts | 4 +- test/harness.ts | 15 +- test/internal/ban-limits.json | 4 +- test/regression/issue/08093.test.ts | 4 +- 41 files changed, 985 insertions(+), 383 deletions(-) diff --git a/src/StandaloneModuleGraph.zig b/src/StandaloneModuleGraph.zig index 9b045a1fad..b09e527d0c 100644 --- a/src/StandaloneModuleGraph.zig +++ b/src/StandaloneModuleGraph.zig @@ -540,10 +540,10 @@ pub const StandaloneModuleGraph = struct { pub fn inject(bytes: []const u8, self_exe: [:0]const u8, inject_options: InjectOptions, target: *const CompileTarget) bun.FileDescriptor { var buf: bun.PathBuffer = undefined; - var zname: [:0]const u8 = bun.span(bun.fs.FileSystem.instance.tmpname("bun-build", &buf, @as(u64, @bitCast(std.time.milliTimestamp()))) catch |err| { + var zname: [:0]const u8 = bun.fs.FileSystem.tmpname("bun-build", &buf, @as(u64, @bitCast(std.time.milliTimestamp()))) catch |err| { Output.prettyErrorln("error: failed to get temporary file name: {s}", .{@errorName(err)}); return bun.invalid_fd; - }); + }; const cleanup = struct { pub fn toClean(name: [:0]const u8, fd: bun.FileDescriptor) void { diff --git a/src/bun.js/ModuleLoader.zig b/src/bun.js/ModuleLoader.zig index 09921adc0a..9a291d5def 100644 --- a/src/bun.js/ModuleLoader.zig +++ b/src/bun.js/ModuleLoader.zig @@ -38,7 +38,7 @@ pub fn resolveEmbeddedFile(vm: *VirtualMachine, input_path: []const u8, extname: // atomically write to a tmpfile and then move it to the final destination var tmpname_buf: bun.PathBuffer = undefined; - const tmpfilename = bun.sliceTo(bun.fs.FileSystem.instance.tmpname(extname, &tmpname_buf, bun.hash(file.name)) catch return null, 0); + const tmpfilename = bun.fs.FileSystem.tmpname(extname, &tmpname_buf, bun.hash(file.name)) catch return null; const tmpdir: bun.FD = .fromStdDir(bun.fs.FileSystem.instance.tmpdir() catch return null); diff --git a/src/bun.js/RuntimeTranspilerCache.zig b/src/bun.js/RuntimeTranspilerCache.zig index b951af8bdf..7fa3c3e472 100644 --- a/src/bun.js/RuntimeTranspilerCache.zig +++ b/src/bun.js/RuntimeTranspilerCache.zig @@ -163,7 +163,7 @@ pub const RuntimeTranspilerCache = struct { // atomically write to a tmpfile and then move it to the final destination var tmpname_buf: bun.PathBuffer = undefined; - const tmpfilename = bun.sliceTo(try bun.fs.FileSystem.instance.tmpname(std.fs.path.extension(destination_path.slice()), &tmpname_buf, input_hash), 0); + const tmpfilename = try bun.fs.FileSystem.tmpname(std.fs.path.extension(destination_path.slice()), &tmpname_buf, input_hash); const output_bytes = output_code.byteSlice(); diff --git a/src/compile_target.zig b/src/compile_target.zig index aa0483ae5a..5ec1c5cecc 100644 --- a/src/compile_target.zig +++ b/src/compile_target.zig @@ -248,7 +248,7 @@ pub fn downloadToPath(this: *const CompileTarget, env: *bun.DotEnv.Loader, alloc const libarchive = bun.libarchive; var tmpname_buf: [1024]u8 = undefined; - const tempdir_name = bun.span(try bun.fs.FileSystem.instance.tmpname("tmp", &tmpname_buf, bun.fastRandom())); + const tempdir_name = try bun.fs.FileSystem.tmpname("tmp", &tmpname_buf, bun.fastRandom()); var tmpdir = try std.fs.cwd().makeOpenPath(tempdir_name, .{}); defer tmpdir.close(); defer std.fs.cwd().deleteTree(tempdir_name) catch {}; diff --git a/src/fs.zig b/src/fs.zig index 6f6542b874..fcb2264b97 100644 --- a/src/fs.zig +++ b/src/fs.zig @@ -47,7 +47,7 @@ pub const FileSystem = struct { } var tmpname_id_number = std.atomic.Value(u32).init(0); - pub fn tmpname(_: *const FileSystem, extname: string, buf: []u8, hash: u64) ![*:0]u8 { + pub fn tmpname(extname: string, buf: []u8, hash: u64) std.fmt.BufPrintError![:0]u8 { const hex_value = @as(u64, @truncate(@as(u128, @intCast(hash)) | @as(u128, @intCast(std.time.nanoTimestamp())))); return try std.fmt.bufPrintZ(buf, ".{any}-{any}.{s}", .{ diff --git a/src/install/PackageInstaller.zig b/src/install/PackageInstaller.zig index ee0ad7e6bc..fa615f787b 100644 --- a/src/install/PackageInstaller.zig +++ b/src/install/PackageInstaller.zig @@ -121,7 +121,6 @@ pub const PackageInstaller = struct { return (try bun.sys.openDirAtWindowsA(.fromStdDir(root), this.path.items, .{ .can_rename_or_delete = false, - .create = false, .read_only = false, }).unwrap()).stdDir(); } @@ -132,11 +131,9 @@ pub const PackageInstaller = struct { break :brk try root.makeOpenPath(this.path.items, .{ .iterate = true, .access_sub_paths = true }); } - // TODO: is this `makePath` necessary with `.create = true` below - try bun.MakePath.makePath(u8, root, this.path.items); break :brk (try bun.sys.openDirAtWindowsA(.fromStdDir(root), this.path.items, .{ .can_rename_or_delete = false, - .create = true, + .op = .open_or_create, .read_only = false, }).unwrap()).stdDir(); }; diff --git a/src/install/PackageManager.zig b/src/install/PackageManager.zig index 28f8eaf6ae..6746975443 100644 --- a/src/install/PackageManager.zig +++ b/src/install/PackageManager.zig @@ -454,7 +454,7 @@ var ensureTempNodeGypScriptOnce = bun.once(struct { const tempdir = manager.getTemporaryDirectory(); var path_buf: bun.PathBuffer = undefined; - const node_gyp_tempdir_name = bun.span(try Fs.FileSystem.instance.tmpname("node-gyp", &path_buf, 12345)); + const node_gyp_tempdir_name = try Fs.FileSystem.tmpname("node-gyp", &path_buf, 12345); // used later for adding to path for scripts manager.node_gyp_tempdir_name = try manager.allocator.dupe(u8, node_gyp_tempdir_name); diff --git a/src/install/PackageManager/PackageManagerDirectories.zig b/src/install/PackageManager/PackageManagerDirectories.zig index c9063a651d..3f83c43ce3 100644 --- a/src/install/PackageManager/PackageManagerDirectories.zig +++ b/src/install/PackageManager/PackageManagerDirectories.zig @@ -41,7 +41,7 @@ var getTemporaryDirectoryOnce = bun.once(struct { }; }; var tmpbuf: bun.PathBuffer = undefined; - const tmpname = Fs.FileSystem.instance.tmpname("hm", &tmpbuf, bun.fastRandom()) catch unreachable; + const tmpname = Fs.FileSystem.tmpname("hm", &tmpbuf, bun.fastRandom()) catch unreachable; var timer: std.time.Timer = if (manager.options.log_level != .silent) std.time.Timer.start() catch unreachable else undefined; brk: while (true) { var file = tempdir.createFileZ(tmpname, .{ .truncate = true }) catch |err2| { diff --git a/src/install/PackageManager/install_with_manager.zig b/src/install/PackageManager/install_with_manager.zig index cb4663e145..ffd797cca9 100644 --- a/src/install/PackageManager/install_with_manager.zig +++ b/src/install/PackageManager/install_with_manager.zig @@ -38,7 +38,7 @@ pub fn installWithManager( manager.options.enable.force_save_lockfile = manager.options.enable.force_save_lockfile or (load_result == .ok and // if migrated always save a new lockfile - (load_result.ok.was_migrated or + (load_result.ok.migrated != .none or // if loaded from binary and save-text-lockfile is passed (load_result.ok.format == .binary and @@ -776,9 +776,29 @@ pub fn installWithManager( } switch (manager.options.node_linker) { + .auto => { + if (manager.lockfile.workspace_paths.count() > 0 and + !load_result.migratedFromNpm()) + { + break :install_summary bun.handleOom(installIsolatedPackages( + manager, + ctx, + install_root_dependencies, + workspace_filters, + null, + )); + } + break :install_summary try installHoistedPackages( + manager, + ctx, + workspace_filters, + install_root_dependencies, + log_level, + null, + ); + }, + .hoisted, - // TODO - .auto, => break :install_summary try installHoistedPackages( manager, ctx, @@ -788,15 +808,14 @@ pub fn installWithManager( null, ), - .isolated => break :install_summary installIsolatedPackages( + .isolated, + => break :install_summary bun.handleOom(installIsolatedPackages( manager, ctx, install_root_dependencies, workspace_filters, null, - ) catch |err| switch (err) { - error.OutOfMemory => bun.outOfMemory(), - }, + )), } }; diff --git a/src/install/PackageManager/patchPackage.zig b/src/install/PackageManager/patchPackage.zig index a8c286ae15..6221283813 100644 --- a/src/install/PackageManager/patchPackage.zig +++ b/src/install/PackageManager/patchPackage.zig @@ -207,10 +207,10 @@ pub fn doPatchCommit( }, .posix); }; - const random_tempdir = bun.span(bun.fs.FileSystem.instance.tmpname("node_modules_tmp", buf2[0..], bun.fastRandom()) catch |e| { + const random_tempdir = bun.fs.FileSystem.tmpname("node_modules_tmp", buf2[0..], bun.fastRandom()) catch |e| { Output.err(e, "failed to make tempdir", .{}); Global.crash(); - }); + }; // If the package has nested a node_modules folder, we don't want this to // appear in the patch file when we run git diff. @@ -235,10 +235,10 @@ pub fn doPatchCommit( break :has_nested_node_modules true; }; - const patch_tag_tmpname = bun.span(bun.fs.FileSystem.instance.tmpname("patch_tmp", buf3[0..], bun.fastRandom()) catch |e| { + const patch_tag_tmpname = bun.fs.FileSystem.tmpname("patch_tmp", buf3[0..], bun.fastRandom()) catch |e| { Output.err(e, "failed to make tempdir", .{}); Global.crash(); - }); + }; var bunpatchtagbuf: BuntagHashBuf = undefined; // If the package was already patched then it might have a ".bun-tag-XXXXXXXX" @@ -395,7 +395,7 @@ pub fn doPatchCommit( // write the patch contents to temp file then rename var tmpname_buf: [1024]u8 = undefined; - const tempfile_name = bun.span(try bun.fs.FileSystem.instance.tmpname("tmp", &tmpname_buf, bun.fastRandom())); + const tempfile_name = try bun.fs.FileSystem.tmpname("tmp", &tmpname_buf, bun.fastRandom()); const tmpdir = manager.getTemporaryDirectory().handle; const tmpfd = switch (bun.sys.openat( .fromStdDir(tmpdir), @@ -708,7 +708,7 @@ pub fn preparePatch(manager: *PackageManager) !void { // meaning that changes to the folder will also change the package in the cache. // // So we will overwrite the folder by directly copying the package in cache into it - overwritePackageInNodeModulesFolder(manager, cache_dir, cache_dir_subpath, module_folder) catch |e| { + overwritePackageInNodeModulesFolder(cache_dir, cache_dir_subpath, module_folder) catch |e| { Output.prettyError( "error: error overwriting folder in node_modules: {s}\n", .{@errorName(e)}, @@ -729,170 +729,49 @@ pub fn preparePatch(manager: *PackageManager) !void { } fn overwritePackageInNodeModulesFolder( - manager: *PackageManager, cache_dir: std.fs.Dir, cache_dir_subpath: []const u8, node_modules_folder_path: []const u8, ) !void { - var node_modules_folder = try std.fs.cwd().openDir(node_modules_folder_path, .{ .iterate = true }); - defer node_modules_folder.close(); + FD.cwd().deleteTree(node_modules_folder_path) catch {}; - const IGNORED_PATHS: []const bun.OSPathSlice = &[_][]const bun.OSPathChar{ - bun.OSPathLiteral("node_modules"), - bun.OSPathLiteral(".git"), - bun.OSPathLiteral("CMakeFiles"), + var dest_subpath: bun.RelPath(.{ .sep = .auto, .unit = .os }) = .from(node_modules_folder_path); + defer dest_subpath.deinit(); + + const src_path: bun.AbsPath(.{ .sep = .auto, .unit = .os }) = src_path: { + if (comptime Environment.isWindows) { + var path_buf: bun.WPathBuffer = undefined; + const abs_path = try bun.getFdPathW(.fromStdDir(cache_dir), &path_buf); + + var src_path: bun.AbsPath(.{ .sep = .auto, .unit = .os }) = .from(abs_path); + src_path.append(cache_dir_subpath); + + break :src_path src_path; + } + + // unused if not windows + break :src_path .init(); + }; + defer src_path.deinit(); + + var cached_package_folder = try cache_dir.openDir(cache_dir_subpath, .{ .iterate = true }); + defer cached_package_folder.close(); + + const ignore_directories: []const bun.OSPathSlice = &.{ + comptime bun.OSPathLiteral("node_modules"), + comptime bun.OSPathLiteral(".git"), + comptime bun.OSPathLiteral("CMakeFiles"), }; - const FileCopier = struct { - pub fn copy( - destination_dir_: std.fs.Dir, - walker: *Walker, - in_dir: if (bun.Environment.isWindows) []const u16 else void, - out_dir: if (bun.Environment.isWindows) []const u16 else void, - buf1: if (bun.Environment.isWindows) []u16 else void, - buf2: if (bun.Environment.isWindows) []u16 else void, - tmpdir_in_node_modules: if (bun.Environment.isWindows) std.fs.Dir else void, - ) !u32 { - var real_file_count: u32 = 0; + var copier: bun.install.FileCopier = try .init( + .fromStdDir(cached_package_folder), + src_path, + dest_subpath, + ignore_directories, + ); + defer copier.deinit(); - var copy_file_state: bun.CopyFileState = .{}; - var pathbuf: bun.PathBuffer = undefined; - var pathbuf2: bun.PathBuffer = undefined; - // _ = pathbuf; // autofix - - while (try walker.next().unwrap()) |entry| { - if (entry.kind != .file) continue; - real_file_count += 1; - const createFile = std.fs.Dir.createFile; - - // 1. rename original file in node_modules to tmp_dir_in_node_modules - // 2. create the file again - // 3. copy cache flie to the newly re-created file - // 4. profit - if (comptime bun.Environment.isWindows) { - var tmpbuf: [1024]u8 = undefined; - const basename = bun.strings.fromWPath(pathbuf2[0..], entry.basename); - const tmpname = bun.span(bun.fs.FileSystem.instance.tmpname(basename, tmpbuf[0..], bun.fastRandom()) catch |e| { - Output.prettyError("error: copying file {s}", .{@errorName(e)}); - Global.crash(); - }); - - const entrypath = bun.strings.fromWPath(pathbuf[0..], entry.path); - pathbuf[entrypath.len] = 0; - const entrypathZ = pathbuf[0..entrypath.len :0]; - - if (bun.sys.renameatConcurrently( - .fromStdDir(destination_dir_), - entrypathZ, - .fromStdDir(tmpdir_in_node_modules), - tmpname, - .{ .move_fallback = true }, - ).asErr()) |e| { - Output.prettyError("error: copying file {}", .{e}); - Global.crash(); - } - - var outfile = createFile(destination_dir_, entrypath, .{}) catch |e| { - Output.prettyError("error: failed to create file {s} ({s})", .{ entrypath, @errorName(e) }); - Global.crash(); - }; - outfile.close(); - - const infile_path = bun.path.joinStringBufWZ(buf1, &[_][]const u16{ in_dir, entry.path }, .auto); - const outfile_path = bun.path.joinStringBufWZ(buf2, &[_][]const u16{ out_dir, entry.path }, .auto); - - bun.copyFileWithState(infile_path, outfile_path, ©_file_state).unwrap() catch |err| { - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); - Global.crash(); - }; - } else if (comptime Environment.isPosix) { - var in_file = try entry.dir.openat(entry.basename, bun.O.RDONLY, 0).unwrap(); - defer in_file.close(); - - @memcpy(pathbuf[0..entry.path.len], entry.path); - pathbuf[entry.path.len] = 0; - - if (bun.sys.unlinkat( - .fromStdDir(destination_dir_), - pathbuf[0..entry.path.len :0], - ).asErr()) |e| { - Output.prettyError("error: copying file {}", .{e.withPath(entry.path)}); - Global.crash(); - } - - var outfile = try createFile(destination_dir_, entry.path, .{}); - defer outfile.close(); - - const stat = in_file.stat().unwrap() catch continue; - _ = bun.c.fchmod(outfile.handle, @intCast(stat.mode)); - - bun.copyFileWithState(in_file, .fromStdFile(outfile), ©_file_state).unwrap() catch |err| { - Output.prettyError("{s}: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) }); - Global.crash(); - }; - } - } - - return real_file_count; - } - }; - - var pkg_in_cache_dir = try cache_dir.openDir(cache_dir_subpath, .{ .iterate = true }); - defer pkg_in_cache_dir.close(); - var walker = bun.handleOom(Walker.walk(.fromStdDir(pkg_in_cache_dir), manager.allocator, &.{}, IGNORED_PATHS)); - defer walker.deinit(); - - var buf1: if (bun.Environment.isWindows) bun.WPathBuffer else void = undefined; - var buf2: if (bun.Environment.isWindows) bun.WPathBuffer else void = undefined; - var in_dir: if (bun.Environment.isWindows) []const u16 else void = undefined; - var out_dir: if (bun.Environment.isWindows) []const u16 else void = undefined; - - if (comptime bun.Environment.isWindows) { - const inlen = bun.windows.GetFinalPathNameByHandleW(pkg_in_cache_dir.fd, &buf1, buf1.len, 0); - if (inlen == 0) { - const e = bun.windows.Win32Error.get(); - const err = if (e.toSystemErrno()) |sys_err| bun.errnoToZigErr(sys_err) else error.Unexpected; - Output.prettyError("error: copying file {}", .{err}); - Global.crash(); - } - in_dir = buf1[0..inlen]; - const outlen = bun.windows.GetFinalPathNameByHandleW(node_modules_folder.fd, &buf2, buf2.len, 0); - if (outlen == 0) { - const e = bun.windows.Win32Error.get(); - const err = if (e.toSystemErrno()) |sys_err| bun.errnoToZigErr(sys_err) else error.Unexpected; - Output.prettyError("error: copying file {}", .{err}); - Global.crash(); - } - out_dir = buf2[0..outlen]; - var tmpbuf: [1024]u8 = undefined; - const tmpname = bun.span(bun.fs.FileSystem.instance.tmpname("tffbp", tmpbuf[0..], bun.fastRandom()) catch |e| { - Output.prettyError("error: copying file {s}", .{@errorName(e)}); - Global.crash(); - }); - const temp_folder_in_node_modules = try node_modules_folder.makeOpenPath(tmpname, .{}); - defer { - node_modules_folder.deleteTree(tmpname) catch {}; - } - _ = try FileCopier.copy( - node_modules_folder, - &walker, - in_dir, - out_dir, - &buf1, - &buf2, - temp_folder_in_node_modules, - ); - } else if (Environment.isPosix) { - _ = try FileCopier.copy( - node_modules_folder, - &walker, - {}, - {}, - {}, - {}, - {}, - ); - } + try copier.copy().unwrap(); } fn nodeModulesFolderForDependencyIDs(iterator: *Lockfile.Tree.Iterator(.node_modules), ids: []const IdPair) !?Lockfile.Tree.Iterator(.node_modules).Next { @@ -1086,7 +965,6 @@ const PatchArgKind = enum { const string = []const u8; const stringZ = [:0]const u8; -const Walker = @import("../../walker_skippable.zig"); const std = @import("std"); const bun = @import("bun"); diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig index aefbfb8eea..5770c5d4f9 100644 --- a/src/install/extract_tarball.zig +++ b/src/install/extract_tarball.zig @@ -146,9 +146,9 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) }; var resolved: string = ""; - const tmpname = try FileSystem.instance.tmpname(basename[0..@min(basename.len, 32)], std.mem.asBytes(&tmpname_buf), bun.fastRandom()); + const tmpname = try FileSystem.tmpname(basename[0..@min(basename.len, 32)], std.mem.asBytes(&tmpname_buf), bun.fastRandom()); { - var extract_destination = bun.MakePath.makeOpenPath(tmpdir, bun.span(tmpname), .{}) catch |err| { + var extract_destination = bun.MakePath.makeOpenPath(tmpdir, tmpname, .{}) catch |err| { log.addErrorFmt( null, logger.Loc.Empty, @@ -213,7 +213,7 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) logger.Loc.Empty, bun.default_allocator, "{s} decompressing \"{s}\" to \"{}\"", - .{ @errorName(err), name, bun.fmt.fmtPath(u8, std.mem.span(tmpname), .{}) }, + .{ @errorName(err), name, bun.fmt.fmtPath(u8, tmpname, .{}) }, ) catch unreachable; return error.InstallFailed; }; @@ -315,9 +315,8 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) const path_to_use = path2; while (true) { - const dir_to_move = bun.sys.openDirAtWindowsA(.fromStdDir(this.temp_dir), bun.span(tmpname), .{ + const dir_to_move = bun.sys.openDirAtWindowsA(.fromStdDir(this.temp_dir), tmpname, .{ .can_rename_or_delete = true, - .create = false, .iterable = false, .read_only = true, }).unwrap() catch |err| { @@ -348,7 +347,7 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) // and then delete that temp dir // The goal is to make it more difficult for an application to reach this folder var tmpname_bytes = std.mem.asBytes(&tmpname_buf); - const tmpname_len = std.mem.sliceTo(tmpname, 0).len; + const tmpname_len = tmpname.len; tmpname_bytes[tmpname_len..][0..4].* = .{ 't', 'm', 'p', 0 }; const tempdest = tmpname_bytes[0 .. tmpname_len + 3 :0]; @@ -396,7 +395,6 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) // 2b. Delete the temporary directory version ONLY if we're not using a provided temporary directory // 3. If rename still fails, fallback to racily deleting the cache directory version and then renaming the temporary directory version again. // - const src = bun.sliceTo(tmpname, 0); if (create_subdir) { if (bun.Dirname.dirname(u8, folder_name)) |folder| { @@ -406,7 +404,7 @@ fn extract(this: *const ExtractTarball, log: *logger.Log, tgz_bytes: []const u8) if (bun.sys.renameatConcurrently( .fromStdDir(tmpdir), - src, + tmpname, .fromStdDir(cache_dir), folder_name, .{ .move_fallback = true }, diff --git a/src/install/install.zig b/src/install/install.zig index daeb49d773..091d353d13 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -255,6 +255,7 @@ pub const PackageInstall = @import("./PackageInstall.zig").PackageInstall; pub const Repository = @import("./repository.zig").Repository; pub const Resolution = @import("./resolution.zig").Resolution; pub const Store = @import("./isolated_install/Store.zig").Store; +pub const FileCopier = @import("./isolated_install/FileCopier.zig").FileCopier; pub const ArrayIdentityContext = @import("../identity_context.zig").ArrayIdentityContext; pub const IdentityContext = @import("../identity_context.zig").IdentityContext; diff --git a/src/install/isolated_install.zig b/src/install/isolated_install.zig index 8576956ef2..f291cf12d7 100644 --- a/src/install/isolated_install.zig +++ b/src/install/isolated_install.zig @@ -559,71 +559,6 @@ pub fn installIsolatedPackages( }; }; - const cwd = FD.cwd(); - - const root_node_modules_dir, const is_new_root_node_modules, const bun_modules_dir, const is_new_bun_modules = root_dirs: { - const node_modules_path = bun.OSPathLiteral("node_modules"); - const bun_modules_path = bun.OSPathLiteral("node_modules/" ++ Store.modules_dir_name); - const existing_root_node_modules_dir = sys.openatOSPath(cwd, node_modules_path, bun.O.DIRECTORY | bun.O.RDONLY, 0o755).unwrap() catch { - sys.mkdirat(cwd, node_modules_path, 0o755).unwrap() catch |err| { - Output.err(err, "failed to create the './node_modules' directory", .{}); - Global.exit(1); - }; - - sys.mkdirat(cwd, bun_modules_path, 0o755).unwrap() catch |err| { - Output.err(err, "failed to create the './node_modules/.bun' directory", .{}); - Global.exit(1); - }; - - const new_root_node_modules_dir = sys.openatOSPath(cwd, node_modules_path, bun.O.DIRECTORY | bun.O.RDONLY, 0o755).unwrap() catch |err| { - Output.err(err, "failed to open the './node_modules' directory", .{}); - Global.exit(1); - }; - - const new_bun_modules_dir = sys.openatOSPath(cwd, bun_modules_path, bun.O.DIRECTORY | bun.O.RDONLY, 0o755).unwrap() catch |err| { - Output.err(err, "failed to open the './node_modules/.bun' directory", .{}); - Global.exit(1); - }; - - break :root_dirs .{ - new_root_node_modules_dir, - true, - new_bun_modules_dir, - true, - }; - }; - - const existing_bun_modules_dir = sys.openatOSPath(cwd, bun_modules_path, bun.O.DIRECTORY | bun.O.RDONLY, 0o755).unwrap() catch { - sys.mkdirat(cwd, bun_modules_path, 0o755).unwrap() catch |err| { - Output.err(err, "failed to create the './node_modules/.bun' directory", .{}); - Global.exit(1); - }; - - const new_bun_modules_dir = sys.openatOSPath(cwd, bun_modules_path, bun.O.DIRECTORY | bun.O.RDONLY, 0o755).unwrap() catch |err| { - Output.err(err, "failed to open the './node_modules/.bun' directory", .{}); - Global.exit(1); - }; - - break :root_dirs .{ - existing_root_node_modules_dir, - false, - new_bun_modules_dir, - true, - }; - }; - - break :root_dirs .{ - existing_root_node_modules_dir, - false, - existing_bun_modules_dir, - false, - }; - }; - _ = root_node_modules_dir; - _ = is_new_root_node_modules; - _ = bun_modules_dir; - // _ = is_new_bun_modules; - { var root_node: *Progress.Node = undefined; var download_node: Progress.Node = undefined; @@ -696,6 +631,161 @@ pub fn installIsolatedPackages( }; } + const is_new_bun_modules = is_new_bun_modules: { + const node_modules_path = bun.OSPathLiteral("node_modules"); + const bun_modules_path = bun.OSPathLiteral("node_modules/" ++ Store.modules_dir_name); + + sys.mkdirat(FD.cwd(), node_modules_path, 0o755).unwrap() catch { + sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch { + break :is_new_bun_modules false; + }; + + // 'node_modules' exists and 'node_modules/.bun' doesn't + + if (comptime Environment.isWindows) { + // Windows: + // 1. create 'node_modules/.old_modules-{hex}' + // 2. for each entry in 'node_modules' rename into 'node_modules/.old_modules-{hex}' + // 3. for each workspace 'node_modules' rename into 'node_modules/.old_modules-{hex}/old_{basename}_modules' + + var rename_path: bun.AutoRelPath = .init(); + defer rename_path.deinit(); + + { + var mkdir_path: bun.RelPath(.{ .sep = .auto, .unit = .u16 }) = .from("node_modules"); + defer mkdir_path.deinit(); + + mkdir_path.appendFmt(".old_modules-{s}", .{&std.fmt.bytesToHex(std.mem.asBytes(&bun.fastRandom()), .lower)}); + rename_path.append(mkdir_path.slice()); + + // 1 + sys.mkdirat(FD.cwd(), mkdir_path.sliceZ(), 0o755).unwrap() catch { + break :is_new_bun_modules true; + }; + } + + const node_modules = bun.openDirForIteration(FD.cwd(), "node_modules").unwrap() catch { + break :is_new_bun_modules true; + }; + + var entry_path: bun.AutoRelPath = .from("node_modules"); + defer entry_path.deinit(); + + // 2 + var node_modules_iter = bun.DirIterator.iterate(node_modules, .u8); + while (node_modules_iter.next().unwrap() catch break :is_new_bun_modules true) |entry| { + if (bun.strings.startsWithChar(entry.name.slice(), '.')) { + continue; + } + + var entry_path_save = entry_path.save(); + defer entry_path_save.restore(); + + entry_path.append(entry.name.slice()); + + var rename_path_save = rename_path.save(); + defer rename_path_save.restore(); + + rename_path.append(entry.name.slice()); + + sys.renameat(FD.cwd(), entry_path.sliceZ(), FD.cwd(), rename_path.sliceZ()).unwrap() catch {}; + } + + // 3 + for (lockfile.workspace_paths.values()) |workspace_path| { + var workspace_node_modules: bun.AutoRelPath = .from(workspace_path.slice(string_buf)); + defer workspace_node_modules.deinit(); + + const basename = workspace_node_modules.basename(); + + workspace_node_modules.append("node_modules"); + + var rename_path_save = rename_path.save(); + defer rename_path_save.restore(); + + rename_path.appendFmt(".old_{s}_modules", .{basename}); + + sys.renameat(FD.cwd(), workspace_node_modules.sliceZ(), FD.cwd(), rename_path.sliceZ()).unwrap() catch {}; + } + } else { + + // Posix: + // 1. rename existing 'node_modules' to temp location + // 2. create new 'node_modules' directory + // 3. rename temp into 'node_modules/.old_modules-{hex}' + // 4. attempt renaming 'node_modules/.old_modules-{hex}/.cache' to 'node_modules/.cache' + // 5. rename each workspace 'node_modules' into 'node_modules/.old_modules-{hex}/old_{basename}_modules' + var temp_node_modules_buf: bun.PathBuffer = undefined; + const temp_node_modules = bun.fs.FileSystem.tmpname("tmp_modules", &temp_node_modules_buf, bun.fastRandom()) catch unreachable; + + // 1 + sys.renameat(FD.cwd(), "node_modules", FD.cwd(), temp_node_modules).unwrap() catch { + break :is_new_bun_modules true; + }; + + // 2 + sys.mkdirat(FD.cwd(), node_modules_path, 0o755).unwrap() catch |err| { + Output.err(err, "failed to create './node_modules'", .{}); + Global.exit(1); + }; + + sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch |err| { + Output.err(err, "failed to create './node_modules/.bun'", .{}); + Global.exit(1); + }; + + var rename_path: bun.AutoRelPath = .from("node_modules"); + defer rename_path.deinit(); + + rename_path.appendFmt(".old_modules-{s}", .{&std.fmt.bytesToHex(std.mem.asBytes(&bun.fastRandom()), .lower)}); + + // 3 + sys.renameat(FD.cwd(), temp_node_modules, FD.cwd(), rename_path.sliceZ()).unwrap() catch { + break :is_new_bun_modules true; + }; + + rename_path.append(".cache"); + + var cache_path: bun.AutoRelPath = .from("node_modules"); + defer cache_path.deinit(); + + cache_path.append(".cache"); + + // 4 + sys.renameat(FD.cwd(), rename_path.sliceZ(), FD.cwd(), cache_path.sliceZ()).unwrap() catch {}; + + // remove .cache so we can append destination for each workspace + rename_path.undo(1); + + // 5 + for (lockfile.workspace_paths.values()) |workspace_path| { + var workspace_node_modules: bun.AutoRelPath = .from(workspace_path.slice(string_buf)); + defer workspace_node_modules.deinit(); + + const basename = workspace_node_modules.basename(); + + workspace_node_modules.append("node_modules"); + + var rename_path_save = rename_path.save(); + defer rename_path_save.restore(); + + rename_path.appendFmt(".old_{s}_modules", .{basename}); + + sys.renameat(FD.cwd(), workspace_node_modules.sliceZ(), FD.cwd(), rename_path.sliceZ()).unwrap() catch {}; + } + } + + break :is_new_bun_modules true; + }; + + sys.mkdirat(FD.cwd(), bun_modules_path, 0o755).unwrap() catch |err| { + Output.err(err, "failed to create './node_modules/.bun'", .{}); + Global.exit(1); + }; + + break :is_new_bun_modules true; + }; + // add the pending task count upfront manager.incrementPendingTasks(@intCast(store.entries.len)); for (0..store.entries.len) |_entry_id| { @@ -841,6 +931,17 @@ pub fn installIsolatedPackages( }; if (!missing_from_cache) { + if (patch_info == .patch) { + var patch_log: bun.logger.Log = .init(manager.allocator); + installer.applyPackagePatch(entry_id, patch_info.patch, &patch_log); + if (patch_log.hasErrors()) { + // monotonic is okay because we haven't started the task yet (it isn't running + // on another thread) + entry_steps[entry_id.get()].store(.done, .monotonic); + installer.onTaskFail(entry_id, .{ .patching = patch_log }); + continue; + } + } installer.startTask(entry_id); continue; } diff --git a/src/install/isolated_install/FileCopier.zig b/src/install/isolated_install/FileCopier.zig index 07d6dd313a..0076e15736 100644 --- a/src/install/isolated_install/FileCopier.zig +++ b/src/install/isolated_install/FileCopier.zig @@ -1,6 +1,4 @@ pub const FileCopier = struct { - src_dir: FD, - src_path: bun.AbsPath(.{ .sep = .auto, .unit = .os }), dest_subpath: bun.RelPath(.{ .sep = .auto, .unit = .os }), walker: Walker, @@ -12,7 +10,6 @@ pub const FileCopier = struct { skip_dirnames: []const bun.OSPathSlice, ) OOM!FileCopier { return .{ - .src_dir = src_dir, .src_path = src_path, .dest_subpath = dest_subpath, .walker = try .walk( @@ -28,7 +25,7 @@ pub const FileCopier = struct { this.walker.deinit(); } - pub fn copy(this: *FileCopier) OOM!sys.Maybe(void) { + pub fn copy(this: *FileCopier) sys.Maybe(void) { var dest_dir = bun.MakePath.makeOpenPath(FD.cwd().stdDir(), this.dest_subpath.sliceZ(), .{}) catch |err| { // TODO: remove the need for this and implement openDir makePath makeOpenPath in bun var errno: bun.sys.E = switch (@as(anyerror, err)) { diff --git a/src/install/isolated_install/Installer.zig b/src/install/isolated_install/Installer.zig index e84e67eb40..b493e2ef46 100644 --- a/src/install/isolated_install/Installer.zig +++ b/src/install/isolated_install/Installer.zig @@ -43,13 +43,67 @@ pub const Installer = struct { pub fn onPackageExtracted(this: *Installer, task_id: install.Task.Id) void { if (this.manager.task_queue.fetchRemove(task_id)) |removed| { + const store = this.store; + + const node_pkg_ids = store.nodes.items(.pkg_id); + + const entries = store.entries.slice(); + const entry_steps = entries.items(.step); + const entry_node_ids = entries.items(.node_id); + + const pkgs = this.lockfile.packages.slice(); + const pkg_names = pkgs.items(.name); + const pkg_name_hashes = pkgs.items(.name_hash); + const pkg_resolutions = pkgs.items(.resolution); + for (removed.value.items) |install_ctx| { const entry_id = install_ctx.isolated_package_install_context; + + const node_id = entry_node_ids[entry_id.get()]; + const pkg_id = node_pkg_ids[node_id.get()]; + const pkg_name = pkg_names[pkg_id]; + const pkg_name_hash = pkg_name_hashes[pkg_id]; + const pkg_res = &pkg_resolutions[pkg_id]; + + const patch_info = bun.handleOom(this.packagePatchInfo(pkg_name, pkg_name_hash, pkg_res)); + + if (patch_info == .patch) { + var log: bun.logger.Log = .init(this.manager.allocator); + this.applyPackagePatch(entry_id, patch_info.patch, &log); + if (log.hasErrors()) { + // monotonic is okay because we haven't started the task yet (it isn't running + // on another thread) + entry_steps[entry_id.get()].store(.done, .monotonic); + this.onTaskFail(entry_id, .{ .patching = log }); + continue; + } + } + this.startTask(entry_id); } } } + pub fn applyPackagePatch(this: *Installer, entry_id: Store.Entry.Id, patch: PatchInfo.Patch, log: *bun.logger.Log) void { + const store = this.store; + const entry_node_ids = store.entries.items(.node_id); + const node_id = entry_node_ids[entry_id.get()]; + const node_pkg_ids = store.nodes.items(.pkg_id); + const pkg_id = node_pkg_ids[node_id.get()]; + const patch_task = install.PatchTask.newApplyPatchHash( + this.manager, + pkg_id, + patch.contents_hash, + patch.name_and_version_hash, + ); + defer patch_task.deinit(); + bun.handleOom(patch_task.apply()); + + if (patch_task.callback.apply.logger.hasErrors()) { + bun.handleOom(patch_task.callback.apply.logger.cloneToWithRecycled(log, true)); + } + } + /// Called from main thread pub fn onTaskFail(this: *Installer, entry_id: Store.Entry.Id, err: Task.Error) void { const string_buf = this.lockfile.buffers.string_bytes.items; @@ -83,6 +137,13 @@ pub const Installer = struct { pkg_res.fmt(string_buf, .auto), }); }, + .patching => |patch_log| { + Output.errGeneric("failed to patch package: {s}@{}", .{ + pkg_name.slice(string_buf), + pkg_res.fmt(string_buf, .auto), + }); + patch_log.print(Output.errorWriter()) catch {}; + }, else => {}, } Output.flush(); @@ -290,6 +351,7 @@ pub const Installer = struct { symlink_dependencies: sys.Error, run_scripts: anyerror, binaries: anyerror, + patching: bun.logger.Log, pub fn clone(this: *const Error, allocator: std.mem.Allocator) Error { return switch (this.*) { @@ -297,6 +359,7 @@ pub const Installer = struct { .symlink_dependencies => |err| .{ .symlink_dependencies = err.clone(allocator) }, .binaries => |err| .{ .binaries = err }, .run_scripts => |err| .{ .run_scripts = err }, + .patching => |log| .{ .patching = log }, }; } }; @@ -516,7 +579,7 @@ pub const Installer = struct { ); defer file_copier.deinit(); - switch (try file_copier.copy()) { + switch (file_copier.copy()) { .result => {}, .err => |err| { if (PackageManager.verbose_install) { @@ -709,7 +772,7 @@ pub const Installer = struct { ); defer file_copier.deinit(); - switch (try file_copier.copy()) { + switch (file_copier.copy()) { .result => {}, .err => |err| { if (PackageManager.verbose_install) { @@ -896,7 +959,7 @@ pub const Installer = struct { installer.appendStorePath(&pkg_cwd, this.entry_id); if (pkg_res.tag != .root and (pkg_res.tag == .workspace or is_trusted)) { - const pkg_scripts: *Package.Scripts = &pkg_script_lists[pkg_id]; + var pkg_scripts: Package.Scripts = pkg_script_lists[pkg_id]; var log = bun.logger.Log.init(bun.default_allocator); defer log.deinit(); @@ -1085,14 +1148,18 @@ pub const Installer = struct { const PatchInfo = union(enum) { none, - remove: struct { + remove: Remove, + patch: Patch, + + pub const Remove = struct { name_and_version_hash: u64, - }, - patch: struct { + }; + + pub const Patch = struct { name_and_version_hash: u64, patch_path: string, contents_hash: u64, - }, + }; pub fn contentsHash(this: *const @This()) ?u64 { return switch (this.*) { @@ -1356,7 +1423,6 @@ const string = []const u8; const Hardlinker = @import("./Hardlinker.zig"); const std = @import("std"); -const FileCopier = @import("./FileCopier.zig").FileCopier; const Symlinker = @import("./Symlinker.zig").Symlinker; const bun = @import("bun"); @@ -1377,6 +1443,7 @@ const String = bun.Semver.String; const install = bun.install; const Bin = install.Bin; const DependencyID = install.DependencyID; +const FileCopier = bun.install.FileCopier; const PackageID = install.PackageID; const PackageInstall = install.PackageInstall; const PackageManager = install.PackageManager; diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 1feb716ae4..c2d29d3c7c 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -112,7 +112,7 @@ pub const LoadResult = union(enum) { ok: struct { lockfile: *Lockfile, loaded_from_binary_lockfile: bool, - was_migrated: bool = false, + migrated: enum { none, npm, yarn, pnpm } = .none, serializer_result: Serializer.SerializerLoadResult, format: LockfileFormat, }, @@ -145,6 +145,13 @@ pub const LoadResult = union(enum) { }; } + pub fn migratedFromNpm(this: *const LoadResult) bool { + return switch (this.*) { + .ok => |ok| ok.migrated == .npm, + else => false, + }; + } + pub fn saveFormat(this: LoadResult, options: *const PackageManager.Options) LockfileFormat { switch (this) { .not_found => { @@ -169,12 +176,12 @@ pub const LoadResult = union(enum) { return .text; } - if (ok.was_migrated) { + if (ok.migrated != .none) { return .binary; } } - if (ok.was_migrated) { + if (ok.migrated != .none) { return .text; } diff --git a/src/install/migration.zig b/src/install/migration.zig index fcf417b140..888cac4d2c 100644 --- a/src/install/migration.zig +++ b/src/install/migration.zig @@ -1086,7 +1086,7 @@ pub fn migrateNPMLockfile( return LoadResult{ .ok = .{ .lockfile = this, - .was_migrated = true, + .migrated = .npm, .loaded_from_binary_lockfile = false, .serializer_result = .{}, .format = .binary, diff --git a/src/install/patch_install.zig b/src/install/patch_install.zig index 3aefd75dec..7039eee6ba 100644 --- a/src/install/patch_install.zig +++ b/src/install/patch_install.zig @@ -147,21 +147,13 @@ pub const PatchTask = struct { // need to switch on version.tag and handle each case appropriately const calc_hash = &this.callback.calc_hash; const hash = calc_hash.result orelse { - const fmt = "\n\nErrors occurred while calculating hash for {s}:\n\n"; - const args = .{this.callback.calc_hash.patchfile_path}; - if (log_level.showProgress()) { - Output.prettyWithPrinterFn(fmt, args, Progress.log, &manager.progress); - } else { - Output.prettyErrorln( - fmt, - args, - ); + if (log_level != .silent) { + if (calc_hash.logger.hasErrors()) { + calc_hash.logger.print(Output.errorWriter()) catch {}; + } else { + Output.errGeneric("Failed to calculate hash for patch {s}", .{this.callback.calc_hash.patchfile_path}); + } } - if (calc_hash.logger.errors > 0) { - Output.prettyErrorln("\n\n", .{}); - calc_hash.logger.print(Output.errorWriter()) catch {}; - } - Output.flush(); Global.crash(); }; @@ -233,7 +225,7 @@ pub const PatchTask = struct { // 4. Apply patches to pkg in temp dir // 5. Add bun tag for patch hash // 6. rename() newly patched pkg to cache - pub fn apply(this: *PatchTask) !void { + pub fn apply(this: *PatchTask) bun.OOM!void { var log = &this.callback.apply.logger; debug("apply patch task", .{}); bun.assert(this.callback == .apply); @@ -280,12 +272,11 @@ pub const PatchTask = struct { // 2. Create temp dir to do all the modifications var tmpname_buf: [1024]u8 = undefined; - const tempdir_name = bun.span( - bun.fs.FileSystem.instance.tmpname("tmp", &tmpname_buf, bun.fastRandom()) catch |err| switch (err) { - // max len is 1+16+1+8+3, well below 1024 - error.NoSpaceLeft => unreachable, - }, - ); + const tempdir_name = bun.fs.FileSystem.tmpname("tmp", &tmpname_buf, bun.fastRandom()) catch |err| switch (err) { + // max len is 1+16+1+8+3, well below 1024 + error.NoSpaceLeft => unreachable, + }; + const system_tmpdir = this.tempdir; const pkg_name = this.callback.apply.pkgname; @@ -428,12 +419,10 @@ pub const PatchTask = struct { const stat: bun.Stat = switch (bun.sys.stat(absolute_patchfile_path)) { .err => |e| { if (e.getErrno() == .NOENT) { - const fmt = "\n\nerror: could not find patch file {s}\n\nPlease make sure it exists.\n\nTo create a new patch file run:\n\n bun patch {s}\n"; - const args = .{ + bun.handleOom(log.addErrorFmt(null, Loc.Empty, this.manager.allocator, "Couldn't find patch file: '{s}'\n\nTo create a new patch file run:\n\n bun patch {s}", .{ this.callback.calc_hash.patchfile_path, this.manager.lockfile.patched_dependencies.get(this.callback.calc_hash.name_and_version_hash).?.path.slice(this.manager.lockfile.buffers.string_bytes.items), - }; - bun.handleOom(log.addErrorFmt(null, Loc.Empty, this.manager.allocator, fmt, args)); + })); return null; } log.addWarningFmt( @@ -596,7 +585,6 @@ const bun = @import("bun"); const Global = bun.Global; const Output = bun.Output; const PackageManager = bun.PackageManager; -const Progress = bun.Progress; const ThreadPool = bun.ThreadPool; const String = bun.Semver.String; const Task = bun.install.Task; diff --git a/src/install/pnpm.zig b/src/install/pnpm.zig index e1b7cf0e8d..c959c9157f 100644 --- a/src/install/pnpm.zig +++ b/src/install/pnpm.zig @@ -832,7 +832,7 @@ pub fn migratePnpmLockfile( .ok = .{ .lockfile = lockfile, .loaded_from_binary_lockfile = false, - .was_migrated = true, + .migrated = .pnpm, .serializer_result = .{}, .format = .text, }, diff --git a/src/install/repository.zig b/src/install/repository.zig index 1fe39d2e41..59feeef8b6 100644 --- a/src/install/repository.zig +++ b/src/install/repository.zig @@ -301,7 +301,7 @@ pub const Repository = extern struct { try writer.writeByte('+'); } else if (Dependency.isSCPLikePath(this.repo.repo.slice(this.string_buf))) { // try writer.print("ssh:{s}", .{if (this.opts.replace_slashes) "++" else "//"}); - try writer.writeAll("ssh:++"); + try writer.writeAll("ssh++"); } try writer.print("{}", .{this.repo.repo.fmtStorePath(this.string_buf)}); diff --git a/src/install/yarn.zig b/src/install/yarn.zig index 23e71140d5..6baaf5e9cb 100644 --- a/src/install/yarn.zig +++ b/src/install/yarn.zig @@ -1681,7 +1681,7 @@ pub fn migrateYarnLockfile( const result = LoadResult{ .ok = .{ .lockfile = this, - .was_migrated = true, + .migrated = .yarn, .loaded_from_binary_lockfile = false, .serializer_result = .{}, .format = .binary, diff --git a/src/paths/Path.zig b/src/paths/Path.zig index 363f69f180..43805eec73 100644 --- a/src/paths/Path.zig +++ b/src/paths/Path.zig @@ -398,11 +398,6 @@ pub fn Path(comptime opts: Options) type { return this; } pub fn from(input: anytype) Result(@This()) { - switch (comptime @TypeOf(input)) { - []u8, []const u8, [:0]u8, [:0]const u8 => {}, - []u16, []const u16, [:0]u16, [:0]const u16 => {}, - else => @compileError("unsupported type: " ++ @typeName(@TypeOf(input))), - } const trimmed = switch (comptime opts.kind) { .abs => trimmed: { bun.debugAssert(isInputAbsolute(input)); diff --git a/src/semver/SemverString.zig b/src/semver/SemverString.zig index 649737a868..05e4e80f07 100644 --- a/src/semver/SemverString.zig +++ b/src/semver/SemverString.zig @@ -177,11 +177,14 @@ pub const String = extern struct { pub fn format(this: StorePathFormatter, comptime _: string, _: std.fmt.FormatOptions, writer: anytype) @TypeOf(writer).Error!void { for (this.str.slice(this.buf)) |c| { - switch (c) { - '/' => try writer.writeByte('+'), - '\\' => try writer.writeByte('+'), - else => try writer.writeByte(c), - } + const n = switch (c) { + '/' => '+', + '\\' => '+', + ':' => '+', + '#' => '+', + else => c, + }; + try writer.writeByte(n); } } }; diff --git a/src/sys.zig b/src/sys.zig index 2062abc9ed..d97d3311a2 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -716,7 +716,7 @@ else mkdiratPosix; pub fn mkdiratW(dir_fd: bun.FileDescriptor, file_path: [:0]const u16, _: i32) Maybe(void) { - const dir_to_make = openDirAtWindowsNtPath(dir_fd, file_path, .{ .iterable = false, .can_rename_or_delete = true, .create = true }); + const dir_to_make = openDirAtWindowsNtPath(dir_fd, file_path, .{ .iterable = false, .can_rename_or_delete = true, .op = .only_create }); if (dir_to_make == .err) { return .{ .err = dir_to_make.err }; } @@ -953,7 +953,7 @@ fn openDirAtWindowsNtPath( return openWindowsDevicePath( path, flags, - if (options.create) w.FILE_OPEN_IF else w.FILE_OPEN, + if (options.op != .only_open) w.FILE_OPEN_IF else w.FILE_OPEN, w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT | open_reparse_point, ); @@ -987,7 +987,11 @@ fn openDirAtWindowsNtPath( null, 0, FILE_SHARE, - if (options.create) w.FILE_OPEN_IF else w.FILE_OPEN, + switch (options.op) { + .only_open => w.FILE_OPEN, + .only_create => w.FILE_CREATE, + .open_or_create => w.FILE_OPEN_IF, + }, w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT | open_reparse_point, null, 0, @@ -1059,12 +1063,18 @@ fn openWindowsDevicePath( return .{ .result = .fromNative(rc) }; } -pub const WindowsOpenDirOptions = packed struct { +pub const WindowsOpenDirOptions = struct { iterable: bool = false, no_follow: bool = false, can_rename_or_delete: bool = false, - create: bool = false, + op: Op = .only_open, read_only: bool = false, + + const Op = enum { + only_open, + only_create, + open_or_create, + }; }; fn openDirAtWindowsT( diff --git a/test/cli/install/__snapshots__/bun-install.test.ts.snap b/test/cli/install/__snapshots__/bun-install.test.ts.snap index 68e0ef1f8d..1925990f4e 100644 --- a/test/cli/install/__snapshots__/bun-install.test.ts.snap +++ b/test/cli/install/__snapshots__/bun-install.test.ts.snap @@ -34,22 +34,3 @@ error: "workspaces.packages" expects an array of strings, e.g. `; exports[`should handle modified git resolutions in bun.lock 1`] = `"{"lockfileVersion":0,"workspaces":{"":{"dependencies":{"jquery":"3.7.1"}}},"packages":{"jquery":["jquery@git+ssh://git@github.com/dylan-conway/install-test-8.git#3a1288830817d13da39e9231302261896f8721ea",{},"3a1288830817d13da39e9231302261896f8721ea"]}}"`; - -exports[`should read install.saveTextLockfile from bunfig.toml 1`] = ` -"{ - "lockfileVersion": 1, - "workspaces": { - "": { - "name": "foo", - }, - "packages/pkg1": { - "name": "pkg-one", - "version": "1.0.0", - }, - }, - "packages": { - "pkg-one": ["pkg-one@workspace:packages/pkg1"], - } -} -" -`; diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts index c6b0785ba2..c2553b2b9d 100644 --- a/test/cli/install/bun-add.test.ts +++ b/test/cli/install/bun-add.test.ts @@ -1064,6 +1064,9 @@ it("should add dependency alongside workspaces", async () => { name: "foo", version: "0.0.1", workspaces: ["packages/*"], + "dependencies": { + "bar": "workspace:*", + }, }), ); await mkdir(join(package_dir, "packages", "bar"), { recursive: true }); @@ -1097,7 +1100,14 @@ it("should add dependency alongside workspaces", async () => { expect(await exited).toBe(0); expect(urls.sort()).toEqual([`${root_url}/baz`, `${root_url}/baz-0.0.3.tgz`]); expect(requested).toBe(2); - expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".bin", ".cache", "bar", "baz"]); + expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ + ".bin", + ".bun", + ".cache", + expect.stringContaining(".old_modules-"), + "bar", + "baz", + ]); expect(await readdirSorted(join(package_dir, "node_modules", ".bin"))).toHaveBins(["baz-run"]); expect(join(package_dir, "node_modules", ".bin", "baz-run")).toBeValidBin(join("..", "baz", "index.js")); expect(await readlink(join(package_dir, "node_modules", "bar"))).toBeWorkspaceLink(join("..", "packages", "bar")); @@ -1117,6 +1127,7 @@ it("should add dependency alongside workspaces", async () => { version: "0.0.1", workspaces: ["packages/*"], dependencies: { + bar: "workspace:*", baz: "^0.0.3", }, }, @@ -2124,9 +2135,7 @@ it("should add dependencies to workspaces directly", async () => { expect(await readdirSorted(join(package_dir, "moo"))).toEqual(["bunfig.toml", "node_modules", "package.json"]); expect(await readdirSorted(join(package_dir, "moo", "node_modules", "foo"))).toEqual(["package.json"]); if (process.platform === "win32") { - expect(await file(await readlink(join(package_dir, "moo", "node_modules", "foo", "package.json"))).json()).toEqual( - fooPackage, - ); + expect(await file(join(package_dir, "moo", "node_modules", "foo", "package.json")).json()).toEqual(fooPackage); } else { expect(await file(join(package_dir, "moo", "node_modules", "foo", "package.json")).json()).toEqual(fooPackage); } @@ -2137,7 +2146,11 @@ it("should add dependencies to workspaces directly", async () => { foo: `file:${add_path.replace(/\\/g, "/")}`, }, }); - expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([".cache", "moo"]); + expect(await readdirSorted(join(package_dir, "node_modules"))).toEqual([ + ".bun", + ".cache", + expect.stringContaining(".old_modules-"), + ]); }); it("should redirect 'install --save X' to 'add'", async () => { diff --git a/test/cli/install/bun-install-cpu-os.test.ts b/test/cli/install/bun-install-cpu-os.test.ts index 1da5872dfe..40cf531218 100644 --- a/test/cli/install/bun-install-cpu-os.test.ts +++ b/test/cli/install/bun-install-cpu-os.test.ts @@ -19,7 +19,9 @@ expect.extend({ beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); describe("bun install --cpu and --os flags", () => { diff --git a/test/cli/install/bun-install-patch.test.ts b/test/cli/install/bun-install-patch.test.ts index 99e1d7fd57..2e51cf6fec 100644 --- a/test/cli/install/bun-install-patch.test.ts +++ b/test/cli/install/bun-install-patch.test.ts @@ -1,5 +1,6 @@ import { $ } from "bun"; import { beforeAll, describe, expect, it, setDefaultTimeout, test } from "bun:test"; +import { rmSync } from "fs"; import { bunEnv, bunExe, normalizeBunSnapshot as normalizeBunSnapshot_, tempDirWithFiles } from "harness"; import { join } from "path"; @@ -114,14 +115,7 @@ index c8950c17b265104bcf27f8c345df1a1b13a78950..7ce57ab96400ab0ff4fac7e06f6e02c2 } : (x: string) => x; - const versions: [version: string, patchVersion?: string][] = [ - ["1.0.0"], - ["github:i-voted-for-trump/is-even", "github:i-voted-for-trump/is-even#585f800"], - [ - "git@github.com:i-voted-for-trump/is-even.git", - "git+ssh://git@github.com:i-voted-for-trump/is-even.git#585f8002bb16f7bec723a47349b67df451f1b25d", - ], - ]; + const versions: [version: string, patchVersion?: string][] = [["1.0.0"]]; describe("should patch a dependency when its dependencies are not hoisted", async () => { // is-even depends on is-odd ^0.1.2 and we add is-odd 3.0.1, which should be hoisted @@ -559,7 +553,7 @@ index 832d92223a9ec491364ee10dcbe3ad495446ab80..7e079a817825de4b8c3d01898490dc7e await Bun.write(join(filedir, "package.json"), JSON.stringify(pkgjsonWithPatch)); await using proc = Bun.spawn({ - cmd: [bunExe(), "install"], + cmd: [bunExe(), "install", "--linker=hoisted"], env: bunEnv, cwd: filedir, stdout: "pipe", @@ -577,4 +571,374 @@ index 832d92223a9ec491364ee10dcbe3ad495446ab80..7e079a817825de4b8c3d01898490dc7e expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`"bun install ()"`); } }); + + describe("bun patch with --linker=isolated", () => { + test("should create patch for package and commit it", async () => { + const filedir = tempDirWithFiles("patch-isolated", { + "package.json": JSON.stringify({ + "name": "bun-patch-isolated-test", + "module": "index.ts", + "type": "module", + "dependencies": { + "is-even": "1.0.0", + }, + }), + "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(2));`, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Run bun patch command + const { stdout: patchStdout } = await $`${bunExe()} patch is-even`.env(bunEnv).cwd(filedir); + const patchOutput = patchStdout.toString(); + const relativePatchPath = + patchOutput.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath).toBeTruthy(); + const patchPath = join(filedir, relativePatchPath!); + + // Edit the patched package + const indexPath = join(patchPath, "index.js"); + const originalContent = await Bun.file(indexPath).text(); + const modifiedContent = originalContent.replace( + "module.exports = function isEven(i) {", + 'module.exports = function isEven(i) {\n console.log("PATCHED with isolated linker!");', + ); + await Bun.write(indexPath, modifiedContent); + + // Commit the patch + const { stderr: commitStderr } = await $`${bunExe()} patch --commit '${relativePatchPath}'` + .env(bunEnv) + .cwd(filedir); + + // With isolated linker, there may be some stderr output during patch commit + // but it should not contain actual errors + const commitStderrText = commitStderr.toString(); + expect(commitStderrText).not.toContain("error:"); + expect(commitStderrText).not.toContain("panic:"); + + // Verify patch file was created + const patchFile = join(filedir, "patches", "is-even@1.0.0.patch"); + expect(await Bun.file(patchFile).exists()).toBe(true); + + // Verify package.json was updated + const pkgJson = await Bun.file(join(filedir, "package.json")).json(); + expect(pkgJson.patchedDependencies).toEqual({ + "is-even@1.0.0": "patches/is-even@1.0.0.patch", + }); + + // Run the code to verify patch was applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("PATCHED with isolated linker!"); + }); + + test("should patch transitive dependency with isolated linker", async () => { + const filedir = tempDirWithFiles("patch-isolated-transitive", { + "package.json": JSON.stringify({ + "name": "bun-patch-isolated-transitive-test", + "module": "index.ts", + "type": "module", + "dependencies": { + "is-even": "1.0.0", + }, + }), + "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(3));`, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + await $`${bunExe()} patch is-odd`.env(bunEnv).cwd(filedir); + + // Patch transitive dependency (is-odd) + const { stdout: patchStdout } = await $`${bunExe()} patch is-odd@0.1.2`.env(bunEnv).cwd(filedir); + const patchOutput = patchStdout.toString(); + const relativePatchPath = + patchOutput.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath).toBeTruthy(); + const patchPath = join(filedir, relativePatchPath!); + + // Edit the patched package + const indexPath = join(patchPath, "index.js"); + const originalContent = await Bun.file(indexPath).text(); + const modifiedContent = originalContent.replace( + "module.exports = function isOdd(i) {", + 'module.exports = function isOdd(i) {\n console.log("Transitive patch with isolated!");', + ); + await Bun.write(indexPath, modifiedContent); + + // Commit the patch + const { stderr: commitStderr } = await $`${bunExe()} patch --commit '${relativePatchPath}'` + .env(bunEnv) + .cwd(filedir); + + await $`${bunExe()} i --linker isolated`.env(bunEnv).cwd(filedir); + + // With isolated linker, there may be some stderr output during patch commit + // but it should not contain actual errors + const commitStderrText = commitStderr.toString(); + expect(commitStderrText).not.toContain("error:"); + expect(commitStderrText).not.toContain("panic:"); + + // Verify patch was applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("Transitive patch with isolated!"); + }); + + test("should handle scoped packages with isolated linker", async () => { + const filedir = tempDirWithFiles("patch-isolated-scoped", { + "package.json": JSON.stringify({ + "name": "bun-patch-isolated-scoped-test", + "module": "index.ts", + "type": "module", + "dependencies": { + "@zackradisic/hls-dl": "0.0.1", + }, + }), + "index.ts": /* ts */ `import hlsDl from '@zackradisic/hls-dl'; console.log("Testing scoped package");`, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Patch scoped package + const { stdout: patchStdout } = await $`${bunExe()} patch @zackradisic/hls-dl`.env(bunEnv).cwd(filedir); + const patchOutput = patchStdout.toString(); + const relativePatchPath = + patchOutput.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath).toBeTruthy(); + const patchPath = join(filedir, relativePatchPath!); + + // Create a new index.js in the patched package + const indexPath = join(patchPath, "index.js"); + await Bun.write(indexPath, `module.exports = () => 'SCOPED PACKAGE PATCHED with isolated!';`); + + // Update package.json to point to the new index.js + const pkgJsonPath = join(patchPath, "package.json"); + const pkgJson = await Bun.file(pkgJsonPath).json(); + pkgJson.main = "./index.js"; + await Bun.write(pkgJsonPath, JSON.stringify(pkgJson, null, 2)); + + // Commit the patch + const { stderr: commitStderr } = await $`${bunExe()} patch --commit '${relativePatchPath}'` + .env(bunEnv) + .cwd(filedir); + + // With isolated linker, there may be some stderr output during patch commit + // but it should not contain actual errors + const commitStderrText = commitStderr.toString(); + expect(commitStderrText).not.toContain("error:"); + expect(commitStderrText).not.toContain("panic:"); + + // Update index.ts to actually use the patched module + await Bun.write( + join(filedir, "index.ts"), + /* ts */ `import hlsDl from '@zackradisic/hls-dl'; console.log(hlsDl());`, + ); + + // Verify patch was applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("SCOPED PACKAGE PATCHED with isolated!"); + }); + + test("should work with workspaces and isolated linker", async () => { + const filedir = tempDirWithFiles("patch-isolated-workspace", { + "package.json": JSON.stringify({ + "name": "workspace-root", + "workspaces": ["packages/*"], + }), + packages: { + app: { + "package.json": JSON.stringify({ + "name": "app", + "dependencies": { + "is-even": "1.0.0", + }, + }), + "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(4));`, + }, + }, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Patch from workspace root + const { stdout: patchStdout } = await $`${bunExe()} patch is-even`.env(bunEnv).cwd(filedir); + const patchOutput = patchStdout.toString(); + const relativePatchPath = + patchOutput.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath).toBeTruthy(); + const patchPath = join(filedir, relativePatchPath!); + + // Edit the patched package + const indexPath = join(patchPath, "index.js"); + const originalContent = await Bun.file(indexPath).text(); + const modifiedContent = originalContent.replace( + "module.exports = function isEven(i) {", + 'module.exports = function isEven(i) {\n console.log("WORKSPACE PATCH with isolated!");', + ); + await Bun.write(indexPath, modifiedContent); + + // Commit the patch + const { stderr: commitStderr } = await $`${bunExe()} patch --commit '${relativePatchPath}'` + .env(bunEnv) + .cwd(filedir); + + // With isolated linker, there may be some stderr output during patch commit + // but it should not contain actual errors + const commitStderrText = commitStderr.toString(); + expect(commitStderrText).not.toContain("error:"); + expect(commitStderrText).not.toContain("panic:"); + + // Verify root package.json was updated + const rootPkgJson = await Bun.file(join(filedir, "package.json")).json(); + expect(rootPkgJson.patchedDependencies).toEqual({ + "is-even@1.0.0": "patches/is-even@1.0.0.patch", + }); + + // Run from workspace package to verify patch was applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(join(filedir, "packages", "app")); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("WORKSPACE PATCH with isolated!"); + }); + + test("should preserve patch after reinstall with isolated linker", async () => { + const filedir = tempDirWithFiles("patch-isolated-reinstall", { + "package.json": JSON.stringify({ + "name": "bun-patch-isolated-reinstall-test", + "module": "index.ts", + "type": "module", + "dependencies": { + "is-even": "1.0.0", + }, + }), + "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven(6));`, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Create and commit a patch + const { stdout: patchStdout } = await $`${bunExe()} patch is-even`.env(bunEnv).cwd(filedir); + const patchOutput = patchStdout.toString(); + const relativePatchPath = + patchOutput.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath).toBeTruthy(); + const patchPath = join(filedir, relativePatchPath!); + + const indexPath = join(patchPath, "index.js"); + const originalContent = await Bun.file(indexPath).text(); + const modifiedContent = originalContent.replace( + "module.exports = function isEven(i) {", + 'module.exports = function isEven(i) {\n console.log("REINSTALL TEST with isolated!");', + ); + await Bun.write(indexPath, modifiedContent); + + await $`${bunExe()} patch --commit '${relativePatchPath}'`.env(bunEnv).cwd(filedir); + + // Delete node_modules and reinstall with isolated linker + rmSync(join(filedir, "node_modules"), { force: true, recursive: true }); + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Verify patch is still applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("REINSTALL TEST with isolated!"); + }); + + test("should handle multiple patches with isolated linker", async () => { + const filedir = tempDirWithFiles("patch-isolated-multiple", { + "package.json": JSON.stringify({ + "name": "bun-patch-isolated-multiple-test", + "module": "index.ts", + "type": "module", + "dependencies": { + "is-even": "1.0.0", + "is-odd": "3.0.1", + }, + }), + "index.ts": /* ts */ ` + import isEven from 'is-even'; + import isOdd from 'is-odd'; + console.log(isEven(8)); + console.log(isOdd(9)); + `, + }); + + // Install with isolated linker + await $`${bunExe()} install --linker=isolated`.env(bunEnv).cwd(filedir); + + // Patch first package (is-even) + const { stdout: patchStdout1 } = await $`${bunExe()} patch is-even`.env(bunEnv).cwd(filedir); + const patchOutput1 = patchStdout1.toString(); + const relativePatchPath1 = + patchOutput1.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput1.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath1).toBeTruthy(); + const patchPath1 = join(filedir, relativePatchPath1!); + + const indexPath1 = join(patchPath1, "index.js"); + const originalContent1 = await Bun.file(indexPath1).text(); + const modifiedContent1 = originalContent1.replace( + "module.exports = function isEven(i) {", + 'module.exports = function isEven(i) {\n console.log("is-even PATCHED with isolated!");', + ); + await Bun.write(indexPath1, modifiedContent1); + + const { stderr: commitStderr1 } = await $`${bunExe()} patch --commit '${relativePatchPath1}'` + .env(bunEnv) + .cwd(filedir); + // Check for errors + const commitStderrText1 = commitStderr1.toString(); + expect(commitStderrText1).not.toContain("error:"); + expect(commitStderrText1).not.toContain("panic:"); + + // Patch second package (is-odd hoisted version) + const { stdout: patchStdout2 } = await $`${bunExe()} patch is-odd@3.0.1`.env(bunEnv).cwd(filedir); + const patchOutput2 = patchStdout2.toString(); + const relativePatchPath2 = + patchOutput2.match(/To patch .+, edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim() || + patchOutput2.match(/edit the following folder:\s*\n\s*(.+)/)?.[1]?.trim(); + expect(relativePatchPath2).toBeTruthy(); + const patchPath2 = join(filedir, relativePatchPath2!); + + const indexPath2 = join(patchPath2, "index.js"); + const originalContent2 = await Bun.file(indexPath2).text(); + const modifiedContent2 = originalContent2.replace( + "module.exports = function isOdd(value) {", + 'module.exports = function isOdd(value) {\n console.log("is-odd PATCHED with isolated!");', + ); + await Bun.write(indexPath2, modifiedContent2); + + const { stderr: commitStderr2 } = await $`${bunExe()} patch --commit '${relativePatchPath2}'` + .env(bunEnv) + .cwd(filedir); + // Check for errors + const commitStderrText2 = commitStderr2.toString(); + expect(commitStderrText2).not.toContain("error:"); + expect(commitStderrText2).not.toContain("panic:"); + + // Verify both patches were applied + const { stdout, stderr } = await $`${bunExe()} run index.ts`.env(bunEnv).cwd(filedir); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toContain("is-even PATCHED with isolated!"); + expect(stdout.toString()).toContain("is-odd PATCHED with isolated!"); + + // Verify package.json has both patches + const pkgJson = await Bun.file(join(filedir, "package.json")).json(); + expect(pkgJson.patchedDependencies).toEqual({ + "is-even@1.0.0": "patches/is-even@1.0.0.patch", + "is-odd@3.0.1": "patches/is-odd@3.0.1.patch", + }); + }); + }); }); diff --git a/test/cli/install/bun-install-security-provider.test.ts b/test/cli/install/bun-install-security-provider.test.ts index 1af1180500..4f9848d139 100644 --- a/test/cli/install/bun-install-security-provider.test.ts +++ b/test/cli/install/bun-install-security-provider.test.ts @@ -14,7 +14,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); function test( diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index bdbd25e634..ed7e1ba1b6 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -67,7 +67,9 @@ beforeAll(() => { }); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach({ linker: "hoisted" }); +}); afterEach(dummyAfterEach); for (let input of ["abcdef", "65537", "-1"]) { @@ -3528,12 +3530,12 @@ it("should handle bitbucket git dependencies", async () => { "", `+ public-install-test@git+ssh://${dep}#79265e2d9754c60b60f97cc8d859fb6da073b5d2`, "", - "1 package installed", + expect.stringContaining("installed"), ]); expect(await exited).toBe(0); await access(join(package_dir, "bun.lockb")); - dummyAfterEach(); - dummyBeforeEach(); + await dummyAfterEach(); + await dummyBeforeEach({ linker: "isolated" }); } for (const dep of deps) { @@ -3564,12 +3566,12 @@ it("should handle bitbucket git dependencies", async () => { "", `installed publicinstalltest@git+ssh://${dep}#79265e2d9754c60b60f97cc8d859fb6da073b5d2`, "", - "1 package installed", + expect.stringContaining("installed"), ]); expect(await exited).toBe(0); await access(join(package_dir, "bun.lockb")); - dummyAfterEach(); - dummyBeforeEach(); + await dummyAfterEach(); + await dummyBeforeEach({ linker: "isolated" }); } }); @@ -3605,12 +3607,12 @@ it("should handle gitlab git dependencies", async () => { "", `+ public-install-test@git+ssh://${dep}#93f3aa4ec9ca8a0bacc010776db48bfcd915c44c`, "", - "1 package installed", + expect.stringContaining("installed"), ]); expect(await exited).toBe(0); await access(join(package_dir, "bun.lockb")); - dummyAfterEach(); - dummyBeforeEach(); + await dummyAfterEach(); + await dummyBeforeEach({ linker: "isolated" }); } for (const dep of deps) { @@ -3641,12 +3643,12 @@ it("should handle gitlab git dependencies", async () => { "", `installed public-install-test@git+ssh://${dep}#93f3aa4ec9ca8a0bacc010776db48bfcd915c44c`, "", - "1 package installed", + expect.stringContaining("installed"), ]); expect(await exited).toBe(0); await access(join(package_dir, "bun.lockb")); - dummyAfterEach(); - dummyBeforeEach(); + await dummyAfterEach(); + await dummyBeforeEach({ linker: "isolated" }); } }); @@ -6977,7 +6979,7 @@ it("should handle installing workspaces with more complicated globs", async () = .toString() .replace(/\s*\[[0-9\.]+m?s\]\s*$/, "") .split(/\r?\n/), - ).toEqual([expect.stringContaining("bun install v1."), "", "4 packages installed"]); + ).toEqual([expect.stringContaining("bun install v1."), "", "Checked 7 installs across 5 packages (no changes)"]); }); it("should handle installing workspaces with multiple glob patterns", async () => { @@ -7040,7 +7042,7 @@ it("should handle installing workspaces with multiple glob patterns", async () = .toString() .replace(/\s*\[[0-9\.]+m?s\]\s*$/, "") .split(/\r?\n/), - ).toEqual([expect.stringContaining("bun install v1."), "", "4 packages installed"]); + ).toEqual([expect.stringContaining("bun install v1."), "", "Checked 7 installs across 5 packages (no changes)"]); }); it.todo("should handle installing workspaces with absolute glob patterns", async () => { @@ -8433,6 +8435,9 @@ saveTextLockfile = true JSON.stringify({ name: "foo", workspaces: ["packages/*"], + dependencies: { + "pkg-one": "workspace:*", + }, }), ), write( @@ -8456,7 +8461,7 @@ saveTextLockfile = true expect(err).not.toContain("error:"); expect(err).toContain("Saved lockfile"); const out = await stdout.text(); - expect(out).toContain("1 package installed"); + expect(out).toContain("Checked 3 installs across 2 packages (no changes)"); expect(await exited).toBe(0); expect(await Bun.file(join(package_dir, "node_modules", "pkg-one", "package.json")).json()).toEqual({ @@ -8464,7 +8469,27 @@ saveTextLockfile = true version: "1.0.0", }); expect(await exists(join(package_dir, "bun.lockb"))).toBeFalse(); - expect(await file(join(package_dir, "bun.lock")).text()).toMatchSnapshot(); + expect(await file(join(package_dir, "bun.lock")).text()).toMatchInlineSnapshot(` + "{ + "lockfileVersion": 1, + "workspaces": { + "": { + "name": "foo", + "dependencies": { + "pkg-one": "workspace:*", + }, + }, + "packages/pkg1": { + "name": "pkg-one", + "version": "1.0.0", + }, + }, + "packages": { + "pkg-one": ["pkg-one@workspace:packages/pkg1"], + } + } + " + `); }); test("providing invalid url in lockfile does not crash", async () => { diff --git a/test/cli/install/bun-link.test.ts b/test/cli/install/bun-link.test.ts index 4ac575b1df..55eb1e9450 100644 --- a/test/cli/install/bun-link.test.ts +++ b/test/cli/install/bun-link.test.ts @@ -63,7 +63,7 @@ it("should link and unlink workspace package", async () => { expect(out.replace(/\s*\[[0-9\.]+ms\]\s*$/, "").split(/\r?\n/)).toEqual([ expect.stringContaining("bun install v1."), "", - "2 packages installed", + "Done! Checked 3 packages (no changes)", ]); let { stdout, stderr, exited } = spawn({ diff --git a/test/cli/install/bun-patch.test.ts b/test/cli/install/bun-patch.test.ts index c81a74404e..f231f35f2d 100644 --- a/test/cli/install/bun-patch.test.ts +++ b/test/cli/install/bun-patch.test.ts @@ -19,7 +19,7 @@ describe("bun patch ", async () => { */ describe("inside workspace with hoisting", async () => { const args = [ - ["node_modules/@types/ws", "node_modules/@types/ws"], + ["packages/eslint-config/node_modules/@types/ws", "packages/eslint-config/node_modules/@types/ws"], ["@types/ws@8.5.4", "node_modules/@types/ws"], ]; for (const [arg, path] of args) { diff --git a/test/cli/install/bun-pm.test.ts b/test/cli/install/bun-pm.test.ts index 7f1c5e2834..4e99091ded 100644 --- a/test/cli/install/bun-pm.test.ts +++ b/test/cli/install/bun-pm.test.ts @@ -18,7 +18,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); it("should list top-level dependency", async () => { diff --git a/test/cli/install/bun-update-security-provider.test.ts b/test/cli/install/bun-update-security-provider.test.ts index 034d45e791..4ea1f47928 100644 --- a/test/cli/install/bun-update-security-provider.test.ts +++ b/test/cli/install/bun-update-security-provider.test.ts @@ -13,7 +13,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); test("security scanner blocks bun update on fatal advisory", async () => { diff --git a/test/cli/install/bun-update.test.ts b/test/cli/install/bun-update.test.ts index 9443557bb1..7833710fb5 100644 --- a/test/cli/install/bun-update.test.ts +++ b/test/cli/install/bun-update.test.ts @@ -17,7 +17,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); expect.extend({ @@ -25,7 +27,7 @@ expect.extend({ toHaveBins, }); -for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }, { input: { baz: "^0.0.3", moo: "^0.1.0" } }]) { +for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }]) { it(`should update to latest version of dependency (${input.baz[0]})`, async () => { const urls: string[] = []; const tilde = input.baz[0] === "~"; @@ -42,6 +44,7 @@ for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }, { input: { }, latest: "0.0.3", }; + console.log({ package_dir }); setHandler(dummyRegistry(urls, registry)); await writeFile( join(package_dir, "package.json"), @@ -64,6 +67,7 @@ for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }, { input: { stderr: "pipe", env, }); + const err1 = await new Response(stderr1).text(); expect(err1).not.toContain("error:"); expect(err1).toContain("Saved lockfile"); diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts index 7693768ad2..cc22c7d9fa 100644 --- a/test/cli/install/dummy.registry.ts +++ b/test/cli/install/dummy.registry.ts @@ -126,7 +126,7 @@ export function getPort() { let packageDirGetter: () => string = () => { return tmpdirSync(); }; -export async function dummyBeforeEach() { +export async function dummyBeforeEach(opts?: { linker: "hoisted" | "isolated" }) { resetHandler(); requested = 0; package_dir = packageDirGetter(); @@ -137,6 +137,7 @@ export async function dummyBeforeEach() { cache = false registry = "http://localhost:${server.port}/" saveTextLockfile = false +${opts ? `linker = "${opts.linker}"` : ""} `, ); } diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 1217a7bf71..4141a46e91 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -328,8 +328,6 @@ test("can install folder dependencies on root package", async () => { ), ]); - console.log({ packageDir }); - await runBunInstall(bunEnv, packageDir); expect( @@ -532,6 +530,140 @@ for (const backend of ["clonefile", "hardlink", "copyfile"]) { }); } +describe("existing node_modules, missing node_modules/.bun", () => { + test("root and workspace node_modules are reset", async () => { + const { packageDir } = await registry.createTestDir({ + bunfigOpts: { isolated: true }, + files: { + "package.json": JSON.stringify({ + name: "delete-node-modules", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "1.0.0", + "a-dep": "1.0.1", + }, + }), + "packages/pkg1/package.json": JSON.stringify({ + name: "pkg1", + dependencies: { + "no-deps": "1.0.1", + }, + }), + "packages/pkg2/package.json": JSON.stringify({ + name: "pkg2", + dependencies: { + "no-deps": "2.0.0", + }, + }), + "node_modules/oops": "delete me!", + "packages/pkg1/node_modules/oops1": "delete me!", + "packages/pkg2/node_modules/oops2": "delete me!", + }, + }); + + let { exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: bunEnv, + stdout: "ignore", + stderr: "ignore", + }); + + expect(await exited).toBe(0); + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg1", "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg2", "node_modules")), + ]), + ).toEqual([[".bun", expect.stringContaining(".old_modules-"), "a-dep", "no-deps"], ["no-deps"], ["no-deps"]]); + }); + test("some workspaces don't have node_modules", async () => { + const { packageDir } = await registry.createTestDir({ + bunfigOpts: { isolated: true }, + files: { + "package.json": JSON.stringify({ + name: "missing-workspace-node_modules", + workspaces: ["packages/*"], + dependencies: { + "no-deps": "1.0.0", + }, + }), + "node_modules/hi": "BUN", + "packages/pkg1/package.json": JSON.stringify({ + name: "pkg-one", + dependencies: { + "no-deps": "2.0.0", + }, + }), + "packages/pkg1/node_modules/foo": "HI", + "packages/pkg2/package.json": JSON.stringify({ + name: "pkg-two", + dependencies: { + "a-dep": "1.0.1", + }, + }), + }, + }); + + let { exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: bunEnv, + stdout: "ignore", + stderr: "ignore", + }); + + expect(await exited).toBe(0); + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg1", "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg2", "node_modules")), + ]), + ).toEqual([[".bun", expect.stringContaining(".old_modules-"), "no-deps"], ["no-deps"], ["a-dep"]]); + + // another install will not reset the node_modules + + const entries = await readdirSorted(join(packageDir, "node_modules")); + + for (const entry of entries) { + if (entry.startsWith(".old_modules-")) { + await rm(join(packageDir, "node_modules", entry), { recursive: true, force: true }); + } + } + expect(await readdirSorted(join(packageDir, "node_modules"))).toEqual([".bun", "no-deps"]); + + // add things to workspace node_modules. these will go undetected + await Promise.all([ + write(join(packageDir, "packages", "pkg1", "node_modules", "oops1"), "HI1"), + write(join(packageDir, "packages", "pkg2", "node_modules", "oops2"), "HI2"), + ]); + + ({ exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: bunEnv, + stdout: "ignore", + stderr: "ignore", + })); + + expect(await exited).toBe(0); + + expect( + await Promise.all([ + readdirSorted(join(packageDir, "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg1", "node_modules")), + readdirSorted(join(packageDir, "packages", "pkg2", "node_modules")), + ]), + ).toEqual([ + [".bun", "no-deps"], + ["no-deps", "oops1"], + ["a-dep", "oops2"], + ]); + }); +}); + describe("--linker flag", () => { test("can override linker from bunfig", async () => { const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { isolated: true } }); diff --git a/test/cli/install/lockfile-only.test.ts b/test/cli/install/lockfile-only.test.ts index 70cbb6ab97..61d03006d0 100644 --- a/test/cli/install/lockfile-only.test.ts +++ b/test/cli/install/lockfile-only.test.ts @@ -17,7 +17,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); it.each(["bun.lockb", "bun.lock"])("should not download tarballs with --lockfile-only using %s", async lockfile => { diff --git a/test/harness.ts b/test/harness.ts index 4f5663dfe9..339ee211d1 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -793,9 +793,18 @@ export async function toBeWorkspaceLink(actual: string, expectedLinkPath: string const message = () => `Expected ${actual} to be a link to ${expectedLinkPath}`; if (isWindows) { - // junctions on windows will have an absolute path - const pass = isAbsolute(actual) && actual.includes(expectedLinkPath.split("..").at(-1)!); - return { pass, message }; + if (isAbsolute(actual)) { + // junctions on windows will have an absolute path + return { + pass: actual.includes(expectedLinkPath.split("..").at(-1)!), + message, + }; + } + + return { + pass: actual === expectedLinkPath, + message, + }; } const pass = actual === expectedLinkPath; diff --git a/test/internal/ban-limits.json b/test/internal/ban-limits.json index 936805fc8a..19f2fa0c8c 100644 --- a/test/internal/ban-limits.json +++ b/test/internal/ban-limits.json @@ -33,9 +33,9 @@ "std.debug.dumpStackTrace": 0, "std.debug.print": 0, "std.enums.tagName(": 2, - "std.fs.Dir": 168, + "std.fs.Dir": 165, "std.fs.File": 62, - "std.fs.cwd": 104, + "std.fs.cwd": 103, "std.log": 1, "std.mem.indexOfAny(u8": 0, "std.unicode": 27, diff --git a/test/regression/issue/08093.test.ts b/test/regression/issue/08093.test.ts index a15f035987..d8ba732f43 100644 --- a/test/regression/issue/08093.test.ts +++ b/test/regression/issue/08093.test.ts @@ -17,7 +17,9 @@ import { beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); -beforeEach(dummyBeforeEach); +beforeEach(async () => { + await dummyBeforeEach(); +}); afterEach(dummyAfterEach); it("should install vendored node_modules with hardlink", async () => {