Created attachment 171846 [details] subsonic-standalone diff See attached diff for changes. When I have a chance I'll also work on updating and applying the same changes to the www/madsonic-standalone port. ####CHANGES#### * Updated to version 6.0 - http://www.subsonic.org/pages/changelog.jsp * Make use of PKGNAMESUFFIX * Remove no longer needed ${PORTSDIR} * Make use of NO_WRKSUBDIR * Specify JAVA_RUN * Install application files to DATADIR instead of in PREFIX/PORTNAME - See 5.15.5 https://www.freebsd.org/doc/en/books/porters-handbook/install.html * Use MKDIR/INSTALL_MAN when appropriate * Fix typos and missing plist entry causing ffmpeg link not to be installed * (Possibly) Empty directories should be created during post-install * Add `REQUIRE: LOGIN` and `KEYWORD: shutdown` - See 6.27 https://www.freebsd.org/doc/en/books/porters-handbook/rc-scripts.html * Remove folder options from rc.conf - These commandline switches only have an effect before they are overwritten in the subsonic WebUI. To make things act more as expected, set them to sane default but don't expose them as changes in rc.conf likely will not. * Use FreeBSD rc.d framework instead of subsonic.sh script - pidfile no longer needed - standardize how variable are set - utilize daemon to watch process * Set LC_TYPE to 'UTF-8' - allows files with complex characters to be properly handled * Instead of exposing subsonic_http_port and subsonic_https_port, change options to subsonic_port and subsonic_ssl - Subsonic only can be accessed from HTTP or HTTPS at any one time. If you specify an HTTPS port other than 0 (disabled) HTTP does not work as expected and is just a redirect to HTTPS. I believe this change makes the settings act more as expected. * Allow specifying a custom SSL Keystore and associated password
Created attachment 171847 [details] subsonic-standalone diff
Created attachment 171848 [details] subsonic-standalone diff
Created attachment 171855 [details] subsonic-standalone diff Tweaked things slightly. Please review when you have a chance, thanks
Assign to actual maintainer.
Fix Component. Unless you are proposing a change in Mk/* , "Individual ports" is most likely the correct Component.
Whoops, I usually get that one right. Been looking at the screen too much today.
Thanks for the patch. Please note that maintainer feedback and approval requests require email address for notification.
(In reply to Vladimir Krstulja from comment #7) not sure what I need to differently to add then before requesting feedback? sorry, not super familiar with this system. thanks
It has been a week with no action from the maintainer. I also emailed him around 5 days ago to see if he was planning on reviewing this bug report, or a similar one (linked below), and he has not responded. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210651 @Vladimir Krstulja or @Rene Ladan , what should be my next course of action to get these two bug reports reviewed? Thanks
According to the book Jeremy has another week to reply (he could for example be on holidays). After two weeks, the "maintainer timeout" condition applies which normally means implicit approval.
sounds good. thanks for the reply
(In reply to joshruehlig from comment #8) When requesting maintainer-feedback(?) for the entire issue, or maintainer-approval(?) for an attachment, you need to supply the maintainer email address. That way they are notified of your request and Bugzilla knows whom to allow to respond to it. That is also why we insist the maintainer email address and Bugzilla account address for the maintainers be the same. ;)
Created attachment 172372 [details] subsonic-standalone diff fix typo
Hi, Thanks for your patch. What the difference between this bug and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210651 ?
Hi Jeremie, They are different ports. One is for www/subsonic-standalone, the other is for www/madsonic-standalone.
Thanks for updating the port. I've been wanting to do it but didn't find the time. I'm going to try to get some time today to review it, and the other one as well.
I started with an empty scratch jail and the port and its dependencies are still being compiled :-). Anyways, from the detailed summary you gave (thanks!), here are my comments: - I would rather leave "-standalone" in ${PORTNAME}; otherwise I think this port will be identified as "subsonic", like the non-standalone version and this may cause unforeseen problems (like, for instance, I don't know how the upgrade process will handle this). I can be convinced to use ${PKGNAMESUFFIX}, but please give a good reason. - I'm not super fan of installing the port to ${DATADIR}. I find it weird to have the .sh there. This is the main reason I ended up installing in ${PREFIX}/${PORTDIRNAME}. But well, I guess it's OK. Nonetheless I am concerned about the upgrade path; have you tested this? - Please make either a compat shim for the options you removed, or a big warning the option has been deprecated, giving the replacement method. -- Jeremie
Created attachment 172628 [details] subsonic-standalone diff Wouldn't it just be faster to 'pkg install openjdk ffmpeg' then test this port, instead of spend a day building java, lol. =P * I'm fine with having "-standalone" in the name as well. Just thought it was cleaner the way I proposed, but I not sure of the consequences so keeping it how it is, is safest until we look into this more. * If you see the pkg-plist changes I don't install the script anymore. That is not needed with a proper rc.d script that utilizes daemon to control the process. I tested an upgrade from the subsonic-standalone v5.2 and it worked as expected. * Concerining the music and playlist folder options, I do not see it as a huge deal. People currently using www/subsonic-standalone aren't affected since these options don't do anything after first run. And people who are new to trying this port would not necessarily even know the option were there before. I explained this here https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210608#c0, these options really only need to be tweaked by package maintainers since they only effect new installs. Users should be setting this from the subsonic WebUI for it to actually work. Concerning the changes to http_port & https_port > port & ssl, thanks for the idea. I added a 'pkg-install' script to handle this during install so things will continue to work as they were. If we do want to inform people of the option changes during install/upgrade we could add notes to the 'pkg-message'. The attached diff... - reverts the PKGNAMESUFFIX change - handles the changes to http_port/https_port/port/ssl I'll also do the same changes for madsonic when I get a chance.
Created attachment 172766 [details] subsonic-standalone diff Improve the script logic
Jeremie, Any idea how much more time you need for your review of this bug and the madsonic-standalone bug? https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210651 Thanks
@Josh, it would be great if you split the existing patch into two, one for the fixes and the other just for the version update, so the former can be MFH'd (merged) to the quarterly branch and users. Please also note that the maintainer timeout period resets for each new/updated attachment.
Created attachment 172810 [details] subsonic-standalone fixes * I decided to put back the use of PKGNAMESUFFIX. Feel free to modify my diff if you like - This is what the www/subsonic port uses to distinguish between distribution methods. - I checked and the note here says if ports have the same PORTNAME to add a PKGNAMESUFFIX, which is exactly what I am doing. https://www.freebsd.org/doc/en/books/porters-handbook/makefile-naming.html#porting-pkgnameprefix-suffix - portlint complains if a "-" is in PORTNAME but I think that is for other reasons. * Added CONFLICTS_INSTALL with the various subsonic package names
Created attachment 172811 [details] subsonic-standalone update
@Kubilay As suggested I split the patch to the fixes, and the version update.
Created attachment 172814 [details] subsonic-standalone fixes Realized the unneeded ${PORTSDIR}/ was already removed back in April 2016.
Thank you Josh, I presume these pass QA (poudriere) ?
I have never used poudriere, just portlint. I'll try it out.
I see poudriere needs a host system. I've been testing in a jail. Is there another testing method I can use?
(In reply to joshruehlig from comment #28) In the short term, the following is OK: make stage && make check-plist && make stage-qa && make package
Created attachment 172820 [details] subsonic-standalone output Output of "make stage && make check-plist && make stage-qa && make package" attached.
Thank you Josh. Attachments are not requires unless specifically requested, just confirming in a comment as follows is sufficient for now: portlint: OK (looks fine.) testport: OK (poudriere: <archs>, <versions>, <options> tested) Now all we need is maintainer approval to move forward, Nice work!
ok sounds good, just wasn't sure if you needed all the output. As a reminder my nearly identical patch for www/madsonic-standalone also needs review/approval. Thanks!
Looks good to me. I approved it. Sorry for the delay in response, but I had little time lately (baby, family visiting, work...). Josh, if you're interested, I could transfer the maintainership to you for this port.
koobs@ can you approve it as well, I don't have the port commit bit.
Sounds good. Now worries, totally understood that life can be hectic. No need to change maintainership. I don't plan on working on any other changes/fixes for this port. Updates should be easy to implement for anyone.
FYI, I put subsonic-standalone fixes on review on Phabricator, I was told there are a few glitches which needs to be taken care of: https://reviews.freebsd.org/D7304 I'll take care of the updates.
@Jeremie don't forget to add to Reviewers the person/people who said it was needed, they're on the hook to review :)
I haven't seen any activity on Phabricator other than the comment which I addressed https://reviews.freebsd.org/D7304#153593. Would it help if I included this fix in the diff posted here? What other "glitches" needed to be taken care of? Thanks, Josh
A commit references this bug: Author: jlh Date: Mon Aug 22 13:33:24 UTC 2016 New revision: 420615 URL: https://svnweb.freebsd.org/changeset/ports/420615 Log: Minor improvements. Most noticeable changes in no particular order: - The rc.d script doesn't use the provided shell script to start the daemon, but starts it directly. - This is now installed in ${DATADIR}, not in ${PREFIX}/${PORTDIRNAME}. - Define CONFLICTS_INSTALL so as to avoid colliding with non standalone version. - Use post-install-${OPTION}-on. PR: 210608 Submitted by: joshruehlig@gmail.com Reviewed by: crees Approved by: crees Differential Revision: https://reviews.freebsd.org/D7304 Changes: head/www/subsonic-standalone/Makefile head/www/subsonic-standalone/files/subsonic.in head/www/subsonic-standalone/pkg-plist
Awesome thanks for everyone's work on this! The fixes are in, so I assume it is ready for the version update then?
Yes, I'm in the process of getting the approval.
A commit references this bug: Author: jlh Date: Mon Aug 22 21:15:16 UTC 2016 New revision: 420636 URL: https://svnweb.freebsd.org/changeset/ports/420636 Log: Update to version 6.0. PR: 210608 Submitted by: joshruehlig@gmail.com Approved by: brd Changes: head/www/subsonic-standalone/Makefile head/www/subsonic-standalone/distinfo
Finally updated! Thanks for the work and sorry for the long time it took.
Sweet, thanks for the persistence.