authorgravatar for timbessmail@gmail.comTimothy Bess <timbessmail@gmail.com> 2025-03-09 18:05:11-04:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2025-09-30 18:58:42-07:00
logcbe3dd12c4885adb1323742b53df1079e8efb97f
treee9db83d681fa048a5d2982d2cee82faecbad73a9
parentf50c6479774d49fc21c5652b39bcad3c2512867b

Fix zig build lazy -> eager dependency promotion

Before, this had a subtle ordering bug where duplicate deps that are specified as both lazy and eager in different parts of the dependency tree end up not getting fetched depending on the ordering. I modified it to resubmit lazy deps that were promoted to eager for fetching so that it will be around for the builds that expect it to be eager downstream of this.

1 files changed, 29 insertions(+), 19 deletions(-)

src/Package/Fetch.zig+29-19
......@@ -734,28 +734,34 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
734734 // calling run(); no need to add it again.
735735 //
736736 // If we add a dep as lazy and then later try to add the same dep as eager,
737 // eagerness takes precedence and the existing entry is updated.
737 // eagerness takes precedence and the existing entry is updated and re-scheduled
738 // for fetching.
738739
739740 for (dep_names, deps) |dep_name, dep| {
741 var promoted_existing_to_eager = false;
740742 const new_fetch = &new_fetches[new_fetch_index];
741743 const location: Location = switch (dep.location) {
742 .url => |url| .{ .remote = .{
743 .url = url,
744 .hash = h: {
745 const h = dep.hash orelse break :h null;
746 const pkg_hash: Package.Hash = .fromSlice(h);
747 if (h.len == 0) break :h pkg_hash;
748 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);
749 if (gop.found_existing) {
750 if (!dep.lazy) {
751 gop.value_ptr.*.lazy_status = .eager;
744 .url => |url| .{
745 .remote = .{
746 .url = url,
747 .hash = h: {
748 const h = dep.hash orelse break :h null;
749 const pkg_hash: Package.Hash = .fromSlice(h);
750 if (h.len == 0) break :h pkg_hash;
751 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);
752 if (gop.found_existing) {
753 if (!dep.lazy and gop.value_ptr.*.lazy_status != .eager) {
754 gop.value_ptr.*.lazy_status = .eager;
755 promoted_existing_to_eager = true;
756 } else {
757 continue;
758 }
752759 }
753 continue;
754 }
755 gop.value_ptr.* = new_fetch;
756 break :h pkg_hash;
760 gop.value_ptr.* = new_fetch;
761 break :h pkg_hash;
762 },
757763 },
758 } },
764 },
759765 .path => |rel_path| l: {
760766 // This might produce an invalid path, which is checked for
761767 // at the beginning of run().
......@@ -763,10 +769,12 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
763769 const pkg_hash = relativePathDigest(new_root, cache_root);
764770 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);
765771 if (gop.found_existing) {
766 if (!dep.lazy) {
772 if (!dep.lazy and gop.value_ptr.*.lazy_status != .eager) {
767773 gop.value_ptr.*.lazy_status = .eager;
774 promoted_existing_to_eager = true;
775 } else {
776 continue;
768777 }
769 continue;
770778 }
771779 gop.value_ptr.* = new_fetch;
772780 break :l .{ .relative_path = new_root };
......@@ -774,7 +782,9 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
774782 };
775783 prog_names[new_fetch_index] = dep_name;
776784 new_fetch_index += 1;
777 f.job_queue.all_fetches.appendAssumeCapacity(new_fetch);
785 if (!promoted_existing_to_eager) {
786 f.job_queue.all_fetches.appendAssumeCapacity(new_fetch);
787 }
778788 new_fetch.* = .{
779789 .arena = std.heap.ArenaAllocator.init(gpa),
780790 .location = location,