From 9957c8d17d6f887db7b293f0c6f0e393630d346e Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Mon, 15 Aug 2022 10:38:11 -0400 Subject: [PATCH 1/8] when installing script ensure that Script InstallPath is added to env Path variable so script can be invoked without prepending install folder path --- src/code/InstallHelper.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 75f6d54f2..33c1471f5 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -575,6 +575,24 @@ private List InstallPackage( _cmdletPassedIn.WriteVerbose(String.Format("Successfully installed package '{0}' to location '{1}'", pkg.Name, installPath)); pkgsSuccessfullyInstalled.Add(pkg); + + if (!isModule) + { + // Add Scripts install path to Path environment variable so that it can be ran without prepending the path + if (!Environment.GetEnvironmentVariable("PATH").Contains(installPath)) + { + // need to add Scripts install path + string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); + if (!String.IsNullOrEmpty(currentPathEnvVarValue)) + { + Environment.SetEnvironmentVariable("PATH", installPath + ";" + currentPathEnvVarValue, EnvironmentVariableTarget.User); + } + else + { + Environment.SetEnvironmentVariable("PATH", installPath, EnvironmentVariableTarget.User); + } + } + } } catch (Exception e) { From 21395d9229f6449204e64e7616ce3858da098b9e Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Tue, 16 Aug 2022 16:43:56 -0400 Subject: [PATCH 2/8] add prompting for setting env var, add scope based env var setting, and pass scope to InstallHelper --- src/code/InstallHelper.cs | 57 ++++++++++++++++++++++++++--------- src/code/InstallPSResource.cs | 3 +- src/code/SavePSResource.cs | 3 +- src/code/UpdatePSResource.cs | 3 +- 4 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 33c1471f5..fe19411b8 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -35,6 +35,8 @@ internal class InstallHelper : PSCmdlet public const string PSScriptFileExt = ".ps1"; private const string MsgRepositoryNotTrusted = "Untrusted repository"; private const string MsgInstallUntrustedPackage = "You are installing the modules from an untrusted repository. If you trust this repository, change its Trusted value by running the Set-PSResourceRepository cmdlet. Are you sure you want to install the PSresource from '{0}' ?"; + private const string ScriptPATHPromptQuery = "Your system has not been configured with a default script installation path yet, which means you can only run a script by specifying the full path to the script file. This action places the script into the folder '{0}', and adds that folder to your PATH environment variable. Do you want to add the script installation path '{0}' to the PATH environment variable?"; + private const string PATHEnvVarAlteration = "Updating PATH environment variable"; private CancellationToken _cancellationToken; private readonly PSCmdlet _cmdletPassedIn; private List _pathsToInstallPkg; @@ -82,7 +84,8 @@ public List InstallPackages( bool skipDependencyCheck, bool authenticodeCheck, bool savePkg, - List pathsToInstallPkg) + List pathsToInstallPkg, + ScopeType scope) { _cmdletPassedIn.WriteVerbose(string.Format("Parameters passed in >>> Name: '{0}'; Version: '{1}'; Prerelease: '{2}'; Repository: '{3}'; " + "AcceptLicense: '{4}'; Quiet: '{5}'; Reinstall: '{6}'; TrustRepository: '{7}'; NoClobber: '{8}'; AsNupkg: '{9}'; IncludeXml '{10}'; SavePackage '{11}'", @@ -135,7 +138,8 @@ public List InstallPackages( repository: repository, trustRepository: _trustRepository, credential: _credential, - skipDependencyCheck: skipDependencyCheck); + skipDependencyCheck: skipDependencyCheck, + scope: scope); } #endregion @@ -147,7 +151,8 @@ private List ProcessRepositories( string[] repository, bool trustRepository, PSCredential credential, - bool skipDependencyCheck) + bool skipDependencyCheck, + ScopeType scope) { var listOfRepositories = RepositorySettings.Read(repository, out string[] _); var yesToAll = false; @@ -233,7 +238,8 @@ private List ProcessRepositories( repo.Uri.AbsoluteUri, repo.CredentialInfo, credential, - isLocalRepo); + isLocalRepo, + scope: scope); foreach (PSResourceInfo pkg in pkgsInstalled) { @@ -317,7 +323,8 @@ private List InstallPackage( string repoUri, PSCredentialInfo repoCredentialInfo, PSCredential credential, - bool isLocalRepo) + bool isLocalRepo, + ScopeType scope) { List pkgsSuccessfullyInstalled = new List(); int totalPkgs = pkgsToInstall.Count; @@ -576,20 +583,40 @@ private List InstallPackage( _cmdletPassedIn.WriteVerbose(String.Format("Successfully installed package '{0}' to location '{1}'", pkg.Name, installPath)); pkgsSuccessfullyInstalled.Add(pkg); - if (!isModule) + if (!_savePkg && !isModule) { // Add Scripts install path to Path environment variable so that it can be ran without prepending the path - if (!Environment.GetEnvironmentVariable("PATH").Contains(installPath)) + string envPathValue = Environment.GetEnvironmentVariable("PATH"); // todo anam- does this get all targets? + string installPathwithBackSlash = installPath + "\\"; + if (!envPathValue.Contains(installPath) && !envPathValue.Contains(installPathwithBackSlash)) { - // need to add Scripts install path - string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); - if (!String.IsNullOrEmpty(currentPathEnvVarValue)) - { - Environment.SetEnvironmentVariable("PATH", installPath + ";" + currentPathEnvVarValue, EnvironmentVariableTarget.User); - } - else + // Prompt for altering PATH environment variable. + var message = string.Format(CultureInfo.InvariantCulture, ScriptPATHPromptQuery, installPath); + bool allowSettingEnvVar = _cmdletPassedIn.ShouldContinue(message, PATHEnvVarAlteration); + + if (allowSettingEnvVar) { - Environment.SetEnvironmentVariable("PATH", installPath, EnvironmentVariableTarget.User); + // Determine scope for which to add installPath and also add to the Process target. + if (scope == ScopeType.CurrentUser) + { + string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); + string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + ";" + currentPathEnvVarValue; + + Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.User); + } + else + { + // ScopeType.AllUser + string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine); + string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + ";" + currentPathEnvVarValue; + + Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.Machine); + } + + // Also update Process target + string currentProcessPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Process); + string newProcessPathEnvVarValue = String.IsNullOrEmpty(currentProcessPathEnvVarValue) ? installPath : installPath + ";" + currentProcessPathEnvVarValue; + Environment.SetEnvironmentVariable("PATH", newProcessPathEnvVarValue, EnvironmentVariableTarget.Process); } } } diff --git a/src/code/InstallPSResource.cs b/src/code/InstallPSResource.cs index 4149a2ba5..130ccfacc 100644 --- a/src/code/InstallPSResource.cs +++ b/src/code/InstallPSResource.cs @@ -528,7 +528,8 @@ private void ProcessInstallHelper(string[] pkgNames, VersionRange pkgVersion, bo skipDependencyCheck: SkipDependencyCheck, authenticodeCheck: AuthenticodeCheck, savePkg: false, - pathsToInstallPkg: _pathsToInstallPkg); + pathsToInstallPkg: _pathsToInstallPkg, + scope: Scope); if (PassThru) { diff --git a/src/code/SavePSResource.cs b/src/code/SavePSResource.cs index 3f2b2ce26..63d9d096a 100644 --- a/src/code/SavePSResource.cs +++ b/src/code/SavePSResource.cs @@ -266,7 +266,8 @@ private void ProcessSaveHelper(string[] pkgNames, bool pkgPrerelease, string[] p skipDependencyCheck: SkipDependencyCheck, authenticodeCheck: AuthenticodeCheck, savePkg: true, - pathsToInstallPkg: new List { _path }); + pathsToInstallPkg: new List { _path }, + scope: ScopeType.CurrentUser); if (PassThru) { diff --git a/src/code/UpdatePSResource.cs b/src/code/UpdatePSResource.cs index bce9ed295..ad581edf8 100644 --- a/src/code/UpdatePSResource.cs +++ b/src/code/UpdatePSResource.cs @@ -187,7 +187,8 @@ protected override void ProcessRecord() skipDependencyCheck: SkipDependencyCheck, authenticodeCheck: AuthenticodeCheck, savePkg: false, - pathsToInstallPkg: _pathsToInstallPkg); + pathsToInstallPkg: _pathsToInstallPkg, + scope: Scope); if (PassThru) { From 7c6db1251b5492bb2ecb2d36c5246ccb21485af5 Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Wed, 17 Aug 2022 14:16:17 -0400 Subject: [PATCH 3/8] InstallHelper should take ScopeType? param, use Path.PathSeperator for cross platform compat, and add tests --- src/code/InstallHelper.cs | 10 ++--- src/code/SavePSResource.cs | 2 +- test/InstallPSResource.Tests.ps1 | 77 +++++++++++++++++++++++++++++--- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index fe19411b8..a1a700122 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -85,7 +85,7 @@ public List InstallPackages( bool authenticodeCheck, bool savePkg, List pathsToInstallPkg, - ScopeType scope) + ScopeType? scope) { _cmdletPassedIn.WriteVerbose(string.Format("Parameters passed in >>> Name: '{0}'; Version: '{1}'; Prerelease: '{2}'; Repository: '{3}'; " + "AcceptLicense: '{4}'; Quiet: '{5}'; Reinstall: '{6}'; TrustRepository: '{7}'; NoClobber: '{8}'; AsNupkg: '{9}'; IncludeXml '{10}'; SavePackage '{11}'", @@ -139,7 +139,7 @@ public List InstallPackages( trustRepository: _trustRepository, credential: _credential, skipDependencyCheck: skipDependencyCheck, - scope: scope); + scope: scope?? ScopeType.CurrentUser); } #endregion @@ -600,7 +600,7 @@ private List InstallPackage( if (scope == ScopeType.CurrentUser) { string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); - string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + ";" + currentPathEnvVarValue; + string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentPathEnvVarValue; Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.User); } @@ -608,14 +608,14 @@ private List InstallPackage( { // ScopeType.AllUser string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine); - string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + ";" + currentPathEnvVarValue; + string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentPathEnvVarValue; Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.Machine); } // Also update Process target string currentProcessPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Process); - string newProcessPathEnvVarValue = String.IsNullOrEmpty(currentProcessPathEnvVarValue) ? installPath : installPath + ";" + currentProcessPathEnvVarValue; + string newProcessPathEnvVarValue = String.IsNullOrEmpty(currentProcessPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentProcessPathEnvVarValue; Environment.SetEnvironmentVariable("PATH", newProcessPathEnvVarValue, EnvironmentVariableTarget.Process); } } diff --git a/src/code/SavePSResource.cs b/src/code/SavePSResource.cs index 63d9d096a..27f31e3ff 100644 --- a/src/code/SavePSResource.cs +++ b/src/code/SavePSResource.cs @@ -267,7 +267,7 @@ private void ProcessSaveHelper(string[] pkgNames, bool pkgPrerelease, string[] p authenticodeCheck: AuthenticodeCheck, savePkg: true, pathsToInstallPkg: new List { _path }, - scope: ScopeType.CurrentUser); + scope: null); if (PassThru) { diff --git a/test/InstallPSResource.Tests.ps1 b/test/InstallPSResource.Tests.ps1 index bef8111ae..137583ef5 100644 --- a/test/InstallPSResource.Tests.ps1 +++ b/test/InstallPSResource.Tests.ps1 @@ -181,7 +181,7 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { + It "Install module resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName @@ -189,7 +189,7 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { + It "Install module resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { Install-PSResource -Name "testmodule99" -Repository $PSGalleryName -TrustRepository -Scope AllUsers -Verbose $pkg = Get-Module "testmodule99" -ListAvailable $pkg.Name | Should -Be "testmodule99" @@ -197,16 +197,55 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { + It "Install module resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true } + # Windows only + It "Install script resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { + Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser + $pkg = Get-PSResource $testScriptName + $pkg.Name | Should -Be $testScriptName + $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true + + # Ensure script is invokable by name, i.e without prepending script installation path. + $scriptCommand = Get-Command $testScriptName + $scriptCommand.Name | Should -BeLike $testScriptName + $scriptCommand.Source.Contains("Documents") | Should -Be $true + } + + # Windows only + It "Install script resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { + Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope AllUsers + $pkg = Get-PSResource $testScriptName + $pkg.Name | Should -Be $testScriptName + $pkg.InstalledLocation.ToString().Contains("Program Files") + + # Ensure script is invokable by name, i.e without prepending script installation path. + $scriptCommand = Get-Command $testScriptName + $scriptCommand.Name | Should -BeLike $testScriptName + $scriptCommand.Source.Contains("Program Files") | Should -Be $true + } + + # Windows only + It "Install script resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { + Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository + $pkg = Get-PSResource $testScriptName + $pkg.Name | Should -Be $testScriptName + $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true + + # Ensure script is invokable by name, i.e without prepending script installation path. + $scriptCommand = Get-Command $testScriptName + $scriptCommand.Name | Should -BeLike $testScriptName + $scriptCommand.Source.Contains("Documents") | Should -Be $true + } + # Unix only # Expected path should be similar to: '/home/janelane/.local/share/powershell/Modules' - It "Install resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { + It "Install module resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName @@ -215,13 +254,41 @@ Describe 'Test Install-PSResource for Module' { # Unix only # Expected path should be similar to: '/home/janelane/.local/share/powershell/Modules' - It "Install resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { + It "Install module resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true } + # Unix only + # Expected path should be similar to: '/home/janelane/.local/share/powershell/Scripts' + It "Install script resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { + Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser + $pkg = Get-PSResource $testScriptName + $pkg.Name | Should -Be $testScriptName + $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true + + # Ensure script is invokable by name, i.e without prepending script installation path. + $scriptCommand = Get-Command $testScriptName + $scriptCommand.Name | Should -BeLike $testScriptName + $scriptCommand.Source.Contains("$env:HOME/.local") | Should -Be $true + } + + # Unix only + # Expected path should be similar to: '/home/janelane/.local/share/powershell/Scripts' + It "Install script resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { + Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository + $pkg = Get-PSResource $testScriptName + $pkg.Name | Should -Be $testScriptName + $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true + + # Ensure script is invokable by name, i.e without prepending script installation path. + $scriptCommand = Get-Command $testScriptName + $scriptCommand.Name | Should -BeLike $testScriptName + $scriptCommand.Source.Contains("$env:HOME/.local") | Should -Be $true + } + It "Should not install resource that is already installed" { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName From d303536c602ad449157f05e7f2c56822d7b1f0b4 Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Fri, 19 Aug 2022 12:11:00 -0400 Subject: [PATCH 4/8] instead of prompt, use ShouldProcess for modiying user's environment variable --- src/code/InstallHelper.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index a1a700122..36b8ab861 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -590,11 +590,10 @@ private List InstallPackage( string installPathwithBackSlash = installPath + "\\"; if (!envPathValue.Contains(installPath) && !envPathValue.Contains(installPathwithBackSlash)) { - // Prompt for altering PATH environment variable. - var message = string.Format(CultureInfo.InvariantCulture, ScriptPATHPromptQuery, installPath); - bool allowSettingEnvVar = _cmdletPassedIn.ShouldContinue(message, PATHEnvVarAlteration); - - if (allowSettingEnvVar) + if (_cmdletPassedIn.ShouldProcess( + String.Format("Adding {0} to environment PATH variable", installPath), + String.Format(ScriptPATHPromptQuery, installPath), + PATHEnvVarAlteration)) { // Determine scope for which to add installPath and also add to the Process target. if (scope == ScopeType.CurrentUser) From 19e51fd4ff0602a3f9c5b5bb26db5253e2ca3e5c Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Tue, 30 Aug 2022 12:46:19 -0400 Subject: [PATCH 5/8] Update src/code/InstallHelper.cs Co-authored-by: Paul Higinbotham --- src/code/InstallHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 1e6ce7c69..3eb409d7f 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -35,7 +35,8 @@ internal class InstallHelper public const string PSScriptFileExt = ".ps1"; private const string MsgRepositoryNotTrusted = "Untrusted repository"; private const string MsgInstallUntrustedPackage = "You are installing the modules from an untrusted repository. If you trust this repository, change its Trusted value by running the Set-PSResourceRepository cmdlet. Are you sure you want to install the PSresource from '{0}' ?"; - private const string ScriptPATHPromptQuery = "Your system has not been configured with a default script installation path yet, which means you can only run a script by specifying the full path to the script file. This action places the script into the folder '{0}', and adds that folder to your PATH environment variable. Do you want to add the script installation path '{0}' to the PATH environment variable?"; + private const string ScriptPATHPromptQuery = "The installation path for the script does not appear in the {scope} path environment variable. Do you want the script installation path, {path}, added to the {scope} PATH environment variable?"; + private const string PATHEnvVarAlteration = "Updating PATH environment variable"; private CancellationToken _cancellationToken; private readonly PSCmdlet _cmdletPassedIn; From 87537e9e5ece967ddbaccedefe5ec4b2a5b15759 Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Tue, 30 Aug 2022 12:47:56 -0400 Subject: [PATCH 6/8] Update src/code/InstallHelper.cs Co-authored-by: Paul Higinbotham --- src/code/InstallHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 3eb409d7f..5e1a2bcfc 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -591,7 +591,8 @@ private List InstallPackage( if (!_savePkg && !isModule) { - // Add Scripts install path to Path environment variable so that it can be ran without prepending the path + // Add Scripts install path to Path environment variable so that it can be discovered. + string envPathValue = Environment.GetEnvironmentVariable("PATH"); // todo anam- does this get all targets? string installPathwithBackSlash = installPath + "\\"; if (!envPathValue.Contains(installPath) && !envPathValue.Contains(installPathwithBackSlash)) From 365ead615a324760fb1428960a9594b8994a1579 Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Tue, 30 Aug 2022 13:06:48 -0400 Subject: [PATCH 7/8] detect if Script install path is not in environment PATH for appropriate scope and present warning and revert install tests --- src/code/InstallHelper.cs | 46 ++++++------------- test/InstallPSResource.Tests.ps1 | 77 +++----------------------------- 2 files changed, 19 insertions(+), 104 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 5e1a2bcfc..1111c3667 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -35,9 +35,7 @@ internal class InstallHelper public const string PSScriptFileExt = ".ps1"; private const string MsgRepositoryNotTrusted = "Untrusted repository"; private const string MsgInstallUntrustedPackage = "You are installing the modules from an untrusted repository. If you trust this repository, change its Trusted value by running the Set-PSResourceRepository cmdlet. Are you sure you want to install the PSresource from '{0}' ?"; - private const string ScriptPATHPromptQuery = "The installation path for the script does not appear in the {scope} path environment variable. Do you want the script installation path, {path}, added to the {scope} PATH environment variable?"; - - private const string PATHEnvVarAlteration = "Updating PATH environment variable"; + private const string ScriptPATHWarning = "The installation path for the script does not currently appear in the {0} path environment variable. To make the script discoverable, add the script installation path, {1}, to the environment PATH variable."; private CancellationToken _cancellationToken; private readonly PSCmdlet _cmdletPassedIn; private List _pathsToInstallPkg; @@ -591,38 +589,22 @@ private List InstallPackage( if (!_savePkg && !isModule) { - // Add Scripts install path to Path environment variable so that it can be discovered. - - string envPathValue = Environment.GetEnvironmentVariable("PATH"); // todo anam- does this get all targets? string installPathwithBackSlash = installPath + "\\"; - if (!envPathValue.Contains(installPath) && !envPathValue.Contains(installPathwithBackSlash)) + if (scope == ScopeType.CurrentUser) + { + string currentUserPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); + if (!currentUserPathEnvVarValue.Contains(installPath) && !currentUserPathEnvVarValue.Contains(installPathwithBackSlash)) + { + _cmdletPassedIn.WriteWarning(String.Format(ScriptPATHWarning, scope, installPath)); + } + } + else { - if (_cmdletPassedIn.ShouldProcess( - String.Format("Adding {0} to environment PATH variable", installPath), - String.Format(ScriptPATHPromptQuery, installPath), - PATHEnvVarAlteration)) + // ScopeType.AllUsers + string allUsersPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine); + if (!allUsersPathEnvVarValue.Contains(installPath) && !allUsersPathEnvVarValue.Contains(installPathwithBackSlash)) { - // Determine scope for which to add installPath and also add to the Process target. - if (scope == ScopeType.CurrentUser) - { - string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); - string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentPathEnvVarValue; - - Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.User); - } - else - { - // ScopeType.AllUser - string currentPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine); - string newPathEnvVarValue = String.IsNullOrEmpty(currentPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentPathEnvVarValue; - - Environment.SetEnvironmentVariable("PATH", newPathEnvVarValue, EnvironmentVariableTarget.Machine); - } - - // Also update Process target - string currentProcessPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Process); - string newProcessPathEnvVarValue = String.IsNullOrEmpty(currentProcessPathEnvVarValue) ? installPath : installPath + Path.PathSeparator + currentProcessPathEnvVarValue; - Environment.SetEnvironmentVariable("PATH", newProcessPathEnvVarValue, EnvironmentVariableTarget.Process); + _cmdletPassedIn.WriteWarning(String.Format(ScriptPATHWarning, scope, installPath)); } } } diff --git a/test/InstallPSResource.Tests.ps1 b/test/InstallPSResource.Tests.ps1 index 4c6151ef2..b9ab10495 100644 --- a/test/InstallPSResource.Tests.ps1 +++ b/test/InstallPSResource.Tests.ps1 @@ -181,7 +181,7 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install module resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { + It "Install resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName @@ -189,7 +189,7 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install module resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { + It "Install resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { Install-PSResource -Name "testmodule99" -Repository $PSGalleryName -TrustRepository -Scope AllUsers -Verbose $pkg = Get-Module "testmodule99" -ListAvailable $pkg.Name | Should -Be "testmodule99" @@ -197,55 +197,16 @@ Describe 'Test Install-PSResource for Module' { } # Windows only - It "Install module resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { + It "Install resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true } - # Windows only - It "Install script resource under CurrentUser scope - Windows only" -Skip:(!(Get-IsWindows)) { - Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser - $pkg = Get-PSResource $testScriptName - $pkg.Name | Should -Be $testScriptName - $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true - - # Ensure script is invokable by name, i.e without prepending script installation path. - $scriptCommand = Get-Command $testScriptName - $scriptCommand.Name | Should -BeLike $testScriptName - $scriptCommand.Source.Contains("Documents") | Should -Be $true - } - - # Windows only - It "Install script resource under AllUsers scope - Windows only" -Skip:(!((Get-IsWindows) -and (Test-IsAdmin))) { - Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope AllUsers - $pkg = Get-PSResource $testScriptName - $pkg.Name | Should -Be $testScriptName - $pkg.InstalledLocation.ToString().Contains("Program Files") - - # Ensure script is invokable by name, i.e without prepending script installation path. - $scriptCommand = Get-Command $testScriptName - $scriptCommand.Name | Should -BeLike $testScriptName - $scriptCommand.Source.Contains("Program Files") | Should -Be $true - } - - # Windows only - It "Install script resource under no specified scope - Windows only" -Skip:(!(Get-IsWindows)) { - Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository - $pkg = Get-PSResource $testScriptName - $pkg.Name | Should -Be $testScriptName - $pkg.InstalledLocation.ToString().Contains("Documents") | Should -Be $true - - # Ensure script is invokable by name, i.e without prepending script installation path. - $scriptCommand = Get-Command $testScriptName - $scriptCommand.Name | Should -BeLike $testScriptName - $scriptCommand.Source.Contains("Documents") | Should -Be $true - } - # Unix only # Expected path should be similar to: '/home/janelane/.local/share/powershell/Modules' - It "Install module resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { + It "Install resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName @@ -254,41 +215,13 @@ Describe 'Test Install-PSResource for Module' { # Unix only # Expected path should be similar to: '/home/janelane/.local/share/powershell/Modules' - It "Install module resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { + It "Install resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName $pkg.Name | Should -Be $testModuleName $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true } - # Unix only - # Expected path should be similar to: '/home/janelane/.local/share/powershell/Scripts' - It "Install script resource under CurrentUser scope - Unix only" -Skip:(Get-IsWindows) { - Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository -Scope CurrentUser - $pkg = Get-PSResource $testScriptName - $pkg.Name | Should -Be $testScriptName - $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true - - # Ensure script is invokable by name, i.e without prepending script installation path. - $scriptCommand = Get-Command $testScriptName - $scriptCommand.Name | Should -BeLike $testScriptName - $scriptCommand.Source.Contains("$env:HOME/.local") | Should -Be $true - } - - # Unix only - # Expected path should be similar to: '/home/janelane/.local/share/powershell/Scripts' - It "Install script resource under no specified scope - Unix only" -Skip:(Get-IsWindows) { - Install-PSResource -Name $testScriptName -Repository $PSGalleryName -TrustRepository - $pkg = Get-PSResource $testScriptName - $pkg.Name | Should -Be $testScriptName - $pkg.InstalledLocation.ToString().Contains("$env:HOME/.local") | Should -Be $true - - # Ensure script is invokable by name, i.e without prepending script installation path. - $scriptCommand = Get-Command $testScriptName - $scriptCommand.Name | Should -BeLike $testScriptName - $scriptCommand.Source.Contains("$env:HOME/.local") | Should -Be $true - } - It "Should not install resource that is already installed" { Install-PSResource -Name $testModuleName -Repository $PSGalleryName -TrustRepository $pkg = Get-PSResource $testModuleName From 9f7f21f62d55ab12ca564ea9b37b261ebd549dd6 Mon Sep 17 00:00:00 2001 From: Anam Navied Date: Tue, 30 Aug 2022 13:11:16 -0400 Subject: [PATCH 8/8] simplify scope based determination of environment PATH variable --- src/code/InstallHelper.cs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs index 1111c3667..030ae1629 100644 --- a/src/code/InstallHelper.cs +++ b/src/code/InstallHelper.cs @@ -590,22 +590,12 @@ private List InstallPackage( if (!_savePkg && !isModule) { string installPathwithBackSlash = installPath + "\\"; - if (scope == ScopeType.CurrentUser) - { - string currentUserPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.User); - if (!currentUserPathEnvVarValue.Contains(installPath) && !currentUserPathEnvVarValue.Contains(installPathwithBackSlash)) - { - _cmdletPassedIn.WriteWarning(String.Format(ScriptPATHWarning, scope, installPath)); - } - } - else + string envPATHVarValue = Environment.GetEnvironmentVariable("PATH", + scope == ScopeType.CurrentUser ? EnvironmentVariableTarget.User : EnvironmentVariableTarget.Machine); + + if (!envPATHVarValue.Contains(installPath) && !envPATHVarValue.Contains(installPathwithBackSlash)) { - // ScopeType.AllUsers - string allUsersPathEnvVarValue = Environment.GetEnvironmentVariable("PATH", EnvironmentVariableTarget.Machine); - if (!allUsersPathEnvVarValue.Contains(installPath) && !allUsersPathEnvVarValue.Contains(installPathwithBackSlash)) - { - _cmdletPassedIn.WriteWarning(String.Format(ScriptPATHWarning, scope, installPath)); - } + _cmdletPassedIn.WriteWarning(String.Format(ScriptPATHWarning, scope, installPath)); } } }