From cfb4f48941b194151fa1b6a4958b161dba139996 Mon Sep 17 00:00:00 2001 From: Stephen Gutekanst Date: Sat, 5 Mar 2022 16:04:21 -0700 Subject: [PATCH 1/2] Revert "Revert "Merge pull request #10950 from hexops/sg/responsefiles"" This reverts commit 5ab5e2e6731a9f1198df6c53134545ccc6a6bbd3. --- lib/std/build.zig | 36 ++++++ src/main.zig | 310 +++++++++++++++++++++++++--------------------- 2 files changed, 202 insertions(+), 144 deletions(-) diff --git a/lib/std/build.zig b/lib/std/build.zig index 07e9055b1d..5dcd0e513f 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -17,6 +17,7 @@ const fmt_lib = std.fmt; const File = std.fs.File; const CrossTarget = std.zig.CrossTarget; const NativeTargetInfo = std.zig.system.NativeTargetInfo; +const Sha256 = std.crypto.hash.sha2.Sha256; pub const FmtStep = @import("build/FmtStep.zig"); pub const TranslateCStep = @import("build/TranslateCStep.zig"); @@ -2892,6 +2893,41 @@ pub const LibExeObjStep = struct { try zig_args.append("--enable-cache"); + // Windows has an argument length limit of 32,766 characters, macOS 262,144 and Linux + // 2,097,152. If our args exceed 30 KiB, we instead write them to a "response file" and + // pass that to zig, e.g. via 'zig build-lib @args.rsp' + var args_length: usize = 0; + for (zig_args.items) |arg| { + args_length += arg.len + 1; // +1 to account for null terminator + } + if (args_length >= 30 * 1024) { + const args_dir = try fs.path.join( + builder.allocator, + &[_][]const u8{ builder.pathFromRoot("zig-cache"), "args" }, + ); + try std.fs.cwd().makePath(args_dir); + + // Write the args to zig-cache/args/ to avoid conflicts with + // other zig build commands running in parallel. + const partially_quoted = try std.mem.join(builder.allocator, "\" \"", zig_args.items[2..]); + const args = try std.mem.concat(builder.allocator, u8, &[_][]const u8{ "\"", partially_quoted, "\"" }); + + var args_hash: [Sha256.digest_length]u8 = undefined; + Sha256.hash(args, &args_hash, .{}); + var args_hex_hash: [Sha256.digest_length * 2]u8 = undefined; + _ = try std.fmt.bufPrint( + &args_hex_hash, + "{s}", + .{std.fmt.fmtSliceHexLower(&args_hash)}, + ); + + const args_file = try fs.path.join(builder.allocator, &[_][]const u8{ args_dir, args_hex_hash[0..] }); + try std.fs.cwd().writeFile(args_file, args); + + zig_args.shrinkRetainingCapacity(2); + try zig_args.append(try std.mem.concat(builder.allocator, u8, &[_][]const u8{ "@", args_file })); + } + const output_dir_nl = try builder.execFromStep(zig_args.items, &self.step); const build_output_dir = mem.trimRight(u8, output_dir_nl, "\r\n"); diff --git a/src/main.zig b/src/main.zig index ebbbe6da25..958849f0a5 100644 --- a/src/main.zig +++ b/src/main.zig @@ -767,11 +767,33 @@ fn buildOutputType( } soname = .yes_default_value; - const args = all_args[2..]; - var i: usize = 0; - args_loop: while (i < args.len) : (i += 1) { - const arg = args[i]; - if (mem.startsWith(u8, arg, "-")) { + + const Iterator = struct { + resp_file: ?ArgIteratorResponseFile = null, + args: []const []const u8, + i: usize = 0, + fn next(it: *@This()) ?[]const u8 { + if (it.i >= it.args.len) { + if (it.resp_file) |*resp| return if (resp.next()) |sentinel| std.mem.span(sentinel) else null; + return null; + } + defer it.i += 1; + return it.args[it.i]; + } + }; + var args_iter = Iterator{ + .args = all_args[2..], + }; + + args_loop: while (args_iter.next()) |arg| { + if (mem.startsWith(u8, arg, "@")) { + // This is a "compiler response file". We must parse the file and treat its + // contents as command line parameters. + const resp_file_path = arg[1..]; + args_iter.resp_file = initArgIteratorResponseFile(arena, resp_file_path) catch |err| { + fatal("unable to read response file '{s}': {s}", .{ resp_file_path, @errorName(err) }); + }; + } else if (mem.startsWith(u8, arg, "-")) { if (mem.eql(u8, arg, "-h") or mem.eql(u8, arg, "--help")) { try io.getStdOut().writeAll(usage_build_generic); return cleanExit(); @@ -779,73 +801,71 @@ fn buildOutputType( if (arg_mode == .run) { // The index refers to all_args so skip `zig` `run` // and `--` - runtime_args_start = i + 3; + runtime_args_start = args_iter.i + 3; break :args_loop; } else { fatal("unexpected end-of-parameter mark: --", .{}); } } else if (mem.eql(u8, arg, "--pkg-begin")) { - if (i + 2 >= args.len) fatal("Expected 2 arguments after {s}", .{arg}); - i += 1; - const pkg_name = args[i]; - i += 1; - const pkg_path = args[i]; + const pkg_name = args_iter.next(); + const pkg_path = args_iter.next(); + if (pkg_name == null or pkg_path == null) fatal("Expected 2 arguments after {s}", .{arg}); const new_cur_pkg = Package.create( gpa, - fs.path.dirname(pkg_path), - fs.path.basename(pkg_path), + fs.path.dirname(pkg_path.?), + fs.path.basename(pkg_path.?), ) catch |err| { - fatal("Failed to add package at path {s}: {s}", .{ pkg_path, @errorName(err) }); + fatal("Failed to add package at path {s}: {s}", .{ pkg_path.?, @errorName(err) }); }; - try cur_pkg.addAndAdopt(gpa, pkg_name, new_cur_pkg); + try cur_pkg.addAndAdopt(gpa, pkg_name.?, new_cur_pkg); cur_pkg = new_cur_pkg; } else if (mem.eql(u8, arg, "--pkg-end")) { cur_pkg = cur_pkg.parent orelse fatal("encountered --pkg-end with no matching --pkg-begin", .{}); } else if (mem.eql(u8, arg, "--main-pkg-path")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - main_pkg_path = args[i]; + main_pkg_path = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "-cflags")) { extra_cflags.shrinkRetainingCapacity(0); while (true) { - i += 1; - if (i >= args.len) fatal("expected -- after -cflags", .{}); - if (mem.eql(u8, args[i], "--")) break; - try extra_cflags.append(args[i]); + const next_arg = args_iter.next() orelse { + fatal("expected -- after -cflags", .{}); + }; + if (mem.eql(u8, next_arg, "--")) break; + try extra_cflags.append(next_arg); } } else if (mem.eql(u8, arg, "--color")) { - if (i + 1 >= args.len) { + const next_arg = args_iter.next() orelse { fatal("expected [auto|on|off] after --color", .{}); - } - i += 1; - const next_arg = args[i]; + }; color = std.meta.stringToEnum(Color, next_arg) orelse { fatal("expected [auto|on|off] after --color, found '{s}'", .{next_arg}); }; } else if (mem.eql(u8, arg, "--subsystem")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - if (mem.eql(u8, args[i], "console")) { + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; + if (mem.eql(u8, next_arg, "console")) { subsystem = .Console; - } else if (mem.eql(u8, args[i], "windows")) { + } else if (mem.eql(u8, next_arg, "windows")) { subsystem = .Windows; - } else if (mem.eql(u8, args[i], "posix")) { + } else if (mem.eql(u8, next_arg, "posix")) { subsystem = .Posix; - } else if (mem.eql(u8, args[i], "native")) { + } else if (mem.eql(u8, next_arg, "native")) { subsystem = .Native; - } else if (mem.eql(u8, args[i], "efi_application")) { + } else if (mem.eql(u8, next_arg, "efi_application")) { subsystem = .EfiApplication; - } else if (mem.eql(u8, args[i], "efi_boot_service_driver")) { + } else if (mem.eql(u8, next_arg, "efi_boot_service_driver")) { subsystem = .EfiBootServiceDriver; - } else if (mem.eql(u8, args[i], "efi_rom")) { + } else if (mem.eql(u8, next_arg, "efi_rom")) { subsystem = .EfiRom; - } else if (mem.eql(u8, args[i], "efi_runtime_driver")) { + } else if (mem.eql(u8, next_arg, "efi_runtime_driver")) { subsystem = .EfiRuntimeDriver; } else { fatal("invalid: --subsystem: '{s}'. Options are:\n{s}", .{ - args[i], + next_arg, \\ console \\ windows \\ posix @@ -858,67 +878,71 @@ fn buildOutputType( }); } } else if (mem.eql(u8, arg, "-O")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - optimize_mode_string = args[i]; + optimize_mode_string = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--entry")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - entry = args[i]; + entry = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--stack")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - stack_size_override = std.fmt.parseUnsigned(u64, args[i], 0) catch |err| { + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; + stack_size_override = std.fmt.parseUnsigned(u64, next_arg, 0) catch |err| { fatal("unable to parse '{s}': {s}", .{ arg, @errorName(err) }); }; } else if (mem.eql(u8, arg, "--image-base")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - image_base_override = std.fmt.parseUnsigned(u64, args[i], 0) catch |err| { + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; + image_base_override = std.fmt.parseUnsigned(u64, next_arg, 0) catch |err| { fatal("unable to parse '{s}': {s}", .{ arg, @errorName(err) }); }; } else if (mem.eql(u8, arg, "--name")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - provided_name = args[i]; + provided_name = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "-rpath")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try rpath_list.append(args[i]); + try rpath_list.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "--library-directory") or mem.eql(u8, arg, "-L")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try lib_dirs.append(args[i]); + try lib_dirs.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "-F")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try framework_dirs.append(args[i]); + try framework_dirs.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "-framework")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try frameworks.append(args[i]); + try frameworks.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "-install_name")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - install_name = args[i]; + install_name = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "-T") or mem.eql(u8, arg, "--script")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - linker_script = args[i]; + linker_script = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--version-script")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - version_script = args[i]; + version_script = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--library") or mem.eql(u8, arg, "-l")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; // We don't know whether this library is part of libc or libc++ until // we resolve the target, so we simply append to the list for now. - i += 1; - try system_libs.put(args[i], .{ .needed = false }); + try system_libs.put(next_arg, .{ .needed = false }); } else if (mem.eql(u8, arg, "--needed-library") or mem.eql(u8, arg, "-needed-l")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try system_libs.put(args[i], .{ .needed = true }); + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; + try system_libs.put(next_arg, .{ .needed = true }); } else if (mem.eql(u8, arg, "-D") or mem.eql(u8, arg, "-isystem") or mem.eql(u8, arg, "-I") or @@ -927,31 +951,30 @@ fn buildOutputType( mem.eql(u8, arg, "-iframework") or mem.eql(u8, arg, "-iframeworkwithsysroot")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; try clang_argv.append(arg); - try clang_argv.append(args[i]); + try clang_argv.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "--version")) { - if (i + 1 >= args.len) { - fatal("expected parameter after --version", .{}); - } - i += 1; - version = std.builtin.Version.parse(args[i]) catch |err| { - fatal("unable to parse --version '{s}': {s}", .{ args[i], @errorName(err) }); + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; + version = std.builtin.Version.parse(next_arg) catch |err| { + fatal("unable to parse --version '{s}': {s}", .{ next_arg, @errorName(err) }); }; have_version = true; } else if (mem.eql(u8, arg, "-target")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - target_arch_os_abi = args[i]; + target_arch_os_abi = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "-mcpu")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - target_mcpu = args[i]; + target_mcpu = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "-mcmodel")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - machine_code_model = parseCodeModel(args[i]); + machine_code_model = parseCodeModel(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.startsWith(u8, arg, "-ofmt=")) { target_ofmt = arg["-ofmt=".len..]; } else if (mem.startsWith(u8, arg, "-mcpu=")) { @@ -961,50 +984,51 @@ fn buildOutputType( } else if (mem.startsWith(u8, arg, "-O")) { optimize_mode_string = arg["-O".len..]; } else if (mem.eql(u8, arg, "--dynamic-linker")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - target_dynamic_linker = args[i]; + target_dynamic_linker = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--sysroot")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - sysroot = args[i]; + sysroot = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; try clang_argv.append("-isysroot"); - try clang_argv.append(args[i]); + try clang_argv.append(sysroot.?); } else if (mem.eql(u8, arg, "--libc")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - libc_paths_file = args[i]; + libc_paths_file = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--test-filter")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - test_filter = args[i]; + test_filter = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--test-name-prefix")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - test_name_prefix = args[i]; + test_name_prefix = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--test-cmd")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - try test_exec_args.append(args[i]); + try test_exec_args.append(args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }); } else if (mem.eql(u8, arg, "--cache-dir")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - override_local_cache_dir = args[i]; + override_local_cache_dir = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--global-cache-dir")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - override_global_cache_dir = args[i]; + override_global_cache_dir = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--zig-lib-dir")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; - override_lib_dir = args[i]; + override_lib_dir = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; } else if (mem.eql(u8, arg, "--debug-log")) { - if (i + 1 >= args.len) fatal("expected parameter after {s}", .{arg}); - i += 1; + const next_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; if (!build_options.enable_logging) { std.log.warn("Zig was compiled without logging enabled (-Dlog). --debug-log has no effect.", .{}); } else { - try log_scopes.append(gpa, args[i]); + try log_scopes.append(gpa, next_arg); } } else if (mem.eql(u8, arg, "--debug-link-snapshot")) { if (!build_options.enable_link_snapshots) { @@ -1177,11 +1201,9 @@ fn buildOutputType( } else if (mem.eql(u8, arg, "-fno-allow-shlib-undefined")) { linker_allow_shlib_undefined = false; } else if (mem.eql(u8, arg, "-z")) { - i += 1; - if (i >= args.len) { - fatal("expected linker extension flag after '{s}'", .{arg}); - } - const z_arg = args[i]; + const z_arg = args_iter.next() orelse { + fatal("expected parameter after {s}", .{arg}); + }; if (mem.eql(u8, z_arg, "nodelete")) { linker_z_nodelete = true; } else if (mem.eql(u8, z_arg, "notext")) { @@ -4300,6 +4322,17 @@ pub fn lldMain( return @bitCast(u8, @truncate(i8, exit_code)); } +const ArgIteratorResponseFile = process.ArgIteratorGeneral(.{ .comments = true, .single_quotes = true }); + +/// Initialize the arguments from a Response File. "*.rsp" +fn initArgIteratorResponseFile(allocator: Allocator, resp_file_path: []const u8) !ArgIteratorResponseFile { + const max_bytes = 10 * 1024 * 1024; // 10 MiB of command line arguments is a reasonable limit + var cmd_line = try fs.cwd().readFileAlloc(allocator, resp_file_path, max_bytes); + errdefer allocator.free(cmd_line); + + return ArgIteratorResponseFile.initTakeOwnership(allocator, cmd_line); +} + const clang_args = @import("clang_options.zig").list; pub const ClangArgIterator = struct { @@ -4391,17 +4424,6 @@ pub const ClangArgIterator = struct { }; } - const ArgIteratorResponseFile = process.ArgIteratorGeneral(.{ .comments = true, .single_quotes = true }); - - /// Initialize the arguments from a Response File. "*.rsp" - fn initArgIteratorResponseFile(allocator: Allocator, resp_file_path: []const u8) !ArgIteratorResponseFile { - const max_bytes = 10 * 1024 * 1024; // 10 MiB of command line arguments is a reasonable limit - var cmd_line = try fs.cwd().readFileAlloc(allocator, resp_file_path, max_bytes); - errdefer allocator.free(cmd_line); - - return ArgIteratorResponseFile.initTakeOwnership(allocator, cmd_line); - } - fn next(self: *ClangArgIterator) !void { assert(self.has_next); assert(self.next_index < self.argv.len); From c3a2b51a2cdd5a64401df2ad326c20f151455f00 Mon Sep 17 00:00:00 2001 From: Stephen Gutekanst Date: Sat, 5 Mar 2022 15:23:08 -0700 Subject: [PATCH 2/2] fix regression in `zig run` runtime arguments This brings back #10950, which was reverted in 5ab5e2e6731a9f1198df6c53134545ccc6a6bbd3 because it [introduced a regression in `zig run`](https://github.com/ziglang/zig/pull/10950#issuecomment-1049481212) where the runtime arguments passed were incorrect. I've fixed the issue, and notably this was the only location where we directly relied on accessing arguments by index in this code still (all other locations use the iterator proper) and so we should be all good to go now. Signed-off-by: Stephen Gutekanst --- src/main.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main.zig b/src/main.zig index 958849f0a5..3f4d124f00 100644 --- a/src/main.zig +++ b/src/main.zig @@ -799,9 +799,9 @@ fn buildOutputType( return cleanExit(); } else if (mem.eql(u8, arg, "--")) { if (arg_mode == .run) { - // The index refers to all_args so skip `zig` `run` - // and `--` - runtime_args_start = args_iter.i + 3; + // args_iter.i is 1, referring the next arg after "--" in ["--", ...] + // Add +2 to the index so it is relative to all_args + runtime_args_start = args_iter.i + 2; break :args_loop; } else { fatal("unexpected end-of-parameter mark: --", .{});