From 5570377c4997a5abe9f433bc15c927bcde237dc1 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Mon, 28 Sep 2026 15:44:00 -0300 Subject: A Restore of a dump that cannot be read fails at once, and spends no warning The dump was read only by the host after the step, where a failure went to the message row alone: a Restore written to ctl answered success, logged no err, and a second one past the unsaved-text warning did the same. Restore now reads the dump first, before the warning, and says 'Restore: : no such file' (or permission denied, a directory), which fails a ctl write and so logs err. A bare Restore with no dump yet, or a name too long, says so too. Co-Authored-By: Claude Opus 5.5 --- src/builtins.zig | 52 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/builtins.zig b/src/builtins.zig index be0956e0..fbab925c 100644 --- a/src/builtins.zig +++ b/src/builtins.zig @@ -236,16 +236,41 @@ test "Exit asks once about unsaved text, and quits when asked again" { try std.testing.expect(p.quit); } +/// A dump file that exists, for a Restore to get past reading it. +fn testDump(buf: []u8) ![]const u8 { + var tmp = std.testing.tmpDir(.{}); + try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "d.zon", .data = ".{}" }); + var dir: [4096]u8 = undefined; + const at = dir[0..try tmp.dir.realPath(std.testing.io, &dir)]; + return std.fmt.bufPrint(buf, "Restore {s}/d.zon", .{at}); +} + test "Restore asks about unsaved text as Exit does, and restores when asked again" { const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true }); defer p.deinit(); const pane = try p.setTestFile("saved\n"); pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified - try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon")); + var line_buf: [4200]u8 = undefined; + const line = try testDump(&line_buf); + try std.testing.expect(p.executeBuiltinLine(p.active, line)); try std.testing.expect(p.restore_req == null); try std.testing.expect(std.mem.endsWith(u8, pane.msg[0..pane.msg_len], ": Modified (Restore again to discard)")); - try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon")); - try std.testing.expectEqualStrings("/tmp/some.dump.zon", p.restore_req.?); + try std.testing.expect(p.executeBuiltinLine(p.active, line)); + try std.testing.expectEqualStrings(line["Restore ".len..], p.restore_req.?); +} + +test "a Restore of a dump that cannot be read says so, before the unsaved-text warning" { + const p = try Pardes.init(std.testing.allocator, .{ .tty_only = true }); + defer p.deinit(); + const pane = try p.setTestFile("saved\n"); + pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified + p.fs.no_prompt = true; + defer p.fs.no_prompt = false; + try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /nope/missing.zon")); + try std.testing.expect(p.restore_req == null); + try std.testing.expectEqualStrings("Restore: /nope/missing.zon: no such file", p.fs.failure[0..p.fs.failure_len]); + // No warning spent: the Exit after it still asks. + try std.testing.expect(pane.discard_warned == null); } test "a Restore's warning is not an Exit's: each word is warned on its own" { @@ -253,7 +278,8 @@ test "a Restore's warning is not an Exit's: each word is warned on its own" { defer p.deinit(); const pane = try p.setTestFile("saved\n"); pane.file.?.saved_revision = pane.file.?.revision -% 1; // modified - try std.testing.expect(p.executeBuiltinLine(p.active, "Restore /tmp/some.dump.zon")); + var line_buf: [4200]u8 = undefined; + try std.testing.expect(p.executeBuiltinLine(p.active, try testDump(&line_buf))); try std.testing.expect(p.restore_req == null); // The Restore warned; an Exit after it has not been, and asks. try std.testing.expect(p.executeBuiltinLine(p.active, "Exit")); @@ -473,8 +499,22 @@ pub const Restore = struct { pub const scope: Scope = .session; pub const takes_arg = true; pub fn run(c: Ctx) void { - const path = c.arg orelse (c.p.last_dump orelse return); - if (path.len > c.p.restore_buf.len) return; + const path = c.arg orelse (c.p.last_dump orelse return c.p.reportFailure(c.id, "Restore: no dump yet; name one")); + if (path.len > c.p.restore_buf.len) return c.p.reportFailure(c.id, "Restore: that name is too long"); + // A dump that cannot be read is said now, and fails a ctl write: the + // host reads it only after this step, where nobody hears. First, + // so a Restore that could never happen spends no warning. + const bytes = @import("fs.zig").readRestore(c.p.gpa, path, c.p.settings.dump_dir.get()) catch |err| { + var buf: [limits.host_path_cap + 64]u8 = undefined; + const why = switch (err) { + error.FileNotFound => "no such file", + error.PermissionDenied => "permission denied", + error.IsDirectory => "a directory, not a dump", + else => @errorName(err), + }; + return c.p.reportFailure(c.id, std.fmt.bufPrint(&buf, "Restore: {s}: {s}", .{ path, why }) catch "Restore: cannot read the dump"); + }; + c.p.gpa.free(bytes); // acme's Load adds a dump's windows to the ones there; a Restore // replaces them all, so it asks what Exit asks first. if (warnModified(c, .Restore)) return; -- cgit v1.3