#12474: std.fs.Dir.makeOpenPath: optimize case, if path already exists

lib/std/fs.zig

@@ -1459,8 +1459,9 @@ pub const Dir = struct {
         try os.mkdiratW(self.fd, sub_path, default_new_dir_mode);
     }
 
-    /// Calls makeDir recursively to make an entire path. Returns success if the path
-    /// already exists and is a directory.
+    /// Calls makeDir iteratively to make an entire path
+    /// (i.e. creating any parent directories that do not exist).
+    /// Returns success if the path already exists and is a directory.
     /// This function is not atomic, and if it returns an error, the file system may
     /// have been modified regardless.
     pub fn makePath(self: Dir, sub_path: []const u8) !void {
@@ -1483,22 +1484,89 @@ pub const Dir = struct {
         }
     }
 
+    /// Calls makeOpenDirAccessMaskW iteratively to make an entire path
+    /// (i.e. creating any parent directories that do not exist).
+    /// Opens the dir if the path already exists and is a directory.
+    /// This function is not atomic, and if it returns an error, the file system may
+    /// have been modified regardless.
+    fn makeOpenPathAccessMaskW(self: Dir, sub_path: []const u8, access_mask: u32, no_follow: bool) OpenError!Dir {
+        const w = os.windows;
+        var it = try path.componentIterator(sub_path);
+        // If there are no components in the path, then create a dummy component with the full path.
+        var component = it.last() orelse path.NativeUtf8ComponentIterator.Component{
+            .name = "",
+            .path = sub_path,
+        };
+
+        while (true) {
+            const sub_path_w = try w.sliceToPrefixedFileW(self.fd, component.path);
+            const is_last = it.peekNext() == null;
+            var result = self.makeOpenDirAccessMaskW(sub_path_w.span().ptr, access_mask, .{
+                .no_follow = no_follow,
+                .create_disposition = if (is_last) w.FILE_OPEN_IF else w.FILE_CREATE,
+            }) catch |err| switch (err) {
+                error.FileNotFound => |e| {
+                    component = it.previous() orelse return e;
+                    continue;
+                },
+                else => |e| return e,
+            };
+
+            component = it.next() orelse return result;
+            // Don't leak the intermediate file handles
+            result.close();
+        }
+    }
+
     /// This function performs `makePath`, followed by `openDir`.
     /// If supported by the OS, this operation is atomic. It is not atomic on
     /// all operating systems.
+    /// On Windows, this function performs `makeOpenPathAccessMaskW`.
     pub fn makeOpenPath(self: Dir, sub_path: []const u8, open_dir_options: OpenDirOptions) !Dir {
-        // TODO improve this implementation on Windows; we can avoid 1 call to NtClose
-        try self.makePath(sub_path);
-        return self.openDir(sub_path, open_dir_options);
+        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;
+
+                return self.makeOpenPathAccessMaskW(sub_path, base_flags, open_dir_options.no_follow);
+            },
+            else => {
+                return self.openDir(sub_path, open_dir_options) catch |err| switch (err) {
+                    error.FileNotFound => {
+                        try self.makePath(sub_path);
+                        return self.openDir(sub_path, open_dir_options);
+                    },
+                    else => |e| return e,
+                };
+            },
+        };
     }
 
     /// 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);
+        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
@@ -1742,7 +1810,10 @@ pub const Dir = struct {
         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;
-        var dir = try self.openDirAccessMaskW(sub_path_w, flags, args.no_follow);
+        var dir = try self.makeOpenDirAccessMaskW(sub_path_w, flags, .{
+            .no_follow = args.no_follow,
+            .create_disposition = w.FILE_OPEN,
+        });
         return dir;
     }
 
@@ -1765,7 +1836,12 @@ pub const Dir = struct {
         return Dir{ .fd = fd };
     }
 
