Bug 658234 - Review Request: gdal-grass - Standalone GRASS 6 Drivers for GDAL and OGR
Summary: Review Request: gdal-grass - Standalone GRASS 6 Drivers for GDAL and OGR
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 655612
TreeView+ depends on / blocked
 
Reported: 2010-11-29 18:45 UTC by viji
Modified: 2012-04-25 14:06 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-25 14:06:31 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description viji 2010-11-29 18:45:00 UTC
Spec URL: http://viji.fedorapeople.org/SPECS/gdal-grass.spec
SRPM URL: http://viji.fedorapeople.org/SRPMS/gdal-grass-1.4.3-1.fc14.src.rpm
Description: This package contains standalone drivers for GRASS raster and vector files that can be built after GDAL has been built and installed as an
"autoload" driver.

This is particularly useful in resolving problems with GRASS depending
on GDAL, but GDAL with GRASS support depending on GRASS.  With this
package you can configure and install GDAL normally (--without-grass), then
build and install GRASS normally and finally build and install this driver.

To build this driver it is necessary for it to find GDAL and GRASS support
files.

Comment 1 viji 2010-11-29 20:05:46 UTC
Koji F14 Scratch
----------------
http://koji.fedoraproject.org/koji/taskinfo?taskID=2632968

$ rpmlint /home/viji/rpmbuild/SRPMS/gdal-grass-1.4.3-1.fc14.src.rpm
gdal-grass.src: W: spelling-error %description -l en_US autoload -> auto load, auto-load, autolysis
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint /home/viji/rpmbuild/RPMS/x86_64/gdal-grass-1.4.3-1.fc14.x86_64.rpm
gdal-grass.x86_64: W: spelling-error %description -l en_US autoload -> auto load, auto-load, autolysis
gdal-grass.x86_64: W: non-conffile-in-etc /etc/ld.so.conf.d/gdal-grass.conf
gdal-grass.x86_64: W: non-conffile-in-etc /etc/profile.d/gdal-grass.sh
gdal-grass.x86_64: W: dangerous-command-in-%post cp
gdal-grass.x86_64: W: dangerous-command-in-%postun rm
1 packages and 0 specfiles checked; 0 errors, 5 warnings.

$ rpmlint /home/viji/rpmbuild/RPMS/x86_64/gdal-grass-debuginfo-1.4.3-1.fc14.x86_64.rpm
gdal-grass-debuginfo.x86_64: E: debuginfo-without-sources
1 packages and 0 specfiles checked; 1 errors, 0 warnings.

Comment 2 Volker Fröhlich 2010-12-03 13:03:13 UTC
"grasspath" is statically set to GRASS version 6.4. The BRs only require it to be 6.3. The specific versions are also likely not necessary, since no supported version of Fedora ships a lower one. I don't know about EPEL though. You can also drop BR grass-libs, since grass-devel requires it.

RPM will create all your Requires itself, so you don't need them. You'll only need GRASS as Require, since the libraries are in an independent package, I think.

Don't mix %{buildroot} and $RPM_BUILD_ROOT on your spec files. Just stick with either. The preferred build root syntax is also the first one from this list: http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag

The -n option is not necessary for %setup, since the package is exactly called as expected. Please replace names and paths with %{name} and %{_bindir} in %configure. It seems to me, the configure options are not needed, besides --with-autoload, maybe.

Drop the last paragraph from the description, as it is of no interest for the user. Maybe even the second too.

The README file only contains information about installation, so it should not be included. The files section can also be simplified to %{_libdir}/%{name} instead of the three lines starting with %{_libdir}.

I didn't look into the install, post and postun sections yet.

Comment 3 viji 2010-12-04 17:59:07 UTC
- The grass path has been changed to a wildcard. This package is also intended for EPEL 5, so the specific versions are required in BR and R. Though the rpm automatically create Requires, here it is necessary for the specific version bindings.

- The last paragraph of the description has been removed, I think we can keep the second as it is, many people may not aware why there is a separate package even the grass support is in-built for GDAL (the bootstrap issue)

Rest everything has been updated, please do a re-review and look at the remaining portions as well

Thank you so much for your support.

Comment 5 viji 2010-12-04 18:17:53 UTC
Regarding the %configure options, I think we need to keep the --with-gdal and --with-grass options as it is not enabled by default, I cloud not find a reference in the code as well.

Comment 6 Volker Fröhlich 2011-01-11 03:27:00 UTC
I'll try to finish the review this week.

Comment 7 Volker Fröhlich 2012-01-15 09:01:23 UTC
I take over this package. I built a version from the GDAL 1.7.3 sources, which is also a lot simpler than the previous approach.

It requires a patched GDAL though, because of http://trac.osgeo.org/gdal/ticket/4444

I'm not sure about all the details, as you can read in the comments.

http://www.geofrogger.net/review/gdal-grass.spec
http://www.geofrogger.net/review/gdal-grass-1.7.3-1.fc16.src.rpm

It is not necessarily ready for a review, but I'd be happy if you commented on it.

I also contacted the Debian maintainer on most of my questions.

Comment 8 Jason Tibbitts 2012-04-24 23:10:09 UTC
If you're taking this over, please open your own review ticket.  There is, however, no evidence in this ticket that the original submitter wanted someone else to take over.  Just a statement that you would finish the review, then a year later a note that you were taking over.  Can you at least indicate what happened?  (Before closing this ticket a duplicate of the new one you're opening, assuming there's no objection from the original submitter.)

Comment 9 Volker Fröhlich 2012-04-25 07:27:43 UTC
Viji is not active for a year anymore.

We had a chat about various things to change for the review but it didn't turn out, if memory serves me right.

I later did most of these changes on my own, but there is some uncertainty about the details. I contacted the developers but they are not completely sure either.

I'll try to work it out at some time and will open a new ticket then. I guess we can therefore close it.


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