std: explicitly handle error.UnexpectedExitCode in build_runner

[iddev5]

RunStep on unexpected exit code used to return error.UncleanExit, which
was confusing and unclear. When it was changed, the error handling code
in build_runner was not modified, which produced an error trace.

This commit explicitly handles error.UnexpectedExitCode in build_runner
so that the behavior now matches that of zig 0.8.1 after which it was
regressed.

I think this fix is the correct behavior since an unexpected exit code means a failure on the part of the program which did not arose from zig side, and showing an error trace is unnecessary here, though it still returns 1 indication that something somewhere went wrong.

Closes #10381

UPDATE: Now includes test case since #11138 is merged

[matu3ba]

Can you please attach the commit, which regressed the behavior?
Ideally there would be also a test that checks the expected return code(s) to prevent future regressions.
You could use for this from https://github.com/ziglang/zig/pull/11138/files test "build and call child_process" with setting the expected return code like here: https://github.com/matu3ba/chepa/blob/52ec689b65e53bf616898d56b6f2aa7324d4b8c5/build.zig#L31

Unfortunately running zig build from zig code is still a pain in the butt, which could be solved either with #11138 or moving stuff from src/test.zig into libstd.

Another alternative is to add this to the behavior tests in test.

[iddev5]

The commit which regressed it was da2b615

I am not sure how a test case will work here, because with or without this commit, the process will return 1. The matter is that error.UnexpectedExitCode just wasn't handled in the build_runner. Adding a test would mean running the build runner for a test application and checking if it is throwing a "zig error", unless there can be any other kind of test. Are there even any test/testing framework for build runner?

[matu3ba]

Adding a test would mean running the build runner for a test application and checking if it is throwing a "zig error", unless there can be any other kind of test. Are there even any test/testing framework for build runner?

Mhm, is it unfeasible to recreate error.UnexpectedExitCode for each platform?
You can build the builder that builds hello world/something broken and check the return status + stderr, which must be empty instead of having the error trace.

[iddev5]

After thinking for a bit, I think that's a fair solution. With it, a whole line of tests can be created for build_runner and I m interested in doing it. Though that makes this PR depending on your PR #11138 (tmp.getFullPath()). For now I will be doing that in separate which can then be rebased once 11138 is merged.

[iddev5]

After doing my tests, I have concluded that it's not the right solution. The build runner in any case writes the error line that it is unexpected code to stderr along with the command executed, which is correct. It's just that stack trace is extra to it. This means that asserting stderr != 0 won't work as it will never be zero.

I am fairly confident that it's not possible to cleanly test it.

[iddev5]

I managed to make a test here: https://github.com/iddev5/zig/tree/ay-build-runner-testing-fw but this depends on #11138

[matu3ba]

progstr could be made shorter and I dont understand the use case for lib_dir

I dont understand which lib_dir is meant (its not explained in the doc comments) and per se I would assume providing the path to libstd is not a frequent use case.
Explain.

[matu3ba]

With the added doc comment it looks good to me.

[matu3ba]

I have another use case for this:
testing custom compare function of output (not yet in libstd), in my case for testing paths on user system (line-wise string from EOL to start):

The code is roughly looking like this (not sure, if calling the build.zig functions directly without setting a pile of things is feasible):

const min_prog =
    \\const stdout = @import("std").io.getStdOut();
    \\pub fn main() !u8 {
    \\  try stdout.writeAll("/bruh/ok");
    \\  return 0;
    \\}
;

// TODO simpler way to directly invoke the build.zig functions with necessary arguments?
const build_zig =
    \\  const target = b.standardTargetOptions(.{});
    \\  const mode = b.standardReleaseOptions();
    \\  const exe = b.addExecutable("chepa", "src/min_prog.zig");
    \\  exe.setTarget(target);
    \\  exe.setBuildMode(mode);
    \\  const run_cmd = exe.run();
    \\  run_cmd.step.dependOn(b.getInstallStep());
    \\  const run_cmd_step = b.step("run", "Run the app"); // runs min_prog written as zig file in test block
    \\  run_cmd_step.dependOn(&run_cmd.step);
;

test "custom user compare function" {
    // TODO set cwd to have proper zig build dir
}

This also makes me wonder, if path to libstd should be available in test blocks or what other things from the caller.

If anyone has a better idea, how I could test the change for a custom compare function within build/RunStep.zig, please let me know. Maybe build.zig needs stuff to create uniquely temporary folders, ie from a once generated seed of the build.zig hash to test itself?

[Vexu]

This defer should be one line up, though currently it doesn't matter as this test isn't even being referenced. Maybe std/build.zig would be a better place for it?

[iddev5]

Oh no, that was an accident, the test is meant to be referenced. I have moved it to std.build and pushed the changes

[andrewrk]

Reverted in c5e8477. If you would like to continue to work on these changes, please re-open and then request me as a reviewer.

[andrewrk]

Please specify what is being tested rather than referencing a github issue number

[andrewrk]

I don't think runZigBuild belongs in std.testing. We already have test/standalone/* for this.