media-libs/libgroove: new ebuild #22

diogocp · 2015-08-19T18:45:06Z

A streaming audio processing library (andrewrk/libgroove).
Gentoo-Bug: 558190

hasufell · 2015-08-19T19:33:47Z

media-libs/libgroove/libgroove-4.3.0.ebuild

+KEYWORDS="~amd64"
+IUSE="+chromaprint +loudness +sound"
+
+DEPEND="media-video/ffmpeg


no USE flags needed for ffmpeg? sure?

does it not work with media-video/libav?

Upstream has some suggested flags, but from my tests they are all optional runtime dependencies.

Support for libav was recently removed, but it looks like this version still supports it. I think the default should be ffmpeg, to minimize problems when upgrading to the next version, unless @andrewrk thinks libav is better for 4.3.0.

ffmpeg should be fine for 4.3.0. The next release of libgroove will officially only support ffmpeg.

hasufell · 2015-08-19T19:39:19Z

You should reference this bug report https://bugs.gentoo.org/show_bug.cgi?id=558190 somewhere in your commit message. Since there is no consensus on how to do that, just do something (e.g. wrt bug 558190 in the summary or Gentoo-Bug: 558190 in the multiline commit description or whatever)

diogocp · 2015-08-19T22:51:13Z

OK, I think that's all. Please take another look.

hasufell · 2015-08-19T23:29:53Z

The build system unfortunately uses -Werror. We will have to fix/patch that, also see https://devmanual.gentoo.org/ebuild-writing/common-mistakes/index.html under -Werror compiler flag not removed

Here are the offending lines https://github.com/andrewrk/libgroove/blob/master/CMakeLists.txt#L126 ...which also reveal -g in the compiler flags. That has to go out as well.

hasufell · 2015-08-19T23:31:06Z

We should probably provide a static-libs USE flag and rm the foo.a files when it is turned off in src_install.

hasufell · 2015-08-19T23:32:18Z

media-libs/libgroove/libgroove-4.3.0.ebuild

+SRC_URI="https://github.com/andrewrk/libgroove/archive/${PV}.tar.gz -> ${P}.tar.gz"
+
+LICENSE="MIT"
+SLOT="0"


We should probably provide a SUBSLOT, in this case SLOT="0/4" also see https://wiki.gentoo.org/wiki/Sub-slots_and_Slot-Operators

I see that it installs both libgroove.so.4 and libgroove.so.4.3.0. How do we decide between 0/4 and 0/4.3.0?

# objdump -p /usr/lib64/libgrooveplayer.so.4.3.0 | grep SONAME SONAME libgrooveplayer.so.4

This reflects ABI compatibility.

Interesting, thanks.

andrewrk · 2015-08-19T23:40:08Z

The build system unfortunately uses -Werror. We will have to fix/patch that, also see https://devmanual.gentoo.org/ebuild-writing/common-mistakes/index.html under -Werror compiler flag not removed

Here are the offending lines https://github.com/andrewrk/libgroove/blob/master/CMakeLists.txt#L126 ...which also reveal -g in the compiler flags. That has to go out as well.

Upstream maintainer here. My goal is to be friendly to package maintainers like you. I believe that cmake will handle the -g flag correctly depending on the build type, so I'm happy to simply remove that upstream.

As for -Werror, I do want that on for development, but is there a way I could prevent gentoo from having to patch it away? Perhaps if I enabled -Werror only for debug builds and not for release builds?

hasufell · 2015-08-19T23:46:12Z

Perhaps if I enabled -Werror only for debug builds and not for release builds?

Gentoo hacks it's own cmake build type in (although we sometimes fall back to release build type if that causes trouble), so yes, that would probably work.

andrewrk · 2015-08-20T00:03:38Z

ok. I made this change upstream: andrewrk/libgroove@0b619fa and andrewrk/libgroove@856c260

If it would really help to make a new release I could do that. Otherwise I'd rather wait until some other stuff I'm presently working on is done to make a new release.

hasufell · 2015-08-20T00:09:01Z

should be fine to use the patch https://github.com/andrewrk/libgroove/commit/0b619fa3b02ab55593f148bfd867aec2c34f66c1.patch

andrewrk · 2015-08-20T00:10:28Z

don't forget the second commit there. Sorry, I missed the -Werror and -pedantic in the example flags at first.

hasufell · 2015-08-20T00:14:58Z

https://github.com/andrewrk/libgroove/compare/44aa5db96cfe45e56fa1f7d762fb032ca178a35c...856c26013e4356d4fc078da980083bc4d002771d.diff

diogocp · 2015-08-20T01:22:29Z

I had to make another patch because that one failed. :/

hasufell · 2015-08-20T09:49:08Z

media-libs/libgroove/libgroove-4.3.0.ebuild

+
+src_install() {
+	cmake-utils_src_install
+	use static-libs || rm "${D}"/usr/lib/*.a


nah, this needs improvement, it should be

use static-libs || { rm "${ED%/}"/usr/$(get_libdir)/*.a || die "failed to remove static libs!" ;}

and then we have to inherit multilib cmake-utils

Or inherit cmake-utils multilib? I noticed this discussion before, but have not seen any guidelines.

I personally order them by priority, to minimalize the chance that something weird happens when exported phases change. So something like inherit <utility.eclass> <heavy-phase-export.eclass>. But I don't really care here, as long as the end result works.

It's failing because get_libdir returns lib64 but they are installed in /usr/lib.

That's a bug then that needs to be fixed/patched.

andrewrk/libgroove#100

hasufell · 2015-08-20T20:51:56Z

excellent, I think this is good to merge, almost..

@diogocp could you rebase your libgroove against latest gentoo master?
Something like:

git pull --rebase=preserve https://anongit.gentoo.org/git/repo/gentoo.git master
git push -f origin libgroove

diogocp · 2015-08-20T20:58:00Z

Done

Gentoo-Bug: 558190

Adds media-libs/libgroove. Reviewed-by: Julian Ospald <hasufell@gentoo.org> Suggested-by: Diogo Pereira Github-PR: #22 Gentoo-Bug: 558190

diogocp · 2015-08-20T21:04:22Z

Thanks!

diogocp mentioned this pull request Aug 19, 2015

Gentoo ebuild andrewrk/libgroove#99

Closed

hasufell reviewed Aug 19, 2015
View reviewed changes

diogocp force-pushed the libgroove branch from 6ff8fec to 32c25d5 Compare August 19, 2015 21:01

hasufell reviewed Aug 19, 2015
View reviewed changes

diogocp force-pushed the libgroove branch from 32c25d5 to 76947d5 Compare August 20, 2015 01:21

hasufell reviewed Aug 20, 2015
View reviewed changes

hasufell mentioned this pull request Aug 20, 2015

CMake: make installation directories more configurable andrewrk/libgroove#100

Closed

diogocp force-pushed the libgroove branch from b64da81 to e26cdd0 Compare August 20, 2015 14:22

diogocp force-pushed the libgroove branch from e26cdd0 to e62500c Compare August 20, 2015 20:57

media-libs/libgroove: new ebuild

99a5d23

Gentoo-Bug: 558190

diogocp force-pushed the libgroove branch from e62500c to 99a5d23 Compare August 20, 2015 20:58

mgorny merged commit 99a5d23 into gentoo:master Aug 20, 2015

mgorny pushed a commit that referenced this pull request Aug 20, 2015

Merge branch 'libgroove' of github.com:diogocp/gentoo

04ada5c

Adds media-libs/libgroove. Reviewed-by: Julian Ospald <hasufell@gentoo.org> Suggested-by: Diogo Pereira Github-PR: #22 Gentoo-Bug: 558190

diogocp deleted the libgroove branch August 20, 2015 21:04

hasufell mentioned this pull request Aug 21, 2015

CMake: make installation directories more configurable andrewrk/libgroove#102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

media-libs/libgroove: new ebuild #22

media-libs/libgroove: new ebuild #22

diogocp commented Aug 19, 2015

hasufell Aug 19, 2015

diogocp Aug 19, 2015

andrewrk Aug 19, 2015

hasufell commented Aug 19, 2015

diogocp commented Aug 19, 2015

hasufell commented Aug 19, 2015

hasufell commented Aug 19, 2015

hasufell Aug 19, 2015

diogocp Aug 19, 2015

hasufell Aug 20, 2015

diogocp Aug 20, 2015

andrewrk commented Aug 19, 2015

hasufell commented Aug 19, 2015

andrewrk commented Aug 20, 2015

hasufell commented Aug 20, 2015

andrewrk commented Aug 20, 2015

hasufell commented Aug 20, 2015

diogocp commented Aug 20, 2015

hasufell Aug 20, 2015

diogocp Aug 20, 2015

hasufell Aug 20, 2015

diogocp Aug 20, 2015

hasufell Aug 20, 2015

hasufell Aug 20, 2015

hasufell commented Aug 20, 2015

diogocp commented Aug 20, 2015

diogocp commented Aug 20, 2015

media-libs/libgroove: new ebuild #22

media-libs/libgroove: new ebuild #22

Conversation

diogocp commented Aug 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasufell commented Aug 19, 2015

diogocp commented Aug 19, 2015

hasufell commented Aug 19, 2015

hasufell commented Aug 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewrk commented Aug 19, 2015

hasufell commented Aug 19, 2015

andrewrk commented Aug 20, 2015

hasufell commented Aug 20, 2015

andrewrk commented Aug 20, 2015

hasufell commented Aug 20, 2015

diogocp commented Aug 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hasufell commented Aug 20, 2015

diogocp commented Aug 20, 2015

diogocp commented Aug 20, 2015