authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2024-03-01 17:45:33-08:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2024-03-01 17:45:33-08:00
log64f77f32df7656c3d7613d402b332f071ea15557
treece4867ec6c6efba68583e2a5f40e38385c90b7ff
parentb60fc16b4f6b973ce2207fb28b77606d45961972
parentf1dd1ee5ed43732a7a671fb438810462136b83eb
signaturebadge-check Signed by PGP key B5690EEEBB952194

Merge pull request #19136 from squeek502/windows-symlink-path-sep

Fix symLink's handling of `/` path separators on Windows

3 files changed, 60 insertions(+), 16 deletions(-)

lib/std/fs/Dir.zig+10
...@@ -1705,6 +1705,15 @@ pub fn symLink(...@@ -1705,6 +1705,15 @@ pub fn symLink(
1705 var target_path_w: std.os.windows.PathSpace = undefined;1705 var target_path_w: std.os.windows.PathSpace = undefined;
1706 target_path_w.len = try std.unicode.wtf8ToWtf16Le(&target_path_w.data, target_path);1706 target_path_w.len = try std.unicode.wtf8ToWtf16Le(&target_path_w.data, target_path);
1707 target_path_w.data[target_path_w.len] = 0;1707 target_path_w.data[target_path_w.len] = 0;
1708 // However, we need to canonicalize any path separators to `\`, since if
1709 // the target path is relative, then it must use `\` as the path separator.
1710 mem.replaceScalar(
1711 u16,
1712 target_path_w.data[0..target_path_w.len],
1713 mem.nativeToLittle(u16, '/'),
1714 mem.nativeToLittle(u16, '\\'),
1715 );
1716
1708 const sym_link_path_w = try std.os.windows.sliceToPrefixedFileW(self.fd, sym_link_path);1717 const sym_link_path_w = try std.os.windows.sliceToPrefixedFileW(self.fd, sym_link_path);
1709 return self.symLinkW(target_path_w.span(), sym_link_path_w.span(), flags);1718 return self.symLinkW(target_path_w.span(), sym_link_path_w.span(), flags);
1710 }1719 }
...@@ -1744,6 +1753,7 @@ pub fn symLinkW(...@@ -1744,6 +1753,7 @@ pub fn symLinkW(
1744 self: Dir,1753 self: Dir,
1745 /// WTF-16, does not need to be NT-prefixed. The NT-prefixing1754 /// WTF-16, does not need to be NT-prefixed. The NT-prefixing
1746 /// of this path is handled by CreateSymbolicLink.1755 /// of this path is handled by CreateSymbolicLink.
1756 /// Any path separators must be `\`, not `/`.
1747 target_path_w: [:0]const u16,1757 target_path_w: [:0]const u16,
1748 /// WTF-16, must be NT-prefixed or relative1758 /// WTF-16, must be NT-prefixed or relative
1749 sym_link_path_w: []const u16,1759 sym_link_path_w: []const u16,
lib/std/fs/test.zig+49-12
...@@ -72,15 +72,17 @@ const PathType = enum {...@@ -72,15 +72,17 @@ const PathType = enum {
7272
73const TestContext = struct {73const TestContext = struct {
74 path_type: PathType,74 path_type: PathType,
75 path_sep: u8,
75 arena: ArenaAllocator,76 arena: ArenaAllocator,
76 tmp: testing.TmpDir,77 tmp: testing.TmpDir,
77 dir: std.fs.Dir,78 dir: std.fs.Dir,
78 transform_fn: *const PathType.TransformFn,79 transform_fn: *const PathType.TransformFn,
7980
80 pub fn init(path_type: PathType, allocator: mem.Allocator, transform_fn: *const PathType.TransformFn) TestContext {81 pub fn init(path_type: PathType, path_sep: u8, allocator: mem.Allocator, transform_fn: *const PathType.TransformFn) TestContext {
81 const tmp = tmpDir(.{ .iterate = true });82 const tmp = tmpDir(.{ .iterate = true });
82 return .{83 return .{
83 .path_type = path_type,84 .path_type = path_type,
85 .path_sep = path_sep,
84 .arena = ArenaAllocator.init(allocator),86 .arena = ArenaAllocator.init(allocator),
85 .tmp = tmp,87 .tmp = tmp,
86 .dir = tmp.dir,88 .dir = tmp.dir,
...@@ -93,11 +95,37 @@ const TestContext = struct {...@@ -93,11 +95,37 @@ const TestContext = struct {
93 self.tmp.cleanup();95 self.tmp.cleanup();
94 }96 }
9597
96 /// Returns the `relative_path` transformed into the TestContext's `path_type`.98 /// Returns the `relative_path` transformed into the TestContext's `path_type`,
99 /// with any supported path separators replaced by `path_sep`.
97 /// The result is allocated by the TestContext's arena and will be free'd during100 /// The result is allocated by the TestContext's arena and will be free'd during
98 /// `TestContext.deinit`.101 /// `TestContext.deinit`.
99 pub fn transformPath(self: *TestContext, relative_path: [:0]const u8) ![:0]const u8 {102 pub fn transformPath(self: *TestContext, relative_path: [:0]const u8) ![:0]const u8 {
100 return self.transform_fn(self.arena.allocator(), self.dir, relative_path);103 const allocator = self.arena.allocator();
104 const transformed_path = try self.transform_fn(allocator, self.dir, relative_path);
105 if (builtin.os.tag == .windows) {
106 const transformed_sep_path = try allocator.dupeZ(u8, transformed_path);
107 std.mem.replaceScalar(u8, transformed_sep_path, switch (self.path_sep) {
108 '/' => '\\',
109 '\\' => '/',
110 else => unreachable,
111 }, self.path_sep);
112 return transformed_sep_path;
113 }
114 return transformed_path;
115 }
116
117 /// Replaces any path separators with the canonical path separator for the platform
118 /// (e.g. all path separators are converted to `\` on Windows).
119 /// If path separators are replaced, then the result is allocated by the
120 /// TestContext's arena and will be free'd during `TestContext.deinit`.
121 pub fn toCanonicalPathSep(self: *TestContext, path: [:0]const u8) ![:0]const u8 {
122 if (builtin.os.tag == .windows) {
123 const allocator = self.arena.allocator();
124 const transformed_sep_path = try allocator.dupeZ(u8, path);
125 std.mem.replaceScalar(u8, transformed_sep_path, '/', '\\');
126 return transformed_sep_path;
127 }
128 return path;
101 }129 }
102};130};
103131
...@@ -106,15 +134,19 @@ const TestContext = struct {...@@ -106,15 +134,19 @@ const TestContext = struct {
106/// and will be passed a TestContext that can transform a relative path into the path type under test.134/// and will be passed a TestContext that can transform a relative path into the path type under test.
107/// The TestContext will also create a tmp directory for you (and will clean it up for you too).135/// The TestContext will also create a tmp directory for you (and will clean it up for you too).
108fn testWithAllSupportedPathTypes(test_func: anytype) !void {136fn testWithAllSupportedPathTypes(test_func: anytype) !void {
109 try testWithPathTypeIfSupported(.relative, test_func);137 try testWithPathTypeIfSupported(.relative, '/', test_func);
110 try testWithPathTypeIfSupported(.absolute, test_func);138 try testWithPathTypeIfSupported(.absolute, '/', test_func);
111 try testWithPathTypeIfSupported(.unc, test_func);139 try testWithPathTypeIfSupported(.unc, '/', test_func);
140 try testWithPathTypeIfSupported(.relative, '\\', test_func);
141 try testWithPathTypeIfSupported(.absolute, '\\', test_func);
142 try testWithPathTypeIfSupported(.unc, '\\', test_func);
112}143}
113144
114fn testWithPathTypeIfSupported(comptime path_type: PathType, test_func: anytype) !void {145fn testWithPathTypeIfSupported(comptime path_type: PathType, comptime path_sep: u8, test_func: anytype) !void {
115 if (!(comptime path_type.isSupported(builtin.os))) return;146 if (!(comptime path_type.isSupported(builtin.os))) return;
147 if (!(comptime fs.path.isSep(path_sep))) return;
116148
117 var ctx = TestContext.init(path_type, testing.allocator, path_type.getTransformFn());149 var ctx = TestContext.init(path_type, path_sep, testing.allocator, path_type.getTransformFn());
118 defer ctx.deinit();150 defer ctx.deinit();
119151
120 try test_func(&ctx);152 try test_func(&ctx);
...@@ -148,20 +180,25 @@ test "Dir.readLink" {...@@ -148,20 +180,25 @@ test "Dir.readLink" {
148 const dir_target_path = try ctx.transformPath("subdir");180 const dir_target_path = try ctx.transformPath("subdir");
149 try ctx.dir.makeDir(dir_target_path);181 try ctx.dir.makeDir(dir_target_path);
150182
183 // On Windows, symlink targets always use the canonical path separator
184 const canonical_file_target_path = try ctx.toCanonicalPathSep(file_target_path);
185 const canonical_dir_target_path = try ctx.toCanonicalPathSep(dir_target_path);
186
151 // test 1: symlink to a file187 // test 1: symlink to a file
152 try setupSymlink(ctx.dir, file_target_path, "symlink1", .{});188 try setupSymlink(ctx.dir, file_target_path, "symlink1", .{});
153 try testReadLink(ctx.dir, file_target_path, "symlink1");189 try testReadLink(ctx.dir, canonical_file_target_path, "symlink1");
154190
155 // test 2: symlink to a directory (can be different on Windows)191 // test 2: symlink to a directory (can be different on Windows)
156 try setupSymlink(ctx.dir, dir_target_path, "symlink2", .{ .is_directory = true });192 try setupSymlink(ctx.dir, dir_target_path, "symlink2", .{ .is_directory = true });
157 try testReadLink(ctx.dir, dir_target_path, "symlink2");193 try testReadLink(ctx.dir, canonical_dir_target_path, "symlink2");
158194
159 // test 3: relative path symlink195 // test 3: relative path symlink
160 const parent_file = ".." ++ fs.path.sep_str ++ "target.txt";196 const parent_file = ".." ++ fs.path.sep_str ++ "target.txt";
197 const canonical_parent_file = try ctx.toCanonicalPathSep(parent_file);
161 var subdir = try ctx.dir.makeOpenPath("subdir", .{});198 var subdir = try ctx.dir.makeOpenPath("subdir", .{});
162 defer subdir.close();199 defer subdir.close();
163 try setupSymlink(subdir, parent_file, "relative-link.txt", .{});200 try setupSymlink(subdir, canonical_parent_file, "relative-link.txt", .{});
164 try testReadLink(subdir, parent_file, "relative-link.txt");201 try testReadLink(subdir, canonical_parent_file, "relative-link.txt");
165 }202 }
166 }.impl);203 }.impl);
167}204}
lib/std/tar/test.zig+1-4
...@@ -464,9 +464,6 @@ test "tar case sensitivity" {...@@ -464,9 +464,6 @@ test "tar case sensitivity" {
464}464}
465465
466test "tar pipeToFileSystem" {466test "tar pipeToFileSystem" {
467 const builtin = @import("builtin");
468 if (builtin.os.tag == .windows) return error.SkipZigTest;
469
470 // $ tar tvf467 // $ tar tvf
471 // pipe_to_file_system_test/468 // pipe_to_file_system_test/
472 // pipe_to_file_system_test/b/469 // pipe_to_file_system_test/b/
...@@ -494,6 +491,6 @@ test "tar pipeToFileSystem" {...@@ -494,6 +491,6 @@ test "tar pipeToFileSystem" {
494 try testing.expect((try root.dir.statFile("a/file")).kind == .file);491 try testing.expect((try root.dir.statFile("a/file")).kind == .file);
495 // TODO is there better way to test symlink492 // TODO is there better way to test symlink
496 try testing.expect((try root.dir.statFile("b/symlink")).kind == .file); // statFile follows symlink493 try testing.expect((try root.dir.statFile("b/symlink")).kind == .file); // statFile follows symlink
497 var buf: [8]u8 = undefined;494 var buf: [32]u8 = undefined;
498 _ = try root.dir.readLink("b/symlink", &buf);495 _ = try root.dir.readLink("b/symlink", &buf);
499}496}