jamescourtney / FlatSharp

Fast, idiomatic C# implementation of Flatbuffers
Apache License 2.0
497 stars 50 forks source link

Getting TypeLoadException when trying to access FlatSharp types #385

Closed njannink closed 6 months ago

njannink commented 1 year ago

I'm trying to migrate from Google flatbuffers nuget to FlatSharp (7.1.1) and I run into a problem.

Situation

My netstandard2.0 project is compilated without issues, but when I try to run my unittest code I get the following exceptions:

System.TypeLoadException: Virtual static method 'get_LazySerializer' is not implemented on type 'xxx' from assembly 'yyy'.

I guess this has to do with Net7 the static definitions on interfaces, but in a mixed scenario that might not work?

jamescourtney commented 1 year ago

FlatSharp supports both. The these are generally hidden behind #if definitions. The FlatSharp tests actually run on .NET Framework, .NET standard, and .NET Core to test that I don't break things like this.

I've just checked in on the generated code, and the static definitions are indeed hidden behind preprocessor checks:


#if NET7_0_OR_GREATER
        static ISerializer<FlatSharpEndToEndTests.TableMembers.StringTable> IFlatBufferSerializable<FlatSharpEndToEndTests.TableMembers.StringTable>.LazySerializer { get; } = new FlatSharp.Compiler.Generated.N23CE084EA646E303A1AA88FAF338FAD75CE055E8444E3104C8E29DF9CDC39319.Serializer().AsISerializer(global::FlatSharp.FlatBufferDeserializationOption.Lazy);
#endif
#if NET7_0_OR_GREATER
        static ISerializer<FlatSharpEndToEndTests.TableMembers.StringTable> IFlatBufferSerializable<FlatSharpEndToEndTests.TableMembers.StringTable>.GreedySerializer { get; } = new FlatSharp.Compiler.Generated.N23CE084EA646E303A1AA88FAF338FAD75CE055E8444E3104C8E29DF9CDC39319.Serializer().AsISerializer(global::FlatSharp.FlatBufferDeserializationOption.Greedy);
#endif
#if NET7_0_OR_GREATER
        static ISerializer<FlatSharpEndToEndTests.TableMembers.StringTable> IFlatBufferSerializable<FlatSharpEndToEndTests.TableMembers.StringTable>.GreedyMutableSerializer { get; } = new FlatSharp.Compiler.Generated.N23CE084EA646E303A1AA88FAF338FAD75CE055E8444E3104C8E29DF9CDC39319.Serializer().AsISerializer(global::FlatSharp.FlatBufferDeserializationOption.GreedyMutable);
#endif
#if NET7_0_OR_GREATER
        static ISerializer<FlatSharpEndToEndTests.TableMembers.StringTable> IFlatBufferSerializable<FlatSharpEndToEndTests.TableMembers.StringTable>.ProgressiveSerializer { get; } = new FlatSharp.Compiler.Generated.N23CE084EA646E303A1AA88FAF338FAD75CE055E8444E3104C8E29DF9CDC39319.Serializer().AsISerializer(global::FlatSharp.FlatBufferDeserializationOption.Progressive);
#endif

There are a couple of things to check here:

jamescourtney commented 1 year ago

Additionally, it would be enormously helpful if you could share a solution file that reproduces the issue. Thanks!

njannink commented 1 year ago

FlatSharp.zip

I created a repro solution. Schemas dll (netstandard2.1) and a mstest project (Net7)

Please let met know if something is wrong in the setup

jamescourtney commented 1 year ago

Thanks for sharing this! This makes the problem much more clear.

The issue is:

So, the problem is that the version of FlatSharp.Runtime in the output folder expects static interfaces to be implemented, but since the Schemas project was built targeting netstandard2.1, there are no implementations of those methods, which leads to the error.

Honestly, I'm a little surprised Nuget doesn't handle the transitive dependency with more grace. I would have expected that since Tests doesn't reference FlatSharp, but Schemas does, then the netstandard2.1 version would get copied.

As far as fixing this, I think the simplest option is to multitarget your schemas project:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.1;net7.0</TargetFrameworks>
  </PropertyGroup>

Alternatively, you could downgrade the Tests project to net6.0, but that seems less appealing. I'm not quite sure how I could fix this from the FlatSharp side beyond publishing FlatSharp.Runtime and FlatSharp.Runtime.StaticInterfaces as separate packages, which doesn't feel like a great thing to do.

What do you think?

njannink commented 1 year ago

We are shipping our own schemas library so adding a secondary target framework isn't really what we want to do because if will also increase the size of our nugets and the idea behind netstandard2.1 is long term compatibility so I would expect FlatSharp to do the same. Isn't there a possibility to just disable these static interfaces otherwise we would be forced to use FlatSharp6 or stay with the Google library that we are currently using

jamescourtney commented 1 year ago

We are shipping our own schemas library so adding a secondary target framework isn't really what we want to do because if will also increase the size of our nugets

By a very small amount. The FlatSharp.Runtime nupkg is 660kb. It includes entirely different binaries for netstandard2.0, netstandard2.1, net6.0, and net7.0. The reason for this is that it allows FlatSharp to take advantage of new APIs available in new frameworks and not always target the lowest common denominator provided by netstandard. FlatSharp on net7.0 is meaningfully faster than FlatSharp on netstandard due to having more APIs that interact with Span<T>.

the idea behind netstandard2.1 is long term compatibility so I would expect FlatSharp to do the same.

FlatSharp does support netstandard2.1. You are seeing issues by referencing a netstandard2.1 project from a net7 project. Like I outlined above, this is due to Nuget pulling in the wrong version of the FlatSharp.Runtime assembly with your test project. I don't see how this is a FlatSharp problem.

Isn't there a possibility to just disable these static interfaces

I'm a solo developer here, and I don't want to be in the business of maintaining and testing different builds of FlatSharp for every possible permutation of language features. Disabling static interfaces touches more than the codegen, since it also requires a new build of FlatSharp.Runtime.

njannink commented 1 year ago

I understand its more a NuGet issue than a FlatSharp issue.

As a developer its always tempting to use the latest language features like these static interfaces, but functional wise it doesn't add anything new.

Anyway I will probably go for the duplicate targetframeworks for the time-being hoping that Microsoft will resolve this issue.

Maybe we can create a defect out of this for the Roslyn project?

njannink commented 11 months ago

@jamescourtney switching to multiple frameworks makes my tests pass, but it more or less breaks Visual Studio. When using this approach I get Ambiguous references erros in the IDE and it breaks things like refactoring options eg resolve usings.

image

Maybe the Net7 code generation can be a FlatSharp compiler switch or attribute that is default on, but can be disabled in my case?

jamescourtney commented 6 months ago

Hi @njannink -- I've unintentionally discovered a solution for this problem. It's a little bit complicated, but will work. For the record, I continue to see this as a NuGet issue, but there is a way for you to reference netstandard2.1 from a net7.0 project:

Update your test csproj to contain the following:

  <!-- Define a dependency on FlatSharp.Runtime but don't take an assembly reference -->
  <ItemGroup>
    <PackageReference Include="FlatSharp.Runtime" Version="7.6.0" GeneratePathProperty="true" ExcludeAssets="all" />
  </ItemGroup>

  <!-- Take the assembly reference yourself so you can set the version -->
  <ItemGroup>
    <Reference Include="FlatSharp.Runtime">
      <HintPath>$(PkgFlatSharp_Runtime)\lib\netstandard2.1\FlatSharp.Runtime.dll</HintPath>
      <Private>true</Private>
    </Reference>
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\FlatSharp.Schemas\FlatSharp.Schemas.csproj" />
  </ItemGroup>