Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows.DeleteFile: Use FileDispositionInformationEx if possible, but fallback if not #16499

Merged
merged 2 commits into from
Jul 23, 2023
Merged

windows.DeleteFile: Use FileDispositionInformationEx if possible, but fallback if not #16499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ae378e
windows.DeleteFile: Use FileDispositionInformationEx if possible, but…
squeek502 Jul 23, 2023
21ecb1b
Consolidate 'delete a read-only file on windows' test cases
squeek502 Jul 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 18 additions & 25 deletions lib/std/fs/test.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1416,42 +1416,35 @@ test "File.PermissionsUnix" {
try testing.expect(!permissions_unix.unixHas(.other, .execute));
}

test "delete a read-only file on windows with file pending semantics" {
if (builtin.os.tag != .windows or builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1))
test "delete a read-only file on windows" {
if (builtin.os.tag != .windows)
return error.SkipZigTest;

var tmp = tmpDir(.{});
var tmp = testing.tmpDir(.{});
defer tmp.cleanup();
{
const file = try tmp.dir.createFile("test_file", .{ .read = true });
defer file.close();
// Create a file and make it read-only
const metadata = try file.metadata();
var permissions = metadata.permissions();
permissions.setReadOnly(true);
try file.setPermissions(permissions);
try testing.expectError(error.AccessDenied, tmp.dir.deleteFile("test_file"));
// Now make the file not read-only
permissions.setReadOnly(false);
try file.setPermissions(permissions);
}
try tmp.dir.deleteFile("test_file");
}

test "delete a read-only file on windows with posix semantis" {
if (builtin.os.tag != .windows or !builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1))
return error.SkipZigTest;

var tmp = tmpDir(.{});
defer tmp.cleanup();
const file = try tmp.dir.createFile("test_file", .{ .read = true });
defer file.close();
// Create a file and make it read-only
const metadata = try file.metadata();
var permissions = metadata.permissions();
permissions.setReadOnly(true);
try file.setPermissions(permissions);
try tmp.dir.deleteFile("test_file"); // file is unmapped and deleted once last handle closed

// If the OS and filesystem support it, POSIX_SEMANTICS and IGNORE_READONLY_ATTRIBUTE
// is used meaning that the deletion of a read-only file will succeed.
// Otherwise, this delete will fail and the read-only flag must be unset before it's
// able to be deleted.
const delete_result = tmp.dir.deleteFile("test_file");
if (delete_result) {
try testing.expectError(error.FileNotFound, tmp.dir.deleteFile("test_file"));
} else |err| {
try testing.expectEqual(@as(anyerror, error.AccessDenied), err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not strictly test that the deleteFile with file pending semantics succeeds (file does not exist anymore).
The file must be closed for the deletion to be successful (and it should be checked).

// Now make the file not read-only
permissions.setReadOnly(false);
try file.setPermissions(permissions);
try tmp.dir.deleteFile("test_file");
}
}

test "delete a setAsCwd directory on Windows" {
Expand Down
16 changes: 14 additions & 2 deletions lib/std/os/windows.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,8 +940,13 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil
}
defer CloseHandle(tmp_handle);

// FileDispositionInformationEx (and therefore FILE_DISPOSITION_POSIX_SEMANTICS and FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE)
// are only supported on NTFS filesystems, so the version check on its own is only a partial solution. To support non-NTFS filesystems
// like FAT32, we need to fallback to FileDispositionInformation if the usage of FileDispositionInformationEx gives
// us INVALID_PARAMETER.
var need_fallback = true;
if (comptime builtin.target.os.version_range.windows.min.isAtLeast(.win10_rs1)) {
// Deletion with posix semantics.
// Deletion with posix semantics if the filesystem supports it.
var info = FILE_DISPOSITION_INFORMATION_EX{
.Flags = FILE_DISPOSITION_DELETE |
FILE_DISPOSITION_POSIX_SEMANTICS |
Expand All @@ -955,7 +960,14 @@ pub fn DeleteFile(sub_path_w: []const u16, options: DeleteFileOptions) DeleteFil
@sizeOf(FILE_DISPOSITION_INFORMATION_EX),
.FileDispositionInformationEx,
);
} else {
switch (rc) {
// INVALID_PARAMETER here means that the filesystem does not support FileDispositionInformationEx
.INVALID_PARAMETER => {},
// For all other statuses, fall down to the switch below to handle them.
else => need_fallback = false,
}
}
if (need_fallback) {
// Deletion with file pending semantics, which requires waiting or moving
// files to get them removed (from here).
var file_dispo = FILE_DISPOSITION_INFORMATION{
Expand Down