From 5c8b92db7f6e99d075af1cf87d48a5af0c748603 Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 5 Jun 2025 12:13:31 +0100 Subject: [PATCH 1/2] tests: do not require absolute paths from the build system File arguments added to `std.Build.Step.Run` with e.g. `addFileArg` are not necessarily passed as absolute paths. It used to be the case that they were as a consequence of an unnecessary path conversion done by the frontend, but this no longer happens, at least not always, so these tests were sometimes failing when run locally. Therefore, the standalone tests must handle cwd-relative CLI paths correctly. --- test/standalone/dirname/exists_in.zig | 7 +------ test/standalone/dirname/has_basename.zig | 5 ----- test/standalone/dirname/touch.zig | 7 +------ test/standalone/run_output_caching/main.zig | 2 +- test/standalone/self_exe_symlink/create-symlink.zig | 5 ++++- 5 files changed, 7 insertions(+), 19 deletions(-) diff --git a/test/standalone/dirname/exists_in.zig b/test/standalone/dirname/exists_in.zig index 6730200b3f..7aec1f423d 100644 --- a/test/standalone/dirname/exists_in.zig +++ b/test/standalone/dirname/exists_in.zig @@ -29,17 +29,12 @@ fn run(allocator: std.mem.Allocator) !void { return error.BadUsage; }; - if (!std.fs.path.isAbsolute(dir_path)) { - std.log.err("expected to be an absolute path", .{}); - return error.BadUsage; - } - const relpath = args.next() orelse { std.log.err("missing argument", .{}); return error.BadUsage; }; - var dir = try std.fs.openDirAbsolute(dir_path, .{}); + var dir = try std.fs.cwd().openDir(dir_path, .{}); defer dir.close(); _ = try dir.statFile(relpath); diff --git a/test/standalone/dirname/has_basename.zig b/test/standalone/dirname/has_basename.zig index 84f473a07c..49eefa3b48 100644 --- a/test/standalone/dirname/has_basename.zig +++ b/test/standalone/dirname/has_basename.zig @@ -31,11 +31,6 @@ fn run(allocator: std.mem.Allocator) !void { return error.BadUsage; }; - if (!std.fs.path.isAbsolute(path)) { - std.log.err("path must be absolute", .{}); - return error.BadUsage; - } - const basename = args.next() orelse { std.log.err("missing argument", .{}); return error.BadUsage; diff --git a/test/standalone/dirname/touch.zig b/test/standalone/dirname/touch.zig index 3ca714a3af..43fcabf91e 100644 --- a/test/standalone/dirname/touch.zig +++ b/test/standalone/dirname/touch.zig @@ -26,15 +26,10 @@ fn run(allocator: std.mem.Allocator) !void { return error.BadUsage; }; - if (!std.fs.path.isAbsolute(path)) { - std.log.err("path must be absolute: {s}", .{path}); - return error.BadUsage; - } - const dir_path = std.fs.path.dirname(path) orelse unreachable; const basename = std.fs.path.basename(path); - var dir = try std.fs.openDirAbsolute(dir_path, .{}); + var dir = try std.fs.cwd().openDir(dir_path, .{}); defer dir.close(); _ = dir.statFile(basename) catch { diff --git a/test/standalone/run_output_caching/main.zig b/test/standalone/run_output_caching/main.zig index f801971e83..e4e6332f11 100644 --- a/test/standalone/run_output_caching/main.zig +++ b/test/standalone/run_output_caching/main.zig @@ -4,7 +4,7 @@ pub fn main() !void { var args = try std.process.argsWithAllocator(std.heap.page_allocator); _ = args.skip(); const filename = args.next().?; - const file = try std.fs.createFileAbsolute(filename, .{}); + const file = try std.fs.cwd().createFile(filename, .{}); defer file.close(); try file.writeAll(filename); } diff --git a/test/standalone/self_exe_symlink/create-symlink.zig b/test/standalone/self_exe_symlink/create-symlink.zig index 7bc36df8fe..dac5891ba8 100644 --- a/test/standalone/self_exe_symlink/create-symlink.zig +++ b/test/standalone/self_exe_symlink/create-symlink.zig @@ -11,5 +11,8 @@ pub fn main() anyerror!void { const exe_path = it.next() orelse unreachable; const symlink_path = it.next() orelse unreachable; - try std.fs.cwd().symLink(exe_path, symlink_path, .{}); + // If `exe_path` is relative to our cwd, we need to convert it to be relative to the dirname of `symlink_path`. + const exe_rel_path = try std.fs.path.relative(allocator, std.fs.path.dirname(symlink_path) orelse ".", exe_path); + defer allocator.free(exe_rel_path); + try std.fs.cwd().symLink(exe_rel_path, symlink_path, .{}); } From 14e033ed95a43a5ff994df9dc524e5bc4f34452a Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 5 Jun 2025 13:23:20 +0100 Subject: [PATCH 2/2] std.Build.Step.Run: convert relative paths to be relative to child cwd Because any `LazyPath` might be resolved to a relative path, it's incorrect to pass that directly to a child process whose cwd might differ. Instead, if the child has an overriden cwd, we need to convert such paths to be relative to the child cwd using `std.fs.path.relative`. --- lib/std/Build/Step/Run.zig | 68 ++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/lib/std/Build/Step/Run.zig b/lib/std/Build/Step/Run.zig index 7b5d4808a6..ca5525eff4 100644 --- a/lib/std/Build/Step/Run.zig +++ b/lib/std/Build/Step/Run.zig @@ -622,6 +622,18 @@ fn checksContainStderr(checks: []const StdIo.Check) bool { return false; } +/// If `path` is cwd-relative, make it relative to the cwd of the child instead. +/// +/// Whenever a path is included in the argv of a child, it should be put through this function first +/// to make sure the child doesn't see paths relative to a cwd other than its own. +fn convertPathArg(run: *Run, path: Build.Cache.Path) []const u8 { + const b = run.step.owner; + const path_str = path.toString(b.graph.arena) catch @panic("OOM"); + const child_lazy_cwd = run.cwd orelse return path_str; + const child_cwd = child_lazy_cwd.getPath3(b, &run.step).toString(b.graph.arena) catch @panic("OOM"); + return std.fs.path.relative(b.graph.arena, child_cwd, path_str) catch @panic("OOM"); +} + const IndexedOutput = struct { index: usize, tag: @typeInfo(Arg).@"union".tag_type.?, @@ -676,14 +688,14 @@ fn make(step: *Step, options: Step.MakeOptions) !void { man.hash.addBytes(bytes); }, .lazy_path => |file| { - const file_path = file.lazy_path.getPath2(b, step); - try argv_list.append(b.fmt("{s}{s}", .{ file.prefix, file_path })); + const file_path = file.lazy_path.getPath3(b, step); + try argv_list.append(b.fmt("{s}{s}", .{ file.prefix, run.convertPathArg(file_path) })); man.hash.addBytes(file.prefix); - _ = try man.addFile(file_path, null); + _ = try man.addFilePath(file_path, null); }, .decorated_directory => |dd| { const file_path = dd.lazy_path.getPath3(b, step); - const resolved_arg = b.fmt("{s}{}{s}", .{ dd.prefix, file_path, dd.suffix }); + const resolved_arg = b.fmt("{s}{s}{s}", .{ dd.prefix, run.convertPathArg(file_path), dd.suffix }); try argv_list.append(resolved_arg); man.hash.addBytes(resolved_arg); }, @@ -696,7 +708,10 @@ fn make(step: *Step, options: Step.MakeOptions) !void { } const file_path = artifact.installed_path orelse artifact.generated_bin.?.path.?; - try argv_list.append(b.fmt("{s}{s}", .{ pa.prefix, file_path })); + try argv_list.append(b.fmt("{s}{s}", .{ + pa.prefix, + run.convertPathArg(.{ .root_dir = .cwd(), .sub_path = file_path }), + })); _ = try man.addFile(file_path, null); }, @@ -787,11 +802,14 @@ fn make(step: *Step, options: Step.MakeOptions) !void { b.cache_root, output_sub_dir_path, @errorName(err), }); }; - const output_path = placeholder.output.generated_file.path.?; + const arg_output_path = run.convertPathArg(.{ + .root_dir = .cwd(), + .sub_path = placeholder.output.generated_file.getPath(), + }); argv_list.items[placeholder.index] = if (placeholder.output.prefix.len == 0) - output_path + arg_output_path else - b.fmt("{s}{s}", .{ placeholder.output.prefix, output_path }); + b.fmt("{s}{s}", .{ placeholder.output.prefix, arg_output_path }); } try runCommand(run, argv_list.items, has_side_effects, output_dir_path, prog_node, null); @@ -816,12 +834,15 @@ fn make(step: *Step, options: Step.MakeOptions) !void { b.cache_root, output_sub_dir_path, @errorName(err), }); }; - const output_path = try b.cache_root.join(arena, &output_components); - placeholder.output.generated_file.path = output_path; - argv_list.items[placeholder.index] = if (placeholder.output.prefix.len == 0) - output_path - else - b.fmt("{s}{s}", .{ placeholder.output.prefix, output_path }); + const raw_output_path: Build.Cache.Path = .{ + .root_dir = b.cache_root, + .sub_path = b.pathJoin(&output_components), + }; + placeholder.output.generated_file.path = raw_output_path.toString(b.graph.arena) catch @panic("OOM"); + argv_list.items[placeholder.index] = b.fmt("{s}{s}", .{ + placeholder.output.prefix, + run.convertPathArg(raw_output_path), + }); } try runCommand(run, argv_list.items, has_side_effects, tmp_dir_path, prog_node, null); @@ -899,20 +920,23 @@ pub fn rerunInFuzzMode( try argv_list.append(arena, bytes); }, .lazy_path => |file| { - const file_path = file.lazy_path.getPath2(b, step); - try argv_list.append(arena, b.fmt("{s}{s}", .{ file.prefix, file_path })); + const file_path = file.lazy_path.getPath3(b, step); + try argv_list.append(arena, b.fmt("{s}{s}", .{ file.prefix, run.convertPathArg(file_path) })); }, .decorated_directory => |dd| { const file_path = dd.lazy_path.getPath3(b, step); - try argv_list.append(arena, b.fmt("{s}{}{s}", .{ dd.prefix, file_path, dd.suffix })); + try argv_list.append(arena, b.fmt("{s}{s}{s}", .{ dd.prefix, run.convertPathArg(file_path), dd.suffix })); }, .artifact => |pa| { const artifact = pa.artifact; - const file_path = if (artifact == run.producer.?) - b.fmt("{}", .{run.rebuilt_executable.?}) - else - (artifact.installed_path orelse artifact.generated_bin.?.path.?); - try argv_list.append(arena, b.fmt("{s}{s}", .{ pa.prefix, file_path })); + const file_path: []const u8 = p: { + if (artifact == run.producer.?) break :p b.fmt("{}", .{run.rebuilt_executable.?}); + break :p artifact.installed_path orelse artifact.generated_bin.?.path.?; + }; + try argv_list.append(arena, b.fmt("{s}{s}", .{ + pa.prefix, + run.convertPathArg(.{ .root_dir = .cwd(), .sub_path = file_path }), + })); }, .output_file, .output_directory => unreachable, }