Unable to disable Microsoft.Extensions.Logging.SourceGenerator with DisableMicrosoftExtensionsLoggingSourceGenerator

[RussKie]

Description

dotnet/extensions have own generators for LoggerMessageAttribute] and require the runtime's one to be disabled.

I'm having issues building dotnet/extensions-samples:

D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyDataExporter\Log.cs(12,65): error SYSLIB1015: Argument 'level' is not referenced from the logging message (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1015.md) [D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyContext.csproj::TargetFramework=net9.0]
D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(58,36): error CS0757: A partial method may not have multiple implementing declarations [D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyContext.csproj::TargetFramework=net9.0]

The repo-global Directory.Build.targets sets the flag:

<Project>
  <Import Project="Sdk.targets" Sdk="Microsoft.DotNet.Arcade.Sdk" />

  <PropertyGroup>
    <DisableMicrosoftExtensionsLoggingSourceGenerator>true</DisableMicrosoftExtensionsLoggingSourceGenerator>
  </PropertyGroup>

</Project>

However, the runtime's generator is still being executed:

Looking at binlog it looks like _Microsoft_Extensions_Logging_AbstractionsRemoveAnalyzers target is being executed before another target that re-adds the generator back:

I think that AfterTargets must include ResolveReferences target instead - testing it locally everything "just works" (c):
https://github.com/dotnet/runtime/blob/8cb3c6187f9e1338f22f9e37c4dfe615123eaa59/eng/MultiTargetRoslynComponent.targets.template#L21-L24

  <Target Name="_{TargetPrefix}RemoveAnalyzers" 
          Condition="'$({DisableSourceGeneratorPropertyName})' == 'true'"
-         AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets"
+         AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets;ResolveReferences"
          DependsOnTargets="_{TargetPrefix}GatherAnalyzers">

Reproduction Steps

• [dev] Update dependencies from dotnet/arcade extensions-samples#162
• https://dev.azure.com/dnceng-public/public/_build/results?buildId=484506&view=results

Expected behavior

The generator is removed.

Actual behavior

The generator is being re-added.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

dotnet/extensions have own generators for LoggerMessageAttribute] and require the runtime's one to be disabled.

I'm having issues building dotnet/extensions-samples:

D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyDataExporter\Log.cs(12,65): error SYSLIB1015: Argument 'level' is not referenced from the logging message (https://learn.microsoft.com/dotnet/fundamentals/syslib-diagnostics/syslib1015.md) [D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyContext.csproj::TargetFramework=net9.0]
D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(58,36): error CS0757: A partial method may not have multiple implementing declarations [D:\Development\dotnet-r9-samples\src\Telemetry\LatencyMeasurement\LatencyContext\LatencyContext.csproj::TargetFramework=net9.0]

The repo-global Directory.Build.targets sets the flag:

<Project>
  <Import Project="Sdk.targets" Sdk="Microsoft.DotNet.Arcade.Sdk" />

  <PropertyGroup>
    <DisableMicrosoftExtensionsLoggingSourceGenerator>true</DisableMicrosoftExtensionsLoggingSourceGenerator>
  </PropertyGroup>

</Project>

However, the runtime's generator is still being executed:

Looking at binlog it looks like _Microsoft_Extensions_Logging_AbstractionsRemoveAnalyzers target is being executed before another target that re-adds the generator back:

I think that AfterTargets must include ResolveReferences target instead - testing it locally everything "just works" (c):
https://github.com/dotnet/runtime/blob/8cb3c6187f9e1338f22f9e37c4dfe615123eaa59/eng/MultiTargetRoslynComponent.targets.template#L21-L24

  <Target Name="_{TargetPrefix}RemoveAnalyzers" 
          Condition="'$({DisableSourceGeneratorPropertyName})' == 'true'"
-         AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets"
+         AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets;ResolveReferences"
          DependsOnTargets="_{TargetPrefix}GatherAnalyzers">

Reproduction Steps

• [dev] Update dependencies from dotnet/arcade extensions-samples#162
• https://dev.azure.com/dnceng-public/public/_build/results?buildId=484506&view=results

Expected behavior

The generator is removed.

Actual behavior

The generator is being re-added.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author:	RussKie
Assignees:	-
Labels:	untriaged, area-Extensions-Logging
Milestone:	-

[tarekgh]

This is interesting. Isn't extensions target automatically disabling the runtime source gen?

extensions/Directory.Build.targets

Line 89 in 13a6288

Condition="'$(DisableMicrosoftExtensionsLoggingSourceGenerator)' == 'true'"

.

CC @joperezr

I was not aware about the existing of this target https://github.com/dotnet/runtime/blob/a9084c415ffe8785c311d0d65ec2c926b9086b5d/eng/MultiTargetRoslynComponent.targets.template#L21. @ericstj @ViktorHofer any thoughts here?

CC @eerhardt as he is the one introduced this target.

[ericstj]

The part you've highlighted in your log isn't the initial addition - I suspect that's happening in ResolveTargetingPackAssets.

I see from your log that the one that's being included here is from the ASP.NET targeting pack, not the nuget package. The NuGet package's targets aren't meant to work with that one.

I don't think there's a general purpose way to disable the built in generators from the ref-packs, at least not a documented one. You could write your own target to do this. One thing that might work is:

<ItemGroup>
  <OffByDefaultAnalyzer Include="Microsoft.Extensions.Logging.Generators.dll" IsEnabled="false" />
</ItemGroup>

Though we don't document this. https://github.com/dotnet/sdk/blob/06c49af89a8a72c11c7fbede5e476ae45cdb86a3/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets#L440-L455

[RussKie]

Isn't extensions target automatically disabling the runtime source gen? dotnet/extensions@13a6288/Directory.Build.targets#L89.

Yes, and I expect that target to execute but it's not.
I have a theory that could be due to the re-use of the target name, but I haven't tested that hypothesis enough to confirm.

[joperezr]

Reusing the target name was intentional. In MSBuild, reusing a target name will redefine/overwrite it which IIRC was what we wanted here. I haven't looked at this in a while but at least at the time I tested this in .NET 8 it was working correctly so this is perhaps something that is now broken in 9.0. @RussKie where you planning going to look into this further?

[RussKie]

I haven't looked at this in a while but at least at the time I tested this in .NET 8 it was working correctly so this is perhaps something that is now broken in 9.

This isn't working for .NET 8 either.

I see from your log that the one that's being included here is from the ASP.NET targeting pack, not the nuget package. The NuGet package's targets aren't meant to work with that one.

I don't think there's a general purpose way to disable the built in generators from the ref-packs, at least not a documented one.

Thank you, I missed that part.

@ericstj do you have more information why the current set of targets was chosen (AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets"), and ResolveReferences` not used? My (limited) tests suggest the latter fixes the issue and removes analyzers from both NuGet packages and ref packs.
Can we entertain this as a fix?

You could write your own target to do this.

This is precisely what we did (@joperezr did :)) -->

extensions/Directory.Build.targets

Line 89 in 13a6288

Condition="'$(DisableMicrosoftExtensionsLoggingSourceGenerator)' == 'true'"

. However, for some reason this target doesn't get executed. I'm still trying to work out why.

[RussKie]

I think I found the root issue, and it's with the Extensions packages - the targets (https://github.com/dotnet/extensions/blob/main/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/buildTransitive/net6.0/Microsoft.Extensions.Telemetry.Abstractions.targets) are not included for .NET 8, and hence not getting imported...

[RussKie]

@joperezr do you remember why we're not adding the props to the net462 TFM?

[ericstj]

@ericstj do you have more information why the current set of targets was chosen (AfterTargets="ResolvePackageDependenciesForBuild;ResolveNuGetPackageAssets"), and ResolveReferences` not used? My (limited) tests suggest the latter fixes the issue and removes analyzers from both NuGet packages and ref packs.
Can we entertain this as a fix?

It's goal was to control only the analyzer added by the package. It runs after the targets responsible for adding package content. The fact that it removes something from the targeting pack if you change where it runs is not desirable. It wouldn't work for cases where folks don't happen to reference the package yet still get the generator from the targeting pack. If you want something that works everywhere, just create your own target. If we want a more generic publicly documented feature for disabling generators that are part of targeting packs we should ask for that from the SDK.

[joperezr]

We already have a target ourselves, we basically copy pasted the same target that the logging.abstractions package has, and we ship it with our package. This way, apps that get the generator via the package, and apps that get it via the targeting pack will get the same behavior and have the same target remove the generator.

We found the issue why this isn't working for our 9.0 packages, and it is an issue with our package authoring. We are basically incorrectly putting a placeholder in the net8.0 folder, which is causing our targets and props that disable the generator are not used. We have a PR out already to fix this.

[joperezr]

To be clear, this is only an issue in our 9.0 packages (produced from our dev branch), 8.x packages (produced from main) are correct. This regression was introduced when doing the branding updates of the dev branch.

[xakep139]

We found the issue why this isn't working for our 9.0 packages, and it is an issue with our package authoring. We are basically incorrectly putting a placeholder in the net8.0 folder, which is causing our targets and props that disable the generator are not used. We have a PR out already to fix this.

@joperezr do you have a link to the PR? I couldn't find it.

[RussKie]

do you have a link to the PR? I couldn't find it.

👉 #4773