Initialize runspaces with `InitialSessionState` object #1526

andyleejordan · 2021-07-20T00:02:24Z

Instead of passing values for LanguageMode and ExecutionPolicy as
fields that are set on an InitialSessionState object that's eventually
created, we now pass a fully initialized InitialSessionState object
from the start.

We also rename SetExecutionPolicy to RestoreExecutionPolicy to
reflect what it actually does: restores the user's policy after we
overrode it with Bypass while initializing the integrated console's
runspace. We override it because we need to be able to load PSReadLine
etc. without policy issues.

Finally, we fix the EditorConfig setting to enforce spaces after
control-flow keywords (such as if) because, while our codebase is
inconsistent, it is our preferred style going forward.

This replaces #1523.

andyleejordan · 2021-07-20T00:02:47Z

.editorconfig

@@ -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


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

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

andyleejordan · 2021-07-20T00:03:27Z

src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs

        {
-            InitialSessionState initialSessionState;
-            if (Environment.GetEnvironmentVariable("PSES_TEST_USE_CREATE_DEFAULT") == "1") {


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

andyleejordan · 2021-07-20T00:03:56Z

src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs

-            }
+            // Finally, restore the runspace's execution policy to the user's policy instead of
+            // Bypass.
+            this.RestoreExecutionPolicy();


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

@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).

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.

andyleejordan · 2021-07-20T00:04:08Z

src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs

+        /// execution policy hierarchy. We do this because the process policy will always be set to
+        /// Bypass when initializing our runspaces.
+        /// </summary>
+        internal void RestoreExecutionPolicy()


Thanks for renaming this @dkattan!

andyleejordan · 2021-07-20T00:04:26Z

test/PowerShellEditorServices.Test/PowerShellContextFactory.cs

+            // environment.
+            if (VersionUtils.IsWindows)
+            {
+                initialSessionState.ExecutionPolicy = ExecutionPolicy.Bypass;


Yay! This if finally just in the test path.

Instead of passing values for `LanguageMode` and `ExecutionPolicy` as fields that are set on an `InitialSessionState` object that's eventually created, we now pass a fully initialized `InitialSessionState` object from the start. We also rename `SetExecutionPolicy` to `RestoreExecutionPolicy` to reflect what it actually does: restores the user's policy after we overrode it with `Bypass` while initializing the integrated console's runspace. We override it because we need to be able to load `PSReadLine` etc. without policy issues. Finally, we fix the EditorConfig setting to enforce spaces after control-flow keywords (such as `if`) because, while our codebase is inconsistent, it is our preferred style going forward. Co-authored-by: Andrew Schwartzmeyer <[email protected]>

rjmholt · 2021-07-20T17:46:11Z

.editorconfig

@@ -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


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

rjmholt · 2021-07-20T17:46:31Z

src/PowerShellEditorServices.Hosting/Commands/StartEditorServicesCommand.cs

@@ -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;


Suggested change

var initialSessionState = Runspace.DefaultRunspace.InitialSessionState;

InitialSessionState initialSessionState = Runspace.DefaultRunspace.InitialSessionState;

rjmholt · 2021-07-20T17:47:44Z

src/PowerShellEditorServices/Hosting/HostStartupInfo.cs

@@ -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>


The initial part of this description is no longer accurate

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

andyleejordan · 2021-08-18T22:50:15Z

test/PowerShellEditorServices.Test/PowerShellContextFactory.cs

@@ -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();


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

andyleejordan added Issue-Enhancement A feature request (enhancement). Area-Startup labels Jul 20, 2021

andyleejordan requested a review from rjmholt July 20, 2021 00:02

andyleejordan commented Jul 20, 2021

View reviewed changes

andyleejordan force-pushed the andschwa/initial-session-state branch from 1e0488c to 1ec771d Compare July 20, 2021 00:16

dkattan approved these changes Jul 20, 2021

View reviewed changes

andyleejordan enabled auto-merge (squash) July 20, 2021 16:14

rjmholt approved these changes Jul 20, 2021

View reviewed changes

andyleejordan merged commit fc3d4a9 into master Jul 20, 2021

andyleejordan deleted the andschwa/initial-session-state branch July 20, 2021 17:50

rjmholt mentioned this pull request Jul 20, 2021

Pass InitialSessionState instead of LanguageMode #1523

Closed

andyleejordan commented Aug 18, 2021

View reviewed changes

andyleejordan mentioned this pull request Aug 18, 2021

Fix creation of InitialSessionState to use CreateDefault2() #1547

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize runspaces with `InitialSessionState` object #1526

Initialize runspaces with `InitialSessionState` object #1526

andyleejordan commented Jul 20, 2021

andyleejordan Jul 20, 2021

rjmholt Jul 20, 2021

andyleejordan Jul 20, 2021

andyleejordan Jul 20, 2021

andyleejordan Jul 20, 2021

dkattan Jul 20, 2021

andyleejordan Jul 20, 2021

andyleejordan Jul 20, 2021

rjmholt Jul 20, 2021

rjmholt Jul 20, 2021

rjmholt Jul 20, 2021

andyleejordan Jul 20, 2021

andyleejordan Aug 18, 2021

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

Initialize runspaces with InitialSessionState object #1526

Initialize runspaces with InitialSessionState object #1526

Conversation

andyleejordan commented Jul 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Initialize runspaces with `InitialSessionState` object #1526

Initialize runspaces with `InitialSessionState` object #1526