Hi Alexandru,

As I am unable to sponsor you this is an informal (and incomplete) review but
does hi-light some issues which need to be fixed.


?? -  Meets Packaging Guidelines.
?? -  Package named correctly
NA -  Patches named correctly
OK -  Spec file named correctly to match base
OK -  License is valid
OK -  Licence field matches package
OK -  Licence file installed if supplied
OK -  Spec file in American English
?? -  Source matches upstream (md5)
NO *  Locales use %find_lang
OK -  %clean is present and correct
OK -  Package has correct buildroot.
OK -  Specfile Legible
NO *  Builds in Mock
NA -  %post/%postun calls ldconfig for sh libs
OK -  Owns directories it creates
OK -  No duplicate files
NO *  Has %defattr and has correct permissions
NO *  Macros used consistently
OK -  %doc does not affect runtime
NA -  Headers/static libs in -devel
NA -  .pc files in -devel
NA -  .so files in -devel
NA -  -devel requires base
OK -  Contains no .la libtool archive files
OK -  Does not own others files
NA -  .desktop files installed correctly
NO *  BuildRequires correct.
?? -  Package is code or permissible content.
OK -  Package has rm -rf %{buildroot} at top of %install.
      $RPM_BUILD_ROOT used instead
?? -  Package compiles and builds on at least one arch.
?? -  rpmlint output.
NA -  documentation in -doc package
?? -  final provides and requires are sane.
OK -  should have dist tag
?? -  should package latest version

Comments and Suggestions. 

Summary Field should not be in camel caps, in my opinion it would be better as
"Liquid rescale plug-in for the GIMP"
or something more descriptive like "Content-aware resizing plug-in for the GIMP"

The description does not state what the plugin actually does.

Please use the ${name} macrom in the Source0: URL:

%defattr MUST be at top of %files section

Group: would be better as Applications/Multimedia

There is a typo in the URL field.

Package fails to build in mock.
Please see http://marcbradshaw.co.uk/packages/review/gimp-lqr-plugin/ for build logs

Please see the Locale section of
http://fedoraproject.org/wiki/Packaging/Guidelines


-Marc

Was there any response to this?  It's been over a month, and the above
commentary needs to be addressed.

Hi Marc,

I've followed your pointers to resolve the issues. I'm very new when it comes to
mock, so I don't have a fully buildsys right now (I'm still studying) and didn't
test if it builds in mock.

Spec URL: http://tvtransilvania.ro/ics/Fedora/8/gimp-lqr-plugin.spec
SRPM URL: http://tvtransilvania.ro/ics/Fedora/8/gimp-lqr-plugin-0.3.0-1.fc8.src.rpm

I've just successfully completed a mock build too.

Rebuild failed at least on dist-f9, i386.
http://koji.fedoraproject.org/koji/taskinfo?taskID=265339
http://koji.fedoraproject.org/koji/getfile?taskID=265354&name=build.log

ping?

It builds OK on my rawhide machine, i386. Am I doing something wrong?

http://tvtransilvania.ro/ics/Fedora/devel/build.log
http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin-0.3.0-1.fc9.src.rpm

(In reply to comment #7)
> It builds OK on my rawhide machine, i386. Am I doing something wrong?
  You actually modified your spec file :)
---------------------------------------------------
@@ -11,7 +8,7 @@
 Source0:       
http://liquidrescale.wikidot.com/local--files/en:download-page/%{name}-%{version}.tar.gz
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-%(%{__id_u} -n)
 
-BuildRequires:  gimp, gimp-devel >= 2.2.13, perl-XML-Parser
+BuildRequires:  gimp, gimp-devel >= 2.2.13, perl-XML-Parser, gettext
 Requires:       gimp >= 2.2.13, perl-XML-Parser
 
 %description
---------------------------------------------------
  Now your srpm builds.
  http://koji.fedoraproject.org/koji/taskinfo?taskID=285397

! Note
  - From next time please change the release number of your
    spec file every time you modify your spec file.

For 0.3.0-1.fc9:
* Perl modules dependency
  c.f. http://fedoraproject.org/wiki/Packaging/Perl
  - When writing perl modules dependency, please write the
    module name as dependency, not writing the rpm name
    directly, for example:
---------------------------------------------------
BuildRequires: perl(XML::Parser)

* Tarball, versioning
  - The written Source0 tarball could not found. Instead I found
   
http://liquidrescale.wikidot.com/local--files/en:download-page/gimp-lqr-plugin-0.3.0-6.tar.gz

    Umm... In this case, we regard the version as "0.3.0.6" as
    rpm "Version" does not accept hyphen.

* CFLAGS
----------------------------------------------------
make %{?_smp_mflags} CFLAGS="%{optflags}"
----------------------------------------------------
  - Please check if this CFLAGS is really needed.
    %configure sets CFLAGS as environ (check what %configure actually
    does by `rpm --eval %configure`) and most recent Makefiles
    treats this properly.

* Timestamps
  - Please check if the following method keeps timestamps on
    installed files
----------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
----------------------------------------------------
    While sometimes this does not work, this usually works
    on recent Makefiles.

* Documents
  - Please add the following documents to %doc.
----------------------------------------------------
AUTHORS
NEWS
----------------------------------------------------

* Directory ownership issue:
  - Please consider the ownership issue of directories.
----------------------------------------------------
%files
%{_datadir}/gimp-lqr-plugin/help/en/gimp-help.xml
%{_datadir}/gimp-lqr-plugin/help/en/index.html
%{_datadir}/gimp-lqr-plugin/help/images/dialog1.png
%{_datadir}/gimp-lqr-plugin/help/images/dialog2.png
.......
----------------------------------------------------

   This causes the following problem, for example.
----------------------------------------------------
[tasaka1@localhost ~]$ rpm -qf /usr/share/gimp-lqr-plugin/help/en/index.html 
gimp-lqr-plugin-0.3.0-1.fc9
[tasaka1@localhost ~]$ rpm -qf /usr/share/gimp-lqr-plugin/help/en/           
file /usr/share/gimp-lqr-plugin/help/en is not owned by any package
[tasaka1@localhost ~]$ rpm -qf /usr/share/gimp-lqr-plugin/help/   
file /usr/share/gimp-lqr-plugin/help is not owned by any package
[tasaka1@localhost ~]$ rpm -qf /usr/share/gimp-lqr-plugin/     
file /usr/share/gimp-lqr-plugin is not owned by any package
----------------------------------------------------
    Here
    - Installing gimp-lpr-plugin binary rpm creates the directories
      /usr/share/gimp-lpr-plugin, .../help, .../help/en to install
      index.html under the last directory
    - But these directories left unowned by any package.
    This autually causes some problems.

    ! Note
    - When we write
------------------------------------------------------
%files
%{_datadir}/gimp-lqr-plugin/
------------------------------------------------------
      This contains the directory %_datadir/gimp-lpr-plugin itself
      _and_ all files/directories/etc under %_datadir/gimp-lpr-plugin,
      where
------------------------------------------------------
%files
%dir %{_datadir}/gimp-lqr-plugin/
------------------------------------------------------
      contains the directory %_datadir/gimp-lqr-plugin only.

ping?

ping again?

I will close this bug as NOTABUG if no response from the reporter is
received by 2008-01-17.

I've had a completely independent (and slightly different) package of this 
plugin sat around for a while pending some tidy ups. I've got it into a 
presentable state and submitted a review request as bug #428916; if the OP of 
this package doesn't reappear it may be worth considering that as an 
alternative.

The OP just finished his vacation and will build the new lqr plugin tonight. 
Alex

Spec URL: http://tvtransilvania.ro/ics/Fedora/9/gimp-lqr-plugin.spec
SRPM URL: http://tvtransilvania.ro/ics/Fedora/9/gimp-lqr-plugin-0.4.0-1.fc9.src.rpm

I have also commited a review request for the lqr library: BZ #429202

Well, I also checked liblqr-1 and

* liblqr-1 (bug 429202) is actually licensed under GPLv3 (strict).
* This package gimp-lpr-plugin is licensed under GPLv2 (strict)
* gimp-lpr-plugin depends on liblqr-1
* GPLv2 (strict) and GPLv3 (strict) are incompatible....

Setting FE-Legal

It seems that the upstreams of the both softwares are same.

Perhaps, given that the upstreams are identical, they could sync up the licenses
to match? :)

Are there any response from upstream?

Carlo Baldassi wrote:
> Ok, I have changed the licensing of the plugin from "GPLv2 strict" to
> "GPLv2 or later", which I hope solves the issue.

I've been studying the licenses and as far as I can tell the two packages are
now fully compatible. I'll post SRPMS later today.

Regards,
Alex

ping again?

This spec file needs to be adjusted to reflect the 0.4.0-4 tarball versioning
done by gimp-lqr-plugin. In addition it should include BuildRequires: lqr-1-devel

Adjusted spec file at http://davidf.sjsoft.com/files/gimp-lqr-plugin.spec

This builds installs and runs fine on my Fedora 8 i386 system.

These improvements are from https://bugzilla.redhat.com/show_bug.cgi?id=428916
which is a duplicate package request

New packages with all the issues fixed(hopefully).
Spec URL: http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin.spec
SRPM URL:
http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin-0.4.0-3.fc9.src.rpm

Still gimp-lqr-plugin is GPLv2 strict and liblqr-1 (bug 429202)
is GPLv3. Note that we are not judging the license issues by the 
license tags in the spec files, but by actually checking the source
tarballs.

Would you ask the upstream to actually release the new version of
gimp-lqr-plugin tarball to fix the license conflict from liblqr-1?

By the way the reason we are saying that gimp-lqr-plugin is actually
licensed under strict GPLv2 is that the source codes under src directory
actually says they are licensed under GPLv2 only.

Spec URL: http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin.spec
SRPM URL:
http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin-0.4.0.4-1.fc9.src.rpm

Okay, now gimp-lqr-plugin is under GPLv2+. Removing FE-Legal.

Once liblqr-1 is imported on devel branch, I will review this.

For 0.4.0.4-1:

* Requires
  - Requires: liblqr-1 >= 0.1.0 is not needed. Library dependencies
    like these are automatically checked and added by rpmbuild itself.

  - Requires: perl(XML::Parser) is not needed. This is used only
    for BuildRequires.

* BuildRequires
  - "BuildRequres: gimp" is not needed ("BuildRequires: gimp-devel"
    is needed)

* Directoy ownership issue
  - The directory %{_datadir}/gimp-lqr-plugin is not owned by
    any package.

* %changelog
  - It is preferable you write %changelog as:
---------------------------------------------------------
* Wed Feb 20 2008 Alexandru Ciobanu <alex@tvtransilvania.ro> - 0.4.0.4-1
- Remode hyphen from versioning.

* Tue Feb 19 2008 Alexandru Ciobanu <alex@tvtransilvania.ro> - 0.4.0-3
- Fixed licensing issue.

* Mon Feb 4 2008 Alexandru Ciobanu <alex@tvtransilvania.ro> - 0.4.0-2
---------------------------------------------------------

Spec URL: http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin.spec
SRPM URL:
http://tvtransilvania.ro/ics/Fedora/devel/gimp-lqr-plugin-0.4.0.4-2.fc9.src.rpm

Okay.

-----------------------------------------------------------------------
   This package (gimp-lqr-plugin) is APPROVED by me
-----------------------------------------------------------------------

New Package CVS Request
=======================
Package Name: gimp-lqr-plugin
Short Description: Content-aware resizing plug-in for the GIMP
Owners: ics
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: yes

cvs done.

*** Bug 428916 has been marked as a duplicate of this bug. ***