diff --git a/PSDepend/PSDependScripts/Nuget.ps1 b/PSDepend/PSDependScripts/Nuget.ps1 index a8c22b6..64f3bc4 100644 --- a/PSDepend/PSDependScripts/Nuget.ps1 +++ b/PSDepend/PSDependScripts/Nuget.ps1 @@ -149,14 +149,17 @@ if(Test-Path $PackagePath) $GetGalleryVersion = { (Find-NugetPackage -Name $DependencyName -PackageSourceUrl $Source -Credential $Credential -IsLatest).Version } # Version string, and equal to current - if( $Version -and $Version -ne 'latest' -and $Version -eq $ExistingVersion) + if($Version -and $Version -ne 'latest') { - Write-Verbose "You have the requested version [$Version] of [$DependencyName]" - if($PSDependAction -contains 'Test') + if(Test-VersionEquality $Version $ExistingVersion) { - return $True + Write-Verbose "You have the requested version [$Version] of [$DependencyName]" + if($PSDependAction -contains 'Test') + { + return $True + } + return $null } - return $null } # latest, and we have latest diff --git a/PSDepend/PSDependScripts/PSGalleryModule.ps1 b/PSDepend/PSDependScripts/PSGalleryModule.ps1 index 65d6689..cf8e2a7 100644 --- a/PSDepend/PSDependScripts/PSGalleryModule.ps1 +++ b/PSDepend/PSDependScripts/PSGalleryModule.ps1 @@ -225,6 +225,17 @@ $Existing = Get-Module -ListAvailable -Name $ModuleName -ErrorAction SilentlyCon if($Existing) { Write-Verbose "Found existing module [$Name]" + + if($Version -and $Version -ne 'latest') { + $matchedInstall = $Existing | Where-Object { Test-VersionEquality $Version $_.Version.ToString() } | Select-Object -First 1 + if ($matchedInstall) { + Write-Verbose "You have the requested version [$Version] of [$Name]" + Import-PSDependModule -Name $ModuleName -Action $PSDependAction -Version $matchedInstall.Version + if($PSDependAction -contains 'Test') { return $true } + return $null + } + } + # Thanks to Brandon Padgett! $ExistingVersion = $Existing | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum $FindModuleParams = @{Name = $Name } @@ -238,18 +249,6 @@ if($Existing) { $FindModuleParams.Add('AllowPrerelease', $AllowPrerelease) } - # Version string, and equal to current - if($Version -and $Version -ne 'latest' -and $Version -eq $ExistingVersion) { - Write-Verbose "You have the requested version [$Version] of [$Name]" - # Conditional import - Import-PSDependModule -Name $ModuleName -Action $PSDependAction -Version $ExistingVersion - - if($PSDependAction -contains 'Test') { - return $true - } - return $null - } - $GalleryVersion = Find-Module @FindModuleParams | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum [System.Version]$parsedExistingVersion = $null [System.Version]$parsedGalleryVersion = $null diff --git a/PSDepend/PSDependScripts/PSGalleryNuget.ps1 b/PSDepend/PSDependScripts/PSGalleryNuget.ps1 index c67e7fa..f028cbb 100644 --- a/PSDepend/PSDependScripts/PSGalleryNuget.ps1 +++ b/PSDepend/PSDependScripts/PSGalleryNuget.ps1 @@ -133,16 +133,19 @@ if(Test-Path $ModulePath) $GetGalleryVersion = { (Find-NugetPackage -Name $Name -PackageSourceUrl $Source -Credential $Credential -IsLatest).Version } # Version string, and equal to current - if( $Version -and $Version -ne 'latest' -and $Version -eq $ExistingVersion) + if($Version -and $Version -ne 'latest') { - Write-Verbose "You have the requested version [$Version] of [$Name]" - # Conditional import - Import-PSDependModule -Name $ModulePath -Action $PSDependAction -Version $ExistingVersion - if($PSDependAction -contains 'Test') + if(Test-VersionEquality $Version $ExistingVersion) { - return $True + Write-Verbose "You have the requested version [$Version] of [$Name]" + # Conditional import + Import-PSDependModule -Name $ModulePath -Action $PSDependAction -Version $ExistingVersion + if($PSDependAction -contains 'Test') + { + return $True + } + return $null } - return $null } # latest, and we have latest diff --git a/PSDepend/PSDependScripts/Package.ps1 b/PSDepend/PSDependScripts/Package.ps1 index a6223ba..4fd9856 100644 --- a/PSDepend/PSDependScripts/Package.ps1 +++ b/PSDepend/PSDependScripts/Package.ps1 @@ -169,21 +169,25 @@ $Existing = Get-Package @GetParam if($Existing) { Write-Verbose "Found existing package [$Name]" - # Thanks to Brandon Padgett! - $ExistingVersion = $Existing | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum - $GetSourceVersion = { Find-Package -Name $Name -Source $Source | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum } - - # Version string, and equal to current - if( $Version -and $Version -ne 'latest' -and $Version -eq $ExistingVersion) + + if($Version -and $Version -ne 'latest') { - Write-Verbose "You have the requested version [$Version] of [$Name]" - if($PSDependAction -contains 'Test') + $matchedInstall = $Existing | Where-Object { Test-VersionEquality $Version $_.Version } | Select-Object -First 1 + if ($matchedInstall) { - return $True + Write-Verbose "You have the requested version [$Version] of [$Name]" + if($PSDependAction -contains 'Test') + { + return $True + } + return $null } - return $null } - + + # Thanks to Brandon Padgett! + $ExistingVersion = $Existing | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum + $GetSourceVersion = { Find-Package -Name $Name -Source $Source | Measure-Object -Property Version -Maximum | Select-Object -ExpandProperty Maximum } + $SourceVersion = (& $GetSourceVersion) [System.Version]$parsedExistingVersion = $null [System.Version]$parsedSourceVersion = $null diff --git a/PSDepend/Private/Test-VersionEquality.ps1 b/PSDepend/Private/Test-VersionEquality.ps1 new file mode 100644 index 0000000..d45327b --- /dev/null +++ b/PSDepend/Private/Test-VersionEquality.ps1 @@ -0,0 +1,87 @@ +function Test-VersionEquality { + <# + .SYNOPSIS + Compare two versions by casting and comparing individual components. + + .DESCRIPTION + Compare two version strings by attempting to parse them as System.Version + and System.Management.Automation.SemanticVersion, and comparing their + components. If parsing fails, fall back to string comparison. + + .PARAMETER ReferenceVersion + The reference version string to compare against. + + .PARAMETER DifferenceVersion + The version string to compare with the reference version. + + .EXAMPLE + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.3' + + Returns true for identical three-part versions. + + .EXAMPLE + Test-VersionEquality -ReferenceVersion '1.2.0' -DifferenceVersion '1.2' + + Returns true when both omit build (treated as 0). + + .EXAMPLE + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.4' + + Returns false when build differs. + #> + [CmdletBinding()] + [OutputType([bool])] + param( + [string]$ReferenceVersion, + [string]$DifferenceVersion + ) + + # First, check if either version string is null or empty. If so, they can't + # be equal. + if ( + [string]::IsNullOrEmpty($ReferenceVersion) -or + [string]::IsNullOrEmpty($DifferenceVersion) + ) { + return $false + } + + # Parsing requires existing references to exist, so we create them. + [System.Version]$parsedRef = $null + [System.Version]$parsedDiff = $null + + # Check if we can parse both versions as System.Version. If we can, we + # compare them using individual components. + # Because System.Version treats missing components as -1, we use Math.Max to + # treat them as 0 for comparison purposes (e.g. 1.2 is treated as 1.2.0.0). + if ([System.Version]::TryParse($ReferenceVersion, [ref]$parsedRef) -and + [System.Version]::TryParse($DifferenceVersion, [ref]$parsedDiff) + ) { + return ( + $parsedRef.Major -eq $parsedDiff.Major -and + $parsedRef.Minor -eq $parsedDiff.Minor -and + [Math]::Max($parsedRef.Build, 0) -eq [Math]::Max($parsedDiff.Build, 0) -and + [Math]::Max($parsedRef.Revision, 0) -eq [Math]::Max($parsedDiff.Revision, 0) + ) + } + + # If they can't be parsed as System.Version, we attempt to parse them as + # SemanticVersion, which can handle prerelease and build metadata. + [System.Management.Automation.SemanticVersion]$parsedRefSemVer = $null + [System.Management.Automation.SemanticVersion]$parsedDiffSemVer = $null + + if ( + [System.Management.Automation.SemanticVersion]::TryParse( + $ReferenceVersion, [ref]$parsedRefSemVer + ) -and + [System.Management.Automation.SemanticVersion]::TryParse( + $DifferenceVersion, [ref]$parsedDiffSemVer + ) + ) { + return $parsedRefSemVer -eq $parsedDiffSemVer + } + + # TODO: Investigate if we want to add additional parsing logic here for + # other version formats (e.g. date or commit based versions) + + return $ReferenceVersion -eq $DifferenceVersion +} diff --git a/Tests/Test-VersionEquality.Tests.ps1 b/Tests/Test-VersionEquality.Tests.ps1 new file mode 100644 index 0000000..97596c1 --- /dev/null +++ b/Tests/Test-VersionEquality.Tests.ps1 @@ -0,0 +1,213 @@ +#requires -Module @{ ModuleName = 'Pester'; ModuleVersion = '5.0.0' } + +BeforeAll { + if (-not $env:BHProjectPath) { + & "$PSScriptRoot\..\build.ps1" -Task 'Build' + } + Remove-Module $env:BHProjectName -ErrorAction SilentlyContinue + Import-Module (Join-Path $env:BHProjectPath $env:BHProjectName) -Force +} + +Describe 'Test-VersionEquality' { + + Context 'Null and empty inputs' { + + It 'Returns false when ReferenceVersion is empty' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '' -DifferenceVersion '1.0.0' + } | Should -BeFalse + } + + It 'Returns false when DifferenceVersion is empty' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.0.0' -DifferenceVersion '' + } | Should -BeFalse + } + + It 'Returns false when both are empty' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '' -DifferenceVersion '' + } | Should -BeFalse + } + } + + Context '[System.Version] two-part (Major.Minor)' { + + It 'Returns true for identical two-part versions' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2' -DifferenceVersion '1.2' + } | Should -BeTrue + } + + It 'Returns false when minor differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2' -DifferenceVersion '1.3' + } | Should -BeFalse + } + + It 'Returns false when major differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.0' -DifferenceVersion '2.0' + } | Should -BeFalse + } + } + + Context '[System.Version] three-part (Major.Minor.Build)' { + + It 'Returns true for identical three-part versions' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.3' + } | Should -BeTrue + } + + It 'Returns true when both omit build (treated as 0)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.0' -DifferenceVersion '1.2' + } | Should -BeTrue + } + + It 'Returns false when build differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.4' + } | Should -BeFalse + } + } + + Context '[System.Version] four-part (Major.Minor.Build.Revision)' { + + It 'Returns true for identical four-part versions' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3.4' -DifferenceVersion '1.2.3.4' + } | Should -BeTrue + } + + It 'Returns true when revision is absent on one side (treated as 0)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3.0' -DifferenceVersion '1.2.3' + } | Should -BeTrue + } + + It 'Returns false when revision differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3.4' -DifferenceVersion '1.2.3.5' + } | Should -BeFalse + } + } + + Context 'SemanticVersion (Major.Minor.Patch[-pre[+build]])' { + + It 'Returns true for identical semver strings' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.3' + } | Should -BeTrue + } + + It 'Returns true for identical semver with pre-release label' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '2.0.0-beta.1' -DifferenceVersion '2.0.0-beta.1' + } | Should -BeTrue + } + + It 'Returns false when patch differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.2.3' -DifferenceVersion '1.2.4' + } | Should -BeFalse + } + + It 'Returns false when pre-release label differs' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '2.0.0-alpha' -DifferenceVersion '2.0.0-beta' + } | Should -BeFalse + } + + It 'Returns false when one has a pre-release label and the other does not' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '2.0.0-beta' -DifferenceVersion '2.0.0' + } | Should -BeFalse + } + } + + Context 'Tricky versions with zero components or zero-prefixed pre-release' { + + # 0.0.0.5 — [System.Version] Build=0 Revision=5 + # Risk: Math.Max(Build,0) normalises absent build (-1) to 0, + # so 0.0.0.5 must NOT equal 0.0.0 even though both have Build→0 after normalisation. + It 'Returns true for 0.0.0.5 equal to itself' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '0.0.0.5' -DifferenceVersion '0.0.0.5' + } | Should -BeTrue + } + + It 'Returns false for 0.0.0.5 vs 0.0.0 (revision 5 vs absent)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '0.0.0.5' -DifferenceVersion '0.0.0' + } | Should -BeFalse + } + + It 'Returns false for 0.0.0.5 vs 0.0.0.4' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '0.0.0.5' -DifferenceVersion '0.0.0.4' + } | Should -BeFalse + } + + # 0.1.0.2 — [System.Version] Build=0 Revision=2; same zero-build concern + It 'Returns true for 0.1.0.2 equal to itself' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '0.1.0.2' -DifferenceVersion '0.1.0.2' + } | Should -BeTrue + } + + It 'Returns false for 0.1.0.2 vs 0.1.0 (revision 2 vs absent)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '0.1.0.2' -DifferenceVersion '0.1.0' + } | Should -BeFalse + } + + # 1.0.2-0alpha.5 — SemanticVersion path; pre-release starts with digit 0 + # (valid because the identifier is alphanumeric, not purely numeric) + It 'Returns true for 1.0.2-0alpha.5 equal to itself' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.0.2-0alpha.5' -DifferenceVersion '1.0.2-0alpha.5' + } | Should -BeTrue + } + + It 'Returns false for 1.0.2-0alpha.5 vs 1.0.2 (pre-release vs none)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.0.2-0alpha.5' -DifferenceVersion '1.0.2' + } | Should -BeFalse + } + + It 'Returns false for 1.0.2-0alpha.5 vs 1.0.2-0alpha.6 (differing pre-release suffix)' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '1.0.2-0alpha.5' -DifferenceVersion '1.0.2-0alpha.6' + } | Should -BeFalse + } + } + + Context 'CalVer and arbitrary string fallback' { + + It 'Returns true for identical CalVer strings' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '2024.01.15' -DifferenceVersion '2024.01.15' + } | Should -BeTrue + } + + It 'Returns false for different CalVer strings' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion '2024.01.15' -DifferenceVersion '2024.02.01' + } | Should -BeFalse + } + + It 'Returns true for identical arbitrary version strings' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion 'latest' -DifferenceVersion 'latest' + } | Should -BeTrue + } + + It 'Returns false for different arbitrary strings' { + InModuleScope PSDepend { + Test-VersionEquality -ReferenceVersion 'latest' -DifferenceVersion 'stable' + } | Should -BeFalse + } + } +} diff --git a/docs/PSDependScripts-ReviewerChecklist.md b/docs/PSDependScripts-ReviewerChecklist.md index c9187b9..223facb 100644 --- a/docs/PSDependScripts-ReviewerChecklist.md +++ b/docs/PSDependScripts-ReviewerChecklist.md @@ -64,7 +64,23 @@ Resolve the inputs once into locals before the main body runs: - `AddToPath` consistently prepends to `$env:PATH` and/or `$env:PSModulePath` via `Add-ToItemCollection`. -### 7. Logging discipline +### 7. Local-first resolution for explicit versions + +When a Dependency declares an explicit version (not `'latest'` or `''`): + +1. Check **all** locally installed versions against the requested version before + making any remote call. +2. Use typed comparison (`[System.Version]` → `[SemanticVersion]` → string + equality) rather than string comparison — `"1.0"` and `"1.0.0.0"` are the + same version. +3. Only fall through to the remote registry if no local match is found. +4. The `'latest'` path legitimately requires a remote call to confirm currency; + the explicit-version path does not. + +This keeps air-gapped and CI environments fast, avoids unnecessary network cost, +and is consistent with the principle that PSDepend should be idempotent. + +### 8. Logging discipline - `Write-Verbose` for normal progress on each decision branch. - `Write-Error` (not `throw`) for recoverable failures. @@ -119,10 +135,14 @@ Use this as a PR review checklist when adding or modifying a script under ### Version comparison (for installers) -- [ ] Both `[SemanticVersion]::TryParse` and `[Version]::TryParse` are - attempted before comparing (see `PSGalleryModule.ps1` lines 262–273). +- [ ] When an explicit version is requested, **all locally installed versions + are checked before any remote call is made** — the check must not be + limited to the maximum installed version (see `PSGalleryModule.ps1`). +- [ ] Version comparison uses `[System.Version]::TryParse`, falling back to + `[System.Management.Automation.SemanticVersion]::TryParse`, then string + equality — never raw string equality alone (handles `"1.0"` vs `"1.0.0.0"`). - [ ] `'latest'` vs explicit-version paths each produce a defensible - early-return. + early-return; only the `'latest'` path is allowed to hit a remote registry. ### Security / hygiene