Opened 17 years ago

Closed 9 years ago

Last modified 9 years ago

#1587 closed defect (invalid)

Error compiling gdal-grass against gdal 1.4.1

Reported by: perrygeo Owned by: warmerdam
Priority: normal Milestone: 1.11.0
Component: ConfigBuild Version: 1.4.1
Severity: normal Keywords: grass
Cc: warmerdam, Markus 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 (1)

plugin_patch_grass63.diff (1.3 KB ) - added by Markus Neteler 17 years ago.
Patch to make GRASS Plugin compile with recent GRASS 6.3-CVS change

Download all attachments as: .zip

Change History (29)

comment:1 by warmerdam, 17 years ago

Cc: warmerdam added
Component: defaultConfigBuild
Keywords: grass added
Owner: changed from warmerdam to Mateusz Łoskot

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.

comment:2 by Markus Neteler, 17 years ago

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

by Markus Neteler, 17 years ago

Attachment: plugin_patch_grass63.diff added

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

comment:3 by Mateusz Łoskot, 17 years ago

Status: newassigned

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?

comment:4 by Markus Neteler, 17 years ago

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

comment:5 by Mateusz Łoskot, 17 years ago

Resolution: fixed
Status: assignedclosed

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!

comment:6 by Markus Neteler, 17 years ago

Resolution: fixed
Status: closedreopened

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

comment:7 by Mateusz Łoskot, 17 years ago

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?

comment:8 by Markus Neteler, 17 years ago

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

comment:10 by Mateusz Łoskot, 17 years ago

Resolution: fixed
Status: reopenedclosed

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?

comment:11 by warmerdam, 17 years ago

Resolution: fixed
Status: closedreopened

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.

comment:12 by Markus Neteler, 17 years ago

Hi Mateusz,

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

thanks, Markus

comment:13 by Mateusz Łoskot, 17 years ago

Fixes applied to the stable branch 1.4 (r11554).

comment:14 by Mateusz Łoskot, 17 years ago

Resolution: fixed
Status: reopenedclosed

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).

comment:15 by Mateusz Łoskot, 17 years ago

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

comment:16 by Markus Neteler, 17 years ago

Have done so and edited the page a bit.

comment:17 by perrygeo, 16 years ago

Resolution: fixed
Status: closedreopened

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

comment:18 by perrygeo, 16 years ago

Oops. Trac wikified the code. It should read:

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

comment:19 by perrygeo, 16 years ago

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

comment:20 by Mateusz Łoskot, 16 years ago

Status: reopenednew

comment:21 by Mateusz Łoskot, 16 years ago

Status: newassigned

comment:22 by warmerdam, 14 years ago

Owner: changed from Mateusz Łoskot to warmerdam
Status: assignednew

reopened, not sure of the status or when it will be investigated again.

comment:23 by warmerdam, 14 years ago

I mean reassigned, not reopened, sorry.

comment:24 by Jukka Rahkonen, 10 years ago

This ticket has been closed and opened very often. What might be the situation now? I can find instructions for compiling the plugin http://grasswiki.osgeo.org/wiki/Compile_and_install_GDAL-GRASS_plugin and a rather old on here http://trac.osgeo.org/gdal/wiki/GRASS

comment:25 by Markus Neteler, 10 years ago

Milestone: 1.11.0

The extracted package is very old:

http://download.osgeo.org/gdal/
...
[ ]	gdal-grass-1.4.1.tar.gz	18-May-2007 10:26 	46K 
[ ]	gdal-grass-1.4.3.tar.gz	05-Aug-2007 14:43 	46K

... not reflecting the current state from within GDAL/OGR.

Sidenote: there is now an initial GRASS GIS 7 release: http://trac.osgeo.org/grass/wiki/Release/7.0.0beta-News

comment:26 by Jukka Rahkonen, 9 years ago

Markus and others, if this issue still exists, could it be possible to write a new ticket that describes better the situation as it is today? After that this 8 years old ticket could be closed. Otherwise it will just stay open and gather dust here till eternity.

comment:27 by Even Rouault, 9 years ago

Resolution: invalid
Status: newclosed

Martin Landa has applied fixes for GRASS 7 support last week. And regenerating gdal-grass archive is in the checklist of HOWTO-RELEASE, although we have probably neglected to do it for some time... We should do it for GDAL 2.0 for GRASS 7 support. I think we can close this ticket now as it doesn't bring much value to keep it open.

comment:28 by Markus Neteler, 9 years ago

See also #2953 (Patch for GDAL-GRASS plugin for GRASS 7)

Note: See TracTickets for help on using tickets.