fix(vcs): VcsArchive.Update re-fetches and atomically replaces dest - #206
Open
gmoigneu wants to merge 1 commit into
Open
fix(vcs): VcsArchive.Update re-fetches and atomically replaces dest#206gmoigneu wants to merge 1 commit into
gmoigneu wants to merge 1 commit into
Conversation
VcsArchive.Update used to return nil without doing anything. Combined with repo.Manager.syncOne only calling Clone when !IsCloned, every sync after the first was a silent no-op for archive sources — even when the upstream archive content (or the configured version: prefix) had changed. Fix: - Extend the VCS.Update interface signature with a url string. Other backends (git, hg, svn, bzr) ignore it because their on-disk metadata already records origin; the archive backend needs it because nothing on disk does. - Plumb cfg.URL through repo.Manager.syncOne and skill.Manager. - VcsArchive.Update re-fetches and re-extracts, staging into a sibling temp directory and atomically swapping (RemoveAll + Rename) on success — same pattern as #121. A crash mid-extract leaves the previous tree untouched (or the new one fully installed); never a half-written hybrid. Files removed upstream disappear because dst is replaced wholesale. - VcsArchive.Update returns an error when called without a url (defensive guard against callers that forget to pass it). Tests: - Replace TestVcsArchive_Update_NoOp with TestVcsArchive_Update_RequiresURL and TestVcsArchive_Update_RefetchesAndReplaces (the latter is the regression test for #124). - Update repo + engine tests that relied on the no-op to drive a local httptest server instead. - Mechanical sed pass for git/hg/svn/bzr/archive _test.go call sites inserting "" as the new url argument. Closes #124
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
1 participant
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.
Summary
Why
`VcsArchive.Update` was `return nil` with a TODO comment. Combined with `repo.Manager.syncOne` only calling `Clone` when `!IsCloned(path)`, every sync after the first was a silent no-op for archive-backed repos. Users pinning to an archive URL got permanently stale content with no signal.
Test plan
Closes #124