Skip to content

Fix Get-Module -FullyQualifiedName option to work with paths #9101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adityapatwardhan merged 7 commits into from
Mar 29, 2019
Merged

Fix Get-Module -FullyQualifiedName option to work with paths #9101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9925dbf
Get-Module with FullyQualifiedName does not skip extra checks anymore
throwaway774 Mar 10, 2019
42f6fc7
clean up
throwaway774 Mar 10, 2019
44b4a91
fixing coding style
throwaway774 Mar 10, 2019
c90216d
Fixing coding style
throwaway774 Mar 10, 2019
3870d87
Fixing coding style
throwaway774 Mar 10, 2019
c1b7237
Remove Linq expression
throwaway774 Mar 29, 2019
6b9ed6f
Fixing coding style
throwaway774 Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions src/System.Management.Automation/engine/Modules/GetModuleCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,11 @@ protected override void ProcessRecord()
var moduleSpecTable = new Dictionary<string, ModuleSpecification>(StringComparer.OrdinalIgnoreCase);
if (FullyQualifiedName != null)
{
// TODO:
// FullyQualifiedName.Name could be a path, in which case it will not match module.Name.
// This is potentially a bug (since version checks are ignored).
// We should normalize FullyQualifiedName.Name here with ModuleIntrinsics.NormalizeModuleName().
for (int modSpecIndex = 0; modSpecIndex < FullyQualifiedName.Length; modSpecIndex++)
{
FullyQualifiedName[modSpecIndex] = FullyQualifiedName[modSpecIndex].WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path);
}

moduleSpecTable = FullyQualifiedName.ToDictionary(moduleSpecification => moduleSpecification.Name, StringComparer.OrdinalIgnoreCase);
strNames.AddRange(FullyQualifiedName.Select(spec => spec.Name));
}
Expand Down Expand Up @@ -545,22 +546,36 @@ private static IEnumerable<PSModuleInfo> FilterModulesForSpecificationMatch(

foreach (PSModuleInfo module in modules)
{
// TODO:
// moduleSpecification.Name may be a path and will not match module.Name when they refer to the same module.
// This actually causes the module to be returned always, so other specification checks are skipped erroneously.
// Instead we need to be able to look up or match modules by path as well (e.g. a new comparer for PSModuleInfo).
IEnumerable<ModuleSpecification> candidateModuleSpecs = GetCandidateModuleSpecs(moduleSpecificationTable, module);

// No table entry means we return the module
if (!moduleSpecificationTable.TryGetValue(module.Name, out ModuleSpecification moduleSpecification))
// Modules with table entries only get returned if they match them
// We skip the name check since modules have already been prefiltered base on the moduleSpec path/name
foreach (ModuleSpecification moduleSpec in candidateModuleSpecs)
{
yield return module;
continue;
if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpec, skipNameCheck: true))
{
yield return module;
}
}
}
}

// Modules with table entries only get returned if they match them
if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification))
/// <summary>
/// Take a dictionary of module specifications and return those that potentially match the module
/// passed in as a parameter (checks on names and paths).
/// </summary>
/// <param name="moduleSpecTable">The module specifications to filter candidates from.</param>
/// <param name="module">The module to find candidates for from the module specification table.</param>
/// <returns>The module specifications matching the module based on name, path and subpath.</returns>
private static IEnumerable<ModuleSpecification> GetCandidateModuleSpecs(
IDictionary<string, ModuleSpecification> moduleSpecTable,
PSModuleInfo module)
{
foreach (ModuleSpecification moduleSpec in moduleSpecTable.Values)
{
if (moduleSpec.Name == module.Name || moduleSpec.Name == module.Path || module.Path.Contains(moduleSpec.Name))
Copy link
Member

@TravisEz13 TravisEz13 Aug 8, 2019

Choose a reason for hiding this comment

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

FYI, this make existing cases of getting a module by name, using FullyQualifiedName case sensitive. I'll submit a PR to fix this.

{
yield return module;
yield return moduleSpec;
}
}
}
Expand Down
17 changes: 13 additions & 4 deletions src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,14 @@ internal List<PSModuleInfo> GetModules(ModuleSpecification[] fullyQualifiedName,
/// </summary>
/// <param name="moduleInfo">The module info object to check.</param>
/// <param name="moduleSpec">The module specification to match the module info object against.</param>
/// <param name="skipNameCheck">True if we should skip the name check on the module specification.</param>
/// <returns>True if the module info object meets all the constraints on the module specification, false otherwise.</returns>
internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec)
internal static bool IsModuleMatchingModuleSpec(
PSModuleInfo moduleInfo,
ModuleSpecification moduleSpec,
bool skipNameCheck = false)
{
return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec);
return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec, skipNameCheck);
}

