Skip to content

Conversation

@SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Jul 22, 2024

PR Summary

PowerShell uses the SMA.dll file location to determine it's base location. However, if a hosted app is built as a single exe, that location is empty which causes PowerShell init to fail.

Change is if SMA.dll file location is empty, use the host exe location instead.

Tested manually building pwsh as singleexe.

PR Context

Fix #24070
Fix #23797

PR Checklist

@SteveL-MSFT SteveL-MSFT changed the title Change getting PowerShell base directory to exe host directory instead of SMA.dll location Change getting PowerShell base directory to exe host directory first instead of SMA.dll location Jul 22, 2024
@iSazonov
Copy link
Collaborator

For information comment from @fMichaleczek #13540 (comment)

@fMichaleczek
Copy link
Contributor

@iSazonov For Browser (Wasm), I will use a native preprocessor (#if BROWSER) to escape from the standard behavior.

@daxian-dbw
Copy link
Member

daxian-dbw commented Aug 5, 2024

@SteveL-MSFT Can you take another look at #18674 and see if all the problems listed there are addressed in this PR?
The main blocking thing that I can tell is how to ship the modules for Win/Unix respectively in the Microsoft.PowerShell.SDK NuGet package: #18674 (comment)

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Aug 12, 2024

@SteveL-MSFT Can you take another look at #18674 and see if all the problems listed there are addressed in this PR? The main blocking thing that I can tell is how to ship the modules for Win/Unix respectively in the Microsoft.PowerShell.SDK NuGet package: #18674 (comment)

Change of thought, I'll update the PR is that if SMA.dll location is valid, we'll use that otherwise fall through to host location. This should resolve the app compat concerns.

@SteveL-MSFT SteveL-MSFT added WG-Engine core PowerShell engine, interpreter, and runtime WG-NeedsReview Needs a review by the labeled Working Group labels Aug 12, 2024
@SteveL-MSFT SteveL-MSFT changed the title Change getting PowerShell base directory to exe host directory first instead of SMA.dll location If SMA.dll location is not found, use host exe to determine $PSHOME location Aug 13, 2024
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Aug 13, 2024
@fMichaleczek
Copy link
Contributor

fMichaleczek commented Aug 13, 2024

Change of thought, I'll update the PR is that if SMA.dll location is valid, we'll use that otherwise fall through to host location. This should resolve the app compat concerns.

The problem with Hosting is when Assembly.Location is null or empty (I dont rembember which one).
I dont ever seen a wrong Assembly.Location (but I may be wrong)

PS > [System.IO.Path]::GetDirectoryName([string]::Empty)
Exception calling "GetDirectoryName" with "1" argument(s): "The path is not of a legal form."
var smaAssembly = typeof(PSObject).Assembly;
if (!string.IsNullOrEmpty(smaAssembly.Location))
{
    return Path.GetDirectoryName(smaAssembly.Location);
}
return AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar);

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Aug 13, 2024

I built a custom single-exe of pwsh and 81 tests fail specifically because SMA.dll no longer exists (it's inside the exe).

PS> [psobject].Assembly | fl *

CodeBase            :
FullName            : System.Management.Automation, Version=7.5.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35
EntryPoint          :
DefinedTypes        : {<>f__AnonymousType0`2[<cacheEntry>j__TPar,<splittedName>j__TPar], <>f__AnonymousType1`2[<<>h__TransparentIdentifier0>j__TPar,<cachedClassName>j__TPar],
                      <>f__AnonymousType2`2[<<>h__TransparentIdentifier1>j__TPar,<cachedModuleName>j__TPar],
                      <>f__AnonymousType3`2[<<>h__TransparentIdentifier2>j__TPar,<cachedResourceName>j__TPar]…}
IsCollectible       : False
ManifestModule      : System.Management.Automation.dll
ReflectionOnly      : False
Location            :
ImageRuntimeVersion : v4.0.30319
GlobalAssemblyCache : False
HostContext         : 0
IsDynamic           : False
ExportedTypes       : {System.Management.Automation.PowerShellAssemblyLoadContextInitializer, System.Management.Automation.PowerShellUnsafeAssemblyLoad,
                      System.Management.Automation.Platform, System.Management.Automation.PSTransactionContext…}
IsFullyTrusted      : True
CustomAttributes    : {[System.Runtime.CompilerServices.ExtensionAttribute()], [System.Runtime.CompilerServices.CompilationRelaxationsAttribute((Int32)8)],
                      [System.Runtime.CompilerServices.RuntimeCompatibilityAttribute(WrapNonExceptionThrows = True)],
                      [System.Diagnostics.DebuggableAttribute((System.Diagnostics.DebuggableAttribute+DebuggingModes)263)]…}
EscapedCodeBase     :
Modules             : {<Unknown>}
SecurityRuleSet     : None

Note that even with 81 failures, 12310 tests passed. More work will be needed to get a single-exe to actually work, but it's not clear to me if there's value of this for PS7 itself vs hosted apps.

@iSazonov
Copy link
Collaborator

Note that even with 81 failures, 12310 tests passed. More work will be needed to get a single-exe to actually work, but it's not clear to me if there's value of this for PS7 itself vs hosted apps.

More recently, things were much worse there. We did not make any efforts in this direction, nevertheless, the situation has become much better!
I don't know where else there are problems, but maybe they are not so difficult to remove them over time.
It's a question for the committee whether we need to support this single-exe scenario.

@fMichaleczek
Copy link
Contributor

fMichaleczek commented Aug 17, 2024

@iSazonov In any case, PowerShell's internal APIs need to be made "FileLess" compatible.

On Browser, we are in the extreme case where API like Environment.ProcessPath is empty.
I have used default BROWSER preprocessor directive to set $PSHome to an arbitrary path /opt/microsoft/powershell,
because I need a fixed path in the boot json

image

image
image

@iSazonov
Copy link
Collaborator

@iSazonov In any case, PowerShell's internal APIs need to be made "FileLess" compatible.

I tried that but that path was rejected. Since we have a progress we're on that path, but implicitly 😄. We're closest to realizing the single exe scenario. "Browser" scenario is more difficult to implement. And I honestly don't understand why we need to load the pwsh engine in the browser (Compare with Windows PowerShell Web Access/Console).

@fMichaleczek
Copy link
Contributor

We're closest to realizing the single exe scenario.

+2 important needed fixes :

return Path.GetDirectoryName(cmdletInfo.ImplementingType.Assembly.Location);

(Assembly.GetEntryAssembly() ?? typeof(PSObject).Assembly).Location),

And I honestly don't understand why we need to load the pwsh engine in the browser (Compare with Windows PowerShell Web Access/Console).

Be hosted on github pages for free. (and make free tools)

@SteveL-MSFT
Copy link
Member Author

We should probably do this iteratively. The main priority for me is enabling hosted single-exe scenarios. pwsh itself as single-exe I don't think is that interesting (other than academically since we can't trim any of .NET which is needed for scripts) and eventually enable web hosted WASM.

@daxian-dbw daxian-dbw enabled auto-merge (squash) August 19, 2024 22:08
@daxian-dbw daxian-dbw merged commit 5eacbad into PowerShell:master Aug 19, 2024
@SteveL-MSFT SteveL-MSFT deleted the app-base branch August 19, 2024 23:25
@daxian-dbw daxian-dbw removed WG-Engine core PowerShell engine, interpreter, and runtime WG-NeedsReview Needs a review by the labeled Working Group labels Aug 20, 2024
chrisdent-de pushed a commit to chrisdent-de/PowerShell that referenced this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

4 participants