arcade icon indicating copy to clipboard operation
arcade copied to clipboard

Use PoC to generate Microsoft.CodeAnalysis.CSharp.Workspaces, 4.0.1

Open andriipatsula opened this issue 2 years ago • 6 comments

The goal of this issue is to try to generate reference assemblies for Microsoft.CodeAnalysis.CSharp.Workspaces, 4.0.1 of version 4.0.1 and figure out the problem with netcoreapp3.1 tfm.

PoC: https://github.com/andriipatsula/PoC-GenAPI-Roslyn

andriipatsula avatar Aug 08 '22 09:08 andriipatsula

cc @tkapin, @MichaelSimons, @crummel

andriipatsula avatar Aug 08 '22 10:08 andriipatsula

Used the same ref assemblies as for netstandard2.0 and got 9613 errors of the same type.

error CS0518: Predefined type 'System.String' is not defined or imported
error CS0518: Predefined type 'System.Object' is not defined or imported
error CS0518: Predefined type 'System.Int32' is not defined or imported
...

we have folder https://github.com/dotnet/source-build-reference-packages/tree/main/src/targetPacks/ILsrc with deasm System.il, System.Security.il ... for different tfm(s) like NETFramework\v2.0

@MichaelSimons , do you know why do we have src/targetPacks/ILsrc/* and potentially this is a problem with netcoreapp3.1

andriipatsula avatar Aug 08 '22 10:08 andriipatsula

@andriipatsula

  • please reassign this issue (and any other Roslyn / GenAPI based issues) to the new epic.
  • please update the description with what is the goal of this issue

tkapin avatar Aug 08 '22 12:08 tkapin

./generate.sh --pkg microsoft.codeanalysis.csharp.workspaces,4.0.1 failures to generate reference package

.../GenerateProjects.proj(25,5): error MSB4018: The "GenerateProjects" task failed unexpectedly.
.../GenerateProjects.proj(25,5): error MSB4018: System.ArgumentException: Found strong name key that doesn't map: Key=979442b78dfc278e Dll=/repos/sbrp/source-build-reference-packages/artifacts/targetPackages/humanizer.core/2.2.0/lib/netstandard1.0/Humanizer.dll
.../GenerateProjects.proj(25,5): error MSB4018:    at Microsoft.DotNet.SourceBuild.Tasks.GenerateProjects.GetStrongNameKeyTuple(ProjectData assemblyInfo) in /repos/sbrp/source-build-reference-packages/src/referencePackageSourceGenerator/Tasks/GenerateProjects.cs:line 261
.../GenerateProjects.proj(25,5): error MSB4018:    at Microsoft.DotNet.SourceBuild.Tasks.GenerateProjects.GetStrongNameKeyFileName(ProjectData assemblyInfo) in /repos/sbrp/source-build-reference-packages/src/referencePackageSourceGenerator/Tasks/GenerateProjects.cs:line 240
.../GenerateProjects.proj(25,5): error MSB4018:    at Microsoft.DotNet.SourceBuild.Tasks.GenerateProjects.Execute() in /repos/sbrp/source-build-reference-packages/src/referencePackageSourceGenerator/Tasks/GenerateProjects.cs:line 121
.../GenerateProjects.proj(25,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
.../GenerateProjects.proj(25,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)

Besides that, the list of necessary hand edits from https://github.com/dotnet/source-build-reference-packages/pull/416 :

  1. Declare Microsoft.CodeAnalysis.CSharp a friend of Microsoft.CodeAnalysis.Common

I don't see we could automate this somehow, during the generation process we don't know who could be a friend in the future...

  1. Added hand crafted internal stubs in Microsoft.CodeAnalysis.Common and Microsoft.CodeAnalysis.Workspaces.Common

Easy way is to allow all internals or just public/protected. We could introduce parameter with the list of internal classes that must be included in output, but the task could be complicated as we'd need to determine all types our internals depend on.

  1. Removed the Microsoft.CodeAnalysis.Analyzers reference. Tooling does not support analyzers packages. Analyzers are not run as part of source-build so I don't think this will be a problem. Will validate in an E2E tarball build.

most probably we could automate this in GenAPI..

  1. Removed netcoreapp3.1 artifacts from the Microsoft.CodeAnalysis.* packages

this most probably related to https://github.com/dotnet/arcade/issues/10327#issuecomment-1207957790

7 ... public override TResult? Accept<TResult>(CSharpSyntaxVisitor<TResult> visitor)

Roslyn backend won't fix this problem, I had the same I manually added where TResult : default to Microsoft.CodeAnalysis.CSharp ref assemblies. We may add some heuristics in Roslyn backend to solve this problem. summary: it won't be solved out of the box by switching to Roslyn backend.

andriipatsula avatar Aug 08 '22 15:08 andriipatsula

@MichaelSimons , do you know why do we have src/targetPacks/ILsrc/* and potentially this is a problem with netcoreapp3.1

src/targetPacks exists to support generating the targeting packs. If we could support generating these via C# code, that would be great. How is this a problem with supporting netcoreapp3.1 based nupkgs? We can add any missing targeting packs.

MichaelSimons avatar Aug 08 '22 19:08 MichaelSimons

Today we had a discussion with the team, and I'd like to summarize: most of the issues described in https://github.com/dotnet/source-build-reference-packages/pull/416 are related to GenAPI as an infrastructure. I am going to create few issues and link them to epic https://github.com/dotnet/arcade/issues/10291.

  1. Removed netcoreapp3.1 artifacts from the Microsoft.CodeAnalysis.* packages

I am going to create an issue to extend GenAPI to remove netcoreapp3.1 artifacts during ref package creation. It's not just about removing a directory, but also modify csproj and nuspec files.

7 ... public override TResult? Accept<TResult>(CSharpSyntaxVisitor<TResult> visitor)

I got the same issue with Roslyn backend, we could either introduce some heuristics or ask Roslyn team to help fix this. It's not working out of the box.

@MichaelSimons , @crummel do you have any idea how could we solve problem 1 & 2:

  • Declare Microsoft.CodeAnalysis.CSharp a friend of Microsoft.CodeAnalysis.Common
  • Added hand crafted internal stubs in Microsoft.CodeAnalysis.Common and Microsoft.CodeAnalysis.Workspaces.Common

As described in comment above, I am not sure if it could be done on GenAPI-backend side.

I'll have a conversation with Roslyn guys about issue №7.

I'd like to finalize discussion related to Microsoft.CodeAnalysis.CSharp.Workspaces.4.0.1, create a tasks and start working on Roslyn backend for GenAPI. +we need to distinguish what could be done on backend side and what in GenAPI itself. And, after I'll close this issue

andriipatsula avatar Aug 09 '22 14:08 andriipatsula