[release/v7.5] Fix macOS preview package identifier detection to use version string#26835
Conversation
…owerShell#26690) Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: TravisEz13 <[email protected]>
daxian-dbw
added
the
CL-BuildPackaging
There was a problem hiding this comment.
Pull request overview
Backports the macOS preview/stable package identifier selection fix to release/v7.5 by basing preview detection on the version string (and LTS flag), rather than the package name.
Changes:
- Propagates
LTSthrough the macOS packaging call chain and updatesNew-MacOSPackageto derive preview status + package identifier via a new helper. - Replaces direct/implicit preview detection in
New-MacOSLauncherwith the centralized helper output. - Adds Pester tests covering
Test-IsPreviewand the newGet-MacOSPackageIdentifierInfohelper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/packaging/packaging.psm1 | Adds Get-MacOSPackageIdentifierInfo, updates macOS packaging/launcher logic to use version-based preview detection, and passes LTS into macOS package creation. |
| test/packaging/packaging.tests.ps1 | Adds unit tests for preview detection and macOS package identifier selection logic. |
| if ($IsPreview.IsPresent) { | ||
| $PackageIdentifier = 'com.microsoft.powershell-preview' | ||
| } | ||
| else { | ||
| $PackageIdentifier = 'com.microsoft.powershell' | ||
| } |
There was a problem hiding this comment.
New-MacOsDistributionPackage now hardcodes com.microsoft.powershell(-preview) even though the same identifiers are also produced by Get-MacOSPackageIdentifierInfo below. This duplicates the source of truth for identifiers and risks future drift. Consider reusing the shared helper (or a shared constant) when $PackageIdentifier is not provided, so the identifier mapping is centralized in one place.
| if ($IsPreview.IsPresent) { | |
| $PackageIdentifier = 'com.microsoft.powershell-preview' | |
| } | |
| else { | |
| $PackageIdentifier = 'com.microsoft.powershell' | |
| } | |
| $macOSPackageInfo = Get-MacOSPackageIdentifierInfo -IsPreview:$IsPreview | |
| $PackageIdentifier = $macOSPackageInfo.PackageIdentifier |
| # Get package identifier info based on version and LTS flag | ||
| $packageInfo = Get-MacOSPackageIdentifierInfo -Version $Version -LTS:$LTS | ||
| $IsPreview = $packageInfo.IsPreview | ||
| $pkgIdentifier = $packageInfo.PackageIdentifier |
There was a problem hiding this comment.
The PR description says preview macOS packages will install to /usr/local/microsoft/powershell/7-preview/, but this change only fixes the package identifier determination. The install location is determined earlier by New-UnixPackage via $Suffix/$Destination (which depends on $IsPreview), and New-UnixPackage does not currently compute $IsPreview from the version string (so it stays $null/false and yields the stable 7/ suffix). To fully fix issue #26673, compute $IsPreview in New-UnixPackage using Test-IsPreview -Version $Version -IsLTS:$LTS and use that value consistently for $Suffix/$Destination (and for macOS launcher generation as needed).
| Context "Get-MacOSPackageIdentifierInfo function (New-MacOSPackage logic)" { | ||
| It "Should detect preview builds and return preview identifier" { | ||
| $result = Get-MacOSPackageIdentifierInfo -Version "7.6.0-preview.6" -LTS:$false | ||
|
|
||
| $result.IsPreview | Should -Be $true | ||
| $result.PackageIdentifier | Should -Be "com.microsoft.powershell-preview" | ||
| } | ||
|
|
||
| It "Should detect stable builds and return stable identifier" { | ||
| $result = Get-MacOSPackageIdentifierInfo -Version "7.6.0" -LTS:$false | ||
|
|
||
| $result.IsPreview | Should -Be $false | ||
| $result.PackageIdentifier | Should -Be "com.microsoft.powershell" | ||
| } | ||
|
|
||
| It "Should treat LTS builds as stable even with preview version string" { | ||
| $result = Get-MacOSPackageIdentifierInfo -Version "7.4.0-preview.1" -LTS:$true | ||
|
|
||
| $result.IsPreview | Should -Be $false | ||
| $result.PackageIdentifier | Should -Be "com.microsoft.powershell" | ||
| } | ||
|
|
||
| It "Should NOT use package name for preview detection (bug fix verification)" { | ||
| # This test verifies the fix for issue #26673 | ||
| # The bug was using ($Name -like '*-preview') which always returned false | ||
| # because preview builds use Name="powershell" not "powershell-preview" | ||
|
|
||
| $Version = "7.6.0-preview.6" | ||
| $Name = "powershell" # Preview builds use "powershell" not "powershell-preview" | ||
|
|
||
| # The INCORRECT logic (the bug): $Name -like '*-preview' | ||
| $incorrectCheck = $Name -like '*-preview' | ||
| $incorrectCheck | Should -Be $false -Because "Package name is 'powershell' not 'powershell-preview'" | ||
|
|
||
| # The CORRECT logic (the fix): uses version string | ||
| $result = Get-MacOSPackageIdentifierInfo -Version $Version -LTS:$false | ||
| $result.IsPreview | Should -Be $true -Because "Version string correctly identifies preview" | ||
| $result.PackageIdentifier | Should -Be "com.microsoft.powershell-preview" | ||
| } |
There was a problem hiding this comment.
The new tests validate Test-IsPreview and Get-MacOSPackageIdentifierInfo, but they don't cover the behavior that actually drives the reported customer impact: the macOS package install path suffix (7-preview vs 7) derived in New-UnixPackage. Adding a focused unit test that asserts the computed Destination/suffix for preview vs stable vs LTS would help prevent regressions (ideally by factoring the suffix/path computation into a small helper that can be tested without building a real pkg).
Backport of #26690 to release/v7.5
Triggered by @daxian-dbw on behalf of @app/copilot-swe-agent
Original CL Label: CL-BuildPackaging
/cc @PowerShell/powershell-maintainers
Impact
REQUIRED: Choose either Tooling Impact or Customer Impact (or both). At least one checkbox must be selected.
Tooling Impact
Customer Impact
macOS preview packages were incorrectly installing to the stable location (/usr/local/microsoft/powershell/7/) instead of the preview location (/usr/local/microsoft/powershell/7-preview/), preventing users from having both stable and preview versions installed simultaneously. Reported in issue #26673.
Regression
REQUIRED: Check exactly one box.
This is not a regression.
Testing
All 7 unit tests pass successfully. Tests directly verify Get-MacOSPackageIdentifierInfo function behavior and cover preview, stable, and LTS build scenarios. Test explicitly documents the bug fix for issue #26673.
Risk
REQUIRED: Check exactly one box.
Changes are minimal and surgical, centralized in a single testable function. All unit tests passing. Logic matches existing patterns in codebase. Only affects macOS package identifier determination.