Cleanup DOTNET_ROOT logic for dotnet run #49913
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes legacy .NET 5.0 compatibility logic from the DOTNET_ROOT environment variable handling in the dotnet run command. The cleanup simplifies the codebase by removing support for .NET 5.0 and earlier versions, which reached end-of-support in May 2022.
Key changes:
- Removed TargetFrameworkVersion parameter and related logic from DOTNET_ROOT variable name resolution
- Simplified test cases to remove .NET 5.0-specific scenarios and legacy 32-bit process handling
- Consolidated environment variable naming to use architecture-specific suffixes consistently
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Common/EnvironmentVariableNames.cs |
Removes TargetFrameworkVersion parameter and legacy logic, simplifies to always use architecture-specific DOTNET_ROOT variables |
src/Cli/dotnet/Commands/Run/RunCommand.cs |
Updates call site to remove TargetFrameworkVersion parameter |
test/dotnet.Tests/EnvironmentVariableNamesTests.cs |
Simplifies test cases by removing .NET 5.0 scenarios and legacy 32-bit process handling |
Comments suppressed due to low confidence (2)
test/dotnet.Tests/EnvironmentVariableNamesTests.cs:58
- This test case expects 'DOTNET_ROOT' for x86 architecture, but the implementation now always returns architecture-specific variables like 'DOTNET_ROOT_X86'. This test case appears to be testing behavior that no longer exists.
[InlineData("os-x86", Architecture.X86, "DOTNET_ROOT")]
test/dotnet.Tests/EnvironmentVariableNamesTests.cs:59
- This test case expects 'DOTNET_ROOT' for x64 architecture, but the implementation now always returns architecture-specific variables like 'DOTNET_ROOT_X64'. This test case appears to be testing behavior that no longer exists.
[InlineData("os-x64", Architecture.X64, "DOTNET_ROOT")]
|
@elinor-fung @baronfel Can I have your eyes on this change please? Note: I want to reuse that for |
The logic around TargetFrameworkVersion appears to exist to support apps targeting net5.0 or earlier.
.NET 5 was out of support on May 10, 2022. I guess .NET 10 can safely drop this and clean this up?
cc @elinor-fung @baronfel Let me know your thoughts here please, and if there is something I'm missing.
This PR changes the behavior in the case of app targeting the same architecture as SDK, or app is targeting an unknown architecture. In this case:
DOTNET_ROOTDOTNET_ROOT(x86)DOTNET_ROOT_<ARCH>DOTNET_ROOT_<ARCH>, regardless of the target framework.