Skip to content

Refactor the module path construction code to make it more robust and easier to maintain #26565

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

Merged
daxian-dbw merged 9 commits into from
Dec 12, 2025
Merged

Refactor the module path construction code to make it more robust and easier to maintain #26565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
114803b
Print the stack trace of the ShellCannotBeStarted error
daxian-dbw Dec 3, 2025
ad1aa09
Add more debug messages
daxian-dbw Dec 3, 2025
64acf5c
Guard the debug message with env var
daxian-dbw Dec 4, 2025
866ee21
Refactor the module path construction code to make it more robust and…
daxian-dbw Dec 9, 2025
96e7a32
Address comments from Copilot
daxian-dbw Dec 10, 2025
45c1d4c
Address comments and check if a path already exists more efficiently
daxian-dbw Dec 10, 2025
fd2ce82
Update the XML doc for a parameter based on Copilot's feedback
daxian-dbw Dec 10, 2025
5610fc3
Address Ilya's comments
daxian-dbw Dec 11, 2025
2af8081
Address a comment
daxian-dbw Dec 12, 2025
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
21 changes: 18 additions & 3 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2201,7 +2201,6 @@ private void ReportException(Exception e, Executor exec)
if (e1 != null)
{
// that didn't work. Write out the error ourselves as a last resort.

ReportExceptionFallback(e, null);
}
}
Expand All @@ -2222,15 +2221,17 @@ private void ReportExceptionFallback(Exception e, string header)
Console.Error.WriteLine(header);
}

if (e == null)
if (e is null)
{
return;
}

// See if the exception has an error record attached to it...
ErrorRecord er = null;
if (e is IContainsErrorRecord icer)
{
er = icer.ErrorRecord;
}

if (e is PSRemotingTransportException)
{
Expand All @@ -2247,8 +2248,22 @@ private void ReportExceptionFallback(Exception e, string header)
}

// Add the position message for the error if it's available.
if (er != null && er.InvocationInfo != null)
if (er?.InvocationInfo is { })
{
Console.Error.WriteLine(er.InvocationInfo.PositionMessage);
}

// Print the stack trace.
Console.Error.WriteLine($"\n--- {e.GetType().FullName} ---");
Console.Error.WriteLine(e.StackTrace);
Comment thread
daxian-dbw marked this conversation as resolved.

Exception inner = e.InnerException;
while (inner is { })
{
Console.Error.WriteLine($"--- inner {inner.GetType().FullName} ---");
Console.Error.WriteLine(inner.StackTrace);
inner = inner.InnerException;
}
}

/// <summary>
Expand Down
177 changes: 78 additions & 99 deletions src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
using System.Management.Automation.Language;
using System.Text;
using System.Threading;

using Microsoft.PowerShell.Commands;
using Microsoft.Win32;

using Dbg = System.Management.Automation.Diagnostics;

Expand All @@ -39,18 +37,22 @@ public class ModuleIntrinsics
private static readonly string s_windowsPowerShellPSHomeModulePath =
Path.Combine(System.Environment.SystemDirectory, "WindowsPowerShell", "v1.0", "Modules");

static ModuleIntrinsics()
{
// Initialize the module path.
SetModulePath();
}

internal ModuleIntrinsics(ExecutionContext context)
{
_context = context;

// And initialize the module path...
SetModulePath();
ModuleTable = new Dictionary<string, PSModuleInfo>(StringComparer.OrdinalIgnoreCase);
}

private readonly ExecutionContext _context;

// Holds the module collection...
internal Dictionary<string, PSModuleInfo> ModuleTable { get; } = new Dictionary<string, PSModuleInfo>(StringComparer.OrdinalIgnoreCase);
internal Dictionary<string, PSModuleInfo> ModuleTable { get; }

private const int MaxModuleNestingDepth = 10;

