OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
455 stars 156 forks source link

Release 8.2.1 does not build using latest Visual Studio 17.7.1 #1025

Closed nicovos closed 1 year ago

nicovos commented 1 year ago

Assemblies affected Microsoft.AspNetCore.OData.dll

Describe the bug I'm using Microsoft Visual Studio Professional 2022 (64-bit) - Current Version 17.7.1 I just upgraded Visual Studio to this latest version. When I build the release 8.2.1 sources then the build process stops at given point.

This is what the output windows displays last.

image

It does not seem to end the last actions that was started.

Reproduce steps Just install version 17.7.1 of Visual Studio. Take the sources of release 8.2.1. Rebuild the solution.

Additional context I have looked into the issue and found that the file ..\Microsoft.AspNetCore.OData\Query\Container\PropertyContainer.generated.cs causes the issue.

The contents of the file is as follow. `

    private class SingleExpandedPropertyWithNext0<T> : SingleExpandedProperty<T>
    {
        public PropertyContainer Next0 { get; set; }

        public override void ToDictionaryCore(Dictionary<string, object> dictionary, IPropertyMapper propertyMapper,
            bool includeAutoSelected)
        {
            base.ToDictionaryCore(dictionary, propertyMapper, includeAutoSelected);
            Next0.ToDictionaryCore(dictionary, propertyMapper, includeAutoSelected);
        }
    }
    private class SingleExpandedPropertyWithNext1<T> : SingleExpandedPropertyWithNext0<T>
    {
        public PropertyContainer Next1 { get; set; }

        public override void ToDictionaryCore(Dictionary<string, object> dictionary, IPropertyMapper propertyMapper,
            bool includeAutoSelected)
        {
            base.ToDictionaryCore(dictionary, propertyMapper, includeAutoSelected);
            Next1.ToDictionaryCore(dictionary, propertyMapper, includeAutoSelected);
        }
    }
    private class SingleExpandedPropertyWithNext2<T> : SingleExpandedPropertyWithNext1<T>
    {
        public PropertyContainer Next2 { get; set; }

`

The file has up classes named SingleExpandedPropertyWithNext0 up to SingleExpandedPropertyWithNext30. Each class is derived from the prior class.

This code does not seem to be supported any more.

xuzhg commented 1 year ago

@nicovos Thanks for reporting this. It seems you help me find a hang-on problem in our build pipeline.

Problem: Our build pipeline installs .NET Core SDK 7.x latest version any time when triggers a new build. It works fine until a new version of 7.x updates from .NET team. Since then, our build hangs on in the source codes building task. We have to downgrade to .NET Core SDK 6.x to make the build pipeline work again.

Do you know any announcement/materials about 'This code does not seem to be supported any more.' from your comment?

Thanks.

nicovos commented 1 year ago

Hi @xuzhg, great that I could help you with your problem. As for my comment. It was more like an assumption. I think something has change in the C#/,NET compiler that is causing an enless loop or an extremely long task when compiling the code that I mentioned. Since it has worked in prior versions it is either something that has changed in the compiler that was intentional or unintentional. So I think it is best to report it to the responsible team to solve the issue in SDK 7.x. Since this could take a while, it may also be best to look into changing the code to prevent the issue. Anyway I will report this issue to the SDK team. Regards, Nico

DSaid commented 1 year ago

Hi @nicovos, The problem is related to Microsoft.Net.Compilers.Toolset version 4.7.0 and is also present in version 4.8.0 In Visual Studio 2022 v17.1, v17.2 and v17.8 preview 1, you can add the Microsoft.Net.Compilers.Toolset v4.6.0 Nuget package in Microsoft.AspNetCore.OData and/or Microsoft.AspNetCore.OData.NewtonsoftJson projects. This will cause Visual Studio to use version 4.6.0 of the compiler and thus allow it to build the solutions. In fact, the code generated by PropertyContainer.generated.tt is compiled by versions 4.7+ but the compilation time increases very quickly with the number of nested classes (see the int levels parameter in the .tt code ). For a standard computer, the compilation time remains humanly acceptable up to int levels = 19. The problem is the same with ServiceHub.RoslynCodeAnalysisService which is launched by Visual Studio when opening the solution. This service consumes a lot of computing power and should be quick. This problem is not solved by adding the previously mentioned Nuget package.

Hi @xuzhg, I've redacted PropertyContainer.generated.tt so that it generates some standalone code that reproduces the problem. See attached file. Do you think this would be of interest to the Microsoft.Net.Compilers.Toolset team? Maybe they could find a solution? PropertyContainer.redacted.zip

xuzhg commented 1 year ago

Hi @nicovos, The problem is related to Microsoft.Net.Compilers.Toolset version 4.7.0 and is also present in version 4.8.0 In Visual Studio 2022 v17.1, v17.2 and v17.8 preview 1, you can add the Microsoft.Net.Compilers.Toolset v4.6.0 Nuget package in Microsoft.AspNetCore.OData and/or Microsoft.AspNetCore.OData.NewtonsoftJson projects. This will cause Visual Studio to use version 4.6.0 of the compiler and thus allow it to build the solutions. In fact, the code generated by PropertyContainer.generated.tt is compiled by versions 4.7+ but the compilation time increases very quickly with the number of nested classes (see the int levels parameter in the .tt code ). For a standard computer, the compilation time remains humanly acceptable up to int levels = 19. The problem is the same with ServiceHub.RoslynCodeAnalysisService which is launched by Visual Studio when opening the solution. This service consumes a lot of computing power and should be quick. This problem is not solved by adding the previously mentioned Nuget package.

Hi @xuzhg, I've redacted PropertyContainer.generated.tt so that it generates some standalone code that reproduces the problem. See attached file. Do you think this would be of interest to the Microsoft.Net.Compilers.Toolset team? Maybe they could find a solution? PropertyContainer.redacted.zip

@DSaid Thanks for your information. That's real helpful. Please forward your changes to Toolset team which maybe useful for them to dig. If you need any help/action from my side, please let me know. Thanks.

robertmclaws commented 1 year ago

It's coming up on time to modernize the OData solution structure and eliminate the unnecessary legacy complications once and for all.

xuzhg commented 1 year ago

It's coming up on time to modernize the OData solution structure and eliminate the unnecessary legacy complications once and for all.

@robertmclaws ASP.NET Core OData 8 project is using the latest solution/project structure. We had a discussion early of this year to 'get rid of' un-supported .NET version (.NET Core APP 3.1 and .NET 5) and update to .NET 6, 7, 8. The concern is that if we remove .NET 3.1 and .NET 5, it looks like a breaking change. We'd update the major version from 8.x.x to 9.x.x of this repro. In this case, we haven't made a decision to do. DO you have any recommendation for this part?

robertmclaws commented 1 year ago

@robertmclaws ASP.NET Core OData 8 project is using the latest solution/project structure. It's really not. There are a bunch of legacy artifacts in the project from the 2010's that are unnecessary, the unit test structure needs work, etc. Just because it's on a modern SDK-style project doesn't mean it's fully modernized.

To your question, that is the problem with calling this version 8.0 instead of OData ASP.NET Core v1.0 (or 2.0, really). It creates version issues because of v7 vs v8 vs future breaking changes.

Personally, across the 70-some-odd packages that I manage across various OSS projects, I don't treat losing support for runtimes as breaking changes. That's because NuGet will simply not allow an incompatible package to be installed. That throws an error, which then forces the developer onto a previous release.

What you do is two things: 1) You put out a build # update (8.11.1, for example) that puts end-of-support notes in the Package Description and Readme, so people can see what to do. 2) Put out an 8.12 version that has the actual removed runtime support, and clearly note that in the readme & package description as well.

That gives the people on the current platforms a package with a clear end-of-the-road notification, and the next version won't be installed if you're still on .NET Core 3.1 for example.

Happy to jump on a call at some point and show you folks how it works.

gathogojr commented 1 year ago

Issue in Visual Studio reported https://developercommunity.visualstudio.com/t/Build-hangs-for-ASPNET-Core-project-whe/10466622 Fixed in Visual Studio 2022 version 17.7.5