Update PATH envrionment variable for package manager executable on Windows #25847
Conversation
src/System.Management.Automation/engine/NativeCommandProcessor.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/engine/NativeCommandProcessor.cs
Outdated
Show resolved
Hide resolved
test/powershell/Language/Scripting/NativeExecution/NativeCommandPathUpdate.Tests.ps1
Show resolved
Hide resolved
test/powershell/Language/Scripting/NativeExecution/NativeCommandPathUpdate.Tests.ps1
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Re-run failed MacOS CIs... |
|
It constantly fails, and the macOS CI in #25864 fails the same way. I will check with @TravisEz13 tomorrow to find out why. |
4f81f71 to
3ae6df7
Compare
|
A Runspace in PowerShell could reuse the same thread, so a thread-static variable is not guaranteed to work as expected. |
| } | ||
|
|
||
| // Disable Path Update if 'EnvironmentProvider' is disabled in the current session, or the current session is restricted. | ||
| bool enabled = context.EngineSessionState.Providers.ContainsKey(EnvironmentProvider.ProviderName) |
There was a problem hiding this comment.
Which brings us back to the question of is the cache worth it? In my opinion, containsKey is much easier than ConditionalWeakTable.
There was a problem hiding this comment.
Now there is also a call to Utils.IsSessionRestricted(context) for the check, which does a command lookup. And it's possible to have more checks in future depending on the need in restricted scenarios.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
📣 Hey @@daxian-dbw, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Implementation for PowerShell/PowerShell-RFC#391. For the PS team members, here is the internal PR for the equivalent change in CMD: https://microsoft.visualstudio.com/OS/_git/os.2020/pullrequest/10131240
A few implementation decisions:
Check the package manager list only once per process (same as CMD). So, changing the package manager list in registry doesn't affect a PowerShell session that is already running.
When detecting user/system PATH value changes, only check new items PREPENDED or APPENDED to the original string (same as CMD). The detection is made simple, which is not intended to detect complex changes to the user/system PATH.
The PATH update feature is disabled when the
Environmentprovider is not available, or when the current session is restricted.Environmentprovider andIsSessionRestrictedare checked only once per PowerShell session.ConditionalWeakTableto cache whether the feature is enabled or not for a given session.thread-staticvariable for the caching because it's possible for Runspace to reuse threads.Tests have been added.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header