CarterCommunity / Carter

Carter is framework that is a thin layer of extension methods and functionality over ASP.NET Core allowing code to be more explicit and most importantly more enjoyable.
MIT License
2.05k stars 172 forks source link

Analyzer exception after update to Carter 8.2.0 #353

Closed goldenbeard closed 4 weeks ago

goldenbeard commented 1 month ago

After updating Carter from v8.1.0 to v8.2.0 I started seeing the following warning in my VS build log and also when running the dotnet build command:

CS8032 An instance of analyzer Carter.Analyzers.CarterModuleShouldNotHaveDependenciesAnalyzer cannot be created from xxx\nuget\packages\carter\8.2.0\analyzers\dotnet\cs\Carter.dll : Exception has been thrown by the target of an invocation..

I am building with Visual Studio 17.10.0 and .Net 8.0.6.

Is there anything I can do to help diagnose this error? I'm sure I can't be the only one experiencing this.

jchannon commented 1 month ago

Any ideas @CollinAlpert ?

CollinAlpert commented 1 month ago

@goldenbeard Either the stack trace of the "Exception has been thrown by the target of an invocation" or a minimal reproduction of the issue would be very helpful to be able to diagnose this.

goldenbeard commented 1 month ago

I could provide a minimal project to reproduce this, but the minimal project is literally a new .Net Core web API project with only Carter 8.2 added in. I'm sure if I send that it will work on your machine 😄 Creating a project like this still fails in Visual Studio build but succeeds without the error on dotnet build. I'll investigate how to turn on diagnostic logging and see if I can get a stack trace for you.

goldenbeard commented 1 month ago

Turning on diagnostic logging in Visual Studio hasn't helped at all. There's no additional information about the analyzer exception logged. I will try pulling the Carter 8.2.0 code and build locally in debug to see if that gives any additional information. vsbuild.zip

CollinAlpert commented 1 month ago

Usually you can expand the "Exception has been thrown by the target of an invocation" popup and it will show you more information including a stack trace.

goldenbeard commented 1 month ago

This is an exception thrown inside the CSC step of the build and only appears in the build log - there's no popup to expand.

CollinAlpert commented 1 month ago

Interesting. If this doesn't appear in Visual Studio it might be a VS bug.

goldenbeard commented 1 month ago

As I mentioned originally, when dependabot updated Carter in a private github enterprise project, I was seeing it there too. We use self-hosted runners for Windows and Ubuntu and saw it for both just running a dotnet build step. We noticed it mostly because we have enabled warnings as errors on that project.

goldenbeard commented 1 month ago

I'm not getting anywhere with this :( I built v8.2.0 of Carter locally as debug and copied the dll and pdb into my package cache analyzsers/cs directory - no difference in the CSC.exe output reported by Visual Studio. I tried setting up devenv.exe as the debug host for the Carter project and debugging that, this also doesn't make any difference as the analyzer appears to work under Visual Studio, it's failing in the msbuild process that VS spawns. I have tried copying the CSC.exe command line from my vs build log and making that the debug host for Carter but VS just fails to launch that, complaining that the 'path is too long' to csc.exe 🤷 I have run out of ideas.

CollinAlpert commented 1 month ago

That's annoying. Can you create a reproduction that fails when executed with csc.exe that I can look at?

goldenbeard commented 1 month ago

I created a new classlib project and managed to reduce down the /references list on the csc.exe command line issued by Visual Studio enough to actually get it to run in a command prompt... I have attached the project (it's literally a new classlib project with Carter 8.2.0 added as a nuget reference) and the command line I ran is in a text file inside the zip. You'll obviously need to update some paths in that to your nuget package cache and run the command from the csproj folder. When running this command I get the following output (just the same as Visual Studio running the build):

Microsoft (R) Visual C# Compiler version 4.10.0-3.24270.2 (e8f775c1) Copyright (C) Microsoft Corporation. All rights reserved.

warning CS8032: An instance of analyzer Carter.Analyzers.CarterModuleShouldNotHaveDependenciesAnalyzer cannot be created from D:\nuget\packages\carter\8.2.0\analyzers\dotnet\cs\Carter.dll : Exception has been thrown by the target of an invocation..

I'm still not able to find the magic that will allow me to use this command as the launch host for a Carter project debug session. With this reduced command it now launches the debugger but exits immediately. Minimal-test.zip

I hope this helps!

CollinAlpert commented 1 month ago

Thanks! I'll take a look on the weekend.

iammukeshm commented 1 month ago
Warning CS8032  An instance of analyzer Carter.Analyzers.CarterModuleShouldNotHaveDependenciesAnalyzer cannot be created from C:\Users\iammu\.nuget\packages\carter\8.2.0\analyzers\dotnet\cs\Carter.dll : Could not load file or assembly 'System.Collections.Immutable, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified..

I am seeing this issue post migrating to 8.2.0. Is this a known issue @CollinAlpert ?

CollinAlpert commented 1 month ago

@jchannon These seem to be related to the compatibility issues I mentioned due to not targeting netstandard2.0 with the analyzer. Would you like me to move the analyzer to a separate project and target netstandard2.0?

jchannon commented 1 month ago

I guess we should but I'm slightly confused because Carter only targets net8 and by making the analyser netstandard2.0 that implies it will work on .NET Framework but Carter doesn't run on .NET Framework but net8 unless .NET Framework can run net8 and additional things. What a mess :)

On Mon, 3 Jun 2024 at 12:13, Collin Alpert @.***> wrote:

@jchannon https://github.com/jchannon These seem to be related to the compatibility issues I mentioned due to not targeting netstandard2.0 with the analyzer. Would you like me to move the analyzer to a separate project and target netstandard2.0?

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/353#issuecomment-2144927324, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJQBMMMQD3FUWONTFSDZFRFUXAVCNFSM6AAAAABIQMW2A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBUHEZDOMZSGQ . You are receiving this because you were mentioned.Message ID: @.***>

CollinAlpert commented 4 weeks ago

The reasoning is that analyzers can theoretically run on NetFx, which is why they need to target netstandard2.0. I agree, what a mess :)

jchannon commented 4 weeks ago

@goldenbeard @iammukeshm can you add a new nuget feed path to your config https://f.feedz.io/carter/carter/nuget/index.json to Carters CI build and install Carter.8.2.1-alpha.0.2.nupkg to see if this fixes it please.

goldenbeard commented 4 weeks ago

@jchannon I've tried as you suggested, but I'm unable to add Carter.8.2.1-alpha.0.2.nupkg into a project as it says it can't find the dependency Carter.Analyzers 1.0.0 on any nuget feed.

goldenbeard commented 4 weeks ago

I discovered recently that project references are now automatically converted to package references by the dotnet pack process. Could this be what's happening here? If your intention was to include the Carter.Analyzers project output in the Carter nuget package then I believe you might be able to override this behaviour using the -IncludeReferencedProjects command line argument to dotnet pack.

CollinAlpert commented 4 weeks ago

@goldenbeard That indeed might be what is happening. @jchannon Can you verify?

jchannon commented 4 weeks ago

I think the issues is the analyzers project isnt packed and pushed to feedz/nuget. I'll need to check the build script today/tonight or if someone is keen they can take a look :)

On Thu, 6 Jun 2024 at 08:56, Collin Alpert @.***> wrote:

@goldenbeard https://github.com/goldenbeard That indeed might be what is happening. @jchannon https://github.com/jchannon Can you verify?

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/353#issuecomment-2151644660, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJVG3JCIL7MR4YW53YDZGAI3LAVCNFSM6AAAAABIQMW2A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRGY2DINRWGA . You are receiving this because you were mentioned.Message ID: @.***>

goldenbeard commented 4 weeks ago

It looks like the -IncludeReferencedPackages is mostly intended for legacy packages.config .Net Framework projects. I've taken a look at the code in the PR and I believe that the fix might be as simple as adding ReferenceOutputAssembly="false" into the ProjectReference of the csproj file.

CollinAlpert commented 4 weeks ago

I believe that the fix might be as simple as adding ReferenceOutputAssembly="false" into the ProjectReference of the csproj file

The problem with that is that pack won't work at all, since the analyzer DLL needs to be packed inside the package. https://github.com/CarterCommunity/Carter/blob/20567067655b1b6710b3bbdfcd6f89815c110649/src/Carter/Carter.csproj#L20

CollinAlpert commented 4 weeks ago

Looking at https://github.com/dotnet/roslyn/issues/26222, the recommendation is to ship the analyzer in a separate package and then include the analyzer package in the library if still necessary. @jchannon Is that something you'd be okay with?

jchannon commented 4 weeks ago

If I look inside the nupkg that is generated the Carter.Analysers project/dll is there so not sure what's going on

Screenshot 2024-06-06 at 10 26 48

goldenbeard commented 4 weeks ago

I believe that the fix might be as simple as adding ReferenceOutputAssembly="false" into the ProjectReference of the csproj file

The problem with that is that pack won't work at all, since the analyzer DLL needs to be packed inside the package.

https://github.com/CarterCommunity/Carter/blob/20567067655b1b6710b3bbdfcd6f89815c110649/src/Carter/Carter.csproj#L20

@CollinAlpert I think it will work just because of the line in the csproj that you quoted. What you are trying to do is include the dll in the analyzers folder of the nuget package but exclude the nuget dependency that's automatically getting added by the pack step of the build.

goldenbeard commented 4 weeks ago

The alternative is, as you suggested, publish the Analyzer as its own nuget package.

jchannon commented 4 weeks ago

Please try Carter.8.2.1-alpha.0.3.nupkg

I've add a pack to the analyzer package to see if that magically works

On Thu, 6 Jun 2024 at 10:47, goldenbeard @.***> wrote:

The alternative is, as you suggested, publish the Analyzer as its own nuget package.

— Reply to this email directly, view it on GitHub https://github.com/CarterCommunity/Carter/issues/353#issuecomment-2151864565, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZVJVYPNDBXN646WAJGYTZGAV4BAVCNFSM6AAAAABIQMW2A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJRHA3DINJWGU . You are receiving this because you were mentioned.Message ID: @.***>

goldenbeard commented 4 weeks ago

@jchannon I have tried that version and can confirm that it found the Carter.Analyzers preview nuget package as a dependency and installed it alongside the Carter preview nuget package. After doing this I get no warnings in the build. Thanks for your help!

jchannon commented 4 weeks ago

jchannon commented 4 weeks ago

8.2.1 is deployed to Nuget!

Thanks @goldenbeard @CollinAlpert @iammukeshm @mariusz96 for the effort, much appreciated!