From 247b0db50fe9dd76ff519f6d3ed88d64d939c15c Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 12 Oct 2017 12:42:27 -0700 Subject: [PATCH 1/6] enable import-module to be case insensitive --- .../engine/Modules/ModuleCmdletBase.cs | 40 ++++++++++++++ .../Import-Module.Tests.ps1 | 53 +++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 1269b141a3a..2f94356cc11 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -323,6 +323,46 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer } } + if (!found) + { + // try a case-insensitive search for the module folder + foreach (string path in modulePath) + { + foreach (string folder in Directory.EnumerateDirectories(path)) + { + if (String.Compare(Path.GetFileName(folder), name, StringComparison.OrdinalIgnoreCase) == 0) + { + module = LoadUsingMultiVersionModuleBase(folder, manifestProcessingFlags, options, out found); + if (!found) + { + // try a case-insensitive search for the module file + foreach (string file in Directory.EnumerateFiles(folder)) + { + string fileName = Path.GetFileNameWithoutExtension(file); + if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) + { + string qualifiedPath = Path.Combine(folder, fileName); + module = LoadUsingExtensions(parentModule, fileName, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); + } + if (found) + { + break; + } + } + } + } + if (found) + { + break; + } + } + if (found) + { + break; + } + } + } + if (found) { // Cache the module's exported commands after importing it, or if the -Refresh flag is used on diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 index 911c74c8fae..3e26bdd7c82 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 @@ -124,3 +124,56 @@ namespace ModuleCmdlets } } +Describe "Import-Module should be case insensitive on Unix" -Tags 'CI' { + BeforeAll { + $defaultParamValues = $PSDefaultParameterValues.Clone() + $defaultPSModuleAutoloadingPreference = $PSModuleAutoloadingPreference + $originalPSModulePath = $env:PSModulePath.Clone() + $modulesPath = "$TestDrive\Modules" + $env:PSModulePath += [System.IO.Path]::PathSeparator + $modulesPath + $PSModuleAutoloadingPreference = "none" + $PSDefaultParameterValues["it:skip"] = !$IsLinux + } + AfterAll { + $global:PSDefaultParameterValues = $defaultParamValues + $global:PSModuleAutoloadingPreference = $defaultPSModuleAutoloadingPreference + $env:PSModulePath = $originalPSModulePath + } + + It "Import-Module can import a module using different casing using '' and manifest:" -TestCases @( + @{modulePath="TESTMODULE/1.1"; manifest=$true}, + @{modulePath="TESTMODULE/1.1"; manifest=$false}, + @{modulePath="TESTMODULE" ; manifest=$true}, + @{modulePath="TESTMODULE" ; manifest=$false} + ) { + param ($modulePath, $manifest) + New-Item -ItemType Directory -Path "$modulesPath/$modulePath" -Force + if ($manifest) { + New-ModuleManifest -Path "$modulesPath/$modulePath/TESTMODULE.psd1" -RootModule "TESTMODULE.psm1" -ModuleVersion 1.1 + } + Set-Content -Path "$modulesPath/$modulePath/TESTMODULE.psm1" -Value "function mytest { 'hello' }" + Import-Module testMODULE + $m = Get-Module TESTmodule + $m | Should BeOfType "System.Management.Automation.PSModuleInfo" + $m.Name | Should BeExactly "TESTMODULE" + mytest | Should BeExactly "hello" + Remove-Module TestModule + Get-Module tESTmODULE | Should BeNullOrEmpty + } + + It "Import-Module will import exact casing if available" { + New-Item -ItemType Directory -Path "$modulesPath\Test" -Force + Set-Content -Path "$modulesPath\Test\Test.psm1" -Value "function casetest { 'first' }" + New-Item -ItemType Directory -Path "$modulesPath\tEST" -Force + Set-Content -Path "$modulesPath\tEST\tEST.psm1" -Value "function casetest { 'second' }" + Import-Module Test + casetest | Should BeExactly 'first' + Remove-Module test + Import-Module test + casetest | Should BeExactly 'second' + Remove-Module test + Import-Module tEST + casetest | Should BeExactly 'second' + Remove-Module test + } +} From 607727270be6e11d2d7471801648f14e23c47982 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 13 Oct 2017 10:59:40 -0700 Subject: [PATCH 2/6] address PR feedback simplify search loops with assumption that module file casing matches module folder name which is appropriate per PSGet --- .../engine/Modules/ModuleCmdletBase.cs | 22 ++++++------------- .../Import-Module.Tests.ps1 | 8 +++---- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 2f94356cc11..2c338316d19 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -323,32 +323,23 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer } } +#if UNIX // case-sensitive filesystems are predominately used only on Unix if (!found) { // try a case-insensitive search for the module folder + // based on PSGet, we can assume the module file casing matches the folder foreach (string path in modulePath) { foreach (string folder in Directory.EnumerateDirectories(path)) { - if (String.Compare(Path.GetFileName(folder), name, StringComparison.OrdinalIgnoreCase) == 0) + string fileName = Path.GetFileName(folder); + if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) { module = LoadUsingMultiVersionModuleBase(folder, manifestProcessingFlags, options, out found); if (!found) { - // try a case-insensitive search for the module file - foreach (string file in Directory.EnumerateFiles(folder)) - { - string fileName = Path.GetFileNameWithoutExtension(file); - if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) - { - string qualifiedPath = Path.Combine(folder, fileName); - module = LoadUsingExtensions(parentModule, fileName, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); - } - if (found) - { - break; - } - } + string qualifiedPath = Path.Combine(folder, fileName); + module = LoadUsingExtensions(parentModule, fileName, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); } } if (found) @@ -362,6 +353,7 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer } } } +#endif if (found) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 index 3e26bdd7c82..6ae4afe7ce3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 @@ -124,7 +124,7 @@ namespace ModuleCmdlets } } -Describe "Import-Module should be case insensitive on Unix" -Tags 'CI' { +Describe "Import-Module should be case insensitive on Linux" -Tags 'CI' { BeforeAll { $defaultParamValues = $PSDefaultParameterValues.Clone() $defaultPSModuleAutoloadingPreference = $PSModuleAutoloadingPreference @@ -147,7 +147,7 @@ Describe "Import-Module should be case insensitive on Unix" -Tags 'CI' { @{modulePath="TESTMODULE" ; manifest=$false} ) { param ($modulePath, $manifest) - New-Item -ItemType Directory -Path "$modulesPath/$modulePath" -Force + New-Item -ItemType Directory -Path "$modulesPath/$modulePath" -Force > $null if ($manifest) { New-ModuleManifest -Path "$modulesPath/$modulePath/TESTMODULE.psd1" -RootModule "TESTMODULE.psm1" -ModuleVersion 1.1 } @@ -162,9 +162,9 @@ Describe "Import-Module should be case insensitive on Unix" -Tags 'CI' { } It "Import-Module will import exact casing if available" { - New-Item -ItemType Directory -Path "$modulesPath\Test" -Force + New-Item -ItemType Directory -Path "$modulesPath\Test" -Force > $null Set-Content -Path "$modulesPath\Test\Test.psm1" -Value "function casetest { 'first' }" - New-Item -ItemType Directory -Path "$modulesPath\tEST" -Force + New-Item -ItemType Directory -Path "$modulesPath\tEST" -Force > $null Set-Content -Path "$modulesPath\tEST\tEST.psm1" -Value "function casetest { 'second' }" Import-Module Test casetest | Should BeExactly 'first' From fac1155c60d63532c20fb1dfde1d413f7180a857 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sun, 15 Oct 2017 13:07:16 -0700 Subject: [PATCH 3/6] [feature] combine loops --- .../engine/Modules/ModuleCmdletBase.cs | 78 +++++++------------ 1 file changed, 30 insertions(+), 48 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 2c338316d19..a3420eab648 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -293,67 +293,49 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer // Now search using the module path... foreach (string path in modulePath) { - // a name takes the form of .../moduleDir/moduleName. - // if only one name has been specified, then the moduleDir - // and moduleName are identical and repeated... - // Also append th ename if the path currently points at a directory - string qualifiedPath = Path.Combine(path, fileBaseName); - - // Load the latest valid version if it is a multi-version module directory - module = LoadUsingMultiVersionModuleBase(qualifiedPath, manifestProcessingFlags, options, out found); - - if (!found) - { - if (name.IndexOfAny(Utils.Separators.Directory) == -1) - { - qualifiedPath = Path.Combine(qualifiedPath, fileBaseName); - } - else if (Directory.Exists(qualifiedPath)) - { - // if it points to a directory, add the basename back onto the path... - qualifiedPath = Path.Combine(qualifiedPath, Path.GetFileName(fileBaseName)); - } - - module = LoadUsingExtensions(parentModule, name, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); - } - - if (found) - { - break; - } - } - -#if UNIX // case-sensitive filesystems are predominately used only on Unix - if (!found) - { - // try a case-insensitive search for the module folder - // based on PSGet, we can assume the module file casing matches the folder - foreach (string path in modulePath) +#if UNIX + foreach (string folder in Directory.EnumerateDirectories(path)) { - foreach (string folder in Directory.EnumerateDirectories(path)) + string fileName = Path.GetFileName(folder); + if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) { - string fileName = Path.GetFileName(folder); - if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) + string qualifiedPath = folder; + name = fileName; +#else + string qualifiedPath = Path.Combine(path, fileBaseName); +#endif + module = LoadUsingMultiVersionModuleBase(qualifiedPath, manifestProcessingFlags, options, out found); + if (!found) { - module = LoadUsingMultiVersionModuleBase(folder, manifestProcessingFlags, options, out found); - if (!found) + if (name.IndexOfAny(Utils.Separators.Directory) == -1) { - string qualifiedPath = Path.Combine(folder, fileName); - module = LoadUsingExtensions(parentModule, fileName, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); +#if UNIX + qualifiedPath = Path.Combine(folder, fileName); +#else + qualifiedPath = Path.Combine(qualifiedPath, fileBaseName); +#endif } + else if (Directory.Exists(qualifiedPath)) + { + // if it points to a directory, add the basename back onto the path... + qualifiedPath = Path.Combine(qualifiedPath, Path.GetFileName(fileBaseName)); + } + + module = LoadUsingExtensions(parentModule, name, qualifiedPath, extension, null, this.BasePrefix, ss, options, manifestProcessingFlags, out found); } - if (found) - { - break; - } +#if UNIX } if (found) { break; } } - } + if (found) + { + break; + } #endif + } if (found) { From b4f7c0172043ab5122f51e39135fbecacbde3b12 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 16 Oct 2017 09:15:13 -0700 Subject: [PATCH 4/6] [feature] removed test that is no longer valid --- .../engine/Modules/ModuleCmdletBase.cs | 4 ---- .../Import-Module.Tests.ps1 | 16 ---------------- 2 files changed, 20 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index a3420eab648..891715a821e 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -309,11 +309,7 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer { if (name.IndexOfAny(Utils.Separators.Directory) == -1) { -#if UNIX - qualifiedPath = Path.Combine(folder, fileName); -#else qualifiedPath = Path.Combine(qualifiedPath, fileBaseName); -#endif } else if (Directory.Exists(qualifiedPath)) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 index 6ae4afe7ce3..7f5ea0f42a7 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 @@ -160,20 +160,4 @@ Describe "Import-Module should be case insensitive on Linux" -Tags 'CI' { Remove-Module TestModule Get-Module tESTmODULE | Should BeNullOrEmpty } - - It "Import-Module will import exact casing if available" { - New-Item -ItemType Directory -Path "$modulesPath\Test" -Force > $null - Set-Content -Path "$modulesPath\Test\Test.psm1" -Value "function casetest { 'first' }" - New-Item -ItemType Directory -Path "$modulesPath\tEST" -Force > $null - Set-Content -Path "$modulesPath\tEST\tEST.psm1" -Value "function casetest { 'second' }" - Import-Module Test - casetest | Should BeExactly 'first' - Remove-Module test - Import-Module test - casetest | Should BeExactly 'second' - Remove-Module test - Import-Module tEST - casetest | Should BeExactly 'second' - Remove-Module test - } } From fd7dcfb231d8e5e2bc947b0309cc2929aace7546 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 16 Oct 2017 22:36:26 -0700 Subject: [PATCH 5/6] [feature] enable test for Windows, removed invalid test where a module with version folder requires a manifest --- .../engine/Modules/ModuleCmdletBase.cs | 10 ++++------ .../Import-Module.Tests.ps1 | 15 ++++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 891715a821e..9f879773a4a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -296,14 +296,12 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer #if UNIX foreach (string folder in Directory.EnumerateDirectories(path)) { - string fileName = Path.GetFileName(folder); - if (String.Compare(fileName, name, StringComparison.OrdinalIgnoreCase) == 0) + string moduleName = Path.GetFileName(folder); + if (String.Compare(moduleName, fileBaseName, StringComparison.OrdinalIgnoreCase) == 0) { - string qualifiedPath = folder; - name = fileName; -#else - string qualifiedPath = Path.Combine(path, fileBaseName); + fileBaseName = moduleName; #endif + string qualifiedPath = Path.Combine(path, fileBaseName); module = LoadUsingMultiVersionModuleBase(qualifiedPath, manifestProcessingFlags, options, out found); if (!found) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 index 7f5ea0f42a7..48b216fc882 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 @@ -103,7 +103,7 @@ using System.Management.Automation; // Windows PowerShell namespace. namespace ModuleCmdlets { - [Cmdlet(VerbsDiagnostic.Test,"BinaryModuleCmdlet1")] + [Cmdlet(VerbsDiagnostic.Test,"BinaryModuleCmdlet1")] public class TestBinaryModuleCmdlet1Command : Cmdlet { protected override void BeginProcessing() @@ -124,25 +124,26 @@ namespace ModuleCmdlets } } -Describe "Import-Module should be case insensitive on Linux" -Tags 'CI' { +Describe "Import-Module should be case insensitive" -Tags 'CI' { BeforeAll { - $defaultParamValues = $PSDefaultParameterValues.Clone() $defaultPSModuleAutoloadingPreference = $PSModuleAutoloadingPreference $originalPSModulePath = $env:PSModulePath.Clone() $modulesPath = "$TestDrive\Modules" $env:PSModulePath += [System.IO.Path]::PathSeparator + $modulesPath $PSModuleAutoloadingPreference = "none" - $PSDefaultParameterValues["it:skip"] = !$IsLinux } + AfterAll { - $global:PSDefaultParameterValues = $defaultParamValues $global:PSModuleAutoloadingPreference = $defaultPSModuleAutoloadingPreference $env:PSModulePath = $originalPSModulePath } + AfterEach { + Remove-Item -Recurse -Path $modulesPath -Force -ErrorAction SilentlyContinue + } + It "Import-Module can import a module using different casing using '' and manifest:" -TestCases @( @{modulePath="TESTMODULE/1.1"; manifest=$true}, - @{modulePath="TESTMODULE/1.1"; manifest=$false}, @{modulePath="TESTMODULE" ; manifest=$true}, @{modulePath="TESTMODULE" ; manifest=$false} ) { @@ -155,7 +156,7 @@ Describe "Import-Module should be case insensitive on Linux" -Tags 'CI' { Import-Module testMODULE $m = Get-Module TESTmodule $m | Should BeOfType "System.Management.Automation.PSModuleInfo" - $m.Name | Should BeExactly "TESTMODULE" + $m.Name | Should Be "TESTMODULE" mytest | Should BeExactly "hello" Remove-Module TestModule Get-Module tESTmODULE | Should BeNullOrEmpty From 169e597f56a548211add0cac5e04bdabdd0d07bb Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 19 Oct 2017 13:48:59 -0700 Subject: [PATCH 6/6] [feature] changed to using Utils.NativeDirectoryExists() for perf reasons --- .../engine/Modules/ModuleCmdletBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 9f879773a4a..bdea23d50d6 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -309,7 +309,7 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer { qualifiedPath = Path.Combine(qualifiedPath, fileBaseName); } - else if (Directory.Exists(qualifiedPath)) + else if (Utils.NativeDirectoryExists(qualifiedPath)) { // if it points to a directory, add the basename back onto the path... qualifiedPath = Path.Combine(qualifiedPath, Path.GetFileName(fileBaseName));