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

Update to .NET 5 #10346

Merged
merged 17 commits into from Dec 8, 2020
Merged

Update to .NET 5 #10346

merged 17 commits into from Dec 8, 2020

Conversation

ghost
Copy link

@ghost ghost commented Dec 2, 2020

.NET 5 brings many performance (especially regex) improvements
https://devblogs.microsoft.com/dotnet/announcing-net-5-0/

Can look at bringing the packages upto date and using a single file executable in a future PR if the maintainers are interested

airhawk777 added 5 commits December 2, 2020 22:04
.NET 5 brings many performance (especially regex) improvements
https://devblogs.microsoft.com/dotnet/announcing-net-5-0/

Can look at bringing the packages upto date and using a single file executable in a future PR if the maintainers are interested
@ghost
Copy link
Author

ghost commented Dec 2, 2020

Run out of time for now, will try and have another look this weekend (unless some one else is keen!)
It's building and integration tests are passing on Windows, Mac and Linux. Integration tests are failing on Mono though. Will try and spin up a VM with Mono. If any of the maintainers have a Mono machine, if could you could try the Mono build that would be great in helping to see if its a build problem or a test problem

@ngosang
Copy link
Member

ngosang commented Dec 2, 2020

@garfield69 @ilike2burnthing I have a lot of work lately. I can only spend time on this project on the weekend. I will go over all the important assignments on Saturday or Sunday. If something can't wait that long, you don't have to wait for me.

@ghost
Copy link
Author

ghost commented Dec 3, 2020

Fixed the issue with Mono, really great work from the maintainers having those integration tests, pretty unusual in OSS

@ilike2burnthing
Copy link
Contributor

As far as I'm aware, all praise goes to @ngosang (at least of late) for those

Copy link
Contributor

@garfield69 garfield69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am extremely confused.

Ran my win10 VirtualBox and started ubuntu 18.04, downloaded the amd package from this PR, extracted it and ran the systemd installer
and the Jackett log shows:

2020-12-04 16:34:21 Info Using HTTP Client: HttpWebClient
2020-12-04 16:34:21 Info Using proxy: Disabled
2020-12-04 16:34:21 Info App config/log directory: /home/garfield69/.config/Jackett
2020-12-04 16:34:21 Info ThreadPool MaxThreads: 32767 workerThreads, 1000 completionPortThreads
2020-12-04 16:34:21 Info Running in Docker: No
2020-12-04 16:34:21 Info File /etc/issue: Ubuntu 18.04.5 LTS \n \l
2020-12-04 16:34:21 Info Jackett variant: Mono
2020-12-04 16:34:21 Info OS version: Unix 4.15.0.126 (64bit OS) (64bit process)
2020-12-04 16:34:21 Info Environment version: 5.0.0 (/home/garfield69/Downloads/Jackett.Binaries.LinuxAMDx64/Jackett/)
2020-12-04 16:34:18 Info Starting Jackett v0.16.2296.0
  • Was expecting Jackett variant CoreLinuxAmdx64 not Mono

downloaded the win package, installed and the Jackett log shows:

2020-12-04 16:47:02 Info Using HTTP Client: HttpWebClient
2020-12-04 16:47:02 Info Using proxy: Disabled
2020-12-04 16:47:02 Info App config/log directory: C:\ProgramData\Jackett
2020-12-04 16:47:02 Info ThreadPool MaxThreads: 682 workerThreads, 1000 completionPortThreads
2020-12-04 16:47:02 Info Running in Docker: No
2020-12-04 16:47:02 Info Jackett variant: FullFrameworkWindows
2020-12-04 16:47:02 Info OS version: Microsoft Windows NT 10.0.19042.0 (64bit OS)
2020-12-04 16:47:02 Info Environment version: 5.0.0 (C:\ProgramData\Jackett\)
2020-12-04 16:47:01 Info Starting Jackett v0.16.2296.0
  • Was expecting Jackett variant CoreWindows not FullFrameworkWindows

Ran win10 VirtualBox and started BigSur, downloaded the mac package, installed and the Jackett log shows:

2020-12-04 21:32:28 Info Using HTTP Client: HttpWebClient
2020-12-04 21:32:28 Info Using proxy: Disabled
2020-12-04 21:32:28 Info App config/log directory: /Users/garfield69/.config/Jackett
2020-12-04 21:32:28 Info ThreadPool MaxThreads: 32767 workerThreads, 1000 completionPortThreads
2020-12-04 21:32:28 Info Running in Docker: No
2020-12-04 21:32:28 Info Jackett variant: Mono
2020-12-04 21:32:28 Info OS version: Unix 11.0.0 (64bit OS) (64bit process)
2020-12-04 21:32:28 Info Environment version: 5.0.0 (/Users/garfield69/Downloads/Jackett/)
2020-12-04 21:31:58 Info Starting Jackett v0.16.2296.0
  • Was expecting Jackett variant CoreMacOs not Mono

@garfield69
Copy link
Contributor

looks like it is failing this conditional

if (DotNetCoreUtil.IsRunningOnDotNetCore)

@garfield69
Copy link
Contributor

so this

runningOnDotNetCore = RuntimeInformation.FrameworkDescription.IndexOf("core", StringComparison.OrdinalIgnoreCase) >= 0;

no longer contains core when .net 5 is active
https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.runtimeinformation.frameworkdescription?view=net-5.0

@ghost
Copy link
Author

ghost commented Dec 4, 2020

Good pickup @garfield69 , sorry my bad
Rather than use FrameworkDescription I went with FrameworkName https://weblog.west-wind.com/posts/2018/Apr/12/Getting-the-NET-Core-Runtime-Version-in-a-Running-Application since it looks like FrameworkDescription is only supported in Full Framework from 4.7.1 and its always hard to know what that maps to in Mono. Whereas for every version of Jackett except Mono, the runtime is bundled so feels safer to look for that
Tested Windows and Mono and it now looks good to me

Also, I'd suggest bumping the minorVersion to 17 for this, but will leave that to you to decide

@ngosang
Copy link
Member

ngosang commented Dec 6, 2020

@airhawk777 Thank you! This is an important change and I want to be sure everything is right. It can take some time.

  • Test Windows installer (.exe) in XP: Version in log traces + search + update WinXP is unsupported
  • Test Windows ZIP in XP: Version in log traces + search + update WinXP is unsupported
  • Test Windows installer (.exe) in Win7sp1: Version in log traces + search + update
  • Test Windows ZIP in Win7sp1: Version in log traces + search + update
  • Test Windows installer (.exe) in Win10: Version in log traces + search + update
  • Test Windows ZIP in Win10: Version in log traces + search + update
  • Test MacOS: Version in log traces + search + update
  • Test Mono: Version in log traces + search + update
  • Test Linux x64: Version in log traces + search + update
  • Test Linux ARM64: Version in log traces + search + update
  • Test Linux ARM32: Version in log traces + search + update
  • Test Docker x64: Version in log traces + search + update
  • Test Docker ARM64: Version in log traces + search + update
  • Test Docker ARM32: Version in log traces + search + update
  • Review Azure Pipeline to be sure we are using the right compilers, etc...
  • Review and fix compile Warnings. There are more than 100 => https://dev.azure.com/Jackett/Jackett/_build/results?buildId=2328&view=results

@garfield69 @ilike2burnthing I will need help with Windows, MacOS tests
@airhawk777 Could you take a look at the warnings? I'm the only C# developer and I don't have too much experience.

@ngosang
Copy link
Member

ngosang commented Dec 6, 2020

Compile warnings are not blocking this issue. They can be fixed later.
My main concern now is the memory usage (on Linux at least). Results after 1 search with 300 trackers enabled.

  • Linux x64 .netcore3.1 => total = 264 MB shared =69 MB
  • Linux x64 .netcore5.0 => total = 366 MB shared = 71 MB

@airhawk777 do you have some info to share? can it be related to the new dependencies included in this pr? memory usage in windows is similar?

@ilike2burnthing
Copy link
Contributor

                            WS   Pe   Pr   Sh  Co   PP
Win10 x64 .netcore3.1 .exe  195  248  154  40  159  270
Win10 x64 .netcore5.0 .exe  183  201  140  43  145  340
Win10 x64 .netcore3.1 .zip  184  234  144  40  151  323
Win10 x64 .netcore5.0 .zip  187  206  142  43  149  355
Legend

WS- Working Set
Pe - Peak Working Set
Pr - Private Working Set
Sh - Shared Working Set
Co - Commit
PP - Paged Pool

221 indexers, of which 31 failed. 21 failed when using proxy/VPN, with little to no difference in memory. All are expected to fail (i.e. this isn't causing indexers to fail which weren't already).

Multiple runs (restarting the Jackett in between) had some variance, figures above show a rough average.

Log traces:

Jackett variant: CoreWindows
Environment version: 5.0.0

Manual updates from 3.1>5.0, 5.0>3.1, and 5.0>5.0 all worked fine without error.

