Merge pull request #2265 from daxian-dbw/improve-modulepath

Improve ModulePath calculation and refactor related method names

src/System.Management.Automation/engine/InitialSessionState.cs

@@ -2823,7 +2823,7 @@ namespace System.Management.Automation.Runspaces
             // If a user has any module with the same name as that of the core module( or nested module inside the core module)
             // in his module path, then that will get loaded instead of the actual nested module (from the GAC - in our case)
             // Hence, searching only from the system module path while loading the core modules
-            ProcessImportModule(initializedRunspace, CoreModulesToImport, ModuleIntrinsics.GetSystemwideModulePath(), publicCommands);
+            ProcessImportModule(initializedRunspace, CoreModulesToImport, ModuleIntrinsics.GetPSHomeModulePath(), publicCommands);
 
             // Win8:328748 - functions defined in global scope end up in a module
             // Since we import the core modules, EngineSessionState's module is set to the last imported module. So, if a function is defined in global scope, it ends up in that module.

src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs

@@ -542,13 +542,13 @@ namespace System.Management.Automation
         }
 
         /// <summary>
-        /// Gets the default system-wide module path.
+        /// Gets the PSHome module path, as known as the "system wide module path" in windows powershell.
         /// </summary>
-        /// <returns>The default system wide module path</returns>
-        internal static string GetSystemwideModulePath()
+        /// <returns>The PSHome module path</returns>
+        internal static string GetPSHomeModulePath()
         {
-            if (s_systemWideModulePath != null)
-                return s_systemWideModulePath;
+            if (s_psHomeModulePath != null)
+                return s_psHomeModulePath;
             
             try
             {
@@ -565,32 +565,33 @@ namespace System.Management.Automation
 #if !UNIX
                     psHome = psHome.ToLowerInvariant().Replace("\\syswow64\\", "\\system32\\");
 #endif
-                    Interlocked.CompareExchange(ref s_systemWideModulePath, Path.Combine(psHome, "Modules"), null);
+                    Interlocked.CompareExchange(ref s_psHomeModulePath, Path.Combine(psHome, "Modules"), null);
                 }
             }
             catch (System.Security.SecurityException) { }
 
-            return s_systemWideModulePath;
+            return s_psHomeModulePath;
         }
 
-        private static string s_systemWideModulePath;
+        private static string s_psHomeModulePath;
 
         /// <summary>
-        /// Get the DSC module path.
+        /// Get the module path that is shared among different users.
+        /// It's known as "Program Files" module path in windows powershell.
         /// </summary>
         /// <returns></returns>
-        internal static string GetDscModulePath()
+        internal static string GetSharedModulePath()
         {
 #if UNIX
             return Platform.SelectProductNameForDirectory(Platform.XDG_Type.SHARED_MODULES);
 #else
-            string dscModulePath = null;
+            string sharedModulePath = null;
             string programFilesPath = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles);
             if (!string.IsNullOrEmpty(programFilesPath))
             {
-                dscModulePath = Path.Combine(programFilesPath, Utils.ModuleDirectory);
+                sharedModulePath = Path.Combine(programFilesPath, Utils.ModuleDirectory);
             }
-            return dscModulePath;
+            return sharedModulePath;
 #endif
         }
 
