Bug 259194 - net-p2p/radarr: Switch to upstream .NET binary release
Summary: net-p2p/radarr: Switch to upstream .NET binary release
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Mark Felder
URL: https://github.com/Radarr/Radarr/rele...
Keywords: feature, needs-qa
Depends on:
Blocks: 260920
  Show dependency treegraph
 
Reported: 2021-10-15 21:51 UTC by Michiel van Baak Jansen
Modified: 2022-01-24 17:29 UTC (History)
5 users (show)

See Also:
bugzilla: maintainer-feedback? (feld)


Attachments
0001-net-p2p-radarr-Switch-to-dotnet-version.patch (7.15 KB, patch)
2021-10-15 21:51 UTC, Michiel van Baak Jansen
no flags Details | Diff
poudriere log (13.89 KB, text/plain)
2021-10-15 22:05 UTC, Michiel van Baak Jansen
no flags Details
0001-net-p2p-radarr-Switch-to-dotnet-version.patch (39.20 KB, patch)
2021-10-15 23:15 UTC, Michiel van Baak Jansen
no flags Details | Diff
0001-net-p2p-radarr-Switch-to-dotnet-version.patch (39.27 KB, patch)
2021-10-21 20:39 UTC, Michiel van Baak Jansen
no flags Details | Diff
0001-net-p2p-radarr-Switch-to-dotnet-version.patch (39.25 KB, patch)
2021-11-26 13:37 UTC, Michiel van Baak Jansen
no flags Details | Diff
0001-net-p2p-radarr-Switch-to-dotnet-version.patch (39.22 KB, patch)
2021-11-30 14:52 UTC, Michiel van Baak Jansen
no flags Details | Diff
0002-net-p2p-radarr-Tell-.NET-to-disable-IPV6-if-system-h.patch (1.13 KB, patch)
2021-12-23 16:02 UTC, Michiel van Baak Jansen
no flags Details | Diff
net-p2p_radarr-switch_to_dotnet_version.diff (38.90 KB, patch)
2021-12-23 16:06 UTC, Michiel van Baak Jansen
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michiel van Baak Jansen 2021-10-15 21:51:10 UTC
Created attachment 228731 [details]
0001-net-p2p-radarr-Switch-to-dotnet-version.patch

Instead of fetching and using the mono build binary for radarr, use the dotnet build binary.
This removes the dependency on the mono runtime on the machine.
It also prepares for the next major release of radarr, which will not be available for mono, but will be dotnet only.

portlint ok
poudriere ok
Runs in 'production' on my setup (13.0 AMD64)
Comment 1 Michiel van Baak Jansen 2021-10-15 22:02:08 UTC
Thanks to the FreeBSD dotnet contributors and the radarr team. They made it possible and release dotnet binaries on the radarr project.
Most changes in this patch are taken from them.
Comment 2 Michiel van Baak Jansen 2021-10-15 22:05:21 UTC
Created attachment 228732 [details]
poudriere log
Comment 3 Michiel van Baak Jansen 2021-10-15 23:15:59 UTC
Created attachment 228735 [details]
0001-net-p2p-radarr-Switch-to-dotnet-version.patch

Updated patch to include the pkg-plist file instead of generating that dynamically in the post-install step
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2021-10-20 23:16:46 UTC
Note, via discussion with Michiel on IRC:

 - This drops i386 support (which the port currently supports)
 - Request FreeBSD i386 build/binary from upstream (open an issue)
 - Add comment to ONLY_FOR_ARCHS pointing to upstream issue

This change (to binary .NET releases) is *transitionary only* until CoreCLR/.NET lands in ports, at which point these port(s) should be build from sources
Comment 5 Michiel van Baak Jansen 2021-10-21 20:39:07 UTC
Created attachment 228931 [details]
0001-net-p2p-radarr-Switch-to-dotnet-version.patch

Changes:

* Dont run find + chmod to make the Radarr binary executable, but use INSTALL_PROGRAM
* Document we requested an x86 binary https://github.com/Radarr/Radarr/issues/6668
* Better way to fill USERS and GROUPS variable
Comment 6 Dries Michiels freebsd_committer freebsd_triage 2021-11-15 11:26:44 UTC
Nice! I'm not sure removing the old mono port of radarr is the way to go (even if it gets stuck on the latest release which still runs on mono). What do you think about adding it as a new port, net-p2p/radarr-dotnet? This way users can choose, latest and greatest with embedded dotnet or using mono (source built) using the existing port (you can add a CONFLICTS_INSTALL line to prevent installing both at the same time). When time comes we can update the official net-p2p/radarr port to use an official dotnet build and drop the net-p2p/radarr-dotnet port, given the "transitionary only" nature.

Few comments / suggestions:
   - DATADIR=	${PREFIX}/${PORTNAME}${PKGNAMESUFFIX}

PKGNAMESUFFIX doesn't seem to be specified in the Makefile anywhere?

   - rc_flags="-r -f -p ${pidfile_child} -P ${pidfile} ${%%PORTNAME%%_exec_dir}/Radarr --data=${%%PORTNAME%%_data_dir} --nobrowser >> /dev/null 2>&1 ${rc_flags}"

Can we use command_args here like the previous script (if possible)? IIRC the benefit of using command_args over rc_flags or name_flags is that users can specify these *_flags in rc.conf and they will simply be appended without overwriting the default, needed, command_args. command_args cannot be set from rc.conf, *_flags can.
Comment 7 Michiel van Baak Jansen 2021-11-16 10:18:53 UTC
(In reply to Dries Michiels from comment #6)
I'm not a fan of keeping the mono port around. radarr is not going to release another version with mono support, the health check in the radarr application will show a warning, the mono version is _NOT_ source built, the dotnet standalone version can run alongside whatever mono application (sonarr, lidarr, readarr just to name a few).

You are right about the PKGNAMESUFFIX, I'll remove that one.

https://docs.freebsd.org/en/articles/rc-scripting/#rcng-daemon-adv states command_args should be used. Is this page outdated? If so, I'll change the rc script. Let me know.

Thanks for the review. Really hope we can get this one going.
Comment 8 Dries Michiels freebsd_committer freebsd_triage 2021-11-21 14:19:57 UTC
I was talking about the tuntime, i.e. source built mono in the ports tree vs a binary bundled dotnet in this package.

Regarding the rc script, can you try running the daemon with your current rc_script but specify some extr aflag in rc.conf with radarr_flags="-a" with something valid. Does it overwrite all flags or appends it (check ps -aux)? If it appends its fine, if it overwrites everything else its a pain and command_args is preferred.
Comment 9 Michiel van Baak Jansen 2021-11-26 13:11:24 UTC
(In reply to Dries Michiels from comment #8)
I added this to my /etc/rc.conf:

# radarr (movies)
radarr_enable="YES"
radarr_user="transmission"
radarr_group="downloadsystem"
radarr_flags="--overridden-rcconf --data=/var/db/radarr --end-override"

When starting radarr using /usr/local/etc/rc.d/radarr start it gives me this running process:

transmission 70108  0.0  0.0 2676956 18808  -  RJ   14:10   0:00.04 /usr/local/radarr/Radarr --data=/var/db/radarr --nobrowser --overridden-rcconf --data=/var/db/radarr --end-override

So it looks like it appends cleanly, and the documentation is up-to-date.
Comment 10 Michiel van Baak Jansen 2021-11-26 13:37:58 UTC
Created attachment 229740 [details]
0001-net-p2p-radarr-Switch-to-dotnet-version.patch

Removed reference to variable that is never set (PKGNAMESUFFIX)
Comment 11 Michiel van Baak Jansen 2021-11-30 14:52:17 UTC
Created attachment 229811 [details]
0001-net-p2p-radarr-Switch-to-dotnet-version.patch

- Removed override of ${DATADIR}
- Install in the same location as before
- Use the same data directory as before

This will make upgrades hassle free as paths stay the same
Comment 12 Mark Felder freebsd_committer freebsd_triage 2021-12-16 19:11:26 UTC
Switching over to this version of the port makes my download client (nzbget) break.

I keep getting a "Protocol not supported" error in the UI, and here's what I get from the console:



[v3.2.2.5080] System.Net.Sockets.SocketException (43): Protocol not supported
   at System.Net.HttpWebRequest.GetResponse()
   at NzbDrone.Common.Http.Dispatchers.ManagedHttpDispatcher.GetResponse(HttpRequest request, CookieContainer cookies) in D:\a\1\s\src\NzbDrone.Common\Http\Dispatchers\ManagedHttpDispatcher.cs:line 81
   at NzbDrone.Common.Http.HttpClient.ExecuteRequest(HttpRequest request, CookieContainer cookieContainer) in D:\a\1\s\src\NzbDrone.Common\Http\HttpClient.cs:line 124
   at NzbDrone.Common.Http.HttpClient.Execute(HttpRequest request) in D:\a\1\s\src\NzbDrone.Common\Http\HttpClient.cs:line 57
   at NzbDrone.Core.Download.Clients.Nzbget.NzbgetProxy.ProcessRequest[T](NzbgetSettings settings, String method, Object[] parameters) in D:\a\1\s\src\NzbDrone.Core\Download\Clients\Nzbget\NzbgetProxy.cs:line 239
   at NzbDrone.Core.Download.Clients.Nzbget.NzbgetProxy.GetConfig(NzbgetSettings settings) in D:\a\1\s\src\NzbDrone.Core\Download\Clients\Nzbget\NzbgetProxy.cs:line 159
   at NzbDrone.Core.Download.Clients.Nzbget.Nzbget.TestCategory() in D:\a\1\s\src\NzbDrone.Core\Download\Clients\Nzbget\Nzbget.cs:line 296
   at NzbDrone.Core.Download.Clients.Nzbget.Nzbget.Test(List`1 failures) in D:\a\1\s\src\NzbDrone.Core\Download\Clients\Nzbget\Nzbget.cs:line 265
   at NzbDrone.Core.Download.DownloadClientBase`1.Test() in D:\a\1\s\src\NzbDrone.Core\Download\DownloadClientBase.cs:line 121
Comment 13 Mark Felder freebsd_committer freebsd_triage 2021-12-16 19:29:37 UTC
(In reply to Mark Felder from comment #12)

trussing the process finds this

81912: socket(PF_INET6,SOCK_STREAM|SOCK_CLOEXEC,IPPROTO_TCP) ERR#43 'Protocol not supported'


so anyone not using a VNET jail also needs to add at a minimum:

ip6 = inherit;

even if they don't use ipv6
Comment 14 Mark Felder freebsd_committer freebsd_triage 2021-12-16 19:47:10 UTC
With Mono the socket is opened like

33968: socket(PF_INET,SOCK_STREAM,IPPROTO_TCP)	 = 16 (0x10)


so I think this is a networking bug in .NET vs Mono




Still investigating
Comment 15 Mark Felder freebsd_committer freebsd_triage 2021-12-16 20:12:16 UTC
From what I can tell by looking through the Radarr codebase they're calling System.Net.Http and not doing any special handling so this is most likely an issue in the DotNet codebase. My gut says it's something in here:

https://github.com/dotnet/runtime/blob/main/src/native/libs/System.Native/pal_networking.c
Comment 16 Michiel van Baak Jansen 2021-12-16 20:44:05 UTC
Thanks a lot for your feedback.
I did not test in a non VNET jail without ipv6. Good thing you did so this came up.

Just like with the mlock, I can add this ip6=inherit setting to the message.
Comment 17 Tomas Weinfurt 2021-12-19 03:24:42 UTC
.NET 6+ use dual mode sockets to avoid the separate AF handling. It would be _nice_ if the jail can recognize V4 mapped addresses and treat it as v4 ... but it may be too early when socket is created. (and I would expect v6 enabled by default in these days) 

Instead of adding V6 to jail, you can disable .NET use of V6.
https://devblogs.microsoft.com/dotnet/dotnet-6-networking-improvements/#an-option-to-globally-disable-ipv6

Perhaps the FreeBSD port should improve detection of V6 availability.
Comment 18 Mark Felder freebsd_committer freebsd_triage 2021-12-20 15:52:19 UTC
(In reply to Tomas Weinfurt from comment #17)

In that case just adding this to the rc script's start_precmd() should be sufficient


    ifconfig | grep -q inet6
    if [ $? == 1 ]; then
      export DOTNET_SYSTEM_NET_DISABLEIPV6=1
    fi
Comment 19 Mark Felder freebsd_committer freebsd_triage 2021-12-20 16:11:07 UTC
(In reply to Mark Felder from comment #18)

A better way to do this would be to rely on the af_exists() function in /etc/network.subr which uses check_kern_features() from /etc/rc.subr, but it's unfortunately not jail-compatible.

Ideally the kern.features sysctls would return the correct answers for the features enabled in the jail, but it currently does not.
Comment 20 Michiel van Baak Jansen 2021-12-23 16:02:06 UTC
Created attachment 230350 [details]
0002-net-p2p-radarr-Tell-.NET-to-disable-IPV6-if-system-h.patch

Attached followup patch to fix the rc script.

Will update new, complete patch in 5mins
Comment 21 Michiel van Baak Jansen 2021-12-23 16:06:56 UTC
Created attachment 230351 [details]
net-p2p_radarr-switch_to_dotnet_version.diff

Same patch as before, but now with ipv6 check in the rc script.
This is the same as applying patch 229811 and then 230350.

poudriere ok
Runs in my setup (13.0 amd64 jail with vnet and ipv6)

feld@ Can you please test this in your setup without v6?
If ok with you, this can go through.

Thanks.
Comment 22 Kubilay Kocak freebsd_committer freebsd_triage 2022-01-19 22:41:33 UTC
Comment on attachment 230351 [details]
net-p2p_radarr-switch_to_dotnet_version.diff

^Triage: Request review from maintainer
Comment 23 Kubilay Kocak freebsd_committer freebsd_triage 2022-01-21 01:30:38 UTC
Comment on attachment 230351 [details]
net-p2p_radarr-switch_to_dotnet_version.diff

Approved by: portmgr (blanket: maintainer timeout, ~1 month)

Open to take pending QA
Comment 24 Mark Felder freebsd_committer freebsd_triage 2022-01-23 20:38:17 UTC
The DOTNET_SYSTEM_NET_DISABLEIPV6=1 stopped working for me and I don't know why but at this point we just need to move forward.
Comment 25 commit-hook freebsd_committer freebsd_triage 2022-01-24 17:22:12 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=e8ad94527c16677bb98216f42f7a31b5bd4c9d5b

commit e8ad94527c16677bb98216f42f7a31b5bd4c9d5b
Author:     Mark Felder <feld@FreeBSD.org>
AuthorDate: 2022-01-24 17:07:50 +0000
Commit:     Mark Felder <feld@FreeBSD.org>
CommitDate: 2022-01-24 17:21:07 +0000

    net-p2p/radarr: Update to 3.2.2.5080

    This release is the first to be be built with CoreCLR/.NET which is
    superceding the Mono project and future versions (4.x)
    of Radarr will only work with the .NET language implementation.

    Previously we used binary builds of Radarr and the Mono runtime would
    be used to execute the binary. The new .NET implementation doesn't
    require a runtime as the compiled binary is natively executable.

    Compiling Radarr with Mono was difficult in the past so we shipped the
    binary and the Mono runtime, but we expect in the future we will be able
    to compile Radarr and other similar apps directly with a FreeBSD
    supported release of .NET.

    I am also releasing myself as maintainer as we have several suitable and
    motivated people keeping up with these latest developments and I don't
    want to continue to block progress.

    PR:     259194

 net-p2p/radarr/Makefile                   |  47 ++-
 net-p2p/radarr/distinfo                   |   6 +-
 net-p2p/radarr/files/pkg-message.in (new) |  33 ++
 net-p2p/radarr/files/radarr.in            |  68 +--
 net-p2p/radarr/pkg-plist (new)            | 673 ++++++++++++++++++++++++++++++
 5 files changed, 775 insertions(+), 52 deletions(-)