-    fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32, no_follow: bool) OpenError!Dir {
+    const MakeOpenDirAccessMaskWOptions = struct {
+        no_follow: bool,
+        create_disposition: u32,
+    };
+
+    fn makeOpenDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32, flags: MakeOpenDirAccessMaskWOptions) OpenError!Dir {
         const w = os.windows;
 
         var result = Dir{
@@ -1786,21 +1862,22 @@ pub const Dir = struct {
             .SecurityDescriptor = null,
             .SecurityQualityOfService = null,
         };
-        const open_reparse_point: w.DWORD = if (no_follow) w.FILE_OPEN_REPARSE_POINT else 0x0;
+        const open_reparse_point: w.DWORD = if (flags.no_follow) w.FILE_OPEN_REPARSE_POINT else 0x0;
         var io: w.IO_STATUS_BLOCK = undefined;
         const rc = w.ntdll.NtCreateFile(
             &result.fd,
             access_mask,
             &attr,
             &io,
             null,
-            0,
+            w.FILE_ATTRIBUTE_NORMAL,
             w.FILE_SHARE_READ | w.FILE_SHARE_WRITE,
-            w.FILE_OPEN,
+            flags.create_disposition,
             w.FILE_DIRECTORY_FILE | w.FILE_SYNCHRONOUS_IO_NONALERT | w.FILE_OPEN_FOR_BACKUP_INTENT | open_reparse_point,
             null,
             0,
         );
+
         switch (rc) {
             .SUCCESS => return result,
             .OBJECT_NAME_INVALID => return error.BadPathName,

lib/std/fs/path.zig

@@ -1507,6 +1507,14 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
         /// For example, if the path is `/a/b/c` and the most recently returned component
         /// is `b`, then this will return the `c` component.
         pub fn next(self: *Self) ?Component {
+            const peek_result = self.peekNext() orelse return null;
+            self.start_index = peek_result.path.len - peek_result.name.len;
+            self.end_index = peek_result.path.len;
+            return peek_result;
+        }
+
+        /// Like `next`, but does not modify the iterator state.
+        pub fn peekNext(self: Self) ?Component {
             var start_index = self.end_index;
             while (start_index < self.path.len and path_type.isSep(T, self.path[start_index])) {
                 start_index += 1;
@@ -1516,11 +1524,9 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
                 end_index += 1;
             }
             if (start_index == end_index) return null;
-            self.start_index = start_index;
-            self.end_index = end_index;
             return .{
-                .name = self.path[self.start_index..self.end_index],
-                .path = self.path[0..self.end_index],
+                .name = self.path[start_index..end_index],
+                .path = self.path[0..end_index],
             };
         }
 
@@ -1529,6 +1535,14 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
         /// For example, if the path is `/a/b/c` and the most recently returned component
         /// is `b`, then this will return the `a` component.
         pub fn previous(self: *Self) ?Component {
+            const peek_result = self.peekPrevious() orelse return null;
+            self.start_index = peek_result.path.len - peek_result.name.len;
+            self.end_index = peek_result.path.len;
+            return peek_result;
+        }
+
+        /// Like `previous`, but does not modify the iterator state.
+        pub fn peekPrevious(self: Self) ?Component {
             var end_index = self.start_index;
             while (true) {
                 if (end_index == self.root_end_index) return null;
@@ -1542,11 +1556,9 @@ pub fn ComponentIterator(comptime path_type: PathType, comptime T: type) type {
                 start_index -= 1;
             }
             if (start_index == end_index) return null;
-            self.start_index = start_index;
-            self.end_index = end_index;
             return .{
-                .name = self.path[self.start_index..self.end_index],
-                .path = self.path[0..self.end_index],
+                .name = self.path[start_index..end_index],
+                .path = self.path[0..end_index],
             };
         }
     };

lib/std/fs/test.zig

@@ -663,6 +663,18 @@ test "file operations on directories" {
     }.impl);
 }
 
+test "makeOpenPath parent dirs do not exist" {
+    var tmp_dir = tmpDir(.{});
+    defer tmp_dir.cleanup();
+
+    var dir = try tmp_dir.dir.makeOpenPath("root_dir/parent_dir/some_dir", .{});
+    dir.close();
+
+    // double check that the full directory structure was created
+    var dir_verification = try tmp_dir.dir.openDir("root_dir/parent_dir/some_dir", .{});
+    dir_verification.close();
+}
+
 test "deleteDir" {
     try testWithAllSupportedPathTypes(struct {
         fn impl(ctx: *TestContext) !void {