Repository navigation
feat(cli): syspi remove drops a usage, re-locks and prunes what nothing else needs from lib/ - #10
Open
devin-ai-integration[bot] wants to merge 6 commits into
Open
devin-ai-integration[bot] wants to merge 6 commits into
devin-ai-integration[bot] wants to merge 6 commits into
Conversation
…ng else needs from lib/ The usage is matched by its canonical identity as add matches it. After the remaining usages re-resolve and re-lock, only installed copies the previous lock placed under lib/ that the new lock no longer claims are removed, and only when their contents still match that lock entry; anything else under lib/ is kept. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
…ot a failed remove Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
A crafted version with path separators or .. components in a lock entry could name a path prune would remove outside lib/; libPath now yields nothing for such an entry. Co-Authored-By: jason.han <hanhuijun@gmail.com>
The tree digest skips .git, .hg and .svn, so a copy a user had started a repository in still matched the lock; digest.MetadataDirs reports them and prune keeps the copy. Co-Authored-By: jason.han <hanhuijun@gmail.com>
…lds repository metadata, or logs to a nil writer A remaining local usage pointing inside a removed package's tree kept that tree; a tree holding .git, .hg or .svn is kept rather than deleted with the unpushed history; Remove defaults Options.Log as Run does instead of panicking after the lock changed. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
There was a problem hiding this comment.
Devin Review found 4 findings. Devin automatically fixed all 4 findings.
Fixed by Devin (4)
- Pruning deletes local repository history — prune keeps a copy that holds .git, .hg or .svn (digest.MetadataDirs) and says so; tested in TestRemoveKeepsWhatPruneMustNotTouch.
- Relative local dependencies deleted during pruning — prune keeps a copy that a local entry of the new lock is or lies inside (localWithin); tested with a local usage of lib//packages/gears.
- Optional logger causes removal panic — Remove defaults a nil Options.Log to io.Discard before anything is written; tested with a nil logger and a still-used resource.
- Crafted locks delete files outside lib — libPath now returns nothing for an entry whose install name is not a plain segment (separators, . or ..) or whose joined path is not a direct child of lib/; TestLibPathStaysUnderLib covers traversal through the version.
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.
Summary
syspi remove [flags] <resource>...— the first of the commands 0.1.0 lacks (list#13 andverify#14 are independent;update/outdatedwill stack on this branch because moving a lock entry needs the same pruning of the copy it leaves behind)..project.jsonby canonical identity, exactly asaddmatches it (origin.Parse(u.Resource).Identity()), so any accepted spelling removes it. An unknown usage is an error that leaves.project.jsonuntouched; when the resource is in the lock only because another package uses it, the message says so (no usage of X; it is locked because Y uses it).install.Run(so-offlineworks from the lock andlib/, andlib/is still only touched after everything resolved and verified). A removed usage that another locked package still uses stays locked and installed; the log names the requester.Options.Pruneininstall.Run: after the new lock is written, eachlib/path the previous lock had placed (libPath(entry):<publisher>-<name>_<version>or.kpar) that the new lock does not claim is removed — but only when its contents still match that old entry (installedTreeMatches/.kparSHA-256). A directory the lock never placed underlib/, or an installed copy whose contents were edited, is kept and said so (kept lib/base_2.1.0: its contents differ from the lock entry that installed it). Pruning runs after the lock is written, so a copy that cannot be removed is a logged leftover the next pruning run removes, never an inconsistency between lock andlib/.libPathandinstalledMatcheslive in the newinternal/install/libpath.go; feat(cli): syspi list prints the locked packages and whether each installed copy is present #13 and feat(cli): syspi verify checks every installed copy under lib/ against the lock #14 add the identical file, so the branches merge in any order..meta.jsonof the consuming project is refreshed asadddoes.Tests:
internal/install/remove_test.go(transitive package stays, user directory and edited copy kept, failing remove changes nothing, archive + standard library in one call) andtests/e2e/remove_test.go(git flow with-offline, refusal,.meta.jsonrefresh, and the git/hg/svn fixtures). Docs: README "Commands",docs/packages.md"Install layout",changes/unreleased/remove.added.md.Proved on a real project:
syspi add pkg:git/github.com/MBSE4U/sysmod-sysmlv2@5.1.5,syspi add pkg:git/github.com/Open-MBEE/OpenSysML-Extensions-Library@1.0.0, a model importingSYSMOD::*andOOSEM::*passes OpenSysML'ssysml -validate; aftersyspi remove pkg:git/github.com/MBSE4U/sysmod-sysmlv2the treelib/mbse4u-sysmod_5.1.5is gone and-validatefails withunresolved reference: SYSMOD.Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/888897f71c264c89a8abc11e9a432241
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/888897f71c264c89a8abc11e9a432241?variant=devin
Requested by: @HuiJun
🟢 Devin fixed all 4 findings · Follow Devin's work