Skip to content

Make initial logging work in constrained language mode #1222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 10, 2020

Conversation

rjmholt
Copy link
Contributor

@rjmholt rjmholt commented Mar 6, 2020

No description provided.

@rjmholt rjmholt changed the title Make initial logging work in constrained language mode WIP: Make initial logging work in constrained language mode Mar 6, 2020
Comment on lines 412 to 425
private static object GetPSVersion()
{
#if CoreCLR
return typeof(PSObject).Assembly
.GetType("System.Management.Automation.PSVersionInfo")
.GetMethod("get_PSVersion", BindingFlags.Static | BindingFlags.Public)
.Invoke(null, Array.Empty<object>());
#else
return typeof(PSObject).Assembly
.GetType("System.Management.Automation.PSVersionInfo", BindingFlags.Instance | BindingFlags.NonPublic)
.GetMethod("get_PSVersion", BindingFlags.Static | BindingFlags.NonPublic)
.Invoke(null, Array.Empty<object>());
#endif
}
Copy link
Member

Choose a reason for hiding this comment

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

isn't this in the VersionUtils class already?

Copy link
Contributor Author

@rjmholt rjmholt Mar 6, 2020

Choose a reason for hiding this comment

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

If it is, we can't use it unfortunately, since we can't load PSES.dll at this stage.

Copy link
Member

Choose a reason for hiding this comment

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

What about using this?

pwsh.Runspace.SessionStateProxy.PSVariable.Get(name);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could try that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the language mode check works, but the GetVariable API doesn't because it throws on concurrent access. At that point we'd need to spin up a new runspace to use it, which wouldn't be good for startup. But I simplified the reflection a bit at least

@rjmholt rjmholt changed the title WIP: Make initial logging work in constrained language mode Make initial logging work in constrained language mode Mar 9, 2020
@@ -339,16 +341,17 @@ private string GetPSOutputEncoding()

private void LogPowerShellDetails()
{
PSLanguageMode languageMode;
using (var pwsh = SMA.PowerShell.Create(SMA.RunspaceMode.CurrentRunspace))
Copy link
Member

@TylerLeonhardt TylerLeonhardt Mar 9, 2020

Choose a reason for hiding this comment

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

Curious if this is the same as [runspace]::DefaultRunspace... probably, right? Since you're getting the LanguageMode.


private static object GetPSVersion()
{
using (var pwsh = SMA.PowerShell.Create())
Copy link
Member

@TylerLeonhardt TylerLeonhardt Mar 9, 2020

Choose a reason for hiding this comment

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

Curious if this is the same as [runspace]::DefaultRunspace... probably, right? Since you're getting PSVersionTable.

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM 1 nit

@@ -11,6 +11,9 @@
using System.Runtime.InteropServices;

using SMA = System.Management.Automation;
Copy link
Member

Choose a reason for hiding this comment

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

Still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed for the $outputencoding log

@rjmholt rjmholt merged commit ce265d7 into PowerShell:master Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants