Bug 4087

Thanks for the patch.

1.23.x is quite a big change from 1.22.x, and likely also contains a few new bugs (as is shown by the recent 1.23.5 release) so it likely is better to let the dust settle first a bit.

Also I'm not 100% sure that 1.23.x is an ABI compatible drop in for 1.22.x, this is something which I will need to check.

Note I'll prepare an update to 1.23.x for at least rawhide eventually but this may take some time.

Just to mention that we also have DavidVa rpm spec for Fedora in UnitedRPMs :)

https://github.com/UnitedRPMs/mpg123/blob/master/mpg123.spec

in https://pkgs.rpmfusion.org/cgit/free/mpg123.git/ we got the patch mention here and update to 1.23.4 , should we build it for rawhide ? 
and for F24 ?

https://pkgs.rpmfusion.org/cgit/free/mpg123.git/commit/?id=2c1d777d9c3980dafb83ed8ba15dbc7615283486

with rename libmpg123 to mpg123-libs, mpg123 stopped provide libmpg123-devel , what should I do ? 
I think mpg123 should provide old package name , like I did in mpv [1],  
at least. I'm going commit it in rawhide soon ...

-%package -n libmpv
+%package libs
 Summary: Dynamic library for Mpv frontends 
+Provides: libmpv = %{version}-%{release}
+Obsoletes: libmpv < %{version}-%{release}


Packages that Buildrequires libmpg123-devel :  
dnf repoquery --arch=src --disablerepo='*'  --enablerepo='rpmfusion*source' --whatrequires libmpg123-devel --qf="%{name} %{version} %{release} %{repoid}"

gstreamer1-plugins-bad-freeworld
mplayer
openmw
qmmp-plugin-pack-freeworld
terminatorX

I was trying compile qmmp-plugin-pack-freeworld in rawhide .
After we should replace buildrequires in each package ? 
Thanks

Hi,

I see that 1.23.6 already has been built for F25, thanks. As said since this is a big update I believe we should stay with 1.22.x for F24 for now.

Igor, do you want to take over rpmfusion mpg123 maintenance in general ?

There are still 2 issues wrt alternatives:

Now that we've stopped using alternatives we are leaving 3 dead symlinks behind in /etc/alternatives:

/etc/alternatives/mpg321_binlink
/etc/alternatives/mpg321_manlink
/etc/alternatives/mpg321_mp3cmdline

I suggest adding a %post which does "rm -f" on these.

While checking this I noticed that there also is a :

/etc/alternatives/mp3-cmdline

Which is put there by the rpmfusion madplay pkg and the mpg123 pkg used to provide a /usr/bin/mp3-cmdline alternative as well. So maybe we should start providing that again (see the madplay spec file to make sure we integrate properly), or drop the mp3-cmdline alternative from the madplay spec as well ?

Regards,

Hans

(In reply to comment #5)
> Hi,
> 
> I see that 1.23.6 already has been built for F25, thanks. As said since this is
> a big update I believe we should stay with 1.22.x for F24 for now.
> 
> Igor, do you want to take over rpmfusion mpg123 maintenance in general ?
yes, but PkgDB didn't allow me to request ACLs at that time.
> 
> There are still 2 issues wrt alternatives:
> 
> Now that we've stopped using alternatives we are leaving 3 dead symlinks behind
> in /etc/alternatives:
> 
> /etc/alternatives/mpg321_binlink
> /etc/alternatives/mpg321_manlink
> /etc/alternatives/mpg321_mp3cmdline
> 
> I suggest adding a %post which does "rm -f" on these.
not sure if it's good idea to do...
> 
> While checking this I noticed that there also is a :
> 
> /etc/alternatives/mp3-cmdline
> 
> Which is put there by the rpmfusion madplay pkg and the mpg123 pkg used to
> provide a /usr/bin/mp3-cmdline alternative as well. So maybe we should start
> providing that again (see the madplay spec file to make sure we integrate
> properly), or drop the mp3-cmdline alternative from the madplay spec as well ?
I will check it.
> 
> Regards,
> 
> Hans

(In reply to comment #6)
> (In reply to comment #5)
> > Now that we've stopped using alternatives we are leaving 3 dead symlinks behind
> > in /etc/alternatives:
> > 
> > /etc/alternatives/mpg321_binlink
> > /etc/alternatives/mpg321_manlink
> > /etc/alternatives/mpg321_mp3cmdline
> > 
> > I suggest adding a %post which does "rm -f" on these.
> not sure if it's good idea to do...

The package put those files there (by calling the alternatives command from %post) so now that we no longer do that and no longer use those files (/usr/bin/mpg123 used to be a symlink to /etc/alternatives/mpg321_binlink) we should remove them.

Hello ! 

I could build qmmp-plugin-pack-freeworld on rawhide without modification .

After we should replace buildrequires ( from libmpg123-devel to mpg123-libs-devel)  in each package ? I or should we wait one or two releases ? 

Third point, you may change owners like in https://bugzilla.rpmfusion.org/show_bug.cgi?id=4006

one better example to change / modify owner of one package : 
https://bugzilla.rpmfusion.org/show_bug.cgi?id=2608

Created attachment 1597 [details]
Readd plugins-extras and review

Patch for review , I don't know if you remove plugins-extras intentionally.

* Thu Aug 04 2016 Sérgio Basto <sergio@serjux.com> - 1.23.6-3
- Readd plugins-extras
- Add scripts to /usr/libexec/mpg123/
- configure with --with-module-suffix=.so 

I add my review based on DavidVa work https://github.com/UnitedRPMs/mpg123/blob/master/mpg123.spec , at the end, I just found that we miss plugins-extras sub-package ... And I add scripts dir, looks cool stuff.

(In reply to comment #10)
> Created attachment 1597 [details]

I will go commit this patch, in new few hour ok ? 

Summary of changes: 

- Readd plugins-extras
- Add contrib scripts to /usr/libexec/mpg123/
- configure with --with-module-suffix=.so

(In reply to comment #11)
> (In reply to comment #10)
> > Created attachment 1597 [details]
> 
> I will go commit this patch, in new few hour ok ? 
> 
> Summary of changes: 
> 
> - Readd plugins-extras
> - Add contrib scripts to /usr/libexec/mpg123/
> - configure with --with-module-suffix=.so

I don't agree. Don't push yet

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Created attachment 1597 [details]
> > 
> > I will go commit this patch, in new few hour ok ? 
> > 
> > Summary of changes: 
> > 
> > - Readd plugins-extras
> > - Add contrib scripts to /usr/libexec/mpg123/
> > - configure with --with-module-suffix=.so
> 
> I don't agree. Don't push yet

what points you don't agree 1, 2 or 3 ?

Created attachment 1600 [details]
0001-configure-with-with-module-suffix-.so.patch

Created attachment 1601 [details]
0002-Readd-plugins-extras.patch

Created attachment 1602 [details]
0003-Add-scripts-to-usr-libexec-mpg123.patch

The 3 proposed modifications, which one you don't approve ? 

Thanks

Comment on attachment 1600 [details]
0001-configure-with-with-module-suffix-.so.patch

* .so is already by default there.
* you dropped all external plugins here.

I'm not going to push this patch.

Comment on attachment 1601 [details]
0002-Readd-plugins-extras.patch

Though I agree to enable OpenAL plugin (should be in different subpackage), others doesn't make sense.

SDL1 is deprecated, NAS is some shit from 80's.
Also looks like you copy-pasted things from old spec which doesn't have proper requirements and etc.

not going to accept in current state.

Comment on attachment 1602 [details]
0003-Add-scripts-to-usr-libexec-mpg123.patch

Why would we do that?

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Now that we've stopped using alternatives we are leaving 3 dead symlinks behind
> > > in /etc/alternatives:
> > > 
> > > /etc/alternatives/mpg321_binlink
> > > /etc/alternatives/mpg321_manlink
> > > /etc/alternatives/mpg321_mp3cmdline
> > > 
> > > I suggest adding a %post which does "rm -f" on these.
> > not sure if it's good idea to do...
> 
> The package put those files there (by calling the alternatives command from
> %post) so now that we no longer do that and no longer use those files
> (/usr/bin/mpg123 used to be a symlink to /etc/alternatives/mpg321_binlink) we
> should remove them.
argh yes, I will add it.

(In reply to comment #17)
> Comment on attachment 1600 [details]
> 0001-configure-with-with-module-suffix-.so.patch
> 
> * .so is already by default there.

no default is la IIRC 

> * you dropped all external plugins here.

IIRC they are enable by default , don't need add it in configure ...

> I'm not going to push this patch.
petty

(In reply to comment #19)
> Comment on attachment 1602 [details]
> 0003-Add-scripts-to-usr-libexec-mpg123.patch
> 
> Why would we do that?

cd mpg123-1.23.6/
ls scripts/
benchmark-cpu.pl  conplay  mpg123info  tag_lyrics.py

add these useful scripts to libexec

Package is retired as it's been moved to fedora repo