Skip to content

Initialize runspaces with InitialSessionState object #1526

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 1 commit into from
Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ insert_final_newline = true
indent_size = 4
trim_trailing_whitespace = true
csharp_space_before_open_square_brackets = true
csharp_space_after_keywords_in_control_flow_statements = false
csharp_space_after_keywords_in_control_flow_statements = true
Copy link
Member Author

Choose a reason for hiding this comment

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

@rjmholt I have no idea why this was false previously.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure either -- might be an autogenerated setting from a long time ago


[*.{json}]
indent_size = 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,15 +349,24 @@ private EditorServicesConfig CreateConfigObject()
var profile = (PSObject)GetVariableValue("profile");

var hostInfo = new HostInfo(HostName, HostProfileId, HostVersion);
var editorServicesConfig = new EditorServicesConfig(hostInfo, Host, SessionDetailsPath, bundledModulesPath, LogPath)

var initialSessionState = Runspace.DefaultRunspace.InitialSessionState;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var initialSessionState = Runspace.DefaultRunspace.InitialSessionState;
InitialSessionState initialSessionState = Runspace.DefaultRunspace.InitialSessionState;

initialSessionState.LanguageMode = Runspace.DefaultRunspace.SessionStateProxy.LanguageMode;

var editorServicesConfig = new EditorServicesConfig(
hostInfo,
Host,
SessionDetailsPath,
bundledModulesPath,
LogPath)
{
FeatureFlags = FeatureFlags,
LogLevel = LogLevel,
ConsoleRepl = GetReplKind(),
AdditionalModules = AdditionalModules,
LanguageServiceTransport = GetLanguageServiceTransport(),
DebugServiceTransport = GetDebugServiceTransport(),
LanguageMode = Runspace.DefaultRunspace.SessionStateProxy.LanguageMode,
InitialSessionState = initialSessionState,
ProfilePaths = new ProfilePathConfig
{
AllUsersAllHosts = GetProfilePathFromProfileObject(profile, ProfileUserKind.AllUsers, ProfileHostKind.AllHosts),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Licensed under the MIT License.

using System.Collections.Generic;
using System.Management.Automation;
using System.Management.Automation.Host;
using System.Management.Automation.Runspaces;

namespace Microsoft.PowerShell.EditorServices.Hosting
{
Expand Down Expand Up @@ -111,10 +111,9 @@ public EditorServicesConfig(
public ProfilePathConfig ProfilePaths { get; set; }

/// <summary>
/// The language mode inherited from the orginal PowerShell process.
/// This will be used when creating runspaces so that we honor the same language mode.
/// The InitialSessionState to use when creating runspaces. LanguageMode can be set here.
/// </summary>
public PSLanguageMode LanguageMode { get; internal set; }
public InitialSessionState InitialSessionState { get; internal set; }

public string StartupBanner { get; set; } = @"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ private HostStartupInfo CreateHostStartupInfo()
profilePaths,
_config.FeatureFlags,
_config.AdditionalModules,
_config.LanguageMode,
_config.InitialSessionState,
_config.LogPath,
(int)_config.LogLevel,
consoleReplEnabled: _config.ConsoleRepl != ConsoleReplKind.None,
Expand Down
14 changes: 7 additions & 7 deletions src/PowerShellEditorServices/Hosting/HostStartupInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

using System;
using System.Collections.Generic;
using System.Management.Automation;
using System.Management.Automation.Host;
using System.Management.Automation.Runspaces;

namespace Microsoft.PowerShell.EditorServices.Hosting
{
Expand Down Expand Up @@ -92,10 +92,10 @@ public sealed class HostStartupInfo
public string LogPath { get; }

/// <summary>
/// The language mode inherited from the orginal PowerShell process.
/// This will be used when creating runspaces so that we honor the same language mode.
/// The InitialSessionState will be inherited from the orginal PowerShell process. This will
/// be used when creating runspaces so that we honor the same InitialSessionState.
/// </summary>
public PSLanguageMode LanguageMode { get; }
public InitialSessionState InitialSessionState { get; }

/// <summary>
/// The minimum log level of log events to be logged.
Expand Down Expand Up @@ -135,7 +135,7 @@ public sealed class HostStartupInfo
/// <param name="currentUsersProfilePath">The path to the user specific profile.</param>
/// <param name="featureFlags">Flags of features to enable.</param>
/// <param name="additionalModules">Names or paths of additional modules to import.</param>
/// <param name="languageMode">The language mode inherited from the orginal PowerShell process. This will be used when creating runspaces so that we honor the same language mode.</param>
/// <param name="initialSessionState">The language mode inherited from the orginal PowerShell process. This will be used when creating runspaces so that we honor the same initialSessionState including allowed modules, cmdlets and language mode.</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

The initial part of this description is no longer accurate

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh dang, I thought I double-checked all of these. Ah well. For the documentation project.

/// <param name="logPath">The path to log to.</param>
/// <param name="logLevel">The minimum log event level.</param>
/// <param name="consoleReplEnabled">Enable console if true.</param>
Expand All @@ -149,7 +149,7 @@ public HostStartupInfo(
ProfilePathInfo profilePaths,
IReadOnlyList<string> featureFlags,
IReadOnlyList<string> additionalModules,
PSLanguageMode languageMode,
InitialSessionState initialSessionState,
string logPath,
int logLevel,
bool consoleReplEnabled,
Expand All @@ -163,7 +163,7 @@ public HostStartupInfo(
ProfilePaths = profilePaths;
FeatureFlags = featureFlags ?? Array.Empty<string>();
AdditionalModules = additionalModules ?? Array.Empty<string>();
LanguageMode = languageMode;
InitialSessionState = initialSessionState;
LogPath = logPath;
LogLevel = logLevel;
ConsoleReplEnabled = consoleReplEnabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using System.Management.Automation.Remoting;
using System.Management.Automation.Runspaces;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -230,7 +229,7 @@ public static PowerShellContextService Create(
logger);

logger.LogTrace("Creating initial PowerShell runspace");
Runspace initialRunspace = PowerShellContextService.CreateRunspace(psHost, hostStartupInfo.LanguageMode);
Runspace initialRunspace = PowerShellContextService.CreateRunspace(psHost, hostStartupInfo.InitialSessionState);
powerShellContext.Initialize(hostStartupInfo.ProfilePaths, initialRunspace, true, hostUserInterface);
powerShellContext.ImportCommandsModuleAsync();

Expand All @@ -256,14 +255,17 @@ public static PowerShellContextService Create(
}

/// <summary>
///
/// Only used in testing. Creates a Runspace given HostStartupInfo instead of a PSHost.
/// </summary>
/// <remarks>
/// TODO: We should use `CreateRunspace` in testing instead of this, if possible.
/// </remarks>
/// <param name="hostDetails"></param>
/// <param name="powerShellContext"></param>
/// <param name="hostUserInterface">The EditorServicesPSHostUserInterface to use for this instance.</param>
/// <param name="logger">An ILogger implementation to use for this instance.</param>
/// <returns></returns>
public static Runspace CreateRunspace(
public static Runspace CreateTestRunspace(
HostStartupInfo hostDetails,
PowerShellContextService powerShellContext,
EditorServicesPSHostUserInterface hostUserInterface,
Expand All @@ -274,38 +276,17 @@ public static Runspace CreateRunspace(
var psHost = new EditorServicesPSHost(powerShellContext, hostDetails, hostUserInterface, logger);
powerShellContext.ConsoleWriter = hostUserInterface;
powerShellContext.ConsoleReader = hostUserInterface;
return CreateRunspace(psHost, hostDetails.LanguageMode);
return CreateRunspace(psHost, hostDetails.InitialSessionState);
}

/// <summary>
///
/// </summary>
/// <param name="psHost">The PSHost that will be used for this Runspace.</param>
/// <param name="languageMode">The language mode inherited from the orginal PowerShell process. This will be used when creating runspaces so that we honor the same language mode.</param>
/// <param name="initialSessionState">This will be used when creating runspaces so that we honor the same InitialSessionState.</param>
/// <returns></returns>
public static Runspace CreateRunspace(PSHost psHost, PSLanguageMode languageMode)
public static Runspace CreateRunspace(PSHost psHost, InitialSessionState initialSessionState)
{
InitialSessionState initialSessionState;
if (Environment.GetEnvironmentVariable("PSES_TEST_USE_CREATE_DEFAULT") == "1") {
Copy link
Member Author

Choose a reason for hiding this comment

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

@rjmholt FYI this code is gone but I don't believe we were using it for anything at the moment.

initialSessionState = InitialSessionState.CreateDefault();
} else {
initialSessionState = InitialSessionState.CreateDefault2();
}

// Create and initialize a new Runspace while honoring the LanguageMode of the original runspace
// that started PowerShell Editor Services. This is because the PowerShell Integrated Console
// should have the same LanguageMode of whatever is set by the system.
initialSessionState.LanguageMode = languageMode;

// We set the process scope's execution policy (which is really the runspace's scope) to
// Bypass so we can import our bundled modules. This is equivalent in scope to the CLI
// argument `-Bypass`, which (for instance) the extension passes. Thus we emulate this
// behavior for consistency such that unit tests can pass in a similar environment.
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
initialSessionState.ExecutionPolicy = ExecutionPolicy.Bypass;
}

Runspace runspace = RunspaceFactory.CreateRunspace(psHost, initialSessionState);

// Windows PowerShell must be hosted in STA mode
Expand Down Expand Up @@ -434,10 +415,9 @@ public void Initialize(
this.PromptContext = new LegacyReadLineContext(this);
}

if (VersionUtils.IsWindows)
{
this.SetExecutionPolicy();
}
// Finally, restore the runspace's execution policy to the user's policy instead of
// Bypass.
this.RestoreExecutionPolicy();
Copy link
Member Author

Choose a reason for hiding this comment

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

@rjmholt I moved the "if windows" to "if not windows" inside the function itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dkattan This still needs to be called because it restores the process scope policy when it was set on the CLI (such as by the extension).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I learned this the hard way in my constrained runspace branch. I makes a lot more sense now that it is called RestoreExecutionPolicy.

}

/// <summary>
Expand Down Expand Up @@ -2152,9 +2132,20 @@ private static string GetStringForPSCommand(PSCommand psCommand)
return stringBuilder.ToString();
}

private void SetExecutionPolicy()
/// <summary>
/// This function restores the execution policy for the process by examining the user's
/// execution policy hierarchy. We do this because the process policy will always be set to
/// Bypass when initializing our runspaces.
/// </summary>
internal void RestoreExecutionPolicy()
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for renaming this @dkattan!

{
this.logger.LogTrace("Setting execution policy...");
// Execution policy is a Windows-only feature.
if (!VersionUtils.IsWindows)
{
return;
}

this.logger.LogTrace("Restoring execution policy...");

// We want to get the list hierarchy of execution policies
// Calling the cmdlet is the simplest way to do that
Expand Down
19 changes: 14 additions & 5 deletions test/PowerShellEditorServices.Test/PowerShellContextFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Management.Automation;
using System.Management.Automation.Runspaces;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
Expand All @@ -13,6 +13,7 @@
using Microsoft.PowerShell.EditorServices.Services;
using Microsoft.PowerShell.EditorServices.Services.PowerShellContext;
using Microsoft.PowerShell.EditorServices.Test.Shared;
using Microsoft.PowerShell.EditorServices.Utility;

namespace Microsoft.PowerShell.EditorServices.Test
{
Expand Down Expand Up @@ -40,6 +41,16 @@ internal static class PowerShellContextFactory
public static PowerShellContextService Create(ILogger logger)
{
PowerShellContextService powerShellContext = new PowerShellContextService(logger, null, isPSReadLineEnabled: false);
var initialSessionState = InitialSessionState.CreateDefault();
Copy link
Member Author

Choose a reason for hiding this comment

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

Wait, @rjmholt @dkattan shouldn't this have been CreateDefault2()???

// We set the process scope's execution policy (which is really the runspace's scope) to
// `Bypass` so we can import our bundled modules. This is equivalent in scope to the CLI
// argument `-ExecutionPolicy Bypass`, which (for instance) the extension passes. Thus
// we emulate this behavior for consistency such that unit tests can pass in a similar
// environment.
if (VersionUtils.IsWindows)
{
initialSessionState.ExecutionPolicy = ExecutionPolicy.Bypass;
Copy link
Member Author

Choose a reason for hiding this comment

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

Yay! This if finally just in the test path.

}

HostStartupInfo testHostDetails = new HostStartupInfo(
"PowerShell Editor Services Test Host",
Expand All @@ -49,16 +60,14 @@ public static PowerShellContextService Create(ILogger logger)
TestProfilePaths,
new List<string>(),
new List<string>(),
// TODO: We want to replace this property with an entire initial session state,
// which would then also control the process-scoped execution policy.
PSLanguageMode.FullLanguage,
initialSessionState,
null,
0,
consoleReplEnabled: false,
usesLegacyReadLine: false,
bundledModulePath: BundledModulePath);

InitialRunspace = PowerShellContextService.CreateRunspace(
InitialRunspace = PowerShellContextService.CreateTestRunspace(
testHostDetails,
powerShellContext,
new TestPSHostUserInterface(powerShellContext, logger),
Expand Down