Enable Searching for Pwsh in the "Program Files (Arm)" directory on Windows Arm64#5225
Enable Searching for Pwsh in the "Program Files (Arm)" directory on Windows Arm64#5225tsmarvin wants to merge 2 commits intoPowerShell:mainfrom
Conversation
…windows arm64 - Add tests case documenting expected load order - Update test cases to refer to v7 instead of v6
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the PowerShell executable search logic on Windows Arm64 by including the "Program Files (Arm)" directory. Key changes include:
- Adding an isArm64 flag in platform details.
- Iterating over multiple possible Program Files paths (including "Program Files (Arm)").
- Updating test cases to expect version 7 of PowerShell instead of version 6.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/core/platform.test.ts | Updated test cases to include "Program Files (Arm)" and version updates. |
| src/platform.ts | Added isArm64 property and modified the logic to try multiple Program Files paths. |
Comments suppressed due to low confidence (1)
src/platform.ts:715
- Update the method's documentation to reflect the new return type, noting that it returns an array of possible ProgramFiles paths instead of a single string.
}: { useAlternateBitness?: boolean } = {}): (string | undefined)[] {
| @@ -562,93 +564,96 @@ export class PowerShellExeFinder { | |||
| }: { useAlternateBitness?: boolean; findPreview?: boolean } = {}): Promise< | |||
| IPossiblePowerShellExe | undefined | |||
| > { | |||
There was a problem hiding this comment.
Consider adding a comment explaining the iteration over multiple ProgramFiles paths to clarify the search order and rationale.
| > { | |
| > { | |
| // Iterate over possible `ProgramFiles` paths to locate PowerShell installations. | |
| // This accounts for different system configurations, such as 32-bit and 64-bit installations, | |
| // and ensures compatibility with alternate bitness settings if specified. |
|
@microsoft-github-policy-service agree |
|
Thanks! Do we have a way to meaningfully test this? Do we need to spin up/add ARM to the testing matrix? |
|
Honestly I wasn't sure what the options were there. I would certainly recommend spinning up an arm CI pipeline if its possible as the related PSES changed I linked doesn't seem to impact my x64 builds but I'm really not sure why tbh. I did add the unit tests to describe the expected order as interim support. |
|
Bah, I thought I re-ran the formatter before committing. Sorry about that, I'll get that fixed later tonight |
|
Hi, we looked at this in triage, but we are trying to find an ARM64 device to test this on to confirm it. |
|
@andyleejordan you can get an ARM64 Windows VM on Azure. If we wanted automated tests we could set it up as a github action onprem runner. EDIT: As of April 2025 Windows ARM runners are available |
PR Summary
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready