Make --linker=isolated the default for bun install

src/install/PackageManager/install_with_manager.zig

@@ -775,10 +775,22 @@ pub fn installWithManager(
             break :install_summary .{};
         }
 
-        switch (manager.options.node_linker) {
+        const linker: Options.NodeLinker = switch (manager.options.node_linker) {
+            .hoisted => .hoisted,
+            .isolated => .isolated,
+            .auto => linker: {
+                if (manager.lockfile.workspace_paths.count() > 0) {
+                    break :linker .isolated;
+                }
+                break :linker .hoisted;
+            },
+        };
+
+        switch (linker) {
+            // handled above
+            .auto => unreachable,
+
             .hoisted,
-            // TODO
-            .auto,
             => break :install_summary try installHoistedPackages(
                 manager,
                 ctx,
@@ -788,7 +800,8 @@ pub fn installWithManager(
                 null,
             ),
 
-            .isolated => break :install_summary installIsolatedPackages(
+            .isolated,
+            => break :install_summary installIsolatedPackages(
                 manager,
                 ctx,
                 install_root_dependencies,

src/install/PackageManager/patchPackage.zig

@@ -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(
             "<r><red>error<r>: error overwriting folder in node_modules: {s}\n<r>",
             .{@errorName(e)},
@@ -729,170 +729,65 @@ 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();
-
-    const IGNORED_PATHS: []const bun.OSPathSlice = &[_][]const bun.OSPathChar{
-        bun.OSPathLiteral("node_modules"),
-        bun.OSPathLiteral(".git"),
-        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 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("<r><red>error<r>: 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("<r><red>error<r>: copying file {}", .{e});
-                        Global.crash();
+    {
+        var dest_subpath: bun.RelPath(.{ .sep = .auto }) = .from(node_modules_folder_path);
+        defer dest_subpath.deinit();
+
+        var link_buf: bun.PathBuffer = undefined;
+        switch (bun.sys.readlink(dest_subpath.sliceZ(), &link_buf)) {
+            .err => {},
+            .result => {
+                bun.sys.unlinkat(FD.cwd(), dest_subpath.sliceZ()).unwrap() catch {
+                    if (comptime Environment.isWindows) {
+                        bun.sys.rmdirat(FD.cwd(), dest_subpath.sliceZ()).unwrap() catch {};
                     }
+                };
+                bun.sys.mkdir(dest_subpath.sliceZ(), 0o755).unwrap() catch {};
+            },
+        }
+    }
 
-                    var outfile = createFile(destination_dir_, entrypath, .{}) catch |e| {
-                        Output.prettyError("<r><red>error<r>: 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, &copy_file_state).unwrap() catch |err| {
-                        Output.prettyError("<r><red>{s}<r>: 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();
+    var dest_subpath: bun.RelPath(.{ .sep = .auto, .unit = .os }) = .from(node_modules_folder_path);
+    defer dest_subpath.deinit();
 
-                    @memcpy(pathbuf[0..entry.path.len], entry.path);
-                    pathbuf[entry.path.len] = 0;
+    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);
 
-                    if (bun.sys.unlinkat(
-                        .fromStdDir(destination_dir_),
-                        pathbuf[0..entry.path.len :0],
-                    ).asErr()) |e| {
-                        Output.prettyError("<r><red>error<r>: copying file {}", .{e.withPath(entry.path)});
-                        Global.crash();
-                    }
+            var src_path: bun.AbsPath(.{ .sep = .auto, .unit = .os }) = .from(abs_path);
+            src_path.append(cache_dir_subpath);
 
-                    var outfile = try createFile(destination_dir_, entry.path, .{});
-                    defer outfile.close();
+            break :src_path src_path;
+        }
 
-                    const stat = in_file.stat().unwrap() catch continue;
-                    _ = bun.c.fchmod(outfile.handle, @intCast(stat.mode));
+        // unused if not windows
+        break :src_path .init();
+    };
+    defer src_path.deinit();
 
-                    bun.copyFileWithState(in_file, .fromStdFile(outfile), &copy_file_state).unwrap() catch |err| {
-                        Output.prettyError("<r><red>{s}<r>: copying file {}", .{ @errorName(err), bun.fmt.fmtOSPath(entry.path, .{}) });
-                        Global.crash();
-                    };
-                }
-            }
+    var cached_package_folder = try cache_dir.openDir(cache_dir_subpath, .{ .iterate = true });
+    defer cached_package_folder.close();
 
-            return real_file_count;
-        }
+    const ignore_directories: []const bun.OSPathSlice = &.{
+        comptime bun.OSPathLiteral("node_modules"),
+        comptime bun.OSPathLiteral(".git"),
+        comptime bun.OSPathLiteral("CMakeFiles"),
     };
 
-    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("<r><red>error<r>: 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("<r><red>error<r>: 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("<r><red>error<r>: 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,
-            {},
-            {},
-            {},
-            {},
-            {},
-        );
-    }
+    var copier: bun.install.FileCopier = try .init(
+        .fromStdDir(cached_package_folder),
+        src_path,
+        dest_subpath,
+        ignore_directories,
+    );
+    defer copier.deinit();
+
+    try copier.copy().unwrap();
 }
 
 fn nodeModulesFolderForDependencyIDs(iterator: *Lockfile.Tree.Iterator(.node_modules), ids: []const IdPair) !?Lockfile.Tree.Iterator(.node_modules).Next {
@@ -1086,7 +981,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");

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;

src/install/isolated_install.zig

@@ -841,6 +841,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;
                     }

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)) {