Commit fc7d87fef1

Jakub Konka <kubkon@jakubkonka.com>
2020-07-19 11:47:00
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`.
1 parent dd366d3
Changed files (5)
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();
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;
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);
 }
 
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),
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,