Fix logging source-gens duplication on WPF

[xakep139]

This is a naive fix for the #4969

We can also consider fixing RemoveDuplicateAnalyzers Target since it basically adds back the analyzer: https://github.com/dotnet/ResXResourceManager/blob/c7f0d75c55917c5de01f807394c37dd2af11251b/src/Directory.Build.targets#L28

Microsoft Reviewers: Open in CodeFlow

[tarekgh]

Reading the comment https://github.com/dotnet/extensions/issues/4969#issuecomment-1991870300 it suggests the issue is not specific to extension telemetry library and it suggest the fix should be somewhere else maybe in SDK targets? I am not sure.

[RussKie]

@tarekgh is there someone else you could suggest to rope in for a review of this?

[tarekgh]

@joperezr can advise about that. I have no idea why the analyzer get injected twice in the WPF scenario (if I am understanding the issue correctly).

[xakep139]

Reading the comment #4969 (comment) it suggests the issue is not specific to extension telemetry library and it suggest the fix should be somewhere else maybe in SDK targets? I am not sure.

In fact, it is specific for Microsoft.Extensions.Telemetry.Abstractions package. I left that comment to highlight that the root cause isn't related to Microsoft.Extensions.Resilience package - as the author wrote there. The latter references former, that's why it fails to compile even if you reference only resilience bits.

[xakep139]

@joperezr can advise about that. I have no idea why the analyzer get injected twice in the WPF scenario (if I am understanding the issue correctly).

Correct, the issue doesn't happen on console projects, nor does it happen for WinForms.

[RussKie]

If it's WPF specific, perhaps we should get WPF folks roped in? E.g., ping @dotnet/dotnet-wpf alias.

[xakep139]

@dotnet/dotnet-wpf folks, can you please suggest a good way of disabling an analyzer/source-generator (or confirm that the approach in this PR is acceptable) for WPF apps?

[ZzZombo]

Is there anything interim for us to implement on our end in order to progress while you figure this out? Thanks in advance.

[RussKie]

Is there anything interim for us to implement on our end in order to progress while you figure this out? Thanks in advance.

Copy the new target into your project, and it should remove the analyzers from the intermediate project.

[RussKie]

Setting to draft for now until we agree this is the best way to proceed.

[ZzZombo]

I can confirm the change works! Thanks!

[RussKie]

I propose the following change instead:

--- a/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/buildTransitive/net8.0/Microsoft.Extensions.Telemetry.Abstractions.targets
+++ b/src/Libraries/Microsoft.Extensions.Telemetry.Abstractions/buildTransitive/net8.0/Microsoft.Extensions.Telemetry.Abstractions.targets
@@ -2,10 +2,10 @@
   <!-- This package should replace the Microsoft.Extensions.Logging.Abstractions source generator. -->
   <Target Name="_Microsoft_Extensions_Logging_AbstractionsRemoveAnalyzers" 
           Condition="'$(DisableMicrosoftExtensionsLoggingSourceGenerator)' == 'true'"
-          AfterTargets="ResolveReferences">
+          BeforeTargets="CoreCompile">
     <ItemGroup>
-      <_Microsoft_Extensions_Logging_AbstractionsAnalyzer Include="@(Analyzer)" Condition="'%(Analyzer.AssemblyName)' == 'Microsoft.Extensions.Logging.Generators' Or
-                                                                                           '%(Analyzer.NuGetPackageId)' == 'Microsoft.Extensions.Logging.Abstractions'" />
+      <_Microsoft_Extensions_Logging_AbstractionsAnalyzer Include="@(Analyzer)" Condition="'%(Analyzer.Filename)' == 'Microsoft.Extensions.Logging.Generators' Or
+                                                                                           '%(Analyzer.Filename)' == 'Microsoft.Extensions.Logging.Abstractions'" />
     </ItemGroup>
 
     <!-- Remove Microsoft.Extensions.Logging.Abstractions Analyzer -->

[xakep139]

@RussKie, thanks! I applied your change, please review

[joperezr]

Thanks @xakep139!