portable-virtualbox@6.4.10.5.1.22: fixed download link, checkver and autoupdate#17954
portable-virtualbox@6.4.10.5.1.22: fixed download link, checkver and autoupdate#17954arvdk wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis pull request updates the Portable-VirtualBox manifest to migrate the primary download endpoint from HTTP to HTTPS and restructures version detection logic. The manifest now uses a PowerShell-based checkver script that extracts both the Portable-VirtualBox and bundled VirtualBox versions from filenames, queries the VirtualBox download index to retrieve build numbers, and auto-generates three HTTPS URL templates for the portable archive, VirtualBox executable, and extension pack. This addresses the non-standard versioning scheme where multiple VirtualBox versions are bundled with a single Portable-VirtualBox release. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
/verify |
|
All changes look good. Wait for review from human collaborators. portable-virtualbox
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
bucket/portable-virtualbox.json (1)
42-46: Run local manifest validation before merge.Please validate this manifest end-to-end with:
scoop config debug truescoop config gh_token <your-github-token>.\bin\checkver.ps1 -App portable-virtualbox -f.\bin\formatjson.ps1 -App portable-virtualbox- Install test(s):
scoop install bucket/portable-virtualbox.json -a <architecture>Reference docs:
- https://github.com/ScoopInstaller/.github/blob/main/.github/CONTRIBUTING.md
- https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests
As per coding guidelines, “Provide clear instructions for testing the manifest locally before submission” and include official contribution/wiki links.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bucket/portable-virtualbox.json` around lines 42 - 46, The manifest bucket/portable-virtualbox.json contains the "url" array entries that must be validated end-to-end before merge; run the local manifest validation and install tests (enable debug and GH token, run checkver and formatjson, then attempt scoop install) as described in the review: run scoop config debug true and scoop config gh_token <token>, then .\bin\checkver.ps1 -App portable-virtualbox -f, .\bin\formatjson.ps1 -App portable-virtualbox and finally test installation with scoop install bucket/portable-virtualbox.json -a <architecture>; fix any checkver/formatjson/install errors and update the "url" entries or version/build variables ($matchVboxversion, $matchVboxbuild, $matchHead) in portable-virtualbox.json accordingly, and add a note in the PR that you ran these validation steps and linked the Scoop contribution/wiki docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@bucket/portable-virtualbox.json`:
- Around line 42-46: The manifest bucket/portable-virtualbox.json contains the
"url" array entries that must be validated end-to-end before merge; run the
local manifest validation and install tests (enable debug and GH token, run
checkver and formatjson, then attempt scoop install) as described in the review:
run scoop config debug true and scoop config gh_token <token>, then
.\bin\checkver.ps1 -App portable-virtualbox -f, .\bin\formatjson.ps1 -App
portable-virtualbox and finally test installation with scoop install
bucket/portable-virtualbox.json -a <architecture>; fix any
checkver/formatjson/install errors and update the "url" entries or version/build
variables ($matchVboxversion, $matchVboxbuild, $matchHead) in
portable-virtualbox.json accordingly, and add a note in the PR that you ran
these validation steps and linked the Scoop contribution/wiki docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7aeff76-6de0-4b2a-a9f3-28a512f9d03e
📒 Files selected for processing (1)
bucket/portable-virtualbox.json
|
/verify |
|
All changes look good. Wait for review from human collaborators. portable-virtualbox
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bucket/portable-virtualbox.json (1)
24-36:⚠️ Potential issue | 🔴 CriticalCritical issue: checkver script has undefined variable and logic error.
The
checkver.scriptuses$pageon line 26 before it's defined on line 30. In Scoop's sequential script execution, this causes an undefined variable error on the first-matchoperation. Additionally:
- Missing
checkver.url— The field is absent entirely. Scoop typically requires this to provide initial page content to the script.- Variable mismatch in autoupdate — The regex captures unnamed group 1
([\\d.]+)and named groupsvboxversionandvboxbuild, but autoupdate references$matchHeadwhich doesn't correspond to any defined group.Reorder the script to fetch the page before matching, add
checkver.urlto specify the initial URL, and verify the autoupdate variable names match the regex capture groups.For testing, run:
.\bin\checkver.ps1 -App portable-virtualbox -f🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@bucket/portable-virtualbox.json` around lines 24 - 36, The checkver.script array references the variable $page in the first -match operation before it is defined by the Invoke-WebRequest call, causing an undefined variable error. Reorder the script lines so that the Invoke-WebRequest line that fetches and assigns content to $page executes before any -match operations that use it. Additionally, add a checkver.url field to the checkver object to provide Scoop with the initial URL for page content. Finally, verify that any autoupdate section references variables that match the named capture groups in the regex pattern (vboxversion and vboxbuild from the regex, not $matchHead which is undefined).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@bucket/portable-virtualbox.json`:
- Around line 24-36: The checkver.script array references the variable $page in
the first -match operation before it is defined by the Invoke-WebRequest call,
causing an undefined variable error. Reorder the script lines so that the
Invoke-WebRequest line that fetches and assigns content to $page executes before
any -match operations that use it. Additionally, add a checkver.url field to the
checkver object to provide Scoop with the initial URL for page content. Finally,
verify that any autoupdate section references variables that match the named
capture groups in the regex pattern (vboxversion and vboxbuild from the regex,
not $matchHead which is undefined).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2cc1aa6b-3a7b-4d4f-bb6d-fb386c2bf9b5
📒 Files selected for processing (1)
bucket/portable-virtualbox.json
1 participant
Closes #1677
Fixed the first of 3 download links. The auto update was not implemented for the latter two, so added those. Since not all information was found on one webpage, I converted it to a script. Looks Ok now, although the 6 numbers do make a long version now.
<manifest-name[@version]|chore>: <general summary of the pull request>