From df92898ec3f805112150ee4b40e0d02b763524bb Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 20 May 2026 20:05:55 -0700 Subject: [PATCH] Maker: fix packagePath function Currently, neither configurer nor Maker is aware of the standard zig package path, and the root path is stored as a bare string rather than relative to a known base directory. Without changing that, we must construct a cwd relative path here rather than using knowledge of the standard package path plus package hash. Also fixes a bug that would have been prevented by implementing the accepted proposal https://github.com/ziglang/zig/issues/25315 --- lib/compiler/Maker.zig | 14 ++++++-------- lib/compiler/Maker/Graph.zig | 1 - lib/compiler/configurer.zig | 22 ++++++++++++---------- lib/std/Build.zig | 1 + lib/std/Build/Cache/Path.zig | 2 +- lib/std/Build/Configuration.zig | 24 +++++++++++++++++------- src/Package/Fetch.zig | 4 ++-- src/main.zig | 32 +++++++++++++++++--------------- 8 files changed, 56 insertions(+), 44 deletions(-) diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig index 52815f7945..133c10e5f3 100644 --- a/lib/compiler/Maker.zig +++ b/lib/compiler/Maker.zig @@ -123,10 +123,6 @@ pub fn main(init: process.Init.Minimal) !void { .local_cache_root = local_cache_directory, .zig_lib_directory = zig_lib_directory, .build_root_directory = build_root_directory, - .pkg_root = .{ - .root_dir = build_root_directory, - .sub_path = "zig-pkg", - }, }; graph.cache.addPrefix(.{ .path = null, .handle = cwd }); @@ -1779,11 +1775,13 @@ pub fn packagePath( .root_dir = graph.build_root_directory, .sub_path = sub_path, }; - const hash = package.hash.slice(c); - const pkg_root = graph.pkg_root; + // Currently, neither configurer nor Maker is aware of the standard zig + // package path, and the root path is stored as a bare string rather than + // relative to a known base directory. Without changing that, we must + // construct a cwd relative path here. return .{ - .root_dir = pkg_root.root_dir, - .sub_path = try Dir.path.join(arena, &.{ pkg_root.sub_path, hash, sub_path }), + .root_dir = .cwd(), + .sub_path = try Dir.path.join(arena, &.{ package.root_path.slice(c), sub_path }), }; } diff --git a/lib/compiler/Maker/Graph.zig b/lib/compiler/Maker/Graph.zig index 4e1c05b8c5..667ce6d06d 100644 --- a/lib/compiler/Maker/Graph.zig +++ b/lib/compiler/Maker/Graph.zig @@ -18,7 +18,6 @@ global_cache_root: Directory, local_cache_root: Directory, zig_lib_directory: Directory, build_root_directory: Directory, -pkg_root: Path, debug_compiler_runtime_libs: ?std.builtin.OptimizeMode = null, incremental: ?bool = null, diff --git a/lib/compiler/configurer.zig b/lib/compiler/configurer.zig index a4c661b1f4..bfbdd14c6e 100644 --- a/lib/compiler/configurer.zig +++ b/lib/compiler/configurer.zig @@ -169,6 +169,7 @@ const Serialize = struct { gop.value_ptr.* = @enumFromInt(try wc.addExtra(@as(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.*; @@ -233,7 +234,7 @@ const Serialize = struct { fn addSystemLib(s: *Serialize, sl: *const std.Build.Module.SystemLib) !Configuration.SystemLib.Index { const wc = s.wc; - return @enumFromInt(try wc.addDeduped(@as(Configuration.SystemLib, .{ + return try wc.addDeduped(Configuration.SystemLib, .{ .flags = .{ .needed = sl.needed, .weak = sl.weak, @@ -242,7 +243,7 @@ const Serialize = struct { .search_strategy = sl.search_strategy, }, .name = try wc.addString(sl.name), - }))); + }); } fn addCSourceFile(s: *Serialize, csf: *const std.Build.Module.CSourceFile) !Configuration.CSourceFile.Index { @@ -291,10 +292,10 @@ const Serialize = struct { fn addEnvironMap(s: *Serialize, opt_map: ?*std.process.Environ.Map) !?Configuration.EnvironMap.Index { const wc = s.wc; const map = opt_map orelse return null; - return @enumFromInt(try wc.addDeduped(@as(Configuration.EnvironMap, .{ + return try wc.addDeduped(Configuration.EnvironMap, .{ .keys = try wc.addStringList(map.array_hash_map.keys()), .values = try wc.addStringList(map.array_hash_map.values()), - }))); + }); } fn initArgsList(s: *Serialize, args: []const Step.Run.Arg) ![]const Configuration.Step.Run.Arg.Index { @@ -643,9 +644,10 @@ const Serialize = struct { comptime assert(std.mem.eql(u8, @typeInfo(Configuration.Module).@"struct".fields[2].name, "import_table")); comptime assert(@typeInfo(Configuration.Module).@"struct".fields[2].type == Configuration.ImportTable.Index); assert(wc.extra.items[@intFromEnum(module_index) + 2] == @intFromEnum(Configuration.ImportTable.Index.invalid)); - wc.extra.items[@intFromEnum(module_index) + 2] = try wc.addDeduped(@as(Configuration.ImportTable, .{ + const import_table_index = try wc.addDeduped(Configuration.ImportTable, .{ .imports = .{ .mal = imports }, - })); + }); + wc.extra.items[@intFromEnum(module_index) + 2] = @intFromEnum(import_table_index); return module_index; } @@ -687,9 +689,9 @@ fn serialize(b: *std.Build, wc: *Configuration.Wip, writer: *Io.Writer) !void { for (dep_steps, step.dependencies.items) |*dest, src| dest.* = @enumFromInt(s.step_map.getIndex(src).?); - const deps: Configuration.Deps.Index = @enumFromInt(try wc.addDeduped(@as(Configuration.Deps, .{ + const deps: Configuration.Deps.Index = try wc.addDeduped(Configuration.Deps, .{ .steps = .{ .slice = dep_steps }, - }))); + }); try wc.steps.ensureTotalCapacity(gpa, s.step_map.entries.capacity); wc.steps.appendAssumeCapacity(.{ @@ -1278,10 +1280,10 @@ fn addOptionalResolvedTarget( optional_resolved_target: ?std.Build.ResolvedTarget, ) !Configuration.ResolvedTarget.OptionalIndex { const resolved_target = optional_resolved_target orelse return .none; - return @enumFromInt(try wc.addDeduped(@as(Configuration.ResolvedTarget, .{ + return .init(try wc.addDeduped(Configuration.ResolvedTarget, .{ .query = try wc.addTargetQuery(&resolved_target.query), .result = try wc.addTarget(resolved_target.result), - }))); + })); } fn addInstallDir(wc: *Configuration.Wip, install_dir: ?std.Build.InstallDir) !Configuration.InstallDestDir { diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 39b4b8993f..9081e3e292 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -1896,6 +1896,7 @@ fn markNeededLazyDep(b: *Build, pkg_hash: []const u8) void { /// In other words, if this function returns `null` it means that the only /// purpose of completing the configure phase is to find out all the other lazy /// dependencies that are also required. +/// /// It is allowed to use this function for non-lazy dependencies, in which case /// it will never return `null`. This allows toggling laziness via /// build.zig.zon without changing build.zig logic. diff --git a/lib/std/Build/Cache/Path.zig b/lib/std/Build/Cache/Path.zig index 14f200579c..91855a0652 100644 --- a/lib/std/Build/Cache/Path.zig +++ b/lib/std/Build/Cache/Path.zig @@ -24,7 +24,7 @@ pub fn cwd() Path { } pub fn initCwd(sub_path: []const u8) Path { - return .{ .root_dir = Cache.Directory.cwd(), .sub_path = sub_path }; + return .{ .root_dir = .cwd(), .sub_path = sub_path }; } pub fn join(p: Path, arena: Allocator, sub_path: []const u8) Allocator.Error!Path { diff --git a/lib/std/Build/Configuration.zig b/lib/std/Build/Configuration.zig index 0a1b56685c..9c25cd4927 100644 --- a/lib/std/Build/Configuration.zig +++ b/lib/std/Build/Configuration.zig @@ -374,25 +374,28 @@ pub const Wip = struct { /// Same as `addExtra` but uses a hash map to possibly return an already /// existing index instead of appending to `extra`. - pub fn addDeduped(wip: *Wip, extra: anytype) Allocator.Error!u32 { + pub fn addDeduped(wip: *Wip, comptime T: type, v: T) Allocator.Error!T.Index { const gpa = wip.gpa; const revert_index = wip.extra.items.len; - const extra_len = Storage.extraLen(extra); - try wip.extra.ensureUnusedCapacity(gpa, extra_len); - const new_index = addExtraAssumeCapacity(wip, extra); + const upper_bound_len = Storage.extraLen(v); + try wip.extra.ensureUnusedCapacity(gpa, upper_bound_len); + try wip.dedupe_table.ensureUnusedCapacityContext(gpa, 1, @as(ExtraSlice.Context, .{ + .extra = wip.extra.items, + })); + const new_index = addExtraAssumeCapacity(wip, v); const len: u32 = @intCast(wip.extra.items.len - new_index); assert(len != 0); - const gop = try wip.dedupe_table.getOrPutContext(gpa, .{ + const gop = wip.dedupe_table.getOrPutAssumeCapacityContext(.{ .index = new_index, .len = len, }, @as(ExtraSlice.Context, .{ .extra = wip.extra.items })); if (gop.found_existing) { wip.extra.items.len = revert_index; - return gop.key_ptr.index; + return @enumFromInt(gop.key_ptr.index); } - return new_index; + return @enumFromInt(new_index); } pub fn addExtraAssumeCapacity(wip: *Wip, extra: anytype) u32 { @@ -1518,6 +1521,7 @@ pub const OptionalGeneratedFileIndex = enum(u32) { pub const Package = struct { dep_prefix: String, hash: String, + root_path: String, pub const Index = enum(u32) { root = max_u32, @@ -2075,6 +2079,12 @@ pub const ResolvedTarget = struct { none = max_u32, _, + pub fn init(i: Index) OptionalIndex { + const result: OptionalIndex = @enumFromInt(@intFromEnum(i)); + assert(result != .none); + return result; + } + pub fn unwrap(this: @This()) ?Index { return switch (this) { .none => null, diff --git a/src/Package/Fetch.zig b/src/Package/Fetch.zig index ee4670070c..c8cb0c8b9d 100644 --- a/src/Package/Fetch.zig +++ b/src/Package/Fetch.zig @@ -782,7 +782,7 @@ fn runResource( f.package_root = try ls.pkg_root.join(arena, computed_package_hash.toSlice()); renameTmpIntoCache(io, package_sub_path, f.package_root) catch |err| { try eb.addRootErrorMessage(.{ .msg = try eb.printString( - "unable to rename temporary directory {f} into package cache directory {f}: {t}", + "failed renaming temporary directory {f} into package cache directory {f}: {t}", .{ package_sub_path, f.package_root, err }, ) }); return error.FetchFailed; @@ -802,7 +802,7 @@ fn runResource( if (!package_sub_path.eql(tmp_directory_path)) { tmp_directory_path.root_dir.handle.deleteDir(io, tmp_directory_path.sub_path) catch |err| switch (err) { error.Canceled => |e| return e, - else => |e| log.warn("failed to delete temporary directory {f}: {t}", .{ tmp_directory_path, e }), + else => |e| log.warn("failed deleting temporary directory {f}: {t}", .{ tmp_directory_path, e }), }; } diff --git a/src/main.zig b/src/main.zig index 1567ae0a11..36340131ac 100644 --- a/src/main.zig +++ b/src/main.zig @@ -4985,16 +4985,16 @@ fn cmdBuild( configure_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--zig", self_exe_path }; make_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--zig-lib-dir", undefined }; - const argv_index_zig_lib_dir = make_argv.items.len - 1; + const make_argv_index_zig_lib_dir = make_argv.items.len - 1; make_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--build-root", undefined }; const make_argv_index_build_root = make_argv.items.len - 1; make_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--local-cache", undefined }; - const argv_index_cache_dir = make_argv.items.len - 1; + const make_argv_index_cache_dir = make_argv.items.len - 1; make_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--global-cache", undefined }; - const argv_index_global_cache_dir = make_argv.items.len - 1; + const make_argv_index_global_cache_dir = make_argv.items.len - 1; make_argv.addManyAsArrayAssumeCapacity(2).* = .{ "--configuration", undefined }; const argv_index_configuration_file = make_argv.items.len - 1; @@ -5285,10 +5285,20 @@ fn cmdBuild( } }); defer _ = make_runner_task.cancel(io) catch {}; - make_argv.items[argv_index_zig_lib_dir] = dirs.zig_lib.path orelse cwd_path; + const pkg_root: Path = if (override_pkg_dir) |p| + .initCwd(p) + else if (system_pkg_dir_path) |p| + .initCwd(p) + else + .{ + .root_dir = build_root.directory, + .sub_path = "zig-pkg", + }; + + make_argv.items[make_argv_index_zig_lib_dir] = dirs.zig_lib.path orelse cwd_path; make_argv.items[make_argv_index_build_root] = build_root.directory.path orelse cwd_path; - make_argv.items[argv_index_global_cache_dir] = dirs.global_cache.path orelse cwd_path; - make_argv.items[argv_index_cache_dir] = dirs.local_cache.path orelse cwd_path; + make_argv.items[make_argv_index_global_cache_dir] = dirs.global_cache.path orelse cwd_path; + make_argv.items[make_argv_index_cache_dir] = dirs.local_cache.path orelse cwd_path; configure_argv.items[conf_argv_index_build_root] = build_root.directory.path orelse cwd_path; @@ -5385,15 +5395,7 @@ fn cmdBuild( .global_cache = dirs.global_cache, .local_storage = &.{ .cache_root = .{ .root_dir = dirs.local_cache, .sub_path = "" }, - .pkg_root = if (override_pkg_dir) |p| - .initCwd(p) - else if (system_pkg_dir_path) |p| - .initCwd(p) - else - .{ - .root_dir = build_root.directory, - .sub_path = "zig-pkg", - }, + .pkg_root = pkg_root, }, .recursive = true, .debug_hash = false,