search

monkeyman192 / MBINCompiler

A tool for decompiling No Man's Sky .MBIN files to XML format
https://monkeyman192.github.io/MBINCompiler
Other
251 stars 49 forks source link

dotnet version proposal discussion #480

Open monkeyman192 opened 2 years ago

monkeyman192 commented 2 years ago

As raised in #478 it would be good to be able to support dotnet 6. This seems to be a LTS so it would be good to support it as MS should support it for a while. Adding support for 6 will be easy via the CI, however the discussion to be had is around the naming of the released artifacts and what we release.

There are a number of options:

  1. Release a libmbin.dll and MBINCompiler.exe which is built using .net 5 (version up for discussion), and then suffix all other artifacts with -netX (eg. libmbin-net4.dll) based on the dotnet version used to build them.
  2. Release all artifacts with the specific version.
  3. Release all dll artifacts with a specific version and leave the MBINCompiler.exe file as-is and built with some pre-determined version.

Irrespective of the above decision, the choice will need to be clearly documented so that anyone downloading it will know what version they are using even if there is no suffix.

cmkushnir commented 2 years ago

I would vote for 1. I'd suggest building both libmbin.dll and mbincompiler.exe to target .NET 6. I probably wouldn't create a libmbin-net5.dll unless it was proven necessary - upgrading a project from 5 to 6 (or 7, 8, ...) should be trivial as they are all .NET core, .NET 4 is the outlier being framework not core.
Even if a .NET 5 project is abandoned users should be able to add a runtime.json or use a command-line pararm to have it use 6 if needed: Control roll-forward behavior. I haven't tested this, but it might (should?) allow a .NET 5 app to use a .NET 6 libmbin.dll.

gregkwaste commented 2 years ago

I think 1 is the way to go as well. And indeed as @cmkushnir mentions, migrating from NET 5 to 6 should be trivial, so I think dropping net 5 completely and working with NET 6 should be fine. And I believe that no more than 2 versions are needed. One for the legacy NET frameworks (up to 4.8) and one for NET 6.

Khaoz-Topsy commented 2 years ago

For what it is worth, I agree with cmkushnir and gregkwaste here 😋

monkeyman192 commented 2 years ago

I also think 1 is the way to go, but just figured I'd present the different options in case anyone else had any good reason for using them. If we go with 1. then we'll need to decide on what version is the default. We currently use net 5 which seems to be slightly troublesome, however changing to net 6 may cause incompatibilities, so I'm not exactly sure which way to go. It may be that we keep 5 as the default for now, and then as app devs figure out whether that roll-forward stuff works or if they migrate to net 6 then we can make it the default and label 5.

I guess we'd need to test how that roll-forward behavior works and then add documentation for app developers on how to support it.

cmkushnir commented 2 years ago

For what it's worth the nmsmb outputs (dll and exe) both target .NET 6 and are able to load libmbin.dll .NET 5 as both a dep ref and explicitly w/o issue. What I'm not sure of is if libmbin.dll ends up using the MS .NET 5 dll's (since that's what it targets) or the MS .NET 6 dll's (since that's what the app targets), my guess is .NET 6.

cmkushnir commented 2 years ago

I would still vote to upgrade both libmbin.dll and mbincompiler.exe to .NET 6. If you are concerned w/ issues then add as new build assets and give them the -net6 tag. I'll rename libmbin-net6.dll to libmbin.dll and use w/ nmsmb as I don't expect issues. If there are no issues after some period of time then you can just change the normal ones to .NET 6 and drop .NET 5.

Kobi-Blade commented 2 years ago

I also vote with upgrade for .NET 6.0, Microsoft has a list of breaking changes that might help predict issues and fix them before hand, https://docs.microsoft.com/en-us/dotnet/core/compatibility/6.0

monkeyman192 commented 1 year ago

As of #528 we new will have a .net 6 libmbin binary as part of the release. For now the "main" version will still be .net 5 (ie. MBINCompiler.exe is still .net 5) Would be good to get some feedback here regarding whether we should remove the .net 5 builds (both libmbin and mbincompiler) and make the .net 6 version the primary one. Not sure how much of an issue doing so would cause...

cmkushnir commented 1 year ago

If no tools need to be .NET 5 then might be easier to just drop 5 support and call 6 libmbin and mbincompiler i.e. drop version from name.

monkeyman192 commented 1 year ago

My worry is that some tools might be using the .net 5 version...

Kobi-Blade commented 8 months ago

Support for .NET 6 will end on November 12, 2024, but upgrading to .NET 8 should be relatively easy, as there have not been major changes to the framework.

However, we might benefit from some optimizations that are available in .NET 8.

As for .NET 5 and third-party tools, I think it is risky to rely on outdated frameworks, but I also acknowledge the challenge of dealing with tools that are no longer supported or maintained.