Commit db8217f9a080f7c645a6448640a9af65f3944818 | zig

author	Andrew Kelley <andrew@ziglang.org> 2023-03-01 12:11:17-07:00
committer	Andrew Kelley <andrew@ziglang.org> 2023-03-01 20:01:23-05:00
log	db8217f9a080f7c645a6448640a9af65f3944818
tree	66469871a26ea98bfbb0fcaf8ccb133150292014
parent	874d3a17ae0f270b3e3f0ece7839d483d749107d

packages: avoid creating multiple modules with same build.zig

When there is a diamond dependency, reuse a *Module instead of creating
a redundant one using the same build.zig file. Otherwise, the compile
error "file exists in multiple modules" would occur.

2 files changed, 30 insertions(+), 8 deletions(-)

src
- Package.zig
- main.zig

src/Package.zig+26-8

 @@ -225,6 +225,7 @@ pub fn fetchAndAddDependencies(
     build_roots_source: *std.ArrayList(u8),
     name_prefix: []const u8,
     color: main.Color,
+    all_modules: *AllModules,
 ) !void {
     const max_bytes = 10 * 1024 * 1024;
     const gpa = thread_pool.allocator;
 @@ -291,6 +292,7 @@ pub fn fetchAndAddDependencies(
             report,
             build_roots_source,
             fqn,
+            all_modules,
         );
         try pkg.fetchAndAddDependencies(
 @@ -304,6 +306,7 @@ pub fn fetchAndAddDependencies(
             build_roots_source,
             sub_prefix,
             color,
+            all_modules,
         );
         try add(pkg, gpa, fqn, sub_pkg);
 @@ -402,6 +405,11 @@ const Report = struct {
     }
 };
+const hex_multihash_len = 2 * Manifest.multihash_len;
+const MultiHashHexDigest = [hex_multihash_len]u8;
+/// This is to avoid creating multiple modules for the same build.zig file.
+pub const AllModules = std.AutoHashMapUnmanaged(MultiHashHexDigest, *Package);
 fn fetchAndUnpack(
     thread_pool: *ThreadPool,
     http_client: *std.http.Client,
 @@ -410,6 +418,7 @@ fn fetchAndUnpack(
     report: Report,
     build_roots_source: *std.ArrayList(u8),
     fqn: []const u8,
+    all_modules: *AllModules,
 ) !*Package {
     const gpa = http_client.allocator;
     const s = fs.path.sep_str;
 @@ -417,9 +426,24 @@ fn fetchAndUnpack(
     // Check if the expected_hash is already present in the global package
     // cache, and thereby avoid both fetching and unpacking.
     if (dep.hash) |h| cached: {
-        const hex_multihash_len = 2 * Manifest.multihash_len;
         const hex_digest = h[0..hex_multihash_len];
         const pkg_dir_sub_path = "p" ++ s ++ hex_digest;
+        const build_root = try global_cache_directory.join(gpa, &.{pkg_dir_sub_path});
+        errdefer gpa.free(build_root);
+        try build_roots_source.writer().print("    pub const {s} = \"{}\";\n", .{
+            std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root),
+        });
+        // The compiler has a rule that a file must not be included in multiple modules,
+        // so we must detect if a module has been created for this package and reuse it.
+        const gop = try all_modules.getOrPut(gpa, hex_digest.*);
+        if (gop.found_existing) {
+            gpa.free(build_root);
+            return gop.value_ptr.*;
+        }
         var pkg_dir = global_cache_directory.handle.openDir(pkg_dir_sub_path, .{}) catch |err| switch (err) {
             error.FileNotFound => break :cached,
             else => |e| return e,
 @@ -432,13 +456,6 @@ fn fetchAndUnpack(
         const owned_src_path = try gpa.dupe(u8, build_zig_basename);
         errdefer gpa.free(owned_src_path);
-        const build_root = try global_cache_directory.join(gpa, &.{pkg_dir_sub_path});
-        errdefer gpa.free(build_root);
-        try build_roots_source.writer().print("    pub const {s} = \"{}\";\n", .{
-            std.zig.fmtId(fqn), std.zig.fmtEscapes(build_root),
-        });
         ptr.* = .{
             .root_src_directory = .{
                 .path = build_root,
 @@ -448,6 +465,7 @@ fn fetchAndUnpack(
             .root_src_path = owned_src_path,
         };
+        gop.value_ptr.* = ptr;
         return ptr;
     }

src/main.zig+4

...	@@ -4244,6 +4244,9 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi	...	@@ -4244,6 +4244,9 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
4244	var build_roots_source = std.ArrayList(u8).init(gpa);	4244	var build_roots_source = std.ArrayList(u8).init(gpa);
4245	defer build_roots_source.deinit();	4245	defer build_roots_source.deinit();
4246		4246
		4247	var all_modules: Package.AllModules = .{};
		4248	defer all_modules.deinit(gpa);
		4249
4247	// Here we borrow main package's table and will replace it with a fresh	4250	// Here we borrow main package's table and will replace it with a fresh
4248	// one after this process completes.	4251	// one after this process completes.
4249	main_pkg.fetchAndAddDependencies(	4252	main_pkg.fetchAndAddDependencies(
...	@@ -4257,6 +4260,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi	...	@@ -4257,6 +4260,7 @@ pub fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !voi
4257	&build_roots_source,	4260	&build_roots_source,
4258	"",	4261	"",
4259	color,	4262	color,
		4263	&all_modules,
4260	) catch \|err\| switch (err) {	4264	) catch \|err\| switch (err) {
4261	error.PackageFetchFailed => process.exit(1),	4265	error.PackageFetchFailed => process.exit(1),
4262	else => \|e\| return e,	4266	else => \|e\| return e,