Open pablopioli opened 1 year ago
Using System.Text.Json
seems like a good idea, but copying large chunks of BouncyCastle into the library is not such a good idea because it creates a fork of BouncyCastle that's specific to certes (and there may be licensing issues). I would suggest you fork BouncyCastle for your own project and reference that (I also do this to get a much smaller BC library build).
The extra parts required to allow for NativeAOT seem like a lot of change for no benefit (to certes). Is there no other way that the serialization can work without marshalling copies of things to new types?
The BouncyCastle part was just for verifying that I could compile all in AOT mode. Now that BouncyCastle Portable has been deprecated (and with several security holes) and replaced with the new BouncyCastle Cryptography library I would take the effort to migrate to BCC 2.2.1, apparently there are only a few changes required. Then I could send pull requests to this project separately.
"The extra parts required to allow for NativeAOT seem like a lot of change for no benefit (to certes). Is there no other way that the serialization can work without marshalling copies of things to new types?" The problem in AOT is that you can't use reflection, the trimmer will remove all the types and properties that you don't reference in at least an reachable line of code. So the solution is to configure everything on compile time. This class https://github.com/pablopioli/certes/blob/main/src/Certes/CertesJsonSerializerContext.cs instructs the compiler to generate all the source code needed to property serialize and deserialize the referenced classes, using source generators.
Additionaly I had to create a couple of hardcoded classes in the cases where you use anonymous classes. Those clases cannot be reached by the source generator. So I copied the data to an intermediate class, I use this class only for serialization and I can avoid touching larger parts of the codebase. Polymorphism was another challenge without a clean resolution.
And, finally, I didn't wanted to drop support for 4.6.2, 6.0, etc. so I had to add a lot of extra code. On the upside, the final DLL includes only the necessary code for the platform.
I took a look and found that upgrading BouncyCastle is really easy. I opened another issue for it and removed the BC code from my repository.
So now the question boils down to, the maintainers of this library will be interested in changing Newtonsoft for Text.Json?
Thanks for the explanation, I see that you still have conditional compilation to reference Newtonsoft, is that necessary even for net462 etc? I thought System.Text.Json was available for netstandard2.0 (so net462 as well).
I have not tested it but Text.Json even lists 4.6.2 as a target
https://www.nuget.org/packages/System.Text.Json/8.0.0-rc.2.23479.6#dependencies-body-tab
I guess it goes with MS intention of deprecate Newtonsonft eventually.
Theoretically it you compile with the SDK of .NET8 the source generator will generate all the necessary code at compile time making the runtime differences negligible.
https://devblogs.microsoft.com/dotnet/system-text-json-in-dotnet-8/
.NET 8 is including a Native AOT mode https://learn.microsoft.com/en-us/dotnet/core/deploying/native-aot
Certes cannot operate in this mode because of the NewtonsoftJson dependency. You need to use System.Text.Json if you want to make it work.
So I added .NET 8 as a compilation target and make all the modifications needed to use Text.Json on this version. Previous versions of .NET wasn't modified
This big update can be seen in https://github.com/pablopioli/certes/commit/4c3bb73179d4d582e573accb00e9f4bef07cfabc
If there is interest in this changes I could make a pull request, but I ask here first because there are a lot of ugly #ifs everywhere.
Additionaly, you will see that I copied the code for BouncyCastle. This was not strictly necessary but this library needed an additional change to make it work on NativeAOT, so this way was easier.
All tests pass on all platforms and it compiles cleanly to native code.