also looking at this
fix(install): a lock pin decides what installs while it satisfies its spec
#263
3 files
+118
-1
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -972,6 +972,30 @@ pub fn installCommandWithOptions(allocator: std.mem.Allocator, args: []const []c | ||
| 972 | 972 | // ── Parallel pipeline: resolve tree → download → extract ── |
| 973 | 973 | const pipeline = ; |
| 974 | 974 | |
| 975 | // A committed lock decides what gets installed. Every declared range | |
| 976 | // used to be resolved against the registry, so the newest match won | |
| 977 | // and the pin was rewritten; `--frozen-lockfile` installed that newer | |
| 978 | // release and only then failed (#260). A spec the pin no longer | |
| 979 | // satisfies still resolves afresh, so editing a dependency works. | |
| 980 | // | |
| 981 | // Checked before anything is installed, which is the other half of | |
| 982 | // the report: a frozen run must fail without having written to | |
| 983 | // `pantry/` first. | |
| 984 | if (options.frozen_lockfile) { | |
| 985 | for (deps_to_install) |dep| { | |
| 986 | if (env_only_domains.contains(helpers.resolvePackageAlias(helpers.normalizePackageName(dep.name)))) continue; | |
| 987 | if (helpers.lockedVersionSatisfying(&lock_file, dep.name, dep.version) != null) continue; | |
| 988 | // `latest`-style specs are covered by any pin, so reaching | |
| 989 | // here means the lock does not cover this dependency at all, | |
| 990 | // or its pin falls outside the declared spec. | |
| 991 | style.printWarn("Lockfile does not satisfy {s}@{s} but --frozen-lockfile is set\n", .{ dep.name, dep.version }); | |
| 992 | return .{ | |
| 993 | .exit_code = 1, | |
| 994 | .message = try allocator.dupe(u8, "Error: lockfile is out of date (--frozen-lockfile)"), | |
| 995 | }; | |
| 996 | } | |
| 997 | } | |
| 998 | ||
| 975 | 999 | // Build pipeline dep list |
| 976 | 1000 | var pipeline_deps = try allocator.alloc(pipeline.PipelineDep, deps_to_install.len); |
| 977 | 1001 | defer allocator.free(pipeline_deps); |
| @@ -981,7 +1005,7 @@ pub fn installCommandWithOptions(allocator: std.mem.Allocator, args: []const []c | ||
| 981 | 1005 | const is_domain = std.mem.indexOfScalar(u8, clean_name, '.') != null; |
| 982 | 1006 | pipeline_deps[di] = .{ |
| 983 | 1007 | .name = clean_name, |
| 984 | .version = dep.version, | |
| 1008 | .version = helpers.lockedVersionSatisfying(&lock_file, dep.name, dep.version) orelse dep.version, | |
| 985 | 1009 | .source = if (is_domain) .pantry else switch (dep.source) { |
| 986 | 1010 | .registry => .npm, |
| 987 | 1011 | .pantry => .pantry, |
| @@ -2490,3 +2490,93 @@ test "addDependencyToDepsYaml repairs a file the append bug already duplicated" | ||
| 2490 | 2490 | \\ |
| 2491 | 2491 | , repaired); |
| 2492 | 2492 | } |
| 2493 | ||
| 2494 | /// The version an install should use for `dep_name`: `pantry.lock`'s pin when | |
| 2495 | /// it still satisfies `declared`, else null to resolve `declared` afresh. | |
| 2496 | /// | |
| 2497 | /// `pantry install` resolved every declared range against the registry and | |
| 2498 | /// took the newest match, so a committed lock never fixed what got installed — | |
| 2499 | /// a plain install rewrote the pin, and `--frozen-lockfile` installed the newer | |
| 2500 | /// release and only then failed. Any upstream release inside a declared range | |
| 2501 | /// turned a frozen CI run red with no change in the project (#260, | |
| 2502 | /// stacksjs/stacks#2905, where curl.se moved 8.22.0 -> 8.23.0 between pushes). | |
| 2503 | /// | |
| 2504 | /// This is `bun install` / `npm ci` semantics: the lock decides while it | |
| 2505 | /// remains valid, and `pantry update` is what moves a pin. A spec the pin no | |
| 2506 | /// longer satisfies — the range was narrowed or bumped — resolves again, which | |
| 2507 | /// is what lets a dependency edit take effect. | |
| 2508 | /// | |
| 2509 | /// An unparseable spec reads as "not satisfied", so resolution falls back to | |
| 2510 | /// the declared spec rather than installing a pin nothing has vouched for. | |
| 2511 | pub fn lockedVersionSatisfying( | |
| 2512 | lock_file: *lib.deps.resolution.LockFile, | |
| 2513 | dep_name: []const u8, | |
| 2514 | declared: []const u8, | |
| 2515 | ) ?[]const u8 { | |
| 2516 | const registry = ; | |
| 2517 | const clean_name = resolvePackageAlias(normalizePackageName(dep_name)); | |
| 2518 | const locked = lib.deps.resolution.lockfile.getLockedVersion(lock_file, clean_name) orelse return null; | |
| 2519 | if (locked.version.len == 0) return null; | |
| 2520 | ||
| 2521 | // `latest`, `*` and an empty spec are satisfied by anything, and a lock | |
| 2522 | // pin on one is the whole point of committing a lock: npm installs the | |
| 2523 | // locked version for `*` too. | |
| 2524 | if (declared.len == 0 or std.mem.eql(u8, declared, "latest") or std.mem.eql(u8, declared, "*")) | |
| 2525 | return locked.version; | |
| 2526 | ||
| 2527 | // An exact spec needs no lock to be deterministic; let it resolve itself | |
| 2528 | // so a mismatch between pin and spec stays visible. | |
| 2529 | if (!isVersionSpecRange(declared)) return null; | |
| 2530 | ||
| 2531 | const satisfies = registry.satisfiesConstraint(locked.version, declared) catch return null; | |
| 2532 | return if (satisfies) locked.version else null; | |
| 2533 | } | |
| 2534 | ||
| 2535 | /// Does this spec admit more than one version? | |
| 2536 | pub fn isVersionSpecRange(spec: []const u8) bool { | |
| 2537 | if (spec.len == 0) return true; | |
| 2538 | return switch (spec[0]) { | |
| 2539 | '^', '~', '>', '<', '=' => true, | |
| 2540 | else => std.mem.indexOfScalar(u8, spec, '*') != null or std.mem.indexOfScalar(u8, spec, 'x') != null, | |
| 2541 | }; | |
| 2542 | } | |
| 2543 | ||
| 2544 | test "which specs admit more than one version" { | |
| 2545 | try std.testing.expect(isVersionSpecRange("^3.53.4")); | |
| 2546 | try std.testing.expect(isVersionSpecRange("~1.2")); | |
| 2547 | try std.testing.expect(isVersionSpecRange(">=2")); | |
| 2548 | try std.testing.expect(isVersionSpecRange("")); | |
| 2549 | try std.testing.expect(!isVersionSpecRange("3.53.4")); | |
| 2550 | // A rolling Zig dev spec is not a range: `ziglang.org: "0.17.0-dev"` | |
| 2551 | // resolves through its own path, and a lock pin on it is reused only | |
| 2552 | // while the registry still publishes that exact build. | |
| 2553 | try std.testing.expect(!isVersionSpecRange("0.17.0-dev")); | |
| 2554 | } | |
| 2555 | ||
| 2556 | test "a lock pin decides while it satisfies the declared spec" { | |
| 2557 | const allocator = std.testing.allocator; | |
| 2558 | var lock = lib.deps.resolution.LockFile.init(allocator); | |
| 2559 | defer lock.deinit(); | |
| 2560 | try lock.addPackage("sqlite.org", "3.53.4", "registry:sqlite.org@3.53.4", null); | |
| 2561 | try lock.addPackage("curl.se", "8.22.0", "registry:curl.se@8.22.0", null); | |
| 2562 | ||
| 2563 | // The reported case: 3.54.1 exists upstream and `^3.53.4` admits it, but | |
| 2564 | // the lock says 3.53.4, so that is what installs (#260). | |
| 2565 | try std.testing.expectEqualStrings("3.53.4", lockedVersionSatisfying(&lock, "sqlite.org", "^3.53.4").?); | |
| 2566 | // Any pin covers a rolling spec, which is what committing a lock is for. | |
| 2567 | try std.testing.expectEqualStrings("3.53.4", lockedVersionSatisfying(&lock, "sqlite.org", "latest").?); | |
| 2568 | try std.testing.expectEqualStrings("3.53.4", lockedVersionSatisfying(&lock, "sqlite.org", "*").?); | |
| 2569 | // curl.se 8.22.0 -> 8.23.0 upstream is exactly what reddened a frozen CI | |
| 2570 | // run with no change in the project (stacksjs/stacks#2905); the pin holds. | |
| 2571 | try std.testing.expectEqualStrings("8.22.0", lockedVersionSatisfying(&lock, "curl.se", "^8").?); | |
| 2572 | ||
| 2573 | // A spec the pin no longer satisfies resolves again, or editing a | |
| 2574 | // dependency could never take effect. | |
| 2575 | try std.testing.expect(lockedVersionSatisfying(&lock, "sqlite.org", "^3.54.0") == null); | |
| 2576 | try std.testing.expect(lockedVersionSatisfying(&lock, "sqlite.org", "~3.52.0") == null); | |
| 2577 | // An exact spec needs no lock to be deterministic, and letting it resolve | |
| 2578 | // keeps a pin/spec disagreement visible rather than silently pinned. | |
| 2579 | try std.testing.expect(lockedVersionSatisfying(&lock, "sqlite.org", "3.54.1") == null); | |
| 2580 | // Nothing locked, nothing to honor. | |
| 2581 | try std.testing.expect(lockedVersionSatisfying(&lock, "zlib.net", "^1") == null); | |
| 2582 | } | |
| @@ -76,6 +76,9 @@ test { | ||
| 76 | 76 | // and publish_conflict.zig exist as separate files — testable logic was |
| 77 | 77 | // moved out of package.zig rather than making its tests run. |
| 78 | 78 | _ = ; |
| 79 | // Same story as package.zig above: 21 tests nothing imported for their own | |
| 80 | // sake, so none of them had ever run. | |
| 81 | _ = ; | |
| 79 | 82 | _ = ; |
| 80 | 83 | _ = ; |
| 81 | 84 | _ = ; |