From 46ffc798b6d9bb7be8b016ef7529092d647151ee Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 18 Mar 2020 16:09:06 -0400 Subject: [PATCH] fix swapped logic for Windows Remove `std.fs.deleteTree`. Callers instead should use `std.fs.cwd().deleteTree`. Add `std.fs.deleteTreeAbsolute` for when the caller has an absolute path. --- doc/docgen.zig | 2 +- lib/std/build.zig | 4 +-- lib/std/fs.zig | 48 ++++++++++++++++----------------- lib/std/fs/watch.zig | 2 +- lib/std/os/test.zig | 6 ++--- src-self-hosted/compilation.zig | 3 +-- src-self-hosted/test.zig | 4 +-- test/cli.zig | 2 +- 8 files changed, 34 insertions(+), 37 deletions(-) diff --git a/doc/docgen.zig b/doc/docgen.zig index 4b9b94dbe4..32ad0cdc5d 100644 --- a/doc/docgen.zig +++ b/doc/docgen.zig @@ -48,7 +48,7 @@ pub fn main() !void { var toc = try genToc(allocator, &tokenizer); try fs.cwd().makePath(tmp_dir_name); - defer fs.deleteTree(tmp_dir_name) catch {}; + defer fs.cwd().deleteTree(tmp_dir_name) catch {}; try genHtml(allocator, &tokenizer, &toc, buffered_out_stream.outStream(), zig_exe); try buffered_out_stream.flush(); diff --git a/lib/std/build.zig b/lib/std/build.zig index 7585fe20e2..fd727c321e 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -377,7 +377,7 @@ pub const Builder = struct { if (self.verbose) { warn("rm {}\n", .{full_path}); } - fs.deleteTree(full_path) catch {}; + fs.cwd().deleteTree(full_path) catch {}; } // TODO remove empty directories @@ -2365,7 +2365,7 @@ pub const RemoveDirStep = struct { const self = @fieldParentPtr(RemoveDirStep, "step", step); const full_path = self.builder.pathFromRoot(self.dir_path); - fs.deleteTree(full_path) catch |err| { + fs.cwd().deleteTree(full_path) catch |err| { warn("Unable to remove {}: {}\n", .{ full_path, @errorName(err) }); return err; }; diff --git a/lib/std/fs.zig b/lib/std/fs.zig index e12c35ab8e..bc02fd0c91 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -261,28 +261,6 @@ pub fn deleteDirW(dir_path: [*:0]const u16) !void { return os.rmdirW(dir_path); } -/// Removes a symlink, file, or directory. -/// If `full_path` is relative, this is equivalent to `Dir.deleteTree` with the -/// current working directory as the open directory handle. -/// If `full_path` is absolute, this is equivalent to `Dir.deleteTree` with the -/// base directory. -pub fn deleteTree(full_path: []const u8) !void { - if (path.isAbsolute(full_path)) { - const dirname = path.dirname(full_path) orelse return error{ - /// Attempt to remove the root file system path. - /// This error is unreachable if `full_path` is relative. - CannotDeleteRootDirectory, - }.CannotDeleteRootDirectory; - - var dir = try cwd().openDir(dirname, .{}); - defer dir.close(); - - return dir.deleteTree(path.basename(full_path)); - } else { - return cwd().deleteTree(full_path); - } -} - pub const Dir = struct { fd: os.fd_t, @@ -518,7 +496,8 @@ pub const Dir = struct { self.end_index = io.Information; switch (rc) { .SUCCESS => {}, - .ACCESS_DENIED => return error.AccessDenied, + .ACCESS_DENIED => return error.AccessDenied, // Double-check that the Dir was opened with iteration ability + else => return w.unexpectedStatus(rc), } } @@ -827,7 +806,7 @@ pub const Dir = struct { // 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 else base_flags | w.FILE_LIST_DIRECTORY; + const flags: u32 = if (args.iterate) base_flags | w.FILE_LIST_DIRECTORY else base_flags; return self.openDirAccessMaskW(sub_path_w, flags); } @@ -1304,7 +1283,7 @@ pub const Dir = struct { } }; -/// Returns an handle to the current working directory that is open for traversal. +/// Returns an handle to the current working directory. It is not opened with iteration capability. /// Closing the returned `Dir` is checked illegal behavior. Iterating over the result is illegal behavior. /// On POSIX targets, this function is comptime-callable. pub fn cwd() Dir { @@ -1382,6 +1361,25 @@ pub fn deleteFileAbsoluteW(absolute_path_w: [*:0]const u16) DeleteFileError!void return cwd().deleteFileW(absolute_path_w); } +/// Removes a symlink, file, or directory. +/// This is equivalent to `Dir.deleteTree` with the base directory. +/// Asserts that the path is absolute. See `Dir.deleteTree` for a function that +/// operates on both absolute and relative paths. +/// Asserts that the path parameter has no null bytes. +pub fn deleteTreeAbsolute(absolute_path: []const u8) !void { + assert(path.isAbsolute(absolute_path)); + const dirname = path.dirname(absolute_path) orelse return error{ + /// Attempt to remove the root file system path. + /// This error is unreachable if `absolute_path` is relative. + CannotDeleteRootDirectory, + }.CannotDeleteRootDirectory; + + var dir = try cwd().openDir(dirname, .{}); + defer dir.close(); + + return dir.deleteTree(path.basename(absolute_path)); +} + pub const Walker = struct { stack: std.ArrayList(StackItem), name_buffer: std.Buffer, diff --git a/lib/std/fs/watch.zig b/lib/std/fs/watch.zig index 3180240a72..96fe1538e4 100644 --- a/lib/std/fs/watch.zig +++ b/lib/std/fs/watch.zig @@ -619,7 +619,7 @@ test "write a file, watch it, write it again" { if (true) return error.SkipZigTest; try fs.cwd().makePath(test_tmp_dir); - defer os.deleteTree(test_tmp_dir) catch {}; + defer fs.cwd().deleteTree(test_tmp_dir) catch {}; const allocator = std.heap.page_allocator; return testFsWatch(&allocator); diff --git a/lib/std/os/test.zig b/lib/std/os/test.zig index ae1df6802b..14a865904b 100644 --- a/lib/std/os/test.zig +++ b/lib/std/os/test.zig @@ -20,7 +20,7 @@ test "makePath, put some files in it, deleteTree" { try fs.cwd().makePath("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c"); try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c" ++ fs.path.sep_str ++ "file.txt", "nonsense"); try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "file2.txt", "blah"); - try fs.deleteTree("os_test_tmp"); + try fs.cwd().deleteTree("os_test_tmp"); if (fs.cwd().openDir("os_test_tmp", .{})) |dir| { @panic("expected error"); } else |err| { @@ -38,7 +38,7 @@ test "access file" { try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", ""); try os.access("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", os.F_OK); - try fs.deleteTree("os_test_tmp"); + try fs.cwd().deleteTree("os_test_tmp"); } fn testThreadIdFn(thread_id: *Thread.Id) void { @@ -47,7 +47,7 @@ fn testThreadIdFn(thread_id: *Thread.Id) void { test "sendfile" { try fs.cwd().makePath("os_test_tmp"); - defer fs.deleteTree("os_test_tmp") catch {}; + defer fs.cwd().deleteTree("os_test_tmp") catch {}; var dir = try fs.cwd().openDir("os_test_tmp", .{}); defer dir.close(); diff --git a/src-self-hosted/compilation.zig b/src-self-hosted/compilation.zig index d9f635ebbc..03e71c102d 100644 --- a/src-self-hosted/compilation.zig +++ b/src-self-hosted/compilation.zig @@ -520,8 +520,7 @@ pub const Compilation = struct { if (comp.tmp_dir.getOrNull()) |tmp_dir_result| if (tmp_dir_result.*) |tmp_dir| { - // TODO evented I/O? - fs.deleteTree(tmp_dir) catch {}; + fs.cwd().deleteTree(tmp_dir) catch {}; } else |_| {}; } diff --git a/src-self-hosted/test.zig b/src-self-hosted/test.zig index e87164c9fb..b146d6607a 100644 --- a/src-self-hosted/test.zig +++ b/src-self-hosted/test.zig @@ -57,11 +57,11 @@ pub const TestContext = struct { errdefer allocator.free(self.zig_lib_dir); try std.fs.cwd().makePath(tmp_dir_name); - errdefer std.fs.deleteTree(tmp_dir_name) catch {}; + errdefer std.fs.cwd().deleteTree(tmp_dir_name) catch {}; } fn deinit(self: *TestContext) void { - std.fs.deleteTree(tmp_dir_name) catch {}; + std.fs.cwd().deleteTree(tmp_dir_name) catch {}; allocator.free(self.zig_lib_dir); self.zig_compiler.deinit(); } diff --git a/test/cli.zig b/test/cli.zig index 9bc4a21c90..4c067d16ae 100644 --- a/test/cli.zig +++ b/test/cli.zig @@ -36,7 +36,7 @@ pub fn main() !void { testMissingOutputPath, }; for (test_fns) |testFn| { - try fs.deleteTree(dir_path); + try fs.cwd().deleteTree(dir_path); try fs.cwd().makeDir(dir_path); try testFn(zig_exe, dir_path); }