| author |  Isaac Freund <mail@isaacfreund.com> 2025-08-16 11:32:10+02:00 |
| committer |  Andrew Kelley <andrew@ziglang.org> 2025-08-16 15:43:48-07:00 |
| log | 551e009da7be279293fe261b729280d29fcf81b0 |
| tree | 2cc356fc7c44c09d8e893f4f1b5a63b3290ec993 |
| parent | 399bace2f20e64e4c10c014dc3b8e202a891c6e4 |
Build.Step.Run: fix missing stdin buffer and flush
Writer.sendFileAll() asserts non-zero buffer capacity in the case that
the fallback is hit. It also requires the caller to flush. The buffer
may be bypassed as an optimization but this is not a guarantee.
Also improve the Writer documentation and add an earlier assert on
buffer capacity in sendFileAll().2 files changed, 21 insertions(+), 3 deletions(-)
lib- std
- Build
- Step
lib/std/Build/Step/Run.zig+9-3
| ... | @@ -1780,9 +1780,10 @@ fn evalGeneric(run: *Run, child: *std.process.Child) !StdIoResult { | ... | @@ -1780,9 +1780,10 @@ fn evalGeneric(run: *Run, child: *std.process.Child) !StdIoResult { |
| 1780 | }; | 1780 | }; |
| 1781 | defer file.close(); | 1781 | defer file.close(); |
| 1782 | // TODO https://github.com/ziglang/zig/issues/23955 | 1782 | // TODO https://github.com/ziglang/zig/issues/23955 |
| 1783 | var buffer: [1024]u8 = undefined; | 1783 | var read_buffer: [1024]u8 = undefined; |
| 1784 | var file_reader = file.reader(&buffer); | 1784 | var file_reader = file.reader(&read_buffer); |
| 1785 | var stdin_writer = child.stdin.?.writer(&.{}); | 1785 | var write_buffer: [1024]u8 = undefined; |
| 1786 | var stdin_writer = child.stdin.?.writer(&write_buffer); | ||
| 1786 | _ = stdin_writer.interface.sendFileAll(&file_reader, .unlimited) catch |err| switch (err) { | 1787 | _ = stdin_writer.interface.sendFileAll(&file_reader, .unlimited) catch |err| switch (err) { |
| 1787 | error.ReadFailed => return run.step.fail("failed to read from {f}: {t}", .{ | 1788 | error.ReadFailed => return run.step.fail("failed to read from {f}: {t}", .{ |
| 1788 | path, file_reader.err.?, | 1789 | path, file_reader.err.?, |
| ... | @@ -1791,6 +1792,11 @@ fn evalGeneric(run: *Run, child: *std.process.Child) !StdIoResult { | ... | @@ -1791,6 +1792,11 @@ fn evalGeneric(run: *Run, child: *std.process.Child) !StdIoResult { |
| 1791 | stdin_writer.err.?, | 1792 | stdin_writer.err.?, |
| 1792 | }), | 1793 | }), |
| 1793 | }; | 1794 | }; |
| 1795 | stdin_writer.interface.flush() catch |err| switch (err) { | ||
| 1796 | error.WriteFailed => return run.step.fail("failed to write to stdin: {t}", .{ | ||
| 1797 | stdin_writer.err.?, | ||
| 1798 | }), | ||
| 1799 | }; | ||
| 1794 | child.stdin.?.close(); | 1800 | child.stdin.?.close(); |
| 1795 | child.stdin = null; | 1801 | child.stdin = null; |
| 1796 | }, | 1802 | }, |
lib/std/Io/Writer.zig+12
| ... | @@ -882,6 +882,9 @@ pub fn writeSliceSwap(w: *Writer, Elem: type, slice: []const Elem) Error!void { | ... | @@ -882,6 +882,9 @@ pub fn writeSliceSwap(w: *Writer, Elem: type, slice: []const Elem) Error!void { |
| 882 | /// Unlike `writeSplat` and `writeVec`, this function will call into `VTable` | 882 | /// Unlike `writeSplat` and `writeVec`, this function will call into `VTable` |
| 883 | /// even if there is enough buffer capacity for the file contents. | 883 | /// even if there is enough buffer capacity for the file contents. |
| 884 | /// | 884 | /// |
| 885 | /// The caller is responsible for flushing. Although the buffer may be bypassed | ||
| 886 | /// as an optimization, this is not a guarantee. | ||
| 887 | /// | ||
| 885 | /// Although it would be possible to eliminate `error.Unimplemented` from the | 888 | /// Although it would be possible to eliminate `error.Unimplemented` from the |
| 886 | /// error set by reading directly into the buffer in such case, this is not | 889 | /// error set by reading directly into the buffer in such case, this is not |
| 887 | /// done because it is more efficient to do it higher up the call stack so that | 890 | /// done because it is more efficient to do it higher up the call stack so that |
| ... | @@ -924,7 +927,16 @@ pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) File | ... | @@ -924,7 +927,16 @@ pub fn sendFileReading(w: *Writer, file_reader: *File.Reader, limit: Limit) File |
| 924 | 927 | ||
| 925 | /// Number of bytes logically written is returned. This excludes bytes from | 928 | /// Number of bytes logically written is returned. This excludes bytes from |
| 926 | /// `buffer` because they have already been logically written. | 929 | /// `buffer` because they have already been logically written. |
| 930 | /// | ||
| 931 | /// The caller is responsible for flushing. Although the buffer may be bypassed | ||
| 932 | /// as an optimization, this is not a guarantee. | ||
| 933 | /// | ||
| 934 | /// Asserts nonzero buffer capacity. | ||
| 927 | pub fn sendFileAll(w: *Writer, file_reader: *File.Reader, limit: Limit) FileAllError!usize { | 935 | pub fn sendFileAll(w: *Writer, file_reader: *File.Reader, limit: Limit) FileAllError!usize { |
| 936 | // The fallback sendFileReadingAll() path asserts non-zero buffer capacity. | ||
| 937 | // Explicitly assert it here as well to ensure the assert is hit even if | ||
| 938 | // the fallback path is not taken. | ||
| 939 | assert(w.buffer.len > 0); | ||
| 928 | var remaining = @intFromEnum(limit); | 940 | var remaining = @intFromEnum(limit); |
| 929 | while (remaining > 0) { | 941 | while (remaining > 0) { |
| 930 | const n = sendFile(w, file_reader, .limited(remaining)) catch |err| switch (err) { | 942 | const n = sendFile(w, file_reader, .limited(remaining)) catch |err| switch (err) { |