From f39b1d6f91db6762b0139dda9cbabd1ff59c128f Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 19 Sep 2017 19:18:43 -0700 Subject: [PATCH 1/7] handle exception for non-Windows where reading the screen buffer is not supported since screen buffer reading isn't supported, default to using redirected output --- .../engine/NativeCommandProcessor.cs | 41 +++++++++++-------- .../Start-Transcript.Tests.ps1 | 26 +++++++++--- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 45fb2f591a7..b9cea2f4aaa 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -185,6 +185,17 @@ internal NativeCommandProcessor(ApplicationInfo applicationInfo, ExecutionContex //Create input writer for providing input to the process. _inputWriter = new ProcessInputWriter(Command); + + try + { + Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( + new Host.Rectangle(_startPosition, _startPosition)); + _scrapeHostOutput = true; + } + catch (NotImplementedException) + { + _scrapeHostOutput = false; + } } /// @@ -398,12 +409,16 @@ private void InitNativeProcess() // redirecting anything. This is a bit tricky as we always run redirected so // we have to see if the redirection is actually being done at the topmost level or not. - //Calculate if input and output are redirected. - bool redirectOutput; - bool redirectError; - bool redirectInput; + // Calculate if input and output are redirected. + bool redirectOutput = true; + bool redirectError = true; + bool redirectInput = true; - CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); + // If we are transcribing and can't scrape, then continue to redirect so we have the output + if (!this.Command.Context.EngineHostInterface.UI.IsTranscribing || _scrapeHostOutput) + { + CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); + } // Find out if it's the only command in the pipeline. bool soloCommand = this.Command.MyInvocation.PipelineLength == 1; @@ -417,7 +432,6 @@ private void InitNativeProcess() } _startPosition = new Host.Coordinates(); - _scrapeHostOutput = false; Exception exceptionToRethrow = null; try @@ -432,19 +446,10 @@ private void InitNativeProcess() // Also, store the Raw UI coordinates so that we can scrape the screen after // if we are transcribing. - try - { - if (this.Command.Context.EngineHostInterface.UI.IsTranscribing) - { - _scrapeHostOutput = true; - _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; - _startPosition.X = 0; - } - } - catch (Host.HostException) + if (this.Command.Context.EngineHostInterface.UI.IsTranscribing && _scrapeHostOutput) { - // The host doesn't support scraping via its RawUI interface - _scrapeHostOutput = false; + _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; + _startPosition.X = 0; } } diff --git a/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 b/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 index cd5907f87d2..805b5db5473 100644 --- a/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 @@ -107,7 +107,7 @@ Describe "Start-Transcript, Stop-Transcript tests" -tags "CI" { ValidateTranscription -scriptToExecute $script -outputFilePath $null -expectedError $expectedError } It "Transcription should remain active if other runspace in the host get closed" { - try{ + try { $ps = [powershell]::Create() $ps.addscript("Start-Transcript -path $transcriptFilePath").Invoke() $ps.addscript('$rs = [system.management.automation.runspaces.runspacefactory]::CreateRunspace()').Invoke() @@ -115,12 +115,11 @@ Describe "Start-Transcript, Stop-Transcript tests" -tags "CI" { $ps.addscript('$rs.Dispose()').Invoke() $ps.addscript('Write-Host "After Dispose"').Invoke() $ps.addscript("Stop-Transcript").Invoke() - } finally { - if ($null -ne $ps) { - $ps.Dispose() - } + } finally { + if ($null -ne $ps) { + $ps.Dispose() } - + } Test-Path $transcriptFilePath | Should be $true $transcriptFilePath | Should contain "After Dispose" @@ -136,4 +135,19 @@ Describe "Start-Transcript, Stop-Transcript tests" -tags "CI" { $transcriptFilePath | Should contain "PowerShell transcript end" } + It "Transcription should record native command output" { + try { + $ps = [powershell]::Create() + $ps.addscript("Start-Transcript -path $transcriptFilePath").Invoke() + $ps.addscript("hostname").Invoke() + $ps.addscript("Stop-Transcript").Invoke() + } finally { + if ($null -ne $ps) { + $ps.Dispose() + } + } + Test-Path $transcriptFilePath | Should be $true + $machineName = [System.Environment]::MachineName + $transcriptFilePath | Should contain $machineName + } } \ No newline at end of file From 5ad677f35deb2b8a242cbee12f8a0f3c69c9f7d7 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Wed, 20 Sep 2017 12:40:58 -0700 Subject: [PATCH 2/7] address PR feedback --- .../Start-Transcript.Tests.ps1 | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 b/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 index 805b5db5473..2d3c28aba22 100644 --- a/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.Powershell.Host/Start-Transcript.Tests.ps1 @@ -136,18 +136,14 @@ Describe "Start-Transcript, Stop-Transcript tests" -tags "CI" { } It "Transcription should record native command output" { - try { - $ps = [powershell]::Create() - $ps.addscript("Start-Transcript -path $transcriptFilePath").Invoke() - $ps.addscript("hostname").Invoke() - $ps.addscript("Stop-Transcript").Invoke() - } finally { - if ($null -ne $ps) { - $ps.Dispose() - } - } + $script = { + Start-Transcript -Path $transcriptFilePath + hostname + Stop-Transcript } + & $script Test-Path $transcriptFilePath | Should be $true + $machineName = [System.Environment]::MachineName $transcriptFilePath | Should contain $machineName } -} \ No newline at end of file +} From 7373ea547da46e047d5fd979f3210ca3cc3170d2 Mon Sep 17 00:00:00 2001 From: "Steve Lee [MSFT]" Date: Fri, 22 Sep 2017 17:04:33 -0700 Subject: [PATCH 3/7] address Dongbo's PR feedback --- .../engine/NativeCommandProcessor.cs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index b9cea2f4aaa..8369b13dc72 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -186,16 +186,7 @@ internal NativeCommandProcessor(ApplicationInfo applicationInfo, ExecutionContex //Create input writer for providing input to the process. _inputWriter = new ProcessInputWriter(Command); - try - { - Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( - new Host.Rectangle(_startPosition, _startPosition)); - _scrapeHostOutput = true; - } - catch (NotImplementedException) - { - _scrapeHostOutput = false; - } + _isTranscribing = this.Command.Context.EngineHostInterface.UI.IsTranscribing; } /// @@ -384,8 +375,8 @@ internal override void ProcessRecord() /// private BlockingCollection _nativeProcessOutputQueue; - private bool _scrapeHostOutput; - + private static bool? s_supportScreenScrape = null; + private bool _isTranscribing; private Host.Coordinates _startPosition; /// @@ -412,10 +403,24 @@ private void InitNativeProcess() // Calculate if input and output are redirected. bool redirectOutput = true; bool redirectError = true; - bool redirectInput = true; + bool redirectInput = this.Command.MyInvocation.ExpectingInput; + + if (null == s_supportScreenScrape) + { + try + { + Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( + new Host.Rectangle(_startPosition, _startPosition)); + s_supportScreenScrape = true; + } + catch (NotImplementedException) + { + s_supportScreenScrape = false; + } + } // If we are transcribing and can't scrape, then continue to redirect so we have the output - if (!this.Command.Context.EngineHostInterface.UI.IsTranscribing || _scrapeHostOutput) + if (!_isTranscribing || (true == s_supportScreenScrape)) { CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); } @@ -446,7 +451,7 @@ private void InitNativeProcess() // Also, store the Raw UI coordinates so that we can scrape the screen after // if we are transcribing. - if (this.Command.Context.EngineHostInterface.UI.IsTranscribing && _scrapeHostOutput) + if (_isTranscribing && (true == s_supportScreenScrape)) { _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; _startPosition.X = 0; @@ -703,8 +708,7 @@ internal override void Complete() _nativeProcess.WaitForExit(); // Capture screen output if we are transcribing - if (this.Command.Context.EngineHostInterface.UI.IsTranscribing && - _scrapeHostOutput) + if (_isTranscribing && (true == s_supportScreenScrape)) { Host.Coordinates endPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; endPosition.X = this.Command.Context.EngineHostInterface.UI.RawUI.BufferSize.Width - 1; From c68cb3951a429af408bb5f76094f421d06016639 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 25 Sep 2017 15:09:11 -0700 Subject: [PATCH 4/7] [feature] moved setting redirection due to transcription to within CalculateIORedirection() function ensure screen scrapping only happens when running standalone --- .../engine/NativeCommandProcessor.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 8369b13dc72..fbb6b3ac1f9 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -405,6 +405,8 @@ private void InitNativeProcess() bool redirectError = true; bool redirectInput = this.Command.MyInvocation.ExpectingInput; + _startPosition = new Host.Coordinates(); + if (null == s_supportScreenScrape) { try @@ -419,11 +421,7 @@ private void InitNativeProcess() } } - // If we are transcribing and can't scrape, then continue to redirect so we have the output - if (!_isTranscribing || (true == s_supportScreenScrape)) - { - CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); - } + CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); // Find out if it's the only command in the pipeline. bool soloCommand = this.Command.MyInvocation.PipelineLength == 1; @@ -436,8 +434,6 @@ private void InitNativeProcess() throw new PipelineStoppedException(); } - _startPosition = new Host.Coordinates(); - Exception exceptionToRethrow = null; try { @@ -707,8 +703,8 @@ internal override void Complete() ConsumeAvailableNativeProcessOutput(blocking: true); _nativeProcess.WaitForExit(); - // Capture screen output if we are transcribing - if (_isTranscribing && (true == s_supportScreenScrape)) + // Capture screen output if we are transcribing and running stand alone + if (_isTranscribing && (true == s_supportScreenScrape) && _runStandAlone) { Host.Coordinates endPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; endPosition.X = this.Command.Context.EngineHostInterface.UI.RawUI.BufferSize.Width - 1; @@ -1294,6 +1290,11 @@ private void CalculateIORedirection(out bool redirectOutput, out bool redirectEr redirectError = true; } } + else if (_isTranscribing && (false == s_supportScreenScrape)) + { + redirectOutput = true; + redirectError = true; + } _runStandAlone = !redirectInput && !redirectOutput && !redirectError; } From 0f2b52aa19a1cd558a52f5ad076b3ef2d4a44814 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 25 Sep 2017 16:15:08 -0700 Subject: [PATCH 5/7] [feature] address PR feedback from Dongbo --- .../engine/NativeCommandProcessor.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index fbb6b3ac1f9..4ac15dc0d2c 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -401,9 +401,9 @@ private void InitNativeProcess() // we have to see if the redirection is actually being done at the topmost level or not. // Calculate if input and output are redirected. - bool redirectOutput = true; - bool redirectError = true; - bool redirectInput = this.Command.MyInvocation.ExpectingInput; + bool redirectOutput; + bool redirectError; + bool redirectInput; _startPosition = new Host.Coordinates(); @@ -1290,7 +1290,10 @@ private void CalculateIORedirection(out bool redirectOutput, out bool redirectEr redirectError = true; } } - else if (_isTranscribing && (false == s_supportScreenScrape)) + + // if screen scraping isn't supported, we enable redirection so that the output is still transcribed + // as redirected output is always transcribed + if (_isTranscribing && (false == s_supportScreenScrape)) { redirectOutput = true; redirectError = true; From 2ff64d8ef83b8f8c894ae3be268ab4148fee74fe Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 26 Sep 2017 15:35:39 -0700 Subject: [PATCH 6/7] [feature] added test for start-job and native commands catch all exceptions trying to read the buffer --- .../engine/NativeCommandProcessor.cs | 24 +++++++++---------- test/powershell/engine/Job/Jobs.Tests.ps1 | 15 ++++++++++++ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 4ac15dc0d2c..e8be68d27c1 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -375,8 +375,8 @@ internal override void ProcessRecord() /// private BlockingCollection _nativeProcessOutputQueue; - private static bool? s_supportScreenScrape = null; - private bool _isTranscribing; + private static bool s_supportScreenScrape = false; + private bool _isTranscribing = false; private Host.Coordinates _startPosition; /// @@ -407,18 +407,16 @@ private void InitNativeProcess() _startPosition = new Host.Coordinates(); - if (null == s_supportScreenScrape) + try { - try - { - Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( - new Host.Rectangle(_startPosition, _startPosition)); - s_supportScreenScrape = true; - } - catch (NotImplementedException) - { - s_supportScreenScrape = false; - } + _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; + Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( + new Host.Rectangle(_startPosition, _startPosition)); + s_supportScreenScrape = true; + } + catch (Exception) + { + // screen scraping not supported on non-Windows or as job } CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); diff --git a/test/powershell/engine/Job/Jobs.Tests.ps1 b/test/powershell/engine/Job/Jobs.Tests.ps1 index 789160d6b9c..dd28559df3e 100644 --- a/test/powershell/engine/Job/Jobs.Tests.ps1 +++ b/test/powershell/engine/Job/Jobs.Tests.ps1 @@ -17,6 +17,21 @@ Describe 'Basic Job Tests' -Tags 'CI' { Receive-Job $job -wait | should be 1 } + It "Create job with native command" { + try { + $nativeJob = Start-job { powershell -c 1+1 } + $nativeJob | Wait-Job + $nativeJob.State | Should BeExactly "Completed" + $nativeJob.HasMoreData | Should Be $true + Receive-Job $nativeJob | Should BeExactly 2 + Remove-Job $nativeJob + { Get-Job $nativeJob -ErrorAction Stop } | ShouldBeErrorId "JobWithSpecifiedNameNotFound,Microsoft.PowerShell.Commands.GetJobCommand" + } + finally { + Remove-Job $nativeJob -Force -ErrorAction SilentlyContinue + } + } + AfterAll { Remove-Job $job -Force } From bb5767d64d7fcd8f7d3ce940d34319b6671b97b2 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 28 Sep 2017 10:44:45 -0700 Subject: [PATCH 7/7] [feature] refactor code --- .../engine/NativeCommandProcessor.cs | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index e8be68d27c1..af785632519 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -375,8 +375,8 @@ internal override void ProcessRecord() /// private BlockingCollection _nativeProcessOutputQueue; - private static bool s_supportScreenScrape = false; - private bool _isTranscribing = false; + private static bool? s_supportScreenScrape = null; + private bool _isTranscribing; private Host.Coordinates _startPosition; /// @@ -407,18 +407,6 @@ private void InitNativeProcess() _startPosition = new Host.Coordinates(); - try - { - _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; - Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( - new Host.Rectangle(_startPosition, _startPosition)); - s_supportScreenScrape = true; - } - catch (Exception) - { - // screen scraping not supported on non-Windows or as job - } - CalculateIORedirection(out redirectOutput, out redirectError, out redirectInput); // Find out if it's the only command in the pipeline. @@ -1289,15 +1277,34 @@ private void CalculateIORedirection(out bool redirectOutput, out bool redirectEr } } - // if screen scraping isn't supported, we enable redirection so that the output is still transcribed - // as redirected output is always transcribed - if (_isTranscribing && (false == s_supportScreenScrape)) + _runStandAlone = !redirectInput && !redirectOutput && !redirectError; + + if (_runStandAlone) { - redirectOutput = true; - redirectError = true; - } + if (null == s_supportScreenScrape) + { + try + { + _startPosition = this.Command.Context.EngineHostInterface.UI.RawUI.CursorPosition; + Host.BufferCell[,] bufferContents = this.Command.Context.EngineHostInterface.UI.RawUI.GetBufferContents( + new Host.Rectangle(_startPosition, _startPosition)); + s_supportScreenScrape = true; + } + catch (Exception) + { + s_supportScreenScrape = false; + } + } - _runStandAlone = !redirectInput && !redirectOutput && !redirectError; + // if screen scraping isn't supported, we enable redirection so that the output is still transcribed + // as redirected output is always transcribed + if (_isTranscribing && (false == s_supportScreenScrape)) + { + redirectOutput = true; + redirectError = true; + _runStandAlone = false; + } + } } private bool ValidateExtension(string path)