Bug 226425

Summary: Merge Review: sox
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: soxAssignee: Richard Shaw <hobbes1069>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: 23CC: fkluknav, gwync, hhorak, hobbes1069, mattdm, twoerner
Target Milestone: ---Flags: hobbes1069: needinfo-
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-11-11 14:04:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Patch to add modular sox built (need to be approved upstream)
none
Patch to avoid_version of module in sox 14.1.0 none

Description Nobody's working on this, feel free to take it 2007-01-31 21:00:32 UTC
Fedora Merge Review: sox

http://cvs.fedora.redhat.com/viewcvs/devel/sox/
Initial Owner: twoerner

Comment 1 Nicolas Chauvet (kwizart) 2008-02-06 15:47:05 UTC
[root@kwizatz push]# libst-config --libs
-lvorbisenc -lvorbisfile -logg -lasound -lm -lgsm
but sox-devel doesn't BR gsm-devel alsa-lib-devel and libvorbis-devel
also I have this:
cc -shared -o ../libmltsox.so factory.o filter_sox.o  -lst `libst-config --libs`
-L../../framework -lmlt
/usr/bin/ld: cannot find -lgsm
collect2: ld returned 1 exit status
since libst-config is missing -L/usr/lib64 or x86_64, so it might need to move
to pkgconfig instead. for now I might use a workaround, but it would be fine to
have this fixed for F-9

I wonder if it would be valuable to have modular built for libs
(then BR libtool-ltdl-devel ), I'm testing this also...

License need to be updated

sox 14.0.1 is available

static lib should be removed and .la file



Comment 2 Jiri Moskovcak 2008-02-07 09:56:57 UTC
Hi Nicolas,
what version are you talking about? The latest version in F9 uses pkgconfig and 
BR contains gsm-devel alsa-lib-devel and libvorbis-devel.

Thank you,
Jirka

Comment 3 Nicolas Chauvet (kwizart) 2008-02-16 14:10:08 UTC
Hi Jirka,

I was thinking about using pkgconfig with sox-devel (and Requiring the related
libs there.) For what i've seen in devel, it only uses pkg-config at library
detection.

Well, see the attached patch.(which i don't expect to use unless approved by the
upstream project). There is maybe a better way to do this (i meant to use
AM_LDFLAGS to avoid_version seems a little hacky). Also it would be good to
provide a pkg-config detection for packages that need sox-devel (Work in
progress, but it will depend on the used scheme, either with or without libltdl,
which change a lot).

As a third part packager; I would provide a sox-libs-freeworld for the libmad
and others libs. Maybe that would need to define a more accurate %file section
for {_libdir}/sox/*.so .

For the amr part , as this is nonfree but also non-redistributable, I cannot
hold any sox-libs-nonfree package and that would be cared at end-users rebuild
of the freeworld package.
for info i've made a nosrc package for amr{nv,wb}, you can see here:
http://kwizart.free.fr/fedora/nosrc/



Comment 4 Nicolas Chauvet (kwizart) 2008-02-16 14:11:17 UTC
Created attachment 295071 [details]
Patch to add modular sox built (need to be approved upstream)

Comment 6 Nicolas Chauvet (kwizart) 2008-04-16 20:27:26 UTC
No news, any advices to add pkgconfig support for sox-devel ?
A fixed support for libtool-ltdl-devel could be nice also...

Comment 7 Nicolas Chauvet (kwizart) 2008-08-01 10:19:46 UTC
My patches have been merged upstream (actually a modified version)

Please add libtool-ltdl-devel BR on the next sox release
(This is the default option on debian for some time already)
Pkg-config support will requires to add Requires: pkgconfig on sox-devel
The next release will create a play and record symlinks to sox.

We will need to take care of them via alternatives and ask on fedora-devel ml
for a potential conflict between command line audio player and recorder with
suches symlinks.



Comment 8 Jiri Moskovcak 2008-08-07 13:45:38 UTC
Hi,
I updated to version 14.1 and make it modular as you suggested can you please take a look at the spec file?

Thanks,
Jirka

Spec: http://people.fedoraproject.org/~jmoskovc/sox.spec
SRPM: http://people.fedoraproject.org/~jmoskovc/sox-14.1.0-1.fc10.src.rpm

Comment 9 Nicolas Chauvet (kwizart) 2008-08-07 15:33:22 UTC
There is a problem. As my patch was truncated.
And actually, wasn't present in 14.1.0
see the attached untruncated backported patch

Comment 10 Nicolas Chauvet (kwizart) 2008-08-07 15:35:02 UTC
Created attachment 313708 [details]
Patch to avoid_version of module in sox 14.1.0

This patch was produced in F-8 gendiff command.
It can be applyed in Rawhide (F-10) by using 
%define _default_patch_fuzz 2
On top of the spec file

Comment 11 Nicolas Chauvet (kwizart) 2008-08-07 15:37:07 UTC
For the spec file side, there are some problems
I will try to address them soon so we can get rid of the merge review for sox.

Comment 12 Nicolas Chauvet (kwizart) 2008-10-20 23:30:16 UTC
The current sox in Rawhide in broken (but should works)
http://sourceforge.net/tracker/index.php?func=detail&aid=2076270&group_id=10706&atid=310706
This is reported by the mlt developer on the mlt review
https://bugzilla.redhat.com/show_bug.cgi?id=459979
Please note that sox-devel in F-9 is (this time) completely broken.
(which the current bug was meant to address).
Do you have any tough to the "Patch to avoid_version of module in sox 14.1.0" which is now a backport ?

Comment 13 Lukáš Nykrýn 2011-08-31 14:12:58 UTC
Checked git version:
f8419589a083775a5c3d481237dcc5e8ccd9ddab
sox-14.3.2-1

YES source files match upstream 
YES package meets naming and versioning guidelines. 
YES specfile is properly named, is cleanly written and uses macros consistently
YES dist tag is present
INFO clean section and buildroot present
	clean section is not necessary any more and buildroot is ignored - they should be removed
YES license field matches the actual license
YES license is open source-compatible 
NO  license text included in package
	there is mention in COPYING file, that licences are in the files LICENSE.GPL and LICENSE.LGPL, but both are missing in package
YES latest version is being packaged
YES BuildRequires are proper
YES compiler flags are appropriate
YES package builds in mock (Rawhide/x86_64)
YES debuginfo package looks complete
NO  rpmlint is silent
	sox.src: W: spelling-error %description -l en_US eXchange -> exchange, exchanges, exchanged
	sox.x86_64: W: spelling-error %description -l en_US eXchange -> exchange, exchanges, exchanged
		these two are OK
	sox.x86_64: W: shared-lib-calls-exit /usr/lib64/libsox.so.1.0.0 exit.5
	sox-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/sox-14.3.2/src/ladspa.h
	4 packages and 0 specfiles checked; 1 errors, 3 warnings
YES final provides and requires look sane
N/A %check is present and all tests pass
YES no shared libraries are added to the regular linker search paths
YES owns the directories it creates
YES doesn't own any directories it shouldn't
YES no duplicates in %files
YES scriptlets must be sane
YES code, not content
N/A large documentation files must go in a -doc subpackage
YES %docs are not necessary for the proper functioning of the package
YES no headers
YES no pkgconfig files
YES no libtool .la droppings
YES not a GUI app

Comment 14 Gwyn Ciesla 2012-04-03 15:00:27 UTC
Lukas, if you're still working on this, please set the review-flag to ? and status to ASSIGNED.  If not, I can take over.  Thanks!

Comment 15 Lukáš Nykrýn 2012-04-04 09:20:38 UTC
Thank you for notification, I forget to set them.
The main issue here is still missing license text.

Comment 16 Lukáš Nykrýn 2013-08-14 12:37:17 UTC
I am not sure if this review is still valid.

Comment 17 Gwyn Ciesla 2013-08-14 12:47:18 UTC
It is.

Comment 18 Cole Robinson 2015-02-11 20:39:05 UTC
Mass reassigning all merge reviews to their component. For more details, see this FESCO ticket:

  https://fedorahosted.org/fesco/ticket/1269

If you don't know what merge reviews are about, please see:

  https://fedoraproject.org/wiki/Merge_Reviews

How to handle this bug is left to the discretion of the package maintainer.

Comment 19 Richard Shaw 2015-05-29 13:50:06 UTC
Ok, sorry if I'm jumping in here but there doesn't seem to be a lot of movement. I did a quick review and didn't find anything major but there's a few minor things to address:

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======
- If (and only if) the source package includes the text of the license(s)
  in its own file, then that file, containing the text of the license(s)
  for the package is included in %license.
  Note: License file COPYING is marked as %doc instead of %license
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
- There is one source file licensed ISC, should it be added to the license tag?
- The requires for the devel package sould be arch specifc (%{?_isa}).

===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "LGPL (v2.1 or later) (with incorrect FSF address)", "GPL (v2
     or later)", "Unknown or generated", "ISC", "LGPL (v2.1 or later)",
     "*No copyright* LGPL (v2.1 or later)". 70 files have unknown license.
     Detailed output of licensecheck in /home/build/fedora-
     review/sox/sox/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[x]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
[ ]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 102400 bytes in 4 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[!]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in sox-
     devel
	 Need to add %{?_isa} to requires.
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: Scriptlets must be sane, if used.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: There are rpmlint messages (see attachment).
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Package should not use obsolete m4 macros


Rpmlint
-------
Checking: sox-14.4.2-1.fc23.x86_64.rpm
          sox-devel-14.4.2-1.fc23.x86_64.rpm
          sox-14.4.2-1.fc23.src.rpm
sox.x86_64: W: spelling-error %description -l en_US eXchange -> exchange, exchanges, exchanged
sox.x86_64: W: shared-lib-calls-exit /usr/lib64/libsox.so.3.0.0 exit.5
sox-devel.x86_64: W: only-non-binary-in-usr-lib
sox.src: W: spelling-error %description -l en_US eXchange -> exchange, exchanges, exchanged
sox.src:8: W: macro-in-comment %{name}
sox.src:8: W: macro-in-comment %{name}
sox.src:8: W: macro-in-comment %{version}
sox.src: W: invalid-url Source0: sox/sox-14.4.2.modified.tar.gz
3 packages and 0 specfiles checked; 0 errors, 8 warnings.




Rpmlint (debuginfo)
-------------------
Checking: sox-debuginfo-14.4.2-1.fc23.x86_64.rpm
sox-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/sox-14.4.2/src/ladspa.h
1 packages and 0 specfiles checked; 1 errors, 0 warnings.





Rpmlint (installed packages)
----------------------------
sox.x86_64: W: spelling-error %description -l en_US eXchange -> exchange, exchanges, exchanged
sox.x86_64: W: shared-lib-calls-exit /usr/lib64/libsox.so.3.0.0 exit.5
sox-devel.x86_64: W: only-non-binary-in-usr-lib
sox-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/sox-14.4.2/src/ladspa.h
3 packages and 0 specfiles checked; 1 errors, 3 warnings.



Requires
--------
sox (rpmlib, GLIBC filtered):
    /sbin/ldconfig
    libFLAC.so.8()(64bit)
    libao.so.4()(64bit)
    libao.so.4(LIBAO4_1.1.0)(64bit)
    libasound.so.2()(64bit)
    libasound.so.2(ALSA_0.9)(64bit)
    libasound.so.2(ALSA_0.9.0rc4)(64bit)
    libc.so.6()(64bit)
    libgcc_s.so.1()(64bit)
    libgcc_s.so.1(GCC_3.3.1)(64bit)
    libgomp.so.1()(64bit)
    libgomp.so.1(GOMP_1.0)(64bit)
    libgomp.so.1(GOMP_4.0)(64bit)
    libgomp.so.1(OMP_1.0)(64bit)
    libgomp.so.1(OMP_3.0)(64bit)
    libgsm.so.1()(64bit)
    libltdl.so.7()(64bit)
    libm.so.6()(64bit)
    libogg.so.0()(64bit)
    libpng16.so.16()(64bit)
    libpng16.so.16(PNG16_0)(64bit)
    libpthread.so.0()(64bit)
    libpulse-simple.so.0()(64bit)
    libpulse-simple.so.0(PULSE_0)(64bit)
    libpulse.so.0()(64bit)
    libpulse.so.0(PULSE_0)(64bit)
    libsndfile.so.1()(64bit)
    libsndfile.so.1(libsndfile.so.1.0)(64bit)
    libsox.so.3()(64bit)
    libvorbis.so.0()(64bit)
    libvorbisenc.so.2()(64bit)
    libvorbisfile.so.3()(64bit)
    libwavpack.so.1()(64bit)
    libz.so.1()(64bit)
    rtld(GNU_HASH)

sox-devel (rpmlib, GLIBC filtered):
    /usr/bin/pkg-config
    libsox.so.3()(64bit)
    pkgconfig
    sox



Provides
--------
sox:
    libsox.so.3()(64bit)
    libsox_fmt_alsa.so()(64bit)
    libsox_fmt_ao.so()(64bit)
    libsox_fmt_pulseaudio.so()(64bit)
    libsox_fmt_wavpack.so()(64bit)
    sox
    sox(x86-64)

sox-devel:
    pkgconfig(sox)
    sox-devel
    sox-devel(x86-64)



Unversioned so-files
--------------------
sox: /usr/lib64/sox/libsox_fmt_alsa.so
sox: /usr/lib64/sox/libsox_fmt_ao.so
sox: /usr/lib64/sox/libsox_fmt_pulseaudio.so
sox: /usr/lib64/sox/libsox_fmt_wavpack.so

Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04
Command line :/bin/fedora-review -r -n sox-14.4.2-1.fc23.src.rpm -m fedora-rawhide-x86_64
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 20 Jan Kurik 2015-07-15 15:23:39 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 23 development cycle.
Changing version to '23'.

(As we did not run this process for some time, it could affect also pre-Fedora 23 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 23 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora23

Comment 21 Richard Shaw 2015-11-09 14:56:51 UTC
(In reply to Richard Shaw from comment #19)
> Issues:
> =======
> - If (and only if) the source package includes the text of the license(s)
>   in its own file, then that file, containing the text of the license(s)
>   for the package is included in %license.

Easy enough to fix...


> - There is one source file licensed ISC, should it be added to the license
> tag?

That is for sndio.c which is used for BSD only so it doesn't apply.


> - The requires for the devel package sould be arch specifc (%{?_isa}).

Easy enough to fix...

Any objections to me committing these fixes?

Comment 22 Richard Shaw 2015-11-11 14:04:29 UTC
Aside from the ENVR oops on F22 the packaging issues have been addressed. Closing.