Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source generator dependency issues #265

Open
Turnerj opened this issue Mar 25, 2021 · 3 comments
Open

Source generator dependency issues #265

Turnerj opened this issue Mar 25, 2021 · 3 comments
Labels
bug Issues describing a bug or pull requests fixing a bug.

Comments

@Turnerj
Copy link
Collaborator

Turnerj commented Mar 25, 2021

Background

When adding the source generator in #252, I stumbled across weird behaviour where I needed to specify additional dependencies that we didn't directly seem to need - in our case, this was System.Text.Encodings.Web. Source generators require some more verbose methods of specifying these NuGet packages too:

<ItemGroup Label="Package References">
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="3.9.0" />
<PackageReference Include="System.Text.Json" Version="5.0.1" GeneratePathProperty="true" PrivateAssets="all" />
<PackageReference Include="System.Text.Encodings.Web" Version="5.0.1" GeneratePathProperty="true" PrivateAssets="all" />
</ItemGroup>
<PropertyGroup>
<GetTargetPathDependsOn>$(GetTargetPathDependsOn);GetDependencyTargetPaths</GetTargetPathDependsOn>
</PropertyGroup>
<Target Name="GetDependencyTargetPaths">
<ItemGroup>
<TargetPathWithTargetPlatformMoniker Include="$(PKGSystem_Text_Json)\lib\netstandard2.0\System.Text.Json.dll" IncludeRuntimeDependency="false" />
<TargetPathWithTargetPlatformMoniker Include="$(PKGSystem_Text_Encodings_Web)\lib\netstandard2.0\System.Text.Encodings.Web.dll" IncludeRuntimeDependency="false" />
</ItemGroup>
</Target>

I thought this was the end of the problems - oh I was very wrong...

The Problem

Hitting the issue with requiring System.Text.Encodings.Web is small and not a big deal. The problem is (as briefly mentioned in #262) that needing to specify System.Text.Encodings.Web isn't a random issue - it is a dependency of System.Text.Json but as packages usually do, they can have many dependencies and their dependencies have dependencies.

The .NET runtime doesn't seem to care if an assembly is missing until it actually needs it which is the crux of our problem. Through the code paths in System.Text.Json, it invokes something in System.Text.Encodings.Web thus why it needed to be specified. When we upgraded to System.Text.Encodings.Web in #261 (updating from 5.0.0 to 5.0.1), that ended up being a breaking change for us.

Generator 'SchemaSourceGenerator' failed to initialize. It will not contribute to the output and compilation errors may occur as a result. Exception was of type 'FileNotFoundException' with message 'Could not load file or assembly
'System.Text.Encodings.Web, Version=5.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies. The system cannot find the file specified.'

Weird how we didn't have any CI errors in #261 then right? Yeah it turns out when compiled within the realm of .NET Framework (eg. MSBuild or Visual Studio) this error occurs but not via dotnet build. In #262 I thought the issue was an environmental one for me as everything else CI-wise seemed good. I raised an issue about this with the Roslyn team via dotnet/roslyn#52017 however the issues is ours, not theirs.

Source generators need to specify all dependencies in the entire dependency tree. The reason I hit this after #261 is that something in moving to System.Text.Encodings.Web 5.0.1 invoked a new path internally to one of its dependencies. The error message doesn't say unfortunately but it is either a dependency on System.Memory or System.Buffers as those are the dependencies for 5.0.1.

As a fun aside, I will note that under .NET Standard 2.0 dependencies for System.Text.Encodings.Web, it added System.Buffers when moving to 5.0.1 - prior to that it only had a dependency on System.Memory

Getting back on track now, fixing this error for System.Text.Encodings.Web can be done by adding System.Memory and System.Buffers directly like I already have for System.Text.Encodings.Web - so far, so good. Because we should be specifying every dependency in the tree, we actually should be specifying:

  • Microsoft.Bcl.AsyncInterfaces, 5.0.0
  • System.Buffers, 4.5.1
  • System.Memory, 4.5.4
  • System.Numerics.Vectors, 4.5.0
  • System.Runtime.CompilerServices.Unsafe, 5.0.0
  • System.Text.Encodings.Web, 5.0.0
  • System.Threading.Tasks.Extensions, 4.5.4

All that just for supporting System.Text.Json BUT even then it isn't as easy as it seems. If any of those packages get new dependencies, we would need to add them too and we can only tell that we need to if we hit the issue. Because our CI doesn't hit the issue, it will be adhoc if we discover it.

Solutions

Solution A

Just specify all those dependencies in that verbose format I showed earlier - it would solve the current problem for local development.

Solution B

Build some general purpose MSBuild target that can take a more normal package reference list and add all that verbose markup and entire dependency tree itself.


I'm personally looking into how Solution B would work as it would be a good issue to solve for the .NET ecosystem.

@Turnerj Turnerj added the bug Issues describing a bug or pull requests fixing a bug. label Mar 25, 2021
@RehanSaeed
Copy link
Owner

Wow, that seems pretty terrible. Is this just a temporary tooling bug that will get resolved in .NET 6?

@Turnerj
Copy link
Collaborator Author

Turnerj commented Mar 25, 2021

In the issue I raised with the Roslyn team, I asked if "there be any plans on changing this behaviour and having the compiler (or some other part of the build process) automatically handle this" however I haven't had a response. It was actually the second time in the thread when I asked, the first time I asked they seemed to have skipped over it only saying that it isn't a restriction but a requirement for source generators.

It is nearly like it is by-design to be this verbose and annoying but it really doesn't seem like a good design.

@Turnerj
Copy link
Collaborator Author

Turnerj commented May 21, 2021

FYI, I've now got a related issue to automating this on the dotnet SDK repo: dotnet/sdk#17775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug or pull requests fixing a bug.
Projects
None yet
Development

No branches or pull requests

2 participants