netstandard 2.1 support for Orleans.Serialization

[jsteinich]

Make Orleans.Serialization compatible with netstandard 2.1 to enable greater usage outside of Orleans.

Note: it would be possible to support netstandard 2.0; however, that requires quite a bit more conditionals or removal of some niceties.

Microsoft Reviewers: Open in CodeFlow

[ReubenBond]

Thank you for doing this. Is it possible to add a < .NET 6.0 target to the serialization unit tests so that it has coverage? Perhaps .NET 4.8.1 (since it's not end of life, unlike .NET Core 3.1 and .NET 5.0)

[jsteinich]

Thank you for doing this. Is it possible to add a < .NET 6.0 target to the serialization unit tests so that it has coverage? Perhaps .NET 4.8.1 (since it's not end of life, unlike .NET Core 3.1 and .NET 5.0)

I went with .NET Core 3.1 since .NET 4.8.1 doesn't actually support netstandard2.1.
I still need to update the NumericsWideningAndNarrowingTests since that is a complete rewrite.

[jsteinich]

There are currently a handful of failing tests. The common aspect is needing to deserialize a string. The failing tests are from FieldCodecTester: CanRoundTripViaSerializer_MemoryStream and CanRoundTripViaSerializer_StreamPooled. All other tests for the same codec are passing. This seems likely to point to a singular issue, but I'm not quite sure what makes these particular tests unqiue.

System.ArgumentException: Destination is too short. (Parameter 'destination')

System.ArgumentException
Destination is too short. (Parameter 'destination')
   at Orleans.Serialization.Buffers.Reader`1.ReadBytes(Span`1 destination) in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization\Buffers\Reader.cs:line 757
   at Orleans.Serialization.Codecs.StringCodec.ReadMultiSegment[TInput](Reader`1& reader, UInt32 numBytes) in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization\Codecs\StringCodec.cs:line 58
   at Orleans.Serialization.Codecs.StringCodec.Orleans.Serialization.Codecs.IFieldCodec<System.String>.ReadValue[TInput](Reader`1& reader, Field field) in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization\Codecs\StringCodec.cs:line 18
   at Orleans.Serialization.Serializer`1.Deserialize[TInput](Reader`1& source) in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization\Serializer.cs:line 722
   at Orleans.Serialization.TestKit.FieldCodecTester`2.<>c__DisplayClass28_0.<CanRoundTripViaSerializer_MemoryStream>g__Test|0(TValue original) in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization.TestKit\FieldCodecTester.cs:line 326
   at Orleans.Serialization.TestKit.FieldCodecTester`2.CanRoundTripViaSerializer_MemoryStream() in C:\Users\jonsteinich\Documents\orleans\src\Orleans.Serialization.TestKit\FieldCodecTester.cs:line 305

[ReubenBond]

LGTM. My only concerns are:

• This is somehow not producing a bitstream which is compatible with .NET 7.0+ (I believe it is, but perhaps I missed something)
• The test is targeting an unsupported version of .NET (.NET Core 3.1)

[ReubenBond]

@jsteinich I opened a PR against your fork to fix the test failures: PerBlue#2

The .NET Core 3.1 tests aren't being run in Azure DevOps yet. I'll see if we can fix that easily

[jsteinich]

@jsteinich I opened a PR against your fork to fix the test failures: PerBlue#2

Confirmed that it fixed the test issues on my end as well. Thanks for fixing that; would have probably taken a bit to track that one down.

• The test is targeting an unsupported version of .NET (.NET Core 3.1)

This is a bit unfortunate, but I don't believe there is a way to run a test against netstandard2.1 directly and anything newer wouldn't fully be testing all conditional code.

[ReubenBond]

This is a bit unfortunate, but I don't believe there is a way to run a test against netstandard2.1 directly and anything newer wouldn't fully be testing all conditional code.

You're right, I don't think we have any options.

[pentp]

These conditions aren't going to work as expected on .NET 5 or 6 because we don't have a build target for these frameworks and so these conditions will be always false when running on .net3.1 - 6.0.

[pentp]

This block could have been made conditional on NET7+ and the existing fallback would work on any other platform.

[pentp]

These conditions are not going to work as expected on .NET 5 or 6.

[ReubenBond]

Perhaps we should add additional targets. There are likely some conditions which can be lowered to previous versions, like this one.

[jsteinich]

I contemplated just using the NET7_0_OR_GREATER everywhere, but felt having conditions that better match the actual dependency made more sense. The net effect (since only 2 build targets exist) is as if only the single condition was used, but I can see how someone looking at the source might be confused.