Commit 614161a7cf65f46cb3e2461ffe2e09d972508a97 | zig

author	Igor Anić <igor.anic@gmail.com> 2024-03-02 17:52:31+01:00
committer	Igor Anić <igor.anic@gmail.com> 2024-03-11 12:22:12+01:00
log	614161a7cf65f46cb3e2461ffe2e09d972508a97
tree	e421443136c21a1b24da91c18d76ece3194f00c9
parent	5ccbb196ad08fd5e58fc8874917a20f9a220d729

std.tar make iterator interface more ergonomic

for the then end users:

1. Don't require user to call file.skip() on file returned from
iterator.next if file is not read. Iterator will now handle this.
Previously that returned header parsing error, without knowing some tar
internals it is hard to understand what is required from user.

2. Use iterator.File.kind enum which is similar to fs.File.Kind,
something familiar. Internal Header.Kind has many types which are not
exposed but the user needs to have else in kind switch to cover those
cases.

3. Add reader interface to the iterator.File.

2 files changed, 75 insertions(+), 53 deletions(-)

lib
- std
  - tar.zig
  - tar
    test.zig

lib/std/tar.zig+63-43

 @@ -258,10 +258,15 @@ pub fn iterator(reader: anytype, options: IteratorOptions) Iterator(@TypeOf(read
         .file_name_buffer = options.file_name_buffer,
         .link_name_buffer = options.link_name_buffer,
         .padding = 0,
-        .file = undefined,
     };
 }
