let std.Build.add{Test,Executable} take a root module and target/optimize options

[dweiller]

The recent std.Build API enhancement in #20388 allows (and will require once deprecated options are remove) passing an explicit root module when using addTest or addExecutable. I think there is a footgun with the new API for packages that export a module to dependents via addModule and also want to use them as the root module of a test or other executable with different target and optimize options. This use-case would benefit from being able to take advantage of the new API by passing the std.Build.Module returned by addModule to addTest and addExecutable, making build scripts more straightforward and avoiding the footgun described below simultaneously.

Consider this build.zig using the old API:

pub fn build(*std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const foo = b.addModule("foo", .{
        .root_source_file = b.path("src/foo.zig"),
    });

    const foo_tests = b.addTest(.{
        .root_source_file = b.path("src/foo.zig"),
        .target = target,
        .optimize = optimize,
    });
    const run_foo_tests = b.addRunArtifact(foo_tests);

    const test_step = b.step("test", "Run tests");
    test_step.dependOn(&run_foo_tests.step);
}

This allows the package to export the foo module to dependents so that it will inherit the target and optimize options from the parent module while also running tests with different target and optimize options.

The new API uses a root_module option rather than root_source_module in addTest so the proper way to upgrade this build.zig to the new API is:

     const foo_tests = b.addTest(.{
-        .root_source_file = b.path("src/foo.zig"),
-        .target = target,
-        .optimize = optimize,
+        .root_module = b.createModule(.{
+            .root_source_path = b.path("src/foo.zig"),
+            .target = target,
+            .optimize = optimize,
+        }),
     });

which means you need to duplicate any setup the foo module needs between the module exported to dependents and the module passed to addTest. The footgun mentioned above is that I think it's quite likely that people will end up doing this if they're not paying attention/aware of the difference:

     const foo = b.addModule("foo", .{
         .root_source_file = b.path("src/foo.zig"),
+        .target = target,
+        .optimize = optimize,
     });

     const foo_tests = b.addTest(.{
-        .root_source_file = b.path("src/foo.zig"),
-        .target = target,
-        .optimize = optimize,
+        .root_module = foo,
     });

leading to the foo module being compiled with native target and Debug optimize mode unless every dependency in the chain between the end user and the foo package correctly passing -Doptimize and -Dtarget to their dependencies.

I think an improvement to the API would allow setting the target and optimize options in test options so you instead write

    const foo = b.addModule("foo", .{
        .root_source_file = b.path("src/foo.zig"),
    });

    const foo_tests = b.addTest(.{
        .root_module = foo,
        .target = target,
        .optimize = optimize,
    });

This API has the benefit that it allows sharing all the setup needed for the foo module, making the correct way to upgrade the original build.zig above the easiest:

     const foo_tests = b.addTest(.{
-        .root_source_file = b.path("src/foo.zig"),
+        .root_module = foo,
         .target = target,
         .optimize = optimize,
     });

and making it less likely that transitive dependencies end up being erroneously compiled with native target and debug optimization mode.

This would require some changes to how these options are handled - we wouldn't want to mutate foo in the addTest call to set the target and optimization modes, and we don't want to clone the module to make a new one with different options. We may need to set these options the compile step and have the settings resolved when the build system hands things off to the compiler, either overriding the options on the root module or providing a default when the module settings are unset while erroring when set on both the root module and compile step.

[dweiller]

As an aside I think this pattern should probably be mentioned in the 0.14 release notes for the #20388 changes to help make sure anyone upgrading their addTest/addExecutable usage gets it right.

[BratishkaErik]

Related: #20042

I' more on the side of always passing needed options as second parameter to dependnecy because of that case

[dweiller]

Perhaps simpler approach could be to change the standardTargetOptions and standardOptimizeOption to return an optional so you get null if nothing is specified on the CLI and have the build.zig pick a default if needed. This would mean that if a dependent doesn't pass those options an exported module can still use them and the default behaviour would be that the exported module inherits its target an optimization mode.

[nektro]

cc #22285