dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.07k stars 2.03k forks source link

Codegen fails for ValueTask<T> with Build but not MSBuild #5300

Closed mehmetakbulut closed 5 years ago

mehmetakbulut commented 5 years ago

When trying to build a project containing a grain interface whose method returns a ValueTask<T>, the code generation fails with the following error when using Microsoft.Orleans.CodeGenerator.Build 2.2.0:

Error   MSB3073 The command ""dotnet" "C:\Users\Mehmet\.nuget\packages\microsoft.orleans.orleanscodegenerator.build\2.2.0\build\..\tasks\netcoreapp2.0\Orleans.CodeGeneration.Build.dll" "@obj\Debug\netcoreapp2.2\codegen\NetCoreWithBuild.orleans.g.args.txt"" exited with code 3.    NetCoreWithBuild    C:\Users\Mehmet\.nuget\packages\microsoft.orleans.orleanscodegenerator.build\2.2.0\build\Microsoft.Orleans.OrleansCodeGenerator.Build.targets   82  

This behavior happens whether the project targets .NET Core 2.2 or .NET Standard 2.0. (one of the lines changes to @obj\Debug\netstandard2.0 ... as expected)

This behavior does not happen if Microsoft.Orleans.CodeGenerator.MSBuild 2.2.0 is used instead.

Repository below reproduces these results with both ..Build and ..MSBuild for both targets: https://github.com/mehmetakbulut/OrleansValueTaskRepro

Documentation and release notes (as far as I have seen?) does not appear to suggest that the code generation package should determine whether ValueTask<T> is a valid return type therefore I'm assuming this is a bug.

sergeybykov commented 5 years ago

Have you tried referencing Microsoft.Orleans.CodeGenerator.MSBuild instead of Microsoft.Orleans.CodeGenerator.Build? It is more resilient to framework and assembly versioning.

mehmetakbulut commented 5 years ago

MSBuild does indeed work fine as I mentioned :)

In the repro I linked above, there are 4 projects and compilation statuses are as below: NetCoreWithBuild: fail NetCoreWithMsBuild: pass NetStandardWithBuild: fail NetStandardWithMsBuild: pass

So the issue appears to be independent of target and is isolated to Build package.

While on this topic, I do have two questions: Is the plan to retire Build eventually? Are there any known drawbacks of MSBuild?

If the plan is to retire, maybe "won't fix" is the appropriate action since ValueTask<T> is really a plus and can be worked around. (with a mention on docs that Build doesn't support it)

sergeybykov commented 5 years ago

Is the plan to retire Build eventually?

Yes, we plan to retire it at some point. The fundamental problem with it is that is uses Reflection for loading application type information, and that is very sensitive to assembly versioning, etc.

Are there any known drawbacks of MSBuild?

It generates more type identity 'noise' because it does not load dependencies. That has no impact on performance, just longer generated code file.