8000 Fix detection of $PSHOME in front of $env:PATH by SteveL-MSFT · Pull Request #11141 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fix detection of $PSHOME in front of $env:PATH #11141

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

Jump to bottom
Merged
TravisEz13 merged 2 commits into from
Nov 23, 2019
Merged

Fix detection of $PSHOME in front of $env:PATH #11141

TravisEz13 merged 2 commits into from
Nov 23, 2019

Conversation

@SteveL-MSFT
Copy link
Member
@SteveL-MSFT SteveL-MSFT commented Nov 21, 2019 •
edited
Loading

PR Summary

Previous code was incorrectly detecting if $PSHOME was in front using .Contains() since a preview would have the path "$PSHOME\preview" in front. Since this affects startup, we don't want to do a bunch of string manipulation with Split() so to cover the most common case we just check if the start of $env:PATH contains $PSHOME with the path separator and if not add to front. Two scenarios that aren't covered:

  • If $PSHOME is somewhere else in $env:PATH it will be duplicated. This is unlikely and will just have a small perf impact searching $env:PATH.
  • If $env:PATH only contains $PSHOME (without the path separator), then it'll also be duplicated, but this is also unlikely with minor impact.

PR Context

PR Checklist

PoshChan
PoshChan reviewed Nov 21, 2019
View reviewed changes
@TravisEz13 TravisEz13 added this to the rc.1-consider milestone Nov 23, 2019
@TravisEz13 TravisEz13 added MustHave CL-Engine Indicates that a PR should be marked as an engine change in the Change Log labels Nov 23, 2019
@TravisEz13 TravisEz13 merged commit a1c2d3c into PowerShell:master Nov 23, 2019
@SteveL-MSFT Steve 8000 L-MSFT deleted the pshome-path branch November 27, 2019 16:12
TravisEz13 pushed a commit that referenced this pull request Dec 4, 2019
@SteveL-MSFT @TravisEz13
Fix detection of $PSHOME in front of $env:PATH (#11141)
f3038fe
@TravisEz13 TravisEz13 modified the milestones: rc.1-approved, 7.0.0-rc.1 Dec 4, 2019
@ghost
Copy link
ghost commented Dec 16, 2019

🎉v7.0.0-rc.1 has been released which incorporates this pull request.:tada:

Handy links:

@kkshinkai kkshinkai mentioned this pull request Jul 28, 2021
Closed
@TravisEz13 TravisEz13 mentioned this pull request Oct 12, 2022
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@anmenaga anmenaga Awaiting requested review from anmenaga

+1 more reviewer

@PoshChan PoshChan PoshChan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 TravisEz13

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log MustHave

Projects

None yet

Milestone

7.0.0-rc.1

Development

Successfully merging this pull request may close these issues.

4 participants

@SteveL-MSFT @daxian-dbw @TravisEz13 @PoshChan
0