everything: Update checkver, fix persistence & uninstaller logic#17079
everything: Update checkver, fix persistence & uninstaller logic#17079SorYoshino wants to merge 3 commits intoScoopInstaller:masterfrom
Conversation
WalkthroughManifest restructured: license changed to an object; added Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Installer as Installer (manifest scripts)
participant FS as Filesystem
participant Reg as Registry
participant Updater as Updater/Checkver
participant Uninstaller as Uninstaller (manifest)
User->>Installer: trigger install
Installer->>FS: extract/copy files (arch-aware), preserve arm64 names
Installer->>FS: copy persisted files to install dir (if present)
Installer->>Reg: generate/write registry scripts (templated, explicit encoding)
Installer->>Updater: register checkver (url + regex)
User->>Uninstaller: trigger uninstall
Uninstaller->>FS: copy persisted files out / restore, remove install files
Uninstaller->>Reg: run registry removal script
Uninstaller-->>User: uninstall complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 33-37: The persistence pattern list used in the ForEach-Object
loop is missing the wildcard for text files—'txt' should be '*.txt'—so update
the pattern array/string used with ForEach-Object (the block referencing
$persist_dir, Test-Path and Copy-Item) to include '*.txt' instead of 'txt' in
both occurrences (the loop that iterates over the patterns and the similar block
at lines ~60-64) so Test-Path and Copy-Item will correctly match and persist
.txt files; ensure you update the same pattern source used by the ForEach-Object
invocation that checks Test-Path -PathType Leaf and calls Copy-Item.
- Around line 99-101: The current "checkver" regex only captures the numeric
portion and drops pre-release letters (e.g., `a`/`b`), so update the "regex"
value in the checkver block to capture an optional single-letter pre-release
suffix (or broader letter sequence if needed) appended to the numeric version
(e.g., capture "1.5.0.1400a" not just "1.5.0.1400"); alternatively, if you only
want stable releases, change the logic in the checkver parsing to filter out any
captures that include a trailing letter and only accept pure numeric versions.
Make this change where the "checkver" object and its "regex" value are defined
so version detection preserves or filters pre-release suffixes as required.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 91-95: The removal step currently hardcodes the service name
('Everything') instead of using the discovered $service, which breaks named
instances; change the Remove-Service and the sc.exe delete invocation to use
$service.Name (and guard that $service is non-null) so both branches call
Remove-Service -Name $service.Name (or Start-Process -FilePath 'sc.exe'
-ArgumentList @('delete', $service.Name) ...) rather than the literal
'Everything'.
♻️ Duplicate comments (1)
bucket/everything.json (1)
99-102: Regex doesn't capture pre-release suffixes (alpha/beta).Per Changes.txt format, pre-release versions use suffixes like
1.5.0.1400a(alpha) or1.4.1.867b(beta). The current regex([\\d.]+)only captures the numeric portion, potentially causing version detection issues when pre-release builds are present in the changelog.
|
/verify |
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
SorYoshino
force-pushed
the
everything
branch
from
ba82321 to
0ea9221
Compare
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
|
/verify |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bucket/everything.json`:
- Around line 87-95: The service lookup fails on PowerShell 6+ because
Get-Service does not expose BinaryPathName cross-platform; update the branch
that currently uses Get-Service to instead call Get-CimInstance -ClassName
Win32_Service (same as the else branch) and then create the computed property
for BinaryPathName from PathName (or change the subsequent match to use PathName
instead of BinaryPathName) so that $services, $service and the $path_regex match
work consistently across PS versions; ensure you still use $base_service_name
for the initial name filter and preserve -ErrorAction SilentlyContinue.
|
Your changes did not pass all checks. Please address the issues in the manifest and comment starting with everything
|
|
The test failure occurs because Since the author may provide the ARM64 hash in |
1 participant
Summary
Refactors
everythingmanifest to ensure reliable data persistence, enhance uninstaller safety with registry cleanup, and modernize version detection.Related issues or pull requests
Changes
licensefield.Copy-Iteminpre_installandpre_uninstallfor*.ini,*.db,*.csv, etc.post_installto dynamically process registry scripts with correct escaping and encoding (unicode).uninstallerblock for lifecycle compliance.un*.reg) during the uninstall phase.Get-Servicefor PS 6+ andGet-CimInstancefor PS 5.1) with path-based regex validation to ensure only the Scoop-managed service is modified.Changes.txtfor more accurate version tracking.Notes
At present, the only script used to extend.everythingis(un)install-context.reg. However, additional scripts could be added in the future, such as those for startup registration or protocol handling. I will submit a separate PR to add them when I have the time. Therefore, I have proactively updated the related logic to useGet-ChildItemto dynamically retrieve the names of.regfilesTesting
The test results are as follows:
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
New Features
Refactor