Honor BundledModulesPath when loading modules in PowerShellContextService #1516
Conversation
|
I like this approach a lot better than my own, which was a bit of a hack. |
dkattan
changed the title
| Path.Combine( | ||
| Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), |
There was a problem hiding this comment.
This is impetus for the change. We are referencing a path relative to the executing assembly instead of honoring the BundledModulePath parameter.
| @@ -190,7 +198,10 @@ public RunspaceDetails CurrentRunspace | |||
| HostStartupInfo hostStartupInfo) | |||
| { | |||
| Validate.IsNotNull(nameof(hostStartupInfo), hostStartupInfo); | |||
|
|
|||
| s_bundledModulesPath = !string.IsNullOrEmpty(hostStartupInfo.BundledModulePath) && Directory.Exists(hostStartupInfo.BundledModulePath) | |||
There was a problem hiding this comment.
Here if a valid path was specified, we use it, otherwise we fallback to the previous logic.
…d the path as a parameter to PowerShellContextService.TryGetPSReadLineProxy()
…d the path as a parameter to PowerShellContextService.TryGetPSReadLineProxy()
…an/PowerShellEditorServices into honor-bundledmodulespath-parameter
| @@ -160,6 +160,7 @@ public async Task CanGetPSReadLineProxy() | |||
| "..", | |||
| "module" | |||
| ); | |||
| System.Console.WriteLine($"Attempting to import {s_bundledModulesPath}"); | |||
There was a problem hiding this comment.
Just FYI I am debugging this test on Windows right now, really want to know why it worked everywhere but there.
There was a problem hiding this comment.
Hah, me too. I think this last change fixed it.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ah maybe not, PowerShell 7 | Windows 10 has already failed.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
| var type = assemblies?.ExportedTypes?.FirstOrDefault(a => a.FullName == "Microsoft.PowerShell.PSConsoleReadLine"); | ||
| if(type is not null) | ||
| { | ||
| System.Console.WriteLine("Found PSConsoleReadLine in loaded assemblies."); |
There was a problem hiding this comment.
@andschwa I didn't think this was going to be necessary, but upon inspecting the logs from the most recent build, it appears it is:
https://dev.azure.com/powershell/PowerShellEditorServices/_build/results?buildId=81345&view=logs&j=5aed7330-0c52-5e0f-84aa-a17a2c59b4c4&t=c5143455-bea5-57f2-b80c-f2c098a4d98c&l=928
Attempting to import D:\a\1\s\module\PSReadLine
psReadLineType is null, searching loaded assemblies...
Found PSConsoleReadLine in loaded assemblies.
| var type = assemblies?.ExportedTypes?.FirstOrDefault(a => a.FullName == "Microsoft.PowerShell.PSConsoleReadLine"); | ||
| if(type is not null) | ||
| { | ||
| logger.LogInformation("Found PSConsoleReadLine in loaded assemblies."); |
There was a problem hiding this comment.
In your testing has it ever been found in the loaded assemblies?
|
This was corrected in #1522 |
No description provided.