Bug 215445 - Review Request: netcdf-decoder Converts WMO GRIB products into NetCDF files
Review Request: netcdf-decoder Converts WMO GRIB products into NetCDF files
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jochen Schmitt
Fedora Package Reviews List
http://www.cora.nwra.com/~orion/fedora/
:
Depends On: 215444
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-13 18:46 EST by Orion Poplawski
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-29 13:01:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Orion Poplawski 2006-11-13 18:46:30 EST
Spec Name or Url: http://www.cora.nwra.com/~orion/fedora/netcdf-decoders.spec
SRPM Name or Url:
http://www.cora.nwra.com/~orion/fedora/netcdf-decoders-4.1.4-1.fc6.src.rpm
Description: 

The NetCDF Decoders package converts WMO GRIB products and textual bulletins
into NetCDF formatted files. This package usually works in conjunction with
the LDM package to covert incoming products to the NetCDF format without the
need to store the raw data. The gribtonc decoder converts most models in the
GRIB format. There are perl decoders for the following textual report types:
METAR, buoy, upper air, and synoptic products. This package requires the
installation of NetCDF, UDUNITS, and the Perl-NetCDF packages to work.
Comment 1 Jochen Schmitt 2006-11-14 12:03:16 EST
God:
+ Rpmlint doesn't complaints source package.
+ Local build works fine.
+ rpmlint doesn't complaints binary package.
+ Mock build works fine.
+ Tarbar in source RPM matches with upstream.
+ RPM contains verbatin copy of the license text.
+ License text seem to be ok for free software

Bad:
- Can't install becouse perlNetCDF) is required.



Comment 2 Orion Poplawski 2006-11-16 13:55:36 EST
> Bad:
> - Can't install becouse perlNetCDF) is required.

perl(NetCDF) is under review in bug 215444, which this depends on.

Thanks!
Comment 3 Orion Poplawski 2007-01-25 12:49:30 EST
Jochen - do you still want to review?  perl(NetCDF) is now available.
Comment 4 Jochen Schmitt 2007-01-25 14:09:04 EST
Good:
+ Tarball in source RPM match with upstream.
+ rpmlint is quite on source rpm.
+ Local build works fine.
+ rpmlint is quite on binary rpm.
+ Local install and uninstalling works fine.

Bad:
- The %makeinstall macro should not be used
  (wiki: PackagingGuidelines#MakeInstall)

Sorry, I have to interupt, becouse I have a install problem with mock on my new
machine.
Comment 5 Orion Poplawski 2007-01-25 14:16:00 EST
(In reply to comment #4)
> Bad:
> - The %makeinstall macro should not be used
>   (wiki: PackagingGuidelines#MakeInstall)

The makefiles do not support DESTDIR, and I really don't want to patch them to
do so.
Comment 6 Jochen Schmitt 2007-01-25 14:28:00 EST
I will cite:

Why the %makeinstall macro should not be used

Fedora's RPM includes a %makeinstall macro but it must NOT be used when make
install DESTDIR=%{buildroot} will work. %makeinstall is a kludge that can work
with Makefiles that don't make use of the DESTDIR variable but it has the
following potential issues:

    *

      %makeinstall overrides a set of environment variables during "make
install". I.e. it performs make prefix="..." includedir="..." ...
    *

      It is error prone, and can have unexpected effects when run against less
than perfect Makefiles.
    *

      It can trigger unnecessary rebuilds when executing "make install"
    *

      If a package contains libtool archives, it can cause broken *.la files to
be installed.

Instead, Fedora packages should use: make DESTDIR=%{buildroot} install or make
DESTDIR=$RPM_BUILD_ROOT install

Thsi is a list of reasons why %Makeinstall should not used.

Sory for any inconviniences.
Comment 7 Jochen Schmitt 2007-01-25 14:30:08 EST
At last I have a positiv message for you.

I have solve my moch issue and can report a positve build of your package on mock.

Best Regards:

Jochen Schmitt
Comment 8 Orion Poplawski 2007-01-25 15:14:55 EST
(In reply to comment #6)
> I will cite:
> 
> Why the %makeinstall macro should not be used
> 
> Fedora's RPM includes a %makeinstall macro but it must NOT be used when make
> install DESTDIR=%{buildroot} will work.
                               ^^^^^^^^^

And I say again, DESTDIR does NOT work with this package, and %makeinstall does.
  If someone can show me a clean way to add DESTDIR support, that would be nice.
 But the configure system used in this package is very outdated and I don't
think it's work spending time on it when a reasonable work around is available.
Comment 9 Jochen Schmitt 2007-01-28 14:15:10 EST
Sorry, You are right. I wil APPROVE you package.
Comment 10 Orion Poplawski 2007-01-29 12:59:09 EST
Thanks.  Checked it and built.  Added to owners.list.  Requesting branches...

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