Bug 429202 - Review Request: liblqr-1 - LiquidRescale library
Review Request: liblqr-1 - LiquidRescale library
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
i386 Linux
low Severity low
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
Depends On:
Blocks: 314871
  Show dependency treegraph
Reported: 2008-01-17 16:59 EST by Alexandru Ciobanu
Modified: 2008-03-02 11:34 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-03-02 11:27:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)
Patch for pkgconfig .pc file to remove unneeded directory (372 bytes, patch)
2008-02-25 13:57 EST, Mamoru TASAKA
no flags Details | Diff

  None (edit)
Description Alexandru Ciobanu 2008-01-17 16:59:48 EST
Spec URL: http://tvtransilvania.ro/ics/Fedora/9/liblqr.spec
SRPM URL: http://tvtransilvania.ro/ics/Fedora/9/liblqr-1-0.1.0-1.fc9.i386.rpm

The LiquidRescale (lqr) library provides a C/C++ API for
performing non-uniform resizing of images by the seam-carving

This package is a part of the LiquidRescale plugin for The GIMP.
Comment 1 Jason Tibbitts 2008-01-17 17:05:52 EST
The SRPM URL does not point to an srpm.
Comment 2 Alexandru Ciobanu 2008-01-17 17:09:28 EST
Pardon me.

Comment 3 Mamoru TASAKA 2008-01-18 11:21:13 EST
The license of this software is actually GPLv3. It is okay
for this software, however this causes a problem on gimp-lqr-plugin
(see bug 314871)
Comment 4 Mamoru TASAKA 2008-01-18 11:28:21 EST
Actually I want to know why the upstream released this as GPLv3
before checking this package.
Comment 5 Mamoru TASAKA 2008-01-31 09:59:56 EST
Once the license issue on bug 314871 is resolved, I will try to
start to check this package.
Comment 7 Mamoru TASAKA 2008-02-20 12:47:50 EST
For general packaging guidelines you can refer to:

For 0.1.0-2:

* spec file name
  - must coincide with the name of main package, i.e.
    spec file must have the name "liblqr-1.spec".

* Group tag
  - Usually the library related rpm has 
    "Group: System Environment/Libraries" and its -devel
    subpackage usually has
    "Group: Development/Libraries".

* Requires/BuildRequires
  - "BuildRequires: glib" is not needed. GIMP uses glib2,
    liblqr-1 also uses glib2 not glib (GLib 1).
    Also liblqr-1 has "BuildRequies: gimp-devel", which 
    also pulls glib2 automatically.

  - "Requires: pkgconfig" must be removed from main package.
    The main package is not for development and pkgconfig must
    not be needed.

  - Also "Requires: gimp" is not needed for main package.
    This library does not need gimp itself, only gimp-libs
    is needed and such dependency are automatically treated
    by rpmbuild.

  - -devel subpackage does not need "Requires: gimp-devel".
    From installed pkgconfig .pc file and header files,
    only "glib2-devel" (and also pkgconfig) is needed as
    Requires for -devel subpackage

  - Also -devel subpackage must have
    "Requires: %{name} = %{version}-%{release}"

* License
  - For this package there is no reason that -devel
    subpackage should have different License tag.

* macros usage consistency
  - If you want to use %__rm , please also use %__make
    %__install for consistency.

* symlink for linkage
  - %{_libdir}/libXXXX.so should be in -devel package
    (%{_libdir}/libXXXX.so.X symlink should be in main package).
    This type of errors can be detected by using rpmlint
    commands (in rpmlint package). 
[tasaka1@localhost liblqr-1]$ rpmlint *0.1.0-2*rpm
liblqr-1.i386: W: devel-file-in-non-devel-package /usr/lib/liblqr-1.so

* Directory ownership issue
  - Please make it sure that all directories which are created
    when installing a rpm are owned by the rpm.
    For this package the directory %{_includedir}/lqr-1/,
   %{_includedir}/lpr must be owned by -devel subpackage.

  ! By the way,
    when you write:
%files devel
    This contains the directory %{_includedir}/lqr-1 and
    all files/directories/etc under %{_includedir}/lqr-1.
Comment 9 Mamoru TASAKA 2008-02-21 12:23:34 EST
For 0.1.0-3:

* Description
  - The %description of -devel package is wrong.
%description devel
The libqr-devel package contains the static libraries and header files

* .so symlink location
(In reply to comment #7)
> * symlink for linkage
>   - %{_libdir}/libXXXX.so should be in -devel package
>     (%{_libdir}/libXXXX.so.X symlink should be in main package).
>     This type of errors can be detected by using rpmlint
>     commands (in rpmlint package). 
> -------------------------------------------------------------
> [tasaka1@localhost liblqr-1]$ rpmlint *0.1.0-2*rpm
> liblqr-1.i386: W: devel-file-in-non-devel-package /usr/lib/liblqr-1.so
> .......
> -------------------------------------------------------------

  - Please check your rpms by rpmlint to detect generic packaging errors.
    For newest rpm:
liblqr-1-devel.i386: E: library-without-ldconfig-postin /usr/lib/liblqr-1.so.0.0.0
liblqr-1-devel.i386: E: library-without-ldconfig-postun /usr/lib/liblqr-1.so.0.0.0
    What I said was:
    - %_libdir/liblqr-1.so.0.0.0 and %_libdir/liblqr-1.so.0 must be
       in main package
    - %_libdir/liblqr-1.so must be moved to -devel package.

* BuildRequires
  - Well, I rechecked BuildRequires and actually glib2-devel is
    sufficient for BuildRequires (gimp-devel is not needed for

* Typo
  - Please fix below (by the way with this typo rebuild actually
find %buildroot -name \*.la -exec %rm -f {} \;
Comment 10 Alexandru Ciobanu 2008-02-25 04:29:31 EST
Thanks Mamoru for your time, here are the new files.
Spec URL: http://tvtransilvania.ro/ics/Fedora/devel/liblqr-1.spec
SRPM URL: http://tvtransilvania.ro/ics/Fedora/devel/liblqr-1-0.1.0-4.fc9.src.rpm
Comment 11 Mamoru TASAKA 2008-02-25 13:57:29 EST
Created attachment 295827 [details]
Patch for pkgconfig .pc file to remove unneeded directory

For 0.1.0-4:

3 issues 

* undefined non-weak symbols
[root@localhost liblqr-1]# rpmlint liblqr-1 liblqr-1-devel
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0 g_free
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0 sqrt
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0 log
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0 exp
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0
liblqr-1.i386: W: undefined-non-weak-symbol /usr/lib/liblqr-1.so.0.0.0
  - liblqr-1.so contains undefined non-weak symbols.
    You can check this by $ ldd -r /usr/lib/liblqr-1.so.

    As this library is to be used for linkage from other packages,
    such symbols cannot be allowed because leaving such symbols
    causes linkage failure.

    The following fixes this issue:
export LDFLAGS="`pkg-config --libs glib-2.0` -lm"
%{__make} %{?_smp_mflags}

* Unneeded libdir in .pc file
  - On i386, /usr/lib/pkgconfig/lqr-1.pc  contains the line:
    10	Libs: -L${libdir}/lqr-1 -llqr-1
    But no libraries are installed under %{_libdir}/lqr-1 so
    -L${libdir}/lqr-1 is unneed. Please apply the patch attached.

* %buildroot v.s. $RPM_BUILD_ROOT
  - Please choose one and don't use both.
Comment 12 Alexandru Ciobanu 2008-02-29 06:32:33 EST
Spec URL: http://tvtransilvania.ro/ics/Fedora/devel/liblqr-1.spec
SRPM URL: http://tvtransilvania.ro/ics/Fedora/devel/liblqr-1-0.1.0-5.fc9.src.rpm

I have a clean rpmlint report now.
Comment 13 Mamoru TASAKA 2008-02-29 11:22:27 EST
Please change the permission of files in srpm to 0644.

- This package (liblqr-1) is okay
- Your another review request (bug 314871) needs fixing on some points,
  however I hope it can be approved after some fixes.

    This package (liblqr-1) is APPROVED by me

Please follow the procedure written on:
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 7/8, you also have
to look at
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.
Comment 14 Mamoru TASAKA 2008-03-01 08:43:43 EST
Now I am sponsoring you. Please follow "Join" wiki again.
Comment 15 Alexandru Ciobanu 2008-03-01 09:24:55 EST
New Package CVS Request
Package Name: liblqr-1
Short Description: LiquidRescale library
Owners: ics
Branches: F-7 F-8
Cvsextras Commits: yes
Comment 16 Kevin Fenzi 2008-03-01 13:34:17 EST
cvs done.
Comment 17 Mamoru TASAKA 2008-03-02 11:34:13 EST
Please don't forget to rebuild this package also on F-7/8.
Also, for F-7/8, please request the rebuilt packages to push to
stable on:

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