Bug 1394147 - Review Request: mpg123 - mp3 audio playback library
Summary: Review Request: mpg123 - mp3 audio playback library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matthew Miller
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 625481 (view as bug list)
Depends On:
Blocks: 1394148 1394582 1395711 1395861 1529207
TreeView+ depends on / blocked
 
Reported: 2016-11-11 08:05 UTC by Wim Taymans
Modified: 2017-12-27 06:29 UTC (History)
12 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2016-11-19 21:16:58 UTC
Type: ---
Embargoed:
mattdm: fedora-review+


Attachments (Terms of Use)

Description Wim Taymans 2016-11-11 08:05:14 UTC
Spec URL: https://people.freedesktop.org/~wtay/SPECS/mpg123.spec
SRPM URL: https://people.freedesktop.org/~wtay/SRPMS/mpg123-1.23.8-1.fc24.src.rpm
Description: Real time MPEG 1.0/2.0/2.5 audio player/decoder for layers 1, 2 and 3 (most commonly MPEG 1.0 layer 3 aka MP3), as well as re-usable decoding and output libraries.
Fedora Account System Username: wtaymans

This is the package taken from rpmfusion and updated to the latest upstream version.

Comment 1 Matthew Miller 2016-11-11 11:19:46 UTC
Not a blocker, but I'm curious about the plugin Recommends:. I have a default Fedora Workstation setup which of course uses pulseaudio. I installed the base rpm and libs but not the plugin, and the command-line tool plays back just fine — what does the plugin gain me?

Comment 2 Matthias Runge 2016-11-11 11:21:57 UTC
nit: typo in plugins-p(l)ulseaudio subpackage.

Comment 3 Neal Gompa 2016-11-11 11:48:29 UTC
@Wim:

Unless the situation has changed recently, you cannot use rich dependencies in Requires, Recommends, or Conflicts until Fedora release engineering tools are updated to handle this without breaking.

CC'ing Dennis Gilmore, who can confirm whether this is still true.

Comment 4 Neal Gompa 2016-11-11 11:56:03 UTC
If it is in fact still the case, you can just flip the relationship to "Supplements" with rich deps in each of the plugin packages. That will work the same way and tools like Koji, Bodhi, pungi, and mash won't choke.

Comment 5 Matthew Miller 2016-11-11 12:07:39 UTC
Also not a blocker, but there are obsolete m4 macros used in autotools. See https://fedorahosted.org/FedoraReview/wiki/AutoTools

Comment 6 Matthew Miller 2016-11-11 12:10:18 UTC
Thanks Neal — good catch. Wim, can you fix that? https://fedoraproject.org/wiki/Packaging:Guidelines#Rich.2FBoolean_dependencies

Otherwise, this looks good to me -- I ran fedora-review and the only issues it shows are due to it being somewhat confused about the devel package being mpg123-libs-devel instead of mpg123-devel. I don't *think* this is an actual problem.

Comment 8 Matthew Miller 2016-11-11 14:28:11 UTC
Neal (or someone) can you comment on the Enhances/Supplements in the updated package?

Comment 9 Neal Gompa 2016-11-11 14:51:37 UTC
The Enhances is pointless, so remove them. You already have a hard Requires on it anyway.

The Supplements look good to me.

Please don't name devel subpackages "libs-devel". That's just asking for trouble because it's inconsistent with how we generally name development subpackages. Please change it to "mpg123-devel".

Comment 10 Matthew Miller 2016-11-11 15:05:05 UTC
Neal, do you think the devel package name is a hard blocker? I agree it would e more consistent. I guess we can do taht with another provides/obsoletes on the existing (slightly odd) name as there is for libmpg123-devel.

Wim, can you fix the "Plulseaudio" typo while we're at it?

Comment 12 Wim Taymans 2016-11-11 15:52:09 UTC
Updated files with Obsoletes: -lib-devel

Comment 13 Matthew Miller 2016-11-11 15:53:10 UTC
Looks great to me. Review accepted.

Comment 14 Gwyn Ciesla 2016-11-11 16:05:30 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/mpg123

Comment 15 Fedora Update System 2016-11-11 17:46:40 UTC
mpg123-1.23.8-3.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-dec08a99bd

Comment 16 Igor Gnatenko 2016-11-12 09:41:26 UTC
(In reply to Neal Gompa from comment #3)
> @Wim:
> 
> Unless the situation has changed recently, you cannot use rich dependencies
> in Requires, Recommends, or Conflicts until Fedora release engineering tools
> are updated to handle this without breaking.
In Recommends you can use it fine.

Comment 17 Neal Gompa 2016-11-12 13:24:53 UTC
(In reply to Igor Gnatenko from comment #16)
> (In reply to Neal Gompa from comment #3)
> > @Wim:
> > 
> > Unless the situation has changed recently, you cannot use rich dependencies
> > in Requires, Recommends, or Conflicts until Fedora release engineering tools
> > are updated to handle this without breaking.
> In Recommends you can use it fine.

No, you cannot use them in Recommends because Yum was adapted to read them, so now that it doesn't ignore them, everything breaks when you put rich dependencies in them.

Comment 18 Greg Bailey 2016-11-12 15:16:26 UTC
Do you have any plans to build this for other branches?

I would be interested in an EPEL7 build, and am willing to help comaintain it if you're interested.

Comment 19 Fedora Update System 2016-11-12 18:23:34 UTC
mpg123-1.23.8-3.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-dec08a99bd

Comment 20 Peter Lemenkov 2016-11-12 20:48:15 UTC
*** Bug 625481 has been marked as a duplicate of this bug. ***

Comment 21 Neal Gompa 2016-11-12 21:27:05 UTC
This has already been approved by Fedora Legal.

Comment 22 Nicolas Chauvet (kwizart) 2016-11-13 09:23:00 UTC
I don't get why Supplements is relevant here ?
You will get mpg123-plugins-pulseaudio as soon as you get pulseaudio. Even if you don't have mpg123. Same for portaudio.

thx for moving the package here.

Comment 23 Fedora Update System 2016-11-19 21:16:58 UTC
mpg123-1.23.8-3.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.