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

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?