Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 0 additions & 27 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,33 +118,6 @@ 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.
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;

// 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);
}
else if (!path.StartsWith(pshome, StringComparison.Ordinal))
{
Environment.SetEnvironmentVariable("PATH", pshome + path);
}

try
{
string profileDir = Platform.CacheDirectory;
Expand Down
141 changes: 103 additions & 38 deletions src/System.Management.Automation/engine/CommandDiscovery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,53 @@ internal class CommandDiscovery

#region ctor

/// <summary>
/// The directory path where the currently running pwsh executable is located.
/// </summary>
private static readonly string _psExeHome;

static CommandDiscovery()
{
#if UNIX
string pwshName = "pwsh";
#else
string pwshName = "pwsh.exe";
#endif

string processPath = Environment.ProcessPath;
if (pwshName.Equals(Path.GetFileName(processPath), StringComparison.Ordinal))
{
// Use 'Environment.ProcessPath' if it points to 'pwsh.exe' or 'pwsh'.
_psExeHome = Path.GetDirectoryName(processPath);
return;
}

// We need to handle 3 cases here:
// - We are running from PowerShell dotnet global tool ('ProcessPath' points to 'dotnet' in this case).
// - We are running from an application that hosts PowerShell via NuGet packages.
// - Rare case where we are running from pwsh, but 'ProcessPath' somehow doesn't point to it.
_psExeHome = Utils.DefaultPowerShellAppBase;

// Check if the pwsh executable exists in pshome.
string exePath = Path.Combine(_psExeHome, pwshName);
if (!File.Exists(exePath))
{
// This means PowerShell is being hosted via NuGet packages.
_psExeHome = null;
return;
}

string dotnetToolPathSegment = string.Format("{0}.store{0}powershell{0}", Path.DirectorySeparatorChar);
int index = _psExeHome.IndexOf(dotnetToolPathSegment, 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', because
// the pwsh executable under $PSHOME is an x86-64 binary that won't work on non-x86/64 platforms.
_psExeHome = _psExeHome[0..index];
}
}

/// <summary>
/// Default constructor...
/// </summary>
Expand Down Expand Up @@ -1199,63 +1246,86 @@ internal void UnregisterLookupCommandInfoAction(string currentAction, string com
/// </remarks>
internal LookupPathCollection GetLookupDirectoryPaths()
{
LookupPathCollection result = new LookupPathCollection();

string path = Environment.GetEnvironmentVariable("PATH");
discoveryTracer.WriteLine("PATH: {0}", path);

discoveryTracer.WriteLine(
"PATH: {0}",
path);

bool isPathCacheValid =
path != null &&
string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase) &&
_cachedPath != null;
bool isPathCacheValid = _cachedLookupPaths is not null
&& string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase);

if (!isPathCacheValid)
{
// Reset the cached lookup paths
_pathCacheKey = null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  1. Is isPathCacheValid evaluated correctly now?
  2. If we evaluate _psHome only for saving in _cachedLookupPaths why do we save it permanently in field?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For 1, I think it's correct now.
For 2, _cachedLookupPaths will be invalidated and reconstructed when the PATH env var is changed, but we don't want to recalculate _psHome in that case.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For 2, the question is rather what if we search for pshome throughout the source code, there is more than one place where it is used according to the old pattern. I.e., I would expect that pshome should be cached in Utils.DefaultPowerShellAppBase, taking into account regular, dotnet tool and hosted scenarios.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I renamed the _psHome variable to _psExeHome to differentiate it from the $PSHOME concept.

_psExeHome is the directory that contains the pwsh executable, which is not necessarily under $PSHOME. For example, the built-in pwsh in the .NET SDK docker container image is located at C:\Program Files\powershell for Windows Server Core, and $PSHOME\pwsh.exe doesn't exist.

The only place I plan to update next is the PwshExePath in PowerShellProcessInstance. The current code is incorrect. I plan to reuse what I have here in the CommandDiscovery, in a separate PR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

Perhaps the best thing that could be done is to have a common code for each scenario somewhere in Utils. (Actually, it was originally like that once.)

_cachedLookupPaths = null;

// Tokenize the path and cache it

_pathCacheKey = path;
bool isPwshExe = _psExeHome is not null;
discoveryTracer.WriteLine(isPwshExe
? "PWSH scenario: Prepend PWSH-EXE home directory to searching paths."
: "NuGet Package scenario: Use PATH as is.");

if (_pathCacheKey != null)
if (path is null)
{
// Cache an empty collection when PATH is null (unset).
_cachedLookupPaths = new LookupPathCollection();
if (isPwshExe)
{
// Add '_psExeHome' when we are executing from a pwsh executable.
_cachedLookupPaths.Add(_psExeHome);
}
}
else
{
// Tokenize the path and cache it
_pathCacheKey = path;
string[] tokenizedPath = _pathCacheKey.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries);
_cachedPath = new Collection<string>();

List<string> pathList = new(capacity: tokenizedPath.Length + 1);
ReadOnlySpan<char> psHomeSpan = default;

if (isPwshExe)
{
// Add '_psExeHome' to the front of the path list so that calling `pwsh` within `pwsh` always starts the same running version.
pathList.Add(_psExeHome);
psHomeSpan = _psExeHome.AsSpan().TrimEnd(Path.DirectorySeparatorChar);
}

foreach (string directory in tokenizedPath)
{
string tempDir = directory.TrimStart();
if (tempDir.EqualsOrdinalIgnoreCase("~"))
string tempDir = directory.Trim();
if (tempDir.StartsWith('~'))
{
tempDir = Environment.GetFolderPath(
string homeDir = Environment.GetFolderPath(
Environment.SpecialFolder.UserProfile,
Environment.SpecialFolderOption.DoNotVerify);

if (tempDir.Length is 1)
{
tempDir = homeDir;
}
else if (tempDir[1] == Path.DirectorySeparatorChar)
{
tempDir = $"{homeDir}{Path.DirectorySeparatorChar}{tempDir.Substring(2)}";
}
}
else if (tempDir.StartsWith("~" + Path.DirectorySeparatorChar))

// Skip the duplicate path if it is the same as '_psExeHome' and we are running from a pwsh executable.
if (isPwshExe && tempDir.Length >= psHomeSpan.Length)
{
tempDir = Environment.GetFolderPath(
Environment.SpecialFolder.UserProfile,
Environment.SpecialFolderOption.DoNotVerify)
+ Path.DirectorySeparatorChar
+ tempDir.Substring(2);
ReadOnlySpan<char> tempDirSpan = tempDir.AsSpan().TrimEnd(Path.DirectorySeparatorChar);
if (psHomeSpan.Equals(tempDirSpan, StringComparison.OrdinalIgnoreCase))
{
continue;
}
}

_cachedPath.Add(tempDir);
result.Add(tempDir);
pathList.Add(tempDir);
}

// Cache the new lookup paths.
_cachedLookupPaths = new LookupPathCollection(pathList);
}
}
else
{
result.AddRange(_cachedPath);
}

// Cache the new lookup paths
return _cachedLookupPaths ??= result;
return _cachedLookupPaths;
}

