Fix for Windows: std.os.windows.DeleteFile()

[suirad]

This commit addresses #5537 using the following test:

const builtin = @import("builtin");
const std = @import("std");
const fs = std.fs;
const assert = std.debug.assert;
const print = std.debug.print;
const testing = std.testing;

test "#5537" {
    if (builtin.os.tag != .windows) return;
    const cwd = fs.cwd();

    cwd.makeDir("tmp") catch |e| {
        switch (e) {
            error.PathAlreadyExists => {},
            else => return e,
        }
    };
    defer cwd.deleteDir("tmp") catch unreachable;

    var file = try cwd.createFile("tmp/tmpfile", .{ .read = true });
    defer {
        file.close();
        cwd.deleteFile("tmp/tmpfile") catch unreachable;
    }

   // Actual test for proper behavior
    testing.expectError(error.DirNotEmpty, cwd.deleteDir("tmp"));
}

I considered adding this test to the std to catch regressions but I wasn't sure if it was wanted to be added and I also wasn't sure exactly where in the repo to add it.

[squeek502]

Nice. Here's a possible test case that can be added to std/fs/test.zig:

test "deleteDir" {
    var tmp_dir = tmpDir(.{});
    defer tmp_dir.cleanup();

    // deleting a non-existent directory
    testing.expectError(error.FileNotFound, tmp_dir.dir.deleteDir("test_dir"));

    var dir = try tmp_dir.dir.makeOpenPath("test_dir", .{});
    var file = try dir.createFile("test_file", .{});
    file.close();
    dir.close();

    // deleting a non-empty directory
    testing.expectError(error.DirNotEmpty, tmp_dir.dir.deleteDir("test_dir"));

    dir = try tmp_dir.dir.openDir("test_dir", .{});
    try dir.deleteFile("test_file");
    dir.close();

    // deleting an empty directory
    try tmp_dir.dir.deleteDir("test_dir");
}

[kubkon]

Nice. Here's a possible test case that can be added to std/fs/test.zig:

test "deleteDir" {
    var tmp_dir = tmpDir(.{});
    defer tmp_dir.cleanup();

    // deleting a non-existent directory
    testing.expectError(error.FileNotFound, tmp_dir.dir.deleteDir("test_dir"));

    var dir = try tmp_dir.dir.makeOpenPath("test_dir", .{});
    var file = try dir.createFile("test_file", .{});
    file.close();
    dir.close();

    // deleting a non-empty directory
    testing.expectError(error.DirNotEmpty, tmp_dir.dir.deleteDir("test_dir"));

    dir = try tmp_dir.dir.openDir("test_dir", .{});
    try dir.deleteFile("test_file");
    dir.close();

    // deleting an empty directory
    try tmp_dir.dir.deleteDir("test_dir");
}

We definitely need a test case added for this, thanks for suggesting this @squeek502!

[kubkon]

@suirad, have you perhaps experimented with ReactOS approach where instead of setting FILE_DELETE_ON_CLOSE in NtCreateFile, we open the file/dir with DELETE and then mark it for deletion with NtSetInformationFile. I'm curious to see whether this will have different behaviour when trying to remove a non-empty directory, i.e., ideally yield an error of some kind rather than succeed and ignore the op altoghether.

[suirad]

I've updated the code adding @squeek502 's test and moving the non empty directory detection into std.os.windows.DeleteFile per @kubkon 's notes.

[kubkon]

Looks great! Just one more nitpick which doesn't tweak the behaviour of this PR, and we're good to go.