Skip to content

Commit cbe3dd1

Browse files
timbessandrewrk
authored andcommitted
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 parent f50c647 commit cbe3dd1

File tree

1 file changed

+29
-19
lines changed

1 file changed

+29
-19
lines changed

src/Package/Fetch.zig

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -734,47 +734,57 @@ 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().
762768
const new_root = try f.package_root.resolvePosix(parent_arena, rel_path);
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 };
773781
},
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,

0 commit comments

Comments
 (0)