Skip to content

Enable discovering modules that have names same as a culture (e.g. Az) #8777

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
daxian-dbw merged 6 commits into from
Feb 20, 2019
Merged

Enable discovering modules that have names same as a culture (e.g. Az) #8777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
69cb998
[feature] enable discoverying modules that have names same as a culture
SteveL-MSFT Jan 29, 2019
907bf2d
[feature] fix CodeFactor issues
SteveL-MSFT Jan 29, 2019
3ed7768
address Rob's feedback
SteveL-MSFT Jan 29, 2019
50d501f
address Ilya's feedback
SteveL-MSFT Jan 29, 2019
60ea02c
change use of System.IO.Path to just Path
SteveL-MSFT Jan 29, 2019
4e1541c
[feature] address Dongbo's feedback
SteveL-MSFT Feb 20, 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
54 changes: 26 additions & 28 deletions src/System.Management.Automation/engine/Modules/ModuleUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,34 @@ internal static class ModuleUtils
// - Ignore files/directories when access is denied;
// - Search top directory only.
private static readonly System.IO.EnumerationOptions s_defaultEnumerationOptions =
new System.IO.EnumerationOptions() { AttributesToSkip = 0 };
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributes.Hidden };

// Default option for UNC path enumeration. Same as above plus a large buffer size.
// For network shares, a large buffer may result in better performance as more results can be batched over the wire.
// The buffer size 16K is recommended in the comment of the 'BufferSize' property:
// "A "large" buffer, for example, would be 16K. Typical is 4K."
private static readonly System.IO.EnumerationOptions s_uncPathEnumerationOptions =
new System.IO.EnumerationOptions() { AttributesToSkip = 0, BufferSize = 16384 };
new System.IO.EnumerationOptions() { AttributesToSkip = FileAttributes.Hidden, BufferSize = 16384 };

private static readonly string EnCulturePath = Path.DirectorySeparatorChar + "en";
private static readonly string EnUsCulturePath = Path.DirectorySeparatorChar + "en-us";

/// <summary>
/// Check if a directory could be a module folder.
/// Check if a directory is likely a localized resources folder.
/// </summary>
internal static bool IsPossibleModuleDirectory(string dir)
/// <param name="dir">Directory to check if it is a possible resource folder.</param>
/// <returns>True if the directory name matches a culture.</returns>
internal static bool IsPossibleResourceDirectory(string dir)
{
// We shouldn't be searching in hidden directories.
FileAttributes attributes = File.GetAttributes(dir);
if (0 != (attributes & FileAttributes.Hidden))
{
return false;
}

// Assume locale directories do not contain modules.
if (dir.EndsWith(@"\en", StringComparison.OrdinalIgnoreCase) ||
dir.EndsWith(@"\en-us", StringComparison.OrdinalIgnoreCase))
if (dir.EndsWith(EnCulturePath, StringComparison.OrdinalIgnoreCase) ||
dir.EndsWith(EnUsCulturePath, StringComparison.OrdinalIgnoreCase))
{
return false;
return true;
}

dir = Path.GetFileName(dir);

// Use some simple pattern matching to avoid the call into GetCultureInfo when we know it will fail (and throw).
if ((dir.Length == 2 && char.IsLetter(dir[0]) && char.IsLetter(dir[1]))
||
Expand All @@ -55,12 +54,12 @@ internal static bool IsPossibleModuleDirectory(string dir)
// This might not throw on invalid culture still
// 4096 is considered the unknown locale - so assume that could be a module
var cultureInfo = new CultureInfo(dir);
return cultureInfo.LCID == 4096;
return cultureInfo.LCID != 4096;
}
catch { }
}

return true;
return false;
}

/// <summary>
Expand All @@ -75,6 +74,7 @@ internal static IEnumerable<string> GetAllAvailableModuleFiles(string topDirecto
Queue<string> directoriesToCheck = new Queue<string>();
directoriesToCheck.Enqueue(topDirectoryToCheck);

bool firstSubDirs = true;
while (directoriesToCheck.Count > 0)
{
string directoryToCheck = directoriesToCheck.Dequeue();
Expand All @@ -83,7 +83,7 @@ internal static IEnumerable<string> GetAllAvailableModuleFiles(string topDirecto
string[] subDirectories = Directory.GetDirectories(directoryToCheck, "*", options);
foreach (string toAdd in subDirectories)
{
if (IsPossibleModuleDirectory(toAdd))
if (firstSubDirs || !IsPossibleResourceDirectory(toAdd))
{
directoriesToCheck.Enqueue(toAdd);
}
Expand All @@ -92,6 +92,7 @@ internal static IEnumerable<string> GetAllAvailableModuleFiles(string topDirecto
catch (IOException) { }
catch (UnauthorizedAccessException) { }

firstSubDirs = false;
string[] files = Directory.GetFiles(directoryToCheck, "*", options);
foreach (string moduleFile in files)
{
Expand Down Expand Up @@ -304,17 +305,14 @@ internal static IEnumerable<string> GetDefaultAvailableModuleFiles(string topDir
{
foreach (var subdirectory in subdirectories)
{
if (IsPossibleModuleDirectory(subdirectory))
if (subdirectory.EndsWith("Microsoft.PowerShell.Management", StringComparison.OrdinalIgnoreCase) ||
subdirectory.EndsWith("Microsoft.PowerShell.Utility", StringComparison.OrdinalIgnoreCase))
{
if (subdirectory.EndsWith("Microsoft.PowerShell.Management", StringComparison.OrdinalIgnoreCase) ||
subdirectory.EndsWith("Microsoft.PowerShell.Utility", StringComparison.OrdinalIgnoreCase))
{
directoriesToCheck.AddFirst(subdirectory);
}
else
{
directoriesToCheck.AddLast(subdirectory);
}
directoriesToCheck.AddFirst(subdirectory);
}
else
{
directoriesToCheck.AddLast(subdirectory);
}
}
}
Expand Down Expand Up @@ -487,7 +485,7 @@ internal static IEnumerable<CommandInfo> GetMatchingCommands(string pattern, Exe
}
}

string moduleShortName = System.IO.Path.GetFileNameWithoutExtension(modulePath);
string moduleShortName = Path.GetFileNameWithoutExtension(modulePath);

IDictionary<string, CommandTypes> exportedCommands = AnalysisCache.GetExportedCommands(modulePath, testOnly: false, context);

Expand Down
2 changes: 1 addition & 1 deletion src/System.Management.Automation/help/HelpFileHelpProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ internal Collection<string> GetExtendedSearchPaths()
// * and SearchOption.AllDirectories gets all the version directories.
string[] directories = Directory.GetDirectories(psModulePath, "*", SearchOption.AllDirectories);

var possibleModuleDirectories = directories.Where(directory => ModuleUtils.IsPossibleModuleDirectory(directory));
var possibleModuleDirectories = directories.Where(directory => !ModuleUtils.IsPossibleResourceDirectory(directory));

foreach (string directory in possibleModuleDirectories)
{
Expand Down
51 changes: 29 additions & 22 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 @@ -10,18 +10,21 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {
New-Item -ItemType Directory -Path "$testdrive\Modules\Foo\2.0" -Force > $null
New-Item -ItemType Directory -Path "$testdrive\Modules\Bar\Download" -Force > $null
New-Item -ItemType Directory -Path "$testdrive\Modules\Zoo\Too" -Force > $null
New-Item -ItemType Directory -Path "$testdrive\Modules\Az" -Force > $null

New-ModuleManifest -Path "$testdrive\Modules\Foo\1.1\Foo.psd1" -ModuleVersion 1.1
New-ModuleManifest -Path "$testdrive\Modules\Foo\2.0\Foo.psd1" -ModuleVersion 2.0
New-ModuleManifest -Path "$testdrive\Modules\Bar\Bar.psd1"
New-ModuleManifest -Path "$testdrive\Modules\Zoo\Zoo.psd1"
New-ModuleManifest -Path "$testdrive\Modules\Az\Az.psd1" -ModuleVersion 1.1

New-Item -ItemType File -Path "$testdrive\Modules\Foo\1.1\Foo.psm1" > $null
New-Item -ItemType File -Path "$testdrive\Modules\Foo\2.0\Foo.psm1" > $null
New-Item -ItemType File -Path "$testdrive\Modules\Bar\Bar.psm1" > $null
New-Item -ItemType File -Path "$testdrive\Modules\Bar\Download\Download.psm1" > $null
New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Zoo.psm1" > $null
New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Too\Zoo.psm1" > $null
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
Expand All @@ -40,11 +43,13 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {

It "Get-Module -ListAvailable" {
$modules = Get-Module -ListAvailable
$modules.Count | Should -Be 4
$modules.Count | Should -Be 5
$modules = $modules | Sort-Object -Property Name, Version
$modules.Name -join "," | Should -BeExactly "Bar,Foo,Foo,Zoo"
$modules[1].Version | Should -Be "1.1"
$modules[2].Version | Should -Be '2.0'
$modules.Name -join "," | Should -BeExactly "Az,Bar,Foo,Foo,Zoo"
$modules[0].Version | Should -Be "1.1"
$modules[1].Version | Should -Be "0.0.1"
$modules[2].Version | Should -Be '1.1'
$modules[3].Version | Should -Be '2.0'
}

It "Get-Module <Name> -ListAvailable" {
Expand All @@ -58,25 +63,27 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {

It "Get-Module -ListAvailable -All" {
$modules = Get-Module -ListAvailable -All
$modules.Count | Should -Be 10
$modules.Count | Should -Be 12
$modules = $modules | Sort-Object -Property Name, Path
$modules.Name -join "," | Should -BeExactly "Bar,Bar,Download,Foo,Foo,Foo,Foo,Zoo,Zoo,Zoo"
$modules.Name -join "," | Should -BeExactly "Az,Az,Bar,Bar,Download,Foo,Foo,Foo,Foo,Zoo,Zoo,Zoo"

$modules[0].ModuleType | Should -BeExactly "Manifest"
$modules[1].ModuleType | Should -BeExactly "Script"
$modules[2].ModuleType | Should -BeExactly "Script"
$modules[3].ModuleType | Should -BeExactly "Manifest"
$modules[3].Version | Should -Be "1.1"
$modules[2].ModuleType | Should -BeExactly "Manifest"
$modules[3].ModuleType | Should -BeExactly "Script"
$modules[4].ModuleType | Should -BeExactly "Script"
$modules[5].ModuleType | Should -BeExactly "Manifest"
$modules[5].Version | Should -Be "2.0"
$modules[5].Version | Should -Be "1.1"
$modules[6].ModuleType | Should -BeExactly "Script"
$modules[7].ModuleType | Should -BeExactly "Script"
$modules[7].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Too\Zoo.psm1").Path
$modules[8].ModuleType | Should -BeExactly "Manifest"
$modules[8].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Zoo.psd1").Path
$modules[7].ModuleType | Should -BeExactly "Manifest"
$modules[7].Version | Should -Be "2.0"
$modules[8].ModuleType | Should -BeExactly "Script"
$modules[9].ModuleType | Should -BeExactly "Script"
$modules[9].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Zoo.psm1").Path
$modules[9].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Too\Zoo.psm1").Path
$modules[10].ModuleType | Should -BeExactly "Manifest"
$modules[10].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Zoo.psd1").Path
$modules[11].ModuleType | Should -BeExactly "Script"
$modules[11].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Zoo.psm1").Path
}

It "Get-Module <Name> -ListAvailable -All" {
Expand All @@ -93,19 +100,19 @@ Describe "Get-Module -ListAvailable" -Tags "CI" {

It "Get-Module <Path> -ListAvailable" {
$modules = Get-Module "$testdrive\Modules\*" -ListAvailable
$modules.Count | Should -Be 4
$modules.Count | Should -Be 5
$modules = $modules | Sort-Object -Property Name, Version
$modules.Name -join "," | Should -BeExactly "Bar,Foo,Foo,Zoo"
$modules[1].Version | Should -Be "1.1"
$modules[2].Version | Should -Be '2.0'
$modules.Name -join "," | Should -BeExactly "Az,Bar,Foo,Foo,Zoo"
$modules[2].Version | Should -Be "1.1"
$modules[3].Version | Should -Be '2.0'
}

It "Get-Module <Path> -ListAvailable -All" {
$modules = Get-Module "$testdrive\Modules\*" -ListAvailable -All
$modules.Count | Should -Be 5
$modules.Count | Should -Be 6
$modules = $modules | Sort-Object -Property Name, Path
$modules.Name -join "," | Should -BeExactly "Bar,Foo,Foo,Zoo,Zoo"
$modules[3].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Too\Zoo.psm1").Path
$modules.Name -join "," | Should -BeExactly "Az,Bar,Foo,Foo,Zoo,Zoo"
$modules[4].Path | Should -BeExactly (Resolve-Path "$testdrive\Modules\Zoo\Too\Zoo.psm1").Path
}

It "Get-Module -FullyQualifiedName <FullyQualifiedName> -ListAvailable" {
Expand Down