std.Build: add new functions to create artifacts/Step.Compile from existing module
Continued where @mlugg left in https://github.com/ziglang/zig/pull/18752. I will not duplicate that description here, because this PR is just a revival.
The first commit of this PR also makes a change (suggested by @andrewrk) to std.Build.Module: it now participates directly in the step graph. This allows us to do away with a bunch of dependency tracking logic there, since it's all handled by the build runner!
Unfortunately, I could not figure out how to resolve recursive and mutual dependencies between modules with this model, so I leaved this change out.
Also, since modules are now exposed to the users that run zig init, I added some small comments that explain to user what is a module and how to use it.
I wanted to add more comments that explain what is a package/artifact, how to use b.dependency() and etc., but this was going out of scope in this PR, I'm leaving it to the future. BTW I think zig init should be splitted again to zig init exe and zig init lib, it's easier to explain to users when you don't have this partial duplication.
Unfortunately, I could not figure out how to resolve recursive and mutual dependencies between modules with this model, so I leaved this change out.
I think that's the right move. Modeling modules as build steps would be convenient if it worked, but it's not really logically coherent, and if it causes issues (which, as you say, it does), that's our fault for misusing the abstraction!
Nice, I needed something like this the other day for TigerBeetle --- we have a vsr module, which we'd love to use as a dependency for other modules, but which also includes a bunch of internal tests. So, ideally, we'd be able to pass the same vsr_module into other_modulue.addImport as well as b.addTest. Right now, we have to essentially duplicate vsr_module definition for the test_step.root_module module created by addTest.
This would be a convenient addition
x86_64-windows-release CI failure seems unrelated. How it there an error during checkout?
It seems like this PR has had no outstanding issues for a while (over 2 months) now and keeps getting rebased/updated several times a week. First off, thank you @BratishkaErik for your continued efforts. Maybe I'm misjudging the effort involved, but I hope you're not getting burnt out by the continued work.
Could maybe one of the maintainers take another look for reviewing/merging this? Seems like @andrewrk signed off the last changes in std.Build.
Otherwise it might make more sense to just keep this lying for a while? (On one hand to prevent burnout and reduce human labor, to a lesser degree also to spare CI resources?)
It seems like this PR has had no outstanding issues for a while (over 2 months) now and keeps getting rebased/updated several times a week. First off, thank you @BratishkaErik for your continued efforts. Maybe I'm misjudging the effort involved, but I hope you're not getting burnt out by the continued work.
Glad to help, and don't worry I'm not getting burnt out by this, it's relatively easy to rebase when you do it often and do not let too many conflicts to accumulate.
Could maybe one of the maintainers take another look for reviewing/merging this? Seems like @andrewrk signed off the last changes in
std.Build. Otherwise it might make more sense to just keep this lying for a while? (On one hand to prevent burnout and reduce human labor, to a lesser degree also to spare CI resources?)
Would be nice to get any responce from team, at least rejection :)
Glad to help, and don't worry I'm not getting burnt out by this, it's relatively easy to rebase when you do it often and do not let too many conflicts to accumulate.
Just want to note that there's not really a reason for you to be rebasing it. I believe it's better to let it sit on green until a team member reviews it. They will do the rebase themselves before merging or ask you.
Just want to note that there's not really a reason for you to be rebasing it. I believe it's better to let it sit on green until a team member reviews it. They will do the rebase themselves before merging or ask you.
Ok, will do so.
It's not super clear to me what the motivation of this is. Could somebody provide some text that could be copy pasted directly into release notes if this were to be merged?
I can do so tomorrow (I know you're also waiting on me to do another release notes comment on a PR, sorry!).
Summary for Andrew
This API solves the problem of reusing a module graph in different contexts. As a motivating example, suppose you're writing a program with 3 modules, a, b, c, where a depends on b depends on c. a is the root module of your executable, but all 3 modules have tests in them, which you'd like test-a, test-b, test-c steps for. Currently, you have to:
- create
std.Build.Modules forbandc, setting up the necessary import - create an executable whose root source file is that of
a, and addbas an import for the new root module - create a test whose root source file is that of
a, and addbas an import for the new root module - create a test whose root source file is that of
b, and addcas an import for the new root module - create a test whose root source file is that of
c
So, in the entire step graph, we end up with a lot of duplicated modules:
- 2 copies of
a(the root module of the exe and the root module of the test) - 2 copes of
b(the root module of the test and the separate module for importing) - 2 copes of
c(the root module of the test and the separate module for importing)
This duplication makes the build system quite clunky and unintuitive to use here. The idea behind this PR is that you instead can create all of your modules (a, b, c) upfront a single time, declaring the appropriate imports, and then create compile steps based on those existing modules. So, the process becomes:
- create
std.Build.Modules fora,b,c, setting up the necessary imports - create an executable from
a - create a test from
a - create a test from
b - create a test from
c
Hopefully you can see that this is much simpler.
This API could exist in addition to the old addExecutable etc APIs. However, the new API isn't really any worse for the very simple use cases which status quo fits: you basically just need to add a .root_module = b.createModule(.{ into your addExecutable options. So, the plan is to replace the old APIs entirely.
For a real-world use case, take a look at this file in the diff of my bit-rotted implementation. This logic used to reach into build system internals to work around precisely the awkwardness described above.
Release Notes
Zig 0.14.0 adds new APIs to the build system to create Compile steps from existing std.Build.Module objects. This allows for module graphs to be defined in a more clear manner, and for components of these graphs to be reused more easily; for instance, a module which exists as a dependency of another can easily have a corresponding test step created. Currently, these APIs exist as addExecutable2, addTest2, addObject2, addStaticLibrary2, and addSharedLibrary2. In the future, they will be renamed to replace the old functions without the 2 suffix. The old APIs (addExecutable etc) are deprecated and will be removed in a future release.
Here is an example of using the new APIs to elegantly construct a build graph containing multiple modules:
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
// First, we create our 3 modules.
const foo = b.createModule(.{
.root_source_file = b.path("src/foo.zig"),
.target = target,
.optimize = optimize,
});
const bar = b.createModule(.{
.root_source_file = b.path("src/bar.zig"),
.target = target,
.optimize = optimize,
});
const qux = b.createModule(.{
.root_source_file = b.path("src/qux.zig"),
.target = target,
.optimize = optimize,
});
// Next, we set up all of their dependencies.
foo.addImport("bar", bar);
foo.addImport("qux", qux);
bar.addImport("qux", qux);
// Finally, we will create all of our `Compile` steps.
// `foo` will be the root of an executable, but all 3 modules also have unit tests we want to run.
const foo_exe = b.addExecutable2(.{
.name = "foo",
.root_module = foo,
});
b.installArtifact(foo_exe);
const foo_test = b.addTest2(.{
.name = "foo",
.root_module = foo,
});
const bar_test = b.addTest2(.{
.name = "bar",
.root_module = bar,
});
const qux_test = b.addTest2(.{
.name = "qux",
.root_module = qux,
});
const test_step = b.step("test", "Run all unit tests");
test_step.dependOn(&b.addRunArtifact(foo_test).step);
test_step.dependOn(&b.addRunArtifact(bar_test).step);
test_step.dependOn(&b.addRunArtifact(qux_test).step);
}
const std = @import("std");
Users of the legacy APIs can easily upgrade by just moving the module-specific parts of the addExecutable (etc) call into a createModule call. For instance, here is the updated version of a simple build script:
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
const exe = b.addExecutable(.{
.name = "hello",
.root_source_file = b.path("src/main.zig"),
.target = target,
.optimize = optimize,
});
b.installArtifact(exe);
}
const std = @import("std");
-->
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
const exe = b.addExecutable2(.{
.name = "hello",
.root_module = b.createModule(.{ // this line was added
.root_source_file = b.path("src/main.zig"),
.target = target,
.optimize = optimize,
}), // this line was added
});
b.installArtifact(exe);
}
const std = @import("std");
As another example, here's a build script from my project where this would be nice - exporting a module for other packages to use and adding a test:
Before:
const texteditor_mod = b.addModule("texteditor", .{
.root_source_file = b.path("src/root.zig"),
.target = target,
.optimize = optimize,
});
texteditor_mod.addImport("blocks", blocks_dep.module("blocks"));
texteditor_mod.addImport("grapheme_cursor", seg_dep.module("grapheme_cursor"));
texteditor_mod.addImport("anywhere", anywhere_dep.module("anywhere"));
texteditor_mod.addImport("tree_sitter", tree_sitter_dep.module("tree_sitter"));
texteditor_mod.linkLibrary(tree_sitter_zig_obj);
texteditor_mod.linkLibrary(tree_sitter_markdown_obj);
const texteditor_test = b.addTest(.{
.root_source_file = b.path("src/root.zig"),
.target = target,
.optimize = optimize,
});
texteditor_test.root_module.addImport("blocks", blocks_dep.module("blocks"));
texteditor_test.root_module.addImport("grapheme_cursor", seg_dep.module("grapheme_cursor"));
texteditor_test.root_module.addImport("anywhere", anywhere_dep.module("anywhere"));
texteditor_test.root_module.addImport("tree_sitter", tree_sitter_dep.module("tree_sitter"));
texteditor_test.root_module.linkLibrary(tree_sitter_zig_obj);
texteditor_test.root_module.linkLibrary(tree_sitter_markdown_obj);
After:
const texteditor_mod = b.addModule("texteditor", .{
.root_source_file = b.path("src/root.zig"),
.target = target,
.optimize = optimize,
});
texteditor_mod.addImport("blocks", blocks_dep.module("blocks"));
texteditor_mod.addImport("grapheme_cursor", seg_dep.module("grapheme_cursor"));
texteditor_mod.addImport("anywhere", anywhere_dep.module("anywhere"));
texteditor_mod.addImport("tree_sitter", tree_sitter_dep.module("tree_sitter"));
texteditor_mod.linkLibrary(tree_sitter_zig_obj);
texteditor_mod.linkLibrary(tree_sitter_markdown_obj);
const texteditor_test = b.addTest2(.{
.root_module = texteditor_mod,
});
Currently, these APIs exist as
addExecutable2,addTest2,addObject2,addStaticLibrary2, andaddSharedLibrary2.
I suppose this should've been "and addLibrary2", unless core team decided to go with old splitted functions? Anyway, many thanks to you mlugg for everything in this and previous PR, from de-facto implenetation to these helpful notes.
After:
pfgithub if you want, you can also use .imports field in addModule function now that you have shared module and all your imports are unconditional. A little bit more "declarative":
const texteditor_mod = b.addModule("texteditor", .{
.root_source_file = b.path("src/root.zig"),
.target = target,
.optimize = optimize,
.imports = &.{
.{ .name = "blocks", .module = blocks_dep.module("blocks") },
.{ .name = "grapheme_cursor", .module = seg_dep.module("grapheme_cursor") },
.{ .name = "anywhere", .module = anywhere_dep.module("anywhere") },
.{ .name = "tree_sitter", .module = tree_sitter_dep.module("tree_sitter") },
},
});
texteditor_mod.linkLibrary(tree_sitter_zig_obj);
texteditor_mod.linkLibrary(tree_sitter_markdown_obj);
const texteditor_test = b.addTest2(.{
.root_module = texteditor_mod,
});
Now that I think about it, if Module.Import was a tuple maybe it would've been shorter and more radable...
I hadn't realised that you combined addStaticLibrary2 and addSharedLibrary2 into an addLibrary. What is your justification for that?
I hadn't realised that you combined
addStaticLibrary2andaddSharedLibrary2into anaddLibrary. What is your justification for that?
To make it easier to change linkage of library by user, consumer or author:
const linkage = b.option(std.builtin.LinkMode, "link-mode", "Link mode for a foo_bar library") orelse .dynamic; // or other default
const lib = b.addLibrary(.{
.name = "foo_bar",
.root_module = mod,
.linkage = linkage,
});
For user it's -Dlink-mode=[static|dynamic], and for consumer it's
const dep_foo_bar = b.dependency("foo_bar", .{ .target = target, .optimize = optimize, .linkage = .static }); // or dynamic
mod.linkLibrary(dep_foor_bar.artifact("foo_bar"));
That might be reasonable, but it'd probably be easier to put that enhancement in a separate PR.
That might be reasonable, but it'd probably be easier to put that enhancement in a separate PR.
Wouldn't this mean that first users will have addShared/StaticLibrary and addShared/StaticLibrary2 after this PR, then after separate PR additional addLibrary function? I think 5 functions of which 2 are deprecated and 2 are double-deprecated (addShared/StaticLibrary -> addShared/StaticLibrary2 -> addLibrary) is too much when migrating.
Or do you suggest that separate PR should just replace new add*Library2 functiosn with addLibrary directly without deprecation, so that there are just 2 deprecated functions (like in this PR currently? addShared/StaticLibrary -> addLibrary)
That might be reasonable, but it'd probably be easier to put that enhancement in a separate PR.
New commit pushed, they (functions for adding library) are now separated again.
I spoke about this PR with Andrew at Handmade Seattle a few days ago, and we agreed that providing the compilation's root module makes for a better API. However, before merging, I'm going to implement an improvement he proposed: rather than having e.g. addExecutable and addExecutable2, only have addExecutable, and (in the non-breaking transitional period) have ExecutableOptions contain both the existing options and root_module, with addExecutable asserting that only one is populated. That way, when we eventually remove the old deprecated usage, no further changes will be required for those who already migrated to the new usage.
@BratishkaErik, thanks for your work on keeping this alive. I'll implement and push the changes outlined above myself; no need for you to do anything!
@BratishkaErik, thanks for your work on keeping this alive.
No problem :)
I'll implement and push the changes outlined above myself; no need for you to do anything!
BTW feel free to remove comments in init template if core team thinks it became too lengthy. Initially I wanted to rework template after this PR so that it shortly explains for newcomers various terminology (from https://github.com/ziglang/zig/issues/14307), how to add dependency and import module from it (many people struggle with this, since they need to add 3 different names for dependency, module-from-dependency and module-in-import and it's not very intuitive what you need to write there), how to link some libraries etc. using commented templates, while people who don't want this could choose init [template] --no-comments or init minimal-module. Like opt-out tutorial.
But since https://github.com/ziglang/zig/issues/20363 and https://github.com/ziglang/zig/pull/21672 are both rejected, these new comments add even more inconvenience to people who don't want to read it, since they need to remove even more lines.
(in the non-breaking transitional period) have
ExecutableOptionscontain both the existing options androot_module, withaddExecutableasserting that only one is populated.
Would all existing fields in artifact options need to be converted to optional types, along with new root_module field?
@BratishkaErik, thanks for doing the finishing touches here.
@andrewrk, are you happy for me to merge this? I've thoroughly reviewed the work locally (refactoring and reworking some parts) and applied the changes we discussed, so I'm happy with it, but figured I should check with you.
I'm probably going to say "yes" but could you make it easier on me by typing up the corresponding release notes for this PR? That will make it very easy to evaluate, plus we need those if it is merged. (also make sure the "breaking" label is correctly applied, or not)
Sure thing -- here are the release notes. They're pretty similar to the ones I wrote up above, just modified for the API we ended up implementing.
Release Notes
Zig 0.14.0 modifies the build system APIs for creating Compile steps, allowing them to be created from existing std.Build.Module objects. This allows for module graphs to be defined in a more clear manner, and for components of these graphs to be reused more easily; for instance, a module which exists as a dependency of another can easily have a corresponding test step created. The new APIs can be used by modifying your calls to addExecutable, addTest, etc. Instead of passing options like root_source_file, target, and optimize directly to these functions, you should pass in the root_module field a *std.Build.Module created using these parameters. Zig 0.14.0 still permits the old, deprecated usages for these functions; the next release will remove them.
Users of the legacy APIs can upgrade with minimal effort by just moving the module-specific parts of their addExecutable (etc) call into a createModule call. For instance, here is the updated version of a simple build script:
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
const exe = b.addExecutable(.{
.name = "hello",
.root_source_file = b.path("src/main.zig"),
.target = target,
.optimize = optimize,
});
b.installArtifact(exe);
}
const std = @import("std");
-->
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
const exe = b.addExecutable(.{
.name = "hello",
.root_module = b.createModule(.{ // this line was added
.root_source_file = b.path("src/main.zig"),
.target = target,
.optimize = optimize,
}), // this line was added
});
b.installArtifact(exe);
}
const std = @import("std");
And, to demostrate the benefits of the new API, here is an example build script which elegantly constructs a complex build graph of multiple modules:
pub fn build(b: *std.Build) void {
const target = b.standardTargetOptions(.{});
const optimize = b.standardOptimizeOption(.{});
// First, we create our 3 modules.
const foo = b.createModule(.{
.root_source_file = b.path("src/foo.zig"),
.target = target,
.optimize = optimize,
});
const bar = b.createModule(.{
.root_source_file = b.path("src/bar.zig"),
.target = target,
.optimize = optimize,
});
const qux = b.createModule(.{
.root_source_file = b.path("src/qux.zig"),
.target = target,
.optimize = optimize,
});
// Next, we set up all of their dependencies.
foo.addImport("bar", bar);
foo.addImport("qux", qux);
bar.addImport("qux", qux);
qux.addImport("bar", bar); // mutual recursion!
// Finally, we will create all of our `Compile` steps.
// `foo` will be the root of an executable, but all 3 modules also have unit tests we want to run.
const foo_exe = b.addExecutable(.{
.name = "foo",
.root_module = foo,
});
b.installArtifact(foo_exe);
const foo_test = b.addTest(.{
.name = "foo",
.root_module = foo,
});
const bar_test = b.addTest(.{
.name = "bar",
.root_module = bar,
});
const qux_test = b.addTest(.{
.name = "qux",
.root_module = qux,
});
const test_step = b.step("test", "Run all unit tests");
test_step.dependOn(&b.addRunArtifact(foo_test).step);
test_step.dependOn(&b.addRunArtifact(bar_test).step);
test_step.dependOn(&b.addRunArtifact(qux_test).step);
}
const std = @import("std");
Oops, sorry for the redundant request. I hate that "show hidden items" UI on GitHub
I've applied "breaking" because:
- While this change is currently backwards-compatible, we will be removing this compatibility in future
- There are some minor unavoidable API breakages in here, such as
std.Build.Step.Compile.root_modulenow being a pointer
Thanks! 🥳
Release notes neglect to mention defineCMacro being deleted
defineCMacro had been deprecated (and hence slated for removal) in a prior release cycle. My understanding is that we only mention these things in the release where they're initially deprecated.
It should definitely be mentioned when it is deleted so that people can search for the missing function in the release notes and find out what to do. That's the main purpose of the release notes.