std.fs: Get some more rename tests passing on Windows #18632
Conversation
Most of these tests were made to pass by ziglang#16717, but there were a few loose ends to tie up with regards to the 'fallback' behavior (when FILE_RENAME_POSIX_SEMANTICS either isn't available or isn't supported by the underlying filesystem). We now do a bit of extra work to get POSIX-like renaming in the fallback path. Closes ziglang#6364
|
These calls to stat() are expensive ( I find cosmopolitan libc's Windows implementations of posix functions to be useful points of reference Here's their Instead of This also skips opening the file for that case - as internally it sounds like it does something like |
|
Thanks for the info/link! I didn't realize Something I haven't investigated yet (but have been meaning to) is the behavior with regards to symlinks. Will need to see how EDIT:
I'll mess around a bit and try to figure things out a bit more. Would also be interesting to benchmark something like cosmopoliation libc's implementation against the Windows |
| // FileRenameInformation has surprising behavior around renaming a directory to the | ||
| // path of a file: it succeeds and replaces the file with the directory. | ||
| // | ||
| // To avoid this behavior and instead return error.NotDir, we (unfortunately) have | ||
| // to query the type of both the source and destination. However, doing so also | ||
| // allows us to return error.IsDir/error.PathAlreadyExists in cases where NtSetInformationFile | ||
| // with FileRenameInformation would have only returned a generic ACCESS_DENIED. |
There was a problem hiding this comment.
I think this is kind of handy behavior by the kernel actually. Before going through with this fallback mechanism, I think it would be worth looking into something like this:
- Remove
std.os.renameatW. Don't try to provide a posix-style API for this function on Windows. - Make
std.os.renamegive a compile error if called on Windows for the same reason. - Update
std.fsand other former callsites to renameatW to directly use the Windows API rather than going through this posix layer. - If necessary, make changes to the API of std.fs operations, taking into account our new understanding of what the Windows kernel is capable of. Define the API such that such a fallback mechanism is not needed, and this logic can be deleted. Sorry, I know you just did a bunch of creative work to figure out how to make it work. The goal however is to make the fs API to minimize such logic as this while still being useful and providing cross platform abstractions.
There was a problem hiding this comment.
I'm having trouble imagining what the usage of the proposed API would be like. If I'm writing cross-platform code, fs.rename/Dir.rename having quite different properties on different platforms seems like it'd make for a rather hard-to-use API, since any relevant properties can't actually be relied on to be consistent across platforms. For example, if you write your code such that it can take advantage of being able to rename a directory onto an existing file, then that property will only hold on Windows, so you'd need to do something else for non-Windows platforms at every callsite of rename. To me, it seems like this would just sort-of move the burden of creating a cross-platform abstraction to the users/callsites of rename (and force users to keep in their heads all the different semantics for each platform).
Maybe it would make sense to split rename into a POSIX-like version and a "native" version? This would still give access to consistent cross-platform rename semantics while also providing a public API for the most syscall-optimal rename implementation for each platform.
There was a problem hiding this comment.
Mainly, one would be aware of the differences in platforms and then take the following actions:
- Rely on only the ability for a renamed file to overwrite an existing file
- Not rely on a directory overwriting a file to produce an error
This is a reasonable set of requirements, still allows for useful abstraction, and eliminates the fallback code from being needed at all.
Most users will not need this fallback logic, and the ones that do should absolutely have this logic pushed up into the application domain.
Maybe it would make sense to split rename into a POSIX-like version and a "native" version?
This is the intention of std.os.rename (related: #5019). My suggestion here is to make this be one of the functions that does not attempt a posix compatibility layer on Windows, since it's not really a directly lowerable operation.
I suppose we could consider trying hard to provide posix compatibilty layers for windows, but I think it would result in better software in practice if users were strongly encouraged instead towards structuring their projects to avoid not-strictly-necessary compatibility layers.
There was a problem hiding this comment.
Just to give a complete picture, these are all the known differences between Windows/POSIX rename when not using FILE_RENAME_POSIX_SEMANTICS (and ignoring symlinks, not tested the symlink behavior yet; I expect that to be another can of worms):
- Renaming a directory onto an existing file succeeds on Windows, fails with
ENOTDIRon POSIX systems - Renaming a directory onto an existing empty directory succeeds on POSIX systems, fails with
ACCESS_DENIEDon Windows - Renaming a directory onto an existing non-empty directory fails on POSIX systems with
ENOTEMPTY, but fails on Windows withACCESS_DENIED - Renaming a file onto any existing directory fails on POSIX systems with
EISDIR, but fails on Windows withACCESS_DENIED
I also did some very basic/preliminary benchmarking out of curiosity (not really relevant to the API design question, just potentially interesting on its own):
Benchmark code
const std = @import("std");
const builtin = @import("builtin");
const fd_t = std.os.fd_t;
const windows = std.os.windows;
const RenameError = std.os.RenameError;
const MAX_PATH_BYTES = std.fs.MAX_PATH_BYTES;
const Implementation = enum {
/// Windows POSIX rename implementation via FILE_RENAME_POSIX_SEMANTICS
windows_posix_rename_semantics,
/// Custom POSIX rename implementation via NtQueryAttributesFile/DeleteFile/FileRenameInformation
emulated,
/// Windows rename via FileRenameInformation, no POSIX emulation
native,
/// Windows rename via FileRenameInformationEx and FILE_RENAME_IGNORE_READONLY_ATTRIBUTE,
/// no POSIX emulation
native_ex,
pub fn isNative(self: Implementation) bool {
return self == .native or self == .native_ex;
}
};
pub fn main() !void {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
const implementations = [_]Implementation{ .windows_posix_rename_semantics, .emulated, .native, .native_ex };
inline for (implementations) |implementation| {
std.debug.print("\n{s}\n", .{@tagName(implementation)});
var timer = try std.time.Timer.start();
const iterations = 1000;
for (0..iterations) |_| {
try testRenameFiles(implementation, tmp.dir);
}
std.debug.print("rename files: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
for (0..iterations) |_| {
try testRenameDirectories(implementation, tmp.dir);
}
std.debug.print("rename dirs: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
for (0..iterations) |_| {
try testRenameOntoEmptyDir(implementation, tmp.dir);
}
std.debug.print("rename onto empty: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
for (0..iterations) |_| {
try testRenameOntoNonEmptyDir(implementation, tmp.dir);
}
std.debug.print("rename onto non-empty: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
for (0..iterations) |_| {
try testRenameFileOntoDir(implementation, tmp.dir);
}
std.debug.print("rename file onto dir: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
for (0..iterations) |_| {
try testRenameDirOntoFile(implementation, tmp.dir);
}
std.debug.print("rename dir onto file: {}ms\n", .{timer.lap() / std.time.ns_per_ms});
}
}
fn testRenameFiles(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const missing_file_path = "missing_file_name";
const something_else_path = "something_else";
try std.testing.expectError(
error.FileNotFound,
rename(implementation, dir, missing_file_path, something_else_path),
);
// Renaming files
const test_file_name = "test_file";
const renamed_test_file_name = "test_file_renamed";
var file = try dir.createFile(test_file_name, .{ .read = true });
file.close();
try rename(implementation, dir, test_file_name, renamed_test_file_name);
// Ensure the file was renamed
try std.testing.expectError(error.FileNotFound, dir.openFile(test_file_name, .{}));
file = try dir.openFile(renamed_test_file_name, .{});
file.close();
// Rename to self succeeds
try rename(implementation, dir, renamed_test_file_name, renamed_test_file_name);
// Rename to existing file succeeds
const existing_file_path = "existing_file";
var existing_file = try dir.createFile(existing_file_path, .{ .read = true });
existing_file.close();
try rename(implementation, dir, renamed_test_file_name, existing_file_path);
try std.testing.expectError(error.FileNotFound, dir.openFile(renamed_test_file_name, .{}));
file = try dir.openFile(existing_file_path, .{});
file.close();
try dir.deleteTree(test_file_name);
try dir.deleteTree(renamed_test_file_name);
try dir.deleteTree(existing_file_path);
}
fn testRenameDirectories(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const test_dir_path = "test_dir";
const test_dir_renamed_path = "test_dir_renamed";
// Renaming directories
try dir.makeDir(test_dir_path);
try rename(implementation, dir, test_dir_path, test_dir_renamed_path);
// Ensure the directory was renamed
{
try std.testing.expectError(error.FileNotFound, dir.openDir(test_dir_path, .{}));
var renamed_dir = try dir.openDir(test_dir_renamed_path, .{});
defer renamed_dir.close();
// Put a file in the directory
var file = try renamed_dir.createFile("test_file", .{ .read = true });
defer file.close();
}
const test_dir_renamed_again_path = "test_dir_renamed_again";
try rename(implementation, dir, test_dir_renamed_path, test_dir_renamed_again_path);
// Ensure the directory was renamed and the file still exists in it
{
try std.testing.expectError(error.FileNotFound, dir.openDir(test_dir_renamed_path, .{}));
var renamed_dir = try dir.openDir(test_dir_renamed_again_path, .{});
defer renamed_dir.close();
var file = try renamed_dir.openFile("test_file", .{});
defer file.close();
}
try dir.deleteTree(test_dir_path);
try dir.deleteTree(test_dir_renamed_path);
try dir.deleteTree(test_dir_renamed_again_path);
}
fn testRenameOntoEmptyDir(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const test_dir_path = "test_dir";
const target_dir_path = "empty_dir_path";
try dir.makeDir(test_dir_path);
try dir.makeDir(target_dir_path);
if (implementation.isNative()) {
rename(implementation, dir, test_dir_path, target_dir_path) catch |err| switch (err) {
error.AccessDenied => {},
else => return err,
};
} else {
try rename(implementation, dir, test_dir_path, target_dir_path);
try std.testing.expectError(error.FileNotFound, dir.openDir(test_dir_path, .{}));
var renamed_dir = try dir.openDir(target_dir_path, .{});
renamed_dir.close();
}
try dir.deleteTree(test_dir_path);
try dir.deleteTree(target_dir_path);
}
fn testRenameOntoNonEmptyDir(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const test_dir_path = "test_dir";
const target_dir_path = "non_empty_dir_path";
try dir.makeDir(test_dir_path);
{
var target_dir = try dir.makeOpenPath(target_dir_path, .{});
defer target_dir.close();
var file = try target_dir.createFile("test_file", .{ .read = true });
defer file.close();
}
// Rename should fail with PathAlreadyExists if target_dir is non-empty
try std.testing.expectError(
if (implementation.isNative()) error.AccessDenied else error.PathAlreadyExists,
rename(implementation, dir, test_dir_path, target_dir_path),
);
// Ensure the directory was not renamed
var test_dir = try dir.openDir(test_dir_path, .{});
test_dir.close();
try dir.deleteTree(test_dir_path);
try dir.deleteTree(target_dir_path);
}
fn testRenameFileOntoDir(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const test_file_path = "test_file";
const test_dir_path = "test_dir";
var file = try dir.createFile(test_file_path, .{ .read = true });
file.close();
try dir.makeDir(test_dir_path);
try std.testing.expectError(
if (implementation.isNative()) error.AccessDenied else error.IsDir,
rename(implementation, dir, test_file_path, test_dir_path),
);
try dir.deleteTree(test_file_path);
try dir.deleteTree(test_dir_path);
}
fn testRenameDirOntoFile(comptime implementation: Implementation, dir: std.fs.Dir) !void {
const test_file_path = "test_file";
const test_dir_path = "test_dir";
var file = try dir.createFile(test_file_path, .{ .read = true });
file.close();
try dir.makeDir(test_dir_path);
if (implementation.isNative()) {
try rename(implementation, dir, test_dir_path, test_file_path);
} else {
try std.testing.expectError(
error.NotDir,
rename(implementation, dir, test_dir_path, test_file_path),
);
}
try dir.deleteTree(test_file_path);
try dir.deleteTree(test_dir_path);
}
fn rename(comptime implementation: Implementation, dir: std.fs.Dir, old_path: []const u8, new_path: []const u8) RenameError!void {
const old_path_w = try windows.sliceToPrefixedFileW(dir.fd, old_path);
const new_path_w = try windows.sliceToPrefixedFileW(dir.fd, new_path);
const func = switch (implementation) {
.windows_posix_rename_semantics => renameWindowsPosixSemantics,
.emulated => renameEmulatedPosix,
.native => renameNative,
.native_ex => renameNativeEx,
};
return func(dir.fd, old_path_w.span(), dir.fd, new_path_w.span(), windows.TRUE);
}
pub fn renameWindowsPosixSemantics(
old_dir_fd: fd_t,
old_path_w: []const u16,
new_dir_fd: fd_t,
new_path_w: []const u16,
ReplaceIfExists: windows.BOOLEAN,
) RenameError!void {
if (comptime !builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs5)) {
@compileError("need >= win10_rs5");
}
const src_fd = windows.OpenFile(old_path_w, .{
.dir = old_dir_fd,
.access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE | windows.GENERIC_READ,
.creation = windows.FILE_OPEN,
.io_mode = .blocking,
.filter = .any, // This function is supposed to rename both files and directories.
.follow_symlinks = false,
}) catch |err| switch (err) {
error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`.
else => |e| return e,
};
defer windows.CloseHandle(src_fd);
const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION_EX) + (MAX_PATH_BYTES - 1);
var rename_info_buf: [struct_buf_len]u8 align(@alignOf(windows.FILE_RENAME_INFORMATION_EX)) = undefined;
const struct_len = @sizeOf(windows.FILE_RENAME_INFORMATION_EX) - 1 + new_path_w.len * 2;
if (struct_len > struct_buf_len) return error.NameTooLong;
const rename_info = @as(*windows.FILE_RENAME_INFORMATION_EX, @ptrCast(&rename_info_buf));
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
var flags: windows.ULONG = windows.FILE_RENAME_POSIX_SEMANTICS | windows.FILE_RENAME_IGNORE_READONLY_ATTRIBUTE;
if (ReplaceIfExists == windows.TRUE) flags |= windows.FILE_RENAME_REPLACE_IF_EXISTS;
rename_info.* = .{
.Flags = flags,
.RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(new_path_w)) null else new_dir_fd,
.FileNameLength = @intCast(new_path_w.len * 2), // already checked error.NameTooLong
.FileName = undefined,
};
@memcpy(@as([*]u16, &rename_info.FileName)[0..new_path_w.len], new_path_w);
const rc = windows.ntdll.NtSetInformationFile(
src_fd,
&io_status_block,
rename_info,
@intCast(struct_len), // already checked for error.NameTooLong
.FileRenameInformationEx,
);
switch (rc) {
.SUCCESS => {},
.INVALID_HANDLE => unreachable,
// INVALID_PARAMETER here means that the filesystem does not support FileRenameInformationEx
.INVALID_PARAMETER => unreachable,
.OBJECT_PATH_SYNTAX_BAD => unreachable,
.ACCESS_DENIED => return error.AccessDenied,
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.OBJECT_PATH_NOT_FOUND => return error.FileNotFound,
.NOT_SAME_DEVICE => return error.RenameAcrossMountPoints,
.OBJECT_NAME_COLLISION => return error.PathAlreadyExists,
.DIRECTORY_NOT_EMPTY => return error.PathAlreadyExists,
.FILE_IS_A_DIRECTORY => return error.IsDir,
.NOT_A_DIRECTORY => return error.NotDir,
else => return windows.unexpectedStatus(rc),
}
}
pub fn renameEmulatedPosix(
old_dir_fd: fd_t,
old_path_w: []const u16,
new_dir_fd: fd_t,
new_path_w: []const u16,
ReplaceIfExists: windows.BOOLEAN,
) RenameError!void {
const old_attr = QueryAttributes(old_dir_fd, old_path_w) catch |err| switch (err) {
error.PermissionDenied => return error.AccessDenied,
else => |e| return e,
};
const maybe_new_attr: ?windows.ULONG = QueryAttributes(new_dir_fd, new_path_w) catch |err| switch (err) {
error.FileNotFound => null,
error.PermissionDenied => return error.AccessDenied,
else => |e| return e,
};
if (maybe_new_attr != null and ReplaceIfExists != windows.TRUE) return error.PathAlreadyExists;
if (maybe_new_attr) |new_attr| {
const old_is_dir = old_attr & windows.FILE_ATTRIBUTE_DIRECTORY != 0;
const new_is_dir = new_attr & windows.FILE_ATTRIBUTE_DIRECTORY != 0;
if (!old_is_dir and new_is_dir) return error.IsDir;
if (old_is_dir and !new_is_dir) return error.NotDir;
if (old_is_dir and new_is_dir) {
windows.DeleteFile(new_path_w, .{ .dir = new_dir_fd, .remove_dir = true }) catch {
return error.PathAlreadyExists;
};
}
}
const src_fd = windows.OpenFile(old_path_w, .{
.dir = old_dir_fd,
.access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE,
.creation = windows.FILE_OPEN,
.io_mode = .blocking,
.filter = .any, // This function is supposed to rename both files and directories.
.follow_symlinks = false,
}) catch |err| switch (err) {
error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`.
else => |e| return e,
};
defer windows.CloseHandle(src_fd);
const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION) + (MAX_PATH_BYTES - 1);
var rename_info_buf: [struct_buf_len]u8 align(@alignOf(windows.FILE_RENAME_INFORMATION)) = undefined;
const struct_len = @sizeOf(windows.FILE_RENAME_INFORMATION) - 1 + new_path_w.len * 2;
if (struct_len > struct_buf_len) return error.NameTooLong;
const rename_info = @as(*windows.FILE_RENAME_INFORMATION, @ptrCast(&rename_info_buf));
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
rename_info.* = .{
.Flags = ReplaceIfExists,
.RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(new_path_w)) null else new_dir_fd,
.FileNameLength = @intCast(new_path_w.len * 2), // already checked error.NameTooLong
.FileName = undefined,
};
@memcpy(@as([*]u16, &rename_info.FileName)[0..new_path_w.len], new_path_w);
const rc =
windows.ntdll.NtSetInformationFile(
src_fd,
&io_status_block,
rename_info,
@intCast(struct_len), // already checked for error.NameTooLong
.FileRenameInformation,
);
switch (rc) {
.SUCCESS => {},
.INVALID_HANDLE => unreachable,
.INVALID_PARAMETER => unreachable,
.OBJECT_PATH_SYNTAX_BAD => unreachable,
.ACCESS_DENIED => return error.AccessDenied,
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.OBJECT_PATH_NOT_FOUND => return error.FileNotFound,
.NOT_SAME_DEVICE => return error.RenameAcrossMountPoints,
.OBJECT_NAME_COLLISION => return error.PathAlreadyExists,
else => return windows.unexpectedStatus(rc),
}
}
pub fn renameNative(
old_dir_fd: fd_t,
old_path_w: []const u16,
new_dir_fd: fd_t,
new_path_w: []const u16,
ReplaceIfExists: windows.BOOLEAN,
) RenameError!void {
const src_fd = windows.OpenFile(old_path_w, .{
.dir = old_dir_fd,
.access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE,
.creation = windows.FILE_OPEN,
.io_mode = .blocking,
.filter = .any, // This function is supposed to rename both files and directories.
.follow_symlinks = false,
}) catch |err| switch (err) {
error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`.
else => |e| return e,
};
defer windows.CloseHandle(src_fd);
const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION) + (MAX_PATH_BYTES - 1);
var rename_info_buf: [struct_buf_len]u8 align(@alignOf(windows.FILE_RENAME_INFORMATION)) = undefined;
const struct_len = @sizeOf(windows.FILE_RENAME_INFORMATION) - 1 + new_path_w.len * 2;
if (struct_len > struct_buf_len) return error.NameTooLong;
const rename_info = @as(*windows.FILE_RENAME_INFORMATION, @ptrCast(&rename_info_buf));
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
rename_info.* = .{
.Flags = ReplaceIfExists,
.RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(new_path_w)) null else new_dir_fd,
.FileNameLength = @intCast(new_path_w.len * 2), // already checked error.NameTooLong
.FileName = undefined,
};
@memcpy(@as([*]u16, &rename_info.FileName)[0..new_path_w.len], new_path_w);
const rc =
windows.ntdll.NtSetInformationFile(
src_fd,
&io_status_block,
rename_info,
@intCast(struct_len), // already checked for error.NameTooLong
.FileRenameInformation,
);
switch (rc) {
.SUCCESS => {},
.INVALID_HANDLE => unreachable,
.INVALID_PARAMETER => unreachable,
.OBJECT_PATH_SYNTAX_BAD => unreachable,
.ACCESS_DENIED => return error.AccessDenied,
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.OBJECT_PATH_NOT_FOUND => return error.FileNotFound,
.NOT_SAME_DEVICE => return error.RenameAcrossMountPoints,
.OBJECT_NAME_COLLISION => return error.PathAlreadyExists,
else => return windows.unexpectedStatus(rc),
}
}
pub fn renameNativeEx(
old_dir_fd: fd_t,
old_path_w: []const u16,
new_dir_fd: fd_t,
new_path_w: []const u16,
ReplaceIfExists: windows.BOOLEAN,
) RenameError!void {
if (comptime !builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs5)) {
@compileError("need >= win10_rs5");
}
const src_fd = windows.OpenFile(old_path_w, .{
.dir = old_dir_fd,
.access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE | windows.GENERIC_READ,
.creation = windows.FILE_OPEN,
.io_mode = .blocking,
.filter = .any, // This function is supposed to rename both files and directories.
.follow_symlinks = false,
}) catch |err| switch (err) {
error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`.
else => |e| return e,
};
defer windows.CloseHandle(src_fd);
const struct_buf_len = @sizeOf(windows.FILE_RENAME_INFORMATION_EX) + (MAX_PATH_BYTES - 1);
var rename_info_buf: [struct_buf_len]u8 align(@alignOf(windows.FILE_RENAME_INFORMATION_EX)) = undefined;
const struct_len = @sizeOf(windows.FILE_RENAME_INFORMATION_EX) - 1 + new_path_w.len * 2;
if (struct_len > struct_buf_len) return error.NameTooLong;
const rename_info = @as(*windows.FILE_RENAME_INFORMATION_EX, @ptrCast(&rename_info_buf));
var io_status_block: windows.IO_STATUS_BLOCK = undefined;
var flags: windows.ULONG = windows.FILE_RENAME_IGNORE_READONLY_ATTRIBUTE;
if (ReplaceIfExists == windows.TRUE) flags |= windows.FILE_RENAME_REPLACE_IF_EXISTS;
rename_info.* = .{
.Flags = flags,
.RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(new_path_w)) null else new_dir_fd,
.FileNameLength = @intCast(new_path_w.len * 2), // already checked error.NameTooLong
.FileName = undefined,
};
@memcpy(@as([*]u16, &rename_info.FileName)[0..new_path_w.len], new_path_w);
const rc = windows.ntdll.NtSetInformationFile(
src_fd,
&io_status_block,
rename_info,
@intCast(struct_len), // already checked for error.NameTooLong
.FileRenameInformationEx,
);
switch (rc) {
.SUCCESS => {},
.INVALID_HANDLE => unreachable,
// INVALID_PARAMETER here means that the filesystem does not support FileRenameInformationEx
.INVALID_PARAMETER => unreachable,
.OBJECT_PATH_SYNTAX_BAD => unreachable,
.ACCESS_DENIED => return error.AccessDenied,
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.OBJECT_PATH_NOT_FOUND => return error.FileNotFound,
.NOT_SAME_DEVICE => return error.RenameAcrossMountPoints,
.OBJECT_NAME_COLLISION => return error.PathAlreadyExists,
.DIRECTORY_NOT_EMPTY => return error.PathAlreadyExists,
.FILE_IS_A_DIRECTORY => return error.IsDir,
.NOT_A_DIRECTORY => return error.NotDir,
else => return windows.unexpectedStatus(rc),
}
}
fn QueryAttributes(dir_fd: fd_t, sub_path_w: []const u16) !windows.ULONG {
const path_len_bytes = std.math.cast(u16, sub_path_w.len * 2) orelse return error.NameTooLong;
var nt_name = windows.UNICODE_STRING{
.Length = path_len_bytes,
.MaximumLength = path_len_bytes,
.Buffer = @constCast(sub_path_w.ptr),
};
var attr = windows.OBJECT_ATTRIBUTES{
.Length = @sizeOf(windows.OBJECT_ATTRIBUTES),
.RootDirectory = if (std.fs.path.isAbsoluteWindowsWTF16(sub_path_w)) null else dir_fd,
.Attributes = 0, // Note we do not use OBJ_CASE_INSENSITIVE here.
.ObjectName = &nt_name,
.SecurityDescriptor = null,
.SecurityQualityOfService = null,
};
var basic_info: windows.FILE_BASIC_INFORMATION = undefined;
switch (windows.ntdll.NtQueryAttributesFile(&attr, &basic_info)) {
.SUCCESS => {},
.OBJECT_NAME_NOT_FOUND => return error.FileNotFound,
.OBJECT_PATH_NOT_FOUND => return error.FileNotFound,
.OBJECT_NAME_INVALID => unreachable,
.INVALID_PARAMETER => unreachable,
.ACCESS_DENIED => return error.PermissionDenied,
.OBJECT_PATH_SYNTAX_BAD => unreachable,
else => |rc| return windows.unexpectedStatus(rc),
}
return basic_info.FileAttributes;
}The results on my computer are:
windows_posix_rename_semantics
rename files: 1769ms
rename dirs: 1617ms
rename onto empty: 873ms
rename onto non-empty: 1221ms
rename file onto dir: 882ms
rename dir onto file: 951ms
emulated
rename files: 1907ms
rename dirs: 1624ms
rename onto empty: 932ms
rename onto non-empty: 900ms
rename file onto dir: 644ms
rename dir onto file: 647ms
native
rename files: 1744ms
rename dirs: 1582ms
rename onto empty: 805ms
rename onto non-empty: 1206ms
rename file onto dir: 860ms
rename dir onto file: 968ms
native_ex
rename files: 1737ms
rename dirs: 1565ms
rename onto empty: 800ms
rename onto non-empty: 1147ms
rename file onto dir: 873ms
rename dir onto file: 961ms
where:
/// Windows POSIX rename implementation via FILE_RENAME_POSIX_SEMANTICS
windows_posix_rename_semantics,
/// Custom POSIX rename implementation via NtQueryAttributesFile/DeleteFile/FileRenameInformation
emulated,
/// Windows rename via FileRenameInformation, no POSIX emulation
native,
/// Windows rename via FileRenameInformationEx and FILE_RENAME_IGNORE_READONLY_ATTRIBUTE,
/// no POSIX emulation
native_ex,There was a problem hiding this comment.
Nice work, as usual.
Renaming a directory onto an existing file succeeds on Windows, fails with ENOTDIR on POSIX systems
Renaming a directory onto an existing empty directory succeeds on POSIX systems, fails with ACCESS_DENIED on Windows
Renaming a directory onto an existing non-empty directory fails on POSIX systems with ENOTEMPTY, but fails on Windows with ACCESS_DENIED
Renaming a file onto any existing directory fails on POSIX systems with EISDIR, but fails on Windows with ACCESS_DENIED
Having this flow chart in the doc comments, along with a list of behaviors that are fully cross platform, I think is an underrated deliverable. This is how we make "sweet spot" abstractions that will provide the building blocks for truly reusable code.
There was a problem hiding this comment.
Another wrinkle:
- The rename semantics and returned errors on Windows actually seem to be dictated by the underlying filesystem or possibly the filesystem driver that controls the particular drive (not sure if 'filesystem driver' is the correct terminology)
- On a local FAT32 drive, the error cases return
OBJECT_NAME_COLLISIONinstead ofACCESS_DENIED, but the rename semantics are the same as outlined above:
testRenameOntoEmptyDir: error.PathAlreadyExists testRenameOntoNonEmptyDir: error.PathAlreadyExists testRenameFileOntoDir: error.PathAlreadyExists testRenameDirOntoFile: success- On a networked SMB drive (NTFS) that lives on a Linux machine, it seems like
renamegains POSIX-like semantics and returnsOBJECT_NAME_COLLISIONfor the error cases:
testRenameOntoEmptyDir: success testRenameOntoNonEmptyDir: error.PathAlreadyExists testRenameFileOntoDir: error.PathAlreadyExists testRenameDirOntoFile: error.PathAlreadyExists - On a local FAT32 drive, the error cases return
So there may not technically be a knowable/defined behavior for FileRenameInformation on Windows, since it seems to ultimately be dictated by the underlying filesystem.
There was a problem hiding this comment.
Would preferring failure if one of the error cases are true be possible to unify the behavior in std across os and drives/filesystems?
EDIT: Ignore the following proposal. Not atomic thus not very well thought out.
testRenameOntoEmptyDir: error.PathAlreadyExists // Always check if path exists before rename
testRenameOntoNonEmptyDir: error.PathAlreadyExists
testRenameFileOntoDir: error.PathAlreadyExists
testRenameDirOntoFile: error.PathAlreadyExists // On Windows, check if file exists before rename
|
I've been poking around to see if I can find examples of well-worn, cross-platform filesystem APIs that Zig could steal semantics from. I'm not finding much. This section of the CERT Secure C Coding Standard about how to securely use I wonder if Zig could have slightly more targeted functions like Also here's a fun corner case: the source and target of the rename are hardlinks to the same underlying file. |
Generally, yes, that's the idea. And in such functions it is more OK to have fallback logic since the semantics of the function explain what must be done, and so of course that fallback code must be done. The problem arises when, instead, it could be lowered in multiple ways, and the std lib takes an opinionated approach, making one system emulate the other. The goal is for the programmer to communicate their intent precisely, in other words, do they need error.PathNotFound to occur if the rename fails due to the destination being a directory? That piece of information is relevant to the implementation, so it needs to be communicated. |
|
Closing since it's a non-goal to have full Any complex, buggy, or bloaty implementations of posix APIs should just be |
Most of these tests were made to pass by #16717, but there were a few loose ends to tie up with regards to the 'fallback' behavior (when FILE_RENAME_POSIX_SEMANTICS either isn't available or isn't supported by the underlying filesystem).
We now do a bit of extra work to get POSIX-like renaming in the fallback path. This feels a bit bad, but the Windows behavior around renaming a directory to the path of a file (it succeeds and replaces the file with the directory) sort of forces our hand, and allows making the rest of the fallback conform to POSIX rename behavior without too much additional trouble.
Closes #6364