ReviewOS

also looking at this

pantry-pm/pantry

fix(install): keep .env-inferred services out of pantry.lock

#254
Merged glennmichael123 wants to merge fix/253-env-deps-lockfile into main
4 files +286 -70
5 days ago

Closes #253.

The problem

.env inference ran before the lock was written and its results flowed into both lock-writing paths, so the committed lockfile depended on whichever database the developer who last ran pantry install had locally:

  • the root workspace's system constraint map gained "postgresql.org": "latest", because installCompanionDepsFile passed the declared list concatenated with the inferred one to syncCompanionLockfile;
  • the resolved package entries gained postgresql.org@18.4, and gnome.org/libxslt@1.1.45 and unicode.org@73.2.0 with it, because the pipeline installs postgres's catalog dependencies too and every successful result was recorded.

CI has no such .env, removed all four again, and failed the lockfile check — so the Stacks release never published.

The rule now

An inference reaches neither path: not the service, not what the catalog says it pulls in, not as a constraint on the workspace.

env_deps.inferredOnlyDomains walks the package catalog from the inferred roots and subtracts everything a declared dependency reaches. The subtraction is the point: unicode.org is inferred-only for this project, but in a workspace that declares something else needing it, it belongs in the lock on its own merits — excluding it there would make the lock depend on the local .env in the opposite direction. Both writers consult that set.

Two consequences follow from the same rule rather than being separate decisions:

  • An inferred service cannot report the lock as out of date, so it is excluded from the --frozen-lockfile comparison as well as from the write. Otherwise installing a package that is deliberately not in the lock would fail every frozen run.
  • The frozen gate inside env_deps is gone, along with lockfileHas. It inferred only what pantry.lock already recorded, which only ever held because inference itself had put it there; with inference outside the lock there is nothing to gate on, and a frozen run now infers exactly what any other run does — which is what #36 wanted in CI to begin with.

One knock-on worth calling out: the fast up-to-date check asked the lockfile about inferred services. Left alone it would have read "not locked, so not up to date" forever and sent every install in a project with a .env down the slow path, so it now asks the disk instead (helpers.isPackageInstalled, the non-lock half of canSkipFromLockfile).

PANTRY_ENV_DEPS=0 is unchanged and still turns the whole thing off; stacksjs/stacks no longer needs it to refresh a lock.

Tests

  • A regression test drives mergeCompanionLockfileEntries with the reported shape — sqlite declared, postgres inferred, all four packages installed — and asserts the three package entries and the root-workspace constraint are all absent, i.e. the lock is byte-identical to one cut on a machine whose .env says sqlite.
  • inferredOnlyDomains is covered for the transitive case, for a declared package that needs the same library, for an outright declaration, and for nothing inferred.
  • The frozen test is replaced: inference is now independent of the lockfile, frozen runs included.

zig build test: 553 pass / 6 skip against 550 on main. The 2 failures (shell.install_route bash + zsh) are identical on stock main on this machine and are untouched by this change — verified by running the suite on both trees with the same toolchain.

🤖 Generated with Claude Code

4 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.