From 5d7cf3f34d13f70de35c2f76db2e44073e6de64e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 14 Aug 2026 01:24:55 -0700 Subject: [PATCH] zig build: revert package related changes to Configuration partial revert of 8e48a5c524f970c4630725e2efe69243d8d79fde partial revert of 8380531aecf20405bad6e008712a39795f224cb3 fixes regression in the build system causing crash when trying to build ffmpeg project from the ecosystem. upon closer inspection, I don't think this is the right direction for adding this data to Configuration. Let's try again and be a little more careful about it. --- lib/compiler/Maker.zig | 11 +- lib/compiler/Maker/ScannedConfig.zig | 45 ------- lib/std/Build/Configuration.zig | 129 +++---------------- lib/std/Build/Serialize.zig | 181 +++------------------------ 4 files changed, 39 insertions(+), 327 deletions(-) diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig index 2889e9aaccecb9095f705d4545db7068b1607edc..e3aefb00343062a9e576e227e1e0a98e04c43f46 100644 --- a/lib/compiler/Maker.zig +++ b/lib/compiler/Maker.zig @@ -2293,7 +2293,7 @@ fn prepare( } } else { log.err("{s}{s}: this step declares an upper bound of {d} bytes of memory, exceeding the available {d} bytes of memory", .{ - conf_step.owner.package(c).depPrefixSlice(c), + conf_step.owner.depPrefixSlice(c), conf_step.name.slice(c), max_rss, maker.available_rss, @@ -3388,13 +3388,12 @@ pub fn generatedPath(maker: *const Maker, index: Configuration.GeneratedFileInde pub fn packagePath( maker: *const Maker, arena: Allocator, - inst_index: Configuration.Package.Instance.Index, + package_index: Configuration.Package.Index, sub_path: []const u8, ) Allocator.Error!Path { const c = &maker.scanned_config.configuration; const graph = maker.graph; - - if (inst_index == .root) return .{ + const package = package_index.get(c) orelse return .{ .root_dir = graph.build_root_directory, .sub_path = sub_path, }; @@ -3405,7 +3404,7 @@ pub fn packagePath( // construct a cwd relative path here. return .{ .root_dir = .cwd(), - .sub_path = try Dir.path.join(arena, &.{ inst_index.package(c).ptr(c).root_path.slice(c), sub_path }), + .sub_path = try Dir.path.join(arena, &.{ package.root_path.slice(c), sub_path }), }; } @@ -4035,7 +4034,7 @@ fn confPathDepToCachePath( .root_dir = graph.build_root_directory, .sub_path = switch (path_dep.pkg.unwrap().?) { .root => sub_path, - else => |index| try Dir.path.join(arena, &.{ index.package(c).ptr(c).root_path.slice(c), sub_path }), + else => |index| try Dir.path.join(arena, &.{ index.get(c).?.root_path.slice(c), sub_path }), }, }, .zig_lib => .{ diff --git a/lib/compiler/Maker/ScannedConfig.zig b/lib/compiler/Maker/ScannedConfig.zig index cc9a3d690bf83f01a9af2e26460108a3f8ea1a92..ed5661851351afb4c9a57b8723e2f4d13cd895ba 100644 --- a/lib/compiler/Maker/ScannedConfig.zig +++ b/lib/compiler/Maker/ScannedConfig.zig @@ -83,51 +83,6 @@ pub fn print(sc: *const ScannedConfig, w: *Writer) Writer.Error!void { try tf.end(); } - { - var tf = try s.beginTupleField("packages", .{}); - for (c.packages) |package| { - var sf = try tf.beginStructField(.{}); - try sf.field("dep_prefix", package.dep_prefix.slice(c), .{}); - try sf.field("hash", package.hash.slice(c), .{}); - try sf.field("root_path", package.root_path.slice(c), .{}); - - var dtf = try sf.beginTupleField("deps", .{}); - for (package.deps.slice(c)) |dep| { - var dsf = try dtf.beginStructField(.{}); - try sc.printStruct(&dsf, Configuration.Package.Dep, dep); - try dsf.end(); - } - try dtf.end(); - - try sf.end(); - } - try tf.end(); - } - - { - var tf = try s.beginTupleField("package_instances", .{}); - for (c.package_instances) |inst| { - var sf = try tf.beginStructField(.{}); - - try sf.fieldPrefix("package"); - if (std.enums.tagName(Configuration.Package.Index, inst.package)) |name| { - try sf.container.serializer.ident(name); - } else { - try sf.container.serializer.int(@backingInt(inst.package)); - } - - var msf = try sf.beginStructField("modules", .{}); - for (inst.modules.keys.slice(c), inst.modules.values.slice(c)) |key, value| { - var msf2 = try msf.beginStructField(key.slice(c), .{}); - try sc.printStruct(&msf2, Configuration.Module, value.get(c)); - try msf2.end(); - } - try msf.end(); - - try sf.end(); - } - } - try s.end(); } diff --git a/lib/std/Build/Configuration.zig b/lib/std/Build/Configuration.zig index 4c19f4daafa8a90c138548d7e9e7b723c809aebc..3221086488b403294a316e6ffde8f5dc0522c947 100644 --- a/lib/std/Build/Configuration.zig +++ b/lib/std/Build/Configuration.zig @@ -15,14 +15,6 @@ unlazy_deps: []String, system_integrations: []SystemIntegration, available_options: []AvailableOption, search_prefixes: []String, -/// Index 0 always exists and is the root package. -packages: []Package, -/// Index 0 always exists and is the root package instance. -/// -/// Unlike `packages`, each item corresponds to a `std.Build`, which is a -/// package that was instantiated by running its build script with specific -/// input options. -package_instances: []Package.Instance, extra: []u32, default_step: Step.Index, generated_files_len: u32, @@ -38,8 +30,6 @@ pub const Header = extern struct { system_integrations_len: u32, available_options_len: u32, search_prefixes_len: u32, - packages_len: u32, - package_instances_len: u32, extra_len: u32, default_step: Step.Index, @@ -68,8 +58,6 @@ pub const Wip = struct { steps: std.ArrayList(Step) = .empty, path_deps: std.ArrayList(PathDep) = .empty, search_prefixes: std.ArrayList(String) = .empty, - packages: std.ArrayList(Package) = .empty, - package_instances: std.ArrayList(Package.Instance) = .empty, extra: std.ArrayList(u32) = .empty, next_generated_file_index: u32 = 0, cache_poison: bool = false, @@ -151,8 +139,6 @@ pub const Wip = struct { wip.steps.deinit(gpa); wip.path_deps.deinit(gpa); wip.search_prefixes.deinit(gpa); - wip.packages.deinit(gpa); - wip.package_instances.deinit(gpa); wip.extra.deinit(gpa); wip.* = undefined; } @@ -172,8 +158,6 @@ pub const Wip = struct { .system_integrations_len = @intCast(wip.system_integrations.items.len), .available_options_len = @intCast(wip.available_options.items.len), .search_prefixes_len = @intCast(wip.search_prefixes.items.len), - .packages_len = @intCast(wip.packages.items.len), - .package_instances_len = @intCast(wip.package_instances.items.len), .extra_len = @intCast(wip.extra.items.len), .default_step = static.default_step, @@ -191,8 +175,6 @@ pub const Wip = struct { @ptrCast(wip.system_integrations.items), @ptrCast(wip.available_options.items), @ptrCast(wip.search_prefixes.items), - @ptrCast(wip.packages.items), - @ptrCast(wip.package_instances.items), @ptrCast(wip.extra.items), }; try w.writeVecAll(&buffers); @@ -494,7 +476,7 @@ pub const AvailableOption = extern struct { pub const Step = extern struct { name: String, - owner: Package.Instance.Index, + owner: Package.Index, deps: Deps.Index, max_rss: MaxRss, extended: Storage.Extended(Flags, union(Tag) { @@ -1543,7 +1525,7 @@ pub const LazyPath = union(@This().Tag) { pub const SourcePath = struct { flags: @This().Flags = .{}, - owner: Package.Instance.Index, + owner: Package.Index, sub_path: String, pub const Flags = packed struct(u32) { @@ -1614,28 +1596,30 @@ pub const OptionalGeneratedFileIndex = enum(u32) { } }; -pub const Package = extern struct { +pub const Package = struct { dep_prefix: String, hash: String, root_path: String, - deps: Dep.List.Index, pub const Index = enum(u32) { - root, + root = max_u32, _, - pub fn ptr(i: @This(), c: *const Configuration) *const Package { - return &c.packages[@backingInt(i)]; + /// Returns `null` for root package. + pub fn get(i: @This(), c: *const Configuration) ?Package { + if (i == .root) return null; + return extraData(c, Package, @backingInt(i)); } pub fn depPrefixSlice(i: @This(), c: *const Configuration) [:0]const u8 { - return ptr(i, c).dep_prefix.slice(c); + const package = get(i, c) orelse return ""; + return package.dep_prefix.slice(c); } }; pub const OptionalIndex = enum(u32) { - root, - none = max_u32, + none = max_u32 - 1, + root = max_u32, _, pub fn init(i: Index) OptionalIndex { @@ -1652,78 +1636,13 @@ pub const Package = extern struct { }; } }; - - pub const Dep = extern struct { - name: String, - /// Must not be `.root`. - package: Package.Index, - - pub const List = struct { - deps: Storage.LengthPrefixedList(Dep), - - pub const Index = enum(u32) { - _, - - pub fn get(this: @This(), c: *const Configuration) List { - return extraData(c, List, @backingInt(this)); - } - - pub fn slice(this: @This(), c: *const Configuration) []const Dep { - return get(this, c).deps.slice; - } - }; - }; - }; - - pub const Instance = extern struct { - package: Package.Index, - modules: PublicModules, - - pub const PublicModules = extern struct { - keys: StringList, - values: Module.List.Index, - }; - - pub const Index = enum(u32) { - root, - _, - - pub fn ptr(this: @This(), c: *const Configuration) *const Package.Instance { - return &c.package_instances[@backingInt(this)]; - } - - pub fn package(this: @This(), c: *const Configuration) Package.Index { - return this.ptr(c).package; - } - }; - - pub const OptionalIndex = enum(u32) { - root, - none = max_u32, - _, - - pub fn init(i: Instance.Index) Instance.OptionalIndex { - const result: Instance.OptionalIndex = @fromBackingInt(@intCast(@backingInt(i))); - assert(result != .none); - return result; - } - - pub fn unwrap(this: @This()) ?Instance.Index { - return switch (this) { - .none => null, - .root => .root, - _ => @fromBackingInt(@intCast(@backingInt(this))), - }; - } - }; - }; }; pub const Module = struct { flags: Flags, flags2: Flags2, import_table: ImportTable.Index, - owner: Package.Instance.Index, + owner: Package.Index, root_source_file: LazyPath.OptionalIndex, resolved_target: ResolvedTarget.OptionalIndex, c_macros: Storage.FlagLengthPrefixedList(.flags, .c_macros, String), @@ -1866,22 +1785,6 @@ pub const Module = struct { }; pub const Index = IndexType(@This()); - - pub const List = struct { - modules: Storage.LengthPrefixedList(Module.Index), - - pub const Index = enum(u32) { - _, - - pub fn get(this: @This(), c: *const Configuration) List { - return extraData(c, List, @backingInt(this)); - } - - pub fn slice(this: @This(), c: *const Configuration) []const Module.Index { - return this.get(c).modules.slice; - } - }; - }; }; pub const ImportTable = struct { @@ -1966,7 +1869,7 @@ pub const OptionalStringList = enum(u32) { pub const PathDep = extern struct { flags: Flags, sub: String, - pkg: Package.Instance.OptionalIndex, + pkg: Package.OptionalIndex, pub const Flags = packed struct(u32) { mode: Mode, @@ -3231,8 +3134,6 @@ pub fn load(arena: Allocator, reader: *Io.Reader) LoadError!Configuration { .system_integrations = try arena.alloc(SystemIntegration, header.system_integrations_len), .available_options = try arena.alloc(AvailableOption, header.available_options_len), .search_prefixes = try arena.alloc(String, header.search_prefixes_len), - .packages = try arena.alloc(Package, header.packages_len), - .package_instances = try arena.alloc(Package.Instance, header.package_instances_len), .extra = try arena.alloc(u32, header.extra_len), .default_step = header.default_step, .generated_files_len = header.generated_files_len, @@ -3246,8 +3147,6 @@ pub fn load(arena: Allocator, reader: *Io.Reader) LoadError!Configuration { @ptrCast(result.system_integrations), @ptrCast(result.available_options), @ptrCast(result.search_prefixes), - @ptrCast(result.packages), - @ptrCast(result.package_instances), @ptrCast(result.extra), }; try reader.readVecAll(&vecs); diff --git a/lib/std/Build/Serialize.zig b/lib/std/Build/Serialize.zig index b835fbd419f76b2bbffc3bc49f843bd5a37f73a2..995a69da87f9a6becfdb35f61683178b5e046c8b 100644 --- a/lib/std/Build/Serialize.zig +++ b/lib/std/Build/Serialize.zig @@ -10,10 +10,7 @@ const log = std.log; arena: Allocator, wc: *Configuration.Wip, module_map: std.array_hash_map.Auto(*std.Build.Module, Configuration.Module.Index) = .empty, -/// Keyed by package hash. Index + 1 corresponds to `Configuration.packages` index. -package_map: std.array_hash_map.String(void) = .empty, -/// Index corresponds to `Configuration.package_instances` index. -package_instance_map: std.array_hash_map.Auto(*std.Build, void) = .empty, +package_map: std.array_hash_map.Auto(*std.Build, Configuration.Package.Index) = .empty, /// Index corresponds to `Configuration.steps` index. step_map: std.array_hash_map.Auto(*Step, void) = .empty, @@ -24,31 +21,6 @@ pub fn write(b: *std.Build, wc: *Configuration.Wip, writer: *std.Io.Writer) !voi var s: Serialize = .{ .wc = wc, .arena = arena }; - // Seed the package_map, which is later used when serializing package - // instances. - try s.traversePackages(b); - - // Next, seed the package_instance_map, which is later used in calls to - // packageInstanceFromBuilder. - - _ = try wc.package_instances.addManyAsSlice(gpa, 1 + b.graph.dependency_cache.count()); - try s.package_instance_map.ensureTotalCapacity(arena, 1 + b.graph.dependency_cache.count()); - - // As serializing package instances also requires serializing the public - // modules of each one, we must first allocate an index for each package - // instance. Otherwise, addModule may access a package instance that hasn't - // been created yet with packageInstanceFromBuilder. - - s.package_instance_map.putAssumeCapacityNoClobber(b, {}); - for (b.graph.dependency_cache.values()) |dep| { - s.package_instance_map.putAssumeCapacityNoClobber(dep.builder, {}); - } - - try s.addPackageInstance(b); - for (b.graph.dependency_cache.values()) |dep| { - try s.addPackageInstance(dep.builder); - } - try wc.path_deps.ensureTotalCapacityPrecise(gpa, graph.configure_dependencies.items.len); for ( graph.configure_dependencies.items, @@ -72,10 +44,10 @@ pub fn write(b: *std.Build, wc: *Configuration.Wip, writer: *std.Io.Writer) !voi .relative => |r| try wc.addString(r.sub_path), }, .pkg = switch (src.lazy_path) { - .src_path => |sp| .init(s.packageInstanceFromBuilder(sp.owner)), + .src_path => |sp| .init(try s.builderToPackage(sp.owner)), .generated => unreachable, .cwd_relative, .relative => .none, - .dependency => |d| .init(s.packageInstanceFromBuilder(d.dependency.builder)), + .dependency => |d| .init(try s.builderToPackage(d.dependency.builder)), }, }; } @@ -112,7 +84,7 @@ pub fn write(b: *std.Build, wc: *Configuration.Wip, writer: *std.Io.Writer) !voi try wc.steps.ensureTotalCapacity(gpa, s.step_map.entries.capacity); wc.steps.appendAssumeCapacity(.{ .name = try wc.addString(step.name), - .owner = s.packageInstanceFromBuilder(step.owner), + .owner = try s.builderToPackage(step.owner), .deps = deps, .max_rss = .fromBytes(step.max_rss), .extended = @fromBackingInt(@intCast(switch (step.tag) { @@ -750,132 +722,19 @@ pub fn packageOptions(b: *std.Build, wc: *Configuration.Wip) Allocator.Error!voi } } -fn traversePackages(s: *Serialize, b: *std.Build) Allocator.Error!void { - const arena = s.arena; - const wc = s.wc; - - try wc.packages.append(wc.gpa, .{ - .dep_prefix = .empty, - .hash = .empty, - .root_path = try wc.addString(try b.root.toString(arena)), - .deps = undefined, - }); - - const deps = try arena.alloc(Configuration.Package.Dep, b.available_deps.len); - for (deps, b.available_deps) |*dest, src| dest.* = try s.makePackageDep("", src[0], src[1]); - - wc.packages.items[0].deps = try wc.addDeduped(Configuration.Package.Dep.List, .{ - .deps = .{ .slice = deps }, - }); -} - -fn makePackageDep(s: *Serialize, parent_dep_prefix: []const u8, name: []const u8, hash: []const u8) Allocator.Error!Configuration.Package.Dep { - const arena = s.arena; - const wc = s.wc; - - if (s.package_map.getIndex(hash)) |index| return .{ - .name = try wc.addString(name), - .package = @fromBackingInt(@intCast(index + 1)), - }; - - const index: Configuration.Package.Index = @fromBackingInt(@intCast(wc.packages.items.len)); - try s.package_map.put(arena, hash, {}); - - const entry = std.Build.package_map.get(hash) orelse unreachable; - - const dep_prefix = try arena.print("{s}{s}.", .{ parent_dep_prefix, name }); - - try wc.packages.append(wc.gpa, .{ - .dep_prefix = try wc.addString(dep_prefix), - .hash = try wc.addString(hash), - .root_path = try wc.addString(entry.build_root), - .deps = undefined, - }); - - const deps = try arena.alloc(Configuration.Package.Dep, entry.deps.len); - for (deps, entry.deps) |*dest, src| dest.* = try s.makePackageDep(dep_prefix, src[0], src[1]); - - wc.packages.items[@backingInt(index)].deps = try wc.addDeduped(Configuration.Package.Dep.List, .{ - .deps = .{ .slice = deps }, - }); - - return .{ - .name = try wc.addString(name), - .package = index, - }; -} - -fn packageFromHash(s: *Serialize, pkg_hash: []const u8) Configuration.Package.Index { - if (pkg_hash.len == 0) return .root; - return @fromBackingInt(@intCast(s.package_map.getIndex(pkg_hash).? + 1)); -} - -fn addPackageInstance(s: *Serialize, b: *std.Build) Allocator.Error!void { - const arena = s.arena; - const wc = s.wc; - - const index = s.package_instance_map.getIndex(b).?; - - const modules_values = try arena.alloc(Configuration.Module.Index, b.modules.count()); - for (modules_values, b.modules.values()) |*dest_value, value| { - dest_value.* = try s.addModule(value); - } - - wc.package_instances.items[index] = .{ - .package = s.packageFromHash(b.pkg_hash), - .modules = .{ - .keys = try wc.addStringList(b.modules.keys()), - .values = try wc.addDeduped(Configuration.Module.List, .{ - .modules = .{ .slice = modules_values }, - }), - }, - }; -} - -fn makeUserValue(s: *Serialize, user_value: *const std.Build.UserValue) Allocator.Error!Configuration.Package.Instance.UserValue { - const arena = s.arena; - const wc = s.wc; - - return switch (user_value.*) { - .flag => .flag, - .scalar => |str| .{ .scalar = try wc.addString(str) }, - .list => |list| .{ .list = try wc.addStringList(list.items) }, - .map => |map| add: { - const keys = try arena.alloc([]const u8, map.count()); - const values = try arena.alloc(Configuration.Package.Instance.UserValue.Standalone.Index, map.count()); - - var i: usize = 0; - var iter = map.iterator(); - while (iter.next()) |entry| : (i += 1) { - const value = try s.makeUserValue(entry.value_ptr.*); - - keys[i] = entry.key_ptr.*; - values[i] = try wc.addDeduped( - Configuration.Package.Instance.UserValue.Standalone, - .{ .flags = .{ .tag = value }, .value = .{ .u = value } }, - ); - } - - break :add .{ .map = try wc.addDeduped( - Configuration.Package.Instance.UserValue.Map, - .{ - .keys = try wc.addStringList(keys), - .values = .{ .slice = values }, - }, - ) }; - }, - .lazy_path => |lp| .{ .lazy_path = try s.addLazyPath(lp) }, - .lazy_path_list => |lp_list| add: { - const paths = try arena.alloc(Configuration.LazyPath.Index, lp_list.items.len); - for (paths, lp_list.items) |*dest, src| dest.* = try s.addLazyPath(src); - break :add .{ .lazy_path_list = .{ .slice = paths } }; - }, - }; -} - -fn packageInstanceFromBuilder(s: *Serialize, b: *std.Build) Configuration.Package.Instance.Index { +fn builderToPackage(s: *Serialize, b: *std.Build) !Configuration.Package.Index { if (b.pkg_hash.len == 0) return .root; - return @fromBackingInt(@intCast(s.package_instance_map.getIndex(b).?)); + const arena = s.arena; + const wc = s.wc; + const gop = try s.package_map.getOrPut(arena, b); + if (!gop.found_existing) { + gop.value_ptr.* = try wc.addExtra(Configuration.Package, .{ + .hash = try wc.addString(b.pkg_hash), + .dep_prefix = try wc.addString(b.dep_prefix), + .root_path = try wc.addString(try b.root.toString(arena)), + }); + } + return gop.value_ptr.*; } fn addOptionalLazyPathEnum(s: *Serialize, lp: ?std.Build.LazyPath) !Configuration.LazyPath.OptionalIndex { @@ -884,7 +743,7 @@ fn addOptionalLazyPathEnum(s: *Serialize, lp: ?std.Build.LazyPath) !Configuratio .src_path => |src_path| i: { const sub_path = try wc.addString(src_path.sub_path); break :i try wc.addExtraErased(Configuration.LazyPath.SourcePath, .{ - .owner = s.packageInstanceFromBuilder(src_path.owner), + .owner = try s.builderToPackage(src_path.owner), .sub_path = sub_path, }); }, @@ -912,7 +771,7 @@ fn addOptionalLazyPathEnum(s: *Serialize, lp: ?std.Build.LazyPath) !Configuratio .dependency => |dependency| i: { const sub_path = try wc.addString(dependency.sub_path); break :i try wc.addExtraErased(Configuration.LazyPath.SourcePath, .{ - .owner = s.packageInstanceFromBuilder(dependency.dependency.builder), + .owner = try s.builderToPackage(dependency.dependency.builder), .sub_path = sub_path, }); }, @@ -1341,7 +1200,7 @@ fn addModule(s: *Serialize, m: *std.Build.Module) !Configuration.Module.Index { const c_macros = try initStringList(s, m.c_macros.items); const export_symbol_names = try initStringList(s, m.export_symbol_names); - const module_index: Configuration.Module.Index = try wc.addDeduped(Configuration.Module, .{ + const module_index: Configuration.Module.Index = try wc.addExtra(Configuration.Module, .{ .flags = .{ .optimize = .init(m.optimize), .strip = .init(m.strip), @@ -1372,7 +1231,7 @@ fn addModule(s: *Serialize, m: *std.Build.Module) !Configuration.Module.Index { .link_libcpp = .init(m.link_libcpp), .no_builtin = .init(m.no_builtin), }, - .owner = s.packageInstanceFromBuilder(m.owner), + .owner = try s.builderToPackage(m.owner), .root_source_file = try s.addOptionalLazyPathEnum(m.root_source_file), .import_table = .invalid, .resolved_target = try addOptionalResolvedTarget(wc, m.resolved_target), -- 2.54.0