Skip to content

fix: restore Windows PowerShell 5.1 import compatibility#126

Merged
tablackburn merged 9 commits into
mainfrom
fix/windows-powershell-5.1-compat
Jun 4, 2026
Merged

fix: restore Windows PowerShell 5.1 import compatibility#126
tablackburn merged 9 commits into
mainfrom
fix/windows-powershell-5.1-compat

Conversation

@tablackburn
Copy link
Copy Markdown
Contributor

Summary

PowerShellBuild 0.8.0 fails to import under Windows PowerShell 5.1 (Desktop). Get-PSBuildCertificate (added in #92) used the PowerShell 7+-only ternary operator; since the module loader dot-sources every Public/*.ps1 at import, the parse failure broke import of the whole module on 5.1 — even though the manifest still declares PowerShellVersion = '3.0'.

Fix

  • Ternary → if/else expression in Get-PSBuildCertificate (5.1-safe; identical PS7 behavior).
  • $IsWindows guard made 5.1-safe using the repo's existing idiom ($null -ne $IsWindows -and -not $IsWindows, as in Build-PSBuildUpdatableHelp). On Desktop edition the automatic variable is absent and the platform is always Windows.

Audit

Swept all .ps1/.psm1/.psd1 under PowerShellBuild/, build/, and tests/ (and re-parsed every module file on a real 5.1 engine) for PS7-only constructs (ternary, ??/??=, ?./?[ ], &&/||, clean{}, -Parallel, Core-only $Is* vars, Core-only cmdlets/types). The two issues in Get-PSBuildCertificate.ps1 were the only real incompatibilities.

Guardrails

  • PSScriptAnalyzer compatibility rules (PSUseCompatibleSyntax/Commands/Types) enabled in ScriptAnalyzerSettings.psd1, targeting Windows PowerShell 5.1 (Desktop) and PowerShell 7. PSUseCompatibleSyntax checks the target language version regardless of the engine it runs on, so it catches a ternary even from pwsh. Two documented false positives are suppressed with justifications (Get-ChildItem -CodeSigningCert provider dynamic parameter; Invoke-Pester -Configuration from the required Pester 5+).
  • Analyze build task fails on any compatibility violation (these rules are Warning-severity, which the existing Error-only gate would not catch).
  • New import-smoke CI job parses and imports the module on the real Windows PowerShell 5.1 engine (shell: powershell).

Verification

  • Parse + dot-source + Import-Module on real Windows PowerShell 5.1: clean (original failed with the documented parse errors).
  • PSScriptAnalyzer with the new settings: 0 compatibility findings; confirmed it flags a reintroduced ternary.
  • psake Test (Analyze + Pester) on pwsh 7: Analyze clean of compatibility issues; 304 Pester tests pass, including all of Get-PSBuildCertificate.tests.ps1.

Support contract

Per maintainer direction, the manifest (PowerShellVersion/CompatiblePSEditions) is left unchanged in this PR; the code is fixed to comply with the existing 5.1/Desktop contract. Formalizing the declared floor is tracked for the v1.0.0 roadmap (#120).

🤖 Generated with Claude Code

Get-PSBuildCertificate used the PowerShell 7+-only ternary operator, which
fails to parse under Windows PowerShell 5.1. The module loader dot-sources
every Public/*.ps1 at import, so this file broke import of the entire module
on 5.1 (Desktop) even though the manifest still declares support for it.

- Replace the ternary with an if/else expression (5.1-safe).
- Make the $IsWindows platform guard 5.1-safe using the repo's existing
  idiom ($null -ne $IsWindows -and -not $IsWindows); on Desktop edition the
  automatic variable is absent and the platform is always Windows. PS7
  behavior is unchanged.

Add guardrails so this cannot silently regress:
- Enable PSUseCompatibleSyntax/Commands/Types (targeting 5.1 and 7.0) in
  ScriptAnalyzerSettings.psd1, with justified suppressions for two
  documented false positives (Get-ChildItem -CodeSigningCert provider
  dynamic parameter; Invoke-Pester -Configuration from required Pester 5+).
- Fail the Analyze build task on any compatibility violation.
- Add an import-smoke CI job that parses and imports the module on the real
  Windows PowerShell 5.1 engine.

The manifest support contract (PowerShellVersion/CompatiblePSEditions) is
intentionally left unchanged here; formalizing the real floor is tracked for
the v1.0.0 roadmap (#120).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 3, 2026 20:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores Windows PowerShell 5.1 (Desktop) module import compatibility by removing PowerShell 7-only syntax from a public function that is dot-sourced during module import, and adds guardrails to prevent similar regressions via ScriptAnalyzer gating and a 5.1 import smoke test.

Changes:

  • Replace PowerShell 7+ ternary usage and make $IsWindows checks safe on Windows PowerShell 5.1 in Get-PSBuildCertificate.
  • Enable PSScriptAnalyzer compatibility rules and fail the build on any PSUseCompatible* findings.
  • Add a CI job that parses and imports the module under Windows PowerShell 5.1.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
psakeFile.ps1 Makes Analyze fail the build on PSUseCompatible* findings while preserving existing error/warning handling.
PowerShellBuild/ScriptAnalyzerSettings.psd1 Enables PSUseCompatibleSyntax/Commands/Types targeting Windows PowerShell 5.1 and PowerShell 7.
PowerShellBuild/Public/Test-PSBuildPester.ps1 Suppresses a documented PSUseCompatibleCommands false positive for Invoke-Pester -Configuration.
PowerShellBuild/Public/Get-PSBuildCertificate.ps1 Removes PS7-only ternary usage and makes the Windows-only guard safe when $IsWindows is absent on 5.1.
CHANGELOG.md Documents the 5.1 import fix and the newly added compatibility guardrails.
.github/workflows/test.yml Adds a Windows PowerShell 5.1 parse+import smoke-test job to CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Promote the Unreleased changelog entries to 0.8.1 and bump the module
manifest ModuleVersion to match. 0.8.1 is a backward-compatible patch
release restoring Windows PowerShell 5.1 import compatibility (see #126).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tablackburn
Copy link
Copy Markdown
Contributor Author

Added the 0.8.1 release bump to this PR: promoted the Unreleased CHANGELOG entries to ## [0.8.1] 2026-06-03 and bumped ModuleVersion to 0.8.1 (backward-compatible patch). Verified the changelog↔manifest version-match and Test-ModuleManifest locally. After merge, I'll tag/publish v0.8.1.

tablackburn and others added 2 commits June 3, 2026 16:23
PSUseCompatibleCommands and PSUseCompatibleTypes intermittently threw a
NullReferenceException inside Invoke-ScriptAnalyzer on the macOS CI runner,
aborting the Analyze task (it passed on the first run, then failed on a
re-run with identical settings; ubuntu and windows passed both times). They
also produced false positives that required per-function suppressions
(Get-ChildItem -CodeSigningCert provider dynamic parameter; Invoke-Pester
-Configuration from the required Pester 5+).

Keep only PSUseCompatibleSyntax, which catches the actual regression class
(PowerShell 7+-only syntax such as the ternary operator), performs no
compatibility-profile deserialization, and is not subject to that NRE. The
two suppressions are removed as no longer needed. Core-only cmdlet/type
usage remains covered by the Windows PowerShell 5.1 import-smoke CI job.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PSUseCompatible* rule family (Syntax, Commands, Types) intermittently
throws inside Invoke-ScriptAnalyzer under -Recurse (NullReferenceException,
and "Value cannot be null (Parameter 'name')"), failing the Analyze task on
a rotating subset of CI runners (macOS one run, windows the next). This is a
reentrancy bug in the analyzer rules, not something the build can gate
around, so ScriptAnalyzerSettings.psd1 and the Analyze task are reverted to
their previous state and the per-function suppressions are dropped.

The Windows PowerShell 5.1 import-smoke CI job remains as the compatibility
guardrail: it parses and imports the module on the real lowest-supported
engine and deterministically catches a PowerShell 7+-only construct (such as
the ternary operator that caused this regression).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@HeyItsGilbert HeyItsGilbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with a suggestion to run the tests in 5.1 as well (vs just an import test).

Comment thread .github/workflows/test.yml Outdated
tablackburn and others added 2 commits June 3, 2026 17:56
Replace the bespoke test + import-smoke jobs with a thin caller of the
org's shared psake/.github ModuleCI workflow, which runs the full
build/test suite on PowerShell 7+ across Linux/Windows/macOS and on the
real Windows PowerShell 5.1 (Desktop) engine. The dedicated 5.1 job
subsumes the previous import-only smoke test, so a regression like the
0.8.0 ternary that broke module import on 5.1 is caught by the standard
test run. Pin to a tag/SHA once the 5.1 job has merged upstream.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Pester task computed $testResultsXml but never passed it to
Invoke-Pester, so no results file was ever written, and the path used
OutputDir (Output/) rather than the tests/out path the CI workflow
uploads. Switch to a Pester 5 configuration object that writes
tests/out/testResults.xml so the shared workflow's upload/publish
steps have results to consume.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Test Results

    4 files    376 suites   20s ⏱️
  316 tests   314 ✅  2 💤 0 ❌
1 264 runs  1 217 ✅ 47 💤 0 ❌

Results for commit aa6283d.

♻️ This comment has been updated with latest results.

Point the caller's TODO at the upstream PR that adds the Windows
PowerShell 5.1 (test_powershell) job to the shared workflow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tablackburn
Copy link
Copy Markdown
Contributor Author

Heads-up on the shared-workflow dependency: the Windows PowerShell 5.1 coverage lives upstream in psake/.github#7, which adds a test_powershell job (full build.ps1 -Task Test -Bootstrap under Desktop on windows-latest) to the shared ModuleCI.yml.

State of play:

tablackburn and others added 2 commits June 3, 2026 21:24
psake/.github#7 is merged; staying on @main intentionally for this
internal CI workflow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build.tests.ps1 Compile/Dot-sourced contexts started Wait-Job builds
with Start-Job -WorkingDirectory, a parameter introduced in PowerShell 6.0
that does not exist on Windows PowerShell 5.1. Under the shared workflow's
full 5.1 test run this threw ParameterBindingException, the TestModule never
built, and all 15 downstream assertions failed. The scriptblock already
Set-Location's to the test module source, so -WorkingDirectory was redundant;
removing it is behavior-identical on pwsh and 5.1-safe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tablackburn tablackburn merged commit c2c184e into main Jun 4, 2026
9 checks passed
@tablackburn tablackburn deleted the fix/windows-powershell-5.1-compat branch June 4, 2026 02:46
tablackburn added a commit that referenced this pull request Jun 4, 2026
## Summary

Removes the `### Added` entry from the 0.8.1 CHANGELOG section. It
documented an internal **CI job**, which isn't a user-facing change to
the module (public functions, build tasks, `$PSBPreference`, or
build/publish behavior).

## Why

- **Convention:** no prior release in this changelog (0.1.0 → 0.8.0)
documents CI, test, or internal-tooling changes — only the module's
user-facing surface. This entry was the sole exception.
- **It was also stale:** the bespoke `Import smoke (Windows PowerShell
5.1)` job it described was replaced in #126 by the shared
`psake/.github` ModuleCI workflow's full 5.1 **test** run, so the text
no longer matched what shipped.

The user-facing `### Fixed` entry (the 5.1 import regression) is
unchanged — that one genuinely affects consumers and stays.

Prep for cutting the `v0.8.1` release so the release notes reflect only
what users care about.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants