Skip to content

Handle versioned #:sdk being first #49807

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 22, 2025
Merged

Handle versioned #:sdk being first #49807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 27 additions & 10 deletions src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,13 +578,20 @@ public static void WriteProjectFile(
var packageDirectives = directives.OfType<CSharpDirective.Package>();
var projectDirectives = directives.OfType<CSharpDirective.Project>();

string sdkValue = "Microsoft.NET.Sdk";
string firstSdkName;
string? firstSdkVersion;

if (sdkDirectives.FirstOrDefault() is { } firstSdk)
{
sdkValue = firstSdk.ToSlashDelimitedString();
firstSdkName = firstSdk.Name;
firstSdkVersion = firstSdk.Version;
processedDirectives++;
}
else
{
firstSdkName = "Microsoft.NET.Sdk";
firstSdkVersion = null;
}

if (isVirtualProject)
{
Expand All @@ -607,13 +614,28 @@ public static void WriteProjectFile(
</ItemGroup>

<!-- We need to explicitly import Sdk props/targets so we can override the targets below. -->
<Import Project="Sdk.props" Sdk="{EscapeValue(sdkValue)}" />
""");

if (firstSdkVersion is null)
{
writer.WriteLine($"""
<Import Project="Sdk.props" Sdk="{EscapeValue(firstSdkName)}" />
""");
}
else
{
writer.WriteLine($"""
<Import Project="Sdk.props" Sdk="{EscapeValue(firstSdkName)}" Version="{EscapeValue(firstSdkVersion)}" />
""");
}
}
else
{
string slashDelimited = firstSdkVersion is null
? firstSdkName
: $"{firstSdkName}/{firstSdkVersion}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

writer.WriteLine($"""
<Project Sdk="{EscapeValue(sdkValue)}">
<Project Sdk="{EscapeValue(slashDelimited)}">

""");
}
Expand Down Expand Up @@ -776,7 +798,7 @@ public static void WriteProjectFile(

if (!sdkDirectives.Any())
{
Debug.Assert(sdkValue == "Microsoft.NET.Sdk");
Debug.Assert(firstSdkName == "Microsoft.NET.Sdk" && firstSdkVersion == null);
writer.WriteLine("""
<Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
""");
Expand Down Expand Up @@ -1136,11 +1158,6 @@ private Sdk() { }
Version = sdkVersion,
};
}

public string ToSlashDelimitedString()
{
return Version is null ? Name : $"{Name}/{Version}";
}
}

/// <summary>
Expand Down
27 changes: 27 additions & 0 deletions test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,33 @@ public void Directives_Duplicate()
]);
}

[Fact] // https://github.com/dotnet/sdk/issues/49797
public void Directives_VersionedSdkFirst()
{
VerifyConversion(
inputCSharp: """
#:sdk [email protected]
Console.WriteLine();
""",
expectedProject: $"""
<Project Sdk="Microsoft.NET.Sdk/9.0.0">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>{ToolsetInfo.CurrentTargetFramework}</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<PublishAot>true</PublishAot>
</PropertyGroup>

</Project>

""",
expectedCSharp: """
Console.WriteLine();
""");
}

private static void Convert(string inputCSharp, out string actualProject, out string? actualCSharp, bool force, string? filePath)
{
var sourceFile = new SourceFile(filePath ?? "/app/Program.cs", SourceText.From(inputCSharp, Encoding.UTF8));
Expand Down
15 changes: 15 additions & 0 deletions test/dotnet.Tests/CommandTests/Run/RunFileTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1555,6 +1555,21 @@ public void SdkReference()
.Should().Pass();
}

[Fact] // https://github.com/dotnet/sdk/issues/49797
public void SdkReference_VersionedSdkFirst()
{
var testInstance = _testAssetsManager.CreateTestDirectory();
File.WriteAllText(Path.Join(testInstance.Path, "Program.cs"), """
#:sdk [email protected]
Console.WriteLine();
""");

new DotnetCommand(Log, "build", "Program.cs")
.WithWorkingDirectory(testInstance.Path)
.Execute()
.Should().Pass();
}

[Theory]
[InlineData("../Lib/Lib.csproj")]
[InlineData("../Lib")]
Expand Down
Loading