From 484f5a45d5b93f818b5c780cc4681a0bf3ace344 Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Tue, 3 Dec 2019 13:28:23 -0800 Subject: [PATCH 1/6] Improve WinCompat code --- .../engine/Modules/ImportModuleCommand.cs | 33 ++++++++++++------- .../engine/Modules/ModuleCmdletBase.cs | 11 ++----- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index 226dd37a812..d397bb3757c 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -1870,18 +1870,7 @@ protected override void ProcessRecord() { if (this.UseWindowsPowerShell) { - var WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); - if (WindowsPowerShellCompatRemotingSession != null) - { - foreach(PSModuleInfo moduleProxy in ImportModule_RemotelyViaPsrpSession(importModuleOptions, this.Name, this.FullyQualifiedName, WindowsPowerShellCompatRemotingSession, true)) - { - moduleProxy.IsWindowsPowerShellCompatModule = true; - System.Threading.Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); - - string message = StringUtil.Format(Modules.WinCompatModuleWarning, moduleProxy.Name, WindowsPowerShellCompatRemotingSession.Name); - WriteWarning(message); - } - } + ImportModulesUsingWinCompat(this.Name, this.FullyQualifiedName, importModuleOptions); } } else @@ -1890,6 +1879,26 @@ protected override void ProcessRecord() } } + internal override IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) + { + IList moduleProxyList = null; + var WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); + if (WindowsPowerShellCompatRemotingSession != null) + { + moduleProxyList = ImportModule_RemotelyViaPsrpSession(importModuleOptions, moduleNames, moduleFullyQualifiedNames, WindowsPowerShellCompatRemotingSession, true); + foreach(PSModuleInfo moduleProxy in moduleProxyList) + { + moduleProxy.IsWindowsPowerShellCompatModule = true; + System.Threading.Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); + + string message = StringUtil.Format(Modules.WinCompatModuleWarning, moduleProxy.Name, WindowsPowerShellCompatRemotingSession.Name); + WriteWarning(message); + } + } + + return moduleProxyList; + } + private void SetModuleBaseForEngineModules(string moduleName, System.Management.Automation.ExecutionContext context) { // Set modulebase of engine modules to point to $pshome diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 68f613135b4..1944184b54b 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -2367,14 +2367,7 @@ internal PSModuleInfo LoadModuleManifest( { if (importingModule) { - var commandInfo = new CmdletInfo("Import-Module", typeof(ImportModuleCommand)); - using var ps = System.Management.Automation.PowerShell.Create(RunspaceMode.CurrentRunspace); - ps.AddCommand(commandInfo); - ps.AddParameter("PassThru", true); - ps.AddParameter("Name", moduleManifestPath); - ps.AddParameter("UseWindowsPowerShell", true); - - var moduleProxies = ps.Invoke(); + var moduleProxies = ImportModulesUsingWinCompat(new string [] {moduleManifestPath}, null, new ImportModuleOptions()); // we are loading by a single ManifestPath so expect max of 1 if(moduleProxies.Count > 0) @@ -4867,6 +4860,8 @@ internal void CleanupWindowsPowerShellCompatResources() } } + internal virtual IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) {throw new System.NotImplementedException();} + private void RemoveTypesAndFormatting( IList formatFilesToRemove, IList typeFilesToRemove) From 6725957076228b7090028afb7ad9234644df8edf Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Wed, 4 Dec 2019 14:38:36 -0800 Subject: [PATCH 2/6] Added tests --- .../CompatiblePSEditions.Module.Tests.ps1 | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 43069749824..bcf3cc159e8 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -30,7 +30,8 @@ function New-EditionCompatibleModule param( [Parameter(Mandatory = $true)][string]$ModuleName, [string]$DirPath, - [string[]]$CompatiblePSEditions) + [string[]]$CompatiblePSEditions, + [string]$ErrorGenerationCode='') $modulePath = Join-Path $DirPath $ModuleName @@ -41,7 +42,7 @@ function New-EditionCompatibleModule New-Item -Path $modulePath -ItemType Directory - New-Item -Path $psm1Path -Value "function Test-$ModuleName { `$true } function Test-${ModuleName}PSEdition { `$PSVersionTable.PSEdition }" -Force + New-Item -Path $psm1Path -Value "$ErrorGenerationCode function Test-$ModuleName { `$true } function Test-${ModuleName}PSEdition { `$PSVersionTable.PSEdition }" -Force if ($CompatiblePSEditions) { @@ -130,7 +131,6 @@ function New-TestNestedModule [scriptblock]::Create($newManifestCmd).Invoke() } - Describe "Get-Module with CompatiblePSEditions-checked paths" -Tag "CI" { BeforeAll { @@ -363,6 +363,43 @@ Describe "Import-Module from CompatiblePSEditions-checked paths" -Tag "CI" { } } +Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { + BeforeAll { + $ModuleName = "DesktopModule" + $basePath = Join-Path $TestDrive "WinCompatModules" + # create an incompatible module that generates an error on import + New-EditionCompatibleModule -ModuleName $ModuleName -CompatiblePSEditions "Desktop" -Dir $basePath -ErrorGenerationCode '1/0;' + } + + Context "Tests that ErrorAction/WarningAction have effect when Import-Module with WinCompat is used" { + BeforeAll { + Add-ModulePath $basePath + } + + AfterAll { + Restore-ModulePath + } + + It "Verify that Error/Warning are generated with default ErrorAction/WarningAction" -Skip:(-not $IsWindows) { + + $out = pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" + + # above should generate 1 error and 1 warning + $out.Count | Should -BeExactly 2 + $out[0] | Should -BeLike "*divide by zero*" + $out[1] | Should -BeLike "*loaded in Windows PowerShell*" + } + + It "Verify that Error/Warning are Not generated with ErrorAction/WarningAction = Ignore" -Skip:(-not $IsWindows) { + + $out = pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -ErrorAction Ignore -WarningAction Ignore" + + # above should not generate any errors or warnings + $out.Count | Should -BeExactly 0 + } + } +} + Describe "PSModulePath changes interacting with other PowerShell processes" -Tag "Feature" { $PSDefaultParameterValues = @{ 'It:Skip' = (-not $IsWindows) } From e9f4407e8eb588ad1e81fbfccef0232125201d3d Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Wed, 4 Dec 2019 14:48:02 -0800 Subject: [PATCH 3/6] Updated list initialization --- .../engine/Modules/ImportModuleCommand.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index d397bb3757c..70f8a1e27cc 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -1881,7 +1881,7 @@ protected override void ProcessRecord() internal override IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) { - IList moduleProxyList = null; + IList moduleProxyList = new List(); var WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); if (WindowsPowerShellCompatRemotingSession != null) { From 1d1074e0a8d4e88bc2ac6da09b5d67316f137875 Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Wed, 4 Dec 2019 15:20:38 -0800 Subject: [PATCH 4/6] Updated tests --- .../CompatiblePSEditions.Module.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index bcf3cc159e8..70a2721a7f7 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -382,7 +382,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { It "Verify that Error/Warning are generated with default ErrorAction/WarningAction" -Skip:(-not $IsWindows) { - $out = pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" + $out = & "$PSHOME\pwsh.exe" -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" # above should generate 1 error and 1 warning $out.Count | Should -BeExactly 2 @@ -392,7 +392,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { It "Verify that Error/Warning are Not generated with ErrorAction/WarningAction = Ignore" -Skip:(-not $IsWindows) { - $out = pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -ErrorAction Ignore -WarningAction Ignore" + $out = & "$PSHOME\pwsh.exe" -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -ErrorAction Ignore -WarningAction Ignore" # above should not generate any errors or warnings $out.Count | Should -BeExactly 0 From 41cb40cbba6702a13200380df46804ac61c62837 Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Wed, 4 Dec 2019 17:34:35 -0800 Subject: [PATCH 5/6] Updated tests 1 --- .../CompatiblePSEditions.Module.Tests.ps1 | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 70a2721a7f7..687472caf86 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -367,6 +367,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { BeforeAll { $ModuleName = "DesktopModule" $basePath = Join-Path $TestDrive "WinCompatModules" + remove-item $basePath -Recurse -ErrorAction SilentlyContinue # create an incompatible module that generates an error on import New-EditionCompatibleModule -ModuleName $ModuleName -CompatiblePSEditions "Desktop" -Dir $basePath -ErrorGenerationCode '1/0;' } @@ -380,22 +381,28 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { Restore-ModulePath } - It "Verify that Error/Warning are generated with default ErrorAction/WarningAction" -Skip:(-not $IsWindows) { - - $out = & "$PSHOME\pwsh.exe" -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" - - # above should generate 1 error and 1 warning - $out.Count | Should -BeExactly 2 - $out[0] | Should -BeLike "*divide by zero*" - $out[1] | Should -BeLike "*loaded in Windows PowerShell*" + It "Verify that Error is generated with default ErrorAction" -Skip:(-not $IsWindows) { + $LogPath = Join-Path $TestDrive (New-Guid).ToString() + pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" *> $LogPath + $LogPath | Should -FileContentMatch 'divide by zero' } - It "Verify that Error/Warning are Not generated with ErrorAction/WarningAction = Ignore" -Skip:(-not $IsWindows) { + It "Verify that Warning is generated with default WarningAction" -Skip:(-not $IsWindows) { + $LogPath = Join-Path $TestDrive (New-Guid).ToString() + pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName" *> $LogPath + $LogPath | Should -FileContentMatch 'loaded in Windows PowerShell' + } - $out = & "$PSHOME\pwsh.exe" -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -ErrorAction Ignore -WarningAction Ignore" + It "Verify that Error is Not generated with -ErrorAction Ignore" -Skip:(-not $IsWindows) { + $LogPath = Join-Path $TestDrive (New-Guid).ToString() + pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -ErrorAction Ignore" *> $LogPath + $LogPath | Should -Not -FileContentMatch 'divide by zero' + } - # above should not generate any errors or warnings - $out.Count | Should -BeExactly 0 + It "Verify that Warning is Not generated with -WarningAction Ignore" -Skip:(-not $IsWindows) { + $LogPath = Join-Path $TestDrive (New-Guid).ToString() + pwsh -NoProfile -NonInteractive -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Import-Module $ModuleName -WarningAction Ignore" *> $LogPath + $LogPath | Should -Not -FileContentMatch 'loaded in Windows PowerShell' } } } From 9e8799a1ca211bf8584fe73ea594da2d6274c520 Mon Sep 17 00:00:00 2001 From: Andrew Menagarishvili Date: Mon, 9 Dec 2019 10:32:28 -0800 Subject: [PATCH 6/6] Addressed feedback --- .../engine/Modules/ImportModuleCommand.cs | 23 ++++++++++--------- .../engine/Modules/ModuleCmdletBase.cs | 2 +- .../CompatiblePSEditions.Module.Tests.ps1 | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index 70f8a1e27cc..6f160e2490b 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -1881,19 +1881,20 @@ protected override void ProcessRecord() internal override IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) { - IList moduleProxyList = new List(); - var WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); - if (WindowsPowerShellCompatRemotingSession != null) + PSSession WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); + if (WindowsPowerShellCompatRemotingSession == null) { - moduleProxyList = ImportModule_RemotelyViaPsrpSession(importModuleOptions, moduleNames, moduleFullyQualifiedNames, WindowsPowerShellCompatRemotingSession, true); - foreach(PSModuleInfo moduleProxy in moduleProxyList) - { - moduleProxy.IsWindowsPowerShellCompatModule = true; - System.Threading.Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); + return new List(); + } - string message = StringUtil.Format(Modules.WinCompatModuleWarning, moduleProxy.Name, WindowsPowerShellCompatRemotingSession.Name); - WriteWarning(message); - } + var moduleProxyList = ImportModule_RemotelyViaPsrpSession(importModuleOptions, moduleNames, moduleFullyQualifiedNames, WindowsPowerShellCompatRemotingSession, usingWinCompat: true); + foreach(PSModuleInfo moduleProxy in moduleProxyList) + { + moduleProxy.IsWindowsPowerShellCompatModule = true; + System.Threading.Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); + + string message = StringUtil.Format(Modules.WinCompatModuleWarning, moduleProxy.Name, WindowsPowerShellCompatRemotingSession.Name); + WriteWarning(message); } return moduleProxyList; diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 1944184b54b..7fa4ec6a574 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -2367,7 +2367,7 @@ internal PSModuleInfo LoadModuleManifest( { if (importingModule) { - var moduleProxies = ImportModulesUsingWinCompat(new string [] {moduleManifestPath}, null, new ImportModuleOptions()); + IList moduleProxies = ImportModulesUsingWinCompat(new string [] {moduleManifestPath}, null, new ImportModuleOptions()); // we are loading by a single ManifestPath so expect max of 1 if(moduleProxies.Count > 0) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 687472caf86..58302834fbb 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -367,7 +367,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "CI" { BeforeAll { $ModuleName = "DesktopModule" $basePath = Join-Path $TestDrive "WinCompatModules" - remove-item $basePath -Recurse -ErrorAction SilentlyContinue + Remove-Item -Path $basePath -Recurse -ErrorAction SilentlyContinue # create an incompatible module that generates an error on import New-EditionCompatibleModule -ModuleName $ModuleName -CompatiblePSEditions "Desktop" -Dir $basePath -ErrorGenerationCode '1/0;' }