Expand Down Expand Up @@ -176,11 +178,9 @@ private PSModuleInfo CreateModuleImplementation(string name, string path, object

sb.SessionState = ss;
}
else
else if (moduleCode is string sbText)
{
var sbText = moduleCode as string;
if (sbText != null)
sb = ScriptBlock.Create(_context, sbText);
sb = ScriptBlock.Create(_context, sbText);
}
}

Expand Down Expand Up @@ -1082,91 +1082,80 @@ internal static string GetExpandedEnvironmentVariable(string name, EnvironmentVa
return result;
}

/// <summary>
/// Checks if a particular string (path) is a member of 'combined path' string (like %Path% or %PSModulePath%)
/// </summary>
/// <param name="pathToScan">'Combined path' string to analyze; can not be null.</param>
/// <param name="pathToLookFor">Path to search for; can not be another 'combined path' (semicolon-separated); can not be null.</param>
/// <returns>Index of pathToLookFor in pathToScan; -1 if not found.</returns>
private static int PathContainsSubstring(string pathToScan, string pathToLookFor)
{
// we don't support if any of the args are null - parent function should ensure this; empty values are ok
Diagnostics.Assert(pathToScan != null, "pathToScan should not be null according to contract of the function");
Diagnostics.Assert(pathToLookFor != null, "pathToLookFor should not be null according to contract of the function");

int pos = 0; // position of the current substring in pathToScan
string[] substrings = pathToScan.Split(Path.PathSeparator, StringSplitOptions.None); // we want to process empty entries
string goodPathToLookFor = pathToLookFor.Trim().TrimEnd(Path.DirectorySeparatorChar); // trailing backslashes and white-spaces will mess up equality comparison
foreach (string substring in substrings)
{
string goodSubstring = substring.Trim().TrimEnd(Path.DirectorySeparatorChar); // trailing backslashes and white-spaces will mess up equality comparison

// We have to use equality comparison on individual substrings (as opposed to simple 'string.IndexOf' or 'string.Contains')
// because of cases like { pathToScan="C:\Temp\MyDir\MyModuleDir", pathToLookFor="C:\Temp" }

if (string.Equals(goodSubstring, goodPathToLookFor, StringComparison.OrdinalIgnoreCase))
{
return pos; // match found - return index of it in the 'pathToScan' string
}
else
{
pos += substring.Length + 1; // '1' is for trailing semicolon
}
}
// if we are here, that means a match was not found
return -1;
}

