Skip to content

Commit a69c446

Browse files
committed
std.tar: remove diagnostic and mode options
Diagnostic error collection has been moved to Unpack. Mode does not have implementation, and after this changes implementation will be in Unpack.
1 parent 44eb6da commit a69c446

File tree

3 files changed

+32
-125
lines changed

3 files changed

+32
-125
lines changed

lib/std/tar.zig

Lines changed: 5 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -21,70 +21,12 @@ const testing = std.testing;
2121

2222
pub const output = @import("tar/output.zig");
2323

24-
/// Provide this to receive detailed error messages.
25-
/// When this is provided, some errors which would otherwise be returned
26-
/// immediately will instead be added to this structure. The API user must check
27-
/// the errors in diagnostics to know whether the operation succeeded or failed.
28-
pub const Diagnostics = struct {
29-
allocator: std.mem.Allocator,
30-
errors: std.ArrayListUnmanaged(Error) = .{},
31-
32-
pub const Error = union(enum) {
33-
unable_to_create_sym_link: struct {
34-
code: anyerror,
35-
file_name: []const u8,
36-
link_name: []const u8,
37-
},
38-
unable_to_create_file: struct {
39-
code: anyerror,
40-
file_name: []const u8,
41-
},
42-
unsupported_file_type: struct {
43-
file_name: []const u8,
44-
file_type: Header.Kind,
45-
},
46-
};
47-
48-
pub fn deinit(d: *Diagnostics) void {
49-
for (d.errors.items) |item| {
50-
switch (item) {
51-
.unable_to_create_sym_link => |info| {
52-
d.allocator.free(info.file_name);
53-
d.allocator.free(info.link_name);
54-
},
55-
.unable_to_create_file => |info| {
56-
d.allocator.free(info.file_name);
57-
},
58-
.unsupported_file_type => |info| {
59-
d.allocator.free(info.file_name);
60-
},
61-
}
62-
}
63-
d.errors.deinit(d.allocator);
64-
d.* = undefined;
65-
}
66-
};
67-
6824
/// pipeToFileSystem options
6925
pub const PipeOptions = struct {
7026
/// Number of directory levels to skip when extracting files.
7127
strip_components: u32 = 0,
72-
/// How to handle the "mode" property of files from within the tar file.
73-
mode_mode: ModeMode = .executable_bit_only,
7428
/// Prevents creation of empty directories.
7529
exclude_empty_directories: bool = false,
76-
/// Collects error messages during unpacking
77-
diagnostics: ?*Diagnostics = null,
78-
79-
pub const ModeMode = enum {
80-
/// The mode from the tar file is completely ignored. Files are created
81-
/// with the default mode when creating files.
82-
ignore,
83-
/// The mode from the tar file is inspected for the owner executable bit
84-
/// only. This bit is copied to the group and other executable bits.
85-
/// Other bits of the mode are left as the default when creating files.
86-
executable_bit_only,
87-
};
8830
};
8931

9032
const Header = struct {
@@ -247,16 +189,13 @@ pub const IteratorOptions = struct {
247189
file_name_buffer: []u8,
248190
/// Use a buffer with length `std.fs.MAX_PATH_BYTES` to match file system capabilities.
249191
link_name_buffer: []u8,
250-
/// Collects error messages during unpacking
251-
diagnostics: ?*Diagnostics = null,
252192
};
253193

254194
/// Iterates over files in tar archive.
255195
/// `next` returns each file in tar archive.
256196
pub fn iterator(reader: anytype, options: IteratorOptions) Iterator(@TypeOf(reader)) {
257197
return .{
258198
.reader = reader,
259-
.diagnostics = options.diagnostics,
260199
.file_name_buffer = options.file_name_buffer,
261200
.link_name_buffer = options.link_name_buffer,
262201
};
@@ -273,7 +212,6 @@ pub const FileKind = enum {
273212
pub fn Iterator(comptime ReaderType: type) type {
274213
return struct {
275214
reader: ReaderType,
276-
diagnostics: ?*Diagnostics = null,
277215

278216
// buffers for heeader and file attributes
279217
header_buffer: [Header.SIZE]u8 = undefined,
@@ -435,15 +373,11 @@ pub fn Iterator(comptime ReaderType: type) type {
435373
},
436374
// All other are unsupported header types
437375
else => {
438-
const d = self.diagnostics orelse return error.TarUnsupportedHeader;
439-
try d.errors.append(d.allocator, .{ .unsupported_file_type = .{
440-
.file_name = try d.allocator.dupe(u8, header.name()),
441-
.file_type = kind,
442-
} });
443376
if (kind == .gnu_sparse) {
444377
try self.skipGnuSparseExtendedHeaders(header);
445378
}
446379
self.reader.skipBytes(size, .{}) catch return error.TarHeadersTooBig;
380+
return error.TarUnsupportedHeader;
447381
},
448382
}
449383
}
@@ -573,24 +507,11 @@ fn PaxIterator(comptime ReaderType: type) type {
573507

574508
/// Saves tar file content to the file systems.
575509
pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions) !void {
576-
switch (options.mode_mode) {
577-
.ignore => {},
578-
.executable_bit_only => {
579-
// This code does not look at the mode bits yet. To implement this feature,
580-
// the implementation must be adjusted to look at the mode, and check the
581-
// user executable bit, then call fchmod on newly created files when
582-
// the executable bit is supposed to be set.
583-
// It also needs to properly deal with ACLs on Windows.
584-
@panic("TODO: unimplemented: tar ModeMode.executable_bit_only");
585-
},
586-
}
587-
588510
var file_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
589511
var link_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
590512
var iter = iterator(reader, .{
591513
.file_name_buffer = &file_name_buffer,
592514
.link_name_buffer = &link_name_buffer,
593-
.diagnostics = options.diagnostics,
594515
});
595516
while (try iter.next()) |file| {
596517
switch (file.kind) {
@@ -605,16 +526,9 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions)
605526
const file_name = stripComponents(file.name, options.strip_components);
606527
if (file_name.len == 0) return error.BadFileName;
607528

608-
if (createDirAndFile(dir, file_name)) |fs_file| {
609-
defer fs_file.close();
610-
try file.writeAll(fs_file);
611-
} else |err| {
612-
const d = options.diagnostics orelse return err;
613-
try d.errors.append(d.allocator, .{ .unable_to_create_file = .{
614-
.code = err,
615-
.file_name = try d.allocator.dupe(u8, file_name),
616-
} });
617-
}
529+
var fs_file = try createDirAndFile(dir, file_name);
530+
defer fs_file.close();
531+
try file.writeAll(fs_file);
618532
},
619533
.sym_link => {
620534
// The file system path of the symbolic link.
@@ -623,14 +537,7 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions)
623537
// The data inside the symbolic link.
624538
const link_name = file.link_name;
625539

626-
createDirAndSymlink(dir, link_name, file_name) catch |err| {
627-
const d = options.diagnostics orelse return error.UnableToCreateSymLink;
628-
try d.errors.append(d.allocator, .{ .unable_to_create_sym_link = .{
629-
.code = err,
630-
.file_name = try d.allocator.dupe(u8, file_name),
631-
.link_name = try d.allocator.dupe(u8, link_name),
632-
} });
633-
};
540+
try createDirAndSymlink(dir, link_name, file_name);
634541
},
635542
}
636543
}
@@ -984,7 +891,6 @@ test pipeToFileSystem {
984891

985892
// Save tar from `reader` to the file system `dir`
986893
pipeToFileSystem(dir, reader, .{
987-
.mode_mode = .ignore,
988894
.strip_components = 1,
989895
.exclude_empty_directories = true,
990896
}) catch |err| {

lib/std/tar/test.zig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,14 +473,14 @@ test "should not overwrite existing file" {
473473
defer root.cleanup();
474474
try testing.expectError(
475475
error.PathAlreadyExists,
476-
tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }),
476+
tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .strip_components = 1 }),
477477
);
478478

479479
// Unpack with strip_components = 0 should pass
480480
fsb.reset();
481481
var root2 = std.testing.tmpDir(.{});
482482
defer root2.cleanup();
483-
try tar.pipeToFileSystem(root2.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 0 });
483+
try tar.pipeToFileSystem(root2.dir, fsb.reader(), .{ .strip_components = 0 });
484484
}
485485

486486
test "case sensitivity" {
@@ -499,7 +499,7 @@ test "case sensitivity" {
499499
var root = std.testing.tmpDir(.{});
500500
defer root.cleanup();
501501

502-
tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }) catch |err| {
502+
tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .strip_components = 1 }) catch |err| {
503503
// on case insensitive fs we fail on overwrite existing file
504504
try testing.expectEqual(error.PathAlreadyExists, err);
505505
return;

src/Package/Unpack.zig

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -54,36 +54,37 @@ const Self = @This();
5454

5555
pub fn tarball(self: *Self, reader: anytype) !void {
5656
const strip_components = 1;
57-
// TODO: replace with switch ignore unsupported tar file types in iterator
58-
var diagnostics: std.tar.Diagnostics = .{ .allocator = self.allocator };
59-
defer diagnostics.deinit();
6057

6158
var file_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
6259
var link_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
6360
var iter = std.tar.iterator(reader, .{
6461
.file_name_buffer = &file_name_buffer,
6562
.link_name_buffer = &link_name_buffer,
66-
.diagnostics = &diagnostics,
6763
});
68-
while (try iter.next()) |entry| {
69-
switch (entry.kind) {
70-
.directory => {}, // skip empty
71-
.file => {
72-
if (entry.size == 0 and entry.name.len == 0) continue;
73-
const file_name = stripComponents(entry.name, strip_components);
74-
if (file_name.len == 0) return error.BadFileName;
75-
if (try self.createFile(file_name)) |file| {
76-
defer file.close();
77-
try entry.writeAll(file);
78-
}
79-
},
80-
.sym_link => {
81-
const file_name = stripComponents(entry.name, strip_components);
82-
if (file_name.len == 0) return error.BadFileName;
83-
const link_name = entry.link_name;
84-
try self.symLink(link_name, file_name);
85-
},
86-
}
64+
while (true) {
65+
if (iter.next() catch |err| switch (err) {
66+
error.TarUnsupportedHeader => continue,
67+
else => return err,
68+
}) |entry| {
69+
switch (entry.kind) {
70+
.directory => {}, // skip empty
71+
.file => {
72+
if (entry.size == 0 and entry.name.len == 0) continue;
73+
const file_name = stripComponents(entry.name, strip_components);
74+
if (file_name.len == 0) return error.BadFileName;
75+
if (try self.createFile(file_name)) |file| {
76+
defer file.close();
77+
try entry.writeAll(file);
78+
}
79+
},
80+
.sym_link => {
81+
const file_name = stripComponents(entry.name, strip_components);
82+
if (file_name.len == 0) return error.BadFileName;
83+
const link_name = entry.link_name;
84+
try self.symLink(link_name, file_name);
85+
},
86+
}
87+
} else break;
8788
}
8889
}
8990

0 commit comments

Comments
 (0)