From ba1fffed039df7eceaa0a085c9cf04a4549afcfe Mon Sep 17 00:00:00 2001 From: Vernon Stinebaker Date: Mon, 4 May 2026 22:40:33 +0800 Subject: [PATCH 1/2] fix(installer): skip re-download when release binary is already cached downloader.download() was called unconditionally on every install, even when the binary was already present in ~/.nullhub/bin/. This made every reinstall (e.g. updating provider config on an existing instance) pay the full network round-trip cost. Add the same existence check that stageLocalBinary already uses: open the target path; if it succeeds the binary is cached and the download is skipped entirely. --- src/installer/orchestrator.zig | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/installer/orchestrator.zig b/src/installer/orchestrator.zig index ac1b190..07a3790 100644 --- a/src/installer/orchestrator.zig +++ b/src/installer/orchestrator.zig @@ -161,12 +161,19 @@ pub fn install( resolved_version = allocator.dupe(u8, release.value.tag_name) catch return error.FetchFailed; const bin_path = p.binary(allocator, opts.component, resolved_version.?) catch return error.DownloadFailed; resolved_bin_path = bin_path; - downloader.download(allocator, asset.browser_download_url, bin_path) catch { - allocator.free(bin_path); - resolved_bin_path = null; - allocator.free(resolved_version.?); - resolved_version = null; - }; + // Skip the download if the binary is already cached on disk. + const already_cached = if (std_compat.fs.openFileAbsolute(bin_path, .{})) |f| blk: { + f.close(); + break :blk true; + } else |_| false; + if (!already_cached) { + downloader.download(allocator, asset.browser_download_url, bin_path) catch { + allocator.free(bin_path); + resolved_bin_path = null; + allocator.free(resolved_version.?); + resolved_version = null; + }; + } } } else |_| {} } From de2a01cb1d9d5897892cf91a375c1004d685d5fb Mon Sep 17 00:00:00 2001 From: Igor Somov Date: Tue, 5 May 2026 11:07:06 -0300 Subject: [PATCH 2/2] fix(installer): reuse cached release binaries --- src/api/updates.zig | 2 +- src/api/wizard.zig | 7 +---- src/installer/downloader.zig | 48 ++++++++++++++++++++++++++++++++++ src/installer/orchestrator.zig | 24 ++++++----------- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/src/api/updates.zig b/src/api/updates.zig index a7748b2..2de1d0d 100644 --- a/src/api/updates.zig +++ b/src/api/updates.zig @@ -174,7 +174,7 @@ pub fn handleApplyUpdateRuntime( const new_bin_path = paths.binary(allocator, component, latest_tag) catch return serverError(); defer allocator.free(new_bin_path); - downloader.download(allocator, asset.browser_download_url, new_bin_path) catch return .{ + downloader.downloadIfMissing(allocator, asset.browser_download_url, new_bin_path) catch return .{ .status = "502 Bad Gateway", .content_type = "application/json", .body = "{\"error\":\"failed to download latest binary\"}", diff --git a/src/api/wizard.zig b/src/api/wizard.zig index ceabc32..7f9c983 100644 --- a/src/api/wizard.zig +++ b/src/api/wizard.zig @@ -480,12 +480,7 @@ fn fetchLatestComponentBinary(allocator: std.mem.Allocator, component: []const u paths.ensureDirs() catch return null; const bin_path = paths.binary(allocator, component, release.value.tag_name) catch return null; - if (std_compat.fs.openFileAbsolute(bin_path, .{})) |f| { - f.close(); - return bin_path; - } else |_| {} - - downloader.download(allocator, asset.browser_download_url, bin_path) catch { + downloader.downloadIfMissing(allocator, asset.browser_download_url, bin_path) catch { allocator.free(bin_path); return null; }; diff --git a/src/installer/downloader.zig b/src/installer/downloader.zig index 81d992a..9186b73 100644 --- a/src/installer/downloader.zig +++ b/src/installer/downloader.zig @@ -32,6 +32,12 @@ pub fn computeSha256(allocator: std.mem.Allocator, file_path: []const u8) ![64]u // ─── Download ──────────────────────────────────────────────────────────────── +pub fn fileExists(file_path: []const u8) bool { + const file = std_compat.fs.openFileAbsolute(file_path, .{}) catch return false; + file.close(); + return true; +} + /// Download a file from `url` to `dest_path` using curl. /// /// Uses an atomic write pattern: downloads to `dest_path.tmp`, then renames @@ -76,6 +82,12 @@ pub fn download(allocator: std.mem.Allocator, url: []const u8, dest_path: []cons } } +/// Download a file only when `dest_path` is not already present. +pub fn downloadIfMissing(allocator: std.mem.Allocator, url: []const u8, dest_path: []const u8) !void { + if (fileExists(dest_path)) return; + try download(allocator, url, dest_path); +} + /// Download a file and verify its SHA256 checksum. /// /// If the checksum does not match `expected_sha256`, the downloaded file is @@ -176,6 +188,42 @@ test "download performs atomic rename and sets executable bit" { } } +test "downloadIfMissing keeps an existing destination" { + const allocator = std.testing.allocator; + + const tmp_dir = "/tmp/test-nullhub-downloader-skip-existing"; + std_compat.fs.deleteTreeAbsolute(tmp_dir) catch {}; + try std_compat.fs.makeDirAbsolute(tmp_dir); + defer std_compat.fs.deleteTreeAbsolute(tmp_dir) catch {}; + + const src_path = try std.fmt.allocPrint(allocator, "{s}/source.txt", .{tmp_dir}); + defer allocator.free(src_path); + const dest_path = try std.fmt.allocPrint(allocator, "{s}/binary", .{tmp_dir}); + defer allocator.free(dest_path); + + { + var file = try std_compat.fs.createFileAbsolute(src_path, .{}); + defer file.close(); + try file.writeAll("fresh content"); + } + { + var file = try std_compat.fs.createFileAbsolute(dest_path, .{}); + defer file.close(); + try file.writeAll("cached content"); + } + + const file_url = try std.fmt.allocPrint(allocator, "file://{s}", .{src_path}); + defer allocator.free(file_url); + + try downloadIfMissing(allocator, file_url, dest_path); + + var file = try std_compat.fs.openFileAbsolute(dest_path, .{}); + defer file.close(); + var buf: [256]u8 = undefined; + const n = try file.readAll(&buf); + try std.testing.expectEqualStrings("cached content", buf[0..n]); +} + test "downloadWithSha256 detects checksum mismatch" { const allocator = std.testing.allocator; diff --git a/src/installer/orchestrator.zig b/src/installer/orchestrator.zig index 446bb67..492bf41 100644 --- a/src/installer/orchestrator.zig +++ b/src/installer/orchestrator.zig @@ -161,19 +161,12 @@ pub fn install( resolved_version = allocator.dupe(u8, release.value.tag_name) catch return error.FetchFailed; const bin_path = p.binary(allocator, opts.component, resolved_version.?) catch return error.DownloadFailed; resolved_bin_path = bin_path; - // Skip the download if the binary is already cached on disk. - const already_cached = if (std_compat.fs.openFileAbsolute(bin_path, .{})) |f| blk: { - f.close(); - break :blk true; - } else |_| false; - if (!already_cached) { - downloader.download(allocator, asset.browser_download_url, bin_path) catch { - allocator.free(bin_path); - resolved_bin_path = null; - allocator.free(resolved_version.?); - resolved_version = null; - }; - } + downloader.downloadIfMissing(allocator, asset.browser_download_url, bin_path) catch { + allocator.free(bin_path); + resolved_bin_path = null; + allocator.free(resolved_version.?); + resolved_version = null; + }; } } else |_| {} } @@ -926,10 +919,9 @@ fn stageLocalBinary(allocator: std.mem.Allocator, p: paths_mod.Paths, component: const bin_path = p.binary(allocator, component, version) catch return null; errdefer allocator.free(bin_path); - if (std_compat.fs.openFileAbsolute(bin_path, .{})) |f| { - f.close(); + if (downloader.fileExists(bin_path)) { return .{ .version = version, .bin_path = bin_path }; - } else |_| {} + } std_compat.fs.copyFileAbsolute(local_path, bin_path, .{}) catch return null; if (comptime std_compat.fs.has_executable_bit) {