std.fs: remove OpenDirOptions.iterate

Author: Vexu

lib/std/fs.zig

@@ -956,14 +956,11 @@ pub const IterableDir = struct {
 
 pub const Dir = struct {
     fd: os.fd_t,
-    iterable: @TypeOf(iterable_safety) = iterable_safety,
 
-    const iterable_safety = if (builtin.mode == .Debug) false else {};
-
-    pub const iterate = @compileError("only 'IterableDir' can be iterated; 'IterableDir' can be obtained with 'openIterableDir' or by opening with 'iterate = true' and using 'intoIterable'");
-    pub const walk = @compileError("only 'IterableDir' can be walked; 'IterableDir' can be obtained with 'openIterableDir' or by opening with 'iterate = true' and using 'intoIterable'");
-    pub const chmod = @compileError("only 'IterableDir' can have its mode changed; 'IterableDir' can be obtained with 'openIterableDir' or by opening with 'iterate = true' and using 'intoIterable'");
-    pub const chown = @compileError("only 'IterableDir' can have its owner changed; 'IterableDir' can be obtained with 'openIterableDir' or by opening with 'iterate = true' and using 'intoIterable'");
+    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,
@@ -1381,6 +1378,15 @@ pub const Dir = struct {
         return self.openDir(sub_path, open_dir_options);
     }
 
+    /// 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 {
+        // TODO improve this implementation on Windows; we can avoid 1 call to NtClose
+        try self.makePath(sub_path);
+        return self.openIterableDir(sub_path, open_dir_options);
+    }
+
     ///  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`
@@ -1530,10 +1536,6 @@ 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,
     };
@@ -1545,12 +1547,12 @@ pub const Dir = struct {
     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(sub_path);
-            return self.openDirW(sub_path_w.span().ptr, args);
+            return self.openDirW(sub_path_w.span().ptr, args, false);
         } else if (builtin.os.tag == .wasi and !builtin.link_libc) {
             return self.openDirWasi(sub_path, args);
         } else {
             const sub_path_c = try os.toPosixPath(sub_path);
-            return self.openDirZ(&sub_path_c, args);
+            return self.openDirZ(&sub_path_c, args, false);
         }
     }
 
