Created attachment 167552 [details] sabnzbdplus patch The attached patch does the following... * replace logic for RUN_DEPENDS with the port system's built in way of doing this * removes setting PATH in rc.d script. This line is currently useless, the rc.d script already has /usr/local/bin in its PATH, and none of the binaries sabnzbd needs are installed in /usr/local/sbin You can verify sabnzbd works without this line by removing it from the rc.d script and starting sabnzbd. If it couldn't find unrar/unzip/etc, it would display an error message in its webui and in its log.
Created attachment 167664 [details] new/sabnzbdplus patch I added another change.. * Replace INSTALL_DATA with INSTALL_MAN for installing PORTDOCS
Thank you for submitting these changes. The only one I hesitate with is the removal of PATH. I remember the reason for it: https://svnweb.freebsd.org/ports?view=revision&revision=299435 The original comment above it explains that in some situations when users try to start sabnzbdplus with "service sabnbzbdplus start" it would fail to start due to a PATH issue. I was able to replicate that several years ago and nobody seemed to have free time to hunt down how or why that was happening, as it should "just work" and you should get a full PATH from rc.subr regardless of what your PATH is when you execute /usr/sbin/service. If you have time to try to replicate this issue with it removed that would be fantastic.
I'll look into the need for PATH and report my results. I actually did notice some differences with the SHELL command is run with when run from 'service' vs /usr/local/etc/rc.d/SCRIPT while debugging some issues with an rc.d script the other day.
Created attachment 167848 [details] screenshot I looked into this further and believe $PATH does not need to be set. The revision of the rc.d script you linked to from 2012 does not use "command" and instead defines its own "start_cmd". In this case some of the rc.subr code isn't being run that adds some extra parameters to the $PATH. I confirmed that rc.subr adds paths to the current $PATH by editing the sabnzbd rc.d script and replacing "command" and "start_precmd" with a script that runs "echo $PATH". Attached are the results when the script is run directly from 'sh' and from 'service'. I exported a minimal $PATH to verify the PATH being echoed wasn't coming from my environment. I got the same behaviour when testing on a FreeBSD 10.1 and 9.3 jail.
Created attachment 167850 [details] new/sabnzbdplus patch The attached patch does the following... * better defines what versions of python sabnzbd can actually run with * removes "+" sign from SUB_LIST which does nothing * replace logic for RUN_DEPENDS with the port system's built in way of doing this * removes setting PATH in rc.d script. See explanation https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=207583#c4. You can verify sabnzbd works without this line by removing it from the rc.d script and starting sabnzbd. If it can't find par2/unrar/unzip/nice, it would display an error message in its webui and its log. * makes patch + reinplace more logical by not replacing %%PREFIX%% with %%DATADIR%% * removes creation of PREFIX/sabnzbd (sabznzbd's config directory) from install. This is already checked for and created during the rc.d script's start_precmd routine. It's better to do it there anyway because that allows users to chose this folder's location and owner using rc.conf variables. * Replace INSTALL_DATA with INSTALL_MAN for installing PORTDOCS
are you interested in taking maintainership of this port? I add that to the commit.
sure, that would be fine. there's more patches where that came from lol. sabnzbd v1.0 should be coming out really soon which includes some dependency changes.
A commit references this bug: Author: feld Date: Tue Mar 8 16:40:01 UTC 2016 New revision: 410634 URL: https://svnweb.freebsd.org/changeset/ports/410634 Log: news/sabnzbdplus: Port cleanup - Better define which versions of python sabnzbd can actually run with - Remove "+" sign from SUB_LIST - Modernize OPTION RUN_DEPENDS - Do not set PATH in rc script - Make patch and reinplace more logical by not replacing %%PREFIX%% with %%DATADIR%% - Remove creation of PREFIX/sabnzbd (sabznzbd's config directory) from install as it is already handled in start_precmd routine - Replace INSTALL_DATA with INSTALL_MAN for installing PORTDOCS - Hand off maintainership PR: 207583 Changes: head/news/sabnzbdplus/Makefile head/news/sabnzbdplus/files/patch-SABnzbd.py head/news/sabnzbdplus/files/sabnzbd.in head/news/sabnzbdplus/pkg-plist
Great, thanks for the quick turn around! Even though I am maintainer I may still need to bug you once and a while since I can't commit things myself. But, this way I can screen suggestions/patches for approval before they get to someone who actually commits stuff. Usually I just submit my patches, and if it's not looked at for a week I'll start contacting people. Thanks!
(In reply to joshruehlig from comment #9) When you submit new updates make sure to mark the PR as patch-ready, maintainer approved, etc and that will help a lot. But yes, feel free to contact me as well or add me to the PR as I am familiar with this port and can help expedite the commit.
Removal of the PATH from the rc script broke my sabnzbd. Both par2 and unrar cannot be found anymore, so I added it back manually on my machine. Please add it back to the rc script.
(In reply to Ralf van der Enden from comment #11) Can you do me a favor and add "echo $PATH" to the start_precmd() of the rc script and try starting the service as normal? I'm very curious what your PATH is before Sabnzbd starts. FYI, hand modifying an rc script will get detected by pkg as the checksum changes and it will intentionally refuse to update or change it when you do updates. Many people don't realize this, so if we come up with an alternative fix you might want to undo those changes or just delete the rc script before the next update. I am hoping we can quickly come up with a proper permanent solution because this is an odd hack that nothing else in the ports tree needs.
reopening due to fallout
(In reply to Mark Felder from comment #12) Sorry, the start_precmd() is actually called sabnzbd_prestart(), so put it in that function.
@Ralf thanks for reporting. If someone submits a patch before I do please add the following to sabnzbd_prestart()... export PATH=${PATH}:%%LOCALBASE%%/bin
A commit references this bug: Author: feld Date: Wed Mar 9 16:59:18 UTC 2016 New revision: 410708 URL: https://svnweb.freebsd.org/changeset/ports/410708 Log: news/sabnzbdplus: Revert removal of PATH in rc script There were reports of fallout so this has been reverted. It is not understood why an explicit PATH needs to be set. In testing without it the correct PATH appears to be exported during the start_precmd routine. PR: 207583 Approved by: maintainer Changes: head/news/sabnzbdplus/Makefile head/news/sabnzbdplus/files/sabnzbd.in
Reverted. Closing for now. We should revisit this issue in another PR. I have started searching through the sabnzbdplus codebase for the usage of os.path() but have not been able to locate any odd behavior yet.
sounds good, thanks for the fix. that's actually better then what we had before because it works for non-standard PREFIX directories.