Bug 244031 - Review Request: qfits - A stand-alone general purpose FITS library
Review Request: qfits - A stand-alone general purpose FITS library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-13 09:36 EDT by Sergio Pascual
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-25 10:20:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Sergio Pascual 2007-06-13 09:36:48 EDT
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-28 23:37:09 EDT
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 08:48:26 EDT
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 15:58:54 EDT
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 09:52:17 EDT
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 09:53:03 EDT
I meant updated license tag, sorry
Comment 6 Mamoru TASAKA 2007-09-20 09:17:32 EDT
Rebuild fails on dist-f8.

http://koji.fedoraproject.org/koji/taskinfo?taskID=167375
Comment 7 Sergio Pascual 2007-09-21 07:51:43 EDT
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 12:59:05 EDT
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 15:55:47 EDT
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 03:10:32 EDT
(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 13:51:45 EDT
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 08:01:07 EDT
Okay.

-----------------------------------------------------
   This package (qfits) is APPROVED by me
-----------------------------------------------------
Comment 13 Sergio Pascual 2007-09-24 09:35:29 EDT
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 12:21:22 EDT
cvs done.

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