@@ -1559,19 +1561,15 @@ pub const Dir = struct {
     ///
     /// Asserts that the path parameter has no null bytes.
     pub fn openIterableDir(self: Dir, sub_path: []const u8, args: OpenDirOptions) OpenError!IterableDir {
-        var adjusted_args = args;
-        adjusted_args.iterate = true;
-        const new_dir = try self.openDir(sub_path, adjusted_args);
-        return IterableDir{ .dir = new_dir };
-    }
-
-    /// Convert `self` into an iterable directory.
-    /// Asserts that `self` was opened with `iterate = true`.
-    pub fn intoIterable(self: Dir) IterableDir {
-        if (builtin.mode == .Debug) {
-            assert(self.iterable);
+        if (builtin.os.tag == .windows) {
+            const sub_path_w = try os.windows.sliceToPrefixedFileW(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 = self };
     }
 
     /// Same as `openDir` except only WASI.
@@ -1619,36 +1617,33 @@ pub const Dir = struct {
             error.FileBusy => unreachable, // can't happen for directories
             else => |e| return e,
         };
-        return Dir{ .fd = fd, .iterable = if (builtin.mode == .Debug) args.iterate else {} };
+        return Dir{ .fd = fd };
     }
 
     /// Same as `openDir` except the parameter is null-terminated.
-    pub fn openDirZ(self: Dir, sub_path_c: [*:0]const u8, args: OpenDirOptions) OpenError!Dir {
+    pub fn openDirZ(self: Dir, sub_path_c: [*:0]const u8, args: OpenDirOptions, iterable: bool) OpenError!Dir {
         if (builtin.os.tag == .windows) {
             const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c);
             return self.openDirW(sub_path_w.span().ptr, args);
         }
         const symlink_flags: u32 = if (args.no_follow) os.O.NOFOLLOW else 0x0;
-        if (!args.iterate) {
+        if (!iterable) {
             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 {
-            var dir = try self.openDirFlagsZ(sub_path_c, os.O.DIRECTORY | os.O.RDONLY | os.O.CLOEXEC | symlink_flags);
-            if (builtin.mode == .Debug) dir.iterable = true;
-            return dir;
+            return self.openDirFlagsZ(sub_path_c, os.O.DIRECTORY | os.O.RDONLY | os.O.CLOEXEC | symlink_flags);
         }
     }
 
     /// 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) OpenError!Dir {
+    pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16, args: OpenDirOptions, iterable: bool) 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 (args.iterate) base_flags | w.FILE_LIST_DIRECTORY else base_flags;
+        const flags: u32 = if (iterable) base_flags | w.FILE_LIST_DIRECTORY else base_flags;
         var dir = try self.openDirAccessMaskW(sub_path_w, flags, args.no_follow);
-        if (builtin.mode == .Debug) dir.iterable = args.iterate;
         return dir;
     }
 

lib/std/fs/test.zig

@@ -11,6 +11,7 @@ const Dir = std.fs.Dir;
 const IterableDir = std.fs.IterableDir;
 const File = std.fs.File;
 const tmpDir = testing.tmpDir;
+const tmpIterableDir = testing.tmpIterableDir;
 
 test "Dir.readLink" {
     var tmp = tmpDir(.{});
@@ -156,14 +157,14 @@ fn testReadLinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void
 }
 
 test "Dir.Iterator" {
-    var tmp_dir = tmpDir(.{ .iterate = true });
+    var tmp_dir = tmpIterableDir(.{});
     defer tmp_dir.cleanup();
 
     // First, create a couple of entries to iterate over.
-    const file = try tmp_dir.dir.createFile("some_file", .{});
+    const file = try tmp_dir.iterable_dir.dir.createFile("some_file", .{});
     file.close();
 
-    try tmp_dir.dir.makeDir("some_dir");
+    try tmp_dir.iterable_dir.dir.makeDir("some_dir");
 
     var arena = ArenaAllocator.init(testing.allocator);
     defer arena.deinit();
@@ -172,7 +173,7 @@ test "Dir.Iterator" {
     var entries = std.ArrayList(IterableDir.Entry).init(allocator);
 
     // Create iterator.
-    var iter = tmp_dir.dir.intoIterable().iterate();
+    var iter = tmp_dir.iterable_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!
@@ -186,14 +187,14 @@ test "Dir.Iterator" {
 }
 
 test "Dir.Iterator twice" {
-    var tmp_dir = tmpDir(.{ .iterate = true });
+    var tmp_dir = tmpIterableDir(.{});
     defer tmp_dir.cleanup();
 
     // First, create a couple of entries to iterate over.
-    const file = try tmp_dir.dir.createFile("some_file", .{});
+    const file = try tmp_dir.iterable_dir.dir.createFile("some_file", .{});
     file.close();
 
-    try tmp_dir.dir.makeDir("some_dir");
+    try tmp_dir.iterable_dir.dir.makeDir("some_dir");
 
     var arena = ArenaAllocator.init(testing.allocator);
     defer arena.deinit();
@@ -204,7 +205,7 @@ test "Dir.Iterator twice" {
         var entries = std.ArrayList(IterableDir.Entry).init(allocator);
 
         // Create iterator.
-        var iter = tmp_dir.dir.intoIterable().iterate();
+        var iter = tmp_dir.iterable_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!
@@ -986,7 +987,7 @@ test "walker" {
     if (builtin.os.tag == .wasi and builtin.link_libc) return error.SkipZigTest;
     if (builtin.os.tag == .wasi and !builtin.link_libc) try os.initPreopensWasi(std.heap.page_allocator, "/");
 
-    var tmp = tmpDir(.{ .iterate = true });
+    var tmp = tmpIterableDir(.{});
     defer tmp.cleanup();
 
     // iteration order of walker is undefined, so need lookup maps to check against
@@ -1012,10 +1013,10 @@ test "walker" {
     });
 
     for (expected_paths.kvs) |kv| {
-        try tmp.dir.makePath(kv.key);
+        try tmp.iterable_dir.dir.makePath(kv.key);
     }
 
-    var walker = try tmp.dir.intoIterable().walk(testing.allocator);
+    var walker = try tmp.iterable_dir.walk(testing.allocator);
     defer walker.deinit();
 
     var num_walked: usize = 0;
@@ -1126,7 +1127,7 @@ test "chmod" {
     defer iterable_dir.close();
 
     try iterable_dir.chmod(0o700);
-    try testing.expect((try iterable_dir.stat()).mode & 0o7777 == 0o700);
+    try testing.expect((try iterable_dir.dir.stat()).mode & 0o7777 == 0o700);
 }
 
 test "chown" {
@@ -1142,7 +1143,7 @@ test "chown" {
 
     try tmp.dir.makeDir("test_dir");
 
-    var iterable_dir = try tmp.dir.openDir("test_dir", .{});
+    var iterable_dir = try tmp.dir.openIterableDir("test_dir", .{});
     defer iterable_dir.close();
     try iterable_dir.chown(null, null);
 }

lib/std/os.zig

@@ -308,7 +308,7 @@ pub fn fchmod(fd: fd_t, mode: mode_t) FChmodError!void {
         switch (system.getErrno(res)) {
             .SUCCESS => return,
             .INTR => continue,
-            .BADF => unreachable, // Can be reached if the fd refers to a directory opened without `OpenDirOptions{ .iterate = true }`
+            .BADF => unreachable, // Can be reached if the fd refers to a non-iterable directory.
 
             .FAULT => unreachable,
             .INVAL => unreachable,
@@ -349,7 +349,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 directory opened without `OpenDirOptions{ .iterate = true }`
+            .BADF => unreachable, // Can be reached if the fd refers to a non-iterable directory.
 
             .FAULT => unreachable,
             .INVAL => unreachable,

lib/std/testing.zig

@@ -363,6 +363,22 @@ 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;
+    }
+};
+
 fn getCwdOrWasiPreopen() std.fs.Dir {
     if (builtin.os.tag == .wasi and !builtin.link_libc) {
         var preopens = std.fs.wasi.PreopenList.init(allocator);
@@ -400,6 +416,28 @@ 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);
+
+    var cwd = getCwdOrWasiPreopen();
+    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();
+    var parent_dir = cache_dir.makeOpenPath("tmp", .{}) catch
+        @panic("unable to make tmp dir for testing: unable to make and open zig-cache/tmp dir");
+    var 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 },

src/test.zig

@@ -54,7 +54,7 @@ test {
             std.fs.path.dirname(@src().file).?, "..", "test", "cases",
         });
 
-        var dir = try std.fs.cwd().openDir(dir_path, .{ .iterate = true });
+        var dir = try std.fs.cwd().openIterableDir(dir_path, .{});
         defer dir.close();
 
         ctx.addTestCasesFromDir(dir);
@@ -1080,7 +1080,7 @@ pub const TestContext = struct {
     /// Each file should include a test manifest as a contiguous block of comments at
     /// the end of the file. The first line should be the test type, followed by a set of
     /// key-value config values, followed by a blank line, then the expected output.
-    pub fn addTestCasesFromDir(ctx: *TestContext, dir: std.fs.Dir) void {
+    pub fn addTestCasesFromDir(ctx: *TestContext, dir: std.fs.IterableDir) void {
         var current_file: []const u8 = "none";
         ctx.addTestCasesFromDirInner(dir, &current_file) catch |err| {
             std.debug.panic("test harness failed to process file '{s}': {s}\n", .{
@@ -1091,12 +1091,12 @@ pub const TestContext = struct {
 
     fn addTestCasesFromDirInner(
         ctx: *TestContext,
-        dir: std.fs.Dir,
+        iterable_dir: std.fs.IterableDir,
         /// This is kept up to date with the currently being processed file so
         /// that if any errors occur the caller knows it happened during this file.
         current_file: *[]const u8,
     ) !void {
-        var it = try dir.intoIterable().walk(ctx.arena);
+        var it = try iterable_dir.walk(ctx.arena);
         var filenames = std.ArrayList([]const u8).init(ctx.arena);
 
         while (try it.next()) |entry| {
@@ -1123,7 +1123,7 @@ pub const TestContext = struct {
                 current_file.* = filename;
 
                 const max_file_size = 10 * 1024 * 1024;
-                const src = try dir.readFileAllocOptions(ctx.arena, filename, max_file_size, null, 1, 0);
+                const src = try iterable_dir.dir.readFileAllocOptions(ctx.arena, filename, max_file_size, null, 1, 0);
 
                 // Parse the manifest
                 var manifest = try TestManifest.parse(ctx.arena, src);