From f65e8c78621c90c9b9932903a9ed99c973dbcf63 Mon Sep 17 00:00:00 2001 From: mlugg Date: Sat, 22 Apr 2023 18:15:19 +0100 Subject: [PATCH 1/2] Deduplicate uses of the same package across dependencies --- lib/std/Build.zig | 16 ++++++++++++++++ src/Package.zig | 17 ++++++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 7ef504851e..d97a5c5d7a 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -124,6 +124,9 @@ host: NativeTargetInfo, dep_prefix: []const u8 = "", modules: std.StringArrayHashMap(*Module), +/// A map from build root dirs to the corresponding `*Dependency`. This is shared with all child +/// `Build`s. +initialized_deps: *std.StringHashMap(*Dependency), pub const ExecError = error{ ReadFailure, @@ -209,6 +212,9 @@ pub fn create( const env_map = try allocator.create(EnvMap); env_map.* = try process.getEnvMap(allocator); + const initialized_deps = try allocator.create(std.StringHashMap(*Dependency)); + initialized_deps.* = std.StringHashMap(*Dependency).init(allocator); + const self = try allocator.create(Build); self.* = .{ .zig_exe = zig_exe, @@ -261,6 +267,7 @@ pub fn create( .args = null, .host = host, .modules = std.StringArrayHashMap(*Module).init(allocator), + .initialized_deps = initialized_deps, }; try self.top_level_steps.put(allocator, self.install_tls.step.name, &self.install_tls); try self.top_level_steps.put(allocator, self.uninstall_tls.step.name, &self.uninstall_tls); @@ -345,6 +352,7 @@ fn createChildOnly(parent: *Build, dep_name: []const u8, build_root: Cache.Direc .host = parent.host, .dep_prefix = parent.fmt("{s}{s}.", .{ parent.dep_prefix, dep_name }), .modules = std.StringArrayHashMap(*Module).init(allocator), + .initialized_deps = parent.initialized_deps, }; try child.top_level_steps.put(allocator, child.install_tls.step.name, &child.install_tls); try child.top_level_steps.put(allocator, child.uninstall_tls.step.name, &child.uninstall_tls); @@ -1560,6 +1568,11 @@ pub fn dependencyInner( comptime build_zig: type, args: anytype, ) *Dependency { + if (b.initialized_deps.get(build_root_string)) |dep| { + // TODO: check args are the same + return dep; + } + const build_root: std.Build.Cache.Directory = .{ .path = build_root_string, .handle = std.fs.cwd().openDir(build_root_string, .{}) catch |err| { @@ -1578,6 +1591,9 @@ pub fn dependencyInner( const dep = b.allocator.create(Dependency) catch @panic("OOM"); dep.* = .{ .builder = sub_builder }; + + b.initialized_deps.put(build_root_string, dep) catch @panic("OOM"); + return dep; } diff --git a/src/Package.zig b/src/Package.zig index f84f0a8a1b..8a2875667a 100644 --- a/src/Package.zig +++ b/src/Package.zig @@ -216,7 +216,7 @@ pub const build_zig_basename = "build.zig"; pub fn fetchAndAddDependencies( pkg: *Package, - root_pkg: *Package, + deps_pkg: *Package, arena: Allocator, thread_pool: *ThreadPool, http_client: *std.http.Client, @@ -272,7 +272,6 @@ pub fn fetchAndAddDependencies( .error_bundle = error_bundle, }; - var any_error = false; const deps_list = manifest.dependencies.values(); for (manifest.dependencies.keys(), 0..) |name, i| { const dep = deps_list[i]; @@ -292,7 +291,7 @@ pub fn fetchAndAddDependencies( ); try sub_pkg.fetchAndAddDependencies( - root_pkg, + deps_pkg, arena, thread_pool, http_client, @@ -307,14 +306,18 @@ pub fn fetchAndAddDependencies( ); try pkg.add(gpa, name, sub_pkg); - try root_pkg.add(gpa, fqn, sub_pkg); + if (deps_pkg.table.get(dep.hash.?)) |other_sub| { + // This should be the same package (and hence module) since it's the same hash + // TODO: dedup multiple versions of the same package + assert(other_sub == sub_pkg); + } else { + try deps_pkg.add(gpa, dep.hash.?, sub_pkg); + } try dependencies_source.writer().print(" pub const {s} = @import(\"{}\");\n", .{ - std.zig.fmtId(fqn), std.zig.fmtEscapes(fqn), + std.zig.fmtId(fqn), std.zig.fmtEscapes(dep.hash.?), }); } - - if (any_error) return error.InvalidBuildManifestFile; } pub fn createFilePkg( From db7496d6efb64d8210816e49008753b5a81d46f4 Mon Sep 17 00:00:00 2001 From: mlugg Date: Sun, 23 Apr 2023 02:11:04 +0100 Subject: [PATCH 2/2] Only add build.zig module dependencies once --- src/Package.zig | 56 ++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/Package.zig b/src/Package.zig index 8a2875667a..f28aac885d 100644 --- a/src/Package.zig +++ b/src/Package.zig @@ -279,7 +279,7 @@ pub fn fetchAndAddDependencies( const sub_prefix = try std.fmt.allocPrint(arena, "{s}{s}.", .{ name_prefix, name }); const fqn = sub_prefix[0 .. sub_prefix.len - 1]; - const sub_pkg = try fetchAndUnpack( + const sub = try fetchAndUnpack( thread_pool, http_client, global_cache_directory, @@ -290,28 +290,30 @@ pub fn fetchAndAddDependencies( all_modules, ); - try sub_pkg.fetchAndAddDependencies( - deps_pkg, - arena, - thread_pool, - http_client, - sub_pkg.root_src_directory, - global_cache_directory, - local_cache_directory, - dependencies_source, - build_roots_source, - sub_prefix, - error_bundle, - all_modules, - ); + if (!sub.found_existing) { + try sub.mod.fetchAndAddDependencies( + deps_pkg, + arena, + thread_pool, + http_client, + sub.mod.root_src_directory, + global_cache_directory, + local_cache_directory, + dependencies_source, + build_roots_source, + sub_prefix, + error_bundle, + all_modules, + ); + } - try pkg.add(gpa, name, sub_pkg); + try pkg.add(gpa, name, sub.mod); if (deps_pkg.table.get(dep.hash.?)) |other_sub| { // This should be the same package (and hence module) since it's the same hash // TODO: dedup multiple versions of the same package - assert(other_sub == sub_pkg); + assert(other_sub == sub.mod); } else { - try deps_pkg.add(gpa, dep.hash.?, sub_pkg); + try deps_pkg.add(gpa, dep.hash.?, sub.mod); } try dependencies_source.writer().print(" pub const {s} = @import(\"{}\");\n", .{ @@ -413,7 +415,7 @@ fn fetchAndUnpack( build_roots_source: *std.ArrayList(u8), fqn: []const u8, all_modules: *AllModules, -) !*Package { +) !struct { mod: *Package, found_existing: bool } { const gpa = http_client.allocator; const s = fs.path.sep_str; @@ -441,7 +443,10 @@ fn fetchAndUnpack( const gop = try all_modules.getOrPut(gpa, hex_digest.*); if (gop.found_existing) { gpa.free(build_root); - return gop.value_ptr.*; + return .{ + .mod = gop.value_ptr.*, + .found_existing = true, + }; } const ptr = try gpa.create(Package); @@ -460,7 +465,10 @@ fn fetchAndUnpack( }; gop.value_ptr.* = ptr; - return ptr; + return .{ + .mod = ptr, + .found_existing = false, + }; } const uri = try std.Uri.parse(dep.url); @@ -575,7 +583,11 @@ fn fetchAndUnpack( std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root), }); - return createWithDir(gpa, global_cache_directory, pkg_dir_sub_path, build_zig_basename); + const mod = try createWithDir(gpa, global_cache_directory, pkg_dir_sub_path, build_zig_basename); + return .{ + .mod = mod, + .found_existing = false, + }; } fn unpackTarball(