Skip to content

Commit dc352e3

Browse files
JustinGroteJustinGrote
authored
♻️ Refactor local module find process
Cleans up the local module search process to handle prerelease edge cases and short circuit faster once a valid module is found.
1 parent ec612fe commit dc352e3

File tree

2 files changed

+109
-93
lines changed

2 files changed

+109
-93
lines changed

‎ModuleFast.psm1

Lines changed: 100 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -192,9 +192,9 @@ function Install-ModuleFast {
192192
$planAlreadySatisfiedMessage = "`u{2705} $($ModulesToInstall.count) Module Specifications have all been satisfied by installed modules. If you would like to check for newer versions remotely, specify -Update"
193193
if ($WhatIfPreference) {
194194
Write-Host -fore DarkGreen $planAlreadySatisfiedMessage
195+
} else {
196+
Write-Verbose $planAlreadySatisfiedMessage
195197
}
196-
#TODO: Deduplicate this with the end into its own function
197-
Write-Verbose $planAlreadySatisfiedMessage
198198
return
199199
}
200200

@@ -335,11 +335,15 @@ function Get-ModuleFastPlan {
335335
Write-Verbose "${moduleSpec}: Evaluating Module Specification"
336336
[ModuleFastInfo]$localMatch = Find-LocalModule $moduleSpec -Update:$Update
337337
if ($localMatch) {
338-
Write-Debug "FOUND local module $($localMatch.Name) $($localMatch.ModuleVersion) at $($localMatch.Location) that satisfies $moduleSpec. Skipping..."
338+
Write-Debug "${localMatch}: 🎯 FOUND satisfing version $($localMatch.ModuleVersion) at $($localMatch.Location). Skipping remote search."
339339
#TODO: Capture this somewhere that we can use it to report in the deploy plan
340340
continue
341341
}
342342

343+
#If we get this far, we didn't find a manifest in this module path
344+
Write-Debug "${moduleSpec}: 🔍 No installed versions matched the spec. Will check remotely."
345+
346+
343347
$task = Get-ModuleInfoAsync @httpContext -Endpoint $Source -Name $moduleSpec.Name
344348
$taskSpecMap[$task] = $moduleSpec
345349
$currentTasks.Add($task)
@@ -532,7 +536,7 @@ function Get-ModuleFastPlan {
532536

533537
[ModuleFastSpec]::new($PSItem.id, $range)
534538
}
535-
Write-Debug "$currentModuleSpec`: has $($dependencies.count) additional dependencies."
539+
Write-Debug "$currentModuleSpec`: has $($dependencies.count) additional dependencies: $($dependencies -join ', ')"
536540

537541
# TODO: Where loop filter maybe
538542
[ModuleFastSpec[]]$dependenciesToResolve = $dependencies | Where-Object {
@@ -1032,6 +1036,9 @@ class ModuleFastSpec {
10321036
[string] ToString() {
10331037
$guid = $this._Guid -ne [Guid]::Empty ? " [$($this._Guid)]" : ''
10341038
$versionRange = $this._VersionRange.ToString() -eq '(, )' ? '' : " $($this._VersionRange)"
1039+
if ($this._VersionRange.MaxVersion -eq $this._VersionRange.MinVersion) {
1040+
$versionRange = "=$($this._VersionRange.MinVersion)"
1041+
}
10351042
return "$($this._Name)$guid$versionRange"
10361043
}
10371044
[int] GetHashCode() {
@@ -1246,20 +1253,21 @@ function Find-LocalModule {
12461253
)
12471254
$ErrorActionPreference = 'Stop'
12481255

1249-
# Search all psmodulepaths for the module
12501256
$modulePaths = $env:PSModulePath.Split([Path]::PathSeparator, [StringSplitOptions]::RemoveEmptyEntries)
1251-
if (-Not $modulePaths) {
1257+
if (-not $modulePaths) {
12521258
Write-Warning 'No PSModulePaths found in $env:PSModulePath. If you are doing isolated testing you can disregard this.'
12531259
return
12541260
}
12551261

1256-
#First property is the manifest path, second property is the actual version (may be different from the folder version as prerelease versions go in the same location)
1262+
#We want to minimize reading the manifest files, so we will do a fast file-based search first and then do a more detailed inspection on high confidence candidate(s). Any module in any folder path that satisfies the spec will be sufficient, we don't care about finding the "latest" version, so we will return the first module that satisfies the spec.
12571263

1264+
#We will store potential candidates in this list, with their evaluated "guessed" version based on the folder name and the path. The first items added to the list should be the highest likelihood candidates in Path priority order, so no sorting should be necessary.
12581265

12591266

1260-
[List[[Tuple[Version, string]]]]$candidateModules = foreach ($modulePath in $modulePaths) {
1267+
foreach ($modulePath in $modulePaths) {
1268+
[List[[Tuple[Version, string]]]]$candidatePaths = @()
12611269
if (-not [Directory]::Exists($modulePath)) {
1262-
Write-Debug "$($ModuleSpec.Name): PSModulePath $modulePath is configured but does not exist, skipping..."
1270+
Write-Debug "${ModuleSpec}: Skipping PSModulePath $modulePath - Configured but does not exist."
12631271
$modulePaths = $modulePaths | Where-Object { $_ -ne $modulePath }
12641272
continue
12651273
}
@@ -1268,12 +1276,10 @@ function Find-LocalModule {
12681276
$moduleBaseDir = [Directory]::GetDirectories($modulePath, $moduleSpec.Name, [EnumerationOptions]@{MatchCasing = 'CaseInsensitive' })
12691277
if ($moduleBaseDir.count -gt 1) { throw "$($moduleSpec.Name) folder is ambiguous, please delete one of these folders: $moduleBaseDir" }
12701278
if (-not $moduleBaseDir) {
1271-
Write-Debug "$($moduleSpec.Name): PSModulePath $modulePath does not have this module. Skipping..."
1279+
Write-Debug "${ModuleSpec}: Skipping PSModulePath $modulePath - Does not have this module."
12721280
continue
12731281
}
12741282

1275-
$manifestName = "$($ModuleSpec.Name).psd1"
1276-
12771283
#We can attempt a fast-search for modules if the ModuleSpec is for a specific version
12781284
$required = $ModuleSpec.Required
12791285
if ($required) {
@@ -1285,111 +1291,92 @@ function Find-LocalModule {
12851291
$manifestPath = Join-Path $moduleFolder $manifestName
12861292

12871293
if (Test-Path $ModuleFolder) {
1288-
#Linux/Mac support requires a case insensitive search on a user supplied argument.
1289-
$manifestPath = [Directory]::GetFiles($moduleFolder, "$($ModuleSpec.Name).psd1", [EnumerationOptions]@{MatchCasing = 'CaseInsensitive' })
1290-
1291-
if ($manifestPath.count -gt 1) { throw "$moduleFolder manifest is ambiguous, please delete one of these: $manifestPath" }
1292-
1293-
#Early return if we found a manifest, we don't need to do further checking
1294-
if ($manifestPath.count -eq 1) {
1295-
[Tuple]::Create([version]$moduleVersion, $manifestPath[0])
1296-
continue
1297-
}
1298-
}
1299-
}
1300-
1301-
#Check for versioned module folders next
1302-
foreach ($folder in [Directory]::GetDirectories($moduleBaseDir)) {
1303-
#Sanity check
1304-
$versionCandidate = Split-Path -Leaf $folder
1305-
[Version]$version = $null
1306-
if (-not [Version]::TryParse($versionCandidate, [ref]$version)) {
1307-
Write-Debug "Could not parse $folder in $moduleBaseDir as a valid version. This is either a bad version directory or this folder is a classic module."
1308-
continue
1294+
$candidatePaths.Add([Tuple]::Create($moduleVersion, $manifestPath))
13091295
}
1296+
} else {
1297+
#Check for versioned module folders next and sort by the folder versions to process them in descending order.
1298+
[Directory]::GetDirectories($moduleBaseDir)
1299+
| ForEach-Object {
1300+
$folder = $PSItem
1301+
$version = $null
1302+
$isVersion = [Version]::TryParse((Split-Path -Leaf $PSItem), [ref]$version)
1303+
if (-not $isVersion) {
1304+
Write-Debug "Could not parse $folder in $moduleBaseDir as a valid version. This is either a bad version directory or this folder is a classic module."
1305+
return
1306+
}
13101307

1311-
#Try to retrieve the manifest
1312-
#TODO: Create a "Assert-CaseSensitiveFileExists" function for this pattern used multiple times
1313-
$versionedManifestPath = [Directory]::GetFiles($folder, $manifestName, [EnumerationOptions]@{MatchCasing = 'CaseInsensitive' })
1314-
1315-
if ($versionedManifestPath.count -gt 1) { throw "$folder manifest is ambiguous, this happens on Linux if you have two manifests with different case sensitivity. Please delete one of these: $versionedManifestPath" }
1308+
#Fast filter items that are above the upper bound, we dont need to read these manifests
1309+
if ($ModuleSpec.Max -and $version -gt $ModuleSpec.Max.Version) {
1310+
Write-Debug "${ModuleSpec}: Skipping $folder - above the upper bound"
1311+
return
1312+
}
13161313

1317-
if (-not $versionedManifestPath) {
1318-
Write-Warning "Found a candidate versioned module folder $folder but no $manifestName manifest was found in the folder. This is an indication of a corrupt module and you should clean this folder up"
1319-
continue
1320-
}
1314+
#We can fast filter items that are below the lower bound, we dont need to read these manifests
1315+
if ($ModuleSpec.Min) {
1316+
#HACK: Nuget does not correctly convert major.minor.build versions.
1317+
[version]$originalBaseVersion = ($modulespec.Min.OriginalVersion -split '-')[0]
1318+
[Version]$minVersion = $originalBaseVersion.Revision -eq -1 ? $originalBaseVersion : $ModuleSpec.Min.Version
1319+
if ($minVersion -lt $ModuleSpec.Min.OriginalVersion) {
1320+
Write-Debug "${ModuleSpec}: Skipping $folder - below the lower bound"
1321+
return
1322+
}
1323+
}
13211324

1322-
if ($versionedManifestPath.count -eq 1) {
1323-
[Tuple]::Create([version]$version, $versionedManifestPath[0])
1325+
$candidatePaths.Add([Tuple]::Create($version, $PSItem))
13241326
}
13251327
}
13261328

13271329
#Check for a "classic" module if no versioned folders were found
1328-
if ($candidateModules.count -eq 0) {
1330+
if ($candidatePaths.count -eq 0) {
13291331
[string[]]$classicManifestPaths = [Directory]::GetFiles($moduleBaseDir, $manifestName, [EnumerationOptions]@{MatchCasing = 'CaseInsensitive' })
13301332
if ($classicManifestPaths.count -gt 1) { throw "$moduleBaseDir manifest is ambiguous, please delete one of these: $classicManifestPath" }
13311333
[string]$classicManifestPath = $classicManifestPaths[0]
13321334
if ($classicManifestPath) {
1333-
#TODO: Optimize this so that import-powershelldatafile is not called twice. This should be a rare occurance so it's not a big deal.
1335+
#NOTE: This does result in Import-PowerShellData getting called twice which isn't ideal for performance, but classic modules should be fairly rare and not worth optimizing.
13341336
[version]$classicVersion = (Import-PowerShellDataFile $classicManifestPath).ModuleVersion
1335-
[Tuple]::Create($classicVersion, $classicManifestPath)
1336-
continue
1337+
Write-Debug "${ModuleSpec}: Found classic module $classicVersion at $moduleBaseDir"
1338+
$candidatePaths.Add([Tuple]::Create($classicVersion, $moduleBaseDir))
13371339
}
13381340
}
13391341

1340-
#If we get this far, we didn't find a manifest in this module path
1341-
Write-Debug "$moduleSpec`: module folder exists at $moduleBaseDir but no modules found that match the version spec."
1342-
}
1343-
1344-
if ($candidateModules.count -eq 0) { return $null }
1345-
1346-
# We have to read the manifests to verify if the specified installed module is a prerelease module, which can affect whether it is selected by this function.
1347-
# TODO: Filter to likely candidates first
1348-
#NOTE: We use the sort rather than FindBestMatch because we want the highest compatible version, due to auto assembly redirect in PSCore
1349-
foreach ($moduleInfo in ($candidateModules | Sort-Object Item1 -Descending)) {
1350-
[NugetVersion]$version = [NugetVersion]::new($moduleInfo.Item1)
1351-
[string]$manifestPath = $moduleInfo.Item2
1352-
1353-
#The ModuleSpec.Max.Version check is to support an edge case where the module prerelease version is actually less than the prerelease constraint but we haven't read the manifest yet to determine that.
1354-
if (-not $ModuleSpec.SatisfiedBy($version) -and $ModuleSpec.Max.Version -ne $version) {
1355-
Write-Debug "$($ModuleSpec.Name): Found a module $($moduleInfo.Item2) that matches the name but does not satisfy the version spec $($ModuleSpec). Skipping..."
1342+
if ($candidatePaths.count -eq 0) {
1343+
Write-Debug "${ModuleSpec}: Skipping PSModulePath $modulePath - No installed versions matched the spec."
13561344
continue
13571345
}
13581346

1359-
$manifestData = Import-PowerShellDataFile -Path $manifestPath -ErrorAction stop
1347+
foreach ($candidateItem in $candidatePaths) {
1348+
[version]$version = $candidateItem.Item1
1349+
[string]$folder = $candidateItem.Item2
13601350

1361-
[Version]$manifestVersionData = $null
1362-
if (-not [Version]::TryParse($manifestData.ModuleVersion, [ref]$manifestVersionData)) {
1363-
Write-Warning "Found a manifest at $manifestPath but the version $($manifestData.ModuleVersion) in the manifest information is not a valid version. This is probably an invalid or corrupt manifest"
1364-
continue
1365-
}
1366-
1367-
[NuGetVersion]$manifestVersion = [NuGetVersion]::new(
1368-
$manifestVersionData,
1369-
$manifestData.PrivateData.PSData.Prerelease
1370-
)
1351+
#Read the module manifest to check for prerelease versions.
1352+
$manifestName = "$($ModuleSpec.Name).psd1"
1353+
$versionedManifestPath = [Directory]::GetFiles($folder, $manifestName, [EnumerationOptions]@{MatchCasing = 'CaseInsensitive' })
13711354

1372-
#Re-Test against the manifest loaded version to be sure
1373-
if (-not $ModuleSpec.SatisfiedBy($manifestVersion)) {
1374-
Write-Debug "$($ModuleSpec.Name): Found a module $($moduleInfo.Item2) that initially matched the name and version folder but after reading the manifest, the version label not satisfy the version spec $($ModuleSpec). This is an edge case and should only occur if you specified a prerelease upper bound that is less than the PreRelease label in the manifest. Skipping..."
1375-
continue
1376-
}
1355+
if ($versionedManifestPath.count -gt 1) { throw "$folder manifest is ambiguous, this happens on Linux if you have two manifests with different case sensitivity. Please delete one of these: $versionedManifestPath" }
13771356

1378-
#If Update is specified, we will be more strict and only report a matching module if it exactly matches the upper bound of the version spec (otherwise there may be a newer module available remotely)
1379-
if ($Update) {
1380-
if ($ModuleSpec.Max -ne $manifestVersion) {
1381-
Write-Debug "$($ModuleSpec.Name): Found a module $($moduleInfo.Item2) that matches the name and version folder but does not exactly match the upper bound of the version spec $($ModuleSpec). Skipping..."
1357+
if (-not $versionedManifestPath) {
1358+
Write-Warning "${ModuleSpec}: Found a candidate versioned module folder $folder but no $manifestName manifest was found in the folder. This is an indication of a corrupt module and you should clean this folder up"
13821359
continue
1383-
} else {
1384-
Write-Debug "$($ModuleSpec.Name): Found a module $($moduleInfo.Item2) that matches the name and version folder and exactly matches the upper bound of the version spec $($ModuleSpec) because -Update was specified, so it will not be evaluated for install"
13851360
}
1386-
}
13871361

1388-
#If we pass all sanity checks, we can return this module as meeting the criteria and skip checking all lower modules.
1389-
return [ModuleFastInfo]::new($ModuleSpec.Name, $manifestVersion, $manifestPath)
1362+
#Read the manifest so we can compare prerelease info. If this matches, we have a valid candidate and don't need to check anything further.
1363+
$manifestCandidate = ConvertFrom-ModuleManifest $versionedManifestPath[0]
1364+
$candidateVersion = $manifestCandidate.ModuleVersion
1365+
1366+
if ($ModuleSpec.SatisfiedBy($candidateVersion)) {
1367+
if ($Update -and ($ModuleSpec.Max -ne $candidateVersion)) {
1368+
Write-Debug "${ModuleSpec}: Skipping $candidateVersion - The -Update was specified and the version does not exactly meet the upper bound of the spec, meaning there is a possible newer version remotely."
1369+
continue
1370+
}
1371+
1372+
#TODO: Collect InstalledButSatisfied Modules into an array so they can later be referenced in the lockfile and/or plan, right now the lockfile only includes modules that changed.
1373+
return $manifestCandidate
1374+
}
1375+
}
13901376
}
13911377
}
13921378

1379+
13931380
function ConvertTo-AuthenticationHeaderValue ([PSCredential]$Credential) {
13941381
$basicCredential = [Convert]::ToBase64String(
13951382
[Encoding]::UTF8.GetBytes(
@@ -1550,6 +1537,30 @@ filter Resolve-FolderVersion([NuGetVersion]$version) {
15501537
[Version]::new($version.Major, $version.Minor, $version.Patch)
15511538
}
15521539

1540+
filter ConvertFrom-ModuleManifest {
1541+
[CmdletBinding()]
1542+
[OutputType([ModuleFastInfo])]
1543+
param(
1544+
[Parameter(Mandatory)][string]$ManifestPath
1545+
)
1546+
$ErrorActionPreference = 'Stop'
1547+
1548+
$ManifestName = Split-Path -Path $ManifestPath -LeafBase
1549+
$manifestData = Import-PowerShellDataFile -Path $ManifestPath -ErrorAction stop
1550+
1551+
[Version]$manifestVersionData = $null
1552+
if (-not [Version]::TryParse($manifestData.ModuleVersion, [ref]$manifestVersionData)) {
1553+
throw [InvalidDataException]"The manifest at $ManifestPath has an invalid ModuleVersion $($manifestData.ModuleVersion). This is probably an invalid or corrupt manifest"
1554+
}
1555+
1556+
[NuGetVersion]$manifestVersion = [NuGetVersion]::new(
1557+
$manifestVersionData,
1558+
$manifestData.PrivateData.PSData.Prerelease
1559+
)
1560+
1561+
return [ModuleFastInfo]::new($ManifestName, $manifestVersion, $ManifestPath)
1562+
}
1563+
15531564
#endregion Helpers
15541565

15551566
### ISSUES

‎ModuleFast.tests.ps1

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -459,30 +459,30 @@ Describe 'Install-ModuleFast' -Tag 'E2E' {
459459
Get-Module Az.Accounts -ListAvailable
460460
| Limit-ModulePath $installTempPath
461461
| Select-Object -ExpandProperty Version
462-
| Sort-Object Version -Descending
462+
| Sort-Object -Descending
463463
| Select-Object -First 1
464464
| Should -Be '2.10.2'
465465

466466
Install-ModuleFast @imfParams 'Az.Compute', 'Az.Accounts' #Should not update
467467
Get-Module Az.Accounts -ListAvailable
468468
| Limit-ModulePath $installTempPath
469469
| Select-Object -ExpandProperty Version
470-
| Sort-Object Version -Descending
470+
| Sort-Object -Descending
471471
| Select-Object -First 1
472472
| Should -Be '2.10.2'
473473

474474
Install-ModuleFast @imfParams 'Az.Compute' -Update #Should disregard local install and update latest Az.Accounts
475475
Get-Module Az.Accounts -ListAvailable
476476
| Limit-ModulePath $installTempPath
477477
| Select-Object -ExpandProperty Version
478-
| Sort-Object Version -Descending
478+
| Sort-Object -Descending
479479
| Select-Object -First 1
480480
| Should -BeGreaterThan ([version]'2.10.2')
481481

482482
Get-Module Az.Compute -ListAvailable
483483
| Limit-ModulePath $installTempPath
484484
| Select-Object -ExpandProperty Version
485-
| Sort-Object Version -Descending
485+
| Sort-Object -Descending
486486
| Select-Object -First 1
487487
| Should -BeGreaterThan ([version]'5.0.0')
488488
}
@@ -506,6 +506,11 @@ Describe 'Install-ModuleFast' -Tag 'E2E' {
506506
Install-ModuleFast @imfParams 'PrereleaseTest=0.0.1-bprerelease' -WarningVariable actual *>&1 | Out-Null
507507
$actual | Should -BeLike '*is newer than existing prerelease version*'
508508
}
509+
It 'Doesnt install prerelease if same-version Prerelease already installed' {
510+
Install-ModuleFast @imfParams 'PrereleaseTest=0.0.1-prerelease'
511+
$plan = Install-ModuleFast @imfParams 'PrereleaseTest=0.0.1-prerelease' -WhatIf
512+
$plan | Should -BeNullOrEmpty
513+
}
509514

510515
It 'Installs from <Name> SpecFile' {
511516
$SCRIPT:Mocks = Resolve-Path "$PSScriptRoot/Test/Mocks"

0 commit comments

Comments
 (0)