Bug 591900 - Review Request: libcapifax - Support for Sending/receiving faxes over CAPI capable devices
Review Request: libcapifax - Support for Sending/receiving faxes over CAPI ca...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Felix Kaechele
Fedora Extras Quality Assurance
:
Depends On: 566909
Blocks: 592487
  Show dependency treegraph
 
Reported: 2010-05-13 09:02 EDT by Louis Lagendijk
Modified: 2010-09-04 01:18 EDT (History)
4 users (show)

See Also:
Fixed In Version: libcapifax-0.7.3-4.fc14
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-09-02 16:48:27 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
felix: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Louis Lagendijk 2010-05-13 09:02:04 EDT
Spec URL: http://www.fazant.net/libcapifax/libcapifax.spec
SRPM URL: http://www.fazant.net/libcapifax/libcapifax-0.7.3-1.fc12.src.rpm
Description: 
libcapifax is a library that supports sending/receiving faxes over capi via
soft DSP (spandsp).
It was originally released as part of the capifax package.
Comment 1 Felix Kaechele 2010-05-14 05:54:23 EDT
This package fails to build due to a missing libcapi. I believe this package requires the libcapi from http://www.tabos.org/ffgtk to work. The libcapi in isdn4k-utils will not work.
And that basically poses a problem because the libcapi mentioned above conflicts with the libcapi in isdn4k-utils. This is not acceptable as per https://fedoraproject.org/wiki/Packaging:Conflicts because the libcapi from ffgtk is meant to replace the libcapi from isdn4k-utils.
Comment 2 Louis Lagendijk 2010-05-14 07:24:45 EDT
Sorry, forgot to clarify this: the modular Capi replaces the old one in isdn4k-utils. The current isdn4k-utils CVS contains both. The isdn4k-utils in rawhide does already contain the new, modular libcapi20. The modular Capi is a drop-in replacement for the current version. The ABI is fully compatible.

I discussed this with the isdn4k-utils maintainer and he indicated that this version will probably be back-ported to F12 and F13 too.  See BZ 566909, https://bugzilla.redhat.com/show_bug.cgi?id=566909. 

I am planning to release libcapifax also for EPEL, with a separate libcapi package (which I plan to start the review for shortly, there are some small, mainly cosmetic issues that I want to solve first). That package will add the new libcapi20, but install it in a different location, so it does not conflict. It will then add the path through a file in /etc/ld.so.conf.d so that we can still override the isdn4k-utils libcapi. For a (not yet completely ready package see  http://www.fazant.net/libcapi20/). This could also be used on F12 and F13 to bridge the gap untill isdn4k-utils is updated.
 
Libcapifax already builds in Rawhide. 
If you want to build on F12 or F13 you can either use the rebuild the rawhide version of isdn4k-utils (with the additiona patches in above BZ) or my libcapi package. Both will work

ffgtk (from tabos.org) will also be available soon as that was the reason to start the whole exercise in the first place.
Comment 3 Louis Lagendijk 2010-05-14 07:36:14 EDT
Here is the rpmlint output. I belive that all warnings can be skipped.
I plan to release the package also for EPEL, hence the rpm-buildroot-usage warning.

[louis@travel capifax-0.7.3-jmb3]$ rpmlint libcapifax.spec ~/rpm/SRPMS/libcapifax-0.7.3-1.fc12.src.rpm ~/rpm/RPMS/x86_64/libcapifax-*fc12*
libcapifax.spec:41: W: rpm-buildroot-usage %build rm -rf %{buildroot}
libcapifax.src: W: spelling-error %description -l en_US capi -> capo, cap, carpi
libcapifax.src: W: spelling-error %description -l en_US spandsp -> spandex, spanned, Spanish
libcapifax.src: W: spelling-error %description -l en_US capifax -> capital, capitol, Capitol
libcapifax.src:41: W: rpm-buildroot-usage %build rm -rf %{buildroot}
libcapifax.x86_64: W: spelling-error %description -l en_US faxies -> faxes, faeries, falsies
libcapifax.x86_64: W: spelling-error %description -l en_US capi -> capo, cap, carpi
libcapifax.x86_64: W: spelling-error %description -l en_US spandsp -> spandex, spanned, Spanish
libcapifax.x86_64: W: spelling-error %description -l en_US capifax -> capital, capitol, Capitol
libcapifax.x86_64: W: incoherent-version-in-changelog 3.0.5a-1-jmb3 ['0.7.3-1.fc12.jmb3', '0.7.3-1.jmb3']
libcapifax.x86_64: W: spelling-error %description -l en_US capi -> capo, cap, carpi
libcapifax.x86_64: W: spelling-error %description -l en_US spandsp -> spandex, spanned, Spanish
libcapifax.x86_64: W: spelling-error %description -l en_US capifax -> capital, capitol, Capitol
libcapifax-devel.x86_64: W: spelling-error Summary(en_US) capifax -> capital, capitol, Capitol
libcapifax-devel.x86_64: W: spelling-error Summary(en_US) capifax -> capital, capitol, Capitol
7 packages and 1 specfiles checked; 0 errors, 15 warnings.
Comment 4 Louis Lagendijk 2010-05-14 07:56:50 EDT
oops, please ignore the rpmlint output above, it contains some old rpm's I had still around. Here is the correct output:
[louis@travel capifax-0.7.3-jmb3]$ rpmlint libcapifax.spec ~/rpm/SRPMS/libcapifax-0.7.3-1.fc12.src.rpm ~/rpm/RPMS/x86_64/libcapifax-*
libcapifax.spec:41: W: rpm-buildroot-usage %build rm -rf %{buildroot}
libcapifax.src: W: spelling-error %description -l en_US capi -> capo, cap, carpi
libcapifax.src: W: spelling-error %description -l en_US spandsp -> spandex, spanned, Spanish
libcapifax.src: W: spelling-error %description -l en_US capifax -> capital, capitol, Capitol
libcapifax.src:41: W: rpm-buildroot-usage %build rm -rf %{buildroot}
libcapifax.x86_64: W: spelling-error %description -l en_US capi -> capo, cap, carpi
libcapifax.x86_64: W: spelling-error %description -l en_US spandsp -> spandex, spanned, Spanish
libcapifax.x86_64: W: spelling-error %description -l en_US capifax -> capital, capitol, Capitol
libcapifax-devel.x86_64: W: spelling-error Summary(en_US) capifax -> capital, capitol, Capitol
4 packages and 1 specfiles checked; 0 errors, 9 warnings.
Comment 5 Felix Kaechele 2010-05-14 08:27:33 EDT
I do have a spec file for ffgtk available if you haven't started one yet. So if you're interested just drop me a line.

Other than that I'll take this review.
Comment 6 Louis Lagendijk 2010-06-01 17:54:09 EDT
Ping, Felix, can you please do the review? The isdn4k-utils is now in updates-testing
Comment 7 Louis Lagendijk 2010-07-17 17:57:28 EDT
Ping, Felix are you still interested in doing the review for libcapifax?
Comment 8 Louis Lagendijk 2010-07-29 17:46:20 EDT
Reset fedora-review flag as Felix has not reacted since he said that he wanted to take the review
Comment 9 Felix Kaechele 2010-07-30 03:31:48 EDT
Hi there, sorry for having this sit idle for so long. Here's your review:

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

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Package is named according to the Package Naming Guidelines.
[x]  Spec file name must match the base package %{name}, in the format %{name}.spec.
[x]  Package meets the Packaging Guidelines.
[x]  Package successfully compiles and builds into binary rpms on at least one supported architecture.
Tested on:
[!]  Rpmlint output:
    libcapifax.spec:41: W: rpm-buildroot-usage %build rm -rf %{buildroot}
[x]  Package is not relocatable.
[x]  Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
[x]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
[!]  License field in the package spec file matches the actual license.
    License type: GPLv2+
[x]  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 %doc.
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
    MD5SUM this package    : 98d1aefd7d25496d3fd2f25c253bfe53
    MD5SUM upstream package: 98d1aefd7d25496d3fd2f25c253bfe53
[x]  Package is not known to require ExcludeArch
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
[-]  The spec file handles locales properly.
[x]  ldconfig called in %post and %postun if required.
[x]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.
[x]  Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[x]  Package consistently uses macros.
[x]  Package contains code, or permissable content.
[-]  Large documentation files are in a -doc subpackage, if required.
[x]  Package uses nothing in %doc for runtime.
[x]  Header files in -devel subpackage, if present.
[-]  Static libraries in -devel subpackage, if present.
[x]  Package requires pkgconfig, if .pc files are present.
[x]  Development .so files in -devel subpackage, if present.
[x]  Fully versioned dependency in subpackages, if present.
[x]  Package does not contain any libtool archives (.la).
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[x]  Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
[!]  Latest version is packaged.
[x]  Package does not include license text files separate from upstream.
[-]  Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
[x]  Reviewer should test that the package builds in mock.
[x]  Package should compile and build into binary rpms on all supported architectures.
    Tested on:
        F12: http://koji.fedoraproject.org/koji/taskinfo?taskID=2357892
        F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2357886
[x]  Package functions as described.
[x]  Scriptlets must be sane, if used.
[x]  The placement of pkgconfig(.pc) files are correct.
[-]  File based requires are sane.


=== Issues ===
1. rm -rf %{buildroot} is not needed in %build as the buildroot is populated in %install, not in %build
2. Specfile lists license as being GPLv2. Sources however specify GPLv2+
3. Upstream of the patches lists a jmb4 version at http://www.tabos.org/ffgtk/download.php

=== Final Notes ===
1. If the aforementioned issues are solved I shall approve this package.
Comment 10 Louis Lagendijk 2010-07-30 17:37:01 EDT
Hello Felix
Thanks for the review!
I corrected the issues you mentioned:
- removed the cleanup of the buildroot from %install AND %clean
- Corrected license. good catch! I don't know how I missed that
- Replaced my own fix for the pc file and replaced that with the jmb4 patch

New versions of the SRPM and specfiel are available from:

http://fazant.net/libcapifax/libcapifax-0.7.3-3.fc12.src.rpm
http://fazant.net/libcapifax/libcapifax.spec
Comment 11 Felix Kaechele 2010-08-07 16:35:08 EDT
Sorry, I was busy last week again.
I see that you removed the %clean section completely. It's required however and does require a rm -rf %{buildroot} also your changelog includes the info that the rm -rf %{buildroot} has moved from %build to %install. %install however does not contain rm -rf %{buildroot} (as it should).
Seems that there was a little misunderstanding regarding that.

Other than that everything else seems fine.
Comment 12 Louis Lagendijk 2010-08-14 12:16:56 EDT
My bad. I indeed misunderstood you. 

I have been abroad this week, hence the late response. 
I put the rm -rf %{buildroot} back in install and %clean

http://fazant.net/libcapifax/libcapifax-0.7.3-4.fc12.src.rpm
http://fazant.net/libcapifax/libcapifax.spec
Comment 13 Felix Kaechele 2010-08-14 18:21:41 EDT
Okay. Package looks good now.

APPROVED
Comment 14 Louis Lagendijk 2010-08-21 10:46:01 EDT
New Package SCM Request
=======================
Package Name: libcapifax
Short Description: Support for Sending/receiving faxes over CAPI capable devices
Owners: llagendijk
Branches: f12 f13 f14 el6
InitialCC:
Comment 15 Kevin Fenzi 2010-08-23 16:57:12 EDT
Git done (by process-git-requests).
Comment 16 Fedora Update System 2010-08-25 18:13:55 EDT
libcapifax-0.7.3-4.fc14 has been submitted as an update for Fedora 14.
http://admin.fedoraproject.org/updates/libcapifax-0.7.3-4.fc14
Comment 17 Fedora Update System 2010-08-25 18:14:49 EDT
libcapifax-0.7.3-4.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libcapifax-0.7.3-4.fc13
Comment 18 Fedora Update System 2010-08-25 18:15:57 EDT
libcapifax-0.7.3-4.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/libcapifax-0.7.3-4.fc12
Comment 19 Fedora Update System 2010-08-25 20:56:01 EDT
libcapifax-0.7.3-4.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libcapifax'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/libcapifax-0.7.3-4.fc13
Comment 20 Fedora Update System 2010-09-02 16:48:23 EDT
libcapifax-0.7.3-4.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Fedora Update System 2010-09-02 16:48:50 EDT
libcapifax-0.7.3-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 22 Fedora Update System 2010-09-04 01:18:30 EDT
libcapifax-0.7.3-4.fc14 has been pushed to the Fedora 14 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.