/// <summary>
Expand All @@ -1269,11 +1339,6 @@ internal LookupPathCollection GetLookupDirectoryPaths()
/// </summary>
private string _pathCacheKey;

/// <summary>
/// The cache of the tokenized PATH directories.
/// </summary>
private Collection<string> _cachedPath;

#endregion internal members

#region environment variable helpers
Expand Down
15 changes: 10 additions & 5 deletions test/powershell/Host/ConsoleHost.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,8 @@ $powershell -c '[System.Management.Automation.Platform]::SelectProductNameForDir
}

Context "PATH environment variable" {
It "`$PSHOME should be in front so that pwsh.exe starts current running PowerShell" {
& $powershell -v | Should -Match $PSVersionTable.GitCommitId
It "Running 'pwsh' should start the currently running PowerShell" {
pwsh -v | Should -Match $PSVersionTable.GitCommitId
}

It "powershell starts if PATH is not set" -Skip:($IsWindows) {
Expand Down Expand Up @@ -1191,10 +1191,15 @@ Describe 'Pwsh startup and PATH' -Tag CI {
}

It 'pwsh starts even if PATH is not defined' {
$pwsh = Join-Path -Path $PSHOME -ChildPath "pwsh"
Remove-Item Env:\PATH
$path = & $pwsh -noprofile -command '$env:PATH'
$path | Should -BeExactly ($PSHOME + [System.IO.Path]::PathSeparator)

$version = pwsh -v
$version | Should -BeExactly "PowerShell $($PSVersionTable.GitCommitId)"
}

It 'pwsh should not alter the PATH environment variable during startup' {
Remove-Item Env:\PATH
pwsh -noprofile -command '$null -eq $env:PATH' | Should -BeExactly 'True'
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ Describe "Environment Tests" -Tags "Feature" {
($out | Where-Object { $_.Name -eq 'TERM' }).Value | Should -BeExactly 'dumb'
$pathSeparator = [System.IO.Path]::PathSeparator
if ($IsWindows) {
($out | Where-Object { $_.Name -eq 'PATH' }).Value | Should -BeLike "*${pathSeparator}mine${pathSeparator}*"
($out | Where-Object { $_.Name -eq 'PATH' }).Value | Should -BeLike "mine${pathSeparator}*"
} else {
($out | Where-Object { $_.Name -eq 'PATH' }).Value | Should -BeLike "*${pathSeparator}mine"
($out | Where-Object { $_.Name -eq 'PATH' }).Value | Should -BeLike "mine"
}
}

Expand Down
Loading