+pub const FileKind = enum {
+    directory,
+    sym_link,
+    file,
+};
 fn Iterator(comptime ReaderType: type) type {
     return struct {
         reader: ReaderType,
 @@ -274,35 +279,43 @@ fn Iterator(comptime ReaderType: type) type {
         // bytes of padding to the end of the block
         padding: usize,
-        // current tar file
+        // not consumed bytes of file from last next iteration
-        file: File,
+        unread_file_bytes: usize = 0,
         pub const File = struct {
             name: []const u8, // name of file, symlink or directory
             link_name: []const u8, // target name of symlink
-            size: u64, // size of the file in bytes
+            size: u64 = 0, // size of the file in bytes
-            mode: u32,
+            mode: u32 = 0,
-            kind: Header.Kind,
+            kind: FileKind = .file,
+            unread_bytes: *usize,
             reader: ReaderType,
+            pub const Reader = std.io.Reader(*Self, ReaderType.Error, read);
+            pub fn reader(self: *Self) Reader {
+                return .{ .context = self };
+            }
+            pub fn read(self: *Self, dest: []u8) ReaderType.Error!usize {
+                const buf = dest[0..@min(dest.len, self.unread_size.*)];
+                const n = try self.reader.read(buf);
+                self.unread_size.* -= n;
+                return n;
+            }
             // Writes file content to writer.
-            pub fn write(self: File, writer: anytype) !void {
+            pub fn writeAll(self: File, writer: anytype) !void {
                 var buffer: [4096]u8 = undefined;
-                var n: u64 = 0;
+                while (self.unread_bytes.* > 0) {
-                while (n < self.size) {
+                    const buf = buffer[0..@min(buffer.len, self.unread_bytes.*)];
-                    const buf = buffer[0..@min(buffer.len, self.size - n)];
                     try self.reader.readNoEof(buf);
                     try writer.writeAll(buf);
-                    n += buf.len;
+                    self.unread_bytes.* -= buf.len;
                 }
             }
-            // Skips file content. Advances reader.
-            pub fn skip(self: File) !void {
-                try self.reader.skipBytes(self.size, .{});
-            }
         };
         const Self = @This();
 @@ -326,14 +339,12 @@ fn Iterator(comptime ReaderType: type) type {
             return nullStr(buf);
         }
-        fn initFile(self: *Self) void {
+        fn newFile(self: *Self) File {
-            self.file = .{
+            return .{
                 .name = self.file_name_buffer[0..0],
                 .link_name = self.link_name_buffer[0..0],
-                .size = 0,
-                .kind = .normal,
-                .mode = 0,
                 .reader = self.reader,
+                .unread_bytes = &self.unread_file_bytes,
             };
         }
 @@ -350,7 +361,12 @@ fn Iterator(comptime ReaderType: type) type {
         /// loop iterates through one or more entries until it collects a all
         /// file attributes.
         pub fn next(self: *Self) !?File {
-            self.initFile();
+            if (self.unread_file_bytes > 0) {
+                // If file content was not consumed by caller
+                try self.reader.skipBytes(self.unread_file_bytes, .{});
+                self.unread_file_bytes = 0;
+            }
+            var file: File = self.newFile();
             while (try self.readHeader()) |header| {
                 const kind = header.kind();
 @@ -360,46 +376,52 @@ fn Iterator(comptime ReaderType: type) type {
                 switch (kind) {
                     // File types to retrun upstream
                     .directory, .normal, .symbolic_link => {
-                        self.file.kind = kind;
+                        file.kind = switch (kind) {
-                        self.file.mode = try header.mode();
+                            .directory => .directory,
+                            .normal => .file,
+                            .symbolic_link => .sym_link,
+                            else => unreachable,
+                        };
+                        file.mode = try header.mode();
                         // set file attributes if not already set by prefix/extended headers
-                        if (self.file.size == 0) {
+                        if (file.size == 0) {
-                            self.file.size = size;
+                            file.size = size;
                         }
-                        if (self.file.link_name.len == 0) {
+                        if (file.link_name.len == 0) {
-                            self.file.link_name = try header.linkName(self.link_name_buffer);
+                            file.link_name = try header.linkName(self.link_name_buffer);
                         }
-                        if (self.file.name.len == 0) {
+                        if (file.name.len == 0) {
-                            self.file.name = try header.fullName(self.file_name_buffer);
+                            file.name = try header.fullName(self.file_name_buffer);
                         }
-                        self.padding = blockPadding(self.file.size);
+                        self.padding = blockPadding(file.size);
-                        return self.file;
+                        self.unread_file_bytes = file.size;
+                        return file;
                     },
                     // Prefix header types
                     .gnu_long_name => {
-                        self.file.name = try self.readString(@intCast(size), self.file_name_buffer);
+                        file.name = try self.readString(@intCast(size), self.file_name_buffer);
                     },
                     .gnu_long_link => {
-                        self.file.link_name = try self.readString(@intCast(size), self.link_name_buffer);
+                        file.link_name = try self.readString(@intCast(size), self.link_name_buffer);
                     },
                     .extended_header => {
                         // Use just attributes from last extended header.
-                        self.initFile();
+                        file = self.newFile();
                         var rdr = paxIterator(self.reader, @intCast(size));
                         while (try rdr.next()) |attr| {
                             switch (attr.kind) {
                                 .path => {
-                                    self.file.name = try attr.value(self.file_name_buffer);
+                                    file.name = try attr.value(self.file_name_buffer);
                                 },
                                 .linkpath => {
-                                    self.file.link_name = try attr.value(self.link_name_buffer);
+                                    file.link_name = try attr.value(self.link_name_buffer);
                                 },
                                 .size => {
                                     var buf: [pax_max_size_attr_len]u8 = undefined;
-                                    self.file.size = try std.fmt.parseInt(u64, try attr.value(&buf), 10);
+                                    file.size = try std.fmt.parseInt(u64, try attr.value(&buf), 10);
                                 },
                             }
                         }
 @@ -574,24 +596,23 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
                     try dir.makePath(file_name);
                 }
             },
-            .normal => {
+            .file => {
                 if (file.size == 0 and file.name.len == 0) return;
                 const file_name = stripComponents(file.name, options.strip_components);
                 if (file_name.len == 0) return error.BadFileName;
                 if (createDirAndFile(dir, file_name)) |fs_file| {
                     defer fs_file.close();
-                    try file.write(fs_file);
+                    try file.writeAll(fs_file);
                 } else |err| {
                     const d = options.diagnostics orelse return err;
                     try d.errors.append(d.allocator, .{ .unable_to_create_file = .{
                         .code = err,
                         .file_name = try d.allocator.dupe(u8, file_name),
                     } });
-                    try file.skip();
                 }
             },
-            .symbolic_link => {
+            .sym_link => {
                 // The file system path of the symbolic link.
                 const file_name = stripComponents(file.name, options.strip_components);
                 if (file_name.len == 0) return error.BadFileName;
 @@ -607,7 +628,6 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
                     } });
                 };
             },
-            else => unreachable,
         }
     }
 }

lib/std/tar/test.zig+12-10

...	@@ -8,7 +8,7 @@ const Case = struct {	...	@@ -8,7 +8,7 @@ const Case = struct {
8	size: u64 = 0,	8	size: u64 = 0,
9	mode: u32 = 0,	9	mode: u32 = 0,
10	link_name: []const u8 = &[0]u8{},	10	link_name: []const u8 = &[0]u8{},
11	kind: tar.Header.Kind = .normal,	11	kind: tar.FileKind = .file,
12	truncated: bool = false, // when there is no file body, just header, usefull for huge files	12	truncated: bool = false, // when there is no file body, just header, usefull for huge files
13	};	13	};
14		14
...	@@ -91,7 +91,7 @@ const cases = [_]Case{	...	@@ -91,7 +91,7 @@ const cases = [_]Case{
91	.{	91	.{
92	.name = "a/b",	92	.name = "a/b",
93	.size = 0,	93	.size = 0,
94	.kind = .symbolic_link,	94	.kind = .sym_link,
95	.mode = 0o777,	95	.mode = 0o777,
96	.link_name = "123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100",	96	.link_name = "123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100",
97	},	97	},
...	@@ -112,7 +112,7 @@ const cases = [_]Case{	...	@@ -112,7 +112,7 @@ const cases = [_]Case{
112	.{	112	.{
113	.name = "foo",	113	.name = "foo",
114	.size = 999,	114	.size = 999,
115	.kind = .normal,	115	.kind = .file,
116	.mode = 0o640,	116	.mode = 0o640,
117	},	117	},
118	},	118	},
...	@@ -153,7 +153,7 @@ const cases = [_]Case{	...	@@ -153,7 +153,7 @@ const cases = [_]Case{
153	.{	153	.{
154	.name = "P1050238.JPG.log",	154	.name = "P1050238.JPG.log",
155	.size = 14,	155	.size = 14,
156	.kind = .normal,	156	.kind = .file,
157	.mode = 0o664,	157	.mode = 0o664,
158	},	158	},
159	},	159	},
...	@@ -168,13 +168,13 @@ const cases = [_]Case{	...	@@ -168,13 +168,13 @@ const cases = [_]Case{
168	.{	168	.{
169	.name = "small.txt",	169	.name = "small.txt",
170	.size = 5,	170	.size = 5,
171	.kind = .normal,	171	.kind = .file,
172	.mode = 0o644,	172	.mode = 0o644,
173	},	173	},
174	.{	174	.{
175	.name = "small2.txt",	175	.name = "small2.txt",
176	.size = 11,	176	.size = 11,
177	.kind = .normal,	177	.kind = .file,
178	.mode = 0o644,	178	.mode = 0o644,
179	},	179	},
180	},	180	},
...	@@ -189,7 +189,7 @@ const cases = [_]Case{	...	@@ -189,7 +189,7 @@ const cases = [_]Case{
189	.{	189	.{
190	.name = "GNU2/GNU2/long-path-name",	190	.name = "GNU2/GNU2/long-path-name",
191	.link_name = "GNU4/GNU4/long-linkpath-name",	191	.link_name = "GNU4/GNU4/long-linkpath-name",
192	.kind = .symbolic_link,	192	.kind = .sym_link,
193	},	193	},
194	},	194	},
195	},	195	},
...	@@ -205,7 +205,7 @@ const cases = [_]Case{	...	@@ -205,7 +205,7 @@ const cases = [_]Case{
205	.{	205	.{
206	.name = "bar",	206	.name = "bar",
207	.link_name = "PAX4/PAX4/long-linkpath-name",	207	.link_name = "PAX4/PAX4/long-linkpath-name",
208	.kind = .symbolic_link,	208	.kind = .sym_link,
209	},	209	},
210	},	210	},
211	},	211	},
...	@@ -369,11 +369,13 @@ test "run test cases" {	...	@@ -369,11 +369,13 @@ test "run test cases" {
369		369
370	if (case.chksums.len > i) {	370	if (case.chksums.len > i) {
371	var md5writer = Md5Writer{};	371	var md5writer = Md5Writer{};
372	try actual.write(&md5writer);	372	try actual.writeAll(&md5writer);
373	const chksum = md5writer.chksum();	373	const chksum = md5writer.chksum();
374	try testing.expectEqualStrings(case.chksums[i], &chksum);	374	try testing.expectEqualStrings(case.chksums[i], &chksum);
375	} else {	375	} else {
376	if (!expected.truncated) try actual.skip(); // skip file content	376	if (expected.truncated) {
		377	iter.unread_file_bytes = 0;
		378	}
377	}	379	}
378	}	380	}
379	try testing.expectEqual(case.files.len, i);	381	try testing.expectEqual(case.files.len, i);