Bug 615319 - Review Request: cgnslib - Computational Fluid Dynamics General Notation System
Summary: Review Request: cgnslib - Computational Fluid Dynamics General Notation System
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Chitlesh GOORAH
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-07-16 13:56 UTC by Shakthi Kannan
Modified: 2010-10-02 20:18 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-10-02 20:18:37 UTC
Type: ---
Embargoed:
chitlesh: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Shakthi Kannan 2010-07-16 13:56:47 UTC
Spec URL: http://shakthimaan.fedorapeople.org/SPECS/cgnslib.spec
SRPM URL: http://shakthimaan.fedorapeople.org/SRPMS/cgnslib-2.5-2.r4.fc14.src.rpm
Description: The Computational Fluid Dynamics General Notation System (CGNS) provides a general, portable, and extensible standard for the storage and
retrieval of computational fluid dynamics (CFD) analysis
data. It consists of a collection of conventions, and free
and open software implementing those conventions. It is
self-descriptive, machine-independent, well-documented, and
administered by an international steering committee.

Comment 1 Shakthi Kannan 2010-07-16 14:05:17 UTC
$ rpmlint cgnslib.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint cgnslib-2.5-2.r4.fc14.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint cgnslib-2.5-2.r4.fc14.i686.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

$ rpmlint cgnslib-devel-2.5-2.r4.fc14.i686.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Successful Koji builds for F-13, F-14 and EL-6.

http://koji.fedoraproject.org/koji/taskinfo?taskID=2324102
http://koji.fedoraproject.org/koji/taskinfo?taskID=2324105
http://koji.fedoraproject.org/koji/taskinfo?taskID=2324108

hdf5 >= 1.8 is not available in EL-5, so build will fail.

Comment 2 Ankur Sinha (FranciscoD) 2010-07-16 17:15:46 UTC
Review:

+ OK
? ISSUES
- NA

--------------------------------------------------------------------------------

+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
? License
? License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
+ Sources match upstream md5sum:
[Ankur@localhost rpmbuild]$ md5sum cgnslib_2.5-4.tar.gz SOURCES/cgnslib_2.5-4.tar.gz 
42063efdf726c81300a51c3495d3224e  cgnslib_2.5-4.tar.gz
42063efdf726c81300a51c3495d3224e  SOURCES/cgnslib_2.5-4.tar.gz


- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

+ Headers/static libs in -devel subpackage.
+ Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
+ .so files in -devel subpackage.
+ -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.


SHOULD Items:

+ Should build in mock.
Checked koji -> http://koji.fedoraproject.org/koji/taskinfo?taskID=2324518

+ Should build on all supported archs
? Should function as described.
+ Should have sane scriptlets.
+ Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
+ Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1.Haven't been able to run rpmlint yet. Mock giving dep errors. Will run it whenever possible and comment

2.Not checked functioning

3.License looks like zlib. Why the LGPL?


"This license is borrowed from the zlib/libpng License:

     http://www.opensource.org/licenses/zlib-license.php

and supercedes the GNU Lesser General Public License (LGPL) which previously governed the use and distribution of the software."

Comment 3 Shakthi Kannan 2010-07-18 04:48:06 UTC
Thanks for the review.

1. Mock runs fine here (on rawhide):

$ mock -r fedora-13-i386 --rebuild cgnslib-2.5-2.r4.fc14.src.rpm 
INFO: mock.py version 1.1.1 starting...
State Changed: init plugins
State Changed: start
INFO: Start(cgnslib-2.5-2.r4.fc14.src.rpm)  Config(fedora-13-i386)
State Changed: lock buildroot
State Changed: clean
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.1
INFO: Mock Version: 1.1.1
INFO: enabled root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: creating cache
State Changed: setup
State Changed: build
INFO: Done(cgnslib-2.5-2.r4.fc14.src.rpm) Config(fedora-13-i386) 23 minutes 39 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-13-i386/result

2. The tests use the built shared library which you can see from the build output.

3. Updated license to use zlib (only):

SPEC: http://shakthimaan.fedorapeople.org/SPECS/cgnslib.spec
SRPM: http://shakthimaan.fedorapeople.org/SRPMS/cgnslib-2.5-3.r4.fc14.src.rpm

Comment 4 Chitlesh GOORAH 2010-07-18 11:12:03 UTC
- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed with an open-source compatible license and
meet other legal requirements as defined in the legal section of Packaging
Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own file,
then that file, containing the text of the license(s) for the package is
included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible. 
- MUST: The sources used to build the package must matches the upstream source,
as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at
least i386.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.
- MUST: If the package does not contain shared library files located in the
dynamic linker's default paths
- MUST: the package is not designed to be relocatable
- MUST: the package owns all directories that it creates.
- MUST: the package does not contain any duplicate files in the %files listing.
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros section
of Packaging Guidelines.
- MUST: The package contains code, or permissable content. This is described in
detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If it
is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package containing GUI applications includes a %{name}.desktop file, and
that file must be properly installed with desktop-file-install in the %install
section.
- MUST: Package does not own files or directories already owned by other packages. 

SHOULD Items:

 - SHOULD: The source package does include license text(s) as COPYING
 - SHOULD: mock builds succcessfully in i386.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD: No scriptlets were used, those scriptlets must be sane. 
 - SHOULD: No subpackages present.

APPROVED

Comment 5 Ankur Sinha (FranciscoD) 2010-07-18 13:01:01 UTC
hey!

I was reviewing this one, why take it like this? It would be nice for you to ask/inform the current reviewer once. It's not like I'd say "no".

Ankur

Comment 6 Shakthi Kannan 2010-07-18 16:05:31 UTC
New Package CVS Request
=======================
Package Name: cgnslib
Short Description: Computational Fluid Dynamics General Notation System
Owners: shakthimaan chitlesh
Branches: F-13 F-14 EL-6

Comment 7 Shakthi Kannan 2010-07-19 16:32:15 UTC
Package Change Request
======================
Package Name: cgnslib
Short Description: Computational Fluid Dynamics General Notation System
Owners: shakthimaan chitlesh
Branches: F-13 EL-6

Comment 8 Shakthi Kannan 2010-07-19 17:18:50 UTC
New Package CVS Request
=======================
Package Name: cgnslib
Short Description: Computational Fluid Dynamics General Notation System
Owners: shakthimaan chitlesh
Branches: F-13 EL-6

Comment 9 Kevin Fenzi 2010-07-20 03:20:28 UTC
CVS done (by process-cvs-requests.py).

Comment 10 Fedora Update System 2010-07-20 04:43:29 UTC
cgnslib-2.5-3.r4.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/cgnslib-2.5-3.r4.fc13


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