@@ -601,20 +602,20 @@ namespace System.Management.Automation
         /// <returns></returns>
         private static string CombineSystemModulePaths()
         {
-            string systemModulePath = GetSystemwideModulePath();
-            string commonModulePath = GetDscModulePath();
+            string psHomeModulePath = GetPSHomeModulePath();
+            string sharedModulePath = GetSharedModulePath();
 
-            bool isSystemPathNullOrEmpty = string.IsNullOrEmpty(systemModulePath);
-            bool isCommonPathNullOrEmpty = string.IsNullOrEmpty(commonModulePath);
+            bool isPSHomePathNullOrEmpty = string.IsNullOrEmpty(psHomeModulePath);
+            bool isSharedPathNullOrEmpty = string.IsNullOrEmpty(sharedModulePath);
 
-            if (!isSystemPathNullOrEmpty && !isCommonPathNullOrEmpty)
+            if (!isPSHomePathNullOrEmpty && !isSharedPathNullOrEmpty)
             {
-                return (commonModulePath + Path.PathSeparator + systemModulePath);
+                return (sharedModulePath + Path.PathSeparator + psHomeModulePath);
             }
 
-            if (!isSystemPathNullOrEmpty || !isCommonPathNullOrEmpty)
+            if (!isPSHomePathNullOrEmpty || !isSharedPathNullOrEmpty)
             {
-                return isSystemPathNullOrEmpty ? commonModulePath : systemModulePath;
+                return isPSHomePathNullOrEmpty ? sharedModulePath : psHomeModulePath;
             }
 
             return null;
@@ -716,13 +717,13 @@ namespace System.Management.Automation
         ///  - inbox nano/iot ps started from sxs ps
         /// If it's likely one of them, then we need to clear the current process module path.
         /// </summary>
-        private static bool NeedToClearProcessModulePath(string currentProcessModulePath, string personalModulePath, string programFilesModulePath, bool runningSxS)
+        private static bool NeedToClearProcessModulePath(string currentProcessModulePath, string personalModulePath, string sharedModulePath, bool runningSxS)
         {
 #if UNIX
             return false;
 #else
             Dbg.Assert(!string.IsNullOrEmpty(personalModulePath), "caller makes sure it's not null or empty");
-            Dbg.Assert(!string.IsNullOrEmpty(programFilesModulePath), "caller makes sure it's not null or empty");
+            Dbg.Assert(!string.IsNullOrEmpty(sharedModulePath), "caller makes sure it's not null or empty");
 
             const string winSxSModuleDirectory = @"PowerShell\Modules";
             const string winLegacyModuleDirectory = @"WindowsPowerShell\Modules";
@@ -736,7 +737,7 @@ namespace System.Management.Automation
                 string hklmModulePath = GetExpandedEnvironmentVariable("PSMODULEPATH", EnvironmentVariableTarget.Machine);
                 string hkcuModulePath = GetExpandedEnvironmentVariable("PSMODULEPATH", EnvironmentVariableTarget.User);
                 string legacyPersonalModulePath = personalModulePath.Replace(winSxSModuleDirectory, winLegacyModuleDirectory);
-                string legacyProgramFilesModulePath = programFilesModulePath.Replace(winSxSModuleDirectory, winLegacyModuleDirectory);
+                string legacyProgramFilesModulePath = sharedModulePath.Replace(winSxSModuleDirectory, winLegacyModuleDirectory);
 
                 return (!string.IsNullOrEmpty(hklmModulePath) && currentProcessModulePath.IndexOf(hklmModulePath, StringComparison.OrdinalIgnoreCase) != -1) ||
                        (!string.IsNullOrEmpty(hkcuModulePath) && currentProcessModulePath.IndexOf(hkcuModulePath, StringComparison.OrdinalIgnoreCase) != -1) ||
@@ -747,7 +748,7 @@ namespace System.Management.Automation
             // The sxs personal and shared module paths are only meaningful for sxs ps, so if they appear
             // in the current process module path, it's likely the running ps was started from a sxs ps.
             string sxsPersonalModulePath = personalModulePath.Replace(winLegacyModuleDirectory, winSxSModuleDirectory);
-            string sxsProgramFilesModulePath = programFilesModulePath.Replace(winLegacyModuleDirectory, winSxSModuleDirectory);
+            string sxsProgramFilesModulePath = sharedModulePath.Replace(winLegacyModuleDirectory, winSxSModuleDirectory);
 
             return currentProcessModulePath.IndexOf(sxsPersonalModulePath, StringComparison.OrdinalIgnoreCase) != -1 ||
                    currentProcessModulePath.IndexOf(sxsProgramFilesModulePath, StringComparison.OrdinalIgnoreCase) != -1;
@@ -759,7 +760,7 @@ namespace System.Management.Automation
         /// show up in current process module path. It doesn't make sense for B to load modules from
         /// A's pshome module path, so remove it in such case.
         /// </summary>
-        private static string RemoveSxSPsHomeModulePath(string currentProcessModulePath)
+        private static string RemoveSxSPsHomeModulePath(string currentProcessModulePath, string personalModulePath, string sharedModulePath, string psHomeModulePath)
         {
 #if UNIX
             const string powershellExeName = "powershell";
@@ -772,17 +773,25 @@ namespace System.Management.Automation
             foreach (var path in currentProcessModulePath.Split(Utils.Separators.PathSeparator, StringSplitOptions.RemoveEmptyEntries))
             {
                 string trimedPath = path.Trim().TrimEnd(Path.DirectorySeparatorChar);
-                if (trimedPath == string.Empty || trimedPath.IndexOfAny(invalidPathChars) != -1)
+                if (trimedPath.IndexOfAny(invalidPathChars) != -1 || !Path.IsPathRooted(trimedPath))
                 {
-                    // Path contains invalid characters. Ignore it.
+                    // Path contains invalid characters or it's not an absolute path. Ignore it.
                     continue;
                 }
 
-                string psExePath = Path.Combine(Path.GetDirectoryName(trimedPath), powershellExeName);
-                if (File.Exists(psExePath))
+                if (!trimedPath.Equals(personalModulePath, StringComparison.OrdinalIgnoreCase) && 
+                    !trimedPath.Equals(sharedModulePath, StringComparison.OrdinalIgnoreCase) &&
+                    !trimedPath.Equals(psHomeModulePath, StringComparison.OrdinalIgnoreCase) &&
+                    trimedPath.EndsWith("Modules", StringComparison.OrdinalIgnoreCase))
                 {
-                    // Path is a PSHome module path. Ignore it.
-                    continue;
+                    string parentDir = Path.GetDirectoryName(trimedPath);
+                    string psExePath = Path.Combine(parentDir, powershellExeName);
+                    string psDepsPath = Path.Combine(parentDir, "powershell.deps.json");
+                    if (File.Exists(psExePath) && File.Exists(psDepsPath))
+                    {
+                        // Path is a PSHome module path from a different powershell core instance. Ignore it.
+                        continue;
+                    }
                 }
 
                 if (modulePathString.Length > 0)
@@ -803,8 +812,8 @@ namespace System.Management.Automation
         public static string GetModulePath(string currentProcessModulePath, string hklmMachineModulePath, string hkcuUserModulePath)
         {
             string personalModulePath = GetPersonalModulePath();
-            string programFilesModulePath = GetDscModulePath(); // aka <Program Files> location
-            string psHomeModulePath = GetSystemwideModulePath(); // $PSHome\Modules location
+            string sharedModulePath = GetSharedModulePath(); // aka <Program Files> location
+            string psHomeModulePath = GetPSHomeModulePath(); // $PSHome\Modules location
 
 #if CORECLR
             bool runningSxS = Platform.IsInbox ? false : true;
@@ -812,7 +821,7 @@ namespace System.Management.Automation
             bool runningSxS = false;
 #endif
             if (!string.IsNullOrEmpty(currentProcessModulePath) && 
-                NeedToClearProcessModulePath(currentProcessModulePath, personalModulePath, programFilesModulePath, runningSxS))
+                NeedToClearProcessModulePath(currentProcessModulePath, personalModulePath, sharedModulePath, runningSxS))
             {
                 // Clear the current process module path in the following cases
                 //  - start sxs ps on windows [machine-wide env:psmodulepath will influence]
@@ -839,7 +848,7 @@ namespace System.Management.Automation
                 currentProcessModulePath += Path.PathSeparator;
                 if (string.IsNullOrEmpty(hklmMachineModulePath)) // EVT.Machine does Not exist
                 {
-                    currentProcessModulePath += CombineSystemModulePaths(); // += (DscModulePath + $PSHome\Modules)
+                    currentProcessModulePath += CombineSystemModulePaths(); // += (SharedModulePath + $PSHome\Modules)
                 }
                 else
                 {
@@ -851,7 +860,7 @@ namespace System.Management.Automation
             else if (runningSxS) // The running powershell is an SxS PS instance
             {
                 // When SxS PS instance A starts SxS PS instance B, A's PSHome module path might be inherited by B. We need to remove that path from B
-                currentProcessModulePath = RemoveSxSPsHomeModulePath(currentProcessModulePath);
+                currentProcessModulePath = RemoveSxSPsHomeModulePath(currentProcessModulePath, personalModulePath, sharedModulePath, psHomeModulePath);
 
                 string personalModulePathToUse = string.IsNullOrEmpty(hkcuUserModulePath) ? personalModulePath : hkcuUserModulePath;
                 string systemModulePathToUse = string.IsNullOrEmpty(hklmMachineModulePath) ? psHomeModulePath : hklmMachineModulePath;
@@ -869,7 +878,7 @@ namespace System.Management.Automation
                     {
                         if (!(hklmMachineModulePath).Equals(currentProcessModulePath, StringComparison.OrdinalIgnoreCase))
                         {
-                            // before returning, use <presence of Windows module path> heuristic to conditionally add programFilesModulePath
+                            // before returning, use <presence of Windows module path> heuristic to conditionally add <Program Files> location
                             int psHomePosition = PathContainsSubstring(currentProcessModulePath, psHomeModulePath); // index of $PSHome\Modules in currentProcessModulePath
                             if (psHomePosition >= 0) // if $PSHome\Modules IS found - insert <Program Files> location before $PSHome\Modules
                             {
@@ -881,7 +890,7 @@ namespace System.Management.Automation
                                     psHomePosition = PathContainsSubstring(currentProcessModulePath, psHomeModulePath);
                                 }
 #endif
-                                return AddToPath(currentProcessModulePath, programFilesModulePath, psHomePosition);
+                                return AddToPath(currentProcessModulePath, sharedModulePath, psHomePosition);
                             } // if $PSHome\Modules NOT found = <scenario 4> = 'PSModulePath has been constrained by a user to create a sand boxed environment without including System Modules'
 
                             return null;
@@ -896,11 +905,11 @@ namespace System.Management.Automation
                             (hklmMachineModulePath).Equals(currentProcessModulePath, StringComparison.OrdinalIgnoreCase) ||
                             (hkcuUserModulePath).Equals(currentProcessModulePath, StringComparison.OrdinalIgnoreCase)))
                         {
-                            // before returning, use <presence of Windows module path> heuristic to conditionally add programFilesModulePath
+                            // before returning, use <presence of Windows module path> heuristic to conditionally add <Program Files> location
                             int psHomePosition = PathContainsSubstring(currentProcessModulePath, psHomeModulePath); // index of $PSHome\Modules in currentProcessModulePath
                             if (psHomePosition >= 0) // if $PSHome\Modules IS found - insert <Program Files> location before $PSHome\Modules
                             {
-                                return AddToPath(currentProcessModulePath, programFilesModulePath, psHomePosition);
+                                return AddToPath(currentProcessModulePath, sharedModulePath, psHomePosition);
                             } // if $PSHome\Modules NOT found = <scenario 4> = 'PSModulePath has been constrained by a user to create a sand boxed environment without including System Modules'
 
                             return null;
@@ -916,15 +925,15 @@ namespace System.Management.Automation
                     {
                         if (hkcuUserModulePath.Equals(currentProcessModulePath, StringComparison.OrdinalIgnoreCase))
                         {
-                            currentProcessModulePath = hkcuUserModulePath + Path.PathSeparator + CombineSystemModulePaths(); // = EVT.User + (DscModulePath + $PSHome\Modules)
+                            currentProcessModulePath = hkcuUserModulePath + Path.PathSeparator + CombineSystemModulePaths(); // = EVT.User + (SharedModulePath + $PSHome\Modules)
                         }
                         else
                         {
-                            // before returning, use <presence of Windows module path> heuristic to conditionally add programFilesModulePath
+                            // before returning, use <presence of Windows module path> heuristic to conditionally add <Program Files> location
                             int psHomePosition = PathContainsSubstring(currentProcessModulePath, psHomeModulePath); // index of $PSHome\Modules in currentProcessModulePath
                             if (psHomePosition >= 0) // if $PSHome\Modules IS found - insert <Program Files> location before $PSHome\Modules
                             {
-                                return AddToPath(currentProcessModulePath, programFilesModulePath, psHomePosition);
+                                return AddToPath(currentProcessModulePath, sharedModulePath, psHomePosition);
                             } // if $PSHome\Modules NOT found = <scenario 4> = 'PSModulePath has been constrained by a user to create a sand boxed environment without including System Modules'
 
                             return null;
@@ -932,11 +941,11 @@ namespace System.Management.Automation
                     }
                     else // EVT.User does Not exist
                     {
-                        // before returning, use <presence of Windows module path> heuristic to conditionally add programFilesModulePath
+                        // before returning, use <presence of Windows module path> heuristic to conditionally add <Program Files> location
                         int psHomePosition = PathContainsSubstring(currentProcessModulePath, psHomeModulePath); // index of $PSHome\Modules in currentProcessModulePath
                         if (psHomePosition >= 0) // if $PSHome\Modules IS found - insert <Program Files> location before $PSHome\Modules
                         {
-                            return AddToPath(currentProcessModulePath, programFilesModulePath, psHomePosition);
+                            return AddToPath(currentProcessModulePath, sharedModulePath, psHomePosition);
                         } // if $PSHome\Modules NOT found = <scenario 4> = 'PSModulePath has been constrained by a user to create a sand boxed environment without including System Modules'
 
                         // Neither key is set so go with what the environment variable is already set to
@@ -950,7 +959,7 @@ namespace System.Management.Automation
             int indexOfPSHomeModulePath = PathContainsSubstring(currentProcessModulePath, psHomeModulePath); // index of $PSHome\Modules in currentProcessModulePath
             // if $PSHome\Modules not found (psHomePosition == -1) - append <Program Files> location to the end;
             // if $PSHome\Modules IS found (psHomePosition >= 0) - insert <Program Files> location before $PSHome\Modules
-            currentProcessModulePath = AddToPath(currentProcessModulePath, programFilesModulePath, indexOfPSHomeModulePath);
+            currentProcessModulePath = AddToPath(currentProcessModulePath, sharedModulePath, indexOfPSHomeModulePath);
 
             return currentProcessModulePath;
         }
@@ -1020,7 +1029,7 @@ namespace System.Management.Automation
 
             if (includeSystemModulePath)
             {
-                var processedPath = ProcessOneModulePath(context, GetSystemwideModulePath(), processedPathSet);
+                var processedPath = ProcessOneModulePath(context, GetPSHomeModulePath(), processedPathSet);
                 if (processedPath != null)
                     yield return processedPath;
             }

test/powershell/enginecore/ModulePath.Tests.ps1

@@ -20,6 +20,15 @@ Describe "SxS Module Path Basic Tests" -tags "CI" {
             $expectedSharedPath = [System.Management.Automation.Platform]::SelectProductNameForDirectory("SHARED_MODULES")
         }
         $expectedSystemPath = Join-Path -Path $PSHOME -ChildPath 'Modules'
+
+        ## Setup a fake PSHome
+        $fakePSHome = Join-Path -Path $TestDrive -ChildPath 'FakePSHome'
+        $fakePSHomeModuleDir = Join-Path -Path $fakePSHome -ChildPath 'Modules'
+        $fakePowerShell = Join-Path -Path $fakePSHome -ChildPath (Split-Path -Path $powershell -Leaf)
+        $fakePSDepsFile = Join-Path -Path $fakePSHome -ChildPath "powershell.deps.json"
+
+        New-Item -Path $fakePSHome -ItemType Directory > $null
+        New-Item -Path $fakePSHomeModuleDir -ItemType Directory > $null
     }
 
     BeforeEach {
@@ -30,6 +39,7 @@ Describe "SxS Module Path Basic Tests" -tags "CI" {
         $env:PSMODULEPATH = $originalModulePath
     }
 
+
     It "validate sxs module path" {
 
         $env:PSMODULEPATH = ""
@@ -38,8 +48,49 @@ Describe "SxS Module Path Basic Tests" -tags "CI" {
         $paths = $defaultModulePath -split [System.IO.Path]::PathSeparator
 
         $paths.Count | Should Be 3
-        $paths[0] | Should Be $expectedUserPath
-        $paths[1] | Should Be $expectedSharedPath
-        $paths[2] | Should Be $expectedSystemPath
+        $paths[0].TrimEnd([System.IO.Path]::DirectorySeparatorChar) | Should Be $expectedUserPath
+        $paths[1].TrimEnd([System.IO.Path]::DirectorySeparatorChar) | Should Be $expectedSharedPath
+        $paths[2].TrimEnd([System.IO.Path]::DirectorySeparatorChar) | Should Be $expectedSystemPath
+    }
+
+
+    It "ignore pshome module path derived from a different powershell core instance" -Skip:(!$IsCoreCLR) {
+
+        ## Create 'powershell' and 'powershell.deps.json' in the fake PSHome folder,
+        ## so that the module path calculation logic would believe it's real.
+        New-Item -Path $fakePowerShell -ItemType File -Force > $null
+        New-Item -Path $fakePSDepsFile -ItemType File -Force > $null
+
+        try {
+
+            ## PSHome module path derived from another powershell core instance should be ignored
+            $env:PSMODULEPATH = $fakePSHomeModuleDir
+            $newModulePath = & $powershell -nopro -c '$env:PSMODULEPATH'
+            $paths = $newModulePath -split [System.IO.Path]::PathSeparator
+
+            $paths.Count | Should Be 3
+            $paths[0] | Should Be $expectedUserPath
+            $paths[1] | Should Be $expectedSharedPath
+            $paths[2] | Should Be $expectedSystemPath
+
+        } finally {
+
+            ## Remove 'powershell' and 'powershell.deps.json' from the fake PSHome folder
+            Remove-Item -Path $fakePowerShell -Force -ErrorAction SilentlyContinue
+            Remove-Item -Path $fakePSDepsFile -Force -ErrorAction SilentlyContinue
+        }
+    }
+
+    It "keep non-pshome module path derived from powershell core instance parent" -Skip:(!$IsCoreCLR) {
+
+        ## non-pshome module path derived from another powershell core instance should be preserved
+        $customeModules = Join-Path -Path $TestDrive -ChildPath 'CustomModules'
+        $env:PSMODULEPATH = $fakePSHomeModuleDir, $customeModules -join ([System.IO.Path]::PathSeparator)
+        $newModulePath = & $powershell -nopro -c '$env:PSMODULEPATH'
+        $paths = $newModulePath -split [System.IO.Path]::PathSeparator
+
+        $paths.Count | Should Be 5
+        $paths -contains $fakePSHomeModuleDir | Should Be $true
+        $paths -contains $customeModules | Should Be $true
     }
 }