Bug 210608 - www/subsonic-standalone: Update to 6.0 and improve port
Summary: www/subsonic-standalone: Update to 6.0 and improve port
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Jeremie Le Hen
URL: https://reviews.freebsd.org/D7304
Keywords: needs-patch, needs-qa, patch
Depends on:
Reported: 2016-06-27 02:44 UTC by joshruehlig
Modified: 2016-08-22 21:31 UTC (History)
3 users (show)

See Also:
jlh: maintainer-feedback+
koobs: merge-quarterly?

subsonic-standalone diff (9.88 KB, text/plain)
2016-06-27 02:44 UTC, joshruehlig
no flags Details
subsonic-standalone diff (9.88 KB, patch)
2016-06-27 02:45 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone diff (9.88 KB, patch)
2016-06-27 02:51 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone diff (9.80 KB, patch)
2016-06-27 06:57 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone diff (9.80 KB, patch)
2016-07-11 11:57 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone diff (10.15 KB, patch)
2016-07-18 08:00 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone diff (10.22 KB, patch)
2016-07-21 03:25 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone fixes (9.85 KB, patch)
2016-07-21 21:40 UTC, joshruehlig
no flags Details | Diff
subsonic-standalone update (1.03 KB, patch)
2016-07-21 21:42 UTC, joshruehlig
joshruehlig: maintainer-approval? (jlh)
Details | Diff
subsonic-standalone fixes (9.04 KB, patch)
2016-07-21 22:06 UTC, joshruehlig
joshruehlig: maintainer-approval? (jlh)
Details | Diff
subsonic-standalone output (1.96 KB, text/plain)
2016-07-22 04:19 UTC, joshruehlig
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description joshruehlig 2016-06-27 02:44:05 UTC
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.


* Updated to version 6.0
- http://www.subsonic.org/pages/changelog.jsp


* 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
Comment 1 joshruehlig 2016-06-27 02:45:50 UTC
Created attachment 171847 [details]
subsonic-standalone diff
Comment 2 joshruehlig 2016-06-27 02:51:00 UTC
Created attachment 171848 [details]
subsonic-standalone diff
Comment 3 joshruehlig 2016-06-27 06:57:49 UTC
Created attachment 171855 [details]
subsonic-standalone diff

Tweaked things slightly. Please review when you have a chance, thanks
Comment 4 Rene Ladan freebsd_committer 2016-06-27 07:14:48 UTC
Assign to actual maintainer.
Comment 5 Rene Ladan freebsd_committer 2016-06-27 07:16:52 UTC
Fix Component. Unless you are proposing a change in Mk/* , "Individual ports" is most likely the correct Component.
Comment 6 joshruehlig 2016-06-27 07:18:11 UTC
Whoops, I usually get that one right. Been looking at the screen too much today.
Comment 7 VK freebsd_triage 2016-06-27 09:37:25 UTC
Thanks for the patch. Please note that maintainer feedback and approval requests require email address for notification.
Comment 8 joshruehlig 2016-06-27 16:09:40 UTC
(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
Comment 9 joshruehlig 2016-07-04 23:46:54 UTC
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.

@Vladimir Krstulja or @Rene Ladan , what should be my next course of action to get these two bug reports reviewed? Thanks
Comment 10 Rene Ladan freebsd_committer 2016-07-06 20:50:04 UTC
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.
Comment 11 joshruehlig 2016-07-06 20:51:27 UTC
sounds good. thanks for the reply
Comment 12 VK freebsd_triage 2016-07-09 01:02:13 UTC
(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. ;)
Comment 13 joshruehlig 2016-07-11 11:57:57 UTC
Created attachment 172372 [details]
subsonic-standalone diff

fix typo
Comment 14 Jeremie Le Hen freebsd_committer 2016-07-13 14:04:04 UTC

Thanks for your patch.

What the difference between this bug and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210651 ?
Comment 15 joshruehlig 2016-07-13 14:51:16 UTC
Hi Jeremie,
They are different ports. One is for www/subsonic-standalone, the other is for www/madsonic-standalone.
Comment 16 Jeremie Le Hen freebsd_committer 2016-07-17 08:25:10 UTC
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.
Comment 17 Jeremie Le Hen freebsd_committer 2016-07-18 04:22:49 UTC
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
Comment 18 joshruehlig 2016-07-18 08:00:36 UTC
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.
Comment 19 joshruehlig 2016-07-21 03:25:09 UTC
Created attachment 172766 [details]
subsonic-standalone diff

Improve the script logic
Comment 20 joshruehlig 2016-07-21 05:42:24 UTC
Any idea how much more time you need for your review of this bug and the madsonic-standalone bug?

Comment 21 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-21 08:38:39 UTC
@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.
Comment 22 joshruehlig 2016-07-21 21:40:22 UTC
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
Comment 23 joshruehlig 2016-07-21 21:42:04 UTC
Created attachment 172811 [details]
subsonic-standalone update
Comment 24 joshruehlig 2016-07-21 21:42:49 UTC
As suggested I split the patch to the fixes, and the version update.
Comment 25 joshruehlig 2016-07-21 22:06:01 UTC
Created attachment 172814 [details]
subsonic-standalone fixes

Realized the unneeded ${PORTSDIR}/ was already removed back in April 2016.
Comment 26 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-22 03:57:08 UTC
Thank you Josh, I presume these pass QA (poudriere) ?
Comment 27 joshruehlig 2016-07-22 04:07:02 UTC
I have never used poudriere, just portlint. I'll try it out.
Comment 28 joshruehlig 2016-07-22 04:09:10 UTC
I see poudriere needs a host system. I've been testing in a jail. Is there another testing method I can use?
Comment 29 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-22 04:13:19 UTC
(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
Comment 30 joshruehlig 2016-07-22 04:19:20 UTC
Created attachment 172820 [details]
subsonic-standalone output

Output of "make stage && make check-plist && make stage-qa && make package" attached.
Comment 31 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-22 04:21:29 UTC
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!
Comment 32 joshruehlig 2016-07-22 04:24:36 UTC
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!
Comment 33 Jeremie Le Hen freebsd_committer 2016-07-23 09:45:09 UTC
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.
Comment 34 Jeremie Le Hen freebsd_committer 2016-07-23 09:47:14 UTC
koobs@ can you approve it as well, I don't have the port commit bit.
Comment 35 joshruehlig 2016-07-23 15:59:10 UTC
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.
Comment 36 Jeremie Le Hen freebsd_committer 2016-07-25 11:33:46 UTC
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.
Comment 37 Kubilay Kocak freebsd_committer freebsd_triage 2016-07-26 12:16:07 UTC
@Jeremie don't forget to add to Reviewers the person/people who said it was needed, they're on the hook to review :)
Comment 38 joshruehlig 2016-08-10 17:03:25 UTC
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?

Comment 39 commit-hook freebsd_committer 2016-08-22 13:33:28 UTC
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

  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

Comment 40 joshruehlig 2016-08-22 17:02:22 UTC
Awesome thanks for everyone's work on this! The fixes are in, so I assume it is ready for the version update then?
Comment 41 Jeremie Le Hen freebsd_committer 2016-08-22 20:33:15 UTC
Yes, I'm in the process of getting the approval.
Comment 42 commit-hook freebsd_committer 2016-08-22 21:15:25 UTC
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

  Update to version 6.0.

  PR:		210608
  Submitted by:	joshruehlig@gmail.com
  Approved by:	brd

Comment 43 Jeremie Le Hen freebsd_committer 2016-08-22 21:16:10 UTC
Finally updated!  Thanks for the work and sorry for the long time it took.
Comment 44 joshruehlig 2016-08-22 21:31:12 UTC
Sweet, thanks for the persistence.