dotnet
[LoggerMessageAttribute] generates CS0757 and CS0102 when combined with XAML and certain NuGet package references.
Description
Build will fail when targeting specific NuGet packages in addition to compiling with a .XAML file. If the .XAML file is removed, or the NuGet package is removed, the build will succeed. It is when .XAML, [LoggerMessage] and specific packages are referenced.
This was discovered while embedding an ASP.NET Core host inside a WPF application. These packages were referenced in a class library that was referenced by the WPF project:
<PackageReference Include="IdentityServer4" Version="4.1.2" />
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.8" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="8.0.8" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.7.0" />
<PackageReference Include="System.Linq.Async" Version="6.0.1" />
<PackageReference Include="System.Text.Encodings.Web" Version="8.0.0" />
I was able to reproduce the error by creating a new WPF project and adding a direct reference to one of these packages.
Reproduction Steps
Create a new C# WPF Application targeting .NET 8.
Add the following package references:
<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.8" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.1" />
</ItemGroup>
Add a class containing a [LoggerMessage] reference:
public static partial class LoggerExtensions
{
[LoggerMessage(LogLevel.Error, "Your message: {Message}")]
public static partial void SomethingFailed(this ILogger logger, string message);
}
Try to build the project.
Expected behavior
Build succeeded.
Actual behavior
Build failed.
1>...obj\Debug\net8.0-windows\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(13,36,13,51): error CS0757: A partial method may not have multiple implementing declarations
1>...obj\Debug\net8.0-windows\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(9,151,9,176): error CS0102: The type 'LoggerExtensions' already contains a definition for '__SomethingFailedCallback'
Regression?
Project will build after rolling back to Visual Studio 17.9.
Known Workarounds
Move the members with [LoggerMessageAttribute] to a class library that does not contain a .XAML file and a reference to one of the packages that trigger the failure.
Configuration
Windows Version 11 23H2 (x64) Visual Studio 17.11 (or 17.10) .NET SDK 8.0.400
Other information
No response
It is wrong to include both packages
<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="8.0.8" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.1" />
The reason is the package Microsoft.AspNetCore.Authentication.JwtBearer depends on the package Microsoft.AspNetCore.App.Ref which already includes the source generator Microsoft.Extensions.Logging.Generators.dll. Referencing the package Microsoft.Extensions.Logging.Abstractions will means including the source generator twice in the compilation which will cause the generated code produced twice and cause compilation error because of the duplication. If you look at the build logs, you can notice the following two entries under the analyzers list:
C:\Program Files\dotnet\packs\Microsoft.AspNetCore.App.Ref\8.0.7\analyzers/dotnet/roslyn4.4/cs/Microsoft.Extensions.Logging.Generators.dll
C:\Users\username\.nuget\packages\microsoft.extensions.logging.abstractions\8.0.1\analyzers\dotnet\roslyn4.4\cs\Microsoft.Extensions.Logging.Generators.dll
To fix this, remove the reference of Microsoft.Extensions.Logging.Abstractions.
CC @ericstj
That doesn't explain why removing the .XAML file resolves the issue. After removing StartupUri in App.xaml and deleting MainWindow.xaml, the build succeeds. It also builds if I revert back to Visual Studio 17.9.
The example provided was simply the minimum code to reproduce the issue. It is impossible to remove the direct reference to Logging in my core application. The main (WPF) project has a reference to two independent assemblies. One of them includes a reference to ASP.NET Core, the other does not. Both of them require logging and completely independent of each other. I can remove the redundant reference from the assembly that references ASP.NET Core, but not the other. Since the main project that contains a .XAML file is references both, the issue remains unresolved.
As mentioned in the original post, I can work around the issue by moving the the .XAML to a separate assembly, but the core issue still exists. Microsoft.Extensions.Logging.Generators.dll should detect if it has already generated code for a class and not generate the code a second time, or at least some investigation needs to be done to determine why the introduction of a .XAML file causes the issue.
p.s.
I'm not sure why the presence App.xaml doesn't cause the issue.
Another way to better explain why I believe this is still a bug is to change the steps to reproduce:
- Create a solution with a WPF application and two class libraries (say, Feature1 and Feature2).
- Make the WPF application reference both Feature1 and Feature2.
- Make Feature1 reference ASP.NET Core.
- Make Feature2 reference Logging.
- Add a class that uses [LoggerMessage] in the WPF project.
Since Feature1 and Feature2 could theoretically be third party libraries, we don't have control over the references. The only workaround would be to move the .XAML files to a separate assembly, or move the classes that use [LoggerMessage] to a separate assembly.
EDIT: This didn't break before upgrading to VS 17.10.
Referencing the package Microsoft.Extensions.Logging.Abstractions will means including the source generator twice in the compilation which will cause the generated code produced twice and cause compilation error because of the duplication. If you look at the build logs, you can notice the following two entries under the analyzers list:
It shouldn't. We are supposed to deduplicate the generators. That's how we're able to allow folks to reference newer packages than the framework they target (EG: System.Text.Json 8.0.0 on net6.0)
I took a look at the repro. The problem here is due to WPF's _CompileTemporaryAssembly Target. WPF generates a temporary project and compiles that - that project it's not running conflict resolution (nor ResolveAssemblyReferences at all). Instead, it seems that the GenerateTemporaryTargetAssembly task tries to pass down already resolved References and Analyzers. These are good, since they are deduplicated by the parent project,.
The problem comes because it looks like WPF runs ResolvePackageAssets which populates a number of items. This will populate Analyzers and then never deduplicate them. The only reason this isn't breaking normal references in the same way is because package resolution populates Reference and the compiler reads ReferencePath.
I'm not sure what changed here to break it, but it's pretty crazy the way it's working today. It's just by luck that the other things raised by ResolvePackageAssets don't cause problems. It's busted to run that without conflict resolution. IMO it shouldn't be run if the goal is to do all that work in the parent project and pass it down.
Could be similar to https://github.com/dotnet/extensions/pull/5024 and the fix should be in wpf instead?
CC @xakep139 @joperezr
I see this is in Future milestone - you might want to have a look sooner than that. Seems like something changed here recently that's regressing folks. @singhashish-wpf
We have this issue currently, and cannot get rid of it. Are there any workarounds we can try?
There are many tickets open on this same issue: https://github.com/dotnet/wpf/issues/10175 https://github.com/dotnet/wpf/issues/7624 https://github.com/dotnet/wpf/issues/10553
https://github.com/dotnet/wpf/issues/10175 Even has clear minimal reproduction steps.
@NicoKno #7624 contains a work around, but the work around itself has downsides as well (at least for us).
This issue is quite an annoying for us and are eager to see resolved. Thanks franchyd for the list.
What is the main issue through which this is tracked and will be resolved so we can only monitor that one to know when it is resolved?
WORK AROUND: Move the .XAML files to a separate assembly, or move the classes that use [LoggerMessage] to a separate assembly.
Following the MVVM pattern, if you have XAML files writing to the log files, this code is probably a good candidate for a view model or even a model or service which should handle logging. If you have code behind in your view that is performing low level code such as native Win32 API calls, you can call methods on your view model to perform logging.
WORK AROUND: Move the .XAML files to a separate assembly, or move the classes that use [LoggerMessage] to a separate assembly.
Following the MVVM pattern, if you have XAML files writing to the log files, this code is probably a good candidate for a view model or even a model or service which should handle logging. If you have code behind in your view that is performing low level code such as native Win32 API calls, you can call methods on your view model to perform logging.
This issue affects all “built in” source generators not just logger message. That list is:
- System.Text.Json.SourceGeneration.JsonSourceGenerator
- Microsoft.AspNetCore.Razor.SourceGenerators.RazorSourceGenerator
- Microsoft.AspNetCore.OpenApi.OpenApiDocumentGenerator
- Microsoft.AspNetCore.Mvc.Generator
- Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator
the one that impacts me is the JSON one.
TL;DR A simple workaround here would be to make sure conflict resolution runs in WPF's generated project:
<Target Name="_EnsureConflictResolution"
Condition="'$(_TargetAssemblyProjectName)' != ''"
AfterTargets="ResolveProjectReferences"
DependsOnTargets="_HandlePackageFileConflicts" />
Simply add this to any WPF project with this issue.
Bumping this back to 10.0.0 milestone. @singhashish-wpf @pchaurasia14 please prioritize.
Just to reiterate what I said before -- this is happening because WPF's targets run package resolution but don't resolve any conflicts that might happen between those packages and framework assemblies/analyzers (or other packages). Conflict resolution has been part of .NET Core since 2.0 and it's what enabled the framework to transition from entirely package-based (with 100+ packages) to using Targeting Packs. It's also what continues to permit newer overlapping packages - like System.Text.Json, or Microsoft.Extensions.Logging.Abstractions - so that folks can use a package newer than their framework and still have that work.
This is broken because of this: https://github.com/dotnet/wpf/blob/20ed0120dae50dee312cddb5d9239821be72535b/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft.WinFX.targets#L346-L349
So WPF is using the ResolveProjectReferences target to try and trigger package resolution 😕 Seems a dubious choice.
As I mentioned before they end up raising a lot more than just source generators (eg: ResolvedCompileFileDefinitions) - it just so happens that they don't run the other targets that would end up copying these over to ReferencePath items that would reach the compiler.
See above for a better workaround. This does use a couple private identifiers: _TargetAssemblyProjectName to determine if it's in WPF's generated project, and _HandlePackageFileConflicts.
A better fix in WPF might be to change what targets are run to resolve packages. I don't know why ResolveProjectReferences was chosen here: https://github.com/dotnet/wpf/commit/bd67c8f87342531d26bad5b4a84f3420e5290ab7#diff-681ca681001fbc20488df7fdbb46a8c40ac2df9dc2210df5ad869aee6a56df16R393-R394
Instead I think would suggest using $(ResolveAssemblyReferencesDependsOn) since this seems to be how the SDK defines the set of targets it needs to raise packages and resolve their conflicts. @dsplaisted what do you think?
I don't know a lot about how the WPF generated projects work. The suggestion to switch to using ResolveAssemblyReferencesDependsOn seems to make sense, but I think the temp project build may be somewhat fragile so it might be easy to change.
