Skip to content

Honor BundledModulesPath when loading modules in PowerShellContextService #1516

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.

Sign up for GitHub

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

Closed
wants to merge 18 commits into from
Closed

Honor BundledModulesPath when loading modules in PowerShellContextService #1516

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
26818b3
Added BundledModulesPath to HostStartupInfo object and if it is valid…
dkattan Jun 28, 2021
2888045
Fixed s_bundledModulesPath assignment.
dkattan Jun 30, 2021
d0bc52d
Merge branch 'PowerShell:master' into honor-bundledmodulespath-parameter
dkattan Jul 6, 2021
e590fad
Fixed missing BundledModulesPath parameter assignment
dkattan Jul 6, 2021
52b16ca
Restored PowerShellEditorServices.Commands.psd1
dkattan Jul 6, 2021
d8738f0
removed #if TEST from PowerShellContextService.cs and instead provide…
dkattan Jul 6, 2021
ee154b0
removed #if TEST from PowerShellContextService.cs and instead provide…
dkattan Jul 6, 2021
b91382b
Merge branch 'honor-bundledmodulespath-parameter' of github.com:dkatt…
dkattan Jul 6, 2021
77186f0
Added logging to CanGetPSReadLineProxy test
dkattan Jul 6, 2021
48d1a96
Adjusted BundledModulesPath, re-enabled Windows test.
dkattan Jul 6, 2021
7037115
Added more logging to tests, implemented additional mechanism to find…
dkattan Jul 6, 2021
a0fa3c5
Implemented fix to find the non VSS cached location of the executing …
dkattan Jul 6, 2021
1443baa
Implemented fix to find the non VSS cached location of the executing …
dkattan Jul 6, 2021
bda11be
Attempted different fix for relative path to module directory.
dkattan Jul 6, 2021
bda40ea
Cleaned up logging
dkattan Jul 6, 2021
01957e6
Removed TEST property. Moved Set-ExecutionPolicy to before attempted …
dkattan Jul 7, 2021
9a045d9
Fixed formatting in PowerShellEditorServices.build.ps1
dkattan Jul 7, 2021
e1e70cf
Fixed issue where BundledModulePath was _not_ actually getting set.
dkattan Jul 7, 2021
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
3 changes: 2 additions & 1 deletion src/PowerShellEditorServices.Hosting/Internal/EditorServicesRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ private HostStartupInfo CreateHostStartupInfo()
_config.LogPath,
(int)_config.LogLevel,
consoleReplEnabled: _config.ConsoleRepl != ConsoleReplKind.None,
usesLegacyReadLine: _config.ConsoleRepl == ConsoleReplKind.LegacyReadLine);
usesLegacyReadLine: _config.ConsoleRepl == ConsoleReplKind.LegacyReadLine,
bundledModulePath: _config.BundledModulePath);
}

private void WriteStartupBanner()
Expand Down
10 changes: 8 additions & 2 deletions src/PowerShellEditorServices/Hosting/HostStartupInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ public sealed class HostStartupInfo
/// cref="Serilog.Events.LogEventLevel"/>, hence it is an <c>int</c>.
/// </remarks>
public int LogLevel { get; }

/// <summary>
/// The path to find the Bundled Modules ///
/// </summary>
public string BundledModulePath { get; }
#endregion

#region Constructors
Expand Down Expand Up @@ -135,6 +138,7 @@ public sealed class HostStartupInfo
/// <param name="logLevel">The minimum log event level.</param>
/// <param name="consoleReplEnabled">Enable console if true.</param>
/// <param name="usesLegacyReadLine">Use PSReadLine if false, otherwise use the legacy readline implementation.</param>
/// <param name="bundledModulePath">The path to the Modules folder, helps with loading the bundled PSReadLine and other included modules</param>
public HostStartupInfo(
string name,
string profileId,
Expand All @@ -147,7 +151,9 @@ public HostStartupInfo(
string logPath,
int logLevel,
bool consoleReplEnabled,
bool usesLegacyReadLine)
bool usesLegacyReadLine,
string bundledModulePath
)
{
Name = name ?? DefaultHostName;
ProfileId = profileId ?? DefaultHostProfileId;
Expand Down
35 changes: 26 additions & 9 deletions src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,18 @@ namespace Microsoft.PowerShell.EditorServices.Services
/// </summary>
internal class PowerShellContextService : IHostSupportsInteractiveSession
{
private static readonly string s_commandsModulePath = Path.GetFullPath(
private static string s_bundledModulesPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "..", "..");

private static string s_commandsModulePath => Path.GetFullPath(
Path.Combine(
s_bundledModulesPath,
"PowerShellEditorServices",
"Commands",
"PowerShellEditorServices.Commands.psd1"));
private static string _psReadLineModulePath => Path.GetFullPath(
Path.Combine(
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is impetus for the change. We are referencing a path relative to the executing assembly instead of honoring the BundledModulePath parameter.

"../../Commands/PowerShellEditorServices.Commands.psd1"));
s_bundledModulesPath,
"PSReadLine"));

