-
Notifications
You must be signed in to change notification settings - Fork 1.6k
simple-dnscrypt: Fix extraction & persistence, refactor uninstaller logic #17059
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
WalkthroughReworks the simple-dnscrypt manifest: cleans metadata, changes persist from multiple entries to a single dnscrypt-proxy.toml, refactors pre_install/pre_uninstall to preserve files, and adds a new uninstaller PowerShell script that stops/removes the dnscrypt-proxy service and related processes. Changes
Sequence Diagram(s)sequenceDiagram
participant Installer as Installer Script
participant Filesystem as Filesystem (APPDIR / Persist)
participant Service as dnscrypt-proxy Service
participant Windows as Windows Service Manager
Installer->>Filesystem: Copy APPDIR files, detect persisted dnscrypt-proxy.toml
Installer->>Service: Check running processes/services
Service-->>Installer: Report status
Installer->>Service: Stop processes (Stop-Process / Stop-Service)
Installer->>Windows: Remove service (sc.exe or Remove-Service)
Installer->>Filesystem: Move *.log/*.txt to persisted dir
Installer->>Filesystem: Ensure dnscrypt-proxy.toml persisted
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
|
All changes look good. Wait for review from human collaborators. simple-dnscrypt
|
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: 2
🤖 Fix all issues with AI agents
In `@bucket/simple-dnscrypt.json`:
- Around line 54-58: The removal commands hardcode 'dnscrypt-proxy' while
detection uses a wildcard "*dnscrypt-proxy*", so change the script to capture
the actual detected service name into a variable (e.g., $svcName) and use that
variable in both the sc.exe argument list and Remove-Service call; update the
branch that uses Start-Process -FilePath 'sc.exe' -ArgumentList @('delete',
'dnscrypt-proxy') to substitute the detected $svcName, and likewise pass
$svcName to Remove-Service -Name 'dnscrypt-proxy' so the script deletes the
exact service found by the wildcard.
- Around line 47-48: The service detection is wrong: change Get-Service usage to
use the -Name parameter (Get-Service -Name '*dnscrypt-proxy*') and then retrieve
the executable path from WMI/CIM (e.g., Get-CimInstance -ClassName Win32_Service
-Filter "Name='${service.Name}'") because ServiceController objects returned by
Get-Service do not have BinaryPathName; replace the current BinaryPathName check
(using $service.BinaryPathName -notmatch $path_regex) with the path from the CIM
object and ensure subsequent checks reference $service.Name when querying the
CIM/WMI service object.
316c328 to
ce66610
Compare
|
/verify |
|
All changes look good. Wait for review from human collaborators. simple-dnscrypt
|
Summary
Refactors
simple-dnscryptto fix extraction issues, improve persistence reliability, and align the uninstaller with Scoop's lifecycle.Related issues or pull requests
Changes
description,homepage, and structured thelicensefield.APPDIRsubfolder to the root directory during installation.persistfield to the main configuration.pre_uninstallto ensure they are preserved correctly in thepersist_dir.Stop-ProcessandStop-Serviceto theuninstallerblock to prevent silent data loss and respect Scoop's process check.Notes
pre_uninstalltouninstaller, Scoop can now properly notify users if the app is running before attempting to kill it.$service.BinaryPathName -notmatch $path_regexprevents the script from accidentally stopping or deleting non-Scoop versions ofdnscrypt-proxy.query.logare created indnscrypt-proxy, the application will fail to start properly.Uninstall.exebacks updnscrypt-proxy.tomlto$env:TEMPand stops thednscrypt-proxyservice. This behavior overlaps with the logic implemented in this PR, soStart-Process -Wait "$dir\Uninstall.exe" | Out-Nullhas been removed.Testing
The test results are as follows:
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.