-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Handle MSIX installation specially when prepend to PATH #27782
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,31 +118,21 @@ internal static int Start( | |
| throw new ConsoleHostStartupException(ConsoleHostStrings.ShellCannotBeStartedWithConfigConflict); | ||
| } | ||
|
|
||
| // Put PSHOME in front of PATH so that calling `pwsh` within `pwsh` always starts the same running version. | ||
| // Put pwsh executable home in front of PATH so that calling `pwsh` within `pwsh` always starts the same running version. | ||
| string psExeHome = GetPSExecutableHome(); | ||
| string path = Environment.GetEnvironmentVariable("PATH"); | ||
| string pshome = Utils.DefaultPowerShellAppBase; | ||
| string dotnetToolsPathSegment = $"{Path.DirectorySeparatorChar}.store{Path.DirectorySeparatorChar}powershell{Path.DirectorySeparatorChar}"; | ||
|
|
||
| int index = pshome.IndexOf(dotnetToolsPathSegment, StringComparison.Ordinal); | ||
| if (index > 0) | ||
| { | ||
| // We're running PowerShell global tool. In this case the real entry executable should be the 'pwsh' | ||
| // or 'pwsh.exe' within the tool folder which should be the path right before the '\.store', not what | ||
| // PSHome is pointing to. | ||
| pshome = pshome[0..index]; | ||
| } | ||
|
|
||
| pshome += Path.PathSeparator; | ||
| psExeHome += Path.PathSeparator; | ||
|
|
||
| // To not impact startup perf, we don't remove duplicates, but we avoid adding a duplicate to the front | ||
| // we also don't handle the edge case where PATH only contains $PSHOME | ||
| if (string.IsNullOrEmpty(path)) | ||
| { | ||
| Environment.SetEnvironmentVariable("PATH", pshome); | ||
| Environment.SetEnvironmentVariable("PATH", psExeHome); | ||
| } | ||
| else if (!path.StartsWith(pshome, StringComparison.Ordinal)) | ||
| else if (!path.StartsWith(psExeHome, StringComparison.Ordinal)) | ||
| { | ||
| Environment.SetEnvironmentVariable("PATH", pshome + path); | ||
| Environment.SetEnvironmentVariable("PATH", psExeHome + path); | ||
| } | ||
|
|
||
| try | ||
|
|
@@ -380,6 +370,43 @@ internal static void ParseCommandLine(string[] args) | |
|
|
||
| private static readonly CommandLineParameterParser s_cpp = new CommandLineParameterParser(); | ||
|
|
||
| private static string GetPSExecutableHome() | ||
| { | ||
| #if UNIX | ||
| const string pwshName = "pwsh"; | ||
| const string dotnetToolPathSegment = "/.store/powershell/"; | ||
| #else | ||
| const string pwshName = "pwsh.exe"; | ||
| const string dotnetToolPathSegment = @"\.store\powershell\"; | ||
| #endif | ||
|
|
||
| string psExePath = Environment.ProcessPath; | ||
| string psExeHome = Path.GetDirectoryName(psExePath); | ||
| string processName = Path.GetFileName(psExePath); | ||
|
daxian-dbw marked this conversation as resolved.
|
||
|
|
||
| // Use 'Environment.ProcessPath' if it points to 'pwsh.exe' or 'pwsh'. | ||
| if (pwshName.Equals(processName, StringComparison.Ordinal)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Environment.ProcessPath return real file name from file system.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My screenshot shows you that's not the case. The Most tools will probably normalize it but you can't guarantee it'll always match the FS on Windows as anything can call
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. I will submit a follow-up PR to correct the comparison on Windows. |
||
| { | ||
| #if !UNIX | ||
| psExeHome = ResolveStablePathIfMsix(psExeHome); | ||
| #endif | ||
| return psExeHome; | ||
| } | ||
|
|
||
| psExeHome = Utils.DefaultPowerShellAppBase; | ||
|
|
||
| int index = psExeHome.IndexOf(dotnetToolPathSegment, StringComparison.Ordinal); | ||
| if (index > 0) | ||
| { | ||
| // We're running PowerShell dotnet tool. In this case the real entry executable should be the 'pwsh' | ||
| // or 'pwsh.exe' within the tool folder which should be the path right before the '\.store', because | ||
| // the pwsh executable under $PSHOME is an x86-64 binary that won't work on non-x86/64 platforms. | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| return psExeHome[0..index]; | ||
| } | ||
|
|
||
| return psExeHome; | ||
| } | ||
|
|
||
| #if UNIX | ||
| /// <summary> | ||
| /// The break handler for the program. Dispatches a break event to the current Executor. | ||
|
|
@@ -409,6 +436,49 @@ private static void MyBreakHandler(object sender, ConsoleCancelEventArgs args) | |
| } | ||
| } | ||
| #else | ||
| /// <summary> | ||
| /// Handle the MSIX package scenario where <paramref name="psExeHome"/> points to the MSIX package folder under "Program Files". | ||
| /// | ||
| /// That path contains a version string and will change with every update. Prepending that path to the PATH environment variable | ||
| /// caused a problem for CMake-based build systems: CMake cached the path to 'pwsh.exe' when running for the first time from the | ||
| /// MSIX PowerShell. That cached path became invalid after the MSIX PowerShell was updated, which broke CMake. | ||
| /// | ||
| /// So, instead of using the "Program Files" package folder path, we need to use the stable path that contains the execution alias | ||
| /// for the specific MSIX package, e.g. use "%LOCALAPPDATA%\Microsoft\WindowsApps\Microsoft.PowerShell_8wekyb3d8bbwe" instead of | ||
| /// "%ProgramFiles%\WindowsApps\Microsoft.PowerShell_7.x.x.0_x64__8wekyb3d8bbwe". | ||
| /// </summary> | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| /// <param name="psExeHome">Path to the directory that contains the pwsh executable.</param> | ||
| private static string ResolveStablePathIfMsix(string psExeHome) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems a bit unwise to hardcode a lot of these checks to specific publishers and paths. Not only does this stop someone from packaging their own MSIX of their PowerShell fork without changing this code it also stops you from installing the MSIX package into a custom volume. Granted the latter still surfaces as being run under Wouldn't a better idea to instead see if you can call GetCurrentPackageFamilyName to see if 1 the package has a package identity associated with it (is an MSIX package) and also get the family name for the later For example take this pwsh script as a POC $APPMODEL_ERROR_NO_PACKAGE = 15700
$k32 = New-CtypesLib Kernel32.dll
$l = 0
$b = [Text.StringBuilder]::new()
$res = $k32.CharSet('Unicode').GetCurrentPackageFamilyName([ref]$l, $b)
if ($res -eq $APPMODEL_ERROR_NO_PACKAGE) {
"PSHome = $PSHome"
}
else {
$null = $b.EnsureCapacity($l)
$null = $k32.GetCurrentPackageFamilyName([ref]$l, $b)
$familyId = $b.ToString()
"PSHome = $env:LocalAppData\Microsoft\WindowsApps\$familyId"
}
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I had the same question, but they clearly aren’t keen on supporting any alternative distributors in any form.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. About "installing the MSIX package into a custom volume", Ilya (@iSazonov) and I had this discussion in #27782 (comment), and I don't find a way to install the MSIX PowerShell to a different drive.
The idea to do it with the path check was to:
If it turns out the path check is not sufficient, we can always get back to the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's certainly possible but luckily when I tested it, Windows goes to some lengths to pretend it's still under If you are interested, you can install an msix package to another volume by using the For example I have an msixbundle of 7.6.6 and a separate volume Add-AppxVolume -Path D:
Add-AppxPackage -Path .\Downloads\PowerShell-7.6.6.msixbundle -Volume D:Once installed the package metadata still points to
I can see the concerns around startup time, I have not measured the cost of calling
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you Jordan Borean (@jborean93) for going extra mile to try installing the MSIX package to a different volume. Copilot couldn't find current Microsoft documentation that explicitly promises, as a public contract, that a package installed to a secondary AppX volume will always have So, I guess I just have to get back to the |
||
| { | ||
| const string msixPublisherSuffix = "_8wekyb3d8bbwe"; | ||
| const string msixPackageBaseName = "Microsoft.PowerShell"; | ||
|
|
||
| if (psExeHome.EndsWith(msixPublisherSuffix, StringComparison.Ordinal)) | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| { | ||
| string programFileDir = Environment.GetFolderPath( | ||
| Environment.SpecialFolder.ProgramFiles, | ||
| Environment.SpecialFolderOption.DoNotVerify); | ||
|
|
||
| string prefix = $"{programFileDir}\\WindowsApps\\{msixPackageBaseName}"; | ||
| if (psExeHome.StartsWith(prefix, StringComparison.Ordinal)) | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| { | ||
| int startIndex = prefix.Length; | ||
| int underbarIndex = psExeHome.IndexOf('_', startIndex); | ||
| if (underbarIndex > 0) | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| { | ||
| ReadOnlySpan<char> channelSuffix = psExeHome.AsSpan(startIndex, underbarIndex - startIndex); | ||
|
daxian-dbw marked this conversation as resolved.
|
||
| string localAppDataDir = Environment.GetFolderPath( | ||
| Environment.SpecialFolder.LocalApplicationData, | ||
| Environment.SpecialFolderOption.DoNotVerify); | ||
|
|
||
| psExeHome = $"{localAppDataDir}\\Microsoft\\WindowsApps\\{msixPackageBaseName}{channelSuffix}{msixPublisherSuffix}"; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return psExeHome; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// The break handler for the program. Dispatches a break event to the current Executor. | ||
| /// </summary> | ||
|
|
||




Uh oh!
There was an error while loading. Please reload this page.