-
Notifications
You must be signed in to change notification settings - Fork 51
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
.Net 3.5 support #52
Comments
I'd be happy to include a net35 build of FsPickler in its nuget package, let me know if you need any help. |
Ok so I tried building with net35 and there were quite a few errors, most of which relate to the lack of concurrent collections. These can be obtained using the TPL backport. The other errors seem more or less fixable. |
I'll give this a bash this evening. Are you okay with adding a dependency on the TPL backport? |
I think I would rather have it as a separate package.
|
Could you elaborate on what you mean by 'separate package' ? I'm only referring to the specific 3.5 build adding it as a dependency. I have a few questions about the port: How do we want to handle the conditional compilation flags for NET35 and NET40? It''s pretty ugly looking if we define them separately: #if NET35
do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET versions < 4.5.")
let bw = new BinaryWriter(stream, encoding)
#else
#if NET40
do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET versions < 4.5.")
let bw = new BinaryWriter(stream, encoding)
#else
let bw = new BinaryWriter(stream, encoding, leaveOpen)
#endif
#endif Unfortunately, we can't use #if NET35OR40
do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET 40.")
let bw = new BinaryWriter(stream, encoding)
#else
let bw = new BinaryWriter(stream, encoding, leaveOpen)
#endif Otherwise, from what I can tell, there's a few non-obvious functions that will need a replacement in 3.5:
If you have any input on these, please let me know. |
I'll halt here for now until I can get some feedback - I'd like to make sure my changes line up with your plans for this, rather than haphazardly changing while being unfamiliar with the codebase and style. You can view my progress here: https://github.com/varon/FsPickler/tree/dotnet35 |
So, the current nuget package bundles bother net45 and net40 builds. Given that a net35 build would introduce a redundant dependency for all the other, I would rather have it as a separate nuget package, i.e. FsPickler.Net35. The conditional flags could be renamed to something like Rergarding your other points:
|
The |
Thanks for the answers. With regards to |
No, I had a look at your branch, it has a few problems in that it has overwritten build configuration for the other target frameworks. Unfortunately, the Visual Studio UI tools do not support targeting multiple frameworks across different build configurations; it needs custom editing of the msbuild files. I created a branch of my own that does this; the net35 build can be accessed by choosing the RELEASE-NET35 build configuration. It seems to be compiling fine at the moment, more work needs to be done to make tests and the C# wrapper library build. |
I just created a PR that resolves this. Please review, let me know if this works with mono/unity before I can merge it. |
Thanks sincerely for the effort! I'll review it in Unity and report back. While I do so, are there any gotchas that one might encounter? Specifically I wondered if the binary serializer would work correctly between different .Net versions? It might be worth documenting this in addition to the missing support for bigInt and stack overflow protection. |
The net35 build works in Unity:
I wasn't able to build from VS, although I don't suppose it's that significant if there's an existing Nuget package for it. |
In general, I would not recommend using FsPickler for communicating between different versions of runtimes. This is because many of the types use field-based serialization, which is highly implementation sensitive. Also, the library has not been designed with version tolerance in mind. What was exactly the build error you were getting? What version of VS are you using? |
I'm using VS2013 Community. It seemed to build perfectly once I'd run the build scripts at least once, but on a fresh clone it seems to fail: Repro steps:
I receive the following errors:
|
Obviously this seems to be a NuGet error from within VS - Probably it isn't referencing the System.Threading package correctly. |
Please disregard my report of a build error. This seems to be standard behaviour for the F# Scaffold Project. From what I understand, one needs to use Paket first to resolve dependencies, and this isn't included in the build process from Visual Studio. It might be worth documenting this, because it was non-obvious to me why I didn't have a one-step build from the IDE. |
Fixed in 0128169. |
Thanks! |
The nuget package version 1.2.12 now incorporates net35 builds. However, this has not been extended to the FsPickler.CSharp package because of a few issues in the FSharp.Core nuget package. This why I'll keep this open for now. |
Closing, as the FsPickler.CSharp problem has been resolved. |
FsPickler has great potential for usage in Unity3D, as the serialization is fast enough to offer some performance benefits to games.
Unfortunately, it doesn't compile for .Net 3.5 at the moment, which is the version of the framework that Unity3D works with. (although it's actually some heavily modified mono version)
If there's any chance at supporting this version, I'm sure FsPickler would see some not-insignificant adoption among Unity3D users, especially if we could put out a Nuget package for it.
I'm currently doing a cursory look at what's going to be involved in porting this to 3.5, as I'd really like to use it.
I'll update this issue with my findings, but I'd appreciate input on this from the maintainers.
The text was updated successfully, but these errors were encountered: