CoreWCF icon indicating copy to clipboard operation
CoreWCF copied to clipboard

Target .NET 6

Open jonlouie opened this issue 3 years ago • 14 comments

Description

  • Change target framework of all projects to .NET 6
  • Remove defunct preprocessor directives related to target frameworks
  • Upgrade all nuget packages
  • Remove Roslyn 3.11 build tools
  • Fix compile time errors and broken tests from .NET 6 upgrade

Testing

  • Ran all unit tests

jonlouie avatar Jul 22 '22 06:07 jonlouie

Is there a specific reason why we're doing this?

Many folks are banking on this library to help them move away from .NET Framework by allowing them to migrate functionaility step by step i.e. Legacy -> ASPNETCore Hosting on .NET Framework -> ASPNETCore Hosting on .NET Core -> Upgrade to NET 6.

The folks who are still waiting on NamedPipe support will be left out in the cold.

MeikTranel avatar Jul 22 '22 07:07 MeikTranel

@MeikTranel, the move to .NET 6 is needed for multiple reasons, one of which is to support TCP port sharing. The 1.x version has the same support lifecycle as asp.net core 2.1 (which runs on .NET Framework) so isn't going to go out of support any time soon. A best effort approach will be done to port anything new to the 1.x version where reasonable. I'm hoping NamedPipe support fits this criteria as it doesn't depend on anything in .NET 6. WCF Core client NamedPipe support is needed to support TCP port sharing, so all the foundational NamedPipe support work will be completed before I work on the port sharing component so I don't see anything blocking bringing it to 1.x simultaneously. I'm being a little non-committal in the language as there is a possibility that I might run into something which blocks it on asp.net core 2.1, but there's nothing I'm currently aware of.

mconnew avatar Jul 25 '22 20:07 mconnew

@jonlouie Why not taking a dependency to ASP.NET core using FrameworkReference ? please see https://docs.microsoft.com/en-us/aspnet/core/fundamentals/target-aspnetcore?view=aspnetcore-6.0&tabs=visual-studio#use-the-aspnet-core-shared-framework

g7ed6e avatar Jul 28 '22 13:07 g7ed6e

@mconnew i see in the PR description that you aim to drop the Roslyn 3.11 Build.Tools, please do notice that this is not required to take the dependency on .NET 6. The Roslyn 3.11 version of the build tools do exist only to ensure compatibility of the source generator feature for VS2019 users.

edit: @jonlouie to drop the 3.11 Build.Tools and thus the VS2019 support it should be sufficient to drop these 3 files:

g7ed6e avatar Jul 28 '22 13:07 g7ed6e

@jonlouie Why not taking a dependency to ASP.NET core using FrameworkReference ? please see https://docs.microsoft.com/en-us/aspnet/core/fundamentals/target-aspnetcore?view=aspnetcore-6.0&tabs=visual-studio#use-the-aspnet-core-shared-framework

@g7ed6e I can look into this. I don't see why we couldn't make this adjustment. @mconnew Do you have any reservations about this?

@mconnew i see in the PR description that you aim to drop the Roslyn 3.11 Build.Tools, please do notice that this is not required to take the dependency on .NET 6. The Roslyn 3.11 version of the build tools do exist only to ensure compatibility of the source generator feature for VS2019 users.

@g7ed6e I removed the Roslyn 3.11 Build.Tools project because VS2019 does not support .NET 6. Unless there is still reason to keep the project within the solution, I will take your suggestions for removing the 3 files mentioned in your last comment.

jonlouie avatar Jul 28 '22 15:07 jonlouie

What about command line building? Can you use Rosslyn 3.11 at the command line to build?

mconnew avatar Jul 28 '22 15:07 mconnew

@MeikTranel, the move to .NET 6 is needed for multiple reasons, one of which is to support TCP port sharing. The 1.x version has the same support lifecycle as asp.net core 2.1 (which runs on .NET Framework) so isn't going to go out of support any time soon. A best effort approach will be done to port anything new to the 1.x version where reasonable. I'm hoping NamedPipe support fits this criteria as it doesn't depend on anything in .NET 6. WCF Core client NamedPipe support is needed to support TCP port sharing, so all the foundational NamedPipe support work will be completed before I work on the port sharing component so I don't see anything blocking bringing it to 1.x simultaneously. I'm being a little non-committal in the language as there is a possibility that I might run into something which blocks it on asp.net core 2.1, but there's nothing I'm currently aware of.

That sounds acceptable to me. Moving on 😂

MeikTranel avatar Jul 28 '22 21:07 MeikTranel

@g7ed6e I removed the Roslyn 3.11 Build.Tools project because VS2019 does not support .NET 6. Unless there is still reason to keep the project within the solution, I will take your suggestions for removing the 3 files mentioned in your last comment.

@jonlouie I did not know that VS2019 does not support .net6. It makes sense indeed to drop support of BuildTools in VS2019 in this PR

g7ed6e avatar Aug 01 '22 14:08 g7ed6e

What about command line building? Can you use Rosslyn 3.11 at the command line to build?

@mconnew As CoreWCF will target .net 6 and .net 6 requires .net 6 sdk to build and Roslyn4 is installed with .net6 i think we can assume only .net5 users would need Roslyn3.11 to command line build. And .net 5 is now EOL.

g7ed6e avatar Aug 01 '22 14:08 g7ed6e

What about command line building? Can you use Rosslyn 3.11 at the command line to build?

@mconnew As CoreWCF will target .net 6 and .net 6 requires .net 6 sdk to build and Roslyn4 is installed with .net6 i think we can assume only .net5 users would need Roslyn3.11 to command line build. And .net 5 is now EOL.

@g7ed6e Thanks for the additional context.

@mconnew I did attempt to build using the command line and Roslyn 3.11. The build output reported 0 errors but there were some concerning warnings, notably CS8032: Could not load file or assembly or one of its dependencies. There were no such warnings using MSBuild with Roslyn 4.x.

Examples:

CSC : warning CS8032: An instance of analyzer Microsoft.AspNetCore.Analyzers.RouteHandlers.RouteHandlerAnalyzer cannot be created from C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.5\analyzers\dotnet\cs\Microsoft.AspNetCore.App.Analyzers.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified.. [D:\Workplace\CoreWCF\src\CoreWCF.WebHttp\src\CoreWCF.WebHttp.csproj]
CSC : warning CS8032: An instance of analyzer Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator cannot be created from C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.5\analyzers\dotnet\roslyn4.0\cs\Microsoft.Extensions.Logging.Generators.dll: Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified.. [D:\Workplace\CoreWCF\src\CoreWCF.WebHttp\src\CoreWCF.WebHttp.csproj]

jonlouie avatar Aug 01 '22 17:08 jonlouie

@mconnew Tests pass locally, but there is a failure in the test pipeline between tests. The error occurs when writing to the test logger- has this been seen before and is it flaky behavior?

https://dev.azure.com/dotnet/CoreWCF/_build/results?buildId=76059&view=logs&j=b022b3f7-954a-5bee-8630-c83c3a9b9ce4&t=8fb4180c-4405-53d3-3e02-65aad02e9564&l=1761

jonlouie avatar Aug 04 '22 22:08 jonlouie

Posting this in case someone else hits a similar issue in the future. I already explained the issue offline to Jon. Basically the ILogger passed to asp.net core which wraps the ITestOutputHelper (for logging things for the test) is being used after the test method has returned. This is likely due to the WebHost not being stopped before the test returns.

mconnew avatar Aug 04 '22 23:08 mconnew

@g7ed6e Thanks for catching those items. I've made all changes- build and test worked locally.

jonlouie avatar Aug 08 '22 18:08 jonlouie