[feature] Add -CustomPipeName to `pwsh` and `Enter-PSHostProcess` by TylerLeonhardt · Pull Request #8889 · PowerShell/PowerShell

TylerLeonhardt · 2019-02-13T19:44:49Z

PR Summary

This allows a user to start PowerShell up with the name of the named pipe that is used for cross process communication (I.e. Enter-PSHostProcess).

Example:

terminal 1

pwsh -custompipename mypipe

terminal 2

Enter-PSHostProcess -CustomPipeName mypipe

PR Context

Today, it is impossible to predictively connect to a PowerShell process without knowing its process id. If you're stopping and starting a powershell process often... you have no mechanism for figuring out the process id.

This introduces a consistent way to connect to a PowerShell process by specifying the NamedPipe yourself.

This is very nice in PowerShell hosted scenarios - for example, in Azure Functions where PowerShell is hosted. This enables the ability to consistently connect to the PowerShell instance hosted there without any additional information besides the CustomPipeName that can be reused.

This feature also means that in the PowerShell extension for VSCode, we can achieve "Attach to Process" debugging with a single stroke of the F5 key.

PR Checklist

src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs

PaulHigin

I will finish the review later, but have a couple of comments now.
Thanks!

src/Microsoft.PowerShell.ConsoleHost/resources/CommandLineParameterParserStrings.resx

src/System.Management.Automation/engine/hostifaces/LocalConnection.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs

src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs

src/System.Management.Automation/engine/InitialSessionState.cs

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

src/System.Management.Automation/engine/remoting/fanin/OutOfProcTransportManager.cs

test/powershell/Modules/Microsoft.PowerShell.Core/Enter-PSHostProcess.Tests.ps1

PaulHigin

Let's remove the connection of pipe listener creation to local runspace, and think about whether we want to replace the existing default IPC channel or create a second one when starting powershell.exe in DebugPipeName mode, although I think we should use a different name for the switch, like IPCPipe or CustomIPCPipe, or something.

src/System.Management.Automation/engine/hostifaces/LocalConnection.cs

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

TylerLeonhardt · 2019-02-15T23:54:15Z

@PaulHigin I've refactored this quite a bit based on your offline feedback. The same experience is there for a user at the CLI.

For users that are going through the API, the can use:

S.M.A.Remoting.RemoteSessionNamedPipeServer.CreateCustomNamedPipeServer("mypipe");

There is 1 thing left to do here:

~~change the name from DebugPipeName to CustomPipeName - everyone feel free to comment~~ changed
~~figure out what should happen when this static function is called multiple times - Should it throw? Or should it overwrite?~~ I've added it to dispose of the current server and recreate the new one

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs

src/Microsoft.PowerShell.ConsoleHost/resources/CommandLineParameterParserStrings.resx

src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

TylerLeonhardt · 2019-02-20T06:59:40Z

Addressed all feedback 👍

PaulHigin

LGTM. But please create new Issue about disposing pipe server objects on process exit (please see comment).

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

TylerLeonhardt · 2019-02-21T03:19:42Z

Ok I included a few CodeFactor and Codacy fixes as well. This should be good to go pending CI ✅

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs

src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs

test/powershell/Modules/Microsoft.PowerShell.Core/Enter-PSHostProcess.Tests.ps1

TylerLeonhardt · 2019-02-21T06:55:07Z

I've addressed @PaulHigin's final point which is that DebugPipeName is not totally accurate as this pipe could be used for other IPC scenarios.

It's mostly used for debugging though... and our usecase is really for debugging... so I've called this out in the -help page.

His recommendation and what I went with was CustomPipeName. I've updated that throughout.

iSazonov · 2019-02-21T09:01:58Z

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

                case ProcessIdParameterSet:
                    Process = GetProcessById(Id);
+                    VerifyProcess(Process);
+                    namedPipeRunspace = CreateNamedPipeRunspace(Process.Id, AppDomainName);


Interesting, do we really need the VerifyProcess() check (that is File.Exist())? Perhaps the runtime error is informative enough.

I'm not quite sure what you're saying. This VerifyProcess is to keep the behavior consistent with the current behavior for Process parameter sets.

iSazonov · 2019-02-21T12:17:20Z

@TylerLeonhardt Please create new Issue in PowerShell-Docs repo and add reference to the PR description.

TylerLeonhardt · 2019-02-22T02:15:04Z

Alright CI is green and codacy and Code Factor are mostly happy. This should be good to go.

iSazonov

LGTM with minor comments.

test/xUnit/csharp/test_NamedPipe.cs

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs

iSazonov · 2019-02-22T06:52:48Z

@TylerLeonhardt Please create new Issue in PowerShell-Docs repo before we merge.

TylerLeonhardt · 2019-02-22T18:34:19Z

Done, @iSazonov!

iSazonov · 2019-02-22T18:40:24Z

@TylerLeonhardt Thanks for your contribution!

TylerLeonhardt requested review from BrucePay, PaulHigin, anmenaga, daxian-dbw and mirichmo as code owners February 13, 2019 19:44

TylerLeonhardt requested a review from SteveL-MSFT February 13, 2019 19:48

SteveL-MSFT requested changes Feb 14, 2019

View reviewed changes

src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs Outdated Show resolved Hide resolved

PaulHigin suggested changes Feb 14, 2019

View reviewed changes

