authorgravatar for timbessmail@gmail.comTimothy Bess <timbessmail@gmail.com> 2025-03-09 18:05:11-04:00
committergravatar for alex@alexrp.comAlex Rønne Petersen <alex@alexrp.com> 2025-10-01 04:36:15+02:00
log0795e2b2ef96c75b8d5e68ed1b8ac23ced28bfce
treec9f7a5aaaef8107c5fed5bd5e18bc72303ff40c5
parent4fd78f9c2639921739836c08f12949e2476ca2ce
signaturebadge-check Signed by SSH key SHA256:7B/LJ7bpR1eX8aCXSr4mtd5M45VMPKcx9zY8e95b5QM

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
...@@ -735,28 +735,34 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {...@@ -735,28 +735,34 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
735 // calling run(); no need to add it again.735 // calling run(); no need to add it again.
736 //736 //
737 // If we add a dep as lazy and then later try to add the same dep as eager,737 // If we add a dep as lazy and then later try to add the same dep as eager,
738 // eagerness takes precedence and the existing entry is updated.738 // eagerness takes precedence and the existing entry is updated and re-scheduled
739 // for fetching.
739740
740 for (dep_names, deps) |dep_name, dep| {741 for (dep_names, deps) |dep_name, dep| {
742 var promoted_existing_to_eager = false;
741 const new_fetch = &new_fetches[new_fetch_index];743 const new_fetch = &new_fetches[new_fetch_index];
742 const location: Location = switch (dep.location) {744 const location: Location = switch (dep.location) {
743 .url => |url| .{ .remote = .{745 .url => |url| .{
744 .url = url,746 .remote = .{
745 .hash = h: {747 .url = url,
746 const h = dep.hash orelse break :h null;748 .hash = h: {
747 const pkg_hash: Package.Hash = .fromSlice(h);749 const h = dep.hash orelse break :h null;
748 if (h.len == 0) break :h pkg_hash;750 const pkg_hash: Package.Hash = .fromSlice(h);
749 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);751 if (h.len == 0) break :h pkg_hash;
750 if (gop.found_existing) {752 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);
751 if (!dep.lazy) {753 if (gop.found_existing) {
752 gop.value_ptr.*.lazy_status = .eager;754 if (!dep.lazy and gop.value_ptr.*.lazy_status != .eager) {
755 gop.value_ptr.*.lazy_status = .eager;
756 promoted_existing_to_eager = true;
757 } else {
758 continue;
759 }
753 }760 }
754 continue;761 gop.value_ptr.* = new_fetch;
755 }762 break :h pkg_hash;
756 gop.value_ptr.* = new_fetch;763 },
757 break :h pkg_hash;
758 },764 },
759 } },765 },
760 .path => |rel_path| l: {766 .path => |rel_path| l: {
761 // This might produce an invalid path, which is checked for767 // This might produce an invalid path, which is checked for
762 // at the beginning of run().768 // at the beginning of run().
...@@ -764,10 +770,12 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {...@@ -764,10 +770,12 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
764 const pkg_hash = relativePathDigest(new_root, cache_root);770 const pkg_hash = relativePathDigest(new_root, cache_root);
765 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);771 const gop = f.job_queue.table.getOrPutAssumeCapacity(pkg_hash);
766 if (gop.found_existing) {772 if (gop.found_existing) {
767 if (!dep.lazy) {773 if (!dep.lazy and gop.value_ptr.*.lazy_status != .eager) {
768 gop.value_ptr.*.lazy_status = .eager;774 gop.value_ptr.*.lazy_status = .eager;
775 promoted_existing_to_eager = true;
776 } else {
777 continue;
769 }778 }
770 continue;
771 }779 }
772 gop.value_ptr.* = new_fetch;780 gop.value_ptr.* = new_fetch;
773 break :l .{ .relative_path = new_root };781 break :l .{ .relative_path = new_root };
...@@ -775,7 +783,9 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {...@@ -775,7 +783,9 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
775 };783 };
776 prog_names[new_fetch_index] = dep_name;784 prog_names[new_fetch_index] = dep_name;
777 new_fetch_index += 1;785 new_fetch_index += 1;
778 f.job_queue.all_fetches.appendAssumeCapacity(new_fetch);786 if (!promoted_existing_to_eager) {
787 f.job_queue.all_fetches.appendAssumeCapacity(new_fetch);
788 }
779 new_fetch.* = .{789 new_fetch.* = .{
780 .arena = std.heap.ArenaAllocator.init(gpa),790 .arena = std.heap.ArenaAllocator.init(gpa),
781 .location = location,791 .location = location,