authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-05-20 22:58:20-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2023-05-20 22:58:20-07:00
logb7cb88384c39a15e3ce9dbeaa5a191786d186d60
tree262dd30843c537e027f997fb46975294c67f259f
parent7621e56938ed6d86827c11ee6272db722f350fed
parentdb7496d6efb64d8210816e49008753b5a81d46f4
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

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

Deduplicate uses of the same package across dependencies closes #15755

2 files changed, 57 insertions(+), 26 deletions(-)

lib/std/Build.zig+16
...@@ -124,6 +124,9 @@ host: NativeTargetInfo,...@@ -124,6 +124,9 @@ host: NativeTargetInfo,
124dep_prefix: []const u8 = "",124dep_prefix: []const u8 = "",
125125
126modules: std.StringArrayHashMap(*Module),126modules: std.StringArrayHashMap(*Module),
127/// A map from build root dirs to the corresponding `*Dependency`. This is shared with all child
128/// `Build`s.
129initialized_deps: *std.StringHashMap(*Dependency),
127130
128pub const ExecError = error{131pub const ExecError = error{
129 ReadFailure,132 ReadFailure,
...@@ -209,6 +212,9 @@ pub fn create(...@@ -209,6 +212,9 @@ pub fn create(
209 const env_map = try allocator.create(EnvMap);212 const env_map = try allocator.create(EnvMap);
210 env_map.* = try process.getEnvMap(allocator);213 env_map.* = try process.getEnvMap(allocator);
211214
215 const initialized_deps = try allocator.create(std.StringHashMap(*Dependency));
216 initialized_deps.* = std.StringHashMap(*Dependency).init(allocator);
217
212 const self = try allocator.create(Build);218 const self = try allocator.create(Build);
213 self.* = .{219 self.* = .{
214 .zig_exe = zig_exe,220 .zig_exe = zig_exe,
...@@ -261,6 +267,7 @@ pub fn create(...@@ -261,6 +267,7 @@ pub fn create(
261 .args = null,267 .args = null,
262 .host = host,268 .host = host,
263 .modules = std.StringArrayHashMap(*Module).init(allocator),269 .modules = std.StringArrayHashMap(*Module).init(allocator),
270 .initialized_deps = initialized_deps,
264 };271 };
265 try self.top_level_steps.put(allocator, self.install_tls.step.name, &self.install_tls);272 try self.top_level_steps.put(allocator, self.install_tls.step.name, &self.install_tls);
266 try self.top_level_steps.put(allocator, self.uninstall_tls.step.name, &self.uninstall_tls);273 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...@@ -345,6 +352,7 @@ fn createChildOnly(parent: *Build, dep_name: []const u8, build_root: Cache.Direc
345 .host = parent.host,352 .host = parent.host,
346 .dep_prefix = parent.fmt("{s}{s}.", .{ parent.dep_prefix, dep_name }),353 .dep_prefix = parent.fmt("{s}{s}.", .{ parent.dep_prefix, dep_name }),
347 .modules = std.StringArrayHashMap(*Module).init(allocator),354 .modules = std.StringArrayHashMap(*Module).init(allocator),
355 .initialized_deps = parent.initialized_deps,
348 };356 };
349 try child.top_level_steps.put(allocator, child.install_tls.step.name, &child.install_tls);357 try child.top_level_steps.put(allocator, child.install_tls.step.name, &child.install_tls);
350 try child.top_level_steps.put(allocator, child.uninstall_tls.step.name, &child.uninstall_tls);358 try child.top_level_steps.put(allocator, child.uninstall_tls.step.name, &child.uninstall_tls);
...@@ -1560,6 +1568,11 @@ pub fn dependencyInner(...@@ -1560,6 +1568,11 @@ pub fn dependencyInner(
1560 comptime build_zig: type,1568 comptime build_zig: type,
1561 args: anytype,1569 args: anytype,
1562) *Dependency {1570) *Dependency {
1571 if (b.initialized_deps.get(build_root_string)) |dep| {
1572 // TODO: check args are the same
1573 return dep;
1574 }
1575
1563 const build_root: std.Build.Cache.Directory = .{1576 const build_root: std.Build.Cache.Directory = .{
1564 .path = build_root_string,1577 .path = build_root_string,
1565 .handle = std.fs.cwd().openDir(build_root_string, .{}) catch |err| {1578 .handle = std.fs.cwd().openDir(build_root_string, .{}) catch |err| {
...@@ -1578,6 +1591,9 @@ pub fn dependencyInner(...@@ -1578,6 +1591,9 @@ pub fn dependencyInner(
15781591
1579 const dep = b.allocator.create(Dependency) catch @panic("OOM");1592 const dep = b.allocator.create(Dependency) catch @panic("OOM");
1580 dep.* = .{ .builder = sub_builder };1593 dep.* = .{ .builder = sub_builder };
1594
1595 b.initialized_deps.put(build_root_string, dep) catch @panic("OOM");
1596
1581 return dep;1597 return dep;
1582}1598}
15831599
src/Package.zig+41-26
...@@ -216,7 +216,7 @@ pub const build_zig_basename = "build.zig";...@@ -216,7 +216,7 @@ pub const build_zig_basename = "build.zig";
216216
217pub fn fetchAndAddDependencies(217pub fn fetchAndAddDependencies(
218 pkg: *Package,218 pkg: *Package,
219 root_pkg: *Package,219 deps_pkg: *Package,
220 arena: Allocator,220 arena: Allocator,
221 thread_pool: *ThreadPool,221 thread_pool: *ThreadPool,
222 http_client: *std.http.Client,222 http_client: *std.http.Client,
...@@ -272,7 +272,6 @@ pub fn fetchAndAddDependencies(...@@ -272,7 +272,6 @@ pub fn fetchAndAddDependencies(
272 .error_bundle = error_bundle,272 .error_bundle = error_bundle,
273 };273 };
274274
275 var any_error = false;
276 const deps_list = manifest.dependencies.values();275 const deps_list = manifest.dependencies.values();
277 for (manifest.dependencies.keys(), 0..) |name, i| {276 for (manifest.dependencies.keys(), 0..) |name, i| {
278 const dep = deps_list[i];277 const dep = deps_list[i];
...@@ -280,7 +279,7 @@ pub fn fetchAndAddDependencies(...@@ -280,7 +279,7 @@ pub fn fetchAndAddDependencies(
280 const sub_prefix = try std.fmt.allocPrint(arena, "{s}{s}.", .{ name_prefix, name });279 const sub_prefix = try std.fmt.allocPrint(arena, "{s}{s}.", .{ name_prefix, name });
281 const fqn = sub_prefix[0 .. sub_prefix.len - 1];280 const fqn = sub_prefix[0 .. sub_prefix.len - 1];
282281
283 const sub_pkg = try fetchAndUnpack(282 const sub = try fetchAndUnpack(
284 thread_pool,283 thread_pool,
285 http_client,284 http_client,
286 global_cache_directory,285 global_cache_directory,
...@@ -291,30 +290,36 @@ pub fn fetchAndAddDependencies(...@@ -291,30 +290,36 @@ pub fn fetchAndAddDependencies(
291 all_modules,290 all_modules,
292 );291 );
293292
294 try sub_pkg.fetchAndAddDependencies(293 if (!sub.found_existing) {
295 root_pkg,294 try sub.mod.fetchAndAddDependencies(
296 arena,295 deps_pkg,
297 thread_pool,296 arena,
298 http_client,297 thread_pool,
299 sub_pkg.root_src_directory,298 http_client,
300 global_cache_directory,299 sub.mod.root_src_directory,
301 local_cache_directory,300 global_cache_directory,
302 dependencies_source,301 local_cache_directory,
303 build_roots_source,302 dependencies_source,
304 sub_prefix,303 build_roots_source,
305 error_bundle,304 sub_prefix,
306 all_modules,305 error_bundle,
307 );306 all_modules,
307 );
308 }
308309
309 try pkg.add(gpa, name, sub_pkg);310 try pkg.add(gpa, name, sub.mod);
310 try root_pkg.add(gpa, fqn, sub_pkg);311 if (deps_pkg.table.get(dep.hash.?)) |other_sub| {
312 // This should be the same package (and hence module) since it's the same hash
313 // TODO: dedup multiple versions of the same package
314 assert(other_sub == sub.mod);
315 } else {
316 try deps_pkg.add(gpa, dep.hash.?, sub.mod);
317 }
311318
312 try dependencies_source.writer().print(" pub const {s} = @import(\"{}\");\n", .{319 try dependencies_source.writer().print(" pub const {s} = @import(\"{}\");\n", .{
313 std.zig.fmtId(fqn), std.zig.fmtEscapes(fqn),320 std.zig.fmtId(fqn), std.zig.fmtEscapes(dep.hash.?),
314 });321 });
315 }322 }
316
317 if (any_error) return error.InvalidBuildManifestFile;
318}323}
319324
320pub fn createFilePkg(325pub fn createFilePkg(
...@@ -410,7 +415,7 @@ fn fetchAndUnpack(...@@ -410,7 +415,7 @@ fn fetchAndUnpack(
410 build_roots_source: *std.ArrayList(u8),415 build_roots_source: *std.ArrayList(u8),
411 fqn: []const u8,416 fqn: []const u8,
412 all_modules: *AllModules,417 all_modules: *AllModules,
413) !*Package {418) !struct { mod: *Package, found_existing: bool } {
414 const gpa = http_client.allocator;419 const gpa = http_client.allocator;
415 const s = fs.path.sep_str;420 const s = fs.path.sep_str;
416421
...@@ -438,7 +443,10 @@ fn fetchAndUnpack(...@@ -438,7 +443,10 @@ fn fetchAndUnpack(
438 const gop = try all_modules.getOrPut(gpa, hex_digest.*);443 const gop = try all_modules.getOrPut(gpa, hex_digest.*);
439 if (gop.found_existing) {444 if (gop.found_existing) {
440 gpa.free(build_root);445 gpa.free(build_root);
441 return gop.value_ptr.*;446 return .{
447 .mod = gop.value_ptr.*,
448 .found_existing = true,
449 };
442 }450 }
443451
444 const ptr = try gpa.create(Package);452 const ptr = try gpa.create(Package);
...@@ -457,7 +465,10 @@ fn fetchAndUnpack(...@@ -457,7 +465,10 @@ fn fetchAndUnpack(
457 };465 };
458466
459 gop.value_ptr.* = ptr;467 gop.value_ptr.* = ptr;
460 return ptr;468 return .{
469 .mod = ptr,
470 .found_existing = false,
471 };
461 }472 }
462473
463 const uri = try std.Uri.parse(dep.url);474 const uri = try std.Uri.parse(dep.url);
...@@ -572,7 +583,11 @@ fn fetchAndUnpack(...@@ -572,7 +583,11 @@ fn fetchAndUnpack(
572 std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root),583 std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root),
573 });584 });
574585
575 return createWithDir(gpa, global_cache_directory, pkg_dir_sub_path, build_zig_basename);586 const mod = try createWithDir(gpa, global_cache_directory, pkg_dir_sub_path, build_zig_basename);
587 return .{
588 .mod = mod,
589 .found_existing = false,
590 };
576}591}
577592
578fn unpackTarball(593fn unpackTarball(