everything-alpha: Check properly for the existence of the Everything service#2279
everything-alpha: Check properly for the existence of the Everything service#2279ymeine wants to merge 1 commit intoScoopInstaller:masterfrom
Conversation
|
All changes look good. Wait for review from human collaborators. everything-alpha
|
ymeine
force-pushed
the
issue-2216-everything-alpha-service-check
branch
from
3eaf78b to
6f51e0e
Compare
|
It should align with |
|
@HUMORCE I guess you meant the opposite then, that In that case, do you mean that I should create an equivalent PR for If that is the case, the current PR here would require a proper review and approval first, so that work is not duplicated with the upcoming PR for |
Yes, label alpha-specific code with commented separately as possible, for better maintainability |
…service (fix ScoopInstaller#2216) In addition to what is described in the linked issue, it also fixes a few other things: - remove the right service during uninstall by using the service name gotten from PowerShell - stop only the processes linked to that application Those fixes are meant to handle a proper cohabitation of different versions of Everything, no matter how many processes or services they each have.
ymeine
force-pushed
the
issue-2216-everything-alpha-service-check
branch
from
6f51e0e to
b932106
Compare
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bucket/everything-alpha.json`:
- Around line 70-73: The uninstall branch calls sc.exe with an unquoted
$serviceName which will break for service names containing spaces or
parentheses; update the uninstall block where $serviceName is used (the "if
($cmd -eq 'uninstall')" branch invoking sc.exe) to pass the service name wrapped
in quotes (e.g., ensure the argument to sc.exe is "\"$serviceName\"" or the
equivalent quoting method your PowerShell script uses) so sc.exe receives the
full name as a single argument.
- Around line 40-45: The pipeline continuation is broken because the pipe is on
the next line and $processes.Length is unreliable for single items; change the
pipeline so the pipe is at the end of the Get-Process line (i.e., Put the "|" at
the end of the Get-Process command before Where-Object) and replace uses of
$processes.Length with an explicit array-wrapping count like
@($processes).Count; update the block that uses $processes (the foreach and
Stop-Process calls) to operate on the same corrected $processes variable.
- Around line 47-58: The service-detection block using Get-Service and
$appPathPattern is broken by a leading pipe and treats $services unreliably as a
scalar; fix it by ensuring the pipeline operator is at the end of the
Get-Service line (or wrap the whole pipeline in @(...)) and coerce results into
an array with @(...): call Get-Service -Name Everything* -ErrorAction
SilentlyContinue | Where-Object { $_.BinaryPathName -match $appPathPattern }
inside @(...) so $services = @(...), then replace .Length checks with array-safe
checks (e.g. ($services).Count or test array membership) and replace direct
indexing $services[0] with stable access after the @(...) coercion so single
results are handled correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18ddf746-ab0e-439a-bff8-5fa3d031d40c
📒 Files selected for processing (1)
bucket/everything-alpha.json
| "$processes = Get-Process -Name 'everything' -ErrorAction SilentlyContinue", | ||
| " | Where-Object { $_.Path -match $appPathPattern }", | ||
| "info \"Found $($processes.Length) Everything process(es) to stop\"", | ||
| "foreach ($process in $processes) {", | ||
| " Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue", | ||
| "}", |
There was a problem hiding this comment.
Pipeline continuation will not work with | at the start of a new line.
In PowerShell, when the previous line is a complete statement (like an assignment), placing | at the start of the next line does not continue the pipeline—it either causes a syntax error or operates on $null. The pipe must be at the end of the preceding line.
Additionally, $processes.Length is unreliable because PowerShell doesn't wrap single objects in an array; .Length on a single Process object returns $null or a string length, not 1. Use @(...).Count to force array context.
Proposed fix
- "$processes = Get-Process -Name 'everything' -ErrorAction SilentlyContinue",
- " | Where-Object { $_.Path -match $appPathPattern }",
- "info \"Found $($processes.Length) Everything process(es) to stop\"",
+ "$processes = @(Get-Process -Name 'everything' -ErrorAction SilentlyContinue |",
+ " Where-Object { $_.Path -match $appPathPattern })",
+ "info \"Found $($processes.Count) Everything process(es) to stop\"",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$processes = Get-Process -Name 'everything' -ErrorAction SilentlyContinue", | |
| " | Where-Object { $_.Path -match $appPathPattern }", | |
| "info \"Found $($processes.Length) Everything process(es) to stop\"", | |
| "foreach ($process in $processes) {", | |
| " Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue", | |
| "}", | |
| "$processes = @(Get-Process -Name 'everything' -ErrorAction SilentlyContinue |", | |
| " Where-Object { $_.Path -match $appPathPattern })", | |
| "info \"Found $($processes.Count) Everything process(es) to stop\"", | |
| "foreach ($process in $processes) {", | |
| " Stop-Process -Id $process.Id -Force -ErrorAction SilentlyContinue", | |
| "}", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bucket/everything-alpha.json` around lines 40 - 45, The pipeline continuation
is broken because the pipe is on the next line and $processes.Length is
unreliable for single items; change the pipeline so the pipe is at the end of
the Get-Process line (i.e., Put the "|" at the end of the Get-Process command
before Where-Object) and replace uses of $processes.Length with an explicit
array-wrapping count like @($processes).Count; update the block that uses
$processes (the foreach and Stop-Process calls) to operate on the same corrected
$processes variable.
| "$services = Get-Service -Name Everything* -ErrorAction SilentlyContinue", | ||
| " | Where-Object { $_.BinaryPathName -match $appPathPattern }", | ||
| "", | ||
| "if ($services.Length -gt 1) {", | ||
| " error 'Multiple Everything services found, this is likely a bug with this manifest.'", | ||
| " break", | ||
| "} elseif ($services.Length -eq 0) {", | ||
| " info 'No Everything service found, continuing with uninstall.'", | ||
| "} else {", | ||
| " $service = $services[0]", | ||
| " $serviceName = $service.Name", | ||
| " info \"Found Everything service: $serviceName\"", |
There was a problem hiding this comment.
Same pipeline continuation and array-handling issues in service detection.
Lines 47-48 have the same broken pipeline continuation (pipe at start of new line). Lines 50, 53 use .Length which is unreliable, and line 56's $services[0] will fail when PowerShell returns a single ServiceController object (not an array).
Wrap in @(...) to ensure array context throughout.
Proposed fix
- "$services = Get-Service -Name Everything* -ErrorAction SilentlyContinue",
- " | Where-Object { $_.BinaryPathName -match $appPathPattern }",
+ "$services = @(Get-Service -Name Everything* -ErrorAction SilentlyContinue |",
+ " Where-Object { $_.BinaryPathName -match $appPathPattern })",
"",
- "if ($services.Length -gt 1) {",
+ "if ($services.Count -gt 1) {",
"error 'Multiple Everything services found, this is likely a bug with this manifest.'",
"break",
- "} elseif ($services.Length -eq 0) {",
+ "} elseif ($services.Count -eq 0) {",
"info 'No Everything service found, continuing with uninstall.'",
"} else {",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$services = Get-Service -Name Everything* -ErrorAction SilentlyContinue", | |
| " | Where-Object { $_.BinaryPathName -match $appPathPattern }", | |
| "", | |
| "if ($services.Length -gt 1) {", | |
| " error 'Multiple Everything services found, this is likely a bug with this manifest.'", | |
| " break", | |
| "} elseif ($services.Length -eq 0) {", | |
| " info 'No Everything service found, continuing with uninstall.'", | |
| "} else {", | |
| " $service = $services[0]", | |
| " $serviceName = $service.Name", | |
| " info \"Found Everything service: $serviceName\"", | |
| "$services = @(Get-Service -Name Everything* -ErrorAction SilentlyContinue |", | |
| " Where-Object { $_.BinaryPathName -match $appPathPattern })", | |
| "", | |
| "if ($services.Count -gt 1) {", | |
| " error 'Multiple Everything services found, this is likely a bug with this manifest.'", | |
| " break", | |
| "} elseif ($services.Count -eq 0) {", | |
| " info 'No Everything service found, continuing with uninstall.'", | |
| "} else {", | |
| " $service = $services[0]", | |
| " $serviceName = $service.Name", | |
| " info \"Found Everything service: $serviceName\"", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bucket/everything-alpha.json` around lines 47 - 58, The service-detection
block using Get-Service and $appPathPattern is broken by a leading pipe and
treats $services unreliably as a scalar; fix it by ensuring the pipeline
operator is at the end of the Get-Service line (or wrap the whole pipeline in
@(...)) and coerce results into an array with @(...): call Get-Service -Name
Everything* -ErrorAction SilentlyContinue | Where-Object { $_.BinaryPathName
-match $appPathPattern } inside @(...) so $services = @(...), then replace
.Length checks with array-safe checks (e.g. ($services).Count or test array
membership) and replace direct indexing $services[0] with stable access after
the @(...) coercion so single results are handled correctly.
| " if ($cmd -eq 'uninstall') {", | ||
| " info 'Removing service'", | ||
| " sc.exe delete $serviceName", | ||
| " }", |
There was a problem hiding this comment.
Quote $serviceName when calling sc.exe to handle names with spaces.
Per issue #2216, service names like "Everything (1.5a)" contain spaces and parentheses. Passing $serviceName unquoted to sc.exe will cause the command to fail. Wrap in quotes.
Proposed fix
- "sc.exe delete $serviceName",
+ "sc.exe delete `\"$serviceName`\"",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bucket/everything-alpha.json` around lines 70 - 73, The uninstall branch
calls sc.exe with an unquoted $serviceName which will break for service names
containing spaces or parentheses; update the uninstall block where $serviceName
is used (the "if ($cmd -eq 'uninstall')" branch invoking sc.exe) to pass the
service name wrapped in quotes (e.g., ensure the argument to sc.exe is
"\"$serviceName\"" or the equivalent quoting method your PowerShell script uses)
so sc.exe receives the full name as a single argument.
z-Fng
changed the title
| " sc.exe delete 'Everything'", | ||
| "", | ||
| "$services = Get-Service -Name Everything* -ErrorAction SilentlyContinue", | ||
| " | Where-Object { $_.BinaryPathName -match $appPathPattern }", |
There was a problem hiding this comment.
Pipeline continuation will not work with
|at the start of a new line.
I can reproduce this issue on PowerShell 5. And the BinaryPathName property is inaccessible.
3 participants
In addition to what is described in the linked issue, it also fixes a few other things:
Those fixes are meant to handle a proper cohabitation of different versions of Everything, no matter how many processes or services they each have.
Closes #2216
Summary by CodeRabbit
Release Notes