Skip to content

Commit 7a3934d

Browse files
authored
fix: harden bru upgrade against real-world cask edge cases (#113)
Five independent upgrade failures, all fixed: - Retry transient HTTP failures (3 tries, backoff) — recovers from mid-stream WriteFailed and HttpConnectionClosing aborts. - Upgrade GUI-only .app casks — the upgrade path skipped anything without CLI binaries, silently no-op'ing discord, obsidian, etc. - Sniff archive magic bytes when a cask URL has no extension (e.g. visual-studio-code's /darwin-arm64/stable). - Fall back to system tar when bottles use hard links — Zig's native iterator aborts on TarUnsupportedHeader (e.g. mingw-w64). - Stage binaries inside .app bundles: stage apps first, probe /Applications as a fallback source root, normalize absolute target paths to basename. Fixes docker-desktop's FileNotFound cluster.
1 parent dba1eae commit 7a3934d

4 files changed

Lines changed: 386 additions & 72 deletions

File tree

src/bottle.zig

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,13 @@ pub const Bottle = struct {
202202
};
203203
}
204204

205-
/// Extract a .tar.gz bottle into the cellar using parallel file writes.
205+
/// Extract a .tar.gz bottle into the cellar.
206+
///
207+
/// Fast path: parallel native extractor. Falls back to system `tar xf`
208+
/// for archives whose typeflags Zig's std.tar iterator can't handle —
209+
/// notably hard links ('h'), which mingw-w64 uses for GCC's wrapper
210+
/// binaries. The iterator returns error.TarUnsupportedHeader on those.
211+
///
206212
/// Returns the keg path (e.g., "/opt/homebrew/Cellar/bat/0.26.1").
207213
/// Caller owns the returned string.
208214
pub fn pour(self: Bottle, archive_path: []const u8, name: []const u8, version: []const u8) ![]const u8 {
@@ -212,6 +218,29 @@ pub const Bottle = struct {
212218
else => return err,
213219
};
214220

221+
const keg_path = try std.fmt.allocPrint(self.allocator, "{s}/{s}/{s}", .{
222+
self.cellar,
223+
name,
224+
version,
225+
});
226+
errdefer self.allocator.free(keg_path);
227+
228+
self.pourNative(archive_path) catch |err| switch (err) {
229+
error.TarUnsupportedHeader => {
230+
// Wipe the partial keg so we don't merge two half-extractions,
231+
// then let GNU/BSD tar take over.
232+
fs.deleteTreeAbsolute(keg_path) catch {};
233+
try self.pourSystemTar(archive_path);
234+
},
235+
else => return err,
236+
};
237+
238+
return keg_path;
239+
}
240+
241+
/// Native parallel extractor — fast path. Iterates the tar entries with
242+
/// std.tar.Iterator and dispatches file writes to worker threads.
243+
fn pourNative(self: Bottle, archive_path: []const u8) !void {
215244
// Open the archive file.
216245
const archive_file = try fs.openFileAbsolute(archive_path, .{});
217246
defer archive_file.close();
@@ -302,13 +331,21 @@ pub const Bottle = struct {
302331

303332
// Drain any remaining tasks.
304333
try spawnAndDrain(tasks.items, &pool);
334+
}
305335

306-
// Construct and return the keg path.
307-
return std.fmt.allocPrint(self.allocator, "{s}/{s}/{s}", .{
308-
self.cellar,
309-
name,
310-
version,
336+
/// Slow path: shell out to system tar. Handles every typeflag the native
337+
/// extractor doesn't (hard links, sparse files, GNU extensions).
338+
fn pourSystemTar(self: Bottle, archive_path: []const u8) !void {
339+
const result = try std.process.Child.run(.{
340+
.allocator = self.allocator,
341+
.argv = &.{ "tar", "xf", archive_path, "-C", self.cellar },
311342
});
343+
defer self.allocator.free(result.stdout);
344+
defer self.allocator.free(result.stderr);
345+
switch (result.term) {
346+
.Exited => |code| if (code != 0) return error.TarFailed,
347+
else => return error.TarFailed,
348+
}
312349
}
313350

314351
/// Extract a bottle with a two-tier extracted-keg cache.
@@ -873,6 +910,68 @@ test "pour extracts tar.gz into cellar" {
873910
try std.testing.expectEqualStrings("#!/bin/sh\necho bat\n", extracted);
874911
}
875912

913+
test "pour falls back to system tar on hardlink typeflag" {
914+
const allocator = std.testing.allocator;
915+
916+
var tmp = std.testing.tmpDir(.{});
917+
defer tmp.cleanup();
918+
919+
// Build a bottle layout with a hardlinked file. The native std.tar
920+
// iterator does not understand typeflag 'h' and returns
921+
// error.TarUnsupportedHeader — the fallback must take over.
922+
try tmp.dir.makePath("hltest/1.0.0/bin");
923+
try tmp.dir.writeFile(.{
924+
.sub_path = "hltest/1.0.0/bin/original",
925+
.data = "I am the source\n",
926+
});
927+
928+
var path_buf: [fs.max_path_bytes]u8 = undefined;
929+
const tmp_path = try tmp.dir.realpath(".", &path_buf);
930+
931+
// Create a hard link with system `ln` (tmpfs may not expose linkAt).
932+
const link_result = try std.process.Child.run(.{
933+
.allocator = allocator,
934+
.argv = &.{ "ln", "hltest/1.0.0/bin/original", "hltest/1.0.0/bin/linked" },
935+
.cwd_dir = tmp.dir,
936+
});
937+
allocator.free(link_result.stdout);
938+
allocator.free(link_result.stderr);
939+
940+
// Tar it up — `tar czf` emits typeflag 'h' for the hardlink entry.
941+
const archive_name = "hltest-1.0.0.tar.gz";
942+
const tar_result = try std.process.Child.run(.{
943+
.allocator = allocator,
944+
.argv = &.{ "tar", "czf", archive_name, "hltest" },
945+
.cwd_dir = tmp.dir,
946+
});
947+
allocator.free(tar_result.stdout);
948+
allocator.free(tar_result.stderr);
949+
950+
try tmp.dir.makeDir("cellar");
951+
var cellar_buf: [fs.max_path_bytes]u8 = undefined;
952+
const cellar_path = try tmp.dir.realpath("cellar", &cellar_buf);
953+
954+
const archive_path = try std.fmt.allocPrint(allocator, "{s}/{s}", .{ tmp_path, archive_name });
955+
defer allocator.free(archive_path);
956+
957+
const bottle = Bottle{
958+
.allocator = allocator,
959+
.cellar = cellar_path,
960+
.prefix = "/opt/homebrew",
961+
};
962+
963+
const keg_path = try bottle.pour(archive_path, "hltest", "1.0.0");
964+
defer allocator.free(keg_path);
965+
966+
// Both files should exist and have identical content after fallback.
967+
const original = try tmp.dir.readFileAlloc(allocator, "cellar/hltest/1.0.0/bin/original", 1024);
968+
defer allocator.free(original);
969+
const linked = try tmp.dir.readFileAlloc(allocator, "cellar/hltest/1.0.0/bin/linked", 1024);
970+
defer allocator.free(linked);
971+
try std.testing.expectEqualStrings("I am the source\n", original);
972+
try std.testing.expectEqualStrings("I am the source\n", linked);
973+
}
974+
876975
test "writeWorker writes files from tasks" {
877976
var tmp = std.testing.tmpDir(.{});
878977
defer tmp.cleanup();

0 commit comments

Comments
 (0)