From 9eb85c4e5eb65ae987463af33ca24225f12b3a8b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 25 May 2026 12:51:23 -0700 Subject: [PATCH] build system: track TODOs outside source code related to #363 --- build.zig | 2 -- lib/compiler/Maker.zig | 8 +++----- lib/compiler/Maker/Graph.zig | 2 +- lib/compiler/Maker/Step.zig | 4 ++-- lib/compiler/Maker/Step/InstallDir.zig | 1 - lib/compiler/Maker/Step/Run.zig | 20 ++++++++++++-------- lib/compiler/Maker/WebServer.zig | 14 -------------- lib/std/Build.zig | 6 +++++- lib/std/Build/Configuration.zig | 4 ++-- lib/std/zig.zig | 8 +++----- 10 files changed, 28 insertions(+), 41 deletions(-) diff --git a/build.zig b/build.zig index bbfb24c4b6..2f00911197 100644 --- a/build.zig +++ b/build.zig @@ -1525,8 +1525,6 @@ fn generateLangRef(b: *std.Build) !std.Build.LazyPath { cmd.addArg("--zig"); cmd.addFileArg(.zig_exe); - // TODO: enhance doctest to use "--listen=-" rather than operating in a - // temporary directory cmd.addArg("--cache-root"); cmd.addDirectoryArg(.cache_root); diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig index 0547d5fa31..643df6b68f 100644 --- a/lib/compiler/Maker.zig +++ b/lib/compiler/Maker.zig @@ -184,7 +184,6 @@ pub fn main(init: process.Init.Minimal) !void { } else if (mem.eql(u8, arg, "--sysroot")) { graph.sysroot = nextArgOrFatal(args, &arg_idx); } else if (mem.eql(u8, arg, "--maxrss")) { - // TODO refactor and reuse the fuzz number parsing here const max_rss_text = nextArgOrFatal(args, &arg_idx); max_rss = std.fmt.parseIntSizeSuffix(max_rss_text, 10) catch |err| fatal("invalid byte size {q}: {t}", .{ max_rss_text, err }); @@ -269,7 +268,6 @@ pub fn main(init: process.Init.Minimal) !void { graph.build_id = std.zig.BuildId.parse(style) catch |err| fatal("unable to parse --build-id style {q}: {t}", .{ style, err }); } else if (mem.eql(u8, arg, "--debounce")) { - // TODO refactor and reuse the timeout parsing code also here const next_arg = nextArg(args, &arg_idx) orelse fatalWithHint("expected u16 after {q}", .{arg}); debounce_interval_ms = std.fmt.parseUnsigned(u16, next_arg, 0) catch |err| { @@ -1887,7 +1885,7 @@ pub fn truncatePath( ) Step.ExtendedMakeError!void { const graph = maker.graph; const io = graph.io; - if (graph.verbose) try graph.handleVerbose(.inherit, null, &.{ + if (graph.verbose) try graph.handleVerbose(null, null, &.{ "truncate", try dest_path.toString(arena), }); const err = e: { @@ -1924,7 +1922,7 @@ pub fn installPath( ) Step.ExtendedMakeError!Dir.PrevStatus { const graph = maker.graph; const io = graph.io; - if (graph.verbose) try graph.handleVerbose(.inherit, null, &.{ + if (graph.verbose) try graph.handleVerbose(null, null, &.{ "install", "-C", try src_path.toString(arena), try dest_path.toString(arena), }); return Dir.updateFile( @@ -1949,7 +1947,7 @@ pub fn installDir( ) Step.ExtendedMakeError!Dir.CreatePathStatus { const graph = maker.graph; const io = graph.io; - if (graph.verbose) try graph.handleVerbose(.inherit, null, &.{ + if (graph.verbose) try graph.handleVerbose(null, null, &.{ "install", "-d", try dest_path.toString(arena), }); return dest_path.root_dir.handle.createDirPathStatus(io, dest_path.sub_path, .default_dir) catch |err| { diff --git a/lib/compiler/Maker/Graph.zig b/lib/compiler/Maker/Graph.zig index bca008ffc0..116132c614 100644 --- a/lib/compiler/Maker/Graph.zig +++ b/lib/compiler/Maker/Graph.zig @@ -69,7 +69,7 @@ enable_rosetta: bool = false, /// before spawning them. pub fn handleVerbose( graph: *const Graph, - cwd: std.process.Child.Cwd, + cwd: ?[]const u8, opt_env: ?*const std.process.Environ.Map, argv: []const []const u8, ) error{OutOfMemory}!void { diff --git a/lib/compiler/Maker/Step.zig b/lib/compiler/Maker/Step.zig index 66e6079ab3..bb65abea0e 100644 --- a/lib/compiler/Maker/Step.zig +++ b/lib/compiler/Maker/Step.zig @@ -341,7 +341,7 @@ pub fn captureChildProcess(s: *Step, maker: *Maker, options: CaptureChildProcess s.result_failed_command = try std.zig.allocPrintCmd(gpa, options.argv, .{}); try handleChildProcUnsupported(s, maker); - try graph.handleVerbose(.inherit, null, options.argv); + try graph.handleVerbose(null, null, options.argv); const result = std.process.run(arena, io, .{ .argv = options.argv, @@ -459,7 +459,7 @@ pub fn evalZigProcess( assert(argv.len != 0); try handleChildProcUnsupported(s, maker); - try graph.handleVerbose(.inherit, null, argv); + try graph.handleVerbose(null, null, argv); const zp = try gpa.create(ZigProcess); defer if (!watch) gpa.destroy(zp); diff --git a/lib/compiler/Maker/Step/InstallDir.zig b/lib/compiler/Maker/Step/InstallDir.zig index 484bebf329..d8b7b7ed18 100644 --- a/lib/compiler/Maker/Step/InstallDir.zig +++ b/lib/compiler/Maker/Step/InstallDir.zig @@ -81,7 +81,6 @@ pub fn make( .file => { for (blank_extensions) |ext| { if (endsWith(u8, entry.path, ext.slice(conf))) { - // TODO check if the file was already there and length 0 try maker.truncatePath(arena, dest_path, step_index); continue :next_entry; } diff --git a/lib/compiler/Maker/Step/Run.zig b/lib/compiler/Maker/Step/Run.zig index e8c90424b1..5c17a637a4 100644 --- a/lib/compiler/Maker/Step/Run.zig +++ b/lib/compiler/Maker/Step/Run.zig @@ -1796,7 +1796,12 @@ fn runCommand( } } - try graph.handleVerbose(cwd, environ_map, argv); + const cwd_string = switch (cwd) { + .path => |p| p, + .dir => unreachable, + .inherit => null, + }; + try graph.handleVerbose(cwd_string, environ_map, argv); const opt_generic_result = spawnChildAndCollect( run_index, @@ -1812,10 +1817,6 @@ fn runCommand( error.InvalidExe, // cpu arch mismatch error.FileNotFound, // can happen with a wrong dynamic linker path => interpret: { - // TODO: learn the target from the binary directly rather than from - // relying on it being a Compile step. This will make this logic - // work even for the edge case that the binary was produced by a - // third party. const producer_index = arg0.producer.value orelse break :interpret; const producer_step = producer_index.ptr(conf); const producer = producer_step.extended.get(conf.extra).compile; @@ -1829,7 +1830,6 @@ fn runCommand( const root_target = std.zig.system.resolveTargetQuery(io, other_target_query) catch unreachable; const link_libc = maker.stepByIndex(producer_index).extended.compile.is_linking_libc; - // TODO get this from the parent process instead const host: std.Target = std.zig.system.resolveTargetQuery(io, .{}) catch |he| switch (he) { error.Canceled => |e| return e, else => builtin.target, @@ -1941,7 +1941,7 @@ fn runCommand( gpa.free(step.result_failed_command.?); step.result_failed_command = null; - try graph.handleVerbose(cwd, environ_map, interp_argv.items); + try graph.handleVerbose(cwd_string, environ_map, interp_argv.items); break :term spawnChildAndCollect( run_index, @@ -2148,7 +2148,11 @@ fn spawnChildAndCollect( // If an error occurs, it's caused by this command: assert(step.result_failed_command == null); step.result_failed_command = try std.zig.allocPrintCmd(gpa, argv, .{ - .cwd = child_cwd, + .cwd = switch (child_cwd) { + .path => |p| p, + .dir => unreachable, + .inherit => null, + }, .child_env = environ_map, .parent_env = &graph.environ_map, }); diff --git a/lib/compiler/Maker/WebServer.zig b/lib/compiler/Maker/WebServer.zig index de30ca3073..943265c939 100644 --- a/lib/compiler/Maker/WebServer.zig +++ b/lib/compiler/Maker/WebServer.zig @@ -225,7 +225,6 @@ pub fn updateStepStatus( ) void { const maker = ws.maker; const all_steps = maker.step_stack.keys(); - // TODO don't do linear search, especially in a hot loop like this const step_idx: u32 = for (all_steps, 0..) |s, i| { if (s == step_index) break @intCast(i); } else unreachable; @@ -569,16 +568,6 @@ pub fn serveTarFile(ws: *WebServer, request: *http.Server.Request, paths: []cons var read_buffer: [1024]u8 = undefined; var file_reader: Io.File.Reader = .initSize(file, io, &read_buffer, stat.size); - // TODO: this logic is completely bogus -- obviously so, because `path.root_dir.path` can - // be cwd-relative. This is also related to why linkification doesn't work in the fuzzer UI: - // it turns out the WASM treats the first path component as the module name, typically - // resulting in modules named "" and "src". The compiler needs to tell the build system - // about the module graph so that the build system can correctly encode this information in - // the tar file. - // - // Additionally, this needs to ensure that all path separators for both prefix and - // sub_path are using the POSIX-style `/` on platforms that don't use it as their native - // path separator. archiver.prefix = path.root_dir.path orelse graph.cache.cwd; try archiver.writeFile(path.sub_path, &file_reader, @intCast(stat.mtime.toSeconds())); } @@ -799,7 +788,6 @@ pub fn updateTimeReportCompile(ws: *WebServer, opts: struct { const io = maker.graph.io; const all_steps = maker.step_stack.keys(); - // TODO don't do linear search const step_idx: u32 = for (all_steps, 0..) |s, i| { if (s == opts.compile_step) break @intCast(i); } else unreachable; @@ -843,7 +831,6 @@ pub fn updateTimeReportGeneric(ws: *WebServer, step_index: Configuration.Step.In const io = maker.graph.io; const all_steps = maker.step_stack.keys(); - // TODO don't do linear search const step_idx: u32 = for (all_steps, 0..) |s, i| { if (s == step_index) break @intCast(i); } else unreachable; @@ -882,7 +869,6 @@ pub fn updateTimeReportRunTest( const io = maker.graph.io; const all_steps = maker.step_stack.keys(); - // TODO don't do linear search const step_idx: u32 = for (all_steps, 0..) |s, i| { if (s == run_step_index) break @intCast(i); } else unreachable; diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 3b31500f2c..d8948380de 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -1890,7 +1890,11 @@ pub fn runFallible(b: *Build, argv: []const []const u8, options: RunOptions) Run const arena = graph.arena; const print_opts: std.zig.AllocPrintCmdOptions = .{ - .cwd = options.cwd, + .cwd = switch (options.cwd) { + .inherit => null, + .path => |p| p, + .dir => null, // Unknown without changing function signature of runFallible. + }, .child_env = options.environ_map, .parent_env = &graph.environ_map, }; diff --git a/lib/std/Build/Configuration.zig b/lib/std/Build/Configuration.zig index 5041fbe4ef..a893441575 100644 --- a/lib/std/Build/Configuration.zig +++ b/lib/std/Build/Configuration.zig @@ -3099,7 +3099,7 @@ pub const Storage = enum { .value = if (match) dataField(buffer, i, container, Field.Value) else null, }; }, - .extended => @compileError("TODO"), + .extended => @compileError("unimplemented"), .length_prefixed_list => { const n = @divExact(@sizeOf(Field.Elem), @sizeOf(u32)); const data_start = i.* + 1; @@ -3260,7 +3260,7 @@ pub const Storage = enum { .flag_union => return switch (value.u) { inline else => |x| setExtraField(buffer, i, @TypeOf(x), x), }, - .extended => @compileError("TODO"), + .extended => @compileError("unimplemented"), .flag_length_prefixed_list => { const len: u32 = @intCast(value.slice.len); if (len == 0) return 0; // Flag bit hides the length prefix. diff --git a/lib/std/zig.zig b/lib/std/zig.zig index 2a3b4792a8..db5e20c8d1 100644 --- a/lib/std/zig.zig +++ b/lib/std/zig.zig @@ -1168,7 +1168,7 @@ pub const ClangCliParam = struct { }; pub const AllocPrintCmdOptions = struct { - cwd: std.process.Child.Cwd = .inherit, + cwd: ?[]const u8 = null, parent_env: ?*const std.process.Environ.Map = null, child_env: ?*const std.process.Environ.Map = null, }; @@ -1212,10 +1212,8 @@ pub fn allocPrintCmd(gpa: Allocator, argv: []const []const u8, options: AllocPri var aw: Io.Writer.Allocating = .init(gpa); defer aw.deinit(); const writer = &aw.writer; - switch (options.cwd) { - .inherit => {}, - .path => |path| writer.print("cd {s} && ", .{path}) catch return error.OutOfMemory, - .dir => @panic("TODO"), + if (options.cwd) |path| { + writer.print("cd {s} && ", .{path}) catch return error.OutOfMemory; } if (options.child_env) |child_env| { for (child_env.keys(), child_env.values()) |key, value| {