fix(composer): record dist archives per commit so locked versions install - #179
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branches move, but Pricore kept one
package_versionsrow per version string and mutated it in place on every sync. Archive files are one-per-commit, so each sync wrote a new file and repointed the row, leaving the old archive on disk with nothing referencing it.composer installfrom a lock pinning an older commit then 404'd.Archives are now recorded individually in a new
dist_archivestable, keyed by commit, so a locked commit resolves by lookup.package_versions.dist_*stays as a write-through cache of the current archive, withRecordDistArchiveActionas the only writer.This also fixes a second, quieter bug on both sync paths: the dist columns were left describing the previous commit after
source_referencemoved, so metadata advertised the old archive — and its matching shasum — under the new reference, and Composer installed stale bytes without complaint. The pointer is now invalidated in the same transaction that moves the reference.Fixes #178
Notes for review
DIST_KEEP_DETACHED_DAYSbounds how long superseded archives are kept, measured from when an archive stopped being current rather than when it was built. Unset by default, so upgrades keep every archive and nothing becomes prunable.dist:cleanupremains unscheduled, as it was before.Known trade-off
Between the reference moving and the new archive being recorded, a version advertises no dist and Composer falls back to cloning. The window lasts as long as the archive download. Building the archive before the transaction would close it and is worth doing as a follow-up.
Deliberately left out
dist:reconcilesweep for files whose version row was already destroyed. Those are recoverable by neither lookup nor path, so they are garbage collection rather than part of this fix.SUM(size)possible; no UI here.dist_urlstill bakes inAPP_URL. Now thatDistArchiveData::urlFor()exists, computing it at read time is a contained change.