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

Multi-targeting strategy for Json and Logging Source Generators for 6.0 NuGet packages #58536

Closed
eerhardt opened this issue Sep 2, 2021 · 3 comments

Comments

@eerhardt
Copy link
Member

eerhardt commented Sep 2, 2021

Background

This issue addresses the 3rd bullet point in #56702 (comment).

Identify multi-targeting strategy for shipping both v1 and v2 implementations for use in various IDE & SDK scenarios

I have a proposal to add Roslyn component multi-targeting support to the .NET SDK here: dotnet/sdk#20355. And a couple prototypes of what it could look like in the SDK, and how our dotnet/runtime packages could be built.

However, given where we are in the release cycle, I'm not confident getting a multi-targeting feature into the SDK in 6.0.100 is feasible at this point in time.

This proposal is to define a plan for addressing the user experience of using the Json and Logging Source Generators when using an older version of Roslyn - specifically when using VS 2019 or an earlier version of the .NET SDK.

With the latest 6.0 NuGet packages, if a developer is using VS 2019 and upgrades their <PackageReference to the 6.0.0-rc1 versions, they start seeing build warnings:

Warning CS8032 An instance of analyzer System.Text.Json.SourceGeneration.JsonSourceGenerator cannot be created from C:\Users\eerhardt\.nuget\packages\system.text.json\6.0.0-rc1\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.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.

This is not a great user experience.

Goals

  1. Developers should be able to upgrade their dependencies to our 6.0 NuGet packages, since the tooling advices them to upgrade when a new version comes out.
  2. Upgrading to the 6.0 packages shouldn't adversely affect their experience (both runtime and design-time).
  3. If user code is trying to use the source generator, but the source generator isn't running, the user should be informed.
  4. A lower priority goal would be to enable the source generators to be used, even if the developer isn't using the latest Roslyn (i.e. 6.0 SDK or VS 2022). But we shouldn't tradeoff the above goals to achieve this.

Proposal

To address the current user experience on older Roslyn versions with our 6.0 packages, we can add the multi-targeting logic into our 2 NuGet packages. This is a stop-gap solution until the SDK can officially support multi-targeting Roslyn components.

The proposal is to add MSBuild targets which:

  1. Discover the Roslyn version which will be used to compile the current project
  2. Choose the appropriate source generator version based on the Roslyn version. For 6.0, we would target two Roslyn versions:
    • v3.9 when compilng with Roslyn < v4.0
    • v4.0 when compilng with Roslyn >= v4.0
      • this is the same asset we place in the net6.0 ref-pack

Since the VS 2019 source generator API has performance issues, we will also include an MSBuild property that turns the Json and Logging source generators off. Developers can set the $(DisableSystemTextJsonSourceGenerator) and $(DisableMicrosoftExtensionsLoggingSourceGenerator) properties to true in their .csproj to disable the source generators in the NuGet packages. Note that this only works in the NuGet package. These properties are not respected by the generators in the "ref-pack". The reason being is that the generators in the "ref-pack" have addressed the performance issues by moving to the incremental API.

Alternative Options

Other options for addressing the current user experience:

  1. Do nothing.
    • Developers using an older Roslyn version will get the warning that the 6.0 source generator isn't working. This meets goal (3) above, but doesn't meet (2) since users who aren't even using the source generator still get a warning.
  2. Disable the source generators when using older Roslyn versions
    • We can add MSBuild targets to our NuGet packages that remove the source generators when the Roslyn version isn't >= 4.0.
    • This allows users to upgrade to the new version without giving a warning when they aren't using the source generator. It also solves any performance issues caused by non-incremental source generators in VS, because the generators won't be used.
    • To meet goal (3), we could write a separate analyzer in roslyn-analyzers that checks for code trying to use the source generators, and logs a diagnostic when the source generator doesn't fill out the implementation.
  3. Push for the full SDK feature in 6.0.100 proposed in Support multi-targeting for Roslyn components sdk#20355.
    • Note we would still need MSBuild targets in our NuGet package to select the v3.9 targeted generator when using VS 2019 or an earlier version of the .NET SDK

@ericstj @terrajobst @danmoseley @stephentoub @steveharter @layomia @maryamariyan - thoughts on this proposal?

cc @chsienki @jaredpar @jmarolf @sharwell

@dotnet-issue-labeler dotnet-issue-labeler bot added area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner labels Sep 2, 2021
@ghost
Copy link

ghost commented Sep 2, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Background

This issue addresses the 3rd bullet point in #56702 (comment).

Identify multi-targeting strategy for shipping both v1 and v2 implementations for use in various IDE & SDK scenarios

I have a proposal to add Roslyn component multi-targeting support to the .NET SDK here: dotnet/sdk#20355. And a couple prototypes of what it could look like in the SDK, and how our dotnet/runtime packages could be built.

However, given where we are in the release cycle, I'm not confident getting a multi-targeting feature into the SDK in 6.0.100 is feasible at this point in time.

This proposal is to define a plan for addressing the user experience of using the Json and Logging Source Generators when using an older version of Roslyn - specifically when using VS 2019 or an earlier version of the .NET SDK.

With the latest 6.0 NuGet packages, if a developer is using VS 2019 and upgrades their <PackageReference to the 6.0.0-rc1 versions, they start seeing build warnings:

Warning CS8032 An instance of analyzer System.Text.Json.SourceGeneration.JsonSourceGenerator cannot be created from C:\Users\eerhardt\.nuget\packages\system.text.json\6.0.0-rc1\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.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.

This is not a great user experience.

Goals

  1. Developers should be able to upgrade their dependencies to our 6.0 NuGet packages, since the tooling advices them to upgrade when a new version comes out.
  2. Upgrading to the 6.0 packages shouldn't adversely affect their experience (both runtime and design-time).
  3. If user code is trying to use the source generator, but the source generator isn't running, the user should be informed.
  4. A lower priority goal would be to enable the source generators to be used, even if the developer isn't using the latest Roslyn (i.e. 6.0 SDK or VS 2022). But we shouldn't tradeoff the above goals to achieve this.

Proposal

To address the current user experience on older Roslyn versions with our 6.0 packages, we can add the multi-targeting logic into our 2 NuGet packages. This is a stop-gap solution until the SDK can officially support multi-targeting Roslyn components.

The proposal is to add MSBuild targets which:

  1. Discover the Roslyn version which will be used to compile the current project
  2. Choose the appropriate source generator version based on the Roslyn version. For 6.0, we would target two Roslyn versions:
    • v3.9 when compilng with Roslyn < v4.0
    • v4.0 when compilng with Roslyn >= v4.0
      • this is the same asset we place in the net6.0 ref-pack

Since the VS 2019 source generator API has performance issues, we will also include an MSBuild property that turns the Json and Logging source generators off. Developers can set the $(DisableSystemTextJsonSourceGenerator) and $(DisableMicrosoftExtensionsLoggingSourceGenerator) properties to true in their .csproj to disable the source generators in the NuGet packages. Note that this only works in the NuGet package. These properties are not respected by the generators in the "ref-pack". The reason being is that the generators in the "ref-pack" have addressed the performance issues by moving to the incremental API.

Alternative Options

Other options for addressing the current user experience:

  1. Do nothing.
    • Developers using an older Roslyn version will get the warning that the 6.0 source generator isn't working. This meets goal (3) above, but doesn't meet (2) since users who aren't even using the source generator still get a warning.
  2. Disable the source generators when using older Roslyn versions
    • We can add MSBuild targets to our NuGet packages that remove the source generators when the Roslyn version isn't >= 4.0.
    • This allows users to upgrade to the new version without giving a warning when they aren't using the source generator. It also solves any performance issues caused by non-incremental source generators in VS, because the generators won't be used.
    • To meet goal (3), we could write a separate analyzer in roslyn-analyzers that checks for code trying to use the source generators, and logs a diagnostic when the source generator doesn't fill out the implementation.
  3. Push for the full SDK feature in 6.0.100 proposed in Support multi-targeting for Roslyn components sdk#20355.
    • Note we would still need MSBuild targets in our NuGet package to select the v3.9 targeted generator when using VS 2019 or an earlier version of the .NET SDK

@ericstj @terrajobst @danmoseley @stephentoub @steveharter @layomia @maryamariyan - thoughts on this proposal?

cc @chsienki @jaredpar @jmarolf @sharwell

Author: eerhardt
Assignees: -
Labels:

area-Infrastructure-libraries, untriaged

Milestone: -

@danmoseley
Copy link
Member

If time is short: is an option to service the JSON package post GA to enable VS2019? Since it is only relevant to folks who consume the OOB, since VS2019 won't support targeting the 6.0 shared framework...

Actually cutting the SG from the OOB scenario would be unfortunate.

@eerhardt
Copy link
Member Author

Closing as this work is now complete - and backported to the 6.0 branch. It will be available in 6.0-rc2.

We chose to take Alternative Option (2) above.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants