From fc7d87fef19964273eb65e8cee05eaffa7a9e72f Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Sun, 19 Jul 2020 11:47:00 +0200 Subject: [PATCH] Move symlink to fs.symlinkAbsolute with SymlinkFlags This way `std.fs.symlinkAbsolute` becomes cross-platform and we can legally include `SymlinkFlags` as an argument that's only used on Windows. Also, now `std.os.symlink` generates a compile error on Windows with a message to instead use `std.os.windows.CreateSymbolicLink`. Finally, this PR also reshuffles the tests between `std.os.test` and `std.fs.test`. --- lib/std/fs.zig | 67 ++++++++++++++++++++++++--- lib/std/fs/test.zig | 44 ++++++++++++++++++ lib/std/os.zig | 31 +++---------- lib/std/os/test.zig | 101 ++++++++++++++--------------------------- lib/std/os/windows.zig | 4 +- 5 files changed, 148 insertions(+), 99 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 6b4c09845c..0c7f8f06d5 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -16,9 +16,6 @@ pub const wasi = @import("fs/wasi.zig"); // TODO audit these APIs with respect to Dir and absolute paths -pub const symLink = os.symlink; -pub const symLinkZ = os.symlinkZ; -pub const symLinkC = @compileError("deprecated: renamed to symlinkZ"); pub const rename = os.rename; pub const renameZ = os.renameZ; pub const renameC = @compileError("deprecated: renamed to renameZ"); @@ -69,7 +66,7 @@ pub const need_async_thread = std.io.is_async and switch (builtin.os.tag) { /// TODO remove the allocator requirement from this API pub fn atomicSymLink(allocator: *Allocator, existing_path: []const u8, new_path: []const u8) !void { - if (symLink(existing_path, new_path, .{})) { + if (symLinkAbsolute(existing_path, new_path, .{})) { return; } else |err| switch (err) { error.PathAlreadyExists => {}, @@ -87,7 +84,7 @@ pub fn atomicSymLink(allocator: *Allocator, existing_path: []const u8, new_path: try crypto.randomBytes(rand_buf[0..]); base64_encoder.encode(tmp_path[dirname.len + 1 ..], &rand_buf); - if (symLink(existing_path, tmp_path, .{})) { + if (symLinkAbsolute(existing_path, tmp_path, .{})) { return rename(tmp_path, new_path); } else |err| switch (err) { error.PathAlreadyExists => continue, @@ -1692,8 +1689,15 @@ pub fn readLinkAbsolute(pathname: []const u8, buffer: *[MAX_PATH_BYTES]u8) ![]u8 return os.readlink(pathname, buffer); } +/// Windows-only. Same as `readlinkW`, except the path parameter is null-terminated, WTF16 +/// encoded. +pub fn readlinkAbsoluteW(pathname_w: [*:0]const u16, buffer: *[MAX_PATH_BYTES]u8) ![]u8 { + assert(path.isAbsoluteWindowsW(pathname_w)); + return os.readlinkW(pathname_w, buffer); +} + /// Same as `readLink`, except the path parameter is null-terminated. -pub fn readLinkAbsoluteZ(pathname_c: [*]const u8, buffer: *[MAX_PATH_BYTES]u8) ![]u8 { +pub fn readLinkAbsoluteZ(pathname_c: [*:0]const u8, buffer: *[MAX_PATH_BYTES]u8) ![]u8 { assert(path.isAbsoluteZ(pathname_c)); return os.readlinkZ(pathname_c, buffer); } @@ -1701,6 +1705,57 @@ pub fn readLinkAbsoluteZ(pathname_c: [*]const u8, buffer: *[MAX_PATH_BYTES]u8) ! pub const readLink = @compileError("deprecated; use Dir.readLink or readLinkAbsolute"); pub const readLinkC = @compileError("deprecated; use Dir.readLinkZ or readLinkAbsoluteZ"); +/// Use with `symLinkAbsolute` to specify whether the symlink will point to a file +/// or a directory. This value is ignored on all hosts except Windows where +/// creating symlinks to different resource types, requires different flags. +/// By default, `symLinkAbsolute` is assumed to point to a file. +pub const SymlinkFlags = struct { + is_directory: bool = false, +}; + +/// Creates a symbolic link named `sym_link_path` which contains the string `target_path`. +/// A symbolic link (also known as a soft link) may point to an existing file or to a nonexistent +/// one; the latter case is known as a dangling link. +/// If `sym_link_path` exists, it will not be overwritten. +/// See also `symLinkAbsoluteZ` and `symLinkAbsoluteW`. +pub fn symLinkAbsolute(target_path: []const u8, sym_link_path: []const u8, flags: SymlinkFlags) !void { + if (builtin.os.tag == .wasi) { + @compileError("symLink is not supported in WASI"); + } + assert(path.isAbsolute(target_path)); + assert(path.isAbsolute(sym_link_path)); + if (builtin.os.tag == .windows) { + return os.windows.CreateSymbolicLink(target_path, sym_link_path, flags.is_directory); + } + return os.symlink(target_path, sym_link_path); +} + +/// Windows-only. Same as `symLinkAbsolute` except the parameters are null-terminated, WTF16 encoded. +/// Note that this function will by default try creating a symbolic link to a file. If you would +/// like to create a symbolic link to a directory, specify this with `SymlinkFlags{ .is_directory = true }`. +/// See also `symLinkAbsolute`, `symLinkAbsoluteZ`. +pub fn symLinkAbsoluteW(target_path_w: [*:0]const u16, sym_link_path_w: [*:0]const u16, flags: SymlinkFlags) !void { + assert(path.isAbsoluteWindowsW(target_path_w)); + assert(path.isAbsoluteWindowsW(sym_link_path_w)); + return os.windows.CreateSymbolicLinkW(target_path_w, sym_link_path_w, flags.is_directory); +} + +/// Same as `symLinkAbsolute` except the parameters are null-terminated pointers. +/// See also `symLinkAbsolute`. +pub fn symLinkAbsoluteZ(target_path_c: [*:0]const u8, sym_link_path_c: [*:0]const u8, flags: SymlinkFlags) !void { + assert(path.isAbsoluteZ(target_path_c)); + assert(path.isAbsoluteZ(sym_link_path_c)); + if (builtin.os.tag == .windows) { + const target_path_w = try os.windows.cStrToWin32PrefixedFileW(target_path_c); + const sym_link_path_w = try os.windows.cStrToWin32PrefixedFileW(sym_link_path_c); + return os.windows.CreateSymbolicLinkW(target_path_w.span().ptr, sym_link_path_w.span().ptr, flags.is_directory); + } + return os.symlinkZ(target_path_c, sym_link_path_c); +} + +pub const symLink = @compileError("deprecated: use symLinkAbsolute"); +pub const symLinkC = @compileError("deprecated: use symLinkAbsoluteC"); + pub const Walker = struct { stack: std.ArrayList(StackItem), name_buffer: std.ArrayList(u8), diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index a3cf2e8002..c20cb2e62f 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -10,6 +10,50 @@ const Dir = std.fs.Dir; const File = std.fs.File; const tmpDir = testing.tmpDir; +test "readLinkAbsolute" { + if (builtin.os.tag == .wasi) return error.SkipZigTest; + + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + + // Create some targets + try tmp.dir.writeFile("file.txt", "nonsense"); + try tmp.dir.makeDir("subdir"); + + // Get base abs path + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + + const base_path = blk: { + const relative_path = try fs.path.join(&arena.allocator, &[_][]const u8{ "zig-cache", "tmp", tmp.sub_path[0..] }); + break :blk try fs.realpathAlloc(&arena.allocator, relative_path); + }; + const allocator = &arena.allocator; + + { + const target_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "file.txt" }); + const symlink_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "symlink1" }); + + // Create symbolic link by path + try fs.symLinkAbsolute(target_path, symlink_path, .{}); + try testReadlinkAbsolute(target_path, symlink_path); + } + { + const target_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "subdir" }); + const symlink_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "symlink2" }); + + // Create symbolic link by path + try fs.symLinkAbsolute(target_path, symlink_path, .{ .is_directory = true }); + try testReadlinkAbsolute(target_path, symlink_path); + } +} + +fn testReadlinkAbsolute(target_path: []const u8, symlink_path: []const u8) !void { + var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; + const given = try fs.readLinkAbsolute(symlink_path, buffer[0..]); + testing.expect(mem.eql(u8, target_path, given)); +} + test "Dir.Iterator" { var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup(); diff --git a/lib/std/os.zig b/lib/std/os.zig index 8f86488c0e..ebe457fe4d 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -1520,14 +1520,6 @@ pub fn getcwd(out_buffer: []u8) GetCwdError![]u8 { } } -/// Use with `symlink` to specify whether the symlink will point to a file -/// or a directory. This value is ignored on all hosts except Windows where -/// creating symlinks to different resource types, requires different flags. -/// By default, symlink is assumed to point to a file. -pub const SymlinkFlags = struct { - is_directory: bool = false, -}; - pub const SymLinkError = error{ /// In WASI, this error may occur when the file descriptor does /// not hold the required rights to create a new symbolic link relative to it. @@ -1550,37 +1542,26 @@ pub const SymLinkError = error{ /// A symbolic link (also known as a soft link) may point to an existing file or to a nonexistent /// one; the latter case is known as a dangling link. /// If `sym_link_path` exists, it will not be overwritten. -/// See also `symlinkC` and `symlinkW`. -pub fn symlink(target_path: []const u8, sym_link_path: []const u8, flags: SymlinkFlags) SymLinkError!void { +/// See also `symlinkZ. +pub fn symlink(target_path: []const u8, sym_link_path: []const u8) SymLinkError!void { if (builtin.os.tag == .wasi) { @compileError("symlink is not supported in WASI; use symlinkat instead"); } if (builtin.os.tag == .windows) { - const target_path_w = try windows.sliceToWin32PrefixedFileW(target_path); - const sym_link_path_w = try windows.sliceToWin32PrefixedFileW(sym_link_path); - return symlinkW(target_path_w.span().ptr, sym_link_path_w.span().ptr, flags); + @compileError("symlink is not supported on Windows; use std.os.windows.CreateSymbolicLink instead"); } const target_path_c = try toPosixPath(target_path); const sym_link_path_c = try toPosixPath(sym_link_path); - return symlinkZ(&target_path_c, &sym_link_path_c, flags); + return symlinkZ(&target_path_c, &sym_link_path_c); } pub const symlinkC = @compileError("deprecated: renamed to symlinkZ"); -/// Windows-only. Same as `symlink` except the parameters are null-terminated, WTF16 encoded. -/// Note that this function will by default try creating a symbolic link to a file. If you would -/// like to create a symbolic link to a directory, specify this with `SymlinkFlags{ .is_directory = true }`. -pub fn symlinkW(target_path: [*:0]const u16, sym_link_path: [*:0]const u16, flags: SymlinkFlags) SymLinkError!void { - return windows.CreateSymbolicLinkW(sym_link_path, target_path, flags.is_directory); -} - /// This is the same as `symlink` except the parameters are null-terminated pointers. /// See also `symlink`. -pub fn symlinkZ(target_path: [*:0]const u8, sym_link_path: [*:0]const u8, flags: SymlinkFlags) SymLinkError!void { +pub fn symlinkZ(target_path: [*:0]const u8, sym_link_path: [*:0]const u8) SymLinkError!void { if (builtin.os.tag == .windows) { - const target_path_w = try windows.cStrToWin32PrefixedFileW(target_path); - const sym_link_path_w = try windows.cStrToWin32PrefixedFileW(sym_link_path); - return symlinkW(target_path_w.span().ptr, sym_link_path_w.span().ptr); + @compileError("symlink is not supported on Windows; use std.os.windows.CreateSymbolicLink instead"); } switch (errno(system.symlink(target_path, sym_link_path))) { 0 => return, diff --git a/lib/std/os/test.zig b/lib/std/os/test.zig index 65179e579f..0d9420d389 100644 --- a/lib/std/os/test.zig +++ b/lib/std/os/test.zig @@ -19,6 +19,41 @@ const tmpDir = std.testing.tmpDir; const Dir = std.fs.Dir; const ArenaAllocator = std.heap.ArenaAllocator; +test "symlink with relative paths" { + if (builtin.os.tag == .wasi) return error.SkipZigTest; + + // First, try relative paths in cwd + var cwd = fs.cwd(); + try cwd.writeFile("file.txt", "nonsense"); + + if (builtin.os.tag == .windows) { + try os.windows.CreateSymbolicLink("file.txt", "symlinked", false); + } else { + try os.symlink("file.txt", "symlinked"); + } + + var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; + const given = try os.readlink("symlinked", buffer[0..]); + expect(mem.eql(u8, "file.txt", given)); + + try cwd.deleteFile("file.txt"); + try cwd.deleteFile("symlinked"); +} + +test "readlink on Windows" { + if (builtin.os.tag != .windows) return error.SkipZigTest; + + try testReadlink("C:\\ProgramData", "C:\\Users\\All Users"); + try testReadlink("C:\\Users\\Default", "C:\\Users\\Default User"); + try testReadlink("C:\\Users", "C:\\Documents and Settings"); +} + +fn testReadlink(target_path: []const u8, symlink_path: []const u8) !void { + var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; + const given = try os.readlink(symlink_path, buffer[0..]); + expect(mem.eql(u8, target_path, given)); +} + test "fstatat" { // enable when `fstat` and `fstatat` are implemented on Windows if (builtin.os.tag == .windows) return error.SkipZigTest; @@ -41,72 +76,6 @@ test "fstatat" { expectEqual(stat, statat); } -test "readlink" { - if (builtin.os.tag == .wasi) return error.SkipZigTest; - - // First, try relative paths in cwd - { - var cwd = fs.cwd(); - try cwd.writeFile("file.txt", "nonsense"); - try os.symlink("file.txt", "symlinked", .{}); - - var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; - const given = try os.readlink("symlinked", buffer[0..]); - expect(mem.eql(u8, "file.txt", given)); - - try cwd.deleteFile("file.txt"); - try cwd.deleteFile("symlinked"); - } - - // Now, let's use tempdir - var tmp = tmpDir(.{}); - // defer tmp.cleanup(); - - // Create some targets - try tmp.dir.writeFile("file.txt", "nonsense"); - try tmp.dir.makeDir("subdir"); - - // Get base abs path - var arena = ArenaAllocator.init(testing.allocator); - defer arena.deinit(); - - - const base_path = blk: { - const relative_path = try fs.path.join(&arena.allocator, &[_][]const u8{ "zig-cache", "tmp", tmp.sub_path[0..] }); - break :blk try fs.realpathAlloc(&arena.allocator, relative_path); - }; - const allocator = &arena.allocator; - - { - const target_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "file.txt" }); - const symlink_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "symlink1" }); - - // Create symbolic link by path - try os.symlink(target_path, symlink_path, .{ .is_directory = false }); - try testReadlink(target_path, symlink_path); - } - { - const target_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "subdir" }); - const symlink_path = try fs.path.join(allocator, &[_][]const u8{ base_path, "symlink2" }); - - // Create symbolic link by path - try os.symlink(target_path, symlink_path, .{ .is_directory = true }); - try testReadlink(target_path, symlink_path); - } - - if (builtin.os.tag == .windows) { - try testReadlink("C:\\ProgramData", "C:\\Users\\All Users"); - try testReadlink("C:\\Users\\Default", "C:\\Users\\Default User"); - try testReadlink("C:\\Users", "C:\\Documents and Settings"); - } -} - -fn testReadlink(target_path: []const u8, symlink_path: []const u8) !void { - var buffer: [fs.MAX_PATH_BYTES]u8 = undefined; - const given = try os.readlink(symlink_path, buffer[0..]); - expect(mem.eql(u8, target_path, given)); -} - test "readlinkat" { // enable when `readlinkat` and `symlinkat` are implemented on Windows if (builtin.os.tag == .windows) return error.SkipZigTest; diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index 82aea077c5..8576acc384 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -609,8 +609,8 @@ pub fn CreateSymbolicLink( target_path: []const u8, is_directory: bool, ) CreateSymbolicLinkError!void { - const sym_link_path_w = try sliceToPrefixedFileW(sym_link_path); - const target_path_w = try sliceToPrefixedFileW(target_path); + const sym_link_path_w = try sliceToWin32PrefixedFileW(sym_link_path); + const target_path_w = try sliceToWin32PrefixedFileW(target_path); return CreateSymbolicLinkW(sym_link_path_w.span().ptr, target_path_w.span().ptr, is_directory); }