/// <summary>
Expand All @@ -435,8 +439,13 @@ internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleS
/// <param name="matchFailureReason">The constraint that caused the match failure, if any.</param>
/// <param name="moduleInfo">The module info object to check.</param>
/// <param name="moduleSpec">The module specification to match the module info object against.</param>
/// <param name="skipNameCheck">True if we should skip the name check on the module specification.</param>
/// <returns>True if the module info object meets all the constraints on the module specification, false otherwise.</returns>
internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec)
internal static bool IsModuleMatchingModuleSpec(
out ModuleMatchFailure matchFailureReason,
PSModuleInfo moduleInfo,
ModuleSpecification moduleSpec,
bool skipNameCheck = false)
{
if (moduleSpec == null)
{
Expand All @@ -447,7 +456,7 @@ internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFail
return IsModuleMatchingConstraints(
out matchFailureReason,
moduleInfo,
moduleSpec.Name,
skipNameCheck ? null : moduleSpec.Name,
moduleSpec.Guid,
moduleSpec.RequiredVersion,
moduleSpec.Version,
Expand Down
101 changes: 99 additions & 2 deletions test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {
New-Item -ItemType File -Path "$testdrive\Modules\Az\Az.psm1" > $null

$fullyQualifiedPathTestCases = @(
# The current behaviour in PowerShell is that version gets ignored when using Get-Module -FullyQualifiedName with a path
@{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 2 }
@{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 1 }
@{ ModPath = "$TestDrive\Modules/Foo\1.1/Foo.psd1"; Name = 'Foo'; Version = '1.1'; Count = 1 }
@{ ModPath = "$TestDrive\Modules/Bar.psd1"; Name = 'Bar'; Version = '0.0'; Count = 1 }
@{ ModPath = "$TestDrive\Modules\Zoo\Too\Zoo.psm1"; Name = 'Zoo'; Version = '0.0'; Count = 1 }
Expand Down Expand Up @@ -225,3 +224,101 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {
}
}
}

Describe 'Get-Module -ListAvailable with path' -Tags "CI" {
BeforeAll {
$moduleName = 'Banana'
$modulePath = Join-Path $TestDrive $moduleName
$v1 = '1.2.3'
$v2 = '4.8.3'
$v1DirPath = Join-Path $modulePath $v1
$v2DirPath = Join-Path $modulePath $v2
$manifestV1Path = Join-Path $v1DirPath "$moduleName.psd1"
$manifestV2Path = Join-Path $v2DirPath "$moduleName.psd1"

New-Item -ItemType Directory $modulePath
New-Item -ItemType Directory -Path $v1DirPath
New-Item -ItemType Directory -Path $v2DirPath
New-ModuleManifest -Path $manifestV1Path -ModuleVersion $v1
New-ModuleManifest -Path $manifestV2Path -ModuleVersion $v2
}

It "Gets all versions by path" {
$modules = Get-Module -ListAvailable $modulePath | Sort-Object -Property Version

$modules | Should -HaveCount 2
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $manifestV1Path
$modules[0].Version | Should -Be $v1
$modules[1].Name | Should -BeExactly $moduleName
$modules[1].Path | Should -BeExactly $manifestV2Path
$modules[1].Version | Should -Be $v2
}

It "Gets all versions by FullyQualifiedName with path with lower version" {
$modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; ModuleVersion = '0.0' } | Sort-Object -Property Version

$modules | Should -HaveCount 2
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $manifestV1Path
$modules[0].Version | Should -Be $v1
$modules[1].Name | Should -BeExactly $moduleName
$modules[1].Path | Should -BeExactly $manifestV2Path
$modules[1].Version | Should -Be $v2
}

It "Gets high version by FullyQualifiedName with path with high version" {
$modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; ModuleVersion = '2.0' } | Sort-Object -Property Version

$modules | Should -HaveCount 1
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $manifestV2Path
$modules[0].Version | Should -Be $v2
}

It "Gets low version by FullyQualifiedName with path with low maximum version" {
$modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; MaximumVersion = '2.0' } | Sort-Object -Property Version

$modules | Should -HaveCount 1
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $manifestV1Path
$modules[0].Version | Should -Be $v1
}

It "Gets low version by FullyQualifiedName with path with low maximum version and version" {
$modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; MaximumVersion = '2.0'; ModuleVersion = '1.0' } | Sort-Object -Property Version

$modules | Should -HaveCount 1
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $manifestV1Path
$modules[0].Version | Should -Be $v1
}

It "Gets correct version by FullyQualifiedName with path with required version" -TestCases @(
@{ Version = $v1 }
@{ Version = $v2 }
) {
param([version]$Version)

switch ($Version)
{
$v1
{
$expectedPath = $manifestV1Path
break
}

$v2
{
$expectedPath = $manifestV2Path
}
}

$modules = Get-Module -ListAvailable -FullyQualifiedName @{ ModuleName = $modulePath; RequiredVersion = $Version }

$modules | Should -HaveCount 1
$modules[0].Name | Should -BeExactly $moduleName
$modules[0].Path | Should -BeExactly $expectedPath
$modules[0].Version | Should -Be $Version
}
}