Bug 195401

Review for this package:-
MUST Items:
     - MUST: rpmlint shows no error 
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package osgcal, in the format
osgcal.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license LGPL.
      - MUST: The License field in the package osgcal.spec file matches the
actual license file LGPL in tarball.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains %{__rm} -rf
%{buildroot}.
      - MUST: This package contains shared library files located in the dynamic
linker's default paths, therefore this package is calling ldconfig in %post and
%postun.
      - MUST: This package used macros.
      - MUST: Document files are included like README.
      - MUST: This pcakge contains devel package also, devel package requires
the base package using a fully versioned dependency: Requires: %{name} =
%{version}-%{release} 
      - MUST: This Package did contained any .la libtool archives
      - MUST: libraries are included in a -devel package.
      - MUST: Files used by pkgconfig (.pc files) are in a -devel package.

Parag,

You did a full review, but didn't asign this ticket to yourself, nor changed the
blokcer bug from FE-NEW to FE-REVIEW. Also you listed no MUST FIX items, yet
didn't approve. I'm confused?

>Also you listed no MUST FIX items, yet
didn't approve. I'm confused?
i didnot get you?

What did you not get:

You found nothing wrong with the package why haven't you approved it yet?
Approving is done by adding a comment containging the word approved and changing
the blockerbug to FE-ACCEPT.

Above is Not an official review as I'm not yet sponsored

Wow, the set of review requests is pretty messed up right now, with packages
being assigned without being reviewed and being marked as approved and then
being unapproved.  So far it seems that most of that mess is caused by one
person, although some of this is no doubt related to the bugzilla crash.

Please try to take more care with these review tickets; the reviewers owe it to
the package submitters to address things in an organized manner.  I will try
work through these today, starting with this package.

OK, builds on x86_64, development.  rpmlint says:

E: osgcal binary-or-shlib-defines-rpath /usr/bin/osgcal ['/usr/lib64']
W: osgcal-devel no-documentation

The rpath one is a blocker (but no offense to the earlier review comments above;
you'd only see this on an x86_64 machine).  The configure script doesn't seem to
support --disable-rpath, but the usual procedure of adding a BuildRequires:
libtool, adding LIBTOOL=/usr/bin/libtool on the make line, and excluding
%{_libdir}/*.a in the files list seems to work fine.  I'll attach a patch and
assume for the purposes of this review that the patch is applied.  Please do
test to ensure that it doesn't break anything.

Note that 0.1.41 is out currently.  At this point I'm not going to block on it,
but you'll probably want to see if you need to update.

You have a %check section but there doesn't actually seem to be a test suite. 
It's good to be proactive, though.

Review:
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* source files match upstream:
   4e05fc0ea3320f502d2565a9ac7d2dbb  osgcal-0.1.40.tar.gz
X latest version is being packaged.
* BuildRequires are proper.
* package builds in mock (development, x86_64).
* rpmlint has only ignorable complaints (ass
* final provides and requires are sane:
  osgcal-0.1.40-1.fc6.x86_64.rpm
   libosgCal.so.0()(64bit)
   osgcal = 0.1.40-1.fc6
  =
   /sbin/ldconfig
   libOpenThreads.so.1()(64bit)
   libProducer.so.1()(64bit)
   libcal3d.so.11()(64bit)
   libgif.so.4()(64bit)
   libglib-2.0.so.0()(64bit)
   libosg.so.1()(64bit)
   libosgCal.so.0()(64bit)
   libosgDB.so.1()(64bit)
   libosgFX.so.1()(64bit)
   libosgGA.so.1()(64bit)
   libosgParticle.so.1()(64bit)
   libosgProducer.so.1()(64bit)
   libosgSim.so.1()(64bit)
   libosgText.so.1()(64bit)
   libosgUtil.so.1()(64bit)
   libxml2.so.2()(64bit)
   libz.so.1()(64bit)

  osgcal-devel-0.1.40-1.fc6.x86_64.rpm
   osgcal-devel = 0.1.40-1.fc6
  =
   cal3d-devel
   libosgCal.so.0()(64bit)
   osgcal = 0.1.40-1.fc6

* shared libraries are present; ldconfig is called properly.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
O %check is present but there doesn't seem to be a test suite to run.
* scriptlets present (ldconfig) are OK.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers present in -devel package.
* pkgconfig files present in -devel package.
* no libtool .la droppings.
* not a GUI app.

APPROVED, conditional on fixing the rpath issue, either with the attached patch
or another method of your choice.

Created attachment 131138 [details]
Patch to resolve rpath issue

0.1.41 is not officially released yet, it is still undergoing some regression
testing.  It won't be official until the tarball is moved to the
http://download.gna.org/underware/sources/ directory.

I always add a "make check" under the %check section as long as it does not
break things, this is to prepare for future releases which may have checking
added, which infact is what 0.1.41 is going to do.

I seem to remember somewhere that rpmlint has an error with rpath on 64 bit
systems.  Since this was only showing up under 64bit builds I assumed it was
triggering that bug.  I want to check with Ville and see if he is aware of
anything before apply the patch.

Spec URL: http://tkmame.retrogames.com/fedora-extras/osgcal.spec
SRPM URL: http://tkmame.retrogames.com/fedora-extras/osgcal-0.1.40-2.src.rpm

%changelog
* Mon Jun 19 2006 Christopher Stone <chris.stone> 0.1.40-2
- Add libtool hack to remove rpath