Support automatic transient dependency packaging for source generators #17775
Comments
|
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
|
@ericstj are you the right person to comment here? |
I don't know, depends on what you want in the way of comments. Here's some thoughts. cc @jaredpar @chsienki @KathleenDollard I would have expected that CopyLocalLockFileAssemblies would be enough to make Roslyn happy, but it doesn't seem to probe next to the analyzer assembly. You have to pass it into the compiler. Here's @Turnerj's repro with that change that demonstrates that (seems like a fix to Roslyn could make this better). ericstj/SourceGeneratorDependencyTest@e80fbd3 Related to this is how analyzers pack. Ideally the same mechanism that decides what should be passed via a project reference to an analyzer also determines what's included in a an analyzers package. Next issue: source generators and analyzers are Next issue: dependencies for plugins that run on desktop don't really work well unless the plugin model supports bindingRedirects. Analyzers do not as far as I know. @jaredpar @chsienki how does this work when dependencies are passed to the compiler via the /analyzer flag, will roslyn automatically unify everyone to the set of dependencies passed in (deduping as well)? |
At a high level the compiler approach is to call Most analyzers / generators that have dependencies tend to be very simple, one layer deep dependencies. Likely because that is all that will work due to the issues you're outlining above. There also isn't a lot of room for us to do binding redirects. In the command line compiler / server yes we could think about doing this. But it would be custom implemented because we can't use Overall though the solution here is getting to a point where the compiler is always running on .NET Core. In that mode we use |
That's what I would have expected, in which case the runtime would probe next to the analyzer assembly, but that doesn't seem to be the case. Looks to me like roslyn makes a copy of the analyzer before loading it and misses copying any adjacent files. This is from a run with This was when loading analyzer from a p2p reference. I don't see how this would ever work since Roslyn is copying each analyzer to its own directory. The repro required a bindingRedirect for In both cases it seems like roslyn shares the same AppDomain for analyzer loads so compiler server path will work if you happen to pass all dependencies to the compiler and dependencies happen to be passed in breadth first order from lowest in the stack up. All of these were tested using .NETFramework. I see no failure in .NETCore, where I believe this case happens to work because the dependencies all happen to exist in the shared framework. |
Yep that is the "details are more complicated part". 😄 How you invoke the compiler changes the details here. The command line will just use The end result is roughly the same though as the shadow copy directory is meant to mirror the original. Hence at a high level it should still be
This is by explicit design. The compiler will only copy the arguments to |
|
I see. So in order to have dependencies passed to the compiler you need to pass everything as an You can't rely on assemblies being next to the analyzer, though this works for csc command, it doesn't work for the compiler server due to the copying done by the compiler (this means that in general analyzers can't rely on doing any file relative loading, good thing to remember). I was noticing what I thought was an ordering issue with loading, but I think this was actually a bindingRedirect issue. It seems the compiler loads all analyzer assemblies before it attempts to get types from those assemblies. This is what allows passing dependencies as analyzers to work, since the compiler loads them up front. The lack of bindingRedirect handling seems like a pretty significant limitation and seems like something folks writing analyzers/generators with dependencies need to consider up front.
Solve this by adding
You're using the wrong item (compile assets, instead of runtime) and you're using an earlier stage of it (before conflict resolution and RAR). Try setting |
|
Thanks @ericstj , I'll try that out when I can as it would be a good workaround until the SDK does this automatically. I mean if it works well, could the SDK have this target built-in so myself (and others) wouldn't need that target for every source generator? Would that simplify things on your end or actually make it more complicated? |
|
I'm not sure having support for that in the SDK makes sense unless the SDK adds first class support for analyzers as project references, and potentially as packages too. If it did that, I'd expect it to have some sort of default understanding for what an analyzer project reference looked like and how to treat it's dependencies. |
|
From my point of view (one where I don't know how much work would actually be required to do this), if adding first class support for analyzers as project references etc too gets me/us/.NET developers closer to a seamless experience with transient dependencies, that would be fantastic. Just right now, dealing with transient dependencies for source generators suck. |
|
I hate to be that person but is there anyway I can help push this forward? Do I need to convince more people this is a problem or help contribute to a PR for this? |
|
@Turnerj I can tell you it's a problem for me, and if you get just ONE person to comment, that means there are 100 others with the same problem but not commenting... I don't have the bandwidth to learn the complex inner workings of Roslyn, msbuild and whatnot just to deliver a source generator. Not a wise use of my dev time. So I'm also quite supportive of a better dev experience for source generators, be it when referenced from another solution project, or when packaged in a nuget. |
|
I am also interested in an automatic solution. Here is a workaround for automatically detecting explicit and transient dependencies (with a bit of overhead) for those publishing generators via NuGet: <!--
https://github.com/dotnet/roslyn/issues/52017#issuecomment-1046216200
This automatically adds explicit and transient dependencies so that they are available at the time the generator is executed.
-->
<Target Name="AddGenerationTimeReferences" AfterTargets="ResolvePackageDependenciesForBuild">
<ItemGroup>
<None Include="@(ResolvedCompileFileDefinitions)" Pack="true" PackagePath="analyzers/dotnet/cs" />
</ItemGroup>
</Target> |
|
This issue needs fixing. I don't find a good way to make it work for "dotnet msbuild" and "msbuild". Here is the snippet that has a workaround That works when I build it from Visual Studio or use "msbuild" from the command line. But that doesn't work with "dotnet msbuild" This works with "dotnet msbuild" but not "msbuild" or Visual Studio |
If you need both to work, you can try to specify it explicitly at run via your custom property, for example <Target Name="GetDependencyTargetPaths" AfterTargets="ResolvePackageDependenciesForBuild">
<ItemGroup Condition="'$(FromMSBuild)">
<TargetPathWithTargetPlatformMoniker Include="@(ResolvedCompileFileDefinitions)" IncludeRuntimeDependency="false" />
</ItemGroup>
<ItemGroup Condition="'$(FromDotnet)">
<None Include="@(ResolvedCompileFileDefinitions)" Pack="true" PackagePath="analyzers/dotnet/cs" />
</ItemGroup>
</Target> |
|
@HavenDV Thanks for the suggestion. That'll work. The only issue is that we need to remember to set that property, depend on what we use. |
|
Can you use the |
That works. Thanks. |
|
I often work with generators and ended up with the following helper library: <PackageReference Include="H.Generators.Extensions" Version="1.4.2" PrivateAssets="all" />I want to note that PrivateAssets="all" is required to rule out some issues. |
|
@HavenDV this package is super useful, thank you for sharing it! |
|
@HavenDV I tried using this package with just this in my generator project: And upon building the generator I get: The generator works as normal (minus the issue as stated from this ticket) with just this in the csproj: Any tips on what's going on? Also not sure if this is related, but I've been having a hell of a time trying to get this thing packaged as nuget for .netstandard2.0. What put me on this path was trying to install this package in nuget and getting this error: Here's a link to the generator as well for good measure. It works locally but is totally borked with nuget. |
H.Generators.Extensions are for Roslyn 4.0+. You can look at this generator, it uses automatic dependency inference based on this package: |
|
@HavenDV should generator packing "just work" then without a dep on transient dependencies? I know this is dipping outside the context of the initial issue but this and @Turnerj's inital blog post are the only resources I've found on people actually shipping generators to nuget as well as the surprisingly large amount of issues that occur. |
|
https://github.com/HavenDV/H.Generators.Extensions/blob/main/src/libs/H.Generators.Extensions/build/H.Generators.Extensions.props |
|
It seems that roslyn-sdk official sample might give us the current solution: CSharpSourceGeneratorSamples |
|
Just here to agree this is awful and needs fixed. I couldn't get the roslyn-sdk official sample solution to work, or any other solutions except for explicitly adding all deps. Now I have a bunch of build warnings but it seems to compile.....for now. |
|
+1 This should work out of the box. |
|
I have received.
|
This is confusing, that official sample and the cookbook has different ways: Also agree that this should work out of the box |
|
I have received.
|
|
Just because the thread was bumped... tbh it would be nice to have a target for generators that supports the latest net core release instead of being pinned to netstandard2.0 - or on the flipside, would just love to have first class modern System.Text.JSON in a source generator without needing the additional package download and dep tree. I suspect that a large % of generators use it (or Newtonsoft) and honestly just having it packed would solve my dependency woes because it's the only thing I need. |
|
Its wild that this issue is still open 3 years later. |
|
I have received.
|
Originally raised as part of an issue with Roslyn (dotnet/roslyn#52017), it was suggested that I raise it here to potentially get the ball rolling on this.
Basically from a user's point of view, having private dependencies in your source generator can become unwieldy very fast when factoring in transient dependencies. For example, this is what it looks like to add
System.Text.Jsonas a private dependency while factoring in transient dependencies.Not only am I needing to be more careful about package version updates and their transient dependencies (the linked Roslyn issue came about because upgrading
System.Text.Encodings.Webfrom5.0.0to5.0.1brought in two more dependencies for .NET Standard 2.0) but it seems like I'm doing the work the compiler/build system should be doing for me.So far to avoid doing that, I've managed to hack in a semi-automated behaviour with the following:
It is not great for multiple reasons (
ResolvedCompileFileDefinitionsisn't always available & includes more than the dependencies that we actually want to bundle) however this still seems like a more practical solution than manually specifying transient dependencies.From my point of view, the ideal scenario is simply specifying
PrivateAssets="all"and the build system takes care of the rest.The text was updated successfully, but these errors were encountered: