Skip to content

Version Specific $ENV:PSModulePath Variables #133

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

Closed
wants to merge 1 commit into from

Conversation

BrucePay
Copy link
Contributor

Initial draft of "Version-Specific $ENV:PSModulePath Variables" RFC.

@iSazonov
Copy link
Contributor

From: PowerShell/PowerShell#7324
I wonder why do we even look for standart/core modules by env:PSModulePath? If we hard-code the path to <pwsh.exe dir>\Modules internally and remove it from env:PSModulePath the problem will go away.
Seems it helps for side-by-side scenarios too.

@daxian-dbw daxian-dbw changed the title Initial draft. Version Specific $ENV:PSModulePath Variables Jul 26, 2018
@SteveL-MSFT
Copy link
Member

If we stay with env var, I noticed another issue that would not be resolved by having $env:PSModulePath6. I was testing a private build of PSCore6 from PSCore6.1. Because the child pwsh inherited the 6.1 paths, those modules are imported first and complain that they can't load because they require 6.1. We could add some code to avoid this, but that seems more like a patch than a solution. Perhaps we should just move away from $env:PSModulePath altogether and require modules to just be in the 3 locations (shared, user, and $PSHOME). The actual paths can be in the configuration json file.

@markekraus
Copy link

@SteveL-MSFT moving away from the env var may be problematic if there is no way to modify the module paths in-process. Several of my work flows for module dev involve pre-prending the curren PSModulePath with the location of the build output. I also use this to allow a folder to serve as an alternate to the 3 paths in pipelines. IMO, this doesn't need to be an env var, but it does need to retain the ability to add paths in-process.

@iSazonov
Copy link
Contributor

@markekraus Do you mean that you redirect PSHome\Modules?

@markekraus
Copy link

@iSazonov No. I mean that I regularly prepend paths to PSModulePath.

For example, In-pipeline I will add c:\git\<project>\inc\powershell\modules to the beginning of $env:PSModule to allow for loading project specific modules. This allows for clobbering commands which are being over written or wrapped by giving the modules in that path a higher priority.

It would be painful to be forced to install the modules every time, or to be forced to import the modules via full path just because PSModules path is now restricted to 3 locations.

This kind of functionality needs to be preserved, but, it doesn't have to be a env var. so long as the existing paths can be inspected and modified. For example, it could be a cmdlet.

@iSazonov
Copy link
Contributor

@markekraus Thanks! My suggestion is only hard-code paths to standard modules (Microsoft.PowerShell.Diagnostics Microsoft.PowerShell.Host, Microsoft.PowerShell.Management, Microsoft.PowerShell.Security, Microsoft.PowerShell.Utility ) and always load them from PSHome\Modules.

@markekraus
Copy link

@iSazonov I'm indifferent to that. My only concern is what @SteveL-MSFT said:

Perhaps we should just move away from $env:PSModulePath altogether and require modules to just be in the 3 locations (shared, user, and $PSHOME). The actual paths can be in the configuration json file.

I don't care how or where the original paths are set for the current process. pwsh 6.0.0 broke the usefulness of the env var anyway as it doesn't get passed to child pwsh processes such as jobs or calling pwsh.exe. But I do care that we should be able to change these paths (add, remove, replace) in-process somehow. If the paths are no longer in an ENV VAR, than we need some cmdlets to manage the paths. Or if it's now managed as a magic variable.. or whatever. Just so long as we are not locked down to only the paths that are arbitrarily decided before my code is running.

@iSazonov
Copy link
Contributor

We could consider limiting the paths as security option for closed environments.

@Jaykul
Copy link
Contributor

Jaykul commented Jul 30, 2018

I wish there was at least one place on the default lists where compatible modules could be installed. Right now we're all juggling, copying, updating and making a proper mess of things just trying to keep our modules up to date in both shells, and it's very frustrating.

I've never really liked the idea of having multiple path variables -- modules were supposed to work and separate paths just means that I'll have to modify my shared profile to update both/all of them to include my development folders.

However, the fact that PowerShell 6 doesn't properly inherit the "environment" variable is a really bad bug, and ...

I'll happily accept this as a work around

It's pretty clear that most people (and production servers) are going to use either PowerShell 5 or PowerShell 6 and not a mix, and just need each of them to work. They don't have my problems, but they do have a problem with the non-environmental nature of the current PowerShell 6 PSModulePath.

P.S. I thought there already was code protecting core modules from being loaded from elsewhere?

@Jaykul
Copy link
Contributor

Jaykul commented Jul 30, 2018

Absolutely not @iSazonov

The only reason why PSModulePath exists is as a convenience, just like every other PATH environment variable. You can't lock it down for security because it's emphatically not a security risk in the first place.

We have tools for preventing attackers from running malicious code. Making it harder for people to find things is not one of them.

P.S. Please note that Import-Module not only accepts direct paths, it doesn't even need anything on disk at all (see New-Module).

@kilasuit
Copy link

kilasuit commented Jul 30, 2018

@markekraus - you can change PSModulePath in process (at least on windows) with

[System.Environment]::SetEnvironmentVariable('PSModulePath','C:\Code\;'+$env:PSModulePath,'Process')

not tested this off windows though but this does work on 6.1.0-preview3 on win10

image

Edit confirmed this works on Ubuntu with 6.0.3 as per above

@Jaykul
Copy link
Contributor

Jaykul commented Jul 30, 2018

I just re-read the previous comments, and I have to say (for what it's worth) that I think the PSModulePath6 really needs to be an Environment path variable as proposed in this change.

Please let's not consider changing it into something other than an Environment variable, or altering it on load (with the possible exception of appending paths), or changing command or module precedence to ignore the PSModulePath -- these things were tried before and did not turn out well (I can forward email threads from 2014 when a change "to prefer PowerShell’s built-in modules" was made "for both security and reliability reasons" the last time, if we don't remember this).

The computing community have been managing available applications/commands (and libraries) via environment path variables for decades (and working around minor gotchas with inheritance).

If you simply make this an environment variable that inherits properly, every single one of these other problems are easily worked around in profile scripts which we all know how to write and are problems which computer users have been dealing with successfully for a very long time.

P.S. @kilasuit I think Mark's trying to say it needs to be changeable, in response to Steve's comment suggesting it could be hard-coded instead of an environment variable.

@markekraus
Copy link

@kilasuit What @Jaykul suggested I meant is correct. I'm aware it can be changed in-process now (I do this often), but what @SteveL-MSFT proposed appears to make that impossible once implemented.

@SteveL-MSFT
Copy link
Member

@markekraus the developer scenario is one we want to support. Having application installers extend PSModulePath, however, was always a bad practice. I would certainly support cmdlets to manage the "path" even we decide to go the non-env-var route.

However, even with the env-var route, the current proposal, as written, has the problem of starting different versions of PowerShell from PowerShell which is not common, but not rare either.

@joeyaiello
Copy link
Contributor

joeyaiello commented Feb 4, 2019

Agnostic of implementation (this may be impossible), we think there's three scenarios that need to addressed here:

  • starting powershell from pwsh
  • starting pwsh from powershell
  • starting pwsh from pwsh

In the first case, we think that it's less likely that PS Core modules will work in Windows PowerShell. People will be calling it for legacy behavior, and they will wants Windows PowerShell to start as it does when created from a fresh process. To that end, if it's in the registry, we should replace what is passed by the parent pwsh process.

In the second case, you may very well want both Windows PS and PS Core modules to run. Given our lean towards attempting backwards compatibility (especially as we move into .NET Core 3.0), we should join the inherited PSModulePath with the default PS Core paths (Users, Program Files, and system32).

In the last case, we think that users operating in a purely PS Core world should do the right thing and modify their manifests and use #requires to prevent incompatible modules in the same shared PSModulePaths from loading. To that end, pwsh called from pwsh should just do basic inheritance per normal environment variable semantics.

In any case, we will not create a new environment variable, as it's a sweeping change to the existing module semantic that can ultimately be avoided with well-documented asymmetry.

@iSazonov
Copy link
Contributor

iSazonov commented Feb 5, 2019

In any case, we will not create a new environment variable, as it's a sweeping change to the existing module semantic that can ultimately be avoided with well-documented asymmetry.

@joeyaiello Could you please clarify? Everyone above agreed that we have to have env:PSModulePath6 to address all these three scenarios. Also you forget about side-by-side pwsh-s of different versions scenario.

@joeyaiello
Copy link
Contributor

Had more discussion on this with @JamesWTruher and @HemantMahawar today. A few notes:

  • we still don't have community consensus on this, and whatever we decided should be very well thought-out
  • I'm in favor of @Jaykul 's initial comment that the inheritance behavior needs to be fixed first and foremost

The scenario I'm trying to solve for is this: PSModulePath often gets set by administrators when deploying additional packages to an IT environment. We want PowerShell Core to err on the side of backwards compatibility and expose all modules that administrators believe should be part of their PowerShell environment.

This will expose potentially incompatible modules to PowerShell Core, but our goal is to continue maximizing back compat in a post-.NET Core 3.0 world, and we don't want to make IT administrators opt in to a world by pushing new policy that is specific to something like PSModulePath6 or PSModulePathCore.

Additionally, the pain of incompatible modules has been our single most powerful lever for updating module compatibility, so surfacing more modules in PS Core will help us increase compat.

More succinctly than I stated above:

  • PowerShell Core should inherit the Windows PowerShell environment variable and add its defaults
  • Window PowerShell should not inherit the environment variable from PowerShell Core, but should replace it with what's in the registry. If folks want to modify from here, using profiles, -Command, or modify it at runtime
  • PowerShell Core should respect PowerShellVersion and not load modules that are incompatible. If modules are side-by-side, it should load the highest version that is compatible.
  • show all found modules in Get-Module -ListAvailable (as we already do with PSEdition) and add PowerShellVersion to the table output of Get-Module -ListAvailable

@iSazonov
Copy link
Contributor

iSazonov commented Feb 14, 2019

Side-by-Side scenarios we need consider:

  1. Windows Powershell and pwsh
  • we need keep Windows PowerShell run:

    • if pwsh installed
    • if pwsh-only modules installed

    starting

    • from cmd
    • from pwsh
  • we need keep pwsh run:
    starting

    • from cmd
    • from pwsh
    • from WindowsPowerShell
  1. Some pwsh versions including previews
  • we must ensure that each particular version runs its dlls and pre-installed modules (serviced by WindowsUpdate!), and not ones from other versions
  1. We need unified way to process modules
  • considering compatibility
  • every PowerShell version should discover modules:
    • in pre-defined, well-known places
    • in targeted by PSModulePath places

Initial problem in #7324 was:

Import-Module Microsoft.PowerShell.Utility
Import-Module : The version of Windows PowerShell on this computer is '5.1.18200.1000'. The module 'C:\program
files\powershell\6-preview\Modules\Microsoft.PowerShell.Utility\Microsoft.PowerShell.Utility.psd1' requires a minimum Windows PowerShell version of '6.1' to run. Verify that you have the minimum
required version of Windows PowerShell installed, and then try again.

That is - Windows PowerShell uses PSModulePath to discover modules and pwsh path is on first place. But we haven't on first place path to pwsh and Windows PowerShell at the same time.

So we can resolve this only having:

  • processing PowerShellVersion
  • loading pre-installed modules from pshome
  • no adding path with pwsh pshome to PSModulePath)

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Feb 25, 2019

@PowerShell/powershell-committee discussed this further. Current proposal is:

  • Do not introduce new $env:PSModulePath6 as it can cause confusion to existing PowerShell users
  • PSCore6 currently will create $env:PSModulePath from config.json file, a new setting EnvVarToAppendToPSModulePath with value PSModulePath (this means the $PSHOME/Modules and user paths are first, then appends the $env:PSModulePath from the system)
  • Some logic needs to be retained so child PSCore6 trims any duplicates in $env:PSModulePath
  • When pwsh is started, we preserve the original $env:PSModulePath. Have whitelist to special case powershell.exe and powershell_ise.exe so when they are started within pwsh, they receive the original $env:PSModulePath. In all other cases, they inherit the current $env:PSModulePath.
  • Since modules are auto-discovered by newest version and not by order of the path, there is a scenario where starting different versions of pwsh from pwsh may result in unexpected behavior.

This should solve the scenario where PSCore6 starting powershell.exe will have powershell.exe not inherit incompatible $env:PSModulePath and for PSCore6 started from powershell.exe to have the complete path.

@SteveL-MSFT SteveL-MSFT added this to the 6.3-Consider milestone Feb 25, 2019
@iSazonov
Copy link
Contributor

It seems this address nothing from my previous comment.

If we take the search list from the config file, then why do we need to change $env:PSModulePath at all?

For every pwsh process the search list should be recreated as $PSHOME/Modules+paths from +config.json+$env:PSModulePath

@joeyaiello
Copy link
Contributor

Would you mind rephrasing your concern? We're having a bit of difficulty parsing your feedback. In particular, it seems like you're primarily concerned about Windows PowerShell being started in PowerShell Core here:

That is - Windows PowerShell uses PSModulePath to discover modules and pwsh path is on first place. But we haven't on first place path to pwsh and Windows PowerShell at the same time.

But then this sounds like a concern about the other direction:

If we take the search list from the config file, then why do we need to change $env:PSModulePath at all?

Unless it's possible that you think we're permanently creating or overwriting an environment variable here?:

PSCore6 currently will create $env:PSModulePath from config.json file

That one will just be created in process, so that users can use the existing mechanisms for editing it in a runspace, and so subsequent pwsh instances created in that pwsh inherit it properly like shells are supposed to.

@iSazonov
Copy link
Contributor

iSazonov commented Mar 5, 2019

Sorry my feedback was confusing. My feedback was that "all is bad". :-)
I will try to set out in steps.

  1. We shouldn't/won't change/patch Windows PowerShell.
  2. Any changes in $env:PSModulePath lead to problems. We tried add paths to end and to begin $env:PSModulePath and in both cases had problems. Such direct solution doesn't work.
  3. So conclusion is - to preserve Windows PowerShell behavior pwsh shouldn't change $env:PSModulePath.
  4. In the case pwsh has a problem finding pre-installed modules in $Home.
  5. As mentioned above we tried to resolve this by adding the path to $env:PSModulePath. Now we concluded in p.3 that we don't do that.
  6. Proposed solution is:
  • pwsh gets $env:PSModulePath - the same as Windows PowerShell
  • pwsh already recognizes compatible Windows PowerShell pre-installed modules - so no problem.
  • pwsh internally convert $env:PSModulePath to module search list
  • the search list is preceded by $Home path (without modifying $env:PSModulePath)
    • and more - I'd preferred that finding the core modules (such as Management, Utility...) will be hard-coded to $Home/modules because they are an integral part of the pwsh version, depend on the engine/sma and should never be loaded into another version.
  • the search list re-evaluated if $env:PSModulePath is modified (in other RFC we want to have new cmdlets for this, yes?)
  • $env:PSModulePath is sent to sub-processes (Windows PowerShell and pwsh) "as is"
    • Windows PowerShell gets it "as is"
    • pwsh gets it "as is" and converts in search list
      • this works for pwsh of the same version and pwsh of another (side-by-side) version

@Jaykul
Copy link
Contributor

Jaykul commented Mar 7, 2019

I feel like you're over-engineering this, and worse, that you're doing so because you think your users are not smart enough to manage it on their own. Really, it's pretty simple:

  • It's an environment variable
    • Therefore, it must be inherited from the caller!
  • Environment variables are easily manipulated
    • The caller can even manipulate it before calling you (including with Start -UseNewEnvironment)
  • Each shell has it's own profile scripts, which can manipulate it

Conclusion: users can handle this on their own, as long as you stop breaking it.

Forcing your modules to the front is unnecessary, and breaks things for developers who configure their development paths in priority. You should only add your own paths if they're not already in the list at all.

Putting environment variables in config.json is confusing, obscure, and unecessary. If you want to do special environment stuff for ise or powershell, you can already do it the way any user can: by adding a function that calls them with Start -UseNewEnvironment ...

Remember that Daniel Webster quote?

A strong conviction that something must be done is the parent of many bad measures.

If you still feel that something must be done, I'd suggest:

  1. Add a parameter to Start-Process to let users pass in a hashtable of -EnvironmentVariables instead of, or along with, -UseNewEnvironment -- consider adding syntax emulating bash's ability to temporarily set environment variables only for a single invocation of a script or executable.
  2. Include commands for working with environment variables -- in particular, the ability to remove duplicates from a PATH-like variable, and to add values to it conditionally (without duplicating). There are modules out there for this already...

@joeyaiello
Copy link
Contributor

Environment variables are not always as simple as they seem. I'd argue that we're doing here is more akin to path_helper on macOS, where a binary gets executed to add values to PATH when shells start up. path_helper is also reading from a root-owned file (/etc/paths and /etc/paths.d).

The original point of this RFC was around the Windows PowerShell <--> PowerShell Core inheritance problem. I think we all agree that Windows PowerShell shouldn't be modified, but there's some tactical stuff we might be able to hack in to solve that problem.

Now, though, the scope has expanded in this conversation to talk about managing the environment variables on non-Windows platforms. And our discussion has turned to the login shell inheritance issue discussed in:

Whatever we do here should be rationalized with our handling of PATH as well. @SteveL-MSFT is going to open an RFC on that behavior, and we might need to rewrite this one to stay in lockstep with PATH.

@iSazonov
Copy link
Contributor

iSazonov commented May 7, 2019

Since MSFT team was announce PowerShell 7.0 as big step to replacement of Windows PowerShell the RFC may lose relevance at all.
Also now we have CompatiblePSEditions in psd1 files.
One gap I see that we still discover pre-installed modules (like Utilites) by the path variable although it should be hard-coded because the modules is compiled with the engine and can depends from internal apis.

@SteveL-MSFT
Copy link
Member

@iSazonov I agree that PS7 makes this somewhat irrelevant, however, we still have an issue with starting PS7 from WinPS and vice-versa. The solution might be to simply special case just those use cases.

@iSazonov
Copy link
Contributor

iSazonov commented May 7, 2019

@SteveL-MSFT I re-read the issue PowerShell/PowerShell#6850 and the RFC and think that we can change PS7 to inherit PSModulePath and share it with WinPS if we force to load modules only with CompatiblePSEditions in manifest (or force to use "-Force"). We could add option to preserve old behavior if this is needed in some scenarios. If we announce this now module owners will have time to update their modules.

@SteveL-MSFT
Copy link
Member

@PowerShell/powershell-committee reviewed this again. We've made a small edit to the proposal here which is what we are approving for PS7.

@iSazonov
Copy link
Contributor

Will it be fixed in new RFC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants