Fix template resolution ordering bug #51861
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #51669
Summary
The issue was that with SDK 9 and SDK 10 installed, the templates used when running
dotnet newwould prefer 9 over 10. This was happening because when we resolve the template folders, we decided to order the folders. However, the order of the folders was determined by the string key, which is just the string value of the folder name. When sorting strings, they'll be alphabetical, so"10"comes before"9"because 1 is less than 9. To fix, I simply sort them by the (already parsed) SemanticVersion value of the folder name. Thus, 9 would come before 10.Testing
For local testing, it would build
10.0.0as the template folder name. I copied a template folder from an installed SDK 9,9.0.11, and dropped it in there beside the 10 folder. The local template directory path isartifacts\bin\redist\Debug\dotnet\templates. I copied a 9 version out of the installed path ofC:\Program Files\dotnet\templates.For
dotnet new sln, the 10 version creates a.slnxfile. For the 9 version, a.slnfile. So, testing was just adding and removing the 9 folder and observing the unintentional change in behavior. When the 9 folder is added, only a.slnwould be created. After making the change to the order withinGetBestVersionsByMajorMinor, I observed that when adding/removing the 9 folder, the 10 templates were now always being loaded. Meaning, a.slnxwas always created, which is the intended behavior.Additionally, I fought a bit with Copilot to create a couple of unit tests. It originally created tests that would not actually test this issue. Meaning, I could revert my change and the test would still pass. 😟 I got it to revise it a couple times and it ended up making okay-enough tests that just check that 9/10 barrier. I made the method internal so the unit tests could directly run it. The test project has InternalsVisibleTo access to the CLI project.