Handle versioned #:sdk being first
#49807
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue 49797 by ensuring versioned #:sdk directives at the top of a C# script are handled correctly for both dotnet run and dotnet project convert.
- Added
SdkReference_VersionedSdkFirsttest inRunFileTests.cs - Added
Directives_VersionedSdkFirsttest inDotnetProjectConvertTests.cs - Updated
VirtualProjectBuildingCommandto split SDK name and version, adjust imports, and remove the oldToSlashDelimitedStringmethod
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | New fact validating build when a versioned #:sdk is first |
| test/dotnet.Tests/CommandTests/Project/Convert/...ConvertTests.cs | New fact validating conversion when a versioned #:sdk is first |
| src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs | Logic to correctly handle first versioned SDK directive and adjust import statements |
|
@RikkiGibson @jaredpar please review a small but important bugfix, thanks |
|
@RikkiGibson @333fred for reviews, thanks |
| { | ||
| string slashDelimited = firstSdkVersion is null | ||
| ? firstSdkName | ||
| : $"{firstSdkName}/{firstSdkVersion}"; |
There was a problem hiding this comment.
More of an abstract question, but given that this is a syntax that project files support today, it seems reasonable that users may expect to write #:sdk MySdk/1.0. Is that something we support or plan to support?
There was a problem hiding this comment.
We used to support that but we unified on a single separator in #48841 to avoid having different syntaxes in the wild leading to confusions. If user writes #:sdk MySdk/1.0 they should get an error message which tells them to use @ instead.
|
/backport to release/10.0.1xx-preview7 |
|
Started backporting to release/10.0.1xx-preview7: https://github.com/dotnet/sdk/actions/runs/16450222540 |
Fixes #49797.