private static readonly Action<Runspace, ApartmentState> s_runspaceApartmentStateSetter;
private static readonly PropertyInfo s_writeStreamProperty;
Expand Down Expand Up @@ -190,7 +198,10 @@ public static PowerShellContextService Create(
HostStartupInfo hostStartupInfo)
{
Validate.IsNotNull(nameof(hostStartupInfo), hostStartupInfo);

s_bundledModulesPath = !string.IsNullOrEmpty(hostStartupInfo.BundledModulePath) && Directory.Exists(hostStartupInfo.BundledModulePath)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here if a valid path was specified, we use it, otherwise we fallback to the previous logic.

? hostStartupInfo.BundledModulePath
: s_bundledModulesPath;

var logger = factory.CreateLogger<PowerShellContextService>();

bool shouldUsePSReadLine = hostStartupInfo.ConsoleReplEnabled
Expand All @@ -215,12 +226,18 @@ public static PowerShellContextService Create(

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

// TODO: This can be moved to the point after the $psEditor object
// gets initialized when that is done earlier than LanguageServer.Initialize
foreach (string module in hostStartupInfo.AdditionalModules)
var modulesToImport = new List<string>();
if(hostStartupInfo.ConsoleReplEnabled)
{
modulesToImport.Add(_psReadLineModulePath);
}
modulesToImport.AddRange(hostStartupInfo.AdditionalModules);
foreach (string module in modulesToImport)
{
var command =
new PSCommand()
Expand Down Expand Up @@ -305,7 +322,7 @@ public static Runspace CreateRunspace(PSHost psHost, PSLanguageMode languageMode
/// <param name="ownsInitialRunspace">If true, the PowerShellContext owns this runspace.</param>
/// <param name="consoleHost">An IHostOutput implementation. Optional.</param>
public void Initialize(
ProfilePathInfo profilePaths,
HostStartupInfo hostStartupInfo,
Runspace initialRunspace,
bool ownsInitialRunspace,
IHostOutput consoleHost)
Expand Down Expand Up @@ -359,7 +376,7 @@ public void Initialize(
this.ConfigureRunspaceCapabilities(this.CurrentRunspace);

// Set the $profile variable in the runspace
this.profilePaths = profilePaths;
this.profilePaths = hostStartupInfo.ProfilePaths;
if (profilePaths != null)
{
this.SetProfileVariableInCurrentRunspace(profilePaths);
Expand Down Expand Up @@ -396,7 +413,7 @@ public void Initialize(

if (powerShellVersion.Major >= 5 &&
this.isPSReadLineEnabled &&
PSReadLinePromptContext.TryGetPSReadLineProxy(logger, initialRunspace, out PSReadLineProxy proxy))
PSReadLinePromptContext.TryGetPSReadLineProxy(logger, initialRunspace, hostStartupInfo.BundledModulePath, out PSReadLineProxy proxy))
{
this.PromptContext = new PSReadLinePromptContext(
this,
Expand Down
16 changes: 2 additions & 14 deletions src/PowerShellEditorServices/Services/PowerShellContext/Session/PSReadLinePromptContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,6 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShellContext

internal class PSReadLinePromptContext : IPromptContext
{
private static readonly string _psReadLineModulePath = Path.Combine(
Path.GetDirectoryName(typeof(PSReadLinePromptContext).Assembly.Location),
"..",
"..",
"..",
#if TEST
// When using xUnit (dotnet test) the assemblies are deployed to the
// test project folder, invalidating our relative path assumption.
"..",
"..",
"module",
#endif
"PSReadLine");

private static readonly Lazy<CmdletInfo> s_lazyInvokeReadLineForEditorServicesCmdletInfo = new Lazy<CmdletInfo>(() =>
{
var type = Type.GetType("Microsoft.PowerShell.EditorServices.Commands.InvokeReadLineForEditorServicesCommand, Microsoft.PowerShell.EditorServices.Hosting");
Expand Down Expand Up @@ -79,9 +65,11 @@ internal PSReadLinePromptContext(
internal static bool TryGetPSReadLineProxy(
ILogger logger,
Runspace runspace,
string bundledModulePath,
out PSReadLineProxy readLineProxy)
{
readLineProxy = null;
string _psReadLineModulePath = Path.Combine(bundledModulePath, "PSReadLine");
logger.LogTrace("Attempting to load PSReadLine");
using (var pwsh = PowerShell.Create())
{
Expand Down
8 changes: 6 additions & 2 deletions test/PowerShellEditorServices.Test/PowerShellContextFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ internal static class PowerShellContextFactory
TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/Test.PowerShellEditorServices_profile.ps1")),
Path.GetFullPath(
TestUtilities.NormalizePath("../../../../PowerShellEditorServices.Test.Shared/ProfileTest.ps1")));
public static readonly string BundledModulesPath =
Path.GetFullPath(
TestUtilities.NormalizePath("../../../../../module"));

public static System.Management.Automation.Runspaces.Runspace initialRunspace;

Expand All @@ -50,7 +53,8 @@ public static PowerShellContextService Create(ILogger logger)
null,
0,
consoleReplEnabled: false,
usesLegacyReadLine: false);
usesLegacyReadLine: false,
bundledModulePath: BundledModulesPath);

initialRunspace = PowerShellContextService.CreateRunspace(
testHostDetails,
Expand All @@ -59,7 +63,7 @@ public static PowerShellContextService Create(ILogger logger)
logger);

powerShellContext.Initialize(
TestProfilePaths,
testHostDetails,
initialRunspace,
ownsInitialRunspace: true,
consoleHost: null);
Expand Down
16 changes: 13 additions & 3 deletions test/PowerShellEditorServices.Test/Session/PowerShellContextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.IO;
using System.Linq;
using System.Management.Automation;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging.Abstractions;
Expand Down Expand Up @@ -152,10 +153,19 @@ await this.powerShellContext.ExecuteCommandAsync<string>(
public async Task CanGetPSReadLineProxy()
{
Skip.If(IsWindows, "This test doesn't work on Windows for some reason.");
string s_bundledModulesPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location),
"..",
"..",
"..",
"..",
"module"
);
System.Console.WriteLine($"Attempting to import {s_bundledModulesPath}");
Copy link
Member

Choose a reason for hiding this comment

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

Just FYI I am debugging this test on Windows right now, really want to know why it worked everywhere but there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, me too. I think this last change fixed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this PR should be the first to go in. It made sense to bring in the Changes from #1520 so I'll close that PR as this handles both issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah maybe not, PowerShell 7 | Windows 10 has already failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

macOS:
executingAssembly /Users/runner/work/1/s/test/PowerShellEditorServices.Test/bin/Debug/netcoreapp3.1/Microsoft.PowerShell.EditorServices.Test.dll

Windows: executingAssembly C:\Users\VssAdministrator\AppData\Local\Temp\b5b3251f-c622-45f2-bc10-c8a646797337\b5b3251f-c622-45f2-bc10-c8a646797337\assembly\dl3\e3727c32\64017e66_b772d701\Microsoft.PowerShell.EditorServices.Test.dll

https://stackoverflow.com/questions/8309411/what-is-cache-appdata-local-assembly-dl3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Evidently we shouldn't be using GetExecutingAssembly().Location as this can return a Volume Shadow Copy cached version of the DLL. We're using it in 9 other places...

Code File Line
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); test\PowerShellEditorServices.Test.E2E\DebugAdapterProtocolMessageTests.cs 26
: Path.GetDirectoryName(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location)); src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs 302
Assembly.GetExecutingAssembly().Location, src\PowerShellEditorServices.Hosting\Commands\StartEditorServicesCommand.cs 344
private static readonly string s_psesBaseDirPath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); src\PowerShellEditorServices.Hosting\EditorServicesLoader.cs 34
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), src\PowerShellEditorServices.Hosting\EditorServicesLoader.cs 39
private static string s_bundledModulesPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "..", ".."); src\PowerShellEditorServices\Services\PowerShellContext\PowerShellContextService.cs 35
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); test\PowerShellEditorServices.Test.E2E\LanguageServerProtocolMessageTests.cs 36
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); test\PowerShellEditorServices.Test.E2E\LSPTestsFixures.cs 31
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location); test\PowerShellEditorServices.Test.E2E\Processes\PsesStdioProcess.cs 19

Assert.True(PSReadLinePromptContext.TryGetPSReadLineProxy(
NullLogger.Instance,
PowerShellContextFactory.initialRunspace,
out PSReadLineProxy proxy));
NullLogger.Instance,
PowerShellContextFactory.initialRunspace,
s_bundledModulesPath,
out PSReadLineProxy proxy));
}

#region Helper Methods
Expand Down