CZEMacLeod / MSBuild.SDK.SystemWeb

This MSBuild SDK is designed to allow for the easy creation and use of SDK (shortform) projects targeting ASP.NET 4.x using System.Web.
MIT License
151 stars 8 forks source link

Setting RoslynToolPath to allow ASPX compilation with newer language versions #55

Closed khellang closed 1 year ago

khellang commented 1 year ago

Hey @CZEMacLeod! πŸ‘‹πŸ»

Thanks for stepping up and creating this amazing SDK πŸ™πŸ»

What do you think about setting RoslynToolPath in the SDK to allow the Roslyn CodeDOM provider to pick up the newer version of the compiler?

We've solved this by using GeneratePathProperty on PackageReference:

<ItemGroup>
  <PackageReference Include="Microsoft.Net.Compilers.Toolset" Version="4.5.0" IncludeAssets="none" PrivateAssets="all" GeneratePathProperty="true" />
</ItemGroup>

And then setting RoslynToolPath:

<PropertyGroup>
  <RoslynToolPath>$(PkgMicrosoft_Net_Compilers_Toolset)\tasks\net472</RoslynToolPath>
</PropertyGroup>

This allows us to pass /langversion:11 to the CodeDOM provider and compile our ASPX-files using the newest version of the language.

Are you willing to accept a PR for this?

CZEMacLeod commented 1 year ago

@khellang This functionality should already be present in the SDK. It includes Microsoft.CodeDom.Providers.DotNetCompilerPlatform which causes the aspx build to use Roslyn, and it includes a version of Microsoft.Net.Compilers.Toolset too. You shouldn't need to do anything, except set the <LangVersion>10</LangVersion> property. The version that is included by the SDK is currently V4.3.0 so should handle up to 10. If you need LangVersion 11, then you can force V4.5.0 by setting the property <MicrosoftNetCompilersToolset_Version >4.5.0</MicrosoftNetCompilersToolset_Version> in your project file.

khellang commented 1 year ago

Just including Microsoft.Net.Compilers.Toolset isn't enough to enable compilation of ASPX files with C# language versions above 7.3 - at least not with dynamic compilation. See https://stackoverflow.com/a/75605051/682105. The reason Microsoft.CodeDom.Providers.DotNetCompilerPlatform has the RoslynToolPath is because it ships with its own version of the compiler, which it copies to the application's bin folder. This is then used at runtime to compile views dynamically.

leusbj commented 1 year ago

@khellang As a temporary work around until something is figured out, perhaps something like the following in your Directory.Build.targets would get you what you need.

<ItemGroup>
  <PackageReference Update="Microsoft.Net.Compilers.Toolset">
    <GeneratePathProperty>true</GeneratePathProperty>
  </PackageReference>
</ItemGroup>

This Sdk's default/automatical inclusion of the Microsoft.Net.Compilers.Toolset PackageReference will happen before Directory.Build.targets is processed, so using an update style statement there would allow you to set the GeneratePathProperty .

khellang commented 1 year ago

I have everything working nicely, so no need for a workaround. I was simply pitching it to be included in the SDK by default ☺️

CZEMacLeod commented 1 year ago

@khellang I've pushed a small update V4.0.88 which should now natively do this. I thought the Microsoft.Net.Compilers.Toolset package set the RoslynToolPath property internally (and I'm slightly surprised it doesn't). There is an internal property _RoslynTasksDirectory which is used by MSBuild, but not a TargetFramework (or resolved framework) based one for RoslynToolPath. This change was included as part of some other ongoing work, but in general, I love feedback and enhancements and generally accept PRs.

khellang commented 1 year ago

It does set it, but it brings its own version of the compiler, matching the version number. So currently the latest version is 3.6.0. This results in the older version of the compiler being used for dynamic compilation at runtime, not the version from the Toolset package ☺️

CZEMacLeod commented 1 year ago

@khellang I think that is where I got confused. Microsoft.CodeDom.Providers.DotNetCompilerPlatform seems to embed a couple of versions of Roslyn depending on the target version of .net and yes - it sets RoslynToolPath based on the last version of Microsoft.Net.Compilers that was embedded in the package.

I thought that Microsoft.Net.Compilers.Toolset (the replacement for Microsoft.Net.Compilers) set RoslynToolPath to point to the newer version included. However, it isn't quite that simple as the version required at runtime and the version require for MSBuild are not the same.

In our case, the SDK only support V4.7.2+ of .NET Framework so it is easier to manage this.

Regardless, the new SDK version does the magic and sets the property from the package using the method you mentioned so no more mismatches between build time and runtime compilers.

Thanks for the input and the clarifications.

khellang commented 1 year ago

Yeah, the change looks good 😎 Thank you so much! πŸ‘πŸ»