-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(scoop-install|update): Update extraction tools ahead of installs/updates #6572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughAdds detection and pre-update of extraction helpers before installs/updates, introduces Get-OutdatedHelper, extracts update logic into lib/update.ps1 and integrates it into scoop-install and scoop-update, and fixes installer filename derivation in lib/install.ps1. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as scoop-install / scoop-update
participant Detector as Get-OutdatedHelper
participant Updater as lib/update.ps1
participant Helpers as Extraction Helpers
participant App as Target App
User->>CLI: run install/update <app>
CLI->>Detector: Get-OutdatedHelper(Manifest, Architecture)
Detector->>Helpers: query installed helper versions/status
Helpers-->>Detector: return helper info
Detector-->>CLI: list of outdated helper objects
loop for each outdated helper
CLI->>Updater: update(helper.App, helper.Global, ...)
Updater->>Helpers: download & install helper
Helpers-->>Updater: updated
Updater-->>CLI: helper ready
end
CLI->>Updater: update(app, global, ...)
Updater->>App: download package (aria2 / cache)
Updater->>App: verify hashes / handle failures
Updater->>App: uninstall previous, run hooks
Updater->>App: install new version, run hooks
App-->>Updater: installation complete
Updater-->>CLI: update finished
CLI-->>User: done
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
lib/depends.ps1 (1)
226-226: Duplicate output for outdated helpers that will be updated.When a helper passes all checks (not held, has permissions), both line 226 (
info) and line 242 (Write-Host) output information about the same outdated tool. Consider removing one to avoid redundant messaging.🔎 Consider removing the info call for tools that will be updated:
Move the info message to only appear for skipped tools, or remove the duplicate Write-Host on line 242 since the
updatefunction will likely also produce output.Also applies to: 242-242
libexec/scoop-update.ps1 (1)
345-347: Stale comment: outdated entries are now PSCustomObjects, not tuples.The comment on line 346 says
$outdated is a list of ($app, $global) tuples, but the data structure was changed toPSCustomObjectwithAppandGlobalproperties.🔎 Apply this diff to update the comment:
$suggested = @{} - # $outdated is a list of ($app, $global) tuples + # $outdated is a list of PSCustomObjects with App and Global properties $outdated | ForEach-Object { update $_.App $_.Global $quiet $independent $suggested $use_cache $check_hash }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)lib/core.ps1(1 hunks)lib/depends.ps1(1 hunks)lib/install.ps1(1 hunks)lib/update.ps1(1 hunks)libexec/scoop-install.ps1(3 hunks)libexec/scoop-update.ps1(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T01:48:00.222Z
Learnt from: z-Fng
Repo: ScoopInstaller/Scoop PR: 6471
File: CHANGELOG.md:9-9
Timestamp: 2025-08-31T01:48:00.222Z
Learning: The Scoop project's CHANGELOG.md follows a convention of tracking PR numbers only, not issue numbers, according to the maintainer z-Fng.
Applied to files:
CHANGELOG.md
🔇 Additional comments (10)
CHANGELOG.md (1)
10-10: LGTM!The changelog entry follows the project's established format and convention of tracking PR numbers only.
lib/install.ps1 (1)
112-114: LGTM!The change correctly uses the
$ProcessorArchitectureparameter (defined at line 97-100) instead of an external$architecturevariable, ensuring the function derives the installer filename using the architecture passed to it.lib/core.ps1 (1)
476-489: LGTM!The inline comments clearly document the synchronization requirement between
Get-HelperPathandGet-OutdatedHelper, improving maintainability and reducing the risk of divergence when helper priority logic changes.libexec/scoop-install.ps1 (1)
136-140: LGTM!The pre-install workflow correctly identifies outdated extraction helpers and updates them before proceeding with the main installation. The flow integrates well with the new
Get-OutdatedHelperfunction andupdatemodule.lib/depends.ps1 (1)
186-200: Consider handling case when app is not yet installed.When
$versionis null (line 187),install_inforeturns null (line 194), andFormat-ArchitectureString $nullfalls back to default architecture (line 195). This works but means helpers are detected based on default architecture rather than the app's intended installation architecture.Is this the intended behavior for apps being installed for the first time? The helper detection may differ from what the actual installation will use if a non-default architecture is specified via
-aflag.lib/update.ps1 (1)
48-50: Thescript:urlfunction reference is valid and will work correctly. Script scope is created when a script file runs, and you can use scope modifiers to refer to functions defined in that scope. In the Scoop codebase,libexec/scoop-update.ps1loadslib/manifest.ps1(which defines theurlfunction at line 209) before loadinglib/update.ps1, so both exist in the same script scope and thescript:urlreference is accessible.libexec/scoop-update.ps1 (4)
27-27: LGTM!The import of
lib/update.ps1correctly loads the refactored update logic.
67-69: LGTM!Style change from
Paramtoparamis consistent with PowerShell conventions.
157-159: LGTM!Consistent style change matching
Sync-Scoop.
308-312: LGTM!Clean transition from tuple-style to
PSCustomObjectwith explicitAppandGlobalproperties improves readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/update.ps1 (1)
1-1: Fixed: $force parameter now included.The past review comment noted that
$forcewas missing from the parameter list but used on lines 22 and 95. This has been corrected—$forceis now the third parameter in the signature, and the function logic works correctly for both normal and force-update scenarios.libexec/scoop-update.ps1 (1)
326-329: Fixed: Filter now correctly references current item.The past review comment noted that line 328 incorrectly used
$outdated.App(the entire collection) instead of$_.App(the current pipeline item). This has been corrected—theWhere-Objectfilter now properly evaluates each item individually, ensuring helpers are correctly removed from the main list before being prepended to the front.
🧹 Nitpick comments (2)
libexec/scoop-update.ps1 (2)
67-67: Good: PowerShell convention followed.Changing
Paramtoparamaligns with PowerShell coding conventions.Also applies to: 157-157
308-311: Good: Explicit PSCustomObject structure improves clarity.Using
[PSCustomObject]@{ App = $app; Global = $global }instead of simple tuples makes the data structure explicit and improves readability, especially at the call site on line 346.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/update.ps1(1 hunks)libexec/scoop-install.ps1(3 hunks)libexec/scoop-update.ps1(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T01:39:35.684Z
Learnt from: z-Fng
Repo: ScoopInstaller/Scoop PR: 6473
File: libexec/scoop-download.ps1:135-135
Timestamp: 2025-08-31T01:39:35.684Z
Learning: In the Scoop codebase, `Invoke-ScoopDownload` in lib/download.ps1 calls `abort` when hash verification fails, which terminates script execution completely rather than trying remaining mirrors. The scoop-download.ps1 script should maintain consistency with this behavior by using `continue app_loop` to skip to the next app rather than trying remaining mirrors for the current app.
Applied to files:
libexec/scoop-update.ps1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (3)
libexec/scoop-install.ps1 (1)
36-36: LGTM: Import aligns with refactoring.The import of
lib/update.ps1is necessary now that theupdatefunction has been extracted fromlibexec/scoop-update.ps1and centralized in a shared library.libexec/scoop-update.ps1 (2)
27-27: LGTM: Import supports refactored update function.The import of
lib/update.ps1is correct now that theupdatefunction has been extracted to a centralized location.
346-346: LGTM: Update call correctly uses PSCustomObject properties.The call to
updateproperly extracts$_.Appand$_.Globalfrom the PSCustomObject and passes all required parameters in the correct order to match the function signature inlib/update.ps1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/core.ps1 (1)
484-484: Minor punctuation inconsistency.The comment on Line 477 ends with a period, but this one doesn't. Consider adding a period for consistency.
🔎 Proposed fix
- # Changes to the extraction tool priority should be synced with the Get-OutdatedHelper function as well + # Changes to the extraction tool priority should be synced with the Get-OutdatedHelper function as well.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)lib/core.ps1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🔇 Additional comments (1)
lib/core.ps1 (1)
477-477: LGTM!The inline comment correctly documents the synchronization requirement between Get-HelperPath and the new Get-OutdatedHelper function, improving maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
libexec/scoop-install.ps1 (1)
143-145: Consider: Error handling for helper updates.The helper updates proceed without explicit error handling. If an extraction tool update fails, the installation continues and may encounter decompression errors later.
Consider whether failed helper updates should:
- Emit a warning but continue (current behavior)
- Block the installation and exit with an error
The current approach allows installations to proceed, which may be acceptable if the existing helper version can still handle some packages.
lib/depends.ps1 (1)
210-231: LGTM: Thorough validation with helpful warnings.The function correctly filters out held apps and globally-installed apps when running without admin rights, providing clear warnings to guide users on resolution steps.
The variable
$globalon line 210 could optionally be renamed to$isGlobalfor clarity, but it doesn't cause shadowing issues since the function has no$Globalparameter.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/core.ps1lib/depends.ps1libexec/scoop-install.ps1libexec/scoop-update.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/core.ps1
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T01:39:35.684Z
Learnt from: z-Fng
Repo: ScoopInstaller/Scoop PR: 6473
File: libexec/scoop-download.ps1:135-135
Timestamp: 2025-08-31T01:39:35.684Z
Learning: In the Scoop codebase, `Invoke-ScoopDownload` in lib/download.ps1 calls `abort` when hash verification fails, which terminates script execution completely rather than trying remaining mirrors. The scoop-download.ps1 script should maintain consistency with this behavior by using `continue app_loop` to skip to the next app rather than trying remaining mirrors for the current app.
Applied to files:
libexec/scoop-update.ps1
🔇 Additional comments (12)
libexec/scoop-update.ps1 (5)
27-27: LGTM: Import added for extracted update function.The import of
lib/update.ps1is correctly placed and necessary for the refactored update function.
67-67: LGTM: Improved style consistency.Changing
Paramtoparamaligns with PowerShell conventions.Also applies to: 157-157
308-311: LGTM: Better data structure for outdated apps.Using
PSCustomObjectwith explicitAppandGlobalproperties improves clarity and enables pipeline operations by property name.
325-336: LGTM: Helper prioritization logic is correct.The reordering approach ensures extraction tools are updated before other apps. Line 335 correctly uses
$_.Appto filter individual pipeline items.The logic loads manifest and install info for all outdated apps to determine which are helpers. This overhead is acceptable given that these lookups are typically fast and the feature prevents decompression errors.
338-341: LGTM: Correctly adapted to PSCustomObject structure.The display and update invocations properly reference the
AppandGlobalproperties of the new data structure.Also applies to: 358-358
libexec/scoop-install.ps1 (3)
36-36: LGTM: Required import added.The import is necessary for the helper update functionality on line 145.
69-69: LGTM: Consistent quote usage.Using single quotes for static strings is a PowerShell best practice.
130-135: LGTM: Helper detection integrated correctly.The code properly constructs the required data structure for
Get-OutdatedHelperusing manifests and architecture information.lib/depends.ps1 (4)
174-181: LGTM: Well-defined parameters.The parameters correctly use
ValueFromPipelineByPropertyName, enabling the pipeline-based invocations inscoop-update.ps1andscoop-install.ps1.
183-191: LGTM: Efficient helper accumulation.The
processblock correctly accumulates unique installed helpers across all pipeline inputs by usingGet-InstallationHelper -Alland filtering for already-installed tools.
196-208: LGTM: Correct handling of helper name variants.The switch statement properly maps helper types to concrete app names, including special-case handling for
innounp-unicode/innounpandwixtoolset/darkvariants.
233-240: LGTM: Correct return type handling.The
PSCustomObjectstructure withAppandGlobalproperties matches the expected data format. The unary comma operator on line 239 (return , $outdated) ensures the return value is always an array, preventing PowerShell from unwrapping single-element arrays—a common PowerShell idiom for consistent return types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libexec/scoop-install.ps1 (2)
130-136: Add error handling for null manifests.If
Get-Manifestfails or returns null for any app, the subsequentGet-InstallationHelpercall withinGet-OutdatedHelpermay fail when attempting to access manifest properties. While manifests should be validated by this point in the flow, defensive error handling would improve robustness.🔎 Add null manifest check
$outdated_helpers = $apps | ForEach-Object { $null, $manifest, $null, $null = Get-Manifest $_ + if (-not $manifest) { + warn "Could not retrieve manifest for '$_'. Skipping helper check." + return + } [PSCustomObject]@{ Manifest = $manifest Architecture = $architecture } } | Get-OutdatedHelper
145-146: Consider whether-independentflag should apply to extraction tool updates.The
$independentparameter is passed to theupdatefunction, which means extraction tool dependency updates might be skipped if the user specified-i/--independent. Since extraction tools are required for the installation process itself (not app dependencies), consider whether they should always update with their dependencies regardless of the-independentflag.If extraction tools should always update with dependencies, consider:
-$outdated_helpers | ForEach-Object { update $_.App $_.Global $false $false $independent $suggested $use_cache $check_hash } +$outdated_helpers | ForEach-Object { update $_.App $_.Global $false $false $false $suggested $use_cache $check_hash }Note: In practice, extraction tools (7zip, lessmsi, innounp, dark) rarely have dependencies, so this may not cause issues.
lib/depends.ps1 (1)
216-217: Variable naming:$globaloverlaps with built-in PowerShell variable.While not technically causing issues in this context (and is a common pattern in this codebase), using
$globalas a variable name can be confusing since$global:is PowerShell's global scope qualifier. A more descriptive name like$isGlobalInstallwould improve clarity.Note: The past review comment about parameter shadowing is not applicable to this function, as
Get-OutdatedHelperdoes not have a$Globalparameter.🔎 Consider renaming for clarity
- $global = installed $app $true - $status = app_status $app $global + $isGlobalInstall = installed $app $true + $status = app_status $app $isGlobalInstallThis would also require updating references on lines 223, 233, and 241.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/depends.ps1libexec/scoop-install.ps1
🔇 Additional comments (8)
libexec/scoop-install.ps1 (2)
36-36: LGTM: Import statement correctly added.The import of
lib/update.ps1is necessary for theupdatefunction called on line 146.
69-69: Minor style improvement: Single quotes preferred.Changing to single quotes is a minor efficiency improvement in PowerShell since they don't support variable interpolation. This is good practice for static strings.
lib/depends.ps1 (6)
160-187: LGTM: Well-documented function signature.The function documentation is clear and complete. The parameter binding with
ValueFromPipelineByPropertyNamecorrectly enables pipeline input from the PSCustomObject created inscoop-install.ps1.
189-197: LGTM: Correct accumulation pattern for pipeline processing.The begin/process blocks correctly accumulate unique, installed helpers from multiple manifests. The duplicate filter on line 195 prevents the same helper from being processed multiple times.
204-210: Helper name resolution logic is correct.The switch statement correctly resolves abstract helper names to concrete app names, including the fallback logic for
innounp-unicode/innounpandwixtoolset/darkvariants.
223-236: Excellent user communication for skipped updates.The warning messages clearly explain why updates are skipped (held apps, permission issues) and provide actionable guidance for users to resolve the issues. This will help reduce confusion and support requests.
239-242: Output format matches downstream consumption.The PSCustomObject with
AppandGlobalproperties correctly matches the expected input format for theupdatefunction call inscoop-install.ps1line 146.
245-245: LGTM: Correct array return pattern.The comma operator before
$outdated(unary array operator) ensures the return value is always an array type, preventing PowerShell from unwrapping single-element arrays. This is the correct PowerShell idiom for functions that should always return arrays.
Description
This change ensures extraction tools are updated automatically ahead of installs/updates, eliminating the need for manual updates and reducing decompression-related errors.
Motivation and Context
Implementation details
Get-OutdatedHelperfunction to retrieve concrete outdated helper apps (extraction tools).updatefunction fromlibexec/scoop-update.ps1to the newly createdlib/update.ps1.$forceparameter. See 4ffe9e6libexec/scoop-install.ps1) and updates (libexec/scoop-update.ps1).How Has This Been Tested?
Tested on my own PC.
Checklist:
developbranch.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.