FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Importing Windows PowerShell modules in PowerShell Core by anmenaga · Pull Request #10973 · PowerShell/PowerShell · GitHub

Importing Windows PowerShell modules in PowerShell Core - #10973

Merged
Dongbo Wang (daxian-dbw) merged 36 commits into
PowerShell:masterfrom
anmenaga:WinCompat
Nov 18, 2019
Merged

Importing Windows PowerShell modules in PowerShell Core#10973
Dongbo Wang (daxian-dbw) merged 36 commits into
PowerShell:masterfrom
anmenaga:WinCompat

Conversation

Andrew (anmenaga) commented Nov 1, 2019
edited
Loading

Copy link
Copy Markdown

PR Summary

Implementation for 'Importing Windows PowerShell modules in PowerShell Core' RFC.

  • Modules, that are non-compatible with PS Core and were previously generating PSEditionNotSupported error, are now loaded into a background Windows PowerShell process connected to PS Core using a WinPSCompatSession PS remoting session using redirected process streams transport (same one used by PS jobs).
  • Behaviour can be forced using Import-Module -UseWindowsPowerShell
  • Module autoload during command discovery is supported
  • a warning is displayed when a module is loaded into compat Windows PS.
  • a new telemetry type is added and reported when a module is loaded into compat Windows PS.
  • operations that do not work on de/serialized objects can be done in WinPSCompatSession remoting session $s = Get-PSSession -Name WinPSCompatSession; Invoke-Command -Session $s -ScriptBlock {Get-WinPSLiveObject | Set-WinPSLiveObject}
  • new functionality is under PSWinCompat experimental feature.

Example:

PR Context

PR Checklist

Copy link
Copy Markdown

This PR might impact tooling, because it might be necessary to debug a module imported via compat.

Copy link
Copy Markdown
Collaborator

$s = Get-PSSession -Name WinPSCompatSession; Invoke-Command -Session $s -ScriptBlock {Get-WinPSLiveObject | Set-WinPSLiveObject}

Given that this is to be a builtin feature, the user experience might be better if encapsulated:

Invoke-InWinPSCompatibilitySession { Get-WinPSLiveObject | Set-WinPSLiveObject }

Copy link
Copy Markdown
Collaborator

This PR might impact tooling, because it might be necessary to debug a module imported via compat.

@alexbuzzbee could you expand on this with an example? What scenario are you envisaging here? Also, it might be best to have this discussion in the RFC.

Copy link
Copy Markdown

Say you have a script module that was written as Windows-specific (because it uses Windows P/Invokes, COM, or Windows-specific .NET APIs), but need to use it in a modern script. A problem is happening inside the Windows-specific module. You need to debug inside it, so the debugging tools need to be able to see and work inside the Windows PowerShell compat session.

Yes, it might be possible to reproduce the issue using a Windows PowerShell script instead, but that would usually be significantly more effort than just debugging the module.

It might be correct that this should go in the RFC; I was just going off the tooling checkboxes and noted that they might not be accurately filled.

Copy link
Copy Markdown
Author

@alexbuzzbee Do these examples fall into scenario that you describe?

Debugging from the same PS process:

Debugging from another PS process:

Copy link
Copy Markdown

Andrew (@anmenaga) All I was trying to point out is that the "I have considered the user experience from a tooling perspective and enumerated concerns in the summary" box should probably be checked instead of "I have considered the user experience from a tooling perspective and don't believe tooling will be impacted."

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Nov 5, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Member

Andrew (@anmenaga) Is this PR ready for review? If so, please remove the WIP: prefix from the title so we can pull in more reviewers.

Andrew (anmenaga) added the Documentation Needed in this repo Documentation is needed in this repo label Nov 13, 2019
Andrew (anmenaga) changed the title WIP: Importing Windows PowerShell modules in PowerShell Core Importing Windows PowerShell modules in PowerShell Core Nov 13, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

telemetry changes look good

"Alias", "nsn", "New-PSSession", $($FullCLR -or $CoreWindows -or $CoreUnix), "", "", ""
"Alias", "nv", "New-Variable", $($FullCLR -or $CoreWindows -or $CoreUnix), "ReadOnly", "", ""
"Alias", "nwsn", "New-PSWorkflowSession", $($FullCLR ), "ReadOnly", "", ""
"Alias", "nwsn", "New-PSWorkflowSession", $($FullCLR -or $CoreWindows ), "", "", ""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why do we need the change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

this alias comes from PSWorkflow module which is incompartible with PS Core.
This test just loads all stuff from a hardcoded set of modules and checks against this huge table; PSWorkflow is one of them. With this PR module can actually be successfully loaded on PS Core and the alias shows up, and the test did not expect this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

DefaultCommands.Tests is designed to tracking that is explicitly in PowerShell Core on different platforms and show difference from Windows PowerShell.

If the test failed I guess it is side effect from other tests and just the tests must do right cleanup.

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Nov 14, 2019

Copy link
Copy Markdown
Member

Andrew (@anmenaga) Can you please resolve the conflict, again :)

Dongbo Wang (daxian-dbw) added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Nov 18, 2019
Dongbo Wang (daxian-dbw) merged commit b218e6f into PowerShell:master Nov 18, 2019

Copy link
Copy Markdown
Collaborator

Great work Andrew (@anmenaga) !

Copy link
Copy Markdown

🎉v7.0.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log Documentation Needed in this repo Documentation is needed in this repo MustHave

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL