Skip to content
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

dotnet version proposal discussion #480

Open
monkeyman192 opened this issue Jul 21, 2022 · 11 comments
Open

dotnet version proposal discussion #480

monkeyman192 opened this issue Jul 21, 2022 · 11 comments
Labels
discussion Informational discussion.

Comments

@monkeyman192
Copy link
Owner

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.

@monkeyman192 monkeyman192 added the discussion Informational discussion. label Jul 21, 2022
@cmkushnir
Copy link
Collaborator

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
Copy link

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
Copy link

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

@monkeyman192
Copy link
Owner Author

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
Copy link
Collaborator

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
Copy link
Collaborator

cmkushnir commented Jul 22, 2022

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
Copy link

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
Copy link
Owner Author

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
Copy link
Collaborator

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
Copy link
Owner Author

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

@Kobi-Blade
Copy link

Kobi-Blade commented Feb 11, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Informational discussion.
Projects
None yet
Development

No branches or pull requests

5 participants