Generators - analyzer using references causing problems CS8785 #43903
Comments
mgravell
changed the title
|
unfortunately nuget does not know how to pack analyzer dependencies and we've never done to work to call nuget to dynamically resolve analyzer dependencies at runtime. Effectively, you need to place the |
|
Sounds like @jmarolf can guess what the error is. We have work to do to actually surface the errors, for now they're just swallowed unless you have a debugger on it. Current 'best guidance' (very loosely speaking) is to |
|
@jmarolf I'm not entirely sure what that means in practice as either an author or consumer. When you say nuget doesn't know... so: how does one most correctly ship an analyzer? Is this:
Or am I misunderstanding the problem? |
chsienki
added
Area-Analyzers
Area-Compilers
New Feature - Source Generators
|
I assume 1. |
|
If your analyzer assembly has unique dependencies (i.e. no other part of the hosting application references the dependency, and no other analyzers reference the dependency), you can place the dependency assembly in the same folder of the NuGet package where the analyzer assembly is. If the dependency is not unique, this may or may not work, and failures may be non-deterministic. The two cases where I know this has occurred in the past both ended up treating the situation like (1): remove or inline the dependency. |
|
Once you "do plugins" you have to solve all the same problems we solve today at build time with nuget (unification etc) |
|
yeah, we haven't tackled runtime-dependency-resolution for analyzers. As @davidfowl says, solving this would likely need to start with additions to nuget. |
|
Oof. Right, I guess I'm going to have to forget about transitive package refs for now, and do something horrible in my build, either with some "and include these .cs files", or maybe some IL-merge/fody fun. This could get interesting. |
|
Was about to submit a repro for this because it seems counter-intuitive that you can't simply reference some other otherwise uncontroversial I guess the least horrible thing to do here would be to post-build copy the package references to the analyzer build folder? |
|
It doesn't look like copying the offending dependency into the same location as the analyzer .DLL itself is working for me, maybe because Example code: |
|
And trying to ILMerge that single dependency quickly descended into madness: |
|
Whatever you do, please don't use |
|
I think the only sane solution is for Roslyn to run analyzers and code generators on .NET Core only. |
|
@tmat right now I'm thinking my best option is to hack a build that imports all the code I need directly as code, but: somewhere in this there is a "the story of package dependencies in analyzers is bad" |
|
@tmat Would love not to; how would you suggest I reference |
|
Currently working on a design to resolve this. Agree it needs to be solved |
Ah, good concept. I didn't think of stepping back into Roslyn to create snapshot tests, and wasn't aware of |
|
I am using it like this: <ItemGroup>
<!-- Take a private dependency on Newtonsoft.Json (PrivateAssets=all) Consumers of this generator will not reference it.
Set GeneratePathProperty=true so we can reference the binaries via the PKGNewtonsoft_Json property -->
<PackageReference Include="Newtonsoft.Json" Version="12.0.1" PrivateAssets="all" GeneratePathProperty="true" />
<!-- Package the generator in the analyzer directory of the nuget package -->
<None Include="$(OutputPath)\$(AssemblyName).dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
<!-- Package the Newtonsoft.Json dependency alongside the generator assembly -->
<None Include="$(PkgNewtonsoft_Json)\lib\netstandard2.0\*.dll" Pack="true" PackagePath="analyzers/dotnet/cs" Visible="false" />
</ItemGroup>as suggested in https://github.com/dotnet/roslyn/blob/master/docs/features/source-generators.cookbook.md#use-functionality-from-nuget-packages |
And I had include all nuget PackageReference as dll's in |
@jmarolf Any movement on this? |
|
@wieslawsoltes Note that Newtonsoft.Json is one of the dependencies we were never able to get to work reliably. We ended up embedding a JSON serialization layer as source code in the analyzer project itself, and now it works. @kjkrum Packaging dependencies manually is not especially difficult (there are many examples already available). If this approach works for you, the new design might simplify it. However, if this approach does not work for you (due to dependency conflicts at runtime), the new design will not fix the issue and despite automating the packaging process it will still not execute correctly. |
|
@sharwell The dependency conflicts issue is a deal breaker. Not only would it be annoying and fragile to manually add and maintain the recursive dependencies of a library like NSwag, but even if it works for me, I can't guarantee that another user of my generator will not have a conflict with some other analyzer. As currently designed, I don't see non-trivial source generators ever being practical. |
|
@kjkrum the only way around this (which is what you need to do for Visual Studio extensions and other .NET extensibility systems) is strong name sign your assemblies. Unless you are willing to do that we cannot guarantee the runtime will not fail to load some mis-matched version from another assembly. |
I'll put up a proposal next week in dotnet/designs |
|
@jmarolf Source generators that take the form of console applications packaged with build targets do not need to be strong name signed and AFAIK do not suffer from dependency conflicts. Those remain my go-to because the issue with zombie MSBuild tasks locking files was either never actually fixed, or was fixed after I gave up hoping for a fix, which was some time after it was claimed to be fixed. I'll watch for your proposal. |
Right, if you do the entire build again (in order to create the compilation context) then you do not need to load the assemblies into the same process at all. They can all live in separate processed. Unfortunately this kills build performance (this build twice model was already possible without a roslyn api). If a console app called from a build target meets your need today I do not think you should move to the roslyn source generators API. The only advantage the roslyn API gives is being able to introspect into the compilation in order to make decisions about what should be generated. We could have separate processes for all source generators in order to isolate them. This would either require us to rebuild the compiler to allow all of its internal state to be serializable (unclear what the performance implications would be here), or have each driver for source generators re-create the compilation context massively degrading build performance. I currently do not think this large re-write is worth it for the use case source generators were intended for: code generated in conjunction with the user typing. Especially since its unclear to me how often conflicting dependencies are going to happen in the wild. For now I only plan to have us show a useful diagnostic so that if this situation ever happens its clear to the developer what is going on. |
using VS 2022 v 17.0.4 with this no longer works: |
|
It gets weird... I am completely rebuilding the whole solution which produces the CS8785. The warning is there, with the explicit assembly name, source generator does not produce the output (as expected :-( ), but monitoring all processes with procmon64 no process seems to be even looking for that particular .dll. (No surprise it does not find it) if I monitor for the sourcegenerator package itself I can see VBCSCompiler is opening the dll from ..nuget\packages...........\analyzers\dotnet\cs.... As a crosscheck, if I monitor for only 'analyzers\dotnet\cs' path fragment, I see only the sourcegenerator package is searched (and successfully found there) So having the referenced assembly there is completely obsolete, and it is a puzzler why I does not see with procmon64 any process to looking for the "missing" assembly... |
StyleCop Analyzers never found a way to make Newtonsoft.Json a reliable dependency of an analyzer (or source generator). We ended up embedding a different JSON library as source. |
|
this issue is more generic and not about how to use json in a source generator... |
|
fair enough... hopefully this second version makes things more generic...
|
|
I've been facing this same issue with a similar CS8784 warning this past week as well trying to include It seems to work in a stand-alone setup with the suggested setup like this: <GetTargetPathDependsOn>$(GetTargetPathDependsOn);GetDependencyTargetPaths</GetTargetPathDependsOn>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.0.1" Pack="false" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.3" Pack="false" PrivateAssets="all" />
<PackageReference Include="YamlDotNet" Version="11.2.1" PrivateAssets="all" Pack="false" GeneratePathProperty="true" />
</ItemGroup>
<Target Name="GetDependencyTargetPaths">
<ItemGroup>
<TargetPathWithTargetPlatformMoniker Include="$(PKGYamlDotNet)\lib\netstandard1.3\YamlDotNet.dll" IncludeRuntimeDependency="false" />
</ItemGroup>
</Target>But is failing when I try and use this setup in our larger project. |
|
This is still a thing apparently. The suggestions here haven't worked for me (referencing the analyzer in the same solution). |
|
Generators need to run out of process. Performance takes a back seat to actually working and being useful. |
|
@kjkrum The performance penalty associated with process isolation for source generators isn't viable. In some instances today, we do run source generators in a separate process from Visual Studio. However, this is not sufficient to resolve the problems because more than one source generator end up running in that process, and their dependencies can conflict. Eventually we'll be able to separate them using AssemblyLoadContext in .NET Core, but that will only work once the remaining tools using .NET Framework move over. |
|
@sharwell Would it be practical to make it optional and let users decide if it's viable? |
|
Was this ever resolved? I'm also looking at figuring out how to just add a System.Text.Json reference to my generator and unclear on how it's meant to work. |
|
@sharwell If the "performance penalty" of console app source generators is viable (which it is) then running incremental source generators with process isolation is also viable. Do not make performance decisions for users. I'd rather wait minutes for a build than years for a minimum viable product. |
|
@kjkrum Just an FYI that Jonathan has sadly passed away. We're continuing to look at this space. But it is frought with challenges. |
We are continuing to work in this space. But we're a small team with a ton of responsibilities. I do have it as a goal to make it so that:
However, i would not expect this to come soon as each of these is a substantial amount of work, and we are often constrained by global requirements around compatibility. |
|
@CyrusNajmabadi Sorry to hear about Jonathon. FWIW, I don't blame your team at all. I blame Microsoft for releasing a product with such incredible potential in a half-finished state, and then leaving its completion to a small team with a ton of responsibilities. |
For every person who wants us waiting before releasing to get all these things done, we have just as many asking to release early in whatever state things are in just so they can use it :-) That very thing happened today already :-D |
|
@CyrusNajmabadi The early release is less of a problem than seemingly abandoning it for years afterward. My assumption is that someone in management is under the mistaken impression that it's "done" and has assigned resources elsewhere. |
It has not been abandoned. Several people, including myself are working in this space. However, the costs are enormous here. Especially given the dramatic impact SGs have on every single other layer that sits above them. |
There are different ways that we can end up making performance decisions for users, and not all of them are bad. For example:
To date, running generators in an isolated process runs afoul of the second case. In other words, we are not withholding a feature here; the requested feature simply does not exist. |
I have an idea for an analyzer I'd like to add, reusing functionality that is already packaged in a netstandard2.0 package.
Repro is here: protobuf-net/protobuf-net.Grpc#85 - the interesting projects are:
In my analyzer csproj I have the two expected refs, plus a commented out one for the tool I want to use:
The analyzer currently just lists the additional files (I have a
<AdditionalFiles Include="test.proto" />). With the above reference commented out, everything kinda works; however, if I uncomment the<PackageReference>and do nothing else, it stops working completely, citing:(the actual "do the thing" code is there but commented out)
This may simply be a packaging thing, but... it isn't obvious how to fix this; I've tried numerous things - with/without the
PrivateAssetshint, with<Analyzer>dll references vs project references to the analyzer, etc. It could be as simple as a dll version conflict, but I can't find any way of getting the info out to tell me what exploded.Any guidance @chsienki ?
The text was updated successfully, but these errors were encountered: