-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use .NET core builds for Radarr #4464
Conversation
Just tested upgrading from previous Radarr to new, NETcore one using aarch64 spk from this test run build https://github.com/SynoCommunity/spksrc/actions/runs/611623265 and it work pefectly fine. Health status reports no issues, finally. @azz22 @houndtt @weedaj @JeremieBergeron @publicarray maybe You are willing to test? |
Hi, |
So I tested this in my VM which was an upgrade from the existing version. It's mostly a clean-install so not much configurations present. While the upgrade completed and says it was running, I could not connect. I then manually stopped and started the app and it seemed to work well. I attempted to restart the app from within the interface but the app only stopped but failed to start again. Manually starting it in the Package Center was successful. Based on these anomalies and the many errors in the log I don't think I'll be using this in my production NAS even though it gives a good health status. Some logs below: My setup:
|
@houndtt something didn’t work right. Do you have the installer log? You grabbed the test files from the GitHub action for this PR right? Based on your logs it is still launching mono and the wrong, out of date version, [v3.0.1.4259] and spewing kestrel errors everywhere where. So I don’t think those provided logs correlate at all to this build. This test build is for 3.0.2. |
This is a known bug in 3.0.2 - the restart button in Radarr's UI is broken under net core. Is fixed in 3.1. |
Here's the installer log: https://pastebin.com/1gNn0iMi and last app run log: https://pastebin.com/3HQJWbnV
The build is 3.0.2 as shown in the screen shot above. I took a log snippet from just after the upgrade. Here's the latest section: https://pastebin.com/nQ9CeWWc Looking at the tail of the log, it seems to have started running the correct version. I don't know if this was after the reboot I did or not but it seems to be no longer throwing out strange log entries. Not sure why the upgrade created them in the first place though. |
@houndtt based on those logs; Kestrel errors are only on 3.0.1 and not in 3.0.2; 3.0.2 appears fine? |
@ta264 generally we compile the libs per architecture rather than download them. This avoids having different digests per architecture and ensures they are compatible. But considering that it is hard to compile |
I put the status to |
Yes, but the problem I am asserting is that the upgrade appears to have caused an in-between state which resulted in the pages and pages of Kestrel errors. From the timeline in the logs I see the following:
Based on the above sequence, it appears that the upgrade completed via the Package Center caused the app to go into this in-between state which resulted in the loads of Kestrel errors. During this time I suspect is when I could not connect to the application even though it showed it was running in the Package Center. After this, I believe I manually stopped it in the Package Center and started it again. This is where I believe the logs stabilised. Following this is where I believe I requested a restart from within the app which was not successful based on the bug reported above by @ta264. I will try to re-create this upgrade scenario later today in the VM to see if I can re-create the above interpretation of events. |
I agree it's a bit of a hack but I think it's probably better than nothing. The mono builds in Radarr will be switched off in the not too distant future (in case you haven't realised, I'm a Radarr dev) - we're planning to release 3.1 but then I think the next release will be 4.0 which won't support mono. We'd love to not leave the non-docker-compatible synology users behind, but we're not willing to maintain mono builds to make that happen I'm afraid. Happy to try to assist to make the hack more palatable. I guess the other option is to wait for DSM7, and any users that can't / won't upgrade will be stuck on a legacy Radarr build. |
@houndtt In the light of morning, I'm guessing that somehow the change to net core meant that the service program didn't manage to shut down the old Radarr version before beginning the upgrade. I can take a look into this in more detail. @publicarray If you have any hints as to how the existing service gets stopped that would be handy. |
Thanks @ta264 for working on this. I appreciate your help.
Thanks, the framework can compile dotnet apps. I suggest you look at #4207 and use |
Thanks, but we don't support third party compilations of the Radarr binaries. If you want to package Radarr and have users able to access our upsteam support you'll need to use the downloaded Radarr binaries. These are what will be grabbed on the first upgrade via the built in mechanism anyway. You're already downloading the mono binaries so this isn't a new exception. Downloading the libstdc++ is a hack (I don't disagree on that at all!) but compiling gcc is a tricky thing! |
@houndtt Can I confirm what version of the synology package you were on before? I guess you were using the updated v3 package since I think you did the work for that, but would be good to confirm. One case we do perhaps need to guard against is a user going from v0.2 to v3, where the name of the PID file changes, but I'm guessing that isn't the case for you. I'm a little surprised I don't see something corresponding to
In the update logs, but this is the first time I've looked at synology packages in any detail. If there is a problem stopping radarr before the upgrade I think I'm going to need some assistance from the experts to fix it. |
@ta264 Sorry but I disagree on your
I see where you're coming from but .NET is different, the framework is embedded into the binary. Previously we compiled mono per arch and the mono binaries worked across arches. |
I'm afraid we'll have to agree to disagree. Compiling the backend is only one aspect - there's also the frontend which requires yarn. I would council that this is a futile effort, because the first upgrade from the built in updater will remove your compiled binaries and replace them with ones from our CI, which are the full standalone build. We have actively decided against shipping framework dependent builds precisely so that we are in control of which framework Radarr is running on, which will avoid many of the issues we faced with mono. We have no plans to ever produce framework independent builds. You may be providing some support but I can assure you a lot of Synology package users come by our discord asking for help. To be blunt, a synocommunity compiled Radarr will be on you to support. |
That's a good point, If you require However, personally I still prefer if the package was build. Regarding yarn, I've made a Jellyfin package that uses nodejs and npm: #3932 It's not merged yet but that should address the front-end issue. |
You'd need to add yarn addionally, nodejs and npm isn't sufficient for Radarr / Sonarr.
Of course that choice is up to you. I've suggested a way that we as Radarr devs would be happy to help support. If you wish to go in a different direction then I wish you well and I'll let you get on with it. |
Thanks @ta264, Sorry to be clear it's not only up to me. The other @SynoCommunity/developers will write what they think first. |
Here is a wild proposal: Maybe we do what AUR does and postfix binary package as Edit: The package must be auto updating. I think this solves some issues where package maintainers which to be in full control. This does come at a risk if an update turns out to be malicious. But this doesn't change how current auto updatable packages work anyway. By adding the postfix this should be a clearer signal to users which packages auto update. |
I think that would be ideal from our point of view. If I understand it you're proposing relabelling the existing As an aside, my view is that it's inconsistent to require that spks in this repository are compiled from source but to allow self-updating packages. If you want to insist on compiling from source, you should also disable the built in updaters (which are downloading binaries). But I'm not sure that you want to commit to having to update your packages for every release of Radarr? A big plus side from our POV of having a self-updating package is we have reasonable number of users on the nightly branch (i.e. an update for every commit to the upstream Radarr repository). This means that issues are normally picked up pretty quickly and fixed. If you disable self-update for Radarr, you are locking synology users into only using the builds we think are stable, but we won't have had chance to see any synology specific issues, so I think you dramatically increase the chance of us pushing out a "stable" build that doesn't work properly on synology, with no easy way of testing fixes. As mentioned, if you are willing to allow a self-updating package, I don't think insisting that spks are built from source is a principle worth falling out over. We're pretty reasonable guys, we'd love to find a solution to keep the optimal experience for Synology users. My views in this thread are born out of the practicalities of contributing to a complex app like radarr rather than just a desire to be in control or any particular principle. I believe self-updating packages are a win for your users, and sticking to the binaries we provides makes it much easier for us to identify where issues are arising. |
Personally I think this is a reasonable approach. I guess a Note that I am a big fan of always rebuilding from scratch. But in this case, creating a decent
This is where the
I believe that having inter-project collaborations is more than welcomed :) |
@ta264, you are correct. I was already using a v3 package prior to executing the update. Unfortunately, when I tried to replicate the issue two different ways this morning I was unable to do so. In my original setup I was running the last build I did which was merged running v3.0.1.4259 (https://github.com/SynoCommunity/spksrc/actions/runs/420620824). When I did a clean install with this same version and then an upgrade from the Package Center the logs look nice and clean: https://pastebin.com/kFWXAAZ6 Furthermore, if I install from the current SynoCommunity published version which was the previous one I built running v3.0.0.4204 (https://github.com/SynoCommunity/spksrc/actions/runs/411699059) this too was clean: https://pastebin.com/8BLEa3Bc I cannot explain why the original upgrade I did failed in the way it did since I can't replicate it on the same setup unfortunately. |
Yes, if you want to compile from source and support Radarr past 3.1 (once we drop mono) you'll need to have a My only concern with renaming |
Thanks a lot for testing! If you do manage to reproduce anything let me know. |
@ta264 thanks I should have caught that error too. I just want to point out that SynoCommunity is more than a distribution platform. Since you have a good technical reason (tie a release to a specific .NET) I think the package is Ok as is. My reason for updating the support links is because we can no longer "control" the code that is run. IMHO not providing support because it's not compiled through your CI is against what I thought OpenSource stands for. On the other hand I understand the difficulty with providing support when the dependencies/.NET don't match. But I think we could have kept the same versions in sync (maybe deviate a little from .NET patch version). To be clear if support requests land here we can still provide support, but we I don't think we can be the main support channel. (not sure if we ever where if support is mostly on discord now 🤷)
There was talk to do it but it requires a complex toolchain setup. I think @hgy59 was working on doing it. If we do get it the aim is to add x86 32bit support too. |
Thanks! |
The package is published but will take around 48 hours for our CDN to distribute it to the package center. |
Okay, the published version appeared on my NAS and I was able to upgrade successfully. The upgrade process took quite some time to complete but seemed to do so without major error. Below are the logs: radarr_install.log
radarr.txt
Looking at the above it's strange that it is trying to access the
If this is a bug then a new issue would need to be opened. Anyone else experiencing this? |
Limited time/resources usually. From my (limited) experience here there are on average 0~3 people with the ability to publish packages for a given week. There over a hundred of packages here and Synology is not a typical Linux environment. Testing is done manually per package. And I do get it wrong a lot because we have very limited architectures/units to test on. Suggestions to improve this are welcome.
I don't disagree, but the reality is mixed. To begin with the package was maintained by synocommunity, I think it's the first time a radarr developer submitted a PR for radarr: https://github.com/SynoCommunity/spksrc/commits/955a8537929aa4151353cca1355ea40d40cbb316/spk/radarr/Makefile
I emphasise and It saddens me too. I don't have any python knowledge especially how it works with Synology. Otherwise, I would have helped review your package. Also, I've tried submission a package to Synology directly. Trust me when I say they have more restrictions than we do. (e.g. all packages must me self-contained, installable offline, updates through their package center only, no syntax errors (that would still work fine anyways), no extra info variables that don't do anything anymore on the latest DSM version, but we use for backwards compatibility reasons, and with no errors in any logfiles (not even mkdir file exists errors which I want to remove here as well since it makes debugging harder and logs more unreadable). Actually I don't mind these restrictions but their error messages where't very clear. |
|
||
DEPENDS = cross/busybox cross/curl cross/mediainfo cross/sqlite cross/radarr | ||
# .NET is not supported on PPC, ARM5 or x86 | ||
UNSUPPORTED_ARCHS = $(PPC_ARCHS) $(ARM5_ARCHES) $(x86_ARCHES) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@publicarray Why not use compilation with Mono implementation for these architectures? (the same way as for Jackett if I remember well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we can add it for one more version:
The mono builds in Radarr will be switched off in the not too distant future (in case you haven't realised, I'm a Radarr dev) - we're planning to release 3.1 but then I think the next release will be 4.0 which won't support mono. We'd love to not leave the non-docker-compatible synology users behind, but we're not willing to maintain mono builds to make that happen I'm afraid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually there shouldn't be any need since the package auto updates until it won't be supported anymore.
As this continues to occur for me, I opened a new bug report here: Radarr/Radarr#6068 |
@publicarray not sure of the best place to mention this, apologies if this is not the best place. Just run through debugging with a user on a 213j where it doesn't work at all - it fails with
I think we might need to mark armv7l as unsupported too. Does this sound plausible or do you think there's something else going on? |
Yes you are correct, sorry must have missed it in my review. I'll disable the offending architecture. Edit: hi3535 was already disabled. So it wont show up in the package center but is still visible on our website. |
Thanks. I think the 213j is an Armada370 which is listed as ARMv7 here: Line 9 in 2f2428c
But the user reported this as the output of
Is it misfiled as arm7 and should be armv7l? Or am I reading too much into the uname output? |
You would want to check something more like Maybe this doesnt help you, but I just had 2 days of tracking down an issue like this. |
Going by deviwiki it's ARMv7 https://deviwiki.com/wiki/List_of_NAS_Devices Im not 100% sure but it would be unusual to leave of the endianness |
This link suggests its as much about how stuff is compiled as the SoC https://stackoverflow.com/questions/29166619/differences-between-arm-versions-armv7-only |
Oh I learned something new today. |
I think that might have been a red herring. Awaiting confirmation from the user, but it my guess is that the armada 370 cpu is armv7 but with an FPU that doesn't support net core. The requirements are listed here. Looking at a core dump in gdb, it's falling over trying to access registers 16-31, so I guess this cpu will have the |
Ok mystery solved I think. This is the output of cpuinfo on a ds213j:
CPUs with vfpv3d16 are not supported by the MS published SDK. It's possible to compile a version that would support it I think but that may be a little too involved... For now I think we need to mark armada370 as not supported for any net core spks. I don't know if the same would affect the other armada variants. |
Looks like it might vbe arm v7 without neon? armv6 binary would run on it then afaik |
.NET Core doesn't support armv6. So essentially Radarr is dropping support for this architecture after the release of 3.1 (hopefully next few weeks). 3.1 will run under mono on this architecture. |
Hi, sorry if this is not the right place to post but as its related thought i would. I tried to upgrade in app tonight from 3.0.2.4552 to 3.2.0.5048 and it didn't work. The app went through the motions, backup, restarted but didn't update. Is this because its .NET and not mono? I've attached my log file for you to peruse. appreciate your help in advance. |
Info logs are typically useful for debugging. for this: stop sonarr, update radarr, start sonarr has typically been doing it. Synology has something weird going on with DSM |
fix makes sense, i suppose....and it worked! Appreciate your help @bakerboy448 . i was starting to think i'd be stuck on that version. |
This reverts commit f0dc885. # Conflicts: # cross/libstdc++/Makefile
This reverts commit f0dc885. # Conflicts: # cross/libstdc++/Makefile
Motivation: Radarr will soon be dropping mono compatible builds so we need to swap to .NET core builds
Linked issues: Fixes #4398, #4300, Fixes #4444
Checklist
all-supported
completed successfullyThe previous attempt at a .NET core build for jackett failed because
libstdc++.so.6
was too old. This version provideslibstdc++.so.6.0.22
from Debian stretch in the packagelib
directory. Tested by @Wuszek on a DS218.