also looking at this
fix(install): keep .env-inferred services out of pantry.lock
#254
4 files
+286
-70
| @@ -167,11 +167,16 @@ fn tryFastUpToDate(allocator: std.mem.Allocator, cwd: []const u8, start_time: i6 | ||
| 167 | 167 | |
| 168 | 168 | // 4a. Services the project's .env names (DB_CONNECTION=pgsql, ...) count |
| 169 | 169 | // as dependencies too, or a changed driver would never be installed. |
| 170 | // | |
| 171 | // Asked of the disk rather than of the lockfile, because an inference is | |
| 172 | // deliberately absent from the lock (#253). Against the lock this read | |
| 173 | // "not locked, so not up to date" for every inferred service, sending a | |
| 174 | // project with a `.env` down the slow path on every install. | |
| 170 | 175 | { |
| 171 | 176 | const inferred = env_deps.infer(allocator, cwd, deps, .{ .report = false }) catch &.{}; |
| 172 | 177 | defer env_deps.freeDeps(allocator, inferred); |
| 173 | 178 | for (inferred) |dep| { |
| 174 | if (!helpers.canSkipFromLockfile(&lockfile.packages, dep.name, dep.version, cwd, allocator, modules_dir)) return null; | |
| 179 | if (!helpers.isPackageInstalled(cwd, modules_dir, dep.name)) return null; | |
| 175 | 180 | } |
| 176 | 181 | } |
| 177 | 182 | |
| @@ -563,7 +568,6 @@ pub fn installCommandWithOptions(allocator: std.mem.Allocator, args: []const []c | ||
| 563 | 568 | ws_file.root_dir, |
| 564 | 569 | companion_options, |
| 565 | 570 | staged_lockfile_path, |
| 566 | options.frozen_lockfile, | |
| 567 | 571 | ), df.path); |
| 568 | 572 | } |
| 569 | 573 | } |
| @@ -701,11 +705,20 @@ pub fn installCommandWithOptions(allocator: std.mem.Allocator, args: []const []c | ||
| 701 | 705 | allocator, |
| 702 | 706 | if (deps_file_path) |path| std.fs.path.dirname(path) orelse cwd else cwd, |
| 703 | 707 | deps, |
| 704 | .{ .frozen_lockfile = options.frozen_lockfile }, | |
| 708 | .{}, | |
| 705 | 709 | ) catch &.{}; |
| 706 | 710 | defer env_deps.freeDeps(allocator, env_inferred); |
| 707 | 711 | try filtered_deps.appendSlice(allocator, env_inferred); |
| 708 | 712 | |
| 713 | // They are installed, and that is all they are. `pantry.lock` records | |
| 714 | // what the workspace declares, so neither these nor what they pull in | |
| 715 | // may reach it — a lock refreshed on a machine whose `.env` says | |
| 716 | // postgres otherwise carries three entries a checkout without that | |
| 717 | // `.env` removes again (#253). | |
| 718 | var env_only_domains = env_deps.inferredOnlyDomains(allocator, env_inferred, deps) catch | |
| 719 | env_deps.DomainSet{ .allocator = allocator, .map = std.StringHashMap(void).init(allocator) }; | |
| 720 | defer env_only_domains.deinit(); | |
| 721 | ||
| 709 | 722 | // Load overrides/resolutions from package.json if it exists |
| 710 | 723 | var override_map = lib.deps.overrides.OverrideMap.init(allocator); |
| 711 | 724 | defer override_map.deinit(); |
| @@ -1219,15 +1232,21 @@ pub fn installCommandWithOptions(allocator: std.mem.Allocator, args: []const []c | ||
| 1219 | 1232 | const resolved_url = try std.fmt.allocPrint(allocator, "registry:{s}@{s}", .{ clean_name, result.version }); |
| 1220 | 1233 | defer allocator.free(resolved_url); |
| 1221 | 1234 | |
| 1222 | if (options.frozen_lockfile) { | |
| 1223 | if (!resolutionLockMatches(&lock_file, clean_name, result.version, resolved_url)) | |
| 1224 | frozen_lockfile_changed = true; | |
| 1225 | } else if (!options.no_save) { | |
| 1226 | if (lockfile_hooks.getLockedVersionForPackage(&lock_file, clean_name)) |previous| { | |
| 1227 | if (!std.mem.eql(u8, previous.version, result.version)) | |
| 1228 | style.printLockPinMoved(clean_name, previous.version, result.version, false); | |
| 1235 | // An inferred service is outside the lock's contract, so it | |
| 1236 | // is neither written to the lock nor able to report the lock | |
| 1237 | // as out of date. It is still installed, and the checkpoint | |
| 1238 | // below still records it. | |
| 1239 | if (!env_only_domains.contains(helpers.resolvePackageAlias(clean_name))) { | |
| 1240 | if (options.frozen_lockfile) { | |
| 1241 | if (!resolutionLockMatches(&lock_file, clean_name, result.version, resolved_url)) | |
| 1242 | frozen_lockfile_changed = true; | |
| 1243 | } else if (!options.no_save) { | |
| 1244 | if (lockfile_hooks.getLockedVersionForPackage(&lock_file, clean_name)) |previous| { | |
| 1245 | if (!std.mem.eql(u8, previous.version, result.version)) | |
| 1246 | style.printLockPinMoved(clean_name, previous.version, result.version, false); | |
| 1247 | } | |
| 1248 | try lockfile_hooks.addPackageToLockfile(&lock_file, clean_name, result.version, resolved_url, null); | |
| 1229 | 1249 | } |
| 1230 | try lockfile_hooks.addPackageToLockfile(&lock_file, clean_name, result.version, resolved_url, null); | |
| 1231 | 1250 | } |
| 1232 | 1251 | // Nothing of it is on disk to record below. |
| 1233 | 1252 | if (result.lock_only) continue; |
| @@ -1975,7 +1994,6 @@ fn installCompanionDepsFile( | ||
| 1975 | 1994 | project_root: []const u8, |
| 1976 | 1995 | options: types.InstallOptions, |
| 1977 | 1996 | lockfile_path: ?[]const u8, |
| 1978 | frozen_environment: bool, | |
| 1979 | 1997 | ) !types.CommandResult { |
| 1980 | 1998 | const parser = ; |
| 1981 | 1999 | const pipeline = ; |
| @@ -1989,13 +2007,10 @@ fn installCompanionDepsFile( | ||
| 1989 | 2007 | allocator.free(declared_deps); |
| 1990 | 2008 | } |
| 1991 | 2009 | |
| 1992 | // Services the project's .env names, unless declared (#36). A non-null | |
| 1993 | // Frozen installs infer only what the committed lock already records. | |
| 1994 | // A staged path is also used by ordinary installs; it does not make their | |
| 1995 | // environment inference frozen. | |
| 1996 | const env_inferred = env_deps.infer(allocator, project_root, declared_deps, .{ | |
| 1997 | .frozen_lockfile = frozen_environment, | |
| 1998 | }) catch &.{}; | |
| 2010 | // Services the project's .env names, unless declared (#36). What they | |
| 2011 | // resolve to is installed but never locked (#253), so a frozen run infers | |
| 2012 | // exactly what any other run does. | |
| 2013 | const env_inferred = env_deps.infer(allocator, project_root, declared_deps, .{}) catch &.{}; | |
| 1999 | 2014 | defer env_deps.freeDeps(allocator, env_inferred); |
| 2000 | 2015 | const deps = if (env_inferred.len == 0) declared_deps else try std.mem.concat(allocator, parser.PackageDependency, &.{ declared_deps, env_inferred }); |
| 2001 | 2016 | defer if (env_inferred.len > 0) allocator.free(deps); |
| @@ -2092,11 +2107,19 @@ fn installCompanionDepsFile( | ||
| 2092 | 2107 | // lockfile so `pantry install` actually refreshes stale runtime pins while |
| 2093 | 2108 | // preserving every workspace and npm entry. |
| 2094 | 2109 | if (!options.no_save) { |
| 2110 | // `declared_deps`, not `deps`: the constraints recorded on the root | |
| 2111 | // workspace are the ones the workspace declares. `env_only_domains` | |
| 2112 | // keeps the same inference out of the resolved package entries, where | |
| 2113 | // it arrives with everything it pulled in. | |
| 2114 | var env_only_domains = try env_deps.inferredOnlyDomains(allocator, env_inferred, declared_deps); | |
| 2115 | defer env_only_domains.deinit(); | |
| 2116 | ||
| 2095 | 2117 | const lockfile_changed = try syncCompanionLockfile( |
| 2096 | 2118 | allocator, |
| 2097 | 2119 | project_root, |
| 2098 | deps, | |
| 2120 | declared_deps, | |
| 2099 | 2121 | pipeline_result.results, |
| 2122 | &env_only_domains, | |
| 2100 | 2123 | options.frozen_lockfile, |
| 2101 | 2124 | lockfile_path, |
| 2102 | 2125 | ); |
| @@ -2118,6 +2141,7 @@ fn syncCompanionLockfile( | ||
| 2118 | 2141 | project_root: []const u8, |
| 2119 | 2142 | deps: []const .PackageDependency, |
| 2120 | 2143 | results: []const .PackageResult, |
| 2144 | env_only_domains: *const env_deps.DomainSet, | |
| 2121 | 2145 | frozen: bool, |
| 2122 | 2146 | lockfile_path_override: ?[]const u8, |
| 2123 | 2147 | ) !bool { |
| @@ -2138,7 +2162,7 @@ fn syncCompanionLockfile( | ||
| 2138 | 2162 | try lib.packages.Lockfile.init(allocator, "1.0.0"); |
| 2139 | 2163 | defer lockfile.deinit(allocator); |
| 2140 | 2164 | |
| 2141 | try mergeCompanionLockfileEntries(allocator, &lockfile, deps, results); | |
| 2165 | try mergeCompanionLockfileEntries(allocator, &lockfile, deps, results, env_only_domains); | |
| 2142 | 2166 | |
| 2143 | 2167 | const changed = if (original) |*existing| |
| 2144 | 2168 | !lockfile_writer.lockfilesEqual(existing, &lockfile) |
| @@ -2161,6 +2185,7 @@ fn mergeCompanionLockfileEntries( | ||
| 2161 | 2185 | lockfile: *lib.packages.Lockfile, |
| 2162 | 2186 | deps: []const .PackageDependency, |
| 2163 | 2187 | results: []const .PackageResult, |
| 2188 | env_only_domains: *const env_deps.DomainSet, | |
| 2164 | 2189 | ) !void { |
| 2165 | 2190 | // Packages whose resolved version depends on which platform installs them, |
| 2166 | 2191 | // so the sweep below can tell a stale pin from another platform's. |
| @@ -2218,6 +2243,7 @@ fn mergeCompanionLockfileEntries( | ||
| 2218 | 2243 | for (results) |result| { |
| 2219 | 2244 | if (!result.success or result.name.len == 0) continue; |
| 2220 | 2245 | const clean_name = helpers.resolvePackageAlias(helpers.normalizePackageName(result.name)); |
| 2246 | if (env_only_domains.contains(clean_name)) continue; | |
| 2221 | 2247 | |
| 2222 | 2248 | // Replace every older pin for this package, regardless of its key's |
| 2223 | 2249 | // version spelling (constraint or resolved version). |
| @@ -2366,7 +2392,9 @@ test "companion deps replace stale system pins without dropping workspace packag | ||
| 2366 | 2392 | .success = true, |
| 2367 | 2393 | }}; |
| 2368 | 2394 | |
| 2369 | try mergeCompanionLockfileEntries(allocator, &lockfile, &deps, &results); | |
| 2395 | var env_only = env_deps.DomainSet{ .allocator = allocator, .map = std.StringHashMap(void).init(allocator) }; | |
| 2396 | defer env_only.deinit(); | |
| 2397 | try mergeCompanionLockfileEntries(allocator, &lockfile, &deps, &results, &env_only); | |
| 2370 | 2398 | |
| 2371 | 2399 | try t.expectEqualStrings("^1.3.14", lockfile.workspaces.get("").?.system.?.get("bun.sh").?); |
| 2372 | 2400 | try t.expect(lockfile.packages.get("bun.sh@^1.3.4") == null); |
| @@ -2375,6 +2403,47 @@ test "companion deps replace stale system pins without dropping workspace packag | ||
| 2375 | 2403 | try t.expect(lockfile.workspaces.get("packages/ui") != null); |
| 2376 | 2404 | } |
| 2377 | 2405 | |
| 2406 | test "a service inferred from .env stays out of the lock, with what it pulled in" { | |
| 2407 | const t = std.testing; | |
| 2408 | const allocator = t.allocator; | |
| 2409 | const parser = ; | |
| 2410 | const pipeline = ; | |
| 2411 | ||
| 2412 | var lockfile = try lib.packages.Lockfile.init(allocator, "1.0.0"); | |
| 2413 | defer lockfile.deinit(allocator); | |
| 2414 | try lockfile.addWorkspace(allocator, "", .{ .name = try allocator.dupe(u8, "fixture") }); | |
| 2415 | ||
| 2416 | // What the workspace declares. postgresql.org is not in it: `.env` said | |
| 2417 | // DB_CONNECTION=postgres on the machine that ran this install. | |
| 2418 | const declared = [_]parser.PackageDependency{.{ .name = "sqlite.org", .version = "^3" }}; | |
| 2419 | const inferred = [_]parser.PackageDependency{.{ .name = "postgresql.org", .version = "latest" }}; | |
| 2420 | ||
| 2421 | // The pipeline installed all of it, postgres's own dependencies included. | |
| 2422 | const results = [_]pipeline.PackageResult{ | |
| 2423 | .{ .name = "sqlite.org", .version = "3.51.0", .success = true }, | |
| 2424 | .{ .name = "postgresql.org", .version = "18.4.0", .success = true }, | |
| 2425 | .{ .name = "gnome.org/libxslt", .version = "1.1.45", .success = true }, | |
| 2426 | .{ .name = "unicode.org", .version = "73.2.0", .success = true }, | |
| 2427 | }; | |
| 2428 | ||
| 2429 | var env_only = try env_deps.inferredOnlyDomains(allocator, &inferred, &declared); | |
| 2430 | defer env_only.deinit(); | |
| 2431 | try mergeCompanionLockfileEntries(allocator, &lockfile, &declared, &results, &env_only); | |
| 2432 | ||
| 2433 | // The declared package is locked, as always. | |
| 2434 | try t.expectEqualStrings("3.51.0", lockfile.packages.get("sqlite.org@3.51.0").?.version); | |
| 2435 | try t.expectEqualStrings("^3", lockfile.workspaces.get("").?.system.?.get("sqlite.org").?); | |
| 2436 | ||
| 2437 | // The three entries the reported lock gained are absent, as is the | |
| 2438 | // constraint that put "postgresql.org": "latest" on the root workspace — | |
| 2439 | // so this lock is byte-identical to one cut on a machine whose .env says | |
| 2440 | // sqlite (#253). | |
| 2441 | try t.expect(lockfile.packages.get("postgresql.org@18.4.0") == null); | |
| 2442 | try t.expect(lockfile.packages.get("gnome.org/libxslt@1.1.45") == null); | |
| 2443 | try t.expect(lockfile.packages.get("unicode.org@73.2.0") == null); | |
| 2444 | try t.expect(lockfile.workspaces.get("").?.system.?.get("postgresql.org") == null); | |
| 2445 | } | |
| 2446 | ||
| 2378 | 2447 | test "a manifest install fails when a package it asked for did not arrive" { |
| 2379 | 2448 | // The bug this pins: this path returned 0 unconditionally while every other |
| 2380 | 2449 | // install path returned 1, so `pantry install` printed "1 package(s) failed |