@ngosang
Copy link
Member

ngosang commented Dec 7, 2020

I did all test in Linux / Mono / Docker and everything is fine.
I was measuring the memory wrong. After using another tool I get similar results to @ilike2burnthing Memory is almost the same.
jackett_171162
@garfield69 Could you perform some tests in MacOS and WindowsXP ? If test pass you can merge.

@garfield69
Copy link
Contributor

tested OK on my win10 VirtualBox macOS Big Sur image.
Don't have an winXP image ready to use, I will dig into my archives an see if I still have a XP installer.

@ilike2burnthing
Copy link
Contributor

If needed, you can extract the VHD from the XP Mode installer - https://www.microsoft.com/en-us/download/details.aspx?id=8002

Got it up and running using Hyper-V, but couldn't get internet access for whatever reason.

@garfield69
Copy link
Contributor

found my winXP sp4 installer cd, installing into VirtualBox image as I type this. Should be done in the next couple hours ;-)

@ghost
Copy link
Author

ghost commented Dec 8, 2020

Will have a look at the compile warnings when I get some spare time. Looks like a new warning with .net 5 so will follow this issue dotnet/sdk#14502 it shouldn’t cause any issues since it’s been doing this all along

Doubt jackett even with current release will work on xp since .NET core requires win7. Jackett read me also specifies a minimum of win7 as well

@garfield69
Copy link
Contributor

WinXP is unsupported as pointed out by airhawk777
Set up Win7sp1 and tested PR both with installer package and zip package. All good.

Will bump the minorversion to 17 on this PR and then publish as a pre-release

@garfield69 garfield69 merged commit 65ca4d7 into Jackett:master Dec 8, 2020
@garfield69
Copy link
Contributor

testing check-for-updates upgrading from 0.16.2374 to 0.17.3 pre-release on win10, win7sp1, ubuntu18,04 and BigSur
All good. ;-)

@ngosang
Copy link
Member

ngosang commented Dec 8, 2020

Nice job! Thank you very much to all.

@ghost
Copy link

ghost commented Mar 3, 2021

@airhawk777

Sorry if this is the wrong place to ask, but I'm getting errors that I'm unfamiliar with. Everything was fine until I updated my Visual Studio 2019 to version 16.9. I am now getting these errors:

NETSDK1005 Assets file 'C:\Users\...\source\repos\Jackett\src\Jackett.Server\obj\project.assets.json' doesn't have a target for 'net5.0'. Ensure that restore has run and that you have included 'net5.0' in the TargetFrameworks for your project.

NU1201 Project Jackett.Server is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Project Jackett.Server does not support any target frameworks.

dotnet restore only restored 4 of 8 projects. I tried removing my cached NuGet packages, bin and obj folders...

I eventually found the fix, which is to add <RuntimeIdentifier>win-x64</RuntimeIdentifier> after the <TargetFrameworks> in Jackett.Server.csproj.

Can someone tell me why I needed to do this now and not before the VS update? Am I doing something wrong or have they changed the way you're supposed to add frameworks? Is this something we should add to this change or am I the only one experiencing this error? And if we were to add this to master, would this have an effect on other platforms (because it's targeting Windows, which I'm developing on)?

@garfield69
Copy link
Contributor

garfield69 commented Mar 4, 2021

@XYZJR I don't know much about your particular issue, but whenever my VS upgrades and I get a problem I just access the tools->nuGet packet manager->packet manager console, and when the prompt is ready and type dotnet restore
then do a build->clean solution followed by a build-> build solution and I'm good till next time.
I've not seeing your exact issue so far on my win VS 2019 16.9.0
In the past when it really got stuffed up, I deleted my local repo, and followed https://github.com/Jackett/Jackett#windows to reset and get going again.

@ghost
Copy link

ghost commented Mar 5, 2021

@garfield69 I re-installed .NET 5 SDK and Visual Studio, then re-cloned the project. dotnet restore, rebuild, the whole thing... It's still giving those same errors.

@garfield69
Copy link
Contributor

@XYZJR that's unfortunate.
I'm not a C# dev, and I only use VS occasionally for testing simple Jackett C# PRs when there are no proper C# devs available.
So that is all the experience I have with the product, sorry.

@ilike2burnthing
Copy link
Contributor

@XYZJR here's a fix from @JigSawFr:

After cloning your fork, execute these commands:

- dotnet msbuild /restore
- dotnet restore
- dotnet build

And it will work as expected otherwise server project building will fail. :)

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.

None yet

3 participants