/// <summary>
/// Adds paths to a 'combined path' string (like %Path% or %PSModulePath%) if they are not already there.
/// </summary>
/// <param name="basePath">Path string (like %Path% or %PSModulePath%).</param>
/// <param name="pathToAdd">Collection of individual paths to add.</param>
/// <param name="pathToAdd">An individual path to add, or multiple paths separated by the path separator character.</param>
/// <param name="insertPosition">-1 to append to the end; 0 to insert in the beginning of the string; etc...</param>
/// <returns>Result string.</returns>
private static string AddToPath(string basePath, string pathToAdd, int insertPosition)
private static string UpdatePath(string basePath, string pathToAdd, ref int insertPosition)
{
// we don't support if any of the args are null - parent function should ensure this; empty values are ok
Diagnostics.Assert(basePath != null, "basePath should not be null according to contract of the function");
Diagnostics.Assert(pathToAdd != null, "pathToAdd should not be null according to contract of the function");
Dbg.Assert(basePath != null, "basePath should not be null according to contract of the function");
Dbg.Assert(pathToAdd != null, "pathToAdd should not be null according to contract of the function");

// The 'pathToAdd' could be a 'combined path' (path-separator-separated).
string[] newPaths = pathToAdd.Split(
Path.PathSeparator,
StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);

StringBuilder result = new StringBuilder(basePath);
if (newPaths.Length is 0)
{
// The 'pathToAdd' doesn't really contain any paths to add.
return basePath;
}

var result = new StringBuilder(basePath, capacity: basePath.Length + pathToAdd.Length + newPaths.Length);
var addedPaths = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
string[] initialPaths = basePath.Split(
Path.PathSeparator,
StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);

foreach (string p in initialPaths)
{
// Remove the trailing directory separators.
// Trailing white spaces were already removed by 'StringSplitOptions.TrimEntries'.
addedPaths.Add(Path.TrimEndingDirectorySeparator(p));
}

if (!string.IsNullOrEmpty(pathToAdd)) // we don't want to append empty paths
foreach (string subPathToAdd in newPaths)
{
foreach (string subPathToAdd in pathToAdd.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries)) // in case pathToAdd is a 'combined path' (semicolon-separated)
// Remove the trailing directory separators.
// Trailing white spaces were already removed by 'StringSplitOptions.TrimEntries'.
string normalizedPath = Path.TrimEndingDirectorySeparator(subPathToAdd);
if (addedPaths.Contains(normalizedPath))
Comment thread
daxian-dbw marked this conversation as resolved.
{
int position = PathContainsSubstring(result.ToString(), subPathToAdd); // searching in effective 'result' value ensures that possible duplicates in pathsToAdd are handled correctly
if (position == -1) // subPathToAdd not found - add it
{
if (insertPosition == -1 || insertPosition > basePath.Length) // append subPathToAdd to the end
{
bool endsWithPathSeparator = false;
if (result.Length > 0)
{
endsWithPathSeparator = (result[result.Length - 1] == Path.PathSeparator);
}
// The normalized sub path was already added - skip it.
continue;
}

if (endsWithPathSeparator)
{
result.Append(subPathToAdd);
}
else
{
result.Append(Path.PathSeparator + subPathToAdd);
}
}
else if (insertPosition > result.Length)
{
// handle case where path is a singleton with no path separator already
result.Append(Path.PathSeparator).Append(subPathToAdd);
}
else // insert at the requested location (this is used by DSC (<Program Files> location) and by 'user-specific location' (SpecialFolder.MyDocuments or EVT.User))
{
result.Insert(insertPosition, subPathToAdd + Path.PathSeparator);
}
// The normalized sub path was not found - add it.
if (insertPosition is -1 || insertPosition >= result.Length)
Comment thread
daxian-dbw marked this conversation as resolved.
{
// Append the normalized sub path to the end.
if (result.Length > 0 && result[^1] != Path.PathSeparator)
{
result.Append(Path.PathSeparator);
}

result.Append(normalizedPath);
// Next insertion should happen at the end.
insertPosition = result.Length;
}
else
{
// Insert at the requested location.
// This is used by the user-specific module path, the shared module path (<Program Files> location), and the PSHome module path.
string strToInsert = normalizedPath + Path.PathSeparator;
result.Insert(insertPosition, strToInsert);

// Next insertion should happen after the just inserted string.
insertPosition += strToInsert.Length;
}

// Add it to the set.
addedPaths.Add(normalizedPath);
}

return result.ToString();
Expand Down Expand Up @@ -1218,7 +1207,7 @@ public static string GetModulePath(string currentProcessModulePath, string hklmM
string psHomeModulePath = GetPSHomeModulePath(); // $PSHome\Modules location

// If the variable isn't set, then set it to the default value
if (currentProcessModulePath == null) // EVT.Process does Not exist - really corner case
if (string.IsNullOrEmpty(currentProcessModulePath)) // EVT.Process does Not exist - really corner case
{
// Handle the default case...
if (string.IsNullOrEmpty(hkcuUserModulePath)) // EVT.User does Not exist -> set to <SpecialFolder.MyDocuments> location
Expand Down Expand Up @@ -1270,21 +1259,6 @@ public static string GetModulePath(string currentProcessModulePath, string hklmM
return currentProcessModulePath;
}

private static string UpdatePath(string path, string pathToAdd, ref int insertIndex)
{
if (!string.IsNullOrEmpty(pathToAdd))
{
path = AddToPath(path, pathToAdd, insertIndex);
insertIndex = path.IndexOf(Path.PathSeparator, PathContainsSubstring(path, pathToAdd));
if (insertIndex != -1)
{
// advance past the path separator
insertIndex++;
}
}
return path;
}

/// <summary>
/// Checks if $env:PSModulePath is not set and sets it as appropriate. Note - because these
/// strings go through the provider, we need to escape any wildcards before passing them
Expand Down Expand Up @@ -1358,11 +1332,16 @@ private static string SetModulePath()
{
string currentModulePath = GetExpandedEnvironmentVariable(Constants.PSModulePathEnvVar, EnvironmentVariableTarget.Process);
#if !UNIX
// if the current process and user env vars are the same, it means we need to append the machine one as it's incomplete
// otherwise, the user modified it and we should use the process one
// if the current process and user env vars are the same, it means we need to append the machine one as it's incomplete.
// Otherwise, the user modified it and we should use the process one.
if (string.CompareOrdinal(GetExpandedEnvironmentVariable(Constants.PSModulePathEnvVar, EnvironmentVariableTarget.User), currentModulePath) == 0)
{
currentModulePath = currentModulePath + Path.PathSeparator + GetExpandedEnvironmentVariable(Constants.PSModulePathEnvVar, EnvironmentVariableTarget.Machine);
string machineScopeValue = GetExpandedEnvironmentVariable(Constants.PSModulePathEnvVar, EnvironmentVariableTarget.Machine);
currentModulePath = string.IsNullOrEmpty(currentModulePath)
? machineScopeValue
: string.IsNullOrEmpty(machineScopeValue)
? currentModulePath
: string.Concat(currentModulePath, Path.PathSeparator, machineScopeValue);
}
#endif
string allUsersModulePath = PowerShellConfig.Instance.GetModulePath(ConfigScope.AllUsers);
Expand Down
27 changes: 27 additions & 0 deletions test/powershell/Host/ConsoleHost.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,33 @@ export $envVarName='$guid'
}
}

Context "-SettingsFile Commandline switch set 'PSModulePath'" {

BeforeAll {
$CustomSettingsFile = Join-Path -Path $TestDrive -ChildPath 'powershell.test.json'
$mPath1 = Join-Path $PSHOME 'Modules'
$mPath2 = Join-Path $TestDrive 'NonExist'
$pathSep = [System.IO.Path]::PathSeparator

## Use multiple paths in the setting.
$ModulePath = "${mPath1}${pathSep}${mPath2}".Replace('\', "\\")
Set-Content -Path $CustomSettingsfile -Value "{`"Microsoft.PowerShell:ExecutionPolicy`":`"Unrestricted`", `"PSModulePath`": `"$ModulePath`" }" -ErrorAction Stop
Comment thread
daxian-dbw marked this conversation as resolved.
}

It "Verify PowerShell PSModulePath should contain paths from config file" {
$psModulePath = & $powershell -NoProfile -SettingsFile $CustomSettingsFile -Command '$env:PSModulePath'

## $mPath1 already exists in the value of env PSModulePath, so it won't be added again.
$index = $psModulePath.IndexOf("${mPath1}${pathSep}", [System.StringComparison]::OrdinalIgnoreCase)
$index | Should -BeGreaterThan 0
$index += $mPath1.Length
$psModulePath.IndexOf($mPath1, $index, [System.StringComparison]::OrdinalIgnoreCase) | Should -BeExactly -1

## $mPath2 should be added at the index position 0.
$psModulePath.StartsWith("${mPath2}${pathSep}", [System.StringComparison]::OrdinalIgnoreCase) | Should -BeTrue
}
}

Context "Pipe to/from powershell" {
BeforeAll {
if ($null -ne $PSStyle) {
Expand Down
Loading