From 6407417b988f362695dcb73fec78bc6459c2c699 Mon Sep 17 00:00:00 2001 From: Pat Tullmann Date: Mon, 16 Sep 2024 21:52:28 -0700 Subject: [PATCH] lib/std/posix/test.zig: enable disabled tests using CWD Four tests in lib/std/posix/test.zig were disabled because they created fixed-name files in the current working directory, and this caused problems if tests were running in parallel with other build's tests. This PR fixes those tests to all use `std.testing.tmpDir` to create unique temporary names and directories. Also clean the tests up to more consistently use `defer` to clean up, or to just rely on tmpDir cleanup to remove individual files. Working on these tests revealed a bunch of stale WASI code paths in posix.zig, fixed by replacing stale `wast.AT.FDCWD` references with just `AT.FDCWD`. Fixes #14968. --- lib/std/posix.zig | 16 ++-- lib/std/posix/test.zig | 201 +++++++++++++++++++++-------------------- 2 files changed, 109 insertions(+), 108 deletions(-) diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 0a640bf62a20..27f76772690b 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -2104,7 +2104,7 @@ pub fn symlink(target_path: []const u8, sym_link_path: []const u8) SymLinkError! if (native_os == .windows) { @compileError("symlink is not supported on Windows; use std.os.windows.CreateSymbolicLink instead"); } else if (native_os == .wasi and !builtin.link_libc) { - return symlinkat(target_path, wasi.AT.FDCWD, sym_link_path); + return symlinkat(target_path, AT.FDCWD, sym_link_path); } const target_path_c = try toPosixPath(target_path); const sym_link_path_c = try toPosixPath(sym_link_path); @@ -2274,7 +2274,7 @@ pub fn linkZ(oldpath: [*:0]const u8, newpath: [*:0]const u8) LinkError!void { /// On other platforms, both paths are an opaque sequence of bytes with no particular encoding. pub fn link(oldpath: []const u8, newpath: []const u8) LinkError!void { if (native_os == .wasi and !builtin.link_libc) { - return linkat(wasi.AT.FDCWD, oldpath, wasi.AT.FDCWD, newpath, 0) catch |err| switch (err) { + return linkat(AT.FDCWD, oldpath, AT.FDCWD, newpath, 0) catch |err| switch (err) { error.NotDir => unreachable, // link() does not support directories else => |e| return e, }; @@ -2411,7 +2411,7 @@ pub const UnlinkError = error{ /// See also `unlinkZ`. pub fn unlink(file_path: []const u8) UnlinkError!void { if (native_os == .wasi and !builtin.link_libc) { - return unlinkat(wasi.AT.FDCWD, file_path, 0) catch |err| switch (err) { + return unlinkat(AT.FDCWD, file_path, 0) catch |err| switch (err) { error.DirNotEmpty => unreachable, // only occurs when targeting directories else => |e| return e, }; @@ -2605,7 +2605,7 @@ pub const RenameError = error{ /// On other platforms, both paths are an opaque sequence of bytes with no particular encoding. pub fn rename(old_path: []const u8, new_path: []const u8) RenameError!void { if (native_os == .wasi and !builtin.link_libc) { - return renameat(wasi.AT.FDCWD, old_path, wasi.AT.FDCWD, new_path); + return renameat(AT.FDCWD, old_path, AT.FDCWD, new_path); } else if (native_os == .windows) { const old_path_w = try windows.sliceToPrefixedFileW(null, old_path); const new_path_w = try windows.sliceToPrefixedFileW(null, new_path); @@ -3000,7 +3000,7 @@ pub const MakeDirError = error{ /// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding. pub fn mkdir(dir_path: []const u8, mode: mode_t) MakeDirError!void { if (native_os == .wasi and !builtin.link_libc) { - return mkdirat(wasi.AT.FDCWD, dir_path, mode); + return mkdirat(AT.FDCWD, dir_path, mode); } else if (native_os == .windows) { const dir_path_w = try windows.sliceToPrefixedFileW(null, dir_path); return mkdirW(dir_path_w.span(), mode); @@ -3089,7 +3089,7 @@ pub const DeleteDirError = error{ /// On other platforms, `dir_path` is an opaque sequence of bytes with no particular encoding. pub fn rmdir(dir_path: []const u8) DeleteDirError!void { if (native_os == .wasi and !builtin.link_libc) { - return unlinkat(wasi.AT.FDCWD, dir_path, AT.REMOVEDIR) catch |err| switch (err) { + return unlinkat(AT.FDCWD, dir_path, AT.REMOVEDIR) catch |err| switch (err) { error.FileSystem => unreachable, // only occurs when targeting files error.IsDir => unreachable, // only occurs when targeting files else => |e| return e, @@ -3278,7 +3278,7 @@ pub const ReadLinkError = error{ /// On other platforms, the result is an opaque sequence of bytes with no particular encoding. pub fn readlink(file_path: []const u8, out_buffer: []u8) ReadLinkError![]u8 { if (native_os == .wasi and !builtin.link_libc) { - return readlinkat(wasi.AT.FDCWD, file_path, out_buffer); + return readlinkat(AT.FDCWD, file_path, out_buffer); } else if (native_os == .windows) { const file_path_w = try windows.sliceToPrefixedFileW(null, file_path); return readlinkW(file_path_w.span(), out_buffer); @@ -4893,7 +4893,7 @@ pub fn access(path: []const u8, mode: u32) AccessError!void { _ = try windows.GetFileAttributesW(path_w.span().ptr); return; } else if (native_os == .wasi and !builtin.link_libc) { - return faccessat(wasi.AT.FDCWD, path, mode, 0); + return faccessat(AT.FDCWD, path, mode, 0); } const path_c = try toPosixPath(path); return accessZ(&path_c, mode); diff --git a/lib/std/posix/test.zig b/lib/std/posix/test.zig index 3254faedfbc0..29756cae69a7 100644 --- a/lib/std/posix/test.zig +++ b/lib/std/posix/test.zig @@ -44,13 +44,12 @@ test "check WASI CWD" { } } -test "chdir smoke test" { +test "chdir absolute parent" { if (native_os == .wasi) return error.SkipZigTest; - if (true) { - // https://github.com/ziglang/zig/issues/14968 - return error.SkipZigTest; - } + // Restore default CWD at end of test. + const orig_cwd = try fs.cwd().openDir(".", .{}); + defer orig_cwd.setAsCwd() catch unreachable; // Get current working directory path var old_cwd_buf: [fs.max_path_bytes]u8 = undefined; @@ -65,47 +64,46 @@ test "chdir smoke test" { } // Next, change current working directory to one level above - if (native_os != .wasi) { // WASI does not support navigating outside of Preopens - const parent = fs.path.dirname(old_cwd) orelse unreachable; // old_cwd should be absolute - try posix.chdir(parent); + const parent = fs.path.dirname(old_cwd) orelse unreachable; // old_cwd should be absolute + try posix.chdir(parent); - // Restore cwd because process may have other tests that do not tolerate chdir. - defer posix.chdir(old_cwd) catch unreachable; + var new_cwd_buf: [fs.max_path_bytes]u8 = undefined; + const new_cwd = try posix.getcwd(new_cwd_buf[0..]); + try expect(mem.eql(u8, parent, new_cwd)); +} - var new_cwd_buf: [fs.max_path_bytes]u8 = undefined; - const new_cwd = try posix.getcwd(new_cwd_buf[0..]); - try expect(mem.eql(u8, parent, new_cwd)); - } +test "chdir relative" { + if (native_os == .wasi) return error.SkipZigTest; - // Next, change current working directory to a temp directory one level below - { - // Create a tmp directory - var tmp_dir_buf: [fs.max_path_bytes]u8 = undefined; - const tmp_dir_path = path: { - var allocator = std.heap.FixedBufferAllocator.init(&tmp_dir_buf); - break :path try fs.path.resolve(allocator.allocator(), &[_][]const u8{ old_cwd, "zig-test-tmp" }); - }; - var tmp_dir = try fs.cwd().makeOpenPath("zig-test-tmp", .{}); + var tmp = tmpDir(.{}); + defer tmp.cleanup(); - // Change current working directory to tmp directory - try posix.chdir("zig-test-tmp"); + // Restore default CWD at end of test. + const orig_cwd = try fs.cwd().openDir(".", .{}); + defer orig_cwd.setAsCwd() catch unreachable; - var new_cwd_buf: [fs.max_path_bytes]u8 = undefined; - const new_cwd = try posix.getcwd(new_cwd_buf[0..]); + // Use the tmpDir parent_dir as the "base" for the test. Then cd into the child + try tmp.parent_dir.setAsCwd(); - // On Windows, fs.path.resolve returns an uppercase drive letter, but the drive letter returned by getcwd may be lowercase - var resolved_cwd_buf: [fs.max_path_bytes]u8 = undefined; - const resolved_cwd = path: { - var allocator = std.heap.FixedBufferAllocator.init(&resolved_cwd_buf); - break :path try fs.path.resolve(allocator.allocator(), &[_][]const u8{new_cwd}); - }; - try expect(mem.eql(u8, tmp_dir_path, resolved_cwd)); + // Capture base working directory path, to build expected full path + var base_cwd_buf: [fs.max_path_bytes]u8 = undefined; + const base_cwd = try posix.getcwd(base_cwd_buf[0..]); - // Restore cwd because process may have other tests that do not tolerate chdir. - tmp_dir.close(); - posix.chdir(old_cwd) catch unreachable; - try fs.cwd().deleteDir("zig-test-tmp"); - } + const dir_name = &tmp.sub_path; + const expected_path = try fs.path.resolve(a, &.{ base_cwd, dir_name }); + defer a.free(expected_path); + + // change current working directory to new directory + try posix.chdir(dir_name); + + var new_cwd_buf: [fs.max_path_bytes]u8 = undefined; + const new_cwd = try posix.getcwd(new_cwd_buf[0..]); + + // On Windows, fs.path.resolve returns an uppercase drive letter, but the drive letter returned by getcwd may be lowercase + const resolved_cwd = try fs.path.resolve(a, &.{new_cwd}); + defer a.free(resolved_cwd); + + try expect(mem.eql(u8, expected_path, resolved_cwd)); } test "open smoke test" { @@ -222,44 +220,42 @@ test "openat smoke test" { } test "symlink with relative paths" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; + if (native_os == .wasi) return error.SkipZigTest; // Can symlink, but can't change into tmpDir - if (true) { - // https://github.com/ziglang/zig/issues/14968 - return error.SkipZigTest; - } - const cwd = fs.cwd(); - cwd.deleteFile("file.txt") catch {}; - cwd.deleteFile("symlinked") catch {}; + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + + const target_name = "symlink-target"; + const symlink_name = "symlinker"; - // First, try relative paths in cwd - try cwd.writeFile(.{ .sub_path = "file.txt", .data = "nonsense" }); + // Restore default CWD at end of test. + const orig_cwd = try fs.cwd().openDir(".", .{}); + defer orig_cwd.setAsCwd() catch unreachable; + + // Create the target file + try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "nonsense" }); + + // Want to test relative paths, so cd into the tmpdir for this test + try tmp.dir.setAsCwd(); if (native_os == .windows) { - std.os.windows.CreateSymbolicLink( - cwd.fd, - &[_]u16{ 's', 'y', 'm', 'l', 'i', 'n', 'k', 'e', 'd' }, - &[_:0]u16{ 'f', 'i', 'l', 'e', '.', 't', 'x', 't' }, - false, - ) catch |err| switch (err) { + const wtarget_name = try std.unicode.wtf8ToWtf16LeAllocZ(a, target_name); + const wsymlink_name = try std.unicode.wtf8ToWtf16LeAllocZ(a, symlink_name); + defer a.free(wtarget_name); + defer a.free(wsymlink_name); + + std.os.windows.CreateSymbolicLink(tmp.dir.fd, wsymlink_name, wtarget_name, false) catch |err| switch (err) { // Symlink requires admin privileges on windows, so this test can legitimately fail. - error.AccessDenied => { - try cwd.deleteFile("file.txt"); - try cwd.deleteFile("symlinked"); - return error.SkipZigTest; - }, + error.AccessDenied => return error.SkipZigTest, else => return err, }; } else { - try posix.symlink("file.txt", "symlinked"); + try posix.symlink(target_name, symlink_name); } var buffer: [fs.max_path_bytes]u8 = undefined; - const given = try posix.readlink("symlinked", buffer[0..]); - try expect(mem.eql(u8, "file.txt", given)); - - try cwd.deleteFile("file.txt"); - try cwd.deleteFile("symlinked"); + const given = try posix.readlink(symlink_name, buffer[0..]); + try expect(mem.eql(u8, target_name, given)); } test "readlink on Windows" { @@ -277,90 +273,95 @@ fn testReadlink(target_path: []const u8, symlink_path: []const u8) !void { } test "link with relative paths" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; + if (native_os == .wasi) return error.SkipZigTest; // Can link, but can't change into tmpDir + if (builtin.cpu.arch == .riscv32 and builtin.os.tag == .linux and !builtin.link_libc) return error.SkipZigTest; // No `fstat()`. switch (native_os) { .wasi, .linux, .solaris, .illumos => {}, else => return error.SkipZigTest, } - if (true) { - // https://github.com/ziglang/zig/issues/14968 - return error.SkipZigTest; - } - var cwd = fs.cwd(); - cwd.deleteFile("example.txt") catch {}; - cwd.deleteFile("new.txt") catch {}; + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + + // Restore default CWD at end of test. + const orig_cwd = try fs.cwd().openDir(".", .{}); + defer orig_cwd.setAsCwd() catch unreachable; + + const target_name = "link-target"; + const link_name = "newlink"; + + try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "example" }); - try cwd.writeFile(.{ .sub_path = "example.txt", .data = "example" }); - try posix.link("example.txt", "new.txt"); + // Test 1: create the relative link from inside tmp + try tmp.dir.setAsCwd(); + try posix.link(target_name, link_name); - const efd = try cwd.openFile("example.txt", .{}); + // Verify + const efd = try tmp.dir.openFile(target_name, .{}); defer efd.close(); - const nfd = try cwd.openFile("new.txt", .{}); + const nfd = try tmp.dir.openFile(link_name, .{}); defer nfd.close(); { const estat = try posix.fstat(efd.handle); const nstat = try posix.fstat(nfd.handle); - try testing.expectEqual(estat.ino, nstat.ino); try testing.expectEqual(@as(@TypeOf(nstat.nlink), 2), nstat.nlink); } - try posix.unlink("new.txt"); + // Test 2: Remove the link and see the stats update + try posix.unlink(link_name); { const estat = try posix.fstat(efd.handle); try testing.expectEqual(@as(@TypeOf(estat.nlink), 1), estat.nlink); } - - try cwd.deleteFile("example.txt"); } test "linkat with different directories" { - if (native_os == .wasi and builtin.link_libc) return error.SkipZigTest; + if (builtin.cpu.arch == .riscv32 and builtin.os.tag == .linux and !builtin.link_libc) return error.SkipZigTest; // No `fstatat()`. switch (native_os) { .wasi, .linux, .solaris, .illumos => {}, else => return error.SkipZigTest, } - if (true) { - // https://github.com/ziglang/zig/issues/14968 - return error.SkipZigTest; - } - var cwd = fs.cwd(); + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + + const target_name = "link-target"; + const link_name = "newlink"; - cwd.deleteFile("example.txt") catch {}; - tmp.dir.deleteFile("new.txt") catch {}; + const subdir = try tmp.dir.makeOpenPath("subdir", .{}); - try cwd.writeFile(.{ .sub_path = "example.txt", .data = "example" }); - try posix.linkat(cwd.fd, "example.txt", tmp.dir.fd, "new.txt", 0); + defer tmp.dir.deleteFile(target_name) catch {}; + try tmp.dir.writeFile(.{ .sub_path = target_name, .data = "example" }); - const efd = try cwd.openFile("example.txt", .{}); + // Test 1: link from file in subdir back up to target in parent directory + try posix.linkat(tmp.dir.fd, target_name, subdir.fd, link_name, 0); + + const efd = try tmp.dir.openFile(target_name, .{}); defer efd.close(); - const nfd = try tmp.dir.openFile("new.txt", .{}); + const nfd = try subdir.openFile(link_name, .{}); + defer nfd.close(); { - defer nfd.close(); const estat = try posix.fstat(efd.handle); const nstat = try posix.fstat(nfd.handle); - try testing.expectEqual(estat.ino, nstat.ino); try testing.expectEqual(@as(@TypeOf(nstat.nlink), 2), nstat.nlink); } - try posix.unlinkat(tmp.dir.fd, "new.txt", 0); + // Test 2: remove link + try posix.unlinkat(subdir.fd, link_name, 0); { const estat = try posix.fstat(efd.handle); try testing.expectEqual(@as(@TypeOf(estat.nlink), 1), estat.nlink); } - - try cwd.deleteFile("example.txt"); } test "fstatat" { @@ -979,7 +980,7 @@ test "POSIX file locking with fcntl" { return error.SkipZigTest; } - var tmp = std.testing.tmpDir(.{}); + var tmp = tmpDir(.{}); defer tmp.cleanup(); // Create a temporary lock file