Bug 427121 - Review Request: grib_api - ECMWF encoding/decoding GRIB software
Summary: Review Request: grib_api - ECMWF encoding/decoding GRIB software
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: 373861
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-01 00:24 UTC by Patrice Dumas
Modified: 2011-02-10 16:41 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-12-03 15:38:46 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Patrice Dumas 2008-01-01 00:24:43 UTC
Spec URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/grib_api.spec
SRPM URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/grib_api-1.3.0-1.fc9.src.rpm
Description: 

Command line tools for interactive and batch processing 
of grib data.
The grib_api-devel package contains libraries and header files for
developing applications that use grib_api.

Comment 1 Jason Tibbitts 2008-02-23 01:18:30 UTC
Builds OK; rpmlint has many complaints about the .sh files in the
documentation being executable, for example:
  grib_api-devel.x86_64: W: spurious-executable-perm 
   /usr/share/doc/grib_api-devel-1.3.0/examples/precision_fortran.sh
which, though I don't like executable documentation in general, I suppose are
OK as long as they don't generate additional dependencies.  (They don't seem
to do so.)

Also,
  grib_api-devel.x86_64: E: zero-length 
   /usr/share/doc/grib_api-devel-1.3.0/data/missing_new.grib2
which I guess is used by one of the examples and needs to be empty (although
you should verify this; we don't really want to be shipping empty files unless
there's some reason for it).

You should use a complete URL for Source0; this seems to work:
http://www.ecmwf.int/products/data/software/download/software_files/%{name}-%{version}.tar.gz

I note that 1.4.0 is out; did you want to update to it?  A naive update fails
to build because __dist_doc seems to have been changed a bit.

I believe the software is LGPLv3; that's what the upstream web site says, and
the LICENSE and source files seem to agree:

* Licensed under the GNU Lesser General Public License which
* incorporates the terms and conditions of version 3 of the GNU
* General Public License.

although that language is kind of bizarre and they also package a copy of the
GPLv3 (and a second copy of LGPLv3 for good measure, I guess) all in the
top-level directory of the tarball.  Can you check with upstream to see
if they intend one or the other?  Without clarification from them I am
inclined to say that LGPLv3 is correct.

The API documentation is about 70% of the -devel package, but I don't think
that's big enough to warrant splitting the package.


* source files match upstream:
   36f31407f0c4aa64991f65f5d362d2b3efd986ea25b0d8f214772b21665a170b  
   grib_api-1.3.0.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK (although some definition of "grib" might be considered to 
   be kind to the users.
* dist tag is present.
* build root is OK.
X license field matches the actual license.
* license is open source-compatible.
* license text included in package.
X latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* debuginfo package looks complete.
? rpmlint has one complaint which may be valid.
* final provides and requires are sane:
  grib_api-1.3.0-1.fc9.x86_64.rpm
   grib_api = 1.3.0-1.fc9
  =
   libjasper.so.1()(64bit)

  grib_api-devel-1.3.0-1.fc9.x86_64.rpm
   grib_api-static = 1.3.0-1.fc9
   grib_api-devel = 1.3.0-1.fc9
  =
   /bin/sh
   grib_api = 1.3.0-1.fc9

* %check is present and all tests pass:
   All 19 tests passed
   All 14 tests passed

* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* headers.
* no pkgconfig files.
* static libraries are in the -devel package, which is OK because there are no
  dynamic libraries provided.  The -static provide is present as required.
* no libtool .la files.


Comment 2 Patrice Dumas 2008-02-23 15:02:44 UTC
(In reply to comment #1)
> Builds OK; rpmlint has many complaints about the .sh files in the
> documentation being executable, for example:
>   grib_api-devel.x86_64: W: spurious-executable-perm 
>    /usr/share/doc/grib_api-devel-1.3.0/examples/precision_fortran.sh
> which, though I don't like executable documentation in general, I suppose are
> OK as long as they don't generate additional dependencies.  (They don't seem
> to do so.)

I agree, but in that case they are auto-documenting the arguments
for the examples. And they are set up such that it should be easy to
rerun them.

> Also,
>   grib_api-devel.x86_64: E: zero-length 
>    /usr/share/doc/grib_api-devel-1.3.0/data/missing_new.grib2
> which I guess is used by one of the examples and needs to be empty (although
> you should verify this; we don't really want to be shipping empty files unless
> there's some reason for it).

This warning is not there anymore.
 
> You should use a complete URL for Source0; this seems to work:
>
http://www.ecmwf.int/products/data/software/download/software_files/%{name}-%{version}.tar.gz
> 
> I note that 1.4.0 is out; did you want to update to it?  A naive update fails
> to build because __dist_doc seems to have been changed a bit.

Yes, I updated to the latest version.

> Without clarification from them I am
> inclined to say that LGPLv3 is correct.

It seems so to me too. I contacted them, but in the mean time I think 
that LGPLv3 is ok.


> * description is OK (although some definition of "grib" might be considered to 
>    be kind to the users.

Done.

> X license field matches the actual license.

Done.

> X latest version is being packaged.

done.


> * %check is present and all tests pass:
>    All 19 tests passed
>    All 14 tests passed

Now one test doesn't pass, I have contacted upstream. I disabled them
in the mean time.


There are .mod files that are not rightly placed for now, still 
waiting on the fortran mod files guideline to be implemented.
I don't think it should be a blocker I'll do things right as soon
as it is implemented.

I also contacted upstream for names that are too generic.


http://www.environnement.ens.fr/perso/dumas/fc-srpms/grib_api.spec
http://www.environnement.ens.fr/perso/dumas/fc-srpms/grib_api-1.4.0-1.fc9.src.rpm


Comment 3 Jason Tibbitts 2008-03-02 21:17:25 UTC
This all looks good to me.  I guess the bad test could be troublesome but if you
don't have a problem with then I won't worry about it.  I guess if you're not
going to need the commented-out scriptlets you could remove them so the spec
looks cleaner, but that's really minor.

Which name did you think was too generic?  I guess "typesizes.mod" might be, but
I don't know how many .mod files we're going to end up seeing.  Everything else
seemed to me to be prefixed, in a subdirectory or simply named such that
conflicts would be improbable.

APPROVED

Comment 4 Patrice Dumas 2008-03-02 21:29:53 UTC
(In reply to comment #3)
> This all looks good to me.  I guess the bad test could be troublesome but if you
> don't have a problem with then I won't worry about it.  

According to upstream it is fixed, but upstream also asked me
to wait for the next release to make a release.

> I guess if you're not
> going to need the commented-out scriptlets you could remove them so the spec
> looks cleaner, but that's really minor.

I am discussing adding shared libs with the debian maintainer (upstream
agreed), so I'll prefer to keep them.

> Which name did you think was too generic?  I guess "typesizes.mod" might be, but
> I don't know how many .mod files we're going to end up seeing.  Everything else
> seemed to me to be prefixed, in a subdirectory or simply named such that
> conflicts would be improbable.

It is because I prefixed 2 of them... .mod files should really not be
generic, I hope that we end up with a lof of .mod files...

Thanks for the review!

New Package CVS Request
=======================
Package Name: grib_api
Short Description: ECMWF encoding/decoding GRIB software
Owners: pertusus
Branches: 
InitialCC: 
Cvsextras Commits: yes



Comment 5 Kevin Fenzi 2008-03-03 20:03:25 UTC
cvs done.

Comment 6 Jason Tibbitts 2008-05-31 00:40:38 UTC
Any reason this is still open?

Comment 7 Patrice Dumas 2008-05-31 08:55:55 UTC
Yes, I am waiting for upstream for the next release to build the 
package, as they asked, and as it isn't built I don't close the bug.

Comment 8 Jason Tibbitts 2008-08-08 16:32:42 UTC
Are we still waiting for upstream to do something?

Comment 9 Patrice Dumas 2008-08-12 15:23:29 UTC
I had a look last month, and it was not ready, but it looks like
there is a new release now. I'll try to update and let you know.
(I don't have a lot of time currently, but I should be done in one
week at most).

Comment 10 Jason Tibbitts 2008-12-02 23:30:39 UTC
Is there any chance of closing this now, nine months after the review has been completed?

Comment 11 Fedora Update System 2008-12-03 11:37:05 UTC
grib_api-1.6.4-1.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/grib_api-1.6.4-1.fc10

Comment 12 Patrice Dumas 2008-12-03 15:38:46 UTC
The release I mentionned in Comment #9 has still tests failing, but the latest release builds and tests fine! Thanks for your patience and remainders.

Comment 13 Fedora Update System 2008-12-24 12:58:59 UTC
grib_api-1.6.4-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2008-12-24 18:41:26 UTC
grib_api-1.6.4-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 15 Jason Tibbitts 2009-03-17 22:39:08 UTC
*** Bug 490757 has been marked as a duplicate of this bug. ***

Comment 16 Orion Poplawski 2010-11-17 22:20:17 UTC
I'd like to see this in EPEL 5/6.  Willing to own it if needed.

Comment 17 Orion Poplawski 2010-12-06 23:52:00 UTC
New Package SCM Request
=======================
Package Name: grib_api
Short Description: ECMWF encoding/decoding GRIB software
Owners: orion
Branches: el6 el5
InitialCC:

Comment 18 Orion Poplawski 2010-12-06 23:53:13 UTC
Sorry -

Package Change Request
======================
Package Name: grib_api
New Branches: el6 el5
Owners: orion

Comment 19 Jens Petersen 2010-12-09 00:36:52 UTC
Can we have an ack from the owner please?

Comment 20 Patrice Dumas 2011-02-09 13:51:30 UTC
Sorry, I didn't read my bug for some time.  Of course Orion can own grib_api for whatever branch he wants.

Comment 21 Orion Poplawski 2011-02-09 15:42:23 UTC
Package Change Request
======================
Package Name: grib_api
New Branches: el6 el5
Owners: orion

Comment 22 Jason Tibbitts 2011-02-10 16:41:31 UTC
Git done (by process-git-requests).


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