build: specify a step option to avoid being run in parallel

[davidgmbb]

Zig Version

0.11.0-dev.2146+9964f1c16

Steps to Reproduce and Observed Behavior

Currently when running custom steps, all are run at the same time. If have a child process executing QEMU and two instances of QEMU are run at the same time, the second instance would error with something in the lines of:

qemu-system-x86_64: Failed to get "write" lock
Is another process using the image [zig-cache/rise_ReleaseFast_x86_64_rise_bios_test.hdd]?

I suppose the Zig user (the programmer of such project) could handle this, but I think it would be very tedious to do.

Expected Behavior

One should be able to specify for a particular step not to be run in parallel, but one at a time. Another reason why this would be annoying even if it was possible to do it is that you get logs mixed.

[InKryption]

A possible work around comes to mind: would making one of the qemu steps depend on the other work?

[davidgmbb]

Currently my test_all step is a dummy step depending on all the others (which are custom too). I could have a test_all step written, which probably is the way to go. Just to give a quick feel of how I have structured my run steps right now after the build parallel changes fixes. Probably there is a better way and I also want to have the core members' opinion on how to best structure this and what direction they are moving on, so I can stick to the best code according to my needs. The code is not published yet, but if needed I can push it.

const RunSteps = struct {
    configuration: Configuration,
    run: Step,
    debug: Step,
    gdb_script: Step,
    disk_image_builder: *RunStep,

    fn getGDBScriptPath(configuration: Configuration) ![]const u8 {
        return try std.mem.concat(b.allocator, u8, &.{ "zig-cache/gdb_script_", @tagName(configuration.bootloader), "_", @tagName(configuration.architecture), "_", @tagName(configuration.boot_protocol) });
    }

    const RunStepsSetup = struct {
        cpu_driver: *CompileStep,
        bootloader_step: ?*CompileStep,
        disk_image_builder: *CompileStep,
        user_modules: []const *CompileStep,
        disk_image_builder_run: *RunStep,
    };

    fn createStep(run_steps: *RunSteps, comptime step: []const u8, setup: RunStepsSetup, suffix: []const u8) !void {
        if (setup.bootloader_step) |bs| {
            @field(run_steps, step).dependOn(&bs.step);
            setup.disk_image_builder_run.step.dependOn(&bs.step);
        }

        for (setup.user_modules) |user_module| {
            @field(run_steps, step).dependOn(&user_module.step);
            setup.disk_image_builder_run.step.dependOn(&user_module.step);
        }

        @field(run_steps, step).dependOn(&setup.cpu_driver.step);
        setup.disk_image_builder_run.step.dependOn(&setup.cpu_driver.step);

        @field(run_steps, step).dependOn(&setup.disk_image_builder.step);
        @field(run_steps, step).dependOn(&setup.disk_image_builder_run.step);

        const final_step = b.step(try std.mem.concat(b.allocator, u8, &.{ @tagName(setup.cpu_driver.kind), "_", suffix }), "Run the operating system through an emulator");
        final_step.dependOn(&@field(run_steps, step));
    }

    const RunError = error{
        failure,
    };

    fn run(step: *Step, progress_node: *std.Progress.Node) !void {
        _ = progress_node;
        const run_steps = @fieldParentPtr(RunSteps, "run", step);

        const is_debug = false;
        const is_test = run_steps.configuration.executable_kind == .@"test";
        const arguments = try qemuCommon(run_steps, .{ .is_debug = is_debug, .is_test = is_test });

        var process = std.ChildProcess.init(arguments.list.items, b.allocator);

        switch (try process.spawnAndWait()) {
            .Exited => |exit_code| {
                if (exit_code & 1 == 0) {
                    return RunError.failure;
                }

                const mask = common.maxInt(@TypeOf(exit_code)) - 1;
                const masked_exit_code = exit_code & mask;

                if (masked_exit_code == 0) {
                    return RunError.failure;
                }

                const qemu_exit_code = @intToEnum(common.QEMU.ExitCode, masked_exit_code >> 1);

                if (qemu_exit_code != .success) {
                    return RunError.failure;
                }
            },
            else => return RunError.failure,
        }
    }

    fn debug(step: *Step, progress_node: *std.Progress.Node) !void {
        _ = progress_node;
        const run_steps = @fieldParentPtr(RunSteps, "debug", step);
        const is_debug = true;
        const is_test = run_steps.configuration.executable_kind == .@"test";
        var arguments = try qemuCommon(run_steps, .{ .is_debug = is_debug, .is_test = is_test });

        if (!(run_steps.configuration.execution_type == .accelerated or (arguments.config.virtualize orelse false))) {
            try arguments.list.append("-S");
        }

        try arguments.list.append("-s");

        const debugger_process_arguments = switch (common.os) {
            .linux => .{ "kitty", "gdb", "-x", try getGDBScriptPath(run_steps.configuration) },
            else => return Error.not_implemented,
        };

        var debugger_process = std.ChildProcess.init(&debugger_process_arguments, b.allocator);
        _ = try debugger_process.spawn();

        var qemu_process = std.ChildProcess.init(arguments.list.items, b.allocator);
        _ = try qemu_process.spawnAndWait();
    }

    fn gdbScript(step: *Step, progress_node: *std.Progress.Node) !void {
        _ = progress_node;
        const run_steps = @fieldParentPtr(RunSteps, "gdb_script", step);

        var gdb_script_buffer = std.ArrayList(u8).init(b.allocator);
        const architecture = run_steps.configuration.architecture;
        common.log.debug("Architecture: {}", .{architecture});
        switch (architecture) {
            .x86_64 => try gdb_script_buffer.appendSlice("set disassembly-flavor intel\n"),
            else => return Error.architecture_not_supported,
        }

        try gdb_script_buffer.appendSlice(try std.mem.concat(b.allocator, u8, &.{ "symbol-file zig-cache/cpu_driver_", try Suffix.cpu_driver.fromConfiguration(b.allocator, run_steps.configuration, null), "\n" }));
        try gdb_script_buffer.appendSlice("target remote localhost:1234\n");

        const base_gdb_script = try std.fs.cwd().readFileAlloc(b.allocator, "config/gdb_script", common.maxInt(usize));
        try gdb_script_buffer.appendSlice(base_gdb_script);

        try std.fs.cwd().writeFile(try getGDBScriptPath(run_steps.configuration), gdb_script_buffer.items);
    }

    const QEMUOptions = packed struct {
        is_test: bool,
        is_debug: bool,
    };

    fn qemuCommon(run_steps: *RunSteps, qemu_options: QEMUOptions) !struct { config: Arguments, list: std.ArrayList([]const u8) } {
        const config_file = try std.fs.cwd().readFileAlloc(b.allocator, try std.mem.concat(b.allocator, u8, &.{"config/" ++ @tagName(run_steps.configuration.execution_environment) ++ ".json"}), common.maxInt(usize));
        var token_stream = std.json.TokenStream.init(config_file);
        const arguments = try std.json.parse(Arguments, &token_stream, .{ .allocator = b.allocator });

        var argument_list = std.ArrayList([]const u8).init(b.allocator);

        try argument_list.append(try std.mem.concat(b.allocator, u8, &.{ "qemu-system-", @tagName(run_steps.configuration.architecture) }));

        if (qemu_options.is_test and !qemu_options.is_debug) {
            try argument_list.appendSlice(&.{ "-device", b.fmt("isa-debug-exit,iobase=0x{x:0>2},iosize=0x{x:0>2}", .{ common.QEMU.isa_debug_exit.io_base, common.QEMU.isa_debug_exit.io_size }) });
        }

        switch (run_steps.configuration.boot_protocol) {
            .uefi => try argument_list.appendSlice(&.{ "-bios", "tools/OVMF_CODE-pure-efi.fd" }),
            else => {},
        }

        var test_configuration = run_steps.configuration;
        test_configuration.executable_kind = .@"test";

        const image_config = try common.ImageConfig.get(b.allocator, common.ImageConfig.default_path);
        const disk_image_path = try std.mem.concat(b.allocator, u8, &.{ "zig-cache/", image_config.image_name, try Suffix.image.fromConfiguration(b.allocator, if (qemu_options.is_test) test_configuration else run_steps.configuration, "_"), ".hdd" });
        try argument_list.appendSlice(&.{ "-drive", b.fmt("file={s},index=0,media=disk,format=raw", .{disk_image_path}) });

        try argument_list.append("-no-reboot");

        if (!qemu_options.is_test) {
            try argument_list.append("-no-shutdown");
        }

        if (ci) {
            try argument_list.appendSlice(&.{ "-display", "none" });
        }

        //if (arguments.vga) |vga| {
        //try argument_list.append("-vga");
        //try argument_list.append(@tagName(vga));
        //}

        if (arguments.smp) |smp| {
            try argument_list.append("-smp");
            const smp_string = b.fmt("{}", .{smp});
            try argument_list.append(smp_string);
        }

        if (arguments.debugcon) |debugcon| {
            try argument_list.append("-debugcon");
            try argument_list.append(@tagName(debugcon));
        }

        if (arguments.memory) |memory| {
            try argument_list.append("-m");
            const memory_argument = b.fmt("{}{c}", .{ memory.amount, @as(u8, switch (memory.unit) {
                .kilobyte => 'K',
                .megabyte => 'M',
                .gigabyte => 'G',
                else => @panic("Unit too big"),
            }) });
            try argument_list.append(memory_argument);
        }

        if (canVirtualizeWithQEMU(run_steps.configuration.architecture) and (run_steps.configuration.execution_type == .accelerated or (arguments.virtualize orelse false))) {
            try argument_list.appendSlice(&.{
                "-accel",
                switch (common.os) {
                    .windows => "whpx",
                    .linux => "kvm",
                    .macos => "hvf",
                    else => @compileError("OS not supported"),
                },
                "-cpu",
                "host",
            });
        } else {
            // switch (common.cpu.arch) {
            //     .x86_64 => try argument_list.appendSlice(&.{ "-cpu", "qemu64,level=11,+x2apic" }),
            //     else => return Error.architecture_not_supported,
            // }

            if (arguments.trace) |tracees| {
                for (tracees) |tracee| {
                    const tracee_slice = b.fmt("-{s}*", .{tracee});
                    try argument_list.append("-trace");
                    try argument_list.append(tracee_slice);
                }
            }

            if (arguments.log) |log_configuration| {
                var log_what = std.ArrayList(u8).init(b.allocator);

                if (log_configuration.guest_errors) try log_what.appendSlice("guest_errors,");
                if (log_configuration.interrupts) try log_what.appendSlice("int,");
                if (!ci and log_configuration.assembly) try log_what.appendSlice("in_asm,");

                if (log_what.items.len > 0) {
                    // Delete the last comma
                    _ = log_what.pop();

                    try argument_list.append("-d");
                    try argument_list.append(log_what.items);

                    if (log_configuration.interrupts) {
                        try argument_list.appendSlice(&.{ "-machine", "smm=off" });
                    }
                }

                if (log_configuration.file) |log_file| {
                    try argument_list.append("-D");
                    try argument_list.append(log_file);
                }
            }
        }

        return .{ .config = arguments, .list = argument_list };
    }

    const Arguments = struct {
        const VGA = enum {
            std,
            cirrus,
            vmware,
            qxl,
            xenfb,
            tcx,
            cg3,
            virtio,
            none,
        };
        memory: ?struct {
            amount: u64,
            unit: common.SizeUnit,
        },
        virtualize: ?bool,
        vga: ?VGA,
        smp: ?usize,
        debugcon: ?enum {
            stdio,
        },
        log: ?struct {
            file: ?[]const u8,
            guest_errors: bool,
            assembly: bool,
            interrupts: bool,
        },
        trace: ?[]const []const u8,
    };
};

[davidgmbb]

This is the build.zig before the build parallel PR was merged:
https://github.com/davidgm94/rise/blob/caf3b9d9d21d869faa75c10f5b5cc8efbe4bc7f5/build.zig

[mitchellh]

I support this request and you likely already thought of this, but maybe introducing a [hopefully temporary] global mutex on this step for now.

[davidgmbb]

Thanks, yes, probably that's the best temporary solution :)