-
Notifications
You must be signed in to change notification settings - Fork 1.6k
pritunl-client: Update to version 1.3.4466.51 and refactor manifest #16997
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
Conversation
WalkthroughManifest for Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Scoop
participant PowerShell
participant ServiceMgr as Windows Service
participant Registry
User->>Scoop: scoop install pritunl-client
Scoop->>PowerShell: run installer.script (arch-specific)
PowerShell->>ServiceMgr: create/configure pritunl-client service
PowerShell->>Registry: set service failure actions
PowerShell->>ServiceMgr: start service / check status
ServiceMgr-->>PowerShell: status/confirmation
PowerShell-->>Scoop: installation complete
Scoop-->>User: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
All changes look good. Wait for review from human collaborators. pritunl-client
|
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
🤖 Fix all issues with AI agents
In @bucket/pritunl-client.json:
- Around line 75-84: The PowerShell version check in the uninstaller script uses
a float literal (6.0) against the integer $PSVersionTable.PSVersion.Major;
update the condition in the uninstaller script that reads
"$PSVersionTable.PSVersion.Major -lt 6.0" to use an integer comparison ("-lt 6")
so the check is idiomatic and matches the integer type of PSVersion.Major; keep
the surrounding logic in the "uninstaller" script block (the if that chooses
between Start-Process sc.exe and Remove-Service) unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/pritunl-client.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.
Learnt from: Gitoffthelawn
Repo: ScoopInstaller/Extras PR: 16106
File: bucket/czkawka-gui.json:25-25
Timestamp: 2025-09-05T09:41:52.653Z
Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
📚 Learning: 2025-10-19T13:58:23.389Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Applied to files:
bucket/pritunl-client.json
⏰ 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). (2)
- GitHub Check: PowerShell
- GitHub Check: WindowsPowerShell
🔇 Additional comments (7)
bucket/pritunl-client.json (7)
2-14: LGTM!The metadata updates are well-considered:
- Using
HEADin the license URL makes it resilient to default branch name changes.- SHA512 provides stronger hash verification.
- The
suggestblock for VC++ redistributable is a helpful dependency hint for users.
15-32: LGTM - clever handling of upstream naming convention.The installer scripts correctly handle the upstream's architecture-specific file naming (
,1for 64-bit,,2for ARM64). This directly addresses issue #15194 where the manifest failed to create shims due to non-existent expected filenames.
33-33: LGTM!Using
abortintegrates properly with Scoop's error handling, and the admin check is essential for the service management operations.
34-57: LGTM - robust service installation logic.The script handles both fresh installs and updates elegantly:
- Attempts to start an existing service first (update scenario)
- Creates the service only if it doesn't exist
- Configures failure actions via registry for automatic restart on crashes
The failure actions byte array correctly encodes three
SC_ACTION_RESTARTactions with increasing delays (3000ms, 5000ms, 5000ms).
58-61: LGTM - fixes the shim creation issue.The corrected bin entries (
pritunl.exeandpritunl-client.exe) directly resolve issue #15194 where the manifest previously attempted to shim a non-existentpritunlfile.
68-74: LGTM!The pre_uninstall script properly terminates the service and processes before removal. The 1.5-second sleep is a pragmatic approach to ensure Windows fully releases file handles before Scoop attempts to delete the app directory.
88-94: The autoupdate hash extraction regex is malformed and will not work.The regex pattern
(?s)$version.+?$sha512.+?$basenameuses Scoop template variable syntax ($version,$sha512,$basename) but lacks a proper capture group to extract the hash value. Scoop's autoupdate hash extraction requires a regex with an explicit capture group (typically()) to extract the hash. The pattern as written will fail to extract the SHA512 hash from the file.Remove the hash verification block from autoupdate, similar to how CompactGUI manifest handled this limitation when hash extraction became unavailable.
⛔ Skipped due to learnings
Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16378 File: bucket/compactgui.json:20-22 Timestamp: 2025-10-19T13:58:23.389Z Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.Learnt from: Gitoffthelawn Repo: ScoopInstaller/Extras PR: 16106 File: bucket/czkawka-gui.json:25-25 Timestamp: 2025-09-05T09:41:52.653Z Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds in recent versions uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format. This was confirmed by a working 10.0.0 update with valid hash.Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16349 File: bucket/fvim.json:45-49 Timestamp: 2025-10-16T15:59:21.258Z Learning: In Scoop manifests, the `autoupdate.url` and `autoupdate.architecture.<arch>.url` fields must be valid URIs according to the JSON schema (defined with `"format": "uri"`). Variables like `$matchUrlx64` that contain only path segments must be combined with a base URL (e.g., `https://github.com/.../releases/download/`) to form a complete valid URI.Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16328 File: bucket/winutil.json:0-0 Timestamp: 2025-10-13T09:37:06.093Z Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.Learnt from: Gitoffthelawn Repo: ScoopInstaller/Extras PR: 16106 File: bucket/czkawka-gui.json:25-25 Timestamp: 2025-09-05T09:41:52.653Z Learning: For czkawka-gui manifests in Scoop Extras, the correct upstream filename pattern for Windows GUI builds uses "gtk46" (without underscore), not "gtk_46" (with underscore). The autoupdate URL should use "windows_czkawka_gui_gtk46.zip" format.
820d839 to
469b1f3
Compare
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
🤖 Fix all issues with AI agents
In @bucket/pritunl-client.json:
- Around line 34-57: The post_install script should not unconditionally abort
when $service exists but failed to start; update the conditional that calls
abort to only run for fresh installs by checking $cmd -eq 'install' (i.e.,
change the current if ($service) { abort ... } to require ($cmd -eq 'install')
and $service before aborting) so updates skip abort and allow update flows to
proceed while keeping abort behavior for installs; adjust the block around
$service, $cmd, and abort in the post_install sequence where New-Service and
Start-Service are used.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bucket/pritunl-client.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
📚 Learning: 2025-10-13T09:37:06.093Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16328
File: bucket/winutil.json:0-0
Timestamp: 2025-10-13T09:37:06.093Z
Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.
Applied to files:
bucket/pritunl-client.json
📚 Learning: 2025-10-15T11:54:31.320Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16341
File: bucket/foxit-pdf-reader.json:47-50
Timestamp: 2025-10-15T11:54:31.320Z
Learning: In bucket/foxit-pdf-reader.json and bucket/foxit-reader.json, the checkver script uses MaximumRedirection 1 (not 0) for Invoke-WebRequest to properly handle Foxit's backend redirect behavior when parsing the Location header for version detection.
Applied to files:
bucket/pritunl-client.json
📚 Learning: 2025-10-19T13:58:23.389Z
Learnt from: o-l-a-v
Repo: ScoopInstaller/Extras PR: 16378
File: bucket/compactgui.json:20-22
Timestamp: 2025-10-19T13:58:23.389Z
Learning: In the ScoopInstaller/Extras repository, the CompactGUI manifest removed hash verification from the autoupdate block because the hash verification mechanism (scraping SHA-256 from release page HTML) is no longer available in newer CompactGUI releases. GitHub asset digests exist in beta releases but not in v3.8.0, and Scoop doesn't have built-in support for extracting from GitHub API asset digests.
Applied to files:
bucket/pritunl-client.json
⏰ 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 (6)
bucket/pritunl-client.json (6)
2-14: LGTM! Metadata and download configuration look solid.
- Using
HEADin the license URL is more resilient to branch renames.- SHA512 hash with proper prefix format.
innosetup: truecorrectly enables Inno Setup extraction.- The
suggestfield properly hints at the VC++ dependency without making it mandatory.
15-31: Architecture-specific file handling correctly addresses the naming issue.The scripts properly handle upstream's naming convention where
,1suffix denotes 64-bit and,2denotes ARM64 files. This directly fixes issue #15194 where shims failed due to non-existentpritunl.exe.
33-33: LGTM!Using
abortfor Scoop error integration is the correct approach, and the message clearly indicates admin rights are required.
58-61: LGTM!Both
pritunl.exe(GUI) andpritunl-client.exe(CLI) are correctly listed. The architecture-specific installer scripts ensure these files exist with proper names after extraction.
68-84: Well-designed uninstallation with proper compatibility handling.
- The
$cmd -ne 'uninstall'guard correctly preserves the service during updates.- PowerShell version check appropriately falls back to
sc.exe deletefor Windows PowerShell 5.1 (which lacksRemove-Service).- Process termination before service removal prevents file locking issues.
85-94: Regex pattern currently matches the SHA512 file format, but fragility concern is valid.The hash regex
(?s)$version.+?$sha512.+?$basenamecorrectly matches the current file structure. However, this approach is inherent fragile: the pattern depends on the upstream SHA512 file maintaining its exact format (version line followed by hash and filename pairs). Any formatting changes in the remote file would break hash extraction. Consider monitoring for upstream changes or using an alternative verification method if available.
|
/verify |
|
All changes look good. Wait for review from human collaborators. pritunl-client
|
3d8dc43 to
89124e6
Compare
|
/verify |
|
All changes look good. Wait for review from human collaborators. pritunl-client
|
|
The following test was successful:
Powershell log of the updatescoop install ~\Desktop\pritunl-client.json -u
Installing 'pritunl-client' (1.3.4439.70) [64bit] from 'C:\Users\admin\Desktop\pritunl-client.json'
Loading Pritunl.exe from cache
Checking hash of Pritunl.exe ... ok.
Extracting Pritunl.exe ... done.
Running pre_install script...done.
Running installer script...done.
Linking ~\scoop\apps\pritunl-client\current => ~\scoop\apps\pritunl-client\1.3.4439.70
Creating shim for 'pritunl'.
Making C:\Users\admin\scoop\shims\pritunl.exe a GUI binary.
Creating shim for 'pritunl-client'.
Creating shortcut for Pritunl (pritunl.exe)
Running post_install script...done.
'pritunl-client' (1.3.4439.70) was installed successfully!
'pritunl-client' suggests installing 'extras/vcredist2022'.
PS C:\Windows\system32> scoop update pritunl-client -u
scoop update: Option -u not recognized.
PS C:\Windows\system32> scoop update pritunl-client
pritunl-client: 1.3.4439.70 -> 1.3.4466.51
Updating one outdated app:
Updating 'pritunl-client' (1.3.4439.70 -> 1.3.4466.51)
Downloading new version
Loading Pritunl.exe from cache
Checking hash of Pritunl.exe ... ok.
Running pre_uninstall script...done.
Uninstalling 'pritunl-client' (1.3.4439.70)
Running uninstaller script...done.
Removing shim 'pritunl.shim'.
Removing shim 'pritunl.exe'.
Removing shim 'pritunl-client.shim'.
Removing shim 'pritunl-client.exe'.
Unlinking ~\scoop\apps\pritunl-client\current
Installing 'pritunl-client' (1.3.4466.51) [64bit] from 'C:\Users\admin\Desktop\pritunl-client.json'
Loading Pritunl.exe from cache
Extracting Pritunl.exe ... done.
Running pre_install script...done.
Running installer script...done.
Linking ~\scoop\apps\pritunl-client\current => ~\scoop\apps\pritunl-client\1.3.4466.51
Creating shim for 'pritunl'.
Making C:\Users\admin\scoop\shims\pritunl.exe a GUI binary.
Creating shim for 'pritunl-client'.
Creating shortcut for Pritunl (pritunl.exe)
Running post_install script...done.
'pritunl-client' (1.3.4466.51) was installed successfully!I don't know enough about the whole service-handling, so this is as much as I could help in this review for now 😅 |
* Add arm64 version * Update checkver & autoupdate * Refactor service management logic
89124e6 to
5e5bb6a
Compare
|
/verify |
|
All changes look good. Wait for review from human collaborators. pritunl-client
|
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
🤖 Fix all issues with AI agents
In `@bucket/pritunl-client.json`:
- Around line 56-78: The abort is called unconditionally after the retry loop,
causing a reboot prompt even when the service was removed; modify the logic in
the loop around $timer / Get-Service / $service_name so abort is only invoked on
timeout: track success (e.g. set a $removed or $completed flag when $check is
$null and break) or test the final service state or $timer.Elapsed.TotalSeconds
before calling abort, and only call abort if the service still exists or the
timeout was reached; update places that set Write-Progress and break so they
also set that success flag (or rely on checking Get-Service after the loop) to
avoid the spurious abort.
|
Summary
Updates
pritunl-clientto version 1.3.4466.51, refines service installation logic, and adds ARM64 support through post-extraction file management.Related issues or pull requests
Changes
post_install.exewith native PowerShell service creation and hardening.Remove-Service(PS 6.0+) and adding explicit process termination.abortfor better integration with Scoop's error handling.HEADinstead ofmaster).suggestfield for Microsoft Visual C++ 2015-2022 Redistributable.binentries.Notes
,1are for the 64-bit architecture, while those with,2are for the ARM64 architecture.binfield changes:The previous manifest included
pritunl.exewith a--no-mainflag (added in Pritunl Client: Add version 1.0.1953.32 #2055). However, official documentation provides no information regarding this parameter, and its original purpose remains unclear.Testing indicates that while
--dev-toolsis a documented flag, the "Developer Tools" option appears in the UI menu regardless of whether the flag is passed, making it difficult to verify ifpritunl.execorrectly accepts CLI arguments.pritunl-client.exehas been confirmed to accept command-line arguments and is now included in thebinfield for better accessibility.Testing
The test results are as follows:
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
New Features
Bug Fixes
Chores