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

Net 5 migration #3124

Merged
merged 105 commits into from
Apr 21, 2024
Merged

Net 5 migration #3124

merged 105 commits into from
Apr 21, 2024

Conversation

mnaiman
Copy link
Contributor

@mnaiman mnaiman commented Mar 24, 2018

First step in migration to .NET Core as in #2864

@mnaiman mnaiman mentioned this pull request Mar 24, 2018
@mnaiman
Copy link
Contributor Author

mnaiman commented Mar 25, 2018

@kenkendk
Copy link
Member

I see a few changes:

  1. Update all project files
    All good. Seems to trigger nuget issues on Travis though.

  2. Change the aFTP library
    Why the change? I am worried that stuff breaks, because FluentFTP does not have the same quirks as aFTP.

  3. Change some stuff to async
    Why was these changed? Since we do not have a real async interface upwards, it does not seem to help.

  4. Some stuff not working
    The LZMA2 module really needs to be reworked, so that part is fine.
    The LogicalCallContext that seems to not work is problematic because the log module depends on that feature.

@mnaiman
Copy link
Contributor Author

mnaiman commented Mar 28, 2018

Hi @kenkendk thanks for tips

  1. I saw, dont know on first look what is happening

  2. aFTP is superseded by FluentFTP (follow that link https://github.com/duplicati/duplicati/blob/master/thirdparty/System.Net.FtpClient/Homepage.txt)

  3. It is enforced change, because some libraries in new versions (NETSTANDARD20) are missing nonAsync methods. To not spread async stuff in code (which should be task for next refactor) Async methods are instantly "deAsynced".

  4. LogicalCallContext this is something I will need help with to not break critical parts, but first I need to progress further to see if there are not bigger obstacles.

  5. Pull request pending convert to netstandard kenkendk/sharpaescrypt#9

@warwickmm
Copy link
Member

Seems to trigger nuget issues on Travis though.

It looks like AppVeyor is having some issues as well. The reported "build success" is a bit misleading (this is fixed in the upstream master branch).

Bruceforce added a commit to Bruceforce/duplicati that referenced this pull request Apr 5, 2018
@kenkendk
Copy link
Member

kenkendk commented Apr 13, 2018

  1. Ok, all good on that one.

  2. Ok, I will merge in concurrent_processing soon, which makes the entire backup process use async.

  3. The LogicalCallContext has been removed, but the functionality we need can be achieved with System.Threading.AsyncLocal<string>. Unfortunately, that part is not in .netstandard but only in .netcore, and I would really like the logging library to be .netstandard, as everything else depends on in so we would have to make everything .netcoreapp2.0.

Actually, .NetStandard 1.6+ supports AsyncLocal so we are good on that. I will look into upgrading the logging code to use that.

I need the feature in some other projects as well, so maybe I will figure out a workaround.

  1. Yes, that was significantly more difficult than I expected, but the sharpaescrypt library is now updated to use .netstandard2.0.

Overall, phew... this will take a while to update, but it will be nice once everything runs on .netcore.

@mnaiman
Copy link
Contributor Author

mnaiman commented Jun 24, 2018

Added more libraries and more TODO-DNC for parts that dont know how to solve.

Running discussion on AlphaVSS how to migrate, because it is crucial library.

@mnaiman
Copy link
Contributor Author

mnaiman commented Jun 24, 2018

Localization library causing build to fail tests.

@warwickmm
Copy link
Member

warwickmm commented Jun 14, 2021

I can't get the net4.8 executables to run on windows. Could be something wonky with my system and framework dll signing. The only reason we want 4.8 anyway is for legacy systems so I think we should not support 4.8 on windows (and mac).

Only support a tgz of net4.8 for linux. No RPM's, no Debs.

Then RPMS, debs, docker containers, etc would only be for self contained Net Core.

On the experimental/net5-split branch, I can't get the net4.8 executables to run on Linux either. With this patch, the net5.0 executables work for me. However, withh the net4.8 executables I keep running into an issue with SQLite:

System.DllNotFoundException: SQLite.Interop.dll assembly:<unknown assembly> type:<unknown type> member:(null)

There are a number of "solutions" for this on the internet, but none of them have worked for me. I suspect that the Duplicati.Library.SQLiteHelper targeting netstandard2.0 while the Executables/net48/Duplicati.GUI.TrayIcon targets net48 could be a problem, but I'm not certain.

@warwickmm
Copy link
Member

I updated the experiment/net5-split branch with some more fixes and a workflow to run the unit tests. It's still not clear to me if we want to go the netstandard or experiment/net5-split route.

@tsuckow
Copy link
Contributor

tsuckow commented Jun 29, 2021

It just dawned on me that .net framework has no concept of Linux/Macos native libraries. So most likely instead of including the appropriate dll, it's just pulling in the windows one. If we can get the freebsd interop working with dotnet5 then that is likely more reason to abandon framework.

@warwickmm warwickmm marked this pull request as draft August 11, 2021 17:42
@duplicatibot
Copy link

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/add-secure-and-httponly-attributes-to-cookies/13924/10

@npodbielski
Copy link
Contributor

Oh my god!
It is still on .net framework v4?!

you guys want some help with the migration? You do realize that we are almost there with .net 8? 😄

@tsuckow
Copy link
Contributor

tsuckow commented Aug 10, 2023

All the help we can get. I've been swamped in honey do's and haven't had a chance to keep working on my branch the last few months. There was a desire to backport some of the changes but I've never actually been able to build the v4 branch. I gave up on that and have just been continuing down the branch. If I remember correctly it mostly works with caveats (encryption), the hardest for me to diagnose is macos support, there seems to be a quirk in my build if the .app

@npodbielski
Copy link
Contributor

Ok how eager you guys were to actually do it? Because I do not want to spend few months of my spare time if this won't gonna be merged because project lead does not want too I.e. 😄
Also: can we start with something smaller like migrating one project? I would port it to .net standard 2.0 which is compatible with 4.x and 7.x. Then phase out as much of .net 4.x and move to .net 7.x then phase out .net standard and move everything to .net 8 which gonna be next LTS if I remember correctly.

@tsuckow
Copy link
Contributor

tsuckow commented Aug 10, 2023

If you can manage to pull that off, more power to you. Incremental changes are very likely to be merged.

I personally struggled to get .net standard to work with .net4 because of the structure of the app and the way the build system works with embedding native code. But I'm not a .net dev by trade and have trouble building the unmodified app so maybe for you it is easy.

@npodbielski
Copy link
Contributor

So do you have some idea where I should start with this? Which project would be simple to migrate? Or relatively simple?

@Jojo-1000
Copy link
Contributor

@npodbielski

So do you have some idea where I should start with this? Which project would be simple to migrate? Or relatively simple?

There are the experiment/net5-split, experiment/net5-split-cancellationtoken branches, which I was able to build. I also have some more changes (with current master merged back and all backends converted to cancellation tokens) in my forked branch.

The solutions are converted to target .net standard 2.0, with executables for both .NET 5 and .Net Framework 4.8. It is building, but some features were commented out (look at the project for what was done a few years ago). I don't have write access to update anything.

You would need to ask @gpatel-fr about the timeframe of merging if/once it was finished. Pull requests are pretty backed up right now.

@npodbielski
Copy link
Contributor

@Jojo-1000

Pull requests are pretty backed up right now.
What does it mean? I am not familiar with this phrase? You are merging PRs or not?
Also I was thinking as doing this in much more smaller PRs. Moving everything to .NET 6 or 8 (both LTS, or 8 will be after release) is quite big task - I would not attempt to do it all at once. Something would break for sure.

Right now I looked at Logging and Localizations projects. Both have no dependencies (in solution) so those would be good candidates for moving to .net standard 2.0. The only think problematic there is CallContext that could be rewritten using AsyncLocal<T>.

@npodbielski
Copy link
Contributor

Ok I noticed that you guys already did that: moved those two projects.
So when we could merge anything? @gpatel-fr ?
I see that last commit to master was like 2 months ago.

@npodbielski
Copy link
Contributor

I created #5006
If anyone is interested.

@veleek
Copy link

veleek commented Aug 16, 2023

@npodbielski - One other big thing that I ran into when I looked into this (literally years ago) was that Thread.Abort is no longer supported, and there are a few different places that were using it (for actual flow control, not just error cases). I don't remember what state that got left in, but I feel like I remember making significant progress on it (and look at that, I actually left some reasonable comments on the work!). It could be worth browsing through the branches in my fork and commits to see if there's anything you want to salvage. The changes for Thread.Abort were not trivial.

Another thing was swapping HttpWebRequest for HttpClient everywhere I could. I don't remember if this was just something I thought could be done in place for .NET 4, or something that was required to move stuff to .NET 5. Again, there's a branch that contains those changes too.

@npodbielski
Copy link
Contributor

Yes, that is most worrisome code to me. I mean this kind of webserver... I do not honestly remember if this was good even in 2013-15 which is mostly when this code were written from gitblame. I do not want to modernize it... I would rewrite this to - I would write new, but asp.net core pipeline and OWIN is nothing new - to asp.net core webserver.
I do not think that maintaining this code is viable. How hard it would be? I do not know. I am pretty sure I can do it, but as I already wrote here I do not want to spend few weeks working on something that won't be merged - because this happens with open source projects... your work guys is example of that. You spend, from the looks of it, quite big amount of time trying to do it but it was not merged.

But right now none of this matters because test do not work. I think we need to fix it first, because I do not want to break anything and tests are the only thing that I can rely on doing such rewrite.

@veleek
Copy link

veleek commented Aug 17, 2023

1406083864652713648

@kenkendk
Copy link
Member

Yes, that is most worrisome code to me. I mean this kind of webserver... I do not honestly remember if this was good even in 2013-15 which is mostly when this code were written from gitblame. I do not want to modernize it... I would rewrite this to - I would write new, but asp.net core pipeline and OWIN is nothing new - to asp.net core webserver.

It was not good back then, but it was a working cross-platform HTTP server, which I could not find anywhere else. It is even older, because I forked it after it got abandoned by the original author. Today with the managed Kestrel server, I do not see any reason not to use that one, as it works so much better and will be more familiar to developers.

@tsuckow
Copy link
Contributor

tsuckow commented Jan 20, 2024

And kestrel was working fine in the port last I checked, any issues are undoubtedly me just forgetting to implement something. I've been swamped with other honeydo projects so haven't been able to get back to working on it for the last several months.

@duplicatibot
Copy link

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/battle-plan-for-migrating-to-net8/17482/1

@kenkendk
Copy link
Member

I realize this PR is both extremely needed and also embarrassingly neglected, but I honestly intend to finally get it done.

Any input to my plan is appreciated: https://forum.duplicati.com/t/battle-plan-for-migrating-to-net8/17482

kenkendk added a commit that referenced this pull request Mar 3, 2024
kenkendk added a commit that referenced this pull request Mar 3, 2024
@kenkendk kenkendk merged commit 4f565a0 into duplicati:master Apr 21, 2024
@duplicatibot
Copy link

This pull request has been mentioned on Duplicati. There might be relevant details there:

https://forum.duplicati.com/t/migrating-from-net-4-to-net-8-in-300-commits/17975/1

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

Successfully merging this pull request may close these issues.