Bug 257701 - Review Request: w3lib - GRIB1 encoder/decoder and search/indexing routines
Summary: Review Request: w3lib - GRIB1 encoder/decoder and search/indexing routines
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 373861
Blocks: 257781
TreeView+ depends on / blocked
 
Reported: 2007-08-27 19:32 UTC by Patrice Dumas
Modified: 2008-12-04 12:04 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-04 12:04:15 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Patrice Dumas 2007-08-27 19:32:54 UTC
Spec URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib.spec
SRPM URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib-1.4-1.fc8.src.rpm
Description: 

This library contains Fortran 90 decoder/encoder
routines for GRIB edition 1, general date manipulation
routines, and a Fortran 90 interface to "C"
language I/O routines.  The user API for the GRIB1 routines
is described in file "grib1.doc".

Comment 1 Jason Tibbitts 2007-11-07 21:05:30 UTC
Now that we've decided where the .mod file should live, this review should be
able to move forward.

Unfortunately what I'm not understanding now is why the debuginfo package comes
up empty.

Comment 2 Patrice Dumas 2007-11-07 21:25:36 UTC
I would prefer to have the rpm macro in the macros before this
moves on. And also I haven't seen the FESCo ratification, but 
maybe I missed the FESCo summary.

Comment 3 Jason Tibbitts 2007-11-07 23:23:14 UTC
Well, you could be waiting for some time until the macro is incorporated, and
that may only happen in rawhide.

Yes, FESCo ratified the module directory proposal last Thursday.  I would have
sent a notice to fedora-packaging had the committee any concerns.

Comment 4 Parag AN(पराग) 2008-01-01 02:40:38 UTC
any progress here?
I see that there are couple of reviews of such type waiting for review.

Comment 5 Patrice Dumas 2008-01-08 08:53:12 UTC
I'd like to have rpm macros accepted before proceeding, be it only
to have consistent names.

Comment 6 Patrice Dumas 2008-01-08 08:54:37 UTC
(In reply to comment #1)

> Unfortunately what I'm not understanding now is why the debuginfo package comes
> up empty.

I guess it is Bug 209316?

Comment 7 Patrice Dumas 2008-05-18 16:19:18 UTC
use fmoddir and update to latest:
http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib-1.6-1.fc10.src.rpm

Comment 8 Tom "spot" Callaway 2008-06-04 00:17:59 UTC
I'm concerned about the lack of licensing here. Given the source (NOAA.gov), it
is likely that it is Public Domain, but it is possible the author was only a
consultant, so it is not safe to assume this.

You should email NCEP.NCO.GRIB to confirm the licensing of that code
(and ask them to note it somewhere). A copy of an email confirming the
license/copyright status will suffice to clear this.

Blocking FE-Legal.

Comment 9 Jason Tibbitts 2008-06-04 03:01:36 UTC
I'll go ahead and review this in preparation for the necessary licensing bits to be in place.

I wonder if there's any point in having a debuginfo package if it's going to end up empty.  It doesn't look like that rpm bug is ever going to be fixed, so this package should probably just turn it off.

Could you perhaps define GRIB in the %description?  This from wikipedia seems useful:
  GRIB (GRIdded Binary) is a mathematically concise data format commonly used in 
  meteorology to store historical and forecasted weather data.

That's about all I have to add.

* source files match upstream:
   15ec3353247105367342eb52c658b33568c995c65754363718b34ab447be49ea  
   w3lib-1.6.tar
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
? description could use a little clarification.
* dist tag is present.
* build root is OK.
? no licensing information whatsoever.
* 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.
X debuginfo package is completely empty.
X rpmlint complains about the debuginfo package.
* final provides and requires are sane:
  w3lib-devel-1.6-1.fc10.x86_64.rpm
   w3lib-static = 1.6-1.fc10
   w3lib-devel = 1.6-1.fc10
  =

* %check is not present; no test suite upstream.
* 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.
* no headers.
* no pkgconfig files.
* static libraries present:
   no dynamic libs present, so the statics are OK in the -devel package.
   -static provide is present.   
* no libtool .la files.

Comment 10 Patrice Dumas 2008-06-04 12:42:00 UTC
(In reply to comment #8)
> You should email NCEP.NCO.GRIB to confirm the licensing of that code
> (and ask them to note it somewhere). A copy of an email confirming the
> license/copyright status will suffice to clear this.

The mail clarifying the license for all the NCEP website is in g2clib.
Should I copy iyt in this package, or just use a comment in the spec?

Comment 11 Tom "spot" Callaway 2008-06-04 13:24:56 UTC
Please copy it in the package. Lifting FE-Legal.

Comment 12 Patrice Dumas 2008-06-04 13:45:39 UTC
(In reply to comment #9)

> I wonder if there's any point in having a debuginfo package if it's going to
end up empty.  It doesn't look like that rpm bug is ever going to be fixed, so
this package should probably just turn it off.

I don't think that unuseful complexity should be added in spec file
to work around rpmbuild bugs except when this leads to really problematic
issues (which is not the case here).

> Could you perhaps define GRIB in the %description?  This from wikipedia seems
useful:
>   GRIB (GRIdded Binary) is a mathematically concise data format commonly used in 
>   meteorology to store historical and forecasted weather data.

I have added the explanation of the acronym such that it isn't mistaken 
for something else, but people using GRIB should know what it is.

%description is now:
 This library contains Fortran 90 decoder/encoder routines for
 the GRIB (GRIdded Binary) data format edition 1, general date manipulation
 routines, and a Fortran 90 interface to "C" language I/O routines.
 The user API for the GRIB1 routines is described in file "grib1.doc".

And Summary:
Summary:        GRIB1 (GRIdded Binary) encoder/decoder and search/indexing routines
(both for devel and main package since only the devel package is
present).

http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib-1.6-2.fc10.src.rpm
http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib.spec


Comment 13 Jason Tibbitts 2008-06-04 17:12:06 UTC
This package is ready to go, but I can't approve it as long as it generates an
empty debuginfo package or there's some indication that the rpm issue will be
fixed in the near term.  We have to work with the tools that we have, rpm
brokenness included, and if you won't add a single line plus a comment then
you're just being hilariously inflexible.  It's not even clear that the rpm
folks consider this a bug.

Comment 14 Mamoru TASAKA 2008-06-04 17:30:22 UTC
I am not familiar with this package itself,
but /usr/lib/rpm/find-debuginfo.sh does nothing for static archives. 
So as far as I see this package the only option for this package is 
just not to create debuginfo rpm.

Comment 15 Patrice Dumas 2008-06-04 19:28:49 UTC
(In reply to comment #13)
> This package is ready to go, but I can't approve it as long as it generates an
> empty debuginfo package or there's some indication that the rpm issue will be
> fixed in the near term.  We have to work with the tools that we have, rpm
> brokenness included, and if you won't add a single line plus a comment then
> you're just being hilariously inflexible.  It's not even clear that the rpm
> folks consider this a bug.

I can't see why I am inflexible. I think that shipping empty debuginfo packages
or not should be left to the packager. Apart from the rpmlint output I can't see
any issue with having empty debuginfo packages. Ignoring empty debuginfo
packages is also a way to work with the tools that we have, rpm brokenness
included. The choice to add a single line plus a comment or not should be left
to the packager in my opinion.

Comment 16 Patrice Dumas 2008-06-04 19:35:44 UTC
(In reply to comment #13)
> you're just being hilariously inflexible.

And it is not hilarious. Opposing to having unimportant things forced upon
maintainers is important. I am not criticizing you, if you find that not having
empty debuginfo is important, I completly understand that you insist on avoiding
empty debuginfo. But it is also important to me since it is not the only package
I own with empty debuginfo package and I am not willing to fix them (in addition
to thinking that this should be up to the maintainer).

Comment 17 Jason Tibbitts 2008-06-04 19:37:17 UTC
http://fedoraproject.org/wiki/Packaging/Guidelines#Debuginfo_packages

"
Packages should produce useful -debuginfo packages, or explicitly disable them
when it is not possible to generate a useful one but rpmbuild would do it anyway. 
"


Comment 18 Tom "spot" Callaway 2008-06-04 19:48:40 UTC
Patrice, what possible value does an empty debuginfo package serve? It will only
confuse people who think they need it for debugging.

Please either rationalize the need to keep it, or explicitly disable it.

Comment 19 Patrice Dumas 2008-06-04 20:02:25 UTC
(In reply to comment #18)
> Patrice, what possible value does an empty debuginfo package serve? It will only
> confuse people who think they need it for debugging.

I can't really see who could be confused. Unless I am wrong the static libs
are not stripped so the symbols are in, and the empty debuginfo is unneeded
anyway. And in my opinion no debuginfo package is not very less confusing than
an empty debuginfo package.
 
> Please either rationalize the need to keep it, or explicitly disable it.

No rational. It is just that to me it should not be a requirement. It adds
unnecessary work for the packager, unnecessary clutter in the specfile 
for no practical gain. And the bug is in rpmbuild.

Anyway since this is already in the guidelines (but I think this is an unneeded
rule, should only be a recommendation), updated src.rpm at:
http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib-1.6-3.fc10.src.rpm
http://www.environnement.ens.fr/perso/dumas/fc-srpms/w3lib.spec

Comment 20 Jason Tibbitts 2008-06-05 22:52:43 UTC
Looks good to me now.

APPROVED

Comment 21 Patrice Dumas 2008-06-14 07:53:26 UTC
New Package CVS Request
=======================
Package Name: w3lib
Short Description: GRIB1 (GRIdded Binary) encoder/decoder and search/indexing
routines
Owners: pertusus
Branches: F-9
InitialCC:
Cvsextras Commits: yes


Comment 22 Kevin Fenzi 2008-06-16 16:02:26 UTC
cvs done.

Comment 23 Jason Tibbitts 2008-08-08 16:35:07 UTC
Any reason this is still open?

Comment 24 Patrice Dumas 2008-08-12 14:51:28 UTC
Yes, it is because I don't want to forget that the F-9 build is 
waiting for Bug 373861 to be also done on F-9.

I can close it and reopen another bug if you want to.

Comment 25 Jason Tibbitts 2008-08-12 15:06:26 UTC
It doesn't matter much to me either way, but as long as this ticket is in my bug list I'm going to periodically send pings.

Comment 26 Patrice Dumas 2008-12-04 12:04:15 UTC
Looks like F-9 will be EOL before _ffmoddir is introduced, so I give up on having this package in F-9.

Thanks for th ereview, Tibbs.


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