Merge pull request #15407 from mlugg/feat/pkg-dedup

Deduplicate uses of the same package across dependencies

closes #15755

Author: andrewrk

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;
 }
 

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,15 +272,14 @@ 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];
 
         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,
@@ -291,30 +290,36 @@ pub fn fetchAndAddDependencies(
             all_modules,
         );
 
-        try sub_pkg.fetchAndAddDependencies(
-            root_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 root_pkg.add(gpa, fqn, 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.mod);
+        } else {
+            try deps_pkg.add(gpa, dep.hash.?, sub.mod);
+        }
 
         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(
@@ -410,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;
 
@@ -438,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);
@@ -457,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);
@@ -572,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(