src/Microsoft.PowerShell.ConsoleHost/resources/CommandLineParameterParserStrings.resx Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/hostifaces/LocalConnection.cs Outdated Show resolved Hide resolved

iSazonov reviewed Feb 14, 2019

View reviewed changes

PaulHigin suggested changes Feb 14, 2019

View reviewed changes

TylerLeonhardt changed the title ~~[feature] Add -DebugPipeName to pwsh and Enter-PSHostProcess~~ [feature] [WIP] Add -DebugPipeName to pwsh and Enter-PSHostProcess Feb 15, 2019

TylerLeonhardt added 8 commits February 15, 2019 15:54

Add DebugPipeName capability

51efb81

[feature] minor fix for Windows

37f0956

[feature] misc code factor fixes

a27b6b5

[feature] add more tests and pass command down to ServerMode

6689e89

[feature] fix path for Windows

6e7578c

[feature] add erroraction stop and rely on a better way to wait

5d03cb6

refactor to a static public method

7fa9459

[feature] change test casing

d7df174

TylerLeonhardt force-pushed the debugpipename-enterpshostprocess branch from d47a632 to d7df174 Compare February 15, 2019 23:55

dispose of the server if we recall the function

79eabb2

TylerLeonhardt changed the title ~~[feature] [WIP] Add -DebugPipeName to pwsh and Enter-PSHostProcess~~ [feature] Add -DebugPipeName to pwsh and Enter-PSHostProcess Feb 16, 2019

PaulHigin suggested changes Feb 19, 2019

View reviewed changes

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/remoting/common/RunspaceConnectionInfo.cs Outdated Show resolved Hide resolved

daxian-dbw reviewed Feb 19, 2019

View reviewed changes

TylerLeonhardt added 2 commits February 19, 2019 22:03

address all feedback

f8ab953

[feature] add test for API and help text

25fbad7

PaulHigin approved these changes Feb 20, 2019

View reviewed changes

src/System.Management.Automation/engine/remoting/common/RemoteSessionNamedPipe.cs Show resolved Hide resolved

TylerLeonhardt mentioned this pull request Feb 20, 2019

Investigate/code clean up ifdef in RemoteSessionNamedPipe.cs #8925

Closed

[feature] fix log message and Codacy and Code Factor fixes

69621ca

iSazonov reviewed Feb 21, 2019

View reviewed changes

Change DebugPipeName to CustomPipeName

73cd2ce

TylerLeonhardt changed the title ~~[feature] Add -DebugPipeName to pwsh and Enter-PSHostProcess~~ [feature] Add -CustomPipeName to pwsh and Enter-PSHostProcess Feb 21, 2019

[feature] touch up summary

636c401

iSazonov reviewed Feb 21, 2019

View reviewed changes

TylerLeonhardt added 5 commits February 21, 2019 14:11

move API tests to xUnit

f8260cf

[feature] Add ParallelXUnitResults.xml

d79da98

[feature] remove debug log

5549bd8

[feature] switch to Start-Process

d9d1779

misc codacy fixes

1edba6c

iSazonov approved these changes Feb 22, 2019

View reviewed changes

test/xUnit/csharp/test_NamedPipe.cs Outdated Show resolved Hide resolved

test/xUnit/csharp/test_NamedPipe.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/remoting/commands/EnterPSHostProcessCommand.cs Outdated Show resolved Hide resolved

[feature] address Ilya's comments

9c3dfb3

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Feb 22, 2019

TylerLeonhardt mentioned this pull request Feb 22, 2019

Doc new -CustomPipeName parameter on pwsh and Enter-PSHostProcess MicrosoftDocs/PowerShell-Docs#3755

Closed

8 tasks

iSazonov self-assigned this Feb 22, 2019

iSazonov merged commit 23eccfd into PowerShell:master Feb 22, 2019

TylerLeonhardt deleted the debugpipename-enterpshostprocess branch February 23, 2019 01:35

This was referenced Feb 27, 2019

Support -CustomPipeName PowerShell/PowerShellEditorServices#871

Merged

Support -CustomPipeName PowerShell/vscode-powershell#1775

Merged

joeyaiello added this to the 6.2.0 milestone Mar 22, 2019

This was referenced Mar 22, 2019

update docs for -CustomPipeName MicrosoftDocs/PowerShell-Docs#4032

Merged

Typo in new parameter set #9205

Merged

Conversation

TylerLeonhardt commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Example:

PR Context

PR Checklist

Uh oh!

Uh oh!

PaulHigin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PaulHigin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TylerLeonhardt commented Feb 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TylerLeonhardt commented Feb 20, 2019

Uh oh!

PaulHigin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TylerLeonhardt commented Feb 21, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TylerLeonhardt commented Feb 21, 2019

Uh oh!

iSazonov Feb 21, 2019

Choose a reason for hiding this comment

Uh oh!

TylerLeonhardt Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Feb 21, 2019

Uh oh!

TylerLeonhardt commented Feb 22, 2019

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iSazonov commented Feb 22, 2019

Uh oh!

TylerLeonhardt commented Feb 22, 2019

Uh oh!

iSazonov commented Feb 22, 2019

Uh oh!

Reviewers

Assignees

Labels

TylerLeonhardt commented Feb 13, 2019 •

edited

Loading

TylerLeonhardt commented Feb 15, 2019 •

edited

Loading

TylerLeonhardt Feb 21, 2019 •

edited

Loading