Bug 244031 - Review Request: qfits - A stand-alone general purpose FITS library
Summary: Review Request: qfits - A stand-alone general purpose FITS library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-06-13 13:36 UTC by Sergio Pascual
Modified: 2007-11-30 22:12 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-09-25 14:20:42 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Sergio Pascual 2007-06-13 13:36:48 UTC
Spec URL: http://astro-sfg.fis.ucm.es/~spr/qfits.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/qfits-6.2.0-0.2.src.rpm
Description: qfits is a stand-alone C library offering easy access to FITS files.

Comment 1 Jason Tibbitts 2007-06-29 03:37:09 UTC
A few issues:

Could you consider using a %description something like:
   qfits is a stand-alone C library offering easy access to FITS (Flexible Image 
   Transport System) files.
so that people like me aren't completely bewildered.  (It's still bewildering,
but it's polite to at least explain acronyms to those who might come across the
package.)

rpmlint finds a few things to complain about:
W: qfits unused-direct-shlib-dependency /usr/lib64/libqfits.so.0.0.0
/lib64/libm.so.6
This is not a big deal; autoconf-generated stuff often tends to link in basic
system libs even when they aren't used.

E: qfits-tools binary-or-shlib-defines-rpath /usr/bin/dtfits ['/usr/lib64']
(and ten other rpath errors)
These are blockers; see the "Beware of Rpath" section of
http://fedoraproject.org/wiki/Packaging/Guidelines

Comment 2 Sergio Pascual 2007-07-01 12:48:26 UTC
I have modified the description according to your recommendation.

It's weird, but I don't get any error about rpaths (I'm building on 2
processors, i386). I have checked the binaries with chrpath. 

Anyway, I have added a --disable-rapth to %configure
Spec URL: http://astro-sfg.fis.ucm.es/~spr/qfits.spec
SRPM URL: http://astro-sfg.fis.ucm.es/~spr/qfits-6.2.0-0.3.src.rpm

Comment 3 Sergio Pascual 2007-07-26 19:58:54 UTC
Moved to brand new fedorapeople

Spec URL: http://sergiopr.fedorapeople.org/qfits.spec
SRPM URL: http://sergiopr.fedorapeople.org/qfits-6.2.0-0.3.src.rpm

Comment 4 Sergio Pascual 2007-09-13 13:52:17 UTC
Updated dist tag

Spec URL: http://sergiopr.fedorapeople.org/qfits.spec
SRPM URL: http://sergiopr.fedorapeople.org/qfits-6.2.0-0.4.src.rpm

Comment 5 Sergio Pascual 2007-09-13 13:53:03 UTC
I meant updated license tag, sorry

Comment 6 Mamoru TASAKA 2007-09-20 13:17:32 UTC
Rebuild fails on dist-f8.

http://koji.fedoraproject.org/koji/taskinfo?taskID=167375

Comment 7 Sergio Pascual 2007-09-21 11:51:43 UTC
It was a problem with "open", thanks

New patched srpm:

Spec URL: http://sergiopr.fedorapeople.org/qfits.spec
SRPM URL: http://sergiopr.fedorapeople.org/qfits-6.2.0-1.fc7.src.rpm

Comment 8 Mamoru TASAKA 2007-09-21 16:59:05 UTC
For 6.2.0-1:

! Naming suggestion
  - Well, IMO it is better that
    * qfits should be renamed to qfits-libs
    * qfits-libs should be renamed to qfits

* Requires for /sbin/ldconfig
  - "Requires(post,postun): /sbin/ldconfig" is not needed.
    (Still %post, %postun themselves are needed)

* defattr
  - We now recommend %defattr(-,root,root,-)

* Timestamps
  - For keeping timestamps, I recommend
-----------------------------------------------------
%{__make} DESTDIR=%{buildroot} INSTALL="%{__install} -p" install
------------------------------------------------------
    This will keep timestamps on installed header files and
    man files
    ! Note: The method above usually works on recent Makefile.

Comment 9 Sergio Pascual 2007-09-21 19:55:47 UTC
I think you mean: 
qfits -> qfits-lib 
qfits-tools -> qfits
Is that correct? I have changed the names accordingly and fixed the other
problems. Here there are the new files:

Spec URL: http://sergiopr.fedorapeople.org/qfits.spec
SRPM URL: http://sergiopr.fedorapeople.org/qfits-6.2.0-2.fc7.src.rpm

Comment 10 Mamoru TASAKA 2007-09-22 07:10:32 UTC
(In reply to comment #9)
* Naming

> I think you mean: 
> qfits -> qfits-lib 
> qfits-tools -> qfits
> Is that correct? 
  - Yes, however
  * Many packagers seem to choose -libs, not -lib: so IMO
     qfits-libs
     qfits
     qfits-devel (not qfits-libs-devel)
    is better
   * Also, qfits should have "qfits-libs = %{version}-%{release}"
     (and qfits-devel should have
      "qfits-libs = %{version}-%{release}")

* Group
  - IMO it is better that the Group of qfits-libs is
    "System Environment/Libraries".

* %check
  - As source code has test/ subdirectory and "make check"
    can be executed, please add %check stage and do
    "make check".

Comment 11 Sergio Pascual 2007-09-23 17:51:45 UTC
I have included your comments in the next version:

Spec URL: http://sergiopr.fedorapeople.org/qfits.spec
SRPM URL: http://sergiopr.fedorapeople.org/qfits-6.2.0-3.fc7.src.rpm



Comment 12 Mamoru TASAKA 2007-09-24 12:01:07 UTC
Okay.

-----------------------------------------------------
   This package (qfits) is APPROVED by me
-----------------------------------------------------

Comment 13 Sergio Pascual 2007-09-24 13:35:29 UTC
New Package CVS Request
=======================
Package Name: qfits
Short Description: A stand-alone general purpose FITS library
Owners: sergiopr
Branches: FC-6 F-7
InitialCC: 
Cvsextras Commits: yes

Comment 14 Kevin Fenzi 2007-09-24 16:21:22 UTC
cvs done.


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