From 32795cf5f8abbd6152c115a392f6233fb41ce558 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 3 Feb 2023 17:22:31 -0700 Subject: [PATCH] std.Build: support exposing and depending on zig modules New API introduced: std.Build.addModule This function exposes a zig module with the given name, which can be used by packages that depend on this one via std.Build.Dependency.module. std.Build.Pkg and related functionality is deleted. Every use case has a straightforward upgrade path using the new Module struct. std.Build.OptionsStep.getPackage is replaced by std.Build.OptionsStep.createModule. std.Build.CompileStep.addPackagePath is replaced by std.Build.CompileStep.addAnonymousModule. This partially addresses #14307 by renaming some of the instances of "package" to "module". Closes #14278 --- lib/std/Build.zig | 130 +++++++++++++-------------- lib/std/Build/CompileStep.zig | 111 +++++++++-------------- lib/std/Build/OptionsStep.zig | 7 +- test/standalone/pkg_import/build.zig | 2 +- 4 files changed, 106 insertions(+), 144 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 684600744318..15c16479570a 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -109,6 +109,8 @@ host: NativeTargetInfo, dep_prefix: []const u8 = "", +modules: std.StringArrayHashMap(*Module), + pub const ExecError = error{ ReadFailure, ExitCodeFailure, @@ -232,6 +234,7 @@ pub fn create( .install_path = undefined, .args = null, .host = host, + .modules = std.StringArrayHashMap(*Module).init(allocator), }; try self.top_level_steps.append(&self.install_tls); try self.top_level_steps.append(&self.uninstall_tls); @@ -305,6 +308,7 @@ fn createChildOnly(parent: *Build, dep_name: []const u8, build_root: []const u8) .glibc_runtimes_dir = parent.glibc_runtimes_dir, .host = parent.host, .dep_prefix = parent.fmt("{s}{s}.", .{ parent.dep_prefix, dep_name }), + .modules = std.StringArrayHashMap(*Module).init(allocator), }; try child.top_level_steps.append(&child.install_tls); try child.top_level_steps.append(&child.uninstall_tls); @@ -539,6 +543,49 @@ pub fn addAssembly(b: *Build, options: AssemblyOptions) *CompileStep { return obj_step; } +pub const AddModuleOptions = struct { + name: []const u8, + source_file: FileSource, + dependencies: []const ModuleDependency = &.{}, +}; + +pub fn addModule(b: *Build, options: AddModuleOptions) void { + b.modules.put(b.dupe(options.name), b.createModule(.{ + .source_file = options.source_file, + .dependencies = options.dependencies, + })) catch @panic("OOM"); +} + +pub const ModuleDependency = struct { + name: []const u8, + module: *Module, +}; + +pub const CreateModuleOptions = struct { + source_file: FileSource, + dependencies: []const ModuleDependency = &.{}, +}; + +/// Prefer to use `addModule` which will make the module available to other +/// packages which depend on this package. +pub fn createModule(b: *Build, options: CreateModuleOptions) *Module { + const module = b.allocator.create(Module) catch @panic("OOM"); + module.* = .{ + .builder = b, + .source_file = options.source_file, + .dependencies = moduleDependenciesToArrayHashMap(b.allocator, options.dependencies), + }; + return module; +} + +fn moduleDependenciesToArrayHashMap(arena: Allocator, deps: []const ModuleDependency) std.StringArrayHashMap(*Module) { + var result = std.StringArrayHashMap(*Module).init(arena); + for (deps) |dep| { + result.put(dep.name, dep.module) catch @panic("OOM"); + } + return result; +} + /// Initializes a RunStep with argv, which must at least have the path to the /// executable. More command line arguments can be added with `addArg`, /// `addArgs`, and `addArtifactArg`. @@ -588,24 +635,6 @@ pub fn dupePath(self: *Build, bytes: []const u8) []u8 { return the_copy; } -/// Duplicates a package recursively. -pub fn dupePkg(self: *Build, package: Pkg) Pkg { - var the_copy = Pkg{ - .name = self.dupe(package.name), - .source = package.source.dupe(self), - }; - - if (package.dependencies) |dependencies| { - const new_dependencies = self.allocator.alloc(Pkg, dependencies.len) catch @panic("OOM"); - the_copy.dependencies = new_dependencies; - - for (dependencies) |dep_package, i| { - new_dependencies[i] = self.dupePkg(dep_package); - } - } - return the_copy; -} - pub fn addWriteFile(self: *Build, file_path: []const u8, data: []const u8) *WriteFileStep { const write_file_step = self.addWriteFiles(); write_file_step.add(file_path, data); @@ -1479,6 +1508,12 @@ pub const Dependency = struct { panic("unable to find artifact '{s}'", .{name}); }; } + + pub fn module(d: *Dependency, name: []const u8) *Module { + return d.builder.modules.get(name) orelse { + panic("unable to find module '{s}'", .{name}); + }; + } }; pub fn dependency(b: *Build, name: []const u8, args: anytype) *Dependency { @@ -1548,10 +1583,13 @@ test "builder.findProgram compiles" { _ = builder.findProgram(&[_][]const u8{}, &[_][]const u8{}) catch null; } -pub const Pkg = struct { - name: []const u8, - source: FileSource, - dependencies: ?[]const Pkg = null, +pub const Module = struct { + builder: *Build, + /// This could either be a generated file, in which case the module + /// contains exactly one file, or it could be a path to the root source + /// file of directory of files which constitute the module. + source_file: FileSource, + dependencies: std.StringArrayHashMap(*Module), }; /// A file that is generated by a build step. @@ -1713,54 +1751,6 @@ pub fn serializeCpu(allocator: Allocator, cpu: std.Target.Cpu) ![]const u8 { } } -test "dupePkg()" { - if (builtin.os.tag == .wasi) return error.SkipZigTest; - - var arena = std.heap.ArenaAllocator.init(std.testing.allocator); - defer arena.deinit(); - - const host = try NativeTargetInfo.detect(.{}); - - var builder = try Build.create( - arena.allocator(), - "test", - "test", - "test", - "test", - host, - ); - defer builder.destroy(); - - var pkg_dep = Pkg{ - .name = "pkg_dep", - .source = .{ .path = "/not/a/pkg_dep.zig" }, - }; - var pkg_top = Pkg{ - .name = "pkg_top", - .source = .{ .path = "/not/a/pkg_top.zig" }, - .dependencies = &[_]Pkg{pkg_dep}, - }; - const duped = builder.dupePkg(pkg_top); - - const original_deps = pkg_top.dependencies.?; - const dupe_deps = duped.dependencies.?; - - // probably the same top level package details - try std.testing.expectEqualStrings(pkg_top.name, duped.name); - - // probably the same dependencies - try std.testing.expectEqual(original_deps.len, dupe_deps.len); - try std.testing.expectEqual(original_deps[0].name, pkg_dep.name); - - // could segfault otherwise if pointers in duplicated package's fields are - // the same as those in stack allocated package's fields - try std.testing.expect(dupe_deps.ptr != original_deps.ptr); - try std.testing.expect(duped.name.ptr != pkg_top.name.ptr); - try std.testing.expect(duped.source.path.ptr != pkg_top.source.path.ptr); - try std.testing.expect(dupe_deps[0].name.ptr != pkg_dep.name.ptr); - try std.testing.expect(dupe_deps[0].source.path.ptr != pkg_dep.source.path.ptr); -} - test { _ = CheckFileStep; _ = CheckObjectStep; diff --git a/lib/std/Build/CompileStep.zig b/lib/std/Build/CompileStep.zig index 12c73bdba6b6..879793f78136 100644 --- a/lib/std/Build/CompileStep.zig +++ b/lib/std/Build/CompileStep.zig @@ -16,7 +16,7 @@ const FileSource = std.Build.FileSource; const PkgConfigPkg = std.Build.PkgConfigPkg; const PkgConfigError = std.Build.PkgConfigError; const ExecError = std.Build.ExecError; -const Pkg = std.Build.Pkg; +const Module = std.Build.Module; const VcpkgRoot = std.Build.VcpkgRoot; const InstallDir = std.Build.InstallDir; const InstallArtifactStep = std.Build.InstallArtifactStep; @@ -99,7 +99,7 @@ root_src: ?FileSource, out_h_filename: []const u8, out_lib_filename: []const u8, out_pdb_filename: []const u8, -packages: ArrayList(Pkg), +modules: std.StringArrayHashMap(*Module), object_src: []const u8, @@ -334,7 +334,7 @@ pub fn create(builder: *std.Build, options: Options) *CompileStep { .out_pdb_filename = builder.fmt("{s}.pdb", .{name}), .major_only_filename = null, .name_only_filename = null, - .packages = ArrayList(Pkg).init(builder.allocator), + .modules = std.StringArrayHashMap(*Module).init(builder.allocator), .include_dirs = ArrayList(IncludeDir).init(builder.allocator), .link_objects = ArrayList(LinkObject).init(builder.allocator), .c_macros = ArrayList([]const u8).init(builder.allocator), @@ -946,29 +946,29 @@ pub fn addFrameworkPath(self: *CompileStep, dir_path: []const u8) void { self.framework_dirs.append(self.builder.dupe(dir_path)) catch @panic("OOM"); } -pub fn addPackage(self: *CompileStep, package: Pkg) void { - self.packages.append(self.builder.dupePkg(package)) catch @panic("OOM"); - self.addRecursiveBuildDeps(package); +/// Adds a module to be used with `@import` and exposing it in the current +/// package's module table using `name`. +pub fn addModule(cs: *CompileStep, name: []const u8, module: *Module) void { + cs.modules.put(cs.builder.dupe(name), module) catch @panic("OOM"); + cs.addRecursiveBuildDeps(module); } -pub fn addOptions(self: *CompileStep, package_name: []const u8, options: *OptionsStep) void { - self.addPackage(options.getPackage(package_name)); +/// Adds a module to be used with `@import` without exposing it in the current +/// package's module table. +pub fn addAnonymousModule(cs: *CompileStep, name: []const u8, options: std.Build.CreateModuleOptions) void { + const module = cs.builder.createModule(options); + return addModule(cs, name, module); } -fn addRecursiveBuildDeps(self: *CompileStep, package: Pkg) void { - package.source.addStepDependencies(&self.step); - if (package.dependencies) |deps| { - for (deps) |dep| { - self.addRecursiveBuildDeps(dep); - } - } +pub fn addOptions(cs: *CompileStep, module_name: []const u8, options: *OptionsStep) void { + addModule(cs, module_name, options.createModule()); } -pub fn addPackagePath(self: *CompileStep, name: []const u8, pkg_index_path: []const u8) void { - self.addPackage(Pkg{ - .name = self.builder.dupe(name), - .source = .{ .path = self.builder.dupe(pkg_index_path) }, - }); +fn addRecursiveBuildDeps(cs: *CompileStep, module: *Module) void { + module.source_file.addStepDependencies(&cs.step); + for (module.dependencies.values()) |dep| { + cs.addRecursiveBuildDeps(dep); + } } /// If Vcpkg was found on the system, it will be added to include and lib @@ -1023,16 +1023,21 @@ fn linkLibraryOrObject(self: *CompileStep, other: *CompileStep) void { self.include_dirs.append(.{ .other_step = other }) catch @panic("OOM"); } -fn makePackageCmd(self: *CompileStep, pkg: Pkg, zig_args: *ArrayList([]const u8)) error{OutOfMemory}!void { - const builder = self.builder; - +fn appendModuleArgs( + cs: *CompileStep, + zig_args: *ArrayList([]const u8), + name: []const u8, + module: *Module, +) error{OutOfMemory}!void { try zig_args.append("--pkg-begin"); - try zig_args.append(pkg.name); - try zig_args.append(builder.pathFromRoot(pkg.source.getPath(self.builder))); - - if (pkg.dependencies) |dependencies| { - for (dependencies) |sub_pkg| { - try self.makePackageCmd(sub_pkg, zig_args); + try zig_args.append(name); + try zig_args.append(module.builder.pathFromRoot(module.source_file.getPath(module.builder))); + + { + const keys = module.dependencies.keys(); + for (module.dependencies.values()) |sub_module, i| { + const sub_name = keys[i]; + try cs.appendModuleArgs(zig_args, sub_name, sub_module); } } @@ -1563,8 +1568,12 @@ fn make(step: *Step) !void { try zig_args.append("--test-no-exec"); } - for (self.packages.items) |pkg| { - try self.makePackageCmd(pkg, &zig_args); + { + const keys = self.modules.keys(); + for (self.modules.values()) |module, i| { + const name = keys[i]; + try self.appendModuleArgs(&zig_args, name, module); + } } for (self.include_dirs.items) |include_dir| { @@ -1942,46 +1951,6 @@ fn getPkgConfigList(self: *std.Build) ![]const PkgConfigPkg { } } -test "addPackage" { - if (builtin.os.tag == .wasi) return error.SkipZigTest; - - var arena = std.heap.ArenaAllocator.init(std.testing.allocator); - defer arena.deinit(); - - const host = try NativeTargetInfo.detect(.{}); - - var builder = try std.Build.create( - arena.allocator(), - "test", - "test", - "test", - "test", - host, - ); - defer builder.destroy(); - - const pkg_dep = Pkg{ - .name = "pkg_dep", - .source = .{ .path = "/not/a/pkg_dep.zig" }, - }; - const pkg_top = Pkg{ - .name = "pkg_dep", - .source = .{ .path = "/not/a/pkg_top.zig" }, - .dependencies = &[_]Pkg{pkg_dep}, - }; - - var exe = builder.addExecutable(.{ - .name = "not_an_executable", - .root_source_file = .{ .path = "/not/an/executable.zig" }, - }); - exe.addPackage(pkg_top); - - try std.testing.expectEqual(@as(usize, 1), exe.packages.items.len); - - const dupe = exe.packages.items[0]; - try std.testing.expectEqualStrings(pkg_top.name, dupe.name); -} - fn addFlag(args: *ArrayList([]const u8), comptime name: []const u8, opt: ?bool) !void { const cond = opt orelse return; try args.ensureUnusedCapacity(1); diff --git a/lib/std/Build/OptionsStep.zig b/lib/std/Build/OptionsStep.zig index c1d2c8454a30..8a50456539c0 100644 --- a/lib/std/Build/OptionsStep.zig +++ b/lib/std/Build/OptionsStep.zig @@ -204,8 +204,11 @@ pub fn addOptionArtifact(self: *OptionsStep, name: []const u8, artifact: *Compil self.step.dependOn(&artifact.step); } -pub fn getPackage(self: *OptionsStep, package_name: []const u8) std.Build.Pkg { - return .{ .name = package_name, .source = self.getSource() }; +pub fn createModule(self: *OptionsStep) *std.Build.Module { + return self.builder.createModule(.{ + .source_file = self.getSource(), + .dependencies = &.{}, + }); } pub fn getSource(self: *OptionsStep) FileSource { diff --git a/test/standalone/pkg_import/build.zig b/test/standalone/pkg_import/build.zig index 5fbc8a67ae78..5ea6c90af776 100644 --- a/test/standalone/pkg_import/build.zig +++ b/test/standalone/pkg_import/build.zig @@ -8,7 +8,7 @@ pub fn build(b: *std.Build) void { .root_source_file = .{ .path = "test.zig" }, .optimize = optimize, }); - exe.addPackagePath("my_pkg", "pkg.zig"); + exe.addAnonymousModule("my_pkg", .{ .source_file = .{ .path = "pkg.zig" } }); const run = exe.run();