diff --git a/src/install/install.zig b/src/install/install.zig index ecb394445b5e1f..9119518f303799 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -6369,13 +6369,14 @@ pub const PackageManager = struct { installer.node_modules.path = path; installer.current_tree_id = ctx.tree_id; const pkg_id = ptask.callback.apply.pkg_id; + const resolution = &manager.lockfile.packages.items(.resolution)[pkg_id]; installer.installPackageWithNameAndResolution( ctx.dependency_id, pkg_id, log_level, ptask.callback.apply.pkgname, - ptask.callback.apply.resolution, + resolution, false, false, ); diff --git a/src/install/patch_install.zig b/src/install/patch_install.zig index a09fce928fa8cb..f3ca73dc225514 100644 --- a/src/install/patch_install.zig +++ b/src/install/patch_install.zig @@ -79,7 +79,7 @@ pub const PatchTask = struct { pkg_id: PackageID, patch_hash: u64, name_and_version_hash: u64, - resolution: *const Resolution, + patchfilepath: []const u8, pkgname: String, @@ -262,14 +262,13 @@ pub const PatchTask = struct { debug("apply patch task", .{}); bun.assert(this.callback == .apply); - const strbuf: []const u8 = this.manager.lockfile.buffers.string_bytes.items; - const patch: *const ApplyPatch = &this.callback.apply; const dir = this.project_dir; const patchfile_path = patch.patchfilepath; + var absolute_patchfile_path_buf: bun.PathBuffer = undefined; // 1. Parse the patch file - const absolute_patchfile_path = bun.path.joinZ(&[_][]const u8{ + const absolute_patchfile_path = bun.path.joinZBuf(&absolute_patchfile_path_buf, &[_][]const u8{ dir, patchfile_path, }, .auto); @@ -310,14 +309,18 @@ pub const PatchTask = struct { const pkg_name = this.callback.apply.pkgname; - var resolution_buf: [512]u8 = undefined; - const resolution_label = std.fmt.bufPrint(&resolution_buf, "{}", .{this.callback.apply.resolution.fmt(strbuf, .posix)}) catch unreachable; - const dummy_node_modules: PackageManager.NodeModulesFolder = .{ .path = std.ArrayList(u8).init(this.manager.allocator), .tree_id = 0, }; + const resolution_label, const resolution_tag = brk: { + // TODO: fix this threadsafety issue. + const resolution = &this.manager.lockfile.packages.items(.resolution)[patch.pkg_id]; + break :brk .{ std.fmt.allocPrint(bun.default_allocator, "{}", .{resolution.fmt(this.manager.lockfile.buffers.string_bytes.items, .posix)}) catch bun.outOfMemory(), resolution.tag }; + }; + defer this.manager.allocator.free(resolution_label); + // 3. copy the unpatched files into temp dir var pkg_install = PreparePatchPackageInstall{ .allocator = bun.default_allocator, @@ -333,7 +336,7 @@ pub const PatchTask = struct { .lockfile = this.manager.lockfile, }; - switch (pkg_install.installImpl(true, system_tmpdir, .copyfile, this.callback.apply.resolution.tag)) { + switch (pkg_install.installImpl(true, system_tmpdir, .copyfile, resolution_tag)) { .success => {}, .failure => |reason| { return try log.addErrorFmtOpts( @@ -345,50 +348,62 @@ pub const PatchTask = struct { }, } - var patch_pkg_dir = system_tmpdir.openDir(tempdir_name, .{}) catch |e| return try log.addErrorFmtOpts( - this.manager.allocator, - "failed trying to open temporary dir to apply patch to package: {s}", - .{@errorName(e)}, - .{}, - ); - defer patch_pkg_dir.close(); - - // 4. apply patch - if (patchfile.apply(this.manager.allocator, bun.toFD(patch_pkg_dir.fd))) |e| { - return try log.addErrorFmtOpts( - this.manager.allocator, - "failed applying patch file: {}", - .{e}, - .{}, - ); - } + { + const patch_pkg_dir = switch (bun.sys.openat( + bun.toFD(system_tmpdir.fd), + tempdir_name, + bun.O.RDONLY | bun.O.DIRECTORY, + 0, + )) { + .result => |fd| fd, + .err => |e| return try log.addSysError( + this.manager.allocator, + e, + "failed trying to open temporary dir to apply patch to package: {s}", + .{resolution_label}, + ), + }; + defer _ = bun.sys.close(patch_pkg_dir); - // 5. Add bun tag - const bun_tag_prefix = bun_hash_tag; - var buntagbuf: BuntagHashBuf = undefined; - @memcpy(buntagbuf[0..bun_tag_prefix.len], bun_tag_prefix); - const hashlen = (std.fmt.bufPrint(buntagbuf[bun_tag_prefix.len..], "{x}", .{this.callback.apply.patch_hash}) catch unreachable).len; - buntagbuf[bun_tag_prefix.len + hashlen] = 0; - const buntagfd = switch (bun.sys.openat( - bun.toFD(patch_pkg_dir.fd), - buntagbuf[0 .. bun_tag_prefix.len + hashlen :0], - bun.O.RDWR | bun.O.CREAT, - 0o666, - )) { - .result => |fd| fd, - .err => |e| { + // 4. apply patch + if (patchfile.apply(this.manager.allocator, patch_pkg_dir)) |e| { return try log.addErrorFmtOpts( this.manager.allocator, - "failed adding bun tag: {}", - .{e.withPath(buntagbuf[0 .. bun_tag_prefix.len + hashlen :0])}, + "failed applying patch file: {}", + .{e.withoutPath()}, .{}, ); - }, - }; - _ = bun.sys.close(buntagfd); + } + + // 5. Add bun tag + const bun_tag_prefix = bun_hash_tag; + var buntagbuf: BuntagHashBuf = undefined; + @memcpy(buntagbuf[0..bun_tag_prefix.len], bun_tag_prefix); + const hashlen = (std.fmt.bufPrint(buntagbuf[bun_tag_prefix.len..], "{x}", .{this.callback.apply.patch_hash}) catch unreachable).len; + buntagbuf[bun_tag_prefix.len + hashlen] = 0; + const buntagfd = switch (bun.sys.openat( + patch_pkg_dir, + buntagbuf[0 .. bun_tag_prefix.len + hashlen :0], + bun.O.RDWR | bun.O.CREAT, + 0o666, + )) { + .result => |fd| fd, + .err => |e| { + return try log.addErrorFmtOpts( + this.manager.allocator, + "failed adding bun tag: {}", + .{e.withPath(buntagbuf[0 .. bun_tag_prefix.len + hashlen :0])}, + .{}, + ); + }, + }; + _ = bun.sys.close(buntagfd); + } // 6. rename to cache dir - const path_in_tmpdir = bun.path.joinZ( + var path_in_tmpdir_buf: bun.PathBuffer = undefined; + const path_in_tmpdir = bun.path.joinZBuf( + &path_in_tmpdir_buf, &[_][]const u8{ tempdir_name, // tempdir_name, @@ -417,11 +432,16 @@ pub const PatchTask = struct { const dir = this.project_dir; const patchfile_path = this.callback.calc_hash.patchfile_path; + var absolute_patchfile_path_buf: bun.PathBuffer = undefined; // parse the patch file - const absolute_patchfile_path = bun.path.joinZ(&[_][]const u8{ - dir, - patchfile_path, - }, .auto); + const absolute_patchfile_path = bun.path.joinZBuf( + &absolute_patchfile_path_buf, + &[_][]const u8{ + dir, + patchfile_path, + }, + .auto, + ); const stat: bun.Stat = switch (bun.sys.stat(absolute_patchfile_path)) { .err => |e| { @@ -543,7 +563,8 @@ pub const PatchTask = struct { name_and_version_hash: u64, ) *PatchTask { const pkg_name = pkg_manager.lockfile.packages.items(.name)[pkg_id]; - const resolution: *const Resolution = &pkg_manager.lockfile.packages.items(.resolution)[pkg_id]; + + const resolution = &pkg_manager.lockfile.packages.items(.resolution)[pkg_id]; var folder_path_buf: bun.PathBuffer = undefined; const stuff = pkg_manager.computeCacheDirAndSubpath( @@ -560,7 +581,6 @@ pub const PatchTask = struct { .callback = .{ .apply = .{ .pkg_id = pkg_id, - .resolution = resolution, .patch_hash = patch_hash, .name_and_version_hash = name_and_version_hash, .cache_dir = stuff.cache_dir, diff --git a/src/patch.zig b/src/patch.zig index 771643d8cefd3a..422d0d8a9436a0 100644 --- a/src/patch.zig +++ b/src/patch.zig @@ -35,25 +35,6 @@ pub const PatchFilePart = union(enum) { pub const PatchFile = struct { parts: List(PatchFilePart) = .{}, - const ScratchBuffer = struct { - buf: std.ArrayList(u8), - - fn deinit(scratch: *@This()) void { - scratch.buf.deinit(); - } - - fn clear(scratch: *@This()) void { - scratch.buf.clearRetainingCapacity(); - } - - fn dupeZ(scratch: *@This(), path: []const u8) [:0]const u8 { - const start = scratch.buf.items.len; - scratch.buf.appendSlice(path) catch unreachable; - scratch.buf.append(0) catch unreachable; - return scratch.buf.items[start .. start + path.len :0]; - } - }; - pub fn deinit(this: *PatchFile, allocator: Allocator) void { for (this.parts.items) |*part| part.deinit(allocator); this.parts.deinit(allocator); @@ -79,6 +60,7 @@ pub const PatchFile = struct { var state: ApplyState = .{}; var sfb = std.heap.stackFallback(1024, allocator); var arena = bun.ArenaAllocator.init(sfb.get()); + defer arena.deinit(); for (this.parts.items) |*part| { defer _ = arena.reset(.retain_capacity); @@ -87,7 +69,7 @@ pub const PatchFile = struct { const pathz = arena.allocator().dupeZ(u8, part.file_deletion.path) catch bun.outOfMemory(); if (bun.sys.unlinkat(patch_dir, pathz).asErr()) |e| { - return e.withPath(pathz); + return e.withoutPath(); } }, .file_rename => { @@ -97,7 +79,7 @@ pub const PatchFile = struct { if (std.fs.path.dirname(to_path)) |todir| { const abs_patch_dir = switch (state.patchDirAbsPath(patch_dir)) { .result => |p| p, - .err => |e| return e, + .err => |e| return e.withoutPath(), }; const path_to_make = bun.path.joinZ(&[_][]const u8{ abs_patch_dir, @@ -108,11 +90,11 @@ pub const PatchFile = struct { .path = .{ .string = bun.PathString.init(path_to_make) }, .recursive = true, .mode = 0o755, - }).asErr()) |e| return e; + }).asErr()) |e| return e.withoutPath(); } if (bun.sys.renameat(patch_dir, from_path, patch_dir, to_path).asErr()) |e| { - return e; + return e.withoutPath(); } }, .file_creation => { @@ -126,7 +108,7 @@ pub const PatchFile = struct { .path = .{ .string = bun.PathString.init(filedir) }, .recursive = true, .mode = @intCast(@intFromEnum(mode)), - }).asErr()) |e| return e; + }).asErr()) |e| return e.withoutPath(); } const newfile_fd = switch (bun.sys.openat( @@ -136,7 +118,7 @@ pub const PatchFile = struct { mode.toBunMode(), )) { .result => |fd| fd, - .err => |e| return e.withPath(filepath.slice()), + .err => |e| return e.withoutPath(), }; defer _ = bun.sys.close(newfile_fd); @@ -180,14 +162,14 @@ pub const PatchFile = struct { while (written < file_contents.len) { switch (bun.sys.write(newfile_fd, file_contents[written..])) { .result => |bytes| written += bytes, - .err => |e| return e.withPath(filepath.slice()), + .err => |e| return e.withoutPath(), } } }, .file_patch => { // TODO: should we compute the hash of the original file and check it against the on in the patch? if (applyPatch(part.file_patch, &arena, patch_dir, &state).asErr()) |e| { - return e; + return e.withoutPath(); } }, .file_mode_change => { @@ -195,22 +177,24 @@ pub const PatchFile = struct { const filepath = arena.allocator().dupeZ(u8, part.file_mode_change.path) catch bun.outOfMemory(); if (comptime bun.Environment.isPosix) { if (bun.sys.fchmodat(patch_dir, filepath, newmode.toBunMode(), 0).asErr()) |e| { - return e; + return e.withoutPath(); } } if (comptime bun.Environment.isWindows) { const absfilepath = switch (state.patchDirAbsPath(patch_dir)) { .result => |p| p, - .err => |e| return e, + .err => |e| return e.withoutPath(), }; - const fd = switch (bun.sys.open(bun.path.joinZ(&[_][]const u8{ absfilepath, filepath }, .auto), bun.O.RDWR, 0)) { - .err => |e| return e, + var buf: bun.PathBuffer = undefined; + const joined_absfilepath = bun.path.joinZBuf(&buf, &[_][]const u8{ absfilepath, filepath }, .auto); + const fd = switch (bun.sys.open(joined_absfilepath, bun.O.RDWR, 0)) { + .err => |e| return e.withoutPath(), .result => |f| f, }; defer _ = bun.sys.close(fd); if (bun.sys.fchmod(fd, newmode.toBunMode()).asErr()) |e| { - return e; + return e.withoutPath(); } } }, diff --git a/src/sys.zig b/src/sys.zig index a52db013ef953e..9a6e8636d6fb77 100644 --- a/src/sys.zig +++ b/src/sys.zig @@ -352,7 +352,19 @@ pub const Error = struct { } pub fn format(self: Error, comptime fmt: []const u8, opts: std.fmt.FormatOptions, writer: anytype) !void { - try self.toShellSystemError().format(fmt, opts, writer); + // We want to reuse the code from SystemError for formatting. + // But, we do not want to call String.createUTF8 on the path/dest strings + // because we're intending to pass them to writer.print() + // which will convert them back into UTF*. + var that = self.withoutPath().toShellSystemError(); + bun.debugAssert(that.path.tag != .WTFStringImpl); + bun.debugAssert(that.dest.tag != .WTFStringImpl); + that.path = bun.String.fromUTF8(self.path); + that.dest = bun.String.fromUTF8(self.dest); + bun.debugAssert(that.path.tag != .WTFStringImpl); + bun.debugAssert(that.dest.tag != .WTFStringImpl); + + return that.format(fmt, opts, writer); } pub inline fn getErrno(this: Error) E { @@ -421,6 +433,14 @@ pub const Error = struct { }; } + /// When the memory of the path/dest buffer is unsafe to use, call this function to clone the error without the path/dest. + pub fn withoutPath(this: *const Error) Error { + var copy = this.*; + copy.path = ""; + copy.dest = ""; + return copy; + } + pub fn name(this: *const Error) []const u8 { if (comptime Environment.isWindows) { const system_errno = brk: { diff --git a/test/cli/install/bun-install-patch.test.ts b/test/cli/install/bun-install-patch.test.ts index 1c47d0197dde20..213725b22b60ae 100644 --- a/test/cli/install/bun-install-patch.test.ts +++ b/test/cli/install/bun-install-patch.test.ts @@ -515,7 +515,7 @@ index 832d92223a9ec491364ee10dcbe3ad495446ab80..7e079a817825de4b8c3d01898490dc7e }, "index.ts": /* ts */ `import isEven from 'is-even'; console.log(isEven())`, }); - + console.log(filedir); await $`${bunExe()} i`.env(bunEnv).cwd(filedir); const pkgjsonWithPatch = {