diff --git a/build.zig b/build.zig index 8cc5b9e4..0134d8ee 100644 --- a/build.zig +++ b/build.zig @@ -164,6 +164,7 @@ pub fn build(b: *std.Build) void { "tests/ghcr_401_retry_test.zig", "tests/net_get_to_writer_test.zig", "tests/bottle_download_test.zig", + "tests/install_download_cancel_test.zig", "tests/linker_core_test.zig", "tests/supervisor_pure_test.zig", "tests/install_pure_test.zig", diff --git a/scripts/regressions/bottle-download-retry-swallows-ctrl-c.sh b/scripts/regressions/bottle-download-retry-swallows-ctrl-c.sh new file mode 100755 index 00000000..07065c9a --- /dev/null +++ b/scripts/regressions/bottle-download-retry-swallows-ctrl-c.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# Regression: a Ctrl-C during a bottle download must end the retry loop, not +# be re-dialled and reported as a network failure. +# +# The bug: downloadBottleToStore's three-attempt loop had no cancellation +# input at all. Its only stop signal was a backoff helper reading cancellation +# off std.Io.sleep - a channel nothing in the process ever arms - while the +# real interrupt state lives in the signals module the same file already +# imports. A Ctrl-C mid-download therefore burned all three attempts, slept +# both backoffs, and ended with a per-formula "DownloadFailed (after 3 +# attempts)" line before the install loop finally reported the interruption. +# +# The fix polls the interrupt flag once per attempt and breaks, which also +# skips the attempts-exhausted line, so the interruption is reported once by +# the caller. +# +# The CLI cannot reach this path offline: MALT_BOTTLE_DOMAIN is https-only, so +# no cleartext loopback registry is reachable from the binary, and driving real +# GHCR would need network plus a race-prone signal. The contract is pinned by +# the integration test instead; this script builds and runs only that binary. +# Hermetic, no network, well under 30s - the test owns and cleans its own temp +# prefix, so nothing is left behind here. + +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +cd "$ROOT" + +SRC="src/cli/install/download.zig" +TEST="tests/install_download_cancel_test.zig" + +# A dropped guard or a dropped test would let the binary go green vacuously. +# Fail loudly instead: both must still be present. +if ! grep -Fqs -- "signals.isInterrupted()" "$SRC"; then + echo "FAIL: the interrupt poll is missing from the bottle retry loop" >&2 + exit 1 +fi +if ! grep -Fqs -- "setInterruptedForTest" "$TEST"; then + echo "FAIL: the download cancellation integration test is missing" >&2 + exit 1 +fi + +BIN="$ROOT/zig-out/test-bin/install_download_cancel_test" +# Always rebuild so the binary reflects current source; zig's cache keeps a +# no-op rebuild cheap. +if ! zig build test-bin >/dev/null 2>&1; then + echo "FAIL: could not build the test binaries (zig build test-bin)" >&2 + exit 1 +fi + +OUT=$("$BIN" 2>&1) && STATUS=0 || STATUS=$? +if [[ "$STATUS" -ne 0 ]]; then + echo "FAIL: a set interrupt flag did not stop the bottle retry loop after one attempt" >&2 + printf '%s\n' "$OUT" | grep -iE "failed|expected|leaked|panic" >&2 || true + exit 1 +fi + +echo "PASS: a Ctrl-C during a bottle download stops the retry loop at one attempt" diff --git a/src/cli/install.zig b/src/cli/install.zig index 5500ff35..972a2ac0 100644 --- a/src/cli/install.zig +++ b/src/cli/install.zig @@ -1006,10 +1006,17 @@ fn runInstall( // each warmed bottle's `/store/` path so a follow-up // real install can consume the bytes. if (flags.download_only) { + // This branch returns before the shared interrupt check below, so it + // owns its own: without it a Ctrl-C reads as a wall of download + // failures. Polled once - the flag drives every job in the loop. + const interrupted = signals.isInterrupted(); + if (interrupted) sink.warn("Interrupted. Cleaning up...", .{}); for (all_jobs.items) |job| { if (!job.succeeded) { output.emitNdjsonEvent(.download_complete, job.name, "failed"); - sink.err("Download failed for {s}", .{job.name}); + // A cancelled job is not a download failure; the interruption + // is already reported once, above. + if (!interrupted) sink.err("Download failed for {s}", .{job.name}); failed_count += 1; continue; } diff --git a/src/cli/install/download.zig b/src/cli/install/download.zig index 19b124a5..25eac4bd 100644 --- a/src/cli/install/download.zig +++ b/src/cli/install/download.zig @@ -94,15 +94,16 @@ pub fn progressBridge(ctx: *anyopaque, bytes_so_far: u64, content_length: ?u64) bar.update(clamped); } -/// Sleeps for `ms` between retries. Returns false when the caller's -/// stop signal cancels the sleep, true otherwise. std.Io cancellation -/// is single-shot per task, so a swallowed Canceled would silently -/// consume the request and the loop would keep retrying. -fn cancellableBackoff(io: std.Io, ms: u64) bool { - std.Io.sleep(io, std.Io.Duration.fromNanoseconds(@intCast(ms * std.time.ns_per_ms)), .awake) catch |e| switch (e) { - error.Canceled => return false, - }; - return true; +/// At file scope so the invariants between the two can be pinned by a unit +/// test, without standing up a registry. +const max_download_attempts: u8 = 3; +const download_retry_delays_ms = [_]u64{ 100, 400 }; + +/// True only when the loop spent its whole budget. Every early exit - an +/// interrupt included - leaves `dl_attempt` short of it, which is what keeps +/// the attempts-exhausted diagnostic off the cancellation path. +fn attemptsExhausted(dl_attempt: u8) bool { + return dl_attempt >= max_download_attempts; } /// Errors that re-running the download cannot rescue: the cause is in the @@ -637,13 +638,16 @@ pub fn downloadBottleToStore( .func = &progressBridge, } else null; - const max_attempts: u8 = 3; - const retry_delays_ms = [_]u64{ 100, 400 }; var dl_attempt: u8 = 0; var dl_ok = false; var last_err: bottle_mod.BottleError = bottle_mod.BottleError.DownloadFailed; var last_mismatch: ?bottle_mod.MismatchInfo = null; - while (dl_attempt < max_attempts) : (dl_attempt += 1) { + while (dl_attempt < max_download_attempts) : (dl_attempt += 1) { + // Re-dialling after a Ctrl-C reports the user's own stop as a + // network failure. Breaking short of the budget also suppresses the + // attempts-exhausted line, so the caller reports it once. + if (signals.isInterrupted()) break; + var mismatch: bottle_mod.MismatchInfo = undefined; if (bottle_mod.download( ctx.io, @@ -672,8 +676,11 @@ pub fn downloadBottleToStore( deps.sink.err(" {s}: {s}", .{ formula.name, @errorName(dl_err) }); break; } - if (dl_attempt + 1 < max_attempts) { - if (!cancellableBackoff(ctx.io, retry_delays_ms[dl_attempt])) break; + if (dl_attempt + 1 < max_download_attempts) { + const delay = download_retry_delays_ms[dl_attempt] * std.time.ns_per_ms; + // A cancelled sleep needs no special case: the next iteration + // polls the flag before spending another attempt. + std.Io.sleep(ctx.io, std.Io.Duration.fromNanoseconds(@intCast(delay)), .awake) catch {}; } } } @@ -691,8 +698,15 @@ pub fn downloadBottleToStore( if (!dl_ok) { if (deps.bar) |bar| bar.finish(); - if (dl_attempt >= max_attempts) { - deps.sink.err(" {s}: {s} (after {d} attempts)", .{ formula.name, @errorName(last_err), max_attempts }); + // A failed attempt wipes the temp dir itself, but an interrupt can + // break before the first one runs. deleteTree no-ops when it is + // already gone, so this covers every failing exit. + atomic.cleanupTempDir(ctx.io, tmp_dir); + // The flag can also be raised while the last attempt is in flight, + // which the loop's own poll can no longer catch - re-check so that + // window reports as a cancel rather than an exhausted budget. + if (attemptsExhausted(dl_attempt) and !signals.isInterrupted()) { + deps.sink.err(" {s}: {s} (after {d} attempts)", .{ formula.name, @errorName(last_err), max_download_attempts }); } allocator.free(tmp_dir); return InstallError.DownloadFailed; @@ -946,56 +960,26 @@ test "MaterializeResult.kegPath reflects keg_path_len after write" { try std.testing.expectEqualStrings(path, r.kegPath()); } -// Patches an inner Io's vtable so `sleep` reports cancellation on the -// configured call index; non-canceled sleeps return immediately so the -// retry-table delays don't pad the test runtime. -const CancelSleepProbe = struct { - var vtable: std.Io.VTable = undefined; - var sleep_calls: usize = 0; - var cancel_at: usize = 1; - - fn wrap(inner: std.Io, cancel_at_call: usize) std.Io { - vtable = inner.vtable.*; - vtable.sleep = sleepMaybeCanceled; - sleep_calls = 0; - cancel_at = cancel_at_call; - return .{ .userdata = inner.userdata, .vtable = &vtable }; +test "attemptsExhausted is false for every attempt an early break can reach" { + // The interrupt poll breaks from inside the loop body, so `dl_attempt` is + // always below the budget there. That is the whole mechanism keeping the + // "(after N attempts)" line off a cancelled download - if this ever went + // true early, a Ctrl-C would be misreported as a network failure again. + var i: u8 = 0; + while (i < max_download_attempts) : (i += 1) { + try std.testing.expect(!attemptsExhausted(i)); } - - fn sleepMaybeCanceled(_: ?*anyopaque, _: std.Io.Timeout) std.Io.Cancelable!void { - sleep_calls += 1; - if (sleep_calls >= cancel_at) return error.Canceled; - } -}; - -test "cancellableBackoff returns true when sleep completes normally" { - var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); - defer threaded.deinit(); - // Zero-ms request keeps the test deterministic without exercising the - // real clock; a successful sleep must report true so the retry loop - // moves on to the next attempt. - try std.testing.expect(cancellableBackoff(threaded.io(), 0)); -} - -test "cancellableBackoff returns false when sleep is cancelled" { - var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); - defer threaded.deinit(); - const cancel_io = CancelSleepProbe.wrap(threaded.io(), 1); - try std.testing.expect(!cancellableBackoff(cancel_io, 100)); - try std.testing.expectEqual(@as(usize, 1), CancelSleepProbe.sleep_calls); + try std.testing.expect(attemptsExhausted(max_download_attempts)); } -test "cancellableBackoff propagates cancellation when called repeatedly" { - var threaded: std.Io.Threaded = .init(std.testing.allocator, .{}); - defer threaded.deinit(); - // Cancel the third call: the prior two complete and report true, - // pinning that the helper isn't inadvertently latched after the - // first non-cancelled sleep. - const cancel_io = CancelSleepProbe.wrap(threaded.io(), 3); - try std.testing.expect(cancellableBackoff(cancel_io, 0)); - try std.testing.expect(cancellableBackoff(cancel_io, 0)); - try std.testing.expect(!cancellableBackoff(cancel_io, 0)); - try std.testing.expectEqual(@as(usize, 3), CancelSleepProbe.sleep_calls); +test "the backoff table covers every gap between attempts" { + // The loop indexes the table by attempt number on all but the last + // attempt, so a budget raised without extending the table would panic on + // a real retry - a path no offline test would otherwise reach. + try std.testing.expectEqual( + @as(usize, max_download_attempts - 1), + download_retry_delays_ms.len, + ); } // --------------------------------------------------------------------------- diff --git a/tests/all.zig b/tests/all.zig index 751215e9..b4e529f4 100644 --- a/tests/all.zig +++ b/tests/all.zig @@ -69,6 +69,7 @@ comptime { _ = @import("info_cli_test.zig"); _ = @import("info_test.zig"); _ = @import("install_ambiguity_test.zig"); + _ = @import("install_download_cancel_test.zig"); _ = @import("install_download_only_test.zig"); _ = @import("install_execute_test.zig"); _ = @import("install_idempotent_test.zig"); diff --git a/tests/install_download_cancel_test.zig b/tests/install_download_cancel_test.zig new file mode 100644 index 00000000..6c87b4ac --- /dev/null +++ b/tests/install_download_cancel_test.zig @@ -0,0 +1,329 @@ +//! malt - bottle-download cancellation integration tests. +//! +//! Pins that a Ctrl-C is honoured *inside* a single formula's download, not +//! only between queued jobs. `downloadBottleToStore` retries a transient +//! failure three times; with the interrupt flag raised it must stop at the +//! attempt in flight and stay quiet, so the caller reports the interruption +//! once instead of the loop misreporting it as a download failure. +//! +//! A loopback `std.http.Server` answers `/token` and `/blobs/…` with a +//! retryable 500, so every attempt is transient and the only thing that can +//! shorten the loop is the interrupt poll. No real network. + +const std = @import("std"); +const malt = @import("malt"); +const test_io = @import("test_io"); +const testing = std.testing; +const client = malt.client; +const download = malt.install_download; +const formula_mod = malt.formula; +const ghcr = malt.ghcr; +const signals = malt.signals; +const store_mod = malt.store; +const net = std.Io.net; + +const c = struct { + extern "c" fn setenv(name: [*:0]const u8, value: [*:0]const u8, overwrite: c_int) c_int; + extern "c" fn unsetenv(name: [*:0]const u8) c_int; +}; + +const bottle_sha = "1111111111111111111111111111111111111111111111111111111111111111"; + +const Stub = struct { + io: std.Io, + listener: *net.Server, + blob_count: usize = 0, + // When true the blob GET succeeds with bytes that cannot hash to the + // expected digest, so the attempt fails as a mismatch rather than a 500. + corrupt_body: bool = false, + // Raises the interrupt flag while this numbered blob GET is being served, + // i.e. with that attempt already in flight. + flag_on_blob: ?usize = null, +}; + +// Keep-alive loop: every blob GET answers a retryable 500 so the install +// loop treats each attempt as transient and would re-dial until it exhausts +// its budget. Ends when the client closes and `receiveHead` fails. +fn serveStub(s: *Stub) void { + const stream = s.listener.accept(s.io) catch return; + defer stream.close(s.io); + var rbuf: [16 * 1024]u8 = undefined; + var wbuf: [16 * 1024]u8 = undefined; + var reader = stream.reader(s.io, &rbuf); + var writer = stream.writer(s.io, &wbuf); + var srv = std.http.Server.init(&reader.interface, &writer.interface); + while (true) { + var req = srv.receiveHead() catch return; + const target = req.head.target; + if (std.mem.indexOf(u8, target, "/token") != null) { + req.respond("{\"token\":\"t1\"}", .{}) catch return; + } else if (std.mem.indexOf(u8, target, "/blobs/") != null) { + s.blob_count += 1; + if (s.flag_on_blob) |n| { + if (s.blob_count == n) malt.signals.setInterruptedForTest(true); + } + if (s.corrupt_body) { + req.respond("not-the-expected-bytes", .{}) catch return; + } else { + req.respond("boom\n", .{ .status = .internal_server_error }) catch return; + } + } else { + req.respond("not found\n", .{ .status = .not_found }) catch return; + } + } +} + +// Counts the per-keg failure lines the loop emits so a test can assert the +// "(after N attempts)" misreport is gone without matching terminal output. +const RecordingSink = struct { + errs: usize = 0, + last: [256]u8 = undefined, + last_len: usize = 0, + + fn writeErr(ctx: ?*anyopaque, msg: []const u8) void { + const self: *RecordingSink = @ptrCast(@alignCast(ctx)); + self.errs += 1; + const n = @min(msg.len, self.last.len); + @memcpy(self.last[0..n], msg[0..n]); + self.last_len = n; + } + fn writeNoop(_: ?*anyopaque, _: []const u8) void {} + + fn text(self: *const RecordingSink) []const u8 { + return self.last[0..self.last_len]; + } +}; + +const formula_json = + \\{ + \\ "name": "cancelpkg", + \\ "full_name": "cancelpkg", + \\ "tap": "homebrew/core", + \\ "desc": "", + \\ "homepage": "", + \\ "versions": {"stable": "1.0"}, + \\ "revision": 0, + \\ "dependencies": [], + \\ "keg_only": false, + \\ "post_install_defined": false, + \\ "oldnames": [], + \\ "bottle": {"stable": {"files": {}}} + \\} +; + +fn setupPrefix(tag: []const u8) ![:0]u8 { + const base = try test_io.uniqueTempPath(testing.allocator, "install_dl_cancel", tag); + defer testing.allocator.free(base); + const path = try testing.allocator.dupeZ(u8, base); + inline for (.{ "store", "tmp", "db" }) |sub| { + const dir = try std.fmt.allocPrint(testing.allocator, "{s}/{s}", .{ path, sub }); + defer testing.allocator.free(dir); + try test_io.cwd().createDirPath(std.Options.debug_io, dir); + } + _ = c.setenv("MALT_PREFIX", path.ptr, 1); + return path; +} + +/// Outcome of one `downloadBottleToStore` run against the always-500 stub. +/// Counts what survives under `/tmp`; 0 means the scratch dir was +/// reclaimed. Errors propagate - a helper that silently returned 0 would let +/// a leak pass as a clean run. +fn countTmpEntries(prefix: []const u8) !usize { + var buf: [512]u8 = undefined; + const path = try std.fmt.bufPrint(&buf, "{s}/tmp", .{prefix}); + var dir = try test_io.openDirAbsolute(std.Options.debug_io, path, .{ .iterate = true }); + defer dir.close(std.Options.debug_io); + var it = dir.iterate(); + var n: usize = 0; + while (try it.next(std.Options.debug_io)) |_| n += 1; + return n; +} + +const Run = struct { + result: anyerror!bool, + blob_gets: usize, + sink: RecordingSink, + /// Entries left under `/tmp` once the call returns. A cancelled + /// download must not strand its scratch dir - nothing sweeps that path. + tmp_leftovers: usize, +}; + +/// Drive the retry loop once with whatever interrupt state the caller armed. +/// Everything (prefix, db, stub) is owned and torn down here so each test +/// body is just arrange-flag → run → assert. +fn runDownload(tag: []const u8, corrupt_body: bool, flag_on_blob: ?usize) !Run { + const prefix = try setupPrefix(tag); + defer { + _ = c.unsetenv("MALT_PREFIX"); + test_io.deleteTreeAbsolute(std.Options.debug_io, prefix) catch {}; + testing.allocator.free(prefix); + } + + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const io = threaded.io(); + + var addr = try net.IpAddress.parseIp4("127.0.0.1", 0); + var listener = try addr.listen(io, .{ .reuse_address = true }); + defer listener.deinit(io); + const port = listener.socket.address.getPort(); + + var base_buf: [64]u8 = undefined; + const base = try std.fmt.bufPrint(&base_buf, "http://127.0.0.1:{d}", .{port}); + + var inner: std.http.Client = .{ .allocator = testing.allocator, .io = io }; + var http = client.HttpClient.initWith(&inner, io, std.process.Environ.empty, testing.allocator); + // Empty budget: net answers the 500 straight back, so one blob GET means + // exactly one install-loop attempt. + http.retry_backoff_ms = &.{}; + + var g = ghcr.GhcrClient.init(io, testing.allocator, &http); + defer g.deinit(); + g.base_url = base; + + var db = try malt.sqlite.Database.open(":memory:"); + defer db.close(); + var store = store_mod.Store.init(io, testing.allocator, &db, prefix); + + var rec = RecordingSink{}; + var f = try formula_mod.parseFormula(testing.allocator, formula_json); + defer f.deinit(); + + // Spawn last: nothing below can fail, so the join at the end is always + // reached and the stub thread can never outlive this frame. + var stub = Stub{ .io = io, .listener = &listener, .corrupt_body = corrupt_body, .flag_on_blob = flag_on_blob }; + const server_thread = try std.Thread.spawn(.{}, serveStub, .{&stub}); + + const ctx: malt.app_ctx.AppCtx = .{ .io = io, .environ = .empty }; + const result = download.downloadBottleToStore(&ctx, testing.allocator, .{ + .ghcr = &g, + .http = &http, + .store = &store, + .sink = .{ + .ctx = &rec, + .writeInfo = RecordingSink.writeNoop, + .writeWarn = RecordingSink.writeNoop, + .writeSuccess = RecordingSink.writeNoop, + .writeErr = RecordingSink.writeErr, + }, + }, &f, .{ + .cellar = ":any", + .url = "https://ghcr.io/v2/homebrew/core/cancelpkg/blobs/sha256:" ++ bottle_sha, + .sha256 = bottle_sha, + }); + + // Close the client so the stub's keep-alive loop ends, then join before + // reading its counter to avoid a data race. A cancelled run issues no + // request at all, so the stub may still be parked in accept() - one + // throwaway connection wakes it and keeps the join bounded. + http.deinit(); + var wake_addr = try net.IpAddress.parseIp4("127.0.0.1", port); + if (net.IpAddress.connect(&wake_addr, io, .{ .mode = .stream })) |s| s.close(io) else |_| {} + server_thread.join(); + + return .{ + .result = result, + .blob_gets = stub.blob_count, + .sink = rec, + .tmp_leftovers = try countTmpEntries(prefix), + }; +} + +test "an interrupt raised before the download starts issues no request at all" { + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(true); + + const run = try runDownload("pre", false, null); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 0), run.blob_gets); + // The caller prints the interruption; the loop must add nothing. + try testing.expectEqual(@as(usize, 0), run.sink.errs); + // Breaking before the first attempt skips the per-attempt cleanup, so the + // scratch dir has to be reclaimed on the way out. + try testing.expectEqual(@as(usize, 0), run.tmp_leftovers); +} + +test "an interrupt during the first attempt stops the retry loop at one request" { + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(false); + // The loop polls once per attempt: poll 1 lets attempt 1 run, poll 2 fires. + signals.armInterruptAfterForTest(2); + defer signals.armInterruptAfterForTest(0); + + const run = try runDownload("mid", false, null); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 1), run.blob_gets); + try testing.expectEqual(@as(usize, 0), run.sink.errs); +} + +test "an uninterrupted transient failure still burns the full retry budget" { + // Guards the fix against over-reach: without an interrupt the loop keeps + // its three attempts and its attempts-exhausted diagnostic. + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(false); + + const run = try runDownload("full", false, null); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 3), run.blob_gets); + try testing.expectEqual(@as(usize, 1), run.sink.errs); + try testing.expect(std.mem.indexOf(u8, run.sink.text(), "after 3 attempts") != null); +} + +test "a cancel keeps a real mismatch diagnostic but drops the attempts line" { + // The fix suppresses only the attempts-exhausted misreport. A checksum + // mismatch actually happened, so that line stays - pinning which of the + // two diagnostics the cancellation path is allowed to silence. + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(false); + signals.armInterruptAfterForTest(2); + defer signals.armInterruptAfterForTest(0); + + const run = try runDownload("mismatch", true, null); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 1), run.blob_gets); + try testing.expectEqual(@as(usize, 1), run.sink.errs); + try testing.expect(std.mem.indexOf(u8, run.sink.text(), "Sha256Mismatch") != null); + try testing.expect(std.mem.indexOf(u8, run.sink.text(), "after 3 attempts") == null); +} + +test "an interrupt before the last attempt stops the retry loop at two requests" { + // Arms the poll that guards attempt 3, so attempts 1 and 2 run and both + // backoff entries are indexed - the case that would trip an off-by-one in + // the delay table under a live cancel. + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(false); + signals.armInterruptAfterForTest(3); + defer signals.armInterruptAfterForTest(0); + + const run = try runDownload("late", false, null); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 2), run.blob_gets); + try testing.expectEqual(@as(usize, 0), run.sink.errs); + try testing.expectEqual(@as(usize, 0), run.tmp_leftovers); +} + +test "an interrupt during the last attempt is not reported as an exhausted budget" { + // The loop polls before each attempt, so a flag raised while the final + // attempt is already in flight escapes it. The report gate re-checks, or + // this window prints the exact misreport the fix removes. + const prior = signals.isInterrupted(); + defer signals.setInterruptedForTest(prior); + signals.setInterruptedForTest(false); + + const run = try runDownload("last", false, 3); + + try testing.expectError(malt.install_record.InstallError.DownloadFailed, run.result); + try testing.expectEqual(@as(usize, 3), run.blob_gets); + try testing.expectEqual(@as(usize, 0), run.sink.errs); + try testing.expectEqual(@as(usize, 0), run.tmp_leftovers); +} diff --git a/tests/install_download_only_test.zig b/tests/install_download_only_test.zig index 5578f946..49c56ce8 100644 --- a/tests/install_download_only_test.zig +++ b/tests/install_download_only_test.zig @@ -936,3 +936,49 @@ test "materializeRubyFormula installs a lib-only keg that ships no binary" { defer testing.allocator.free(link); try testing.expect(pathExists(link)); } + +test "--download-only reports a Ctrl-C as an interruption" { + // The download-only branch returns before the shared interrupt check, so + // without its own poll a cancelled batch never said it was interrupted - + // it just listed per-formula download failures. The arm value lets + // resolution finish and flips the flag around the pool; anything in 3..5 + // lands in that window, so 4 tolerates a poll site moving by one. + const prefix = try setupPrefix("dlint"); + defer testing.allocator.free(prefix); + defer test_io.deleteTreeAbsolute(std.Options.debug_io, prefix) catch {}; + defer _ = c.unsetenv("MALT_PREFIX"); + + const sha = "44" ** 32; + try seedStoreBottle(prefix, sha, "intpkg", "1.0"); + var arena_json = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_json.deinit(); + const json = try warmFormulaJson(arena_json.allocator(), "intpkg", sha); + try seedFormulaCache(prefix, "intpkg", json); + + const prior_quiet = malt.output.isQuiet(); + malt.output.setQuiet(false); + defer malt.output.setQuiet(prior_quiet); + + const prior_interrupted = malt.signals.isInterrupted(); + defer malt.signals.setInterruptedForTest(prior_interrupted); + malt.signals.setInterruptedForTest(false); + malt.signals.armInterruptAfterForTest(4); + defer malt.signals.armInterruptAfterForTest(0); + + var captured: std.ArrayList(u8) = .empty; + defer captured.deinit(testing.allocator); + malt.output.beginStderrCapture(testing.allocator, &captured); + defer malt.output.endStderrCapture(); + + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var threaded: std.Io.Threaded = .init(testing.allocator, .{}); + defer threaded.deinit(); + const ctx: malt.app_ctx.AppCtx = .{ .io = threaded.io(), .environ = .empty }; + install.execute(&ctx, arena.allocator(), &.{ "--download-only", "intpkg" }) catch {}; + + try testing.expect(std.mem.indexOf(u8, captured.items, "Interrupted") != null); + // Paired half of the same flag: a cancelled job must not also be listed + // as a download failure. + try testing.expect(std.mem.indexOf(u8, captured.items, "Download failed for") == null); +}