Skip to content
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.

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

Using Microsoft.Build.CentralPackageVersions causes a compile-time error #26

Closed
Timores opened this issue Nov 24, 2021 · 6 comments
Closed
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested

Comments

@Timores
Copy link

Timores commented Nov 24, 2021

When centralizing all package references in a single file (see https://github.com/microsoft/MSBuildSdks/tree/main/src/CentralPackageVersions), using this project causes a compile-time error, as src/MSBuild.SDK.SystemWeb/Sdk/Sdk.targets defines a version for the 2 referenced packages.

Note that a work-around is simple, as one can define the property ExcludeASPNetCompilers and reference the 2 packages directly in one's own csproj.

@CZEMacLeod
Copy link
Owner

Interesting issue - I'm not entirely sure what you want me to do about it though. As you say, the SDK already has the ExcludeASPNetCompilers flag for this and other scenarios.

I guess the flags etc. could have better documentation. Feel free to submit PRs for the code to add comments, and/or the docs.

I had a look at the MSBuildSdks/CentralPackageVersions source and unfortunately there isn't a property defined to state that CPV is in use and enabled.
Unlike with PackageReferences, there isn't a GeneratePathProperty on SDKs either.

Also, because it is imported so late on in the project, it is difficult to actually detect that the SDK is there.

The best I could come up with was:

Directory.Build.props

<Project>
	<ItemGroup Condition="'$(IncludeUnversionedASPNetCompilers)'=='true'">
		<PackageReference Include="Microsoft.Net.Compilers.Toolset" PrivateAssets="All"/>
		<PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" />
	</ItemGroup>
</Project>

Directory.Build.targets

<Project>
	<Import Project="Sdk.props" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />
	
	<!-- Your content here -->
	
	<Import Project="Sdk.targets" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />
	
	<PropertyGroup Condition="'$(EnableCentralPackageVersions)'!='false' AND '$(CentralPackagesFile)'!=''">
		<IncludeUnversionedASPNetCompilers Condition="'$(ExcludeASPNetCompilers)'=='false'">true</IncludeUnversionedASPNetCompilers>
		<ExcludeASPNetCompilers>true</ExcludeASPNetCompilers>
	</PropertyGroup>
</Project>

Packages.props

<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http:https://schemas.microsoft.com/developer/msbuild/2003">
	<ItemGroup>
		<PackageReference Update="Microsoft.Net.Compilers.Toolset"						Version="4.0.1" />
		<PackageReference Update="Microsoft.CodeDom.Providers.DotNetCompilerPlatform"   Version="3.6.0" />
	</ItemGroup>
</Project>

I've checked that this works with other project types in the solution, and multiple MSBuild.SDK.SystemWeb projects and they don't tread on each other's toes. It also allows you to actually set ExcludeASPNetCompilers to true in one project and it work as expected, but not affect others.

I'm just not quite sure of how we could adapt that to work inside the MSBuild.SDK.SystemWeb SDK itself right now to do the auto-detect.

I'm also not sure if it is something that should actually be in the SDK itself.

@Timores Can you let me know what you think is the way forward for this issue so that we can close it out.

@CZEMacLeod CZEMacLeod added documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested labels Nov 25, 2021
@Timores
Copy link
Author

Timores commented Nov 25, 2021

Thanks a lot for looking at this so quickly.
I could not find a way to auto-detect the use of CentralPackageVersion either.

My idea was to add support within MsBuild.SDK.SystemWeb for a property like IncludeUnversionedASPNetCompilers.

Like changing lines 6-9 of MSBuild.SDK.SystemWeb/src/MSBuild.SDK.SystemWeb/Sdk/Sdk.targets with

  <ItemGroup Condition="'$(ExcludeASPNetCompilers)'=='false'" and '$(IncludeUnversionedASPNetCompilers)' == 'false'>
    <PackageReference Include="Microsoft.Net.Compilers.Toolset" Version="3.8.0" PrivateAssets="All"/>
    <PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" Version="3.6.0" />
  </ItemGroup>
  <ItemGroup Condition="'$(ExcludeASPNetCompilers)'=='false'" and '$(IncludeUnversionedASPNetCompilers)' == 'true'>
    <PackageReference Include="Microsoft.Net.Compilers.Toolset" PrivateAssets="All"/>
    <PackageReference Include="Microsoft.CodeDom.Providers.DotNetCompilerPlatform" />
  </ItemGroup>

I am not sure to have understood your suggestion above (I am a noob with MSBuild and custom SDKs). What I understood is a suggestion to adapt the files of a solution using both CentralPackageVersions and SystemWeb. The changes here seem simpler to me, but your opinion in this case is worth more than mine.

@CZEMacLeod
Copy link
Owner

@Timores I would need to check the evaluation order - but your suggestion is reasonable. It basically puts what I put in Directory.Build.props into the Sdk.targets file.

You would still need the following changes in Directory.Build.targets

Directory.Build.targets

<Project>
	<Import Project="Sdk.props" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />
	
	<!-- Your content here -->
	
	<Import Project="Sdk.targets" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />
	
	<PropertyGroup Condition="'$(EnableCentralPackageVersions)'!='false' AND '$(CentralPackagesFile)'!=''">
		<IncludeUnversionedASPNetCompilers Condition="'$(ExcludeASPNetCompilers)'=='false'">true</IncludeUnversionedASPNetCompilers>
	</PropertyGroup>
</Project>

or you could manually set IncludeUnversionedASPNetCompilers in your MsBuild.SDK.SystemWeb project(s).

The problem is that you cannot detect the CentralPackagesFile until you have imported the Sdk.targets file of Microsoft.Build.CentralPackageVersions which happens after the Sdk.targets file of MsBuild.SDK.SystemWeb

This is why you cannot use

<Sdk Name="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />

and have to use the expanded form

<Import Project="Sdk.props" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />
<!-- Your content here -->
<Import Project="Sdk.targets" Sdk="Microsoft.Build.CentralPackageVersions" Version="2.0.1" />

and then put the detection code after that.

Let me think on this more; I am not sure how many people this change could impact, or how many people are using both systems together versus the amount of effort in adding these changes.

@Timores
Copy link
Author

Timores commented Nov 26, 2021

Indeed, the solution should not be too complex. I currently set ExcludeASPNetCompilers to true in my Web project. Which is not a big deal.
Could you explain the role of the 2 packages that are not referenced? What would not work in a Web projet without them? (I have no issue adding them manually,)

@CZEMacLeod
Copy link
Owner

@Timores I have actually had feedback from @jeffkl in microsoft/MSBuildSdks#320 regarding detecting CPV.
With regards to the two packages

  • Microsoft.Net.Compilers.Toolset - This forces the latest (or specific) compiler tools to be used, regardless of the version installed with MSBuild and/or Visual Studio. It is mainly helpful to build servers which may have a different/older version of the compilers installed.
  • Microsoft.CodeDom.Providers.DotNetCompilerPlatform - This forces webforms type files to compile on the webserver with the latest roslyn based compilers rather than the native ones built into .NET 4.x. This allows you to use newer language features in your markup pages when they are compiled using aspnet_compiler or on the server. It effectively replaces the native implementations when you (or the framework) calls System.CodeDom.Compiler.CodeDomProvider. It also contains a number of fixes and workarounds for issues where the background compiler service is still running when the webapplication terminates, which may lock files in you bin folder which then prevents you from re-compiling.

For the latter, you need to make sure that the follow lines are in your web.config (and if you don't include the package, remove them - although you then lose the benefits).

  <system.codedom>
    <compilers>
      <compiler language="c#;cs;csharp" extension=".cs" type="Microsoft.CodeDom.Providers.DotNetCompilerPlatform.CSharpCodeProvider, Microsoft.CodeDom.Providers.DotNetCompilerPlatform, Version=3.6.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" warningLevel="4" compilerOptions="/langversion:default /nowarn:1659;1699;1701" />
      <compiler language="vb;vbs;visualbasic;vbscript" extension=".vb" type="Microsoft.CodeDom.Providers.DotNetCompilerPlatform.VBCodeProvider, Microsoft.CodeDom.Providers.DotNetCompilerPlatform, Version=3.6.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" warningLevel="4" compilerOptions="/langversion:default /nowarn:41008 /define:_MYTYPE=\&quot;Web\&quot; /optionInfer+" />
    </compilers>
  </system.codedom>

You also need to ensure the version number shown in the web.config matches the assembly version of the package.
The following blog post describes the first version of this package - although I'm sure there are newer/better articles.
Enabling the .NET Compiler Platform (“Roslyn”) in ASP.NET applications

With the release of Microsoft.Build.CentralPackageVersions 2.1.1, I can re-evaluate how difficult it would be to integrate the detection code. Hopefully I will be able to address that soon, time permitting.

@Timores
Copy link
Author

Timores commented Nov 26, 2021

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants