-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Remove WSLENV empty check from IsMicrosoftWSL #10711
Conversation
Can one of the admins verify this patch? |
Could you add more detail to the PR description? I don't quite understand the problem that is being solved, and why this is the best approach to fix it. |
We have code that tries to detect if we're trying to run a Windows executable within a WSL shell, which should be running a Linux binary. The most reliable way to do that is to check environment variables that WSL sets. Unfortunately, one of the 3 variables we picked (WSLENV) can be set in Powershell, outside of WSL, as a way of carrying data across connections. This is causing some Windows users to not be able to use minikube in a valid config. |
/ok-to-test |
kvm2 Driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just built this for Windows and can confirm this fixes the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just built this for Windows and can confirm this fixes the issue.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: NatsumiHB, spowelljr The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes #10689
Removed WSLENV empty check from IsMicrosoftWSL due to Windows Terminal setting the variable outside of WSL.
#10711 (comment)