From 519ba9bb654a4d5caf7440160f018b7a3ae1e95a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 22 Nov 2023 12:35:33 -0700 Subject: [PATCH] Revert "Merge pull request #12060 from Vexu/IterableDir" This reverts commit da94227f783ec3c92859c4713b80a668f1183f96, reversing changes made to 8f943b3d33432a26b7e242c1181e4220ed400501. I was against this change originally, but decided to approve it to keep an open mind. After a year of trying it in practice, I firmly believe that the previous way of doing it was better. --- lib/std/fs.zig | 234 ++++++++++++------------------- lib/std/fs/test.zig | 68 +++++---- lib/std/os.zig | 2 +- lib/std/testing.zig | 38 ----- src/main.zig | 12 +- tools/process_headers.zig | 6 +- tools/update-license-headers.zig | 8 +- tools/update-linux-headers.zig | 6 +- tools/update_glibc.zig | 10 +- tools/update_spirv_features.zig | 6 +- 10 files changed, 146 insertions(+), 244 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 2bcf4ee309..12e1ac34ab 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -310,10 +310,8 @@ pub fn renameW(old_dir: Dir, old_sub_path_w: []const u16, new_dir: Dir, new_sub_ return os.renameatW(old_dir.fd, old_sub_path_w, new_dir.fd, new_sub_path_w); } -/// A directory that can be iterated. It is *NOT* legal to initialize this with a regular `Dir` -/// that has been opened without iteration permission. -pub const IterableDir = struct { - dir: Dir, +pub const Dir = struct { + fd: os.fd_t, pub const Entry = struct { name: []const u8, @@ -879,18 +877,18 @@ pub const IterableDir = struct { else => @compileError("unimplemented"), }; - pub fn iterate(self: IterableDir) Iterator { + pub fn iterate(self: Dir) Iterator { return self.iterateImpl(true); } /// Like `iterate`, but will not reset the directory cursor before the first /// iteration. This should only be used in cases where it is known that the - /// `IterableDir` has not had its cursor modified yet (e.g. it was just opened). - pub fn iterateAssumeFirstIteration(self: IterableDir) Iterator { + /// `Dir` has not had its cursor modified yet (e.g. it was just opened). + pub fn iterateAssumeFirstIteration(self: Dir) Iterator { return self.iterateImpl(false); } - fn iterateImpl(self: IterableDir, first_iter_start_value: bool) Iterator { + fn iterateImpl(self: Dir, first_iter_start_value: bool) Iterator { switch (builtin.os.tag) { .macos, .ios, @@ -901,7 +899,7 @@ pub const IterableDir = struct { .solaris, .illumos, => return Iterator{ - .dir = self.dir, + .dir = self, .seek = 0, .index = 0, .end_index = 0, @@ -909,14 +907,14 @@ pub const IterableDir = struct { .first_iter = first_iter_start_value, }, .linux, .haiku => return Iterator{ - .dir = self.dir, + .dir = self, .index = 0, .end_index = 0, .buf = undefined, .first_iter = first_iter_start_value, }, .windows => return Iterator{ - .dir = self.dir, + .dir = self, .index = 0, .end_index = 0, .first_iter = first_iter_start_value, @@ -924,7 +922,7 @@ pub const IterableDir = struct { .name_data = undefined, }, .wasi => return Iterator{ - .dir = self.dir, + .dir = self, .cookie = os.wasi.DIRCOOKIE_START, .index = 0, .end_index = 0, @@ -945,11 +943,11 @@ pub const IterableDir = struct { dir: Dir, basename: []const u8, path: []const u8, - kind: IterableDir.Entry.Kind, + kind: Dir.Entry.Kind, }; const StackItem = struct { - iter: IterableDir.Iterator, + iter: Dir.Iterator, dirname_len: usize, }; @@ -980,7 +978,7 @@ pub const IterableDir = struct { } try self.name_buffer.appendSlice(base.name); if (base.kind == .directory) { - var new_dir = top.iter.dir.openIterableDir(base.name, .{}) catch |err| switch (err) { + var new_dir = top.iter.dir.openDir(base.name, .{ .iterate = true }) catch |err| switch (err) { error.NameTooLong => unreachable, // no path sep in base.name else => |e| return e, }; @@ -1023,10 +1021,11 @@ pub const IterableDir = struct { }; /// Recursively iterates over a directory. + /// `self` must have been opened with `OpenDirOptions{.iterate = true}`. /// Must call `Walker.deinit` when done. /// The order of returned file system entries is undefined. /// `self` will not be closed after walking it. - pub fn walk(self: IterableDir, allocator: Allocator) !Walker { + pub fn walk(self: Dir, allocator: Allocator) !Walker { var name_buffer = std.ArrayList(u8).init(allocator); errdefer name_buffer.deinit(); @@ -1044,49 +1043,6 @@ pub const IterableDir = struct { }; } - pub fn close(self: *IterableDir) void { - self.dir.close(); - self.* = undefined; - } - - pub const ChmodError = File.ChmodError; - - /// Changes the mode of the directory. - /// The process must have the correct privileges in order to do this - /// successfully, or must have the effective user ID matching the owner - /// of the directory. - pub fn chmod(self: IterableDir, new_mode: File.Mode) ChmodError!void { - const file: File = .{ - .handle = self.dir.fd, - .capable_io_mode = .blocking, - }; - try file.chmod(new_mode); - } - - /// Changes the owner and group of the directory. - /// The process must have the correct privileges in order to do this - /// successfully. The group may be changed by the owner of the directory to - /// any group of which the owner is a member. If the - /// owner or group is specified as `null`, the ID is not changed. - pub fn chown(self: IterableDir, owner: ?File.Uid, group: ?File.Gid) ChownError!void { - const file: File = .{ - .handle = self.dir.fd, - .capable_io_mode = .blocking, - }; - try file.chown(owner, group); - } - - pub const ChownError = File.ChownError; -}; - -pub const Dir = struct { - fd: os.fd_t, - - pub const iterate = @compileError("only 'IterableDir' can be iterated; 'IterableDir' can be obtained with 'openIterableDir'"); - pub const walk = @compileError("only 'IterableDir' can be walked; 'IterableDir' can be obtained with 'openIterableDir'"); - pub const chmod = @compileError("only 'IterableDir' can have its mode changed; 'IterableDir' can be obtained with 'openIterableDir'"); - pub const chown = @compileError("only 'IterableDir' can have its owner changed; 'IterableDir' can be obtained with 'openIterableDir'"); - pub const OpenError = error{ FileNotFound, NotDir, @@ -1529,7 +1485,8 @@ pub const Dir = struct { .windows => { const w = os.windows; const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | - w.SYNCHRONIZE | w.FILE_TRAVERSE; + w.SYNCHRONIZE | w.FILE_TRAVERSE | + (if (open_dir_options.iterate) w.FILE_LIST_DIRECTORY else 0); return self.makeOpenPathAccessMaskW(sub_path, base_flags, open_dir_options.no_follow); }, @@ -1545,32 +1502,6 @@ pub const Dir = struct { }; } - /// This function performs `makePath`, followed by `openIterableDir`. - /// If supported by the OS, this operation is atomic. It is not atomic on - /// all operating systems. - pub fn makeOpenPathIterable(self: Dir, sub_path: []const u8, open_dir_options: OpenDirOptions) !IterableDir { - return switch (builtin.os.tag) { - .windows => { - const w = os.windows; - const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | - w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY; - - return IterableDir{ - .dir = try self.makeOpenPathAccessMaskW(sub_path, base_flags, open_dir_options.no_follow), - }; - }, - else => { - return self.openIterableDir(sub_path, open_dir_options) catch |err| switch (err) { - error.FileNotFound => { - try self.makePath(sub_path); - return self.openIterableDir(sub_path, open_dir_options); - }, - else => |e| return e, - }; - }, - }; - } - /// This function returns the canonicalized absolute pathname of /// `pathname` relative to this `Dir`. If `pathname` is absolute, ignores this /// `Dir` handle and returns the canonicalized absolute pathname of `pathname` @@ -1706,39 +1637,28 @@ pub const Dir = struct { /// such operations are Illegal Behavior. access_sub_paths: bool = true, + /// `true` means the opened directory can be scanned for the files and sub-directories + /// of the result. It means the `iterate` function can be called. + iterate: bool = false, + /// `true` means it won't dereference the symlinks. no_follow: bool = false, }; /// Opens a directory at the given path. The directory is a system resource that remains /// open until `close` is called on the result. + /// The directory cannot be iterated unless the `iterate` option is set to `true`. /// /// Asserts that the path parameter has no null bytes. pub fn openDir(self: Dir, sub_path: []const u8, args: OpenDirOptions) OpenError!Dir { if (builtin.os.tag == .windows) { const sub_path_w = try os.windows.sliceToPrefixedFileW(self.fd, sub_path); - return self.openDirW(sub_path_w.span().ptr, args, false); + return .{ .dir = try self.openDirW(sub_path_w.span().ptr, args) }; } else if (builtin.os.tag == .wasi and !builtin.link_libc) { - return self.openDirWasi(sub_path, args); + return .{ .dir = try self.openDirWasi(sub_path, args) }; } else { const sub_path_c = try os.toPosixPath(sub_path); - return self.openDirZ(&sub_path_c, args, false); - } - } - - /// Opens an iterable directory at the given path. The directory is a system resource that remains - /// open until `close` is called on the result. - /// - /// Asserts that the path parameter has no null bytes. - pub fn openIterableDir(self: Dir, sub_path: []const u8, args: OpenDirOptions) OpenError!IterableDir { - if (builtin.os.tag == .windows) { - const sub_path_w = try os.windows.sliceToPrefixedFileW(self.fd, sub_path); - return IterableDir{ .dir = try self.openDirW(sub_path_w.span().ptr, args, true) }; - } else if (builtin.os.tag == .wasi and !builtin.link_libc) { - return IterableDir{ .dir = try self.openDirWasi(sub_path, args) }; - } else { - const sub_path_c = try os.toPosixPath(sub_path); - return IterableDir{ .dir = try self.openDirZ(&sub_path_c, args, true) }; + return .{ .dir = try self.openDirZ(&sub_path_c, args) }; } } @@ -1790,13 +1710,13 @@ pub const Dir = struct { } /// Same as `openDir` except the parameter is null-terminated. - pub fn openDirZ(self: Dir, sub_path_c: [*:0]const u8, args: OpenDirOptions, iterable: bool) OpenError!Dir { + pub fn openDirZ(self: Dir, sub_path_c: [*:0]const u8, args: OpenDirOptions) OpenError!Dir { if (builtin.os.tag == .windows) { const sub_path_w = try os.windows.cStrToPrefixedFileW(self.fd, sub_path_c); - return self.openDirW(sub_path_w.span().ptr, args, iterable); + return self.openDirW(sub_path_w.span().ptr, args); } const symlink_flags: u32 = if (args.no_follow) os.O.NOFOLLOW else 0x0; - if (!iterable) { + if (!args.iterate) { const O_PATH = if (@hasDecl(os.O, "PATH")) os.O.PATH else 0; return self.openDirFlagsZ(sub_path_c, os.O.DIRECTORY | os.O.RDONLY | os.O.CLOEXEC | O_PATH | symlink_flags); } else { @@ -1806,12 +1726,12 @@ pub const Dir = struct { /// Same as `openDir` except the path parameter is WTF-16 encoded, NT-prefixed. /// This function asserts the target OS is Windows. - pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16, args: OpenDirOptions, iterable: bool) OpenError!Dir { + pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16, args: OpenDirOptions) OpenError!Dir { const w = os.windows; // TODO remove some of these flags if args.access_sub_paths is false const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE; - const flags: u32 = if (iterable) base_flags | w.FILE_LIST_DIRECTORY else base_flags; + const flags: u32 = if (args.iterate) base_flags | w.FILE_LIST_DIRECTORY else base_flags; const dir = try self.makeOpenDirAccessMaskW(sub_path_w, flags, .{ .no_follow = args.no_follow, .create_disposition = w.FILE_OPEN, @@ -2203,7 +2123,7 @@ pub const Dir = struct { const StackItem = struct { name: []const u8, parent_dir: Dir, - iter: IterableDir.Iterator, + iter: Dir.Iterator, fn closeAll(items: []@This()) void { for (items) |*item| item.iter.dir.close(); @@ -2227,7 +2147,10 @@ pub const Dir = struct { handle_entry: while (true) { if (treat_as_dir) { if (stack.unusedCapacitySlice().len >= 1) { - var iterable_dir = top.iter.dir.openIterableDir(entry.name, .{ .no_follow = true }) catch |err| switch (err) { + var iterable_dir = top.iter.dir.openDir(entry.name, .{ + .no_follow = true, + .iterate = true, + }) catch |err| switch (err) { error.NotDir => { treat_as_dir = false; continue :handle_entry; @@ -2318,7 +2241,10 @@ pub const Dir = struct { var treat_as_dir = true; handle_entry: while (true) { if (treat_as_dir) { - break :iterable_dir parent_dir.openIterableDir(name, .{ .no_follow = true }) catch |err| switch (err) { + break :iterable_dir parent_dir.openDir(name, .{ + .no_follow = true, + .iterate = true, + }) catch |err| switch (err) { error.NotDir => { treat_as_dir = false; continue :handle_entry; @@ -2393,12 +2319,12 @@ pub const Dir = struct { fn deleteTreeMinStackSizeWithKindHint(self: Dir, sub_path: []const u8, kind_hint: File.Kind) DeleteTreeError!void { start_over: while (true) { - var iterable_dir = (try self.deleteTreeOpenInitialSubpath(sub_path, kind_hint)) orelse return; - var cleanup_dir_parent: ?IterableDir = null; + var dir = (try self.deleteTreeOpenInitialSubpath(sub_path, kind_hint)) orelse return; + var cleanup_dir_parent: ?Dir = null; defer if (cleanup_dir_parent) |*d| d.close(); var cleanup_dir = true; - defer if (cleanup_dir) iterable_dir.close(); + defer if (cleanup_dir) dir.close(); // Valid use of MAX_PATH_BYTES because dir_name_buf will only // ever store a single path component that was returned from the @@ -2411,12 +2337,15 @@ pub const Dir = struct { // open it, and close the original directory. Repeat. Then start the entire operation over. scan_dir: while (true) { - var dir_it = iterable_dir.iterateAssumeFirstIteration(); + var dir_it = dir.iterateAssumeFirstIteration(); dir_it: while (try dir_it.next()) |entry| { var treat_as_dir = entry.kind == .directory; handle_entry: while (true) { if (treat_as_dir) { - const new_dir = iterable_dir.dir.openIterableDir(entry.name, .{ .no_follow = true }) catch |err| switch (err) { + const new_dir = dir.openDir(entry.name, .{ + .no_follow = true, + .iterate = true, + }) catch |err| switch (err) { error.NotDir => { treat_as_dir = false; continue :handle_entry; @@ -2442,14 +2371,14 @@ pub const Dir = struct { => |e| return e, }; if (cleanup_dir_parent) |*d| d.close(); - cleanup_dir_parent = iterable_dir; - iterable_dir = new_dir; + cleanup_dir_parent = dir; + dir = new_dir; const result = dir_name_buf[0..entry.name.len]; @memcpy(result, entry.name); dir_name = result; continue :scan_dir; } else { - if (iterable_dir.dir.deleteFile(entry.name)) { + if (dir.deleteFile(entry.name)) { continue :dir_it; } else |err| switch (err) { error.FileNotFound => continue :dir_it, @@ -2480,11 +2409,11 @@ pub const Dir = struct { } // Reached the end of the directory entries, which means we successfully deleted all of them. // Now to remove the directory itself. - iterable_dir.close(); + dir.close(); cleanup_dir = false; if (cleanup_dir_parent) |d| { - d.dir.deleteDir(dir_name) catch |err| switch (err) { + d.deleteDir(dir_name) catch |err| switch (err) { // These two things can happen due to file system race conditions. error.FileNotFound, error.DirNotEmpty => continue :start_over, else => |e| return e, @@ -2503,14 +2432,17 @@ pub const Dir = struct { } /// On successful delete, returns null. - fn deleteTreeOpenInitialSubpath(self: Dir, sub_path: []const u8, kind_hint: File.Kind) !?IterableDir { + fn deleteTreeOpenInitialSubpath(self: Dir, sub_path: []const u8, kind_hint: File.Kind) !?Dir { return iterable_dir: { // Treat as a file by default var treat_as_dir = kind_hint == .directory; handle_entry: while (true) { if (treat_as_dir) { - break :iterable_dir self.openIterableDir(sub_path, .{ .no_follow = true }) catch |err| switch (err) { + break :iterable_dir self.openDir(sub_path, .{ + .no_follow = true, + .iterate = true, + }) catch |err| switch (err) { error.NotDir => { treat_as_dir = false; continue :handle_entry; @@ -2764,6 +2696,37 @@ pub const Dir = struct { return Stat.fromSystem(st); } + pub const ChmodError = File.ChmodError; + + /// Changes the mode of the directory. + /// The process must have the correct privileges in order to do this + /// successfully, or must have the effective user ID matching the owner + /// of the directory. Additionally, the directory must have been opened + /// with `OpenDirOptions{ .iterate = true }`. + pub fn chmod(self: Dir, new_mode: File.Mode) ChmodError!void { + const file: File = .{ + .handle = self.fd, + .capable_io_mode = .blocking, + }; + try file.chmod(new_mode); + } + + /// Changes the owner and group of the directory. + /// The process must have the correct privileges in order to do this + /// successfully. The group may be changed by the owner of the directory to + /// any group of which the owner is a member. Additionally, the directory + /// must have been opened with `OpenDirOptions{ .iterate = true }`. If the + /// owner or group is specified as `null`, the ID is not changed. + pub fn chown(self: Dir, owner: ?File.Uid, group: ?File.Gid) ChownError!void { + const file: File = .{ + .handle = self.fd, + .capable_io_mode = .blocking, + }; + try file.chown(owner, group); + } + + pub const ChownError = File.ChownError; + const Permissions = File.Permissions; pub const SetPermissionsError = File.SetPermissionsError; @@ -2829,27 +2792,6 @@ pub fn openDirAbsoluteW(absolute_path_c: [*:0]const u16, flags: Dir.OpenDirOptio return cwd().openDirW(absolute_path_c, flags, false); } -/// Opens a directory at the given path. The directory is a system resource that remains -/// open until `close` is called on the result. -/// See `openIterableDirAbsoluteZ` for a function that accepts a null-terminated path. -/// -/// Asserts that the path parameter has no null bytes. -pub fn openIterableDirAbsolute(absolute_path: []const u8, flags: Dir.OpenDirOptions) File.OpenError!IterableDir { - assert(path.isAbsolute(absolute_path)); - return cwd().openIterableDir(absolute_path, flags); -} - -/// Same as `openIterableDirAbsolute` but the path parameter is null-terminated. -pub fn openIterableDirAbsoluteZ(absolute_path_c: [*:0]const u8, flags: Dir.OpenDirOptions) File.OpenError!IterableDir { - assert(path.isAbsoluteZ(absolute_path_c)); - return IterableDir{ .dir = try cwd().openDirZ(absolute_path_c, flags, true) }; -} -/// Same as `openIterableDirAbsolute` but the path parameter is null-terminated. -pub fn openIterableDirAbsoluteW(absolute_path_c: [*:0]const u16, flags: Dir.OpenDirOptions) File.OpenError!IterableDir { - assert(path.isAbsoluteWindowsW(absolute_path_c)); - return IterableDir{ .dir = try cwd().openDirW(absolute_path_c, flags, true) }; -} - /// Opens a file for reading or writing, without attempting to create a new file, based on an absolute path. /// Call `File.close` to release the resource. /// Asserts that the path is absolute. See `Dir.openFile` for a function that diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 6786aec4b0..fcf978d307 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -8,10 +8,8 @@ const wasi = std.os.wasi; const ArenaAllocator = std.heap.ArenaAllocator; const Dir = std.fs.Dir; -const IterableDir = std.fs.IterableDir; const File = std.fs.File; const tmpDir = testing.tmpDir; -const tmpIterableDir = testing.tmpIterableDir; const PathType = enum { relative, @@ -76,11 +74,11 @@ const TestContext = struct { arena: ArenaAllocator, tmp: testing.TmpIterableDir, dir: std.fs.Dir, - iterable_dir: std.fs.IterableDir, + iterable_dir: std.fs.Dir, transform_fn: *const PathType.TransformFn, pub fn init(path_type: PathType, allocator: mem.Allocator, transform_fn: *const PathType.TransformFn) TestContext { - const tmp = tmpIterableDir(.{}); + const tmp = tmpDir(.{ .iterate = true }); return .{ .path_type = path_type, .arena = ArenaAllocator.init(allocator), @@ -323,28 +321,28 @@ fn testReadLinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void } test "Dir.Iterator" { - var tmp_dir = tmpIterableDir(.{}); + var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); // First, create a couple of entries to iterate over. - const file = try tmp_dir.iterable_dir.dir.createFile("some_file", .{}); + const file = try tmp_dir.dir.createFile("some_file", .{}); file.close(); - try tmp_dir.iterable_dir.dir.makeDir("some_dir"); + try tmp_dir.dir.makeDir("some_dir"); var arena = ArenaAllocator.init(testing.allocator); defer arena.deinit(); const allocator = arena.allocator(); - var entries = std.ArrayList(IterableDir.Entry).init(allocator); + var entries = std.ArrayList(Dir.Entry).init(allocator); // Create iterator. - var iter = tmp_dir.iterable_dir.iterate(); + var iter = tmp_dir.dir.iterate(); while (try iter.next()) |entry| { // We cannot just store `entry` as on Windows, we're re-using the name buffer // which means we'll actually share the `name` pointer between entries! const name = try allocator.dupe(u8, entry.name); - try entries.append(.{ .name = name, .kind = entry.kind }); + try entries.append(Dir.Entry{ .name = name, .kind = entry.kind }); } try testing.expectEqual(@as(usize, 2), entries.items.len); // note that the Iterator skips '.' and '..' @@ -353,7 +351,7 @@ test "Dir.Iterator" { } test "Dir.Iterator many entries" { - var tmp_dir = tmpIterableDir(.{}); + var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); const num = 1024; @@ -369,7 +367,7 @@ test "Dir.Iterator many entries" { defer arena.deinit(); const allocator = arena.allocator(); - var entries = std.ArrayList(IterableDir.Entry).init(allocator); + var entries = std.ArrayList(Dir.Entry).init(allocator); // Create iterator. var iter = tmp_dir.iterable_dir.iterate(); @@ -388,14 +386,14 @@ test "Dir.Iterator many entries" { } test "Dir.Iterator twice" { - var tmp_dir = tmpIterableDir(.{}); + var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); // First, create a couple of entries to iterate over. - const file = try tmp_dir.iterable_dir.dir.createFile("some_file", .{}); + const file = try tmp_dir.dir.createFile("some_file", .{}); file.close(); - try tmp_dir.iterable_dir.dir.makeDir("some_dir"); + try tmp_dir.dir.makeDir("some_dir"); var arena = ArenaAllocator.init(testing.allocator); defer arena.deinit(); @@ -403,15 +401,15 @@ test "Dir.Iterator twice" { var i: u8 = 0; while (i < 2) : (i += 1) { - var entries = std.ArrayList(IterableDir.Entry).init(allocator); + var entries = std.ArrayList(Dir.Entry).init(allocator); // Create iterator. - var iter = tmp_dir.iterable_dir.iterate(); + var iter = tmp_dir.dir.iterate(); while (try iter.next()) |entry| { // We cannot just store `entry` as on Windows, we're re-using the name buffer // which means we'll actually share the `name` pointer between entries! const name = try allocator.dupe(u8, entry.name); - try entries.append(.{ .name = name, .kind = entry.kind }); + try entries.append(Dir.Entry{ .name = name, .kind = entry.kind }); } try testing.expectEqual(@as(usize, 2), entries.items.len); // note that the Iterator skips '.' and '..' @@ -421,7 +419,7 @@ test "Dir.Iterator twice" { } test "Dir.Iterator reset" { - var tmp_dir = tmpIterableDir(.{}); + var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); // First, create a couple of entries to iterate over. @@ -439,7 +437,7 @@ test "Dir.Iterator reset" { var i: u8 = 0; while (i < 2) : (i += 1) { - var entries = std.ArrayList(IterableDir.Entry).init(allocator); + var entries = std.ArrayList(Dir.Entry).init(allocator); while (try iter.next()) |entry| { // We cannot just store `entry` as on Windows, we're re-using the name buffer @@ -485,11 +483,11 @@ test "Dir.Iterator but dir is deleted during iteration" { } } -fn entryEql(lhs: IterableDir.Entry, rhs: IterableDir.Entry) bool { +fn entryEql(lhs: Dir.Entry, rhs: Dir.Entry) bool { return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; } -fn contains(entries: *const std.ArrayList(IterableDir.Entry), el: IterableDir.Entry) bool { +fn contains(entries: *const std.ArrayList(Dir.Entry), el: Dir.Entry) bool { for (entries.items) |entry| { if (entryEql(entry, el)) return true; } @@ -963,7 +961,7 @@ test "makePath in a directory that no longer exists" { try testing.expectError(error.FileNotFound, tmp.dir.makePath("sub-path")); } -fn testFilenameLimits(iterable_dir: IterableDir, maxed_filename: []const u8) !void { +fn testFilenameLimits(iterable_dir: Dir, maxed_filename: []const u8) !void { // setup, create a dir and a nested file both with maxed filenames, and walk the dir { var maxed_dir = try iterable_dir.dir.makeOpenPath(maxed_filename, .{}); @@ -987,7 +985,7 @@ fn testFilenameLimits(iterable_dir: IterableDir, maxed_filename: []const u8) !vo } test "max file name component lengths" { - var tmp = tmpIterableDir(.{}); + var tmp = tmpDir(.{ .iterate = true }); defer tmp.cleanup(); if (builtin.os.tag == .windows) { @@ -1384,7 +1382,7 @@ test "open file with exclusive nonblocking lock twice (absolute paths)" { test "walker" { if (builtin.os.tag == .wasi and builtin.link_libc) return error.SkipZigTest; - var tmp = tmpIterableDir(.{}); + var tmp = tmpDir(.{ .iterate = true }); defer tmp.cleanup(); // iteration order of walker is undefined, so need lookup maps to check against @@ -1410,10 +1408,10 @@ test "walker" { }); for (expected_paths.kvs) |kv| { - try tmp.iterable_dir.dir.makePath(kv.key); + try tmp.dir.makePath(kv.key); } - var walker = try tmp.iterable_dir.walk(testing.allocator); + var walker = try tmp.dir.walk(testing.allocator); defer walker.deinit(); var num_walked: usize = 0; @@ -1437,7 +1435,7 @@ test "walker" { test "walker without fully iterating" { if (builtin.os.tag == .wasi and builtin.link_libc) return error.SkipZigTest; - var tmp = tmpIterableDir(.{}); + var tmp = tmpDir(.{ .iterate = true }); defer tmp.cleanup(); var walker = try tmp.iterable_dir.walk(testing.allocator); @@ -1556,11 +1554,11 @@ test "chmod" { try testing.expectEqual(@as(File.Mode, 0o644), (try file.stat()).mode & 0o7777); try tmp.dir.makeDir("test_dir"); - var iterable_dir = try tmp.dir.openIterableDir("test_dir", .{}); - defer iterable_dir.close(); + var dir = try tmp.dir.openDir("test_dir", .{ .iterate = true }); + defer dir.close(); - try iterable_dir.chmod(0o700); - try testing.expectEqual(@as(File.Mode, 0o700), (try iterable_dir.dir.stat()).mode & 0o7777); + try dir.chmod(0o700); + try testing.expectEqual(@as(File.Mode, 0o700), (try dir.stat()).mode & 0o7777); } test "chown" { @@ -1576,9 +1574,9 @@ test "chown" { try tmp.dir.makeDir("test_dir"); - var iterable_dir = try tmp.dir.openIterableDir("test_dir", .{}); - defer iterable_dir.close(); - try iterable_dir.chown(null, null); + var dir = try tmp.dir.openDir("test_dir", .{ .iterate = true }); + defer dir.close(); + try dir.chown(null, null); } test "File.Metadata" { diff --git a/lib/std/os.zig b/lib/std/os.zig index cf388f76e9..10697b970f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -402,7 +402,7 @@ pub fn fchown(fd: fd_t, owner: ?uid_t, group: ?gid_t) FChownError!void { switch (system.getErrno(res)) { .SUCCESS => return, .INTR => continue, - .BADF => unreachable, // Can be reached if the fd refers to a non-iterable directory. + .BADF => unreachable, // Can be reached if the fd refers to a directory opened without `OpenDirOptions{ .iterate = true }` .FAULT => unreachable, .INVAL => unreachable, diff --git a/lib/std/testing.zig b/lib/std/testing.zig index 09507a2392..a26b6ff321 100644 --- a/lib/std/testing.zig +++ b/lib/std/testing.zig @@ -543,22 +543,6 @@ pub const TmpDir = struct { } }; -pub const TmpIterableDir = struct { - iterable_dir: std.fs.IterableDir, - parent_dir: std.fs.Dir, - sub_path: [sub_path_len]u8, - - const random_bytes_count = 12; - const sub_path_len = std.fs.base64_encoder.calcSize(random_bytes_count); - - pub fn cleanup(self: *TmpIterableDir) void { - self.iterable_dir.close(); - self.parent_dir.deleteTree(&self.sub_path) catch {}; - self.parent_dir.close(); - self.* = undefined; - } -}; - pub fn tmpDir(opts: std.fs.Dir.OpenDirOptions) TmpDir { var random_bytes: [TmpDir.random_bytes_count]u8 = undefined; std.crypto.random.bytes(&random_bytes); @@ -581,28 +565,6 @@ pub fn tmpDir(opts: std.fs.Dir.OpenDirOptions) TmpDir { }; } -pub fn tmpIterableDir(opts: std.fs.Dir.OpenDirOptions) TmpIterableDir { - var random_bytes: [TmpIterableDir.random_bytes_count]u8 = undefined; - std.crypto.random.bytes(&random_bytes); - var sub_path: [TmpIterableDir.sub_path_len]u8 = undefined; - _ = std.fs.base64_encoder.encode(&sub_path, &random_bytes); - - const cwd = std.fs.cwd(); - var cache_dir = cwd.makeOpenPath("zig-cache", .{}) catch - @panic("unable to make tmp dir for testing: unable to make and open zig-cache dir"); - defer cache_dir.close(); - const parent_dir = cache_dir.makeOpenPath("tmp", .{}) catch - @panic("unable to make tmp dir for testing: unable to make and open zig-cache/tmp dir"); - const dir = parent_dir.makeOpenPathIterable(&sub_path, opts) catch - @panic("unable to make tmp dir for testing: unable to make and open the tmp dir"); - - return .{ - .iterable_dir = dir, - .parent_dir = parent_dir, - .sub_path = sub_path, - }; -} - test "expectEqual nested array" { const a = [2][2]f32{ [_]f32{ 1.0, 0.0 }, diff --git a/src/main.zig b/src/main.zig index 8a01ef4cdb..641dd04164 100644 --- a/src/main.zig +++ b/src/main.zig @@ -5698,13 +5698,13 @@ fn fmtPathDir( parent_dir: fs.Dir, parent_sub_path: []const u8, ) FmtError!void { - var iterable_dir = try parent_dir.openIterableDir(parent_sub_path, .{}); - defer iterable_dir.close(); + var dir = try parent_dir.openDir(parent_sub_path, .{ .iterate = true }); + defer dir.close(); - const stat = try iterable_dir.dir.stat(); + const stat = try dir.stat(); if (try fmt.seen.fetchPut(stat.inode, {})) |_| return; - var dir_it = iterable_dir.iterate(); + var dir_it = dir.iterate(); while (try dir_it.next()) |entry| { const is_dir = entry.kind == .directory; @@ -5715,9 +5715,9 @@ fn fmtPathDir( defer fmt.gpa.free(full_path); if (is_dir) { - try fmtPathDir(fmt, full_path, check_mode, iterable_dir.dir, entry.name); + try fmtPathDir(fmt, full_path, check_mode, dir, entry.name); } else { - fmtPathFile(fmt, full_path, check_mode, iterable_dir.dir, entry.name) catch |err| { + fmtPathFile(fmt, full_path, check_mode, dir, entry.name) catch |err| { warn("unable to format '{s}': {s}", .{ full_path, @errorName(err) }); fmt.any_error = true; return; diff --git a/tools/process_headers.zig b/tools/process_headers.zig index ea08d4de40..576e894f98 100644 --- a/tools/process_headers.zig +++ b/tools/process_headers.zig @@ -382,14 +382,14 @@ pub fn main() !void { try dir_stack.append(target_include_dir); while (dir_stack.popOrNull()) |full_dir_name| { - var iterable_dir = std.fs.cwd().openIterableDir(full_dir_name, .{}) catch |err| switch (err) { + var dir = std.fs.cwd().openDir(full_dir_name, .{ .iterate = true }) catch |err| switch (err) { error.FileNotFound => continue :search, error.AccessDenied => continue :search, else => return err, }; - defer iterable_dir.close(); + defer dir.close(); - var dir_it = iterable_dir.iterate(); + var dir_it = dir.iterate(); while (try dir_it.next()) |entry| { const full_path = try std.fs.path.join(allocator, &[_][]const u8{ full_dir_name, entry.name }); diff --git a/tools/update-license-headers.zig b/tools/update-license-headers.zig index 7756cf898c..b2aed7ccdb 100644 --- a/tools/update-license-headers.zig +++ b/tools/update-license-headers.zig @@ -14,9 +14,9 @@ pub fn main() !void { const args = try std.process.argsAlloc(arena); const path_to_walk = args[1]; - const iterable_dir = try std.fs.cwd().openIterableDir(path_to_walk, .{}); + const dir = try std.fs.cwd().openDir(path_to_walk, .{ .iterate = true }); - var walker = try iterable_dir.walk(arena); + var walker = try dir.walk(arena); defer walker.deinit(); var buffer: [500]u8 = undefined; @@ -30,7 +30,7 @@ pub fn main() !void { node.activate(); defer node.end(); - const source = try iterable_dir.dir.readFileAlloc(arena, entry.path, 20 * 1024 * 1024); + const source = try dir.readFileAlloc(arena, entry.path, 20 * 1024 * 1024); if (!std.mem.startsWith(u8, source, expected_header)) { std.debug.print("no match: {s}\n", .{entry.path}); continue; @@ -42,6 +42,6 @@ pub fn main() !void { std.mem.copy(u8, new_source, new_header); std.mem.copy(u8, new_source[new_header.len..], truncated_source); - try iterable_dir.dir.writeFile(entry.path, new_source); + try dir.writeFile(entry.path, new_source); } } diff --git a/tools/update-linux-headers.zig b/tools/update-linux-headers.zig index ef701fc86d..91032618c9 100644 --- a/tools/update-linux-headers.zig +++ b/tools/update-linux-headers.zig @@ -190,14 +190,14 @@ pub fn main() !void { try dir_stack.append(target_include_dir); while (dir_stack.popOrNull()) |full_dir_name| { - var iterable_dir = std.fs.cwd().openIterableDir(full_dir_name, .{}) catch |err| switch (err) { + var dir = std.fs.cwd().openDir(full_dir_name, .{ .iterate = true }) catch |err| switch (err) { error.FileNotFound => continue :search, error.AccessDenied => continue :search, else => return err, }; - defer iterable_dir.close(); + defer dir.close(); - var dir_it = iterable_dir.iterate(); + var dir_it = dir.iterate(); while (try dir_it.next()) |entry| { const full_path = try std.fs.path.join(arena, &[_][]const u8{ full_dir_name, entry.name }); diff --git a/tools/update_glibc.zig b/tools/update_glibc.zig index cf43736c8a..e86ce13454 100644 --- a/tools/update_glibc.zig +++ b/tools/update_glibc.zig @@ -47,7 +47,7 @@ pub fn main() !void { const dest_dir_path = try std.fmt.allocPrint(arena, "{s}/lib/libc/glibc", .{zig_src_path}); - var dest_dir = fs.cwd().openIterableDir(dest_dir_path, .{}) catch |err| { + var dest_dir = fs.cwd().openDir(dest_dir_path, .{ .iterate = true }) catch |err| { fatal("unable to open destination directory '{s}': {s}", .{ dest_dir_path, @errorName(err), }); @@ -72,14 +72,14 @@ pub fn main() !void { if (mem.endsWith(u8, entry.path, ext)) continue :walk; } - glibc_src_dir.copyFile(entry.path, dest_dir.dir, entry.path, .{}) catch |err| { + glibc_src_dir.copyFile(entry.path, dest_dir, entry.path, .{}) catch |err| { log.warn("unable to copy '{s}/{s}' to '{s}/{s}': {s}", .{ glibc_src_path, entry.path, dest_dir_path, entry.path, @errorName(err), }); if (err == error.FileNotFound) { - try dest_dir.dir.deleteFile(entry.path); + try dest_dir.deleteFile(entry.path); } }; } @@ -88,7 +88,7 @@ pub fn main() !void { // Warn about duplicated files inside glibc/include/* that can be omitted // because they are already in generic-glibc/*. - var include_dir = dest_dir.dir.openIterableDir("include", .{}) catch |err| { + var include_dir = dest_dir.openDir("include", .{ .iterate = true }) catch |err| { fatal("unable to open directory '{s}/include': {s}", .{ dest_dir_path, @errorName(err), }); @@ -125,7 +125,7 @@ pub fn main() !void { generic_glibc_path, entry.path, @errorName(e), }), }; - const glibc_include_contents = include_dir.dir.readFileAlloc( + const glibc_include_contents = include_dir.readFileAlloc( arena, entry.path, max_file_size, diff --git a/tools/update_spirv_features.zig b/tools/update_spirv_features.zig index 0c8acf5aef..0215c24507 100644 --- a/tools/update_spirv_features.zig +++ b/tools/update_spirv_features.zig @@ -226,7 +226,7 @@ pub fn main() !void { /// TODO: Unfortunately, neither repository contains a machine-readable list of extension dependencies. fn gather_extensions(allocator: Allocator, spirv_registry_root: []const u8) ![]const []const u8 { const extensions_path = try fs.path.join(allocator, &.{ spirv_registry_root, "extensions" }); - var extensions_dir = try fs.cwd().openIterableDir(extensions_path, .{}); + var extensions_dir = try fs.cwd().openDir(extensions_path, .{ .iterate = true }); defer extensions_dir.close(); var extensions = std.ArrayList([]const u8).init(allocator); @@ -235,7 +235,7 @@ fn gather_extensions(allocator: Allocator, spirv_registry_root: []const u8) ![]c while (try vendor_it.next()) |vendor_entry| { std.debug.assert(vendor_entry.kind == .directory); // If this fails, the structure of SPIRV-Registry has changed. - const vendor_dir = try extensions_dir.dir.openIterableDir(vendor_entry.name, .{}); + const vendor_dir = try extensions_dir.openDir(vendor_entry.name, .{ .iterate = true }); var ext_it = vendor_dir.iterate(); while (try ext_it.next()) |ext_entry| { // There is both a HTML and asciidoc version of every spec (as well as some other directories), @@ -258,7 +258,7 @@ fn gather_extensions(allocator: Allocator, spirv_registry_root: []const u8) ![]c // SPV_EXT_name // ``` - const ext_spec = try vendor_dir.dir.readFileAlloc(allocator, ext_entry.name, std.math.maxInt(usize)); + const ext_spec = try vendor_dir.readFileAlloc(allocator, ext_entry.name, std.math.maxInt(usize)); const name_strings = "Name Strings"; const name_strings_offset = std.mem.indexOf(u8, ext_spec, name_strings) orelse return error.InvalidRegistry;