From bc6cddacbee414f35f8dc36cf487c4dec06308e9 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Mon, 13 Oct 2025 17:23:44 +0000 Subject: [PATCH] Address CodeRabbit nitpicks: improve logging clarity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Show absolute temp path (temp_target_full) in verbose logs instead of tmpname - Set moved=true on EEXIST path to cancel errdefer cleanup - Log absolute destination path in error messages for better diagnostics Co-Authored-By: CodeRabbit AI 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/install/repository.zig | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/install/repository.zig b/src/install/repository.zig index 97496a4af5..3cae7145c2 100644 --- a/src/install/repository.zig +++ b/src/install/repository.zig @@ -524,7 +524,7 @@ pub const Repository = extern struct { errdefer if (!moved) temp_dir.deleteTree(tmpname) catch {}; if (PackageManager.verbose_install) { - Output.prettyErrorln("[{s}] Cloning git repository (bare) to {s}", .{ name, tmpname }); + Output.prettyErrorln("[{s}] Cloning git repository (bare) to {s}", .{ name, temp_target_full }); Output.flush(); } @@ -561,18 +561,21 @@ pub const Repository = extern struct { if (cache_dir.openDirZ(folder_name, .{})) |dir2| { // Best-effort cleanup of our temp path temp_dir.deleteTree(tmpname) catch {}; + moved = true; // cancel errdefer; already cleaned up if (PackageManager.verbose_install) { Output.prettyErrorln("[{s}] Cache dir already exists (another process completed first)", .{name}); Output.flush(); } break :clone dir2; } else |_| {} + const cache_target = try bun.getFdPath(.fromStdDir(cache_dir), &final_path_buf); + const dest_full = Path.joinAbsString(cache_target, &.{folder_name}, .auto); log.addErrorFmt( null, logger.Loc.Empty, allocator, "moving \"{s}\" to cache dir failed: {}\n From: {s}\n To: {s}", - .{ name, err, temp_target_full, folder_name }, + .{ name, err, temp_target_full, dest_full }, ) catch unreachable; return error.InstallFailed; } @@ -650,7 +653,7 @@ pub const Repository = extern struct { errdefer if (!moved) temp_dir.deleteTree(tmpname) catch {}; if (PackageManager.verbose_install) { - Output.prettyErrorln("[{s}] Cloning git repository to {s}", .{ name, tmpname }); + Output.prettyErrorln("[{s}] Cloning git repository to {s}", .{ name, temp_target_full }); Output.flush(); } @@ -713,18 +716,21 @@ pub const Repository = extern struct { if (bun.openDir(cache_dir, folder_name)) |_| { // Best-effort cleanup of our temp path temp_dir.deleteTree(tmpname) catch {}; + moved = true; // cancel errdefer; already cleaned up if (PackageManager.verbose_install) { Output.prettyErrorln("[{s}] Cache dir already exists (another process completed first)", .{name}); Output.flush(); } break :brk try bun.openDir(cache_dir, folder_name); } else |_| {} + const cache_target = try bun.getFdPath(.fromStdDir(cache_dir), &final_path_buf); + const dest_full = Path.joinAbsString(cache_target, &.{folder_name}, .auto); log.addErrorFmt( null, logger.Loc.Empty, allocator, "moving \"{s}\" to cache dir failed: {}\n From: {s}\n To: {s}", - .{ name, err, temp_target_full, folder_name }, + .{ name, err, temp_target_full, dest_full }, ) catch unreachable; return error.InstallFailed; }