Bug 205441

Summary: net-p2p/sonarr: Some suggestions
Product: Ports & Packages Reporter: joshruehlig
Component: Individual Port(s)Assignee: Mark Felder <feld>
Status: Closed FIXED    
Severity: Affects Some People Keywords: needs-qa, patch
Priority: --- Flags: bugzilla: maintainer-feedback? (feld)
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Diff for the rc.d file
none
Diff for net-p2p/sonarr
none
Diff for net-p2p/sonarr none

Description joshruehlig 2015-12-19 22:10:51 UTC 
Created attachment 164395 [details]
Diff for the rc.d file

Here are some suggested changes for the net-p2p/sonarr port. I also included a diff.
Thanks

1) Allow sonarr's daemon user be an rc.conf option. This allows users to more easily run sonarr as a different user for interaction with other programs, for example transmission or sabnzbd.
This also allows running the daemon process as this user without having to set it in 'command_args'.

2) Allow sonarr's data-directory to be an rc.conf option, and default this to /var/db/sonarr.
From the reference I believe PREFIX should only be for port installed files (bin/lib/share...) and configurations (etc). I personally always put metadata directories (files the program is managing) in /var/db/PORTNAME. This is the case with the plexmediaserver, emby-server, syncthing, and btsync ports.
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/install.html

3) Use the pidfile the sonarr program creates and manages. I don't see a need to have the daemon program create another one when the program does this already.

4) Use LOCALBASE instead of PREFIX for called dependencies. I was told this is the best way to do this for the emby-server port, not sure why.

5) Set XDG_CONFIG_HOME to a directory the sonarr user can write to. If this isn't set it will try to write to .mono in its home directory (which doesn't exist) and will experience application lockups.
https://forums.freenas.org/index.php?threads/sonarr-plugin-gui-update-issues.27711/
https://forums.sonarr.tv/t/gui-update-issues/4195
Comment 1 joshruehlig 2015-12-19 23:59:07 UTC 
I forgot, we should check to make sure the data-driectory exists during the prestart routine, this allows the program to work even if the user changes the data-directory. I'll provide an updated diff.
Comment 2 joshruehlig 2015-12-20 00:04:26 UTC 
Created attachment 164399 [details]
Diff for net-p2p/sonarr
Comment 3 Mark Felder freebsd_committer freebsd_triage 2015-12-23 18:55:28 UTC 
(In reply to joshruehlig from comment #0)

Thanks for the patch! Don't take these as criticisms, let me just do a quick brain dump:

1) I was intentionally setting the user in command_args because you have to run daemon(1) as root to change to another user, and you need to be root to write the pidfile in /var/run. Adding a sonarr_user totally blows up the usage of daemon(1), which is a bummer.

2) I intentionally did not put the data in /var/db. There has been a desire for ports to never create files in the "base" filesystem hierarchy (/usr/local/var/{db|run} should probably become a standard). Also, this can chew up a lot of data and if /var/db is on a small root FS it will make someone very angry :-) 

Making datadir configurable is smart, though.

3) I did intentionally ignore sonarr's pidfile but you're right we could use it if we aren't writing into /var/run.

4) I have to use PREFIX in the port Makefile, so I carried this over to the rc script but it's interchangeable, yes. I don't know if the best practices are documented for this...

5) Wow, didn't know this. I don't think I've experienced any issues, but this is good to know. We should probably set it to the same location as the data dir.


I'll probably work through this in a few stages.
Comment 4 joshruehlig 2015-12-23 19:21:31 UTC 
Thanks for taking a look and the reply.

1&3) By using the program's pidfile, setting the user through the rc system will work so changes 1&3 go together well. This would allow user portability like I mentioned earlier which is useful for this program which works tightly with sabnzbd/transmission.

2) That's fine with me having a different default datadirectoryc then my suggestion. But having it editable would be great. I maintain many of the FreeNAS plugins, and keeping stuff out of PREFIX is important because the PBI update process is pretty fragile. At least in FreeBSD9.X

4) I personally don't care if this is implemented. Works the same for my use case either way.

5) Yes, this should be implemented for sure because of the way mono expects an existing directory to write to.

It'll be great once I can base the FreeNAS plugin on this port. One less thing for me to maintain in my personal repo.
Comment 5 joshruehlig 2015-12-29 02:43:47 UTC 
Created attachment 164797 [details]
Diff for net-p2p/sonarr

Ok, I updated the patch based on your comments, while keeping the changes I feel give the greatest benefit/portability.

This patch does the following...

1) Makes sonarr's user an rc.conf option.
2) Makes the data-directory an rc.conf option.
3) Uses the pidfile the sonarr program creates/managers.
4) Sets XDG_CONFIG_HOME which avoids crashes when mono needs to download and store certificates/chains to connect to SSL servers.

* Note- The daemon process now runs as a non-root user by default which is reliant on point #3
Comment 6 Mark Felder freebsd_committer freebsd_triage 2015-12-31 15:05:15 UTC 
looks sane to me, thanks for your work! I'll patch my local instance and then commit after a quick test.
Comment 7 commit-hook freebsd_committer freebsd_triage 2015-12-31 15:23:02 UTC 
A commit references this bug:

Author: feld
Date: Thu Dec 31 15:22:57 UTC 2015
New revision: 404949
URL: https://svnweb.freebsd.org/changeset/ports/404949

Log:
  net-p2p/sonarr: Various improvements

  - Allow running sonarr as a different user
  - Use sonarr's pidfile
  - Allow configuring location of the data directory
  - Export XDG_CONFIG_HOME to a writable directory to prevent runtime problems

  PR:		205441

Changes:
  head/net-p2p/sonarr/Makefile
  head/net-p2p/sonarr/files/sonarr.in
Comment 8 joshruehlig 2015-12-31 17:43:28 UTC 
great thanks. I'll adapt the freenas plugin to use the official port