-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix forwarding DOTNET_ROOT #49634
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
base: main
Are you sure you want to change the base?
Fix forwarding DOTNET_ROOT #49634
Conversation
|
||
// We rename env variable to support --arch switch that relies on DOTNET_ROOT/DOTNET_ROOT(x86) | ||
// We provide VSTEST_WINAPPHOST_ only in case of testhost*.exe removing VSTEST_WINAPPHOST_ prefix and passing as env vars. | ||
return (hasRootVariable, $"VSTEST_WINAPPHOST_{rootVariableName}", rootValue); |
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.
I'm double checking, this is fine because this logic isn't going to be used with old vstest.console.exe because it's guaranteed that we use vstest.console.exe that's part of SDK, and we will only merge this PR after inserting the fix on vstest side. Is that correct?
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.
Yes, the old way of doing this can be removed. It also does not seem that people use that env variable on its own.
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.
This feels much better than what I originally had in mind. Hopefully it's going to work the way we expect it to :)
If we needed to service this in .NET 9 or 8, I assume we will need to also service the respective vstest versions inserted in 8 and 9?
Use VSTEST_DOTNET_ROOT_PATH and VSTEST_DOTNET_ROOT_ARCHITECTURE to tell to child vstest.console where the dotnet sdk is, so we can use it to setup dotnet_root for testhost.exe correctly, so testhost can pick up privately instaled dotnet sdk.
Needs microsoft/vstest#15184 to be merged first.
Fix #49436