Ticket #1587 (assigned defect)

Opened 1 year ago

Last modified 3 months ago

Error compiling gdal-grass against gdal 1.4.1

Reported by: perrygeo Assigned to: mloskot (accepted)
Priority: normal Milestone:
Component: ConfigBuild Version: 1.4.1
Severity: normal Keywords: grass
Cc: warmerdam, neteler

Description

Using gdal 1.4.1 and gdal-grass 1.3.2

I ran into some minor troubles getting gdal-grass plugin (version 1.3.2) installed on gdal: 

./configure --with-gdal=/usr/local/bin/gdal-config
--with-grass=/usr/local/grass-6.3.cvs

g++ -Wall -fPIC  -DUSE_CPL
-DGRASS_GISBASE=\"/usr/local/grass-6.3.cvs\" -I/usr/local/include
-I/usr/local/grass-6.3.cvs/include   -c -o grass57dataset.o
grass57dataset.cpp
grass57dataset.cpp: In static member function 'static GDALDataset*
GRASSDataset::Open(GDALOpenInfo*)':
grass57dataset.cpp:828: error: invalid conversion from 'int (*)(char*,
int)' to 'int (*)(const char*, int)'
grass57dataset.cpp:828: error:   initializing argument 1 of 'int
G_set_error_routine(int (*)(const char*, int))'
make: *** [grass57dataset.o] Error 1

Mateusz writes: It seems prototype of function pointer has changed and first parameter has been redefined from non-const to const.

For the record, changing the following lines seems to fix it. 

# grass57dataset.cpp: 799
typedef int (*GrassErrorHandler)(const char *, int);

# ogrgrassdatasource:91
typedef int (*GrassErrorHandler)(const char *, int);

I can't confirm that the above changes work but at they compile at the very least ;-)

Mateusz writes: Yes, that will work, but probably it also will break backward compatibility, I suppose.

Attachments

plugin_patch_grass63.diff (1.3 kB) - added by neteler on 04/20/07 16:12:32.
Patch to make GRASS Plugin compile with recent GRASS 6.3-CVS change

Change History

04/20/07 13:15:51 changed by warmerdam

  • keywords set to grass.
  • owner changed from warmerdam to mloskot.
  • component changed from default to ConfigBuild.
  • cc set to warmerdam.

Mateusz,

Can you dig into this, with an eye to backward compatibility as well.

When you are happy with updates in trunk we can issue a new gdal-grass plugin release.

04/20/07 16:05:20 changed by neteler

A recent change in the GRASS include/gisdefs.h file triggers this error. Perhaps it needs to be conditionalized on GRASS >=6.3. 

Version is available in include/version.h
static char *GRASS_VERSION_MAJOR   = "6";
static char *GRASS_VERSION_MINOR   = "3";
static char *GRASS_VERSION_RELEASE = "cvs";

Best, Markus

04/20/07 16:12:32 changed by neteler

  • attachment plugin_patch_grass63.diff added.

Patch to make GRASS Plugin compile with recent GRASS 6.3-CVS change

04/20/07 16:27:57 changed by mloskot

  • status changed from new to assigned.

From the irc chat today: 

<glynnc>-int Grass2CPLErrorHook( char * pszMessage, int bFatal )
<glynnc> +int Grass2CPLErrorHook( const char * pszMessage, int bFatal )
<glynnc> At the top of grass57dataset.cpp
<FrankW> Would that not be broken with older versions of GRASS? 
<glynnc> Yes

Markus, Glynn, are you sure it won't break the compatibility?

There are two solutions possible:

  • Support both const and non-const versions with conditional compilation
  • Use only the new version with hope it will work with older GRASS versions

Friends, which one do you recommend?

04/21/07 04:35:33 changed by neteler

Mateusz,

the only solution is to support both const and non-const versions with conditional compilation based on (pseudocode): 

GRASS_VERSION_MAJOR  >= 6 && GRASS_VERSION_MINOR  >= 3

So far there was never a 6.3.x release which helps in this regard. Everything before 6.3.0 will then use the version without const and the first 6.3.0 release in future will take the const variant.

Boosting the plugin version to 1.4.something will also indicate that it is related to the new GDAL release.

Thanks, Markus

04/23/07 05:48:30 changed by mloskot

  • status changed from assigned to closed.
  • resolution set to fixed.

Markus,

I added support for the new prototype and tested it with GRASS 5.x (r11338). Later, I'll try to build GRASS CVS and test it with this version too.

If you'll notice any problems with this fix, please reopen the ticket.

Perry, Markus, thanks for your help!

04/23/07 06:02:07 changed by neteler

  • status changed from closed to reopened.
  • resolution deleted.

Mateusz,

sorry to say, but it fails: 

config.status: creating Makefile
g++ -Wall -fPIC  -DUSE_CPL -DGRASS_GISBASE=\"/home/neteler/grass63/dist.i686-pc-linux-gnu\" -I/usr/local/include -I/home/neteler/grass63/dist.i686-pc-linux-gnu/include   -c -o grass57dataset.o grass57dataset.cpp
grass57dataset.cpp: In static member function ?static GDALDataset* GRASSDataset::Open(GDALOpenInfo*)?:
grass57dataset.cpp:821: error: invalid conversion from ?int (*)(char*, int)? to ?int (*)(const char*, int)?
grass57dataset.cpp:821: error:   initializing argument 1 of ?int G_set_error_routine(int (*)(const char*, int))?
make: *** [grass57dataset.o] Fehler 1
g++ -Wall -fPIC  -DUSE_CPL -DGRASS_GISBASE=\"/home/neteler/grass63/dist.i686-pc-linux-gnu\" -I/usr/local/include -I/home/neteler/grass63/dist.i686-pc-linux-gnu/include   -c -o grass57dataset.o grass57dataset.cpp
grass57dataset.cpp: In static member function ?static GDALDataset* GRASSDataset::Open(GDALOpenInfo*)?:
grass57dataset.cpp:821: error: invalid conversion from ?int (*)(char*, int)? to ?int (*)(const char*, int)?
grass57dataset.cpp:821: error:   initializing argument 1 of ?int G_set_error_routine(int (*)(const char*, int))?
make: *** [grass57dataset.o] Error 1

Because: 

G_set_error_routine /home/neteler/grass63/include/gisdefs.h
int G_set_error_routine(int (*)(const char *, int));

I assume that the trick is that GRASS_VERSION_MAJOR is a string. 

grep GRASS_VERSION_MAJOR /home/neteler/grass63/include/version.h
static char *GRASS_VERSION_MAJOR   = "6";

The current version might be needed to be modified to take care of this fact.

Best, Markus

04/25/07 11:58:03 changed by mloskot

Markus,

Would it be possible to have version definition as a preprocessor macro instead of character object?

Ideally, I think it would be best if there is a version macro defined as a number calculated as: 

MAJOR * 100000 + MINOR * 100 + MICRO

what for 6.3.0 results as 600300.

This way we have a value that can be easily calculated from version string and makes it very easy to compare if one version is lower/higher from another one.

What do you think?

04/25/07 17:25:06 changed by neteler

Mateusz,

I have posted this to the grass-dev list for discussion since I have no real answer myself:

http://grass.itc.it/pipermail/grass-dev/2007-April/030623.html

Will report here.

Markus

04/26/07 15:15:28 changed by neteler

  • cc changed from warmerdam to warmerdam, neteler.

Here the change from Glynn:

http://freegis.org/cgi-bin/viewcvs.cgi/grass6/include/version.h.in.diff?r1=1.4&r2=1.5

And a comment from Glynn:

http://grass.itc.it/pipermail/grass-dev/2007-April/030658.html

Markus

04/27/07 01:51:11 changed by mloskot

  • status changed from reopened to closed.
  • resolution set to fixed.

GRASS drivers has been fixed (r11365) according to new prototypes in GRASS 6.3.

TODO: Now, it's time to prepare new version of gdal-grass plugin. What's the procedure?

04/27/07 15:30:18 changed by warmerdam

  • status changed from closed to reopened.
  • resolution deleted.

Mateusz,

I'm reopening this bug till we get the new plugin dist produced. In theory you should be able to go into gdal/frmts/grass, and do "make dist" to produce a new plugin with the raster and vector code. This would then be uploaded to download.osgeo.org/gdal, and tested.

I think we should also setup a wiki topic about GRASS building, and use of the plugin at: http://trac.osgeo.org/gdal/wiki/BuildHints which should point to where to download them, and talk about versioning and other details.

I assuming the plugin produced would work fine with any of the GDAL 1.4.x versions and trunk.

05/16/07 10:17:12 changed by neteler

Hi Mateusz,

would it be possible to release the gdal-ogr-grass plugin?

thanks, Markus

05/18/07 13:09:50 changed by mloskot

Fixes applied to the stable branch 1.4 (r11554).

05/18/07 13:31:33 changed by mloskot

  • status changed from reopened to closed.
  • resolution set to fixed.

Folks,

Here is new package for the plugin: gdal-grass-1.4.1.tar.gz. The package was generated from the stable branch 1.4, so all fixes will be automatically included in upcoming release GDAL 1.4.2.

I'd appreciate if someone can try & test and confirm if it works (or not).

05/18/07 19:25:59 changed by mloskot

Here is small wiki explaining how to enable GRASS support in GDAL: GRASS linked from BuildHints. Please, evaluate and comment if you can.

05/19/07 03:24:12 changed by neteler

Have done so and edited the page a bit.

05/11/08 15:56:36 changed by perrygeo

  • status changed from closed to reopened.
  • resolution deleted.

The latest grass svn uses version 7.0 so the compiling against gdal-grass-1.4.3 fails due to:

#if GRASS_VERSION_MAJOR >= 6 && GRASS_VERSION_MINOR >= 3

should probably be:

#if (GRASS_VERSION_MAJOR >= 6 && GRASS_VERSION_MINOR >= 3) GRASS_VERSION_MAJOR >= 7

05/11/08 15:57:44 changed by perrygeo

Oops. Trac wikified the code. It should read: 

#if (GRASS_VERSION_MAJOR >= 6 && GRASS_VERSION_MINOR >= 3) GRASS_VERSION_MAJOR >= 7

05/11/08 15:58:25 changed by perrygeo

OK. I give up .. there should be an OR operator in there .. you get the idea!

05/11/08 17:02:36 changed by mloskot

  • status changed from reopened to new.

05/11/08 17:02:42 changed by mloskot

  • status changed from new to assigned.