Bug 246342 - [NEW PORT] audio/strawberry: Music Player
Summary: [NEW PORT] audio/strawberry: Music Player
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Matthias Andree
URL: https://www.strawberrymusicplayer.org/
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-09 22:56 UTC by Daniel Menelkir
Modified: 2020-05-17 20:25 UTC (History)
3 users (show)

See Also:


Attachments
strawberry.diff (3.79 KB, patch)
2020-05-09 22:56 UTC, Daniel Menelkir
no flags Details | Diff
strawberry-v2.diff (4.77 KB, patch)
2020-05-10 07:22 UTC, Daniel Menelkir
no flags Details | Diff
Strawberry WIP #3 (4.94 KB, patch)
2020-05-11 08:41 UTC, daniel.engberg.lists
no flags Details | Diff
strawberry-v3.diff (5.77 KB, patch)
2020-05-11 09:26 UTC, Daniel Menelkir
no flags Details | Diff
strawberry-v4.diff (7.10 KB, patch)
2020-05-13 08:12 UTC, Daniel Menelkir
no flags Details | Diff
strawberry-v5.diff (5.77 KB, patch)
2020-05-16 15:36 UTC, Daniel Menelkir
no flags Details | Diff
strawberry-v6.diff (18.44 KB, patch)
2020-05-16 18:50 UTC, Daniel Menelkir
no flags Details | Diff
diff between Makefile v6 and committed version, for submitter perusal (4.13 KB, patch)
2020-05-17 20:25 UTC, Matthias Andree
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Menelkir 2020-05-09 22:56:00 UTC
Created attachment 214326 [details]
strawberry.diff

Strawberry is a music player and music collection organizer. It is a fork of Clementine released in 2018 aimed at music collectors and audiophiles.
Comment 1 daniel.engberg.lists 2020-05-10 06:40:25 UTC
Hi,

CATEGORIES should be audio not multimedia

According to 
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-distfiles.html you shouldn't use both MASTER_SITES and USE_GITHUB.
Is there anything wrong using the provided archive by upstream?

https://www.strawberrymusicplayer.org/ --> Download - Source --> https://files.jkvinge.net/packages/strawberry/strawberry-0.6.10.tar.xz

I might be interpreting the handbook incorrectly but I think you shouldn't include xorg and gnome in USES
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/using-gnome.html
There's no example for xorg but I assume it follows the same concept.

Having a luck look at the Makefile you seem to be missing a few libs and have a few redundant?

Missing: Boost, taglib (should use ports not bundled), pkgconfig etc

https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L88
https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L89
https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L203

VLC isn't needed and more or less unsupported
https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L123
https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L442

You don't need two audio backends, in that case Pulseaudio seems to be the better choice as alsa-lib is very old in ports (it seems like you can choose?).

There are more so you need to check that part again, preferably all optional libraries should be optional via menu options.
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-options.html

Fails using Poudriere due to missing libs

Best regards,
Daniel
Comment 2 Daniel Menelkir 2020-05-10 07:22:22 UTC
Created attachment 214334 [details]
strawberry-v2.diff

(In reply to daniel.engberg.lists from comment #1)

> CATEGORIES should be audio not multimedia

Fixed.

> Is there anything wrong using the provided archive by upstream?

The link from the main page was giving me some errors, timeout, etc. While from github was straight-forward. I've changed to the main archive.

> I might be interpreting the handbook incorrectly but I think you shouldn't include xorg and gnome in USES

I was following the errors in the end of the build of the port.
Removing those lines, gave me some lines of errors regarding I'm missing libs. Here:

Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libgio-2.0.so.0 from devel/glib20 but it is not declared as a dependency
Warning: you need USE_GNOME+=glib20
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libSM.so.6 from x11/libSM but it is not declared as a dependency
Warning: you need USE_XORG+=sm
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libICE.so.6 from x11/libICE but it is not declared as a dependency
Warning: you need USE_XORG+=ice
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libX11.so.6 from x11/libX11 but it is not declared as a dependency
Warning: you need USE_XORG+=x11
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libXext.so.6 from x11/libXext but it is not declared as a dependency
Warning: you need USE_XORG+=xext
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libxcb.so.1 from x11/libxcb but it is not declared as a dependency
Warning: you need USE_XORG+=xcb
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libvlc.so.5 from multimedia/vlc but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libvlc.so:multimedia/vlc
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libtag.so.1 from audio/taglib but it is not declared as a dependency
Warning: you need LIB_DEPENDS+=libtag.so:audio/taglib
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libglib-2.0.so.0 from devel/glib20 but it is not declared as a dependency
Warning: you need USE_GNOME+=glib20
Error: /usr/local/bin/strawberry is linked to /usr/local/lib/libgobject-2.0.so.0 from devel/glib20 but it is not declared as a dependency
Warning: you need USE_GNOME+=glib20

> Having a luck look at the Makefile you seem to be missing a few libs and have a few redundant?
> Missing: Boost, taglib (should use ports not bundled), pkgconfig etc

Fixed (those you noted). 

> VLC isn't needed and more or less unsupported
> https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L123
> https://github.com/strawberrymusicplayer/strawberry/blob/0.6.10/CMakeLists.txt#L442

Same as I said above. 

> You don't need two audio backends, in that case Pulseaudio seems to be the better choice as alsa-lib is very old in ports (it seems like you can choose?).

Yes, you can select inside the application itself.

> There are more so you need to check that part again, preferably all optional libraries should be optional via menu options.

Most things strawberry uses, it's selectable inside (like audio, etc). I don't think is really needed (for now). 

> Fails using Poudriere due to missing libs

Can I take a look on the log to have some clues if I need any other libs than the included ones in diff?
Comment 3 daniel.engberg.lists 2020-05-10 08:43:48 UTC
(In reply to Daniel Menelkir from comment #2)

Hi,

I think you misread, USES= not USE_XXXX=

Poudriere stops immediately as Boost isn't found during the configuration stage so there isn't much to show.

Instead of patching files it seems like you can pass switches instead

Examples of switches:
https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/strawberry#n27
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=strawberry-git

See:
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/building.html
6.5.4. Using cmake

You also needs to set these anyway as by default strawberry will try to pull in everything it supports which will mismatch with specified dependencies and potentially break the port as it may install more files etc (haven't specifically checked in this case) and case you use ports.

While you can change backends etc I think you should try to limit the usage of dependencies instead of pulling in everything "just because" but that's up to you.
Comment 4 daniel.engberg.lists 2020-05-11 08:41:23 UTC
Created attachment 214370 [details]
Strawberry WIP #3

Make Strawberry more modular, optional libs are now optional
Doesn't pull in GCC :-)
Poudriere testport OK FreeBSD 12.1-RELEASE (AMD64)

portlint -C complains about
WARN: Makefile: "LIB_DEPENDS" has to appear earlier.
WARN: Makefile: "USES" has to appear earlier.
Not sure how to fix that, if it's possible

make test fails (with test option enabled), requires X running?

qt.qpa.xcb: could not connect to display
qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Available platform plugins are: bsdfb, minimal, offscreen, vnc, xcb.

Abort trap (core dumped)
ninja: build stopped: subcommand failed.
*** Error code 1

*** NOT RUN-TESTED ***

I apologize beforehand for some earlier advices as Poudriere and Porter's Handbook seems to disagree in some cases, when possible I've followed what Poudriere suggested.
Comment 5 daniel.engberg.lists 2020-05-11 08:50:01 UTC
Hi Daniel,

I hacked on this a bit and WIP #3 patch should get you a bit further, it's not perfect but I think it'll help you a bit further at least. Feel free to to use it if you want to.

It needs run-testing and I haven't tested the options but they should work (tm), 
APPLEDEV option might need more depends defined.

If you get stuck I'd suggest that you try the mailing list (freebsd-ports) and/or the irc channels. https://wiki.freebsd.org/IRC/Channels

Best regards,
Daniel
Comment 6 Daniel Menelkir 2020-05-11 08:52:08 UTC
(In reply to daniel.engberg.lists from comment #4)

Hi. I was writing a reply right now. :)
I'm doing the proper fixes. Thanks

> I think you misread, USES= not USE_XXXX=
Yes but I have this warning when I try to USE_XORG alone, for example:

Using USE_XORG alone is deprecated, please use USES=xorg

> Instead of patching files it seems like you can pass switches instead
I'm changing this behaviour. 

> WARN: Makefile: "LIB_DEPENDS" has to appear earlier.
> WARN: Makefile: "USES" has to appear earlier.
Nor me, My "fix" was taking BUILD_DEPENDS after all USEs so at least there's only one warning. Despite the fact there's a lot of ports that have the same behavior, so I thin it's "normal" or maybe something that should be implemented in portlint and related tools?

> make test fails (with test option enabled), requires X running?
Well I'm running X while doing this, maybe it's a faulty test behavior from the software itself?

>qt.qpa.plugin: Could not load the Qt platform plugin "xcb" in "" even though it was found.
I have a xcb on USE_XORG, maybe is what is missing? 

I've noticed that it'll forcibly link to vlc and pulseaudio, no matter what CMAKE_OFF you use, this doesn't occurs if you don't have vlc and pulseaudio installed, so it's a bad behavior from software itself, so for now I'll ignore the erros about missing LIB_DEPENDS about it when you have vlc and pulseaudio installed.
Comment 7 Daniel Menelkir 2020-05-11 08:53:53 UTC
(In reply to daniel.engberg.lists from comment #5)

Oh that's really cool, let me check it.
Comment 8 Daniel Menelkir 2020-05-11 09:26:50 UTC
Created attachment 214371 [details]
strawberry-v3.diff

(In reply to daniel.engberg.lists from comment #5)

I've turned all the options on and it seems to build fine (despite the alsa-lib complaining that isn't declared even with the ALSA option enabled, I don't think it's a big issue, anyways).
I've added the missing linked libs to the APPLEDEV.

Later I'll report to the strawberry devs about adding sndio support, maybe they'll add since sndio is well documented.
Comment 9 daniel.engberg.lists 2020-05-11 09:34:38 UTC
I did a quick test having VLC installed and it doesn't seem to get pulled in (WIP #3 patch)?

 readelf -d /usr/ports/audio/strawberry/work/stage/usr/local/bin/strawberry | grep NEEDED
 0x0000000000000001 NEEDED               Shared library: [libexecinfo.so.1]
 0x0000000000000001 NEEDED               Shared library: [libprotobuf.so.22]
 0x0000000000000001 NEEDED               Shared library: [libQt5Concurrent.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5Sql.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5DBus.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5X11Extras.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5Widgets.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5Gui.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5Network.so.5]
 0x0000000000000001 NEEDED               Shared library: [libQt5Core.so.5]
 0x0000000000000001 NEEDED               Shared library: [libglib-2.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libintl.so.8]
 0x0000000000000001 NEEDED               Shared library: [libgio-2.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgobject-2.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgnutls.so.30]
 0x0000000000000001 NEEDED               Shared library: [libchromaprint.so.1]
 0x0000000000000001 NEEDED               Shared library: [libsqlite3.so.0]
 0x0000000000000001 NEEDED               Shared library: [libSM.so.6]
 0x0000000000000001 NEEDED               Shared library: [libICE.so.6]
 0x0000000000000001 NEEDED               Shared library: [libX11.so.6]
 0x0000000000000001 NEEDED               Shared library: [libXext.so.6]
 0x0000000000000001 NEEDED               Shared library: [libxcb.so.1]
 0x0000000000000001 NEEDED               Shared library: [libgstreamer-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgstbase-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgstaudio-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgstapp-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgsttag-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgstpbutils-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libgstvideo-1.0.so.0]
 0x0000000000000001 NEEDED               Shared library: [libpulse.so.0]
 0x0000000000000001 NEEDED               Shared library: [libthr.so.3]
 0x0000000000000001 NEEDED               Shared library: [libtag.so.1]
 0x0000000000000001 NEEDED               Shared library: [libz.so.6]
 0x0000000000000001 NEEDED               Shared library: [libc++.so.1]
 0x0000000000000001 NEEDED               Shared library: [libcxxrt.so.1]
 0x0000000000000001 NEEDED               Shared library: [libm.so.5]
 0x0000000000000001 NEEDED               Shared library: [libgcc_s.so.1]
 0x0000000000000001 NEEDED               Shared library: [libc.so.7]

# pkg info |grep vlc
vlc-3.0.10_1,4                 Qt based multimedia player and streaming server

It does however seem to pull in chromaprint irregardless of option :/

"Chromaprint (Tag fetching from Musicbrainz) (disabled in CMake config)"

The reason I can't do any testing is because I only have a headless box ;-)
Comment 10 Daniel Menelkir 2020-05-11 09:37:06 UTC
(In reply to daniel.engberg.lists from comment #9)

Chromaprint is used by musicbrainz, that is used on that huge quantity of cloud server options (clementine have the same options), maybe something else under the hood is pushing this too.
Comment 11 daniel.engberg.lists 2020-05-11 10:23:44 UTC
Even if it is it's still a bug and we should report it upstream, can you do that?
I'd also think there needs to be a better description for moodbar as I have no idea what it actually is but I guess it's the spectrum visualizer given that it depends on fftw?

Apart from the chromaprint issue (which should be patched unless upstream release a bugfix release promptly) I think this port is ready for reviewing.
Comment 12 Daniel Menelkir 2020-05-11 10:33:32 UTC
(In reply to daniel.engberg.lists from comment #11)

I did it a report about sndio, hope it gets some attention.

https://github.com/strawberrymusicplayer/strawberry/issues/431

The moodbar is like a spectrum visualizer, but fixed. Like.. it's a line with a static spectrum analyzer, with a cursor running through them. Not sure if I've been clear.

> Apart from the chromaprint issue (which should be patched unless upstream release a bugfix release promptly) I think this port is ready for reviewing.

I'm not sure how to report or patch this chromaprint properly, because I'm pretty sure that you'll break something more in those cloud related support, since most of them would use AudioID fingerprint (maybe even the audioscrobblers use it?).
Comment 13 daniel.engberg.lists 2020-05-11 10:56:14 UTC
Wikipedia offered some information about it.
https://en.wikipedia.org/wiki/Moodbar
I guess moodbar DESC can stay as it is given the above.

Strawberry does build without chromaprint (my Poudriere run didn't include chromaprint) however I haven't looked at the code what it actually provides. As for bug report just report that defining -DENABLE_CHROMAPRINT (-DENABLE_CHROMAPRINT:BOOL=false) gets ignored if lib is available and gets pulled in anyway.

One option is to unconditionally depend on chromaprint however perferably we should wait on upstream to look at it first.
Comment 14 Daniel Menelkir 2020-05-11 11:08:48 UTC
(In reply to daniel.engberg.lists from comment #13)

Done.

https://github.com/strawberrymusicplayer/strawberry/issues/432
Comment 15 Daniel Menelkir 2020-05-12 20:57:28 UTC
(In reply to Daniel Menelkir from comment #14)

The chromaprint issue was fixed upstream. 

https://github.com/strawberrymusicplayer/strawberry/commit/43a47f33acae07d7d63054fcca0dcdd6fcc64287
Comment 16 daniel.engberg.lists 2020-05-13 05:42:27 UTC
Great, just import at using make makepatch
https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/slow-patch.html

Does make test work on your end?
Comment 17 Daniel Menelkir 2020-05-13 08:10:53 UTC
(In reply to daniel.engberg.lists from comment #16)

It builds fine, but the make test generates some internal errors. Not sure how bad it is:

05:09:17.388 WARN  unknown                          QSqlDatabasePrivate::removeDatabase: connection '12_thread_34540148848' is still in use, all queries will cease to work. 
[  FAILED  ] CollectionModelTest.RemoveEmptyArtists (5 ms)
[----------] 12 tests from CollectionModelTest (75 ms total)

[----------] Global test environment tear-down
[==========] 12 tests from 1 test suite ran. (75 ms total)
[  PASSED  ] 2 tests.
[  FAILED  ] 10 tests, listed below:
[  FAILED  ] CollectionModelTest.WithInitialArtists
[  FAILED  ] CollectionModelTest.CompilationAlbums
[  FAILED  ] CollectionModelTest.NumericHeaders
[  FAILED  ] CollectionModelTest.MixedCaseHeaders
[  FAILED  ] CollectionModelTest.UnknownArtists
[  FAILED  ] CollectionModelTest.UnknownAlbums
[  FAILED  ] CollectionModelTest.VariousArtistSongs
[  FAILED  ] CollectionModelTest.RemoveSongsLazyLoaded
[  FAILED  ] CollectionModelTest.RemoveEmptyAlbums
[  FAILED  ] CollectionModelTest.RemoveEmptyArtists

10 FAILED TESTS
ninja: build stopped: subcommand failed.
Comment 18 Daniel Menelkir 2020-05-13 08:12:30 UTC
Created attachment 214444 [details]
strawberry-v4.diff

New diff with the chromaprint fix from upstream.
Comment 19 daniel.engberg.lists 2020-05-13 21:15:38 UTC
(In reply to Daniel Menelkir from comment #17)
Hmm... That looks like a bug for upstream
Comment 20 Daniel Menelkir 2020-05-13 21:17:46 UTC
(In reply to daniel.engberg.lists from comment #19)

Yes. It seems to be internal, but I didn't find something related in the software itself. I though it could be something related to the id3 of mp3s, but the id3 are fine. Unless it's related to some of the cloud services, that's the only one thing I didn't had tested because I don't have an account.
Comment 21 Daniel Menelkir 2020-05-16 15:36:47 UTC
Created attachment 214553 [details]
strawberry-v5.diff

Version bump to 0.6.11.
Also, I've created a bug report about the tests: https://github.com/strawberrymusicplayer/strawberry/issues/440
Comment 22 Daniel Menelkir 2020-05-16 18:50:09 UTC
Created attachment 214567 [details]
strawberry-v6.diff

This patch contains the fixes for tests from upstream.
Comment 23 Matthias Andree freebsd_committer 2020-05-17 20:14:24 UTC
Daniel and Daniel,

thank you for your elaborate work on this port.

However, at Daniel Engberg's teasing me I should stop breaking the port, here are my findings (will correct on commit) as of 0.6.11:

- NLS is not optional, src/core/utilities.cpp always includes iconv.h and calls iconv_*(), disabling NLS causes linker error around iconv_() on 12.1-amd64.

- GSTREAMER is not optional, disabling it cause compilation failure in src/core/mainwindow.cpp:229:7: error: member initializer 'transcode_dialog_' does not name a non-static data member or base class
transcode_dialog_([=]() { ...

- AUDIOCD_LIB_DEPENDS= was misspelled ALSA_LIB_DEPENDS=

- "make test" isn't a part of the default build and hence need not be an option, and we can use TEST_DEPENDS=   googletest>=0:devel/googletest to list a package dependency that is checked only if the test is to be run.

Commit coming up.
Comment 24 commit-hook freebsd_committer 2020-05-17 20:20:14 UTC
A commit references this bug:

Author: mandree
Date: Sun May 17 20:19:12 UTC 2020
New revision: 535673
URL: https://svnweb.freebsd.org/changeset/ports/535673

Log:
  audio/strawberry: new port of Qt5-based music player

  ...which is descended from clementine.

  PR:		246342
  Submitted by:	Daniel Menelkir <menelkir@itroll.org> (maintainer)

Changes:
  head/audio/Makefile
  head/audio/strawberry/
  head/audio/strawberry/Makefile
  head/audio/strawberry/distinfo
  head/audio/strawberry/files/
  head/audio/strawberry/files/patch-src_collection_collectionmodel.cpp
  head/audio/strawberry/files/patch-src_organise_organiseformat.cpp
  head/audio/strawberry/files/patch-tests_CMakeLists.txt
  head/audio/strawberry/files/patch-tests_src_collectionmodel__test.cpp
  head/audio/strawberry/files/patch-tests_src_organiseformat__test.cpp
  head/audio/strawberry/files/patch-tests_src_playlist__test.cpp
  head/audio/strawberry/pkg-descr
  head/audio/strawberry/pkg-plist
Comment 25 Matthias Andree freebsd_committer 2020-05-17 20:24:24 UTC
Note that some CMAKE_* options were misnamed for 0.6.11:

ENABLE_PULSE -> ENABLE_LIBPULSE
USE_GSTREAMER -> ENABLE_GSTREAMER
USE_TRANSLATIONS -> ENABLE_TRANSLATIONS

Note FreeBSD has a set of default _DESC fields (NLS_DESC), see /usr/ports/Mk/bsd.options.desc.mk

I have added github.com to MASTER_SITES first because files.jkvinge.net is slow to download and if 100 sites hammer it we're not getting anything from Jonas's site.

Removed blank line between BUILD_DEPENDS and LIB_DEPENDS to appease portlint.

Removed GSTREAMER and NLS options for now and folded options up.
This did away with all the .if stuff past .include <bsd.port.options.mk>

Removed TEST option and replaced TEST_LIB_DEPENDS by TEST_DEPENDS on a package (not a lib).
Comment 26 Matthias Andree freebsd_committer 2020-05-17 20:25:21 UTC
Created attachment 214588 [details]
diff between Makefile v6 and committed version, for submitter perusal

This is a difference set between what was submitted and what I've committed.
Please consider taking my findings upstream.