Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#364 closed defect (fixed)

Compiling QGIS from svn has a compiler error related to GRASS api

Reported by: cgsbob Owned by: grass-dev@…
Priority: major Milestone: 6.4.0
Component: Default Version: svn-trunk
Keywords: Cc: timlinux
CPU: x86-64 Platform: Linux

Description

Compiling QGIS 1.0 Preview 2 with Grass 6.4.svn on X86_64 Ubuntu Hardy Heron system gives me this compiler error:

[ 64%] Building CXX object src/providers/grass/CMakeFiles/qgisgrass.dir/qgsgrassprovider.o
/usr/local/grass-6.4.svn/include/grass/Vect.h: In static member function ‘static int QgsGrassProvider::openMap(QString, QString, QString, QString)’:
/usr/local/grass-6.4.svn/include/grass/Vect.h:197: error: too many arguments to function ‘int Vect_build(Map_info*)’
/home/bobm/src/gis/qgis_svn/src/providers/grass/qgsgrassprovider.cpp:1043: error: at this point in file
/usr/local/grass-6.4.svn/include/grass/Vect.h: In member function ‘bool QgsGrassProvider::closeEdit(bool)’:
/usr/local/grass-6.4.svn/include/grass/Vect.h:199: error: too many arguments to function ‘int Vect_build_partial(Map_info*, int)’
/home/bobm/src/gis/qgis_svn/src/providers/grass/qgsgrassprovider.cpp:1522: error: at this point in file
/usr/local/grass-6.4.svn/include/grass/Vect.h:197: error: too many arguments to function ‘int Vect_build(Map_info*)’
/home/bobm/src/gis/qgis_svn/src/providers/grass/qgsgrassprovider.cpp:1523: error: at this point in file
make[2]: *** [src/providers/grass/CMakeFiles/qgisgrass.dir/qgsgrassprovider.o] Error 1
make[1]: *** [src/providers/grass/CMakeFiles/qgisgrass.dir/all] Error 2

I've already contacted the QGIS devs and they say it is likely a Grass problem.

Attachments (2)

qgsgrassprovider.diff (910 bytes) - added by neteler 11 years ago.
QGIS patch
qgis_grass_api_prob.diff (1.9 KB) - added by neteler 11 years ago.
fix the fix

Download all attachments as: .zip

Change History (16)

comment:1 Changed 11 years ago by cgsbob

After a little digging I found the problem...see http://trac.osgeo.org/grass/changeset/34288#file2. Since this api change is intentional, it seems like QGIS would have to change?

Btw, the problem line in QGIS looks like this:

    Vect_build( map.map, stderr );

comment:2 in reply to:  1 Changed 11 years ago by glynn

Replying to cgsbob:

After a little digging I found the problem...see http://trac.osgeo.org/grass/changeset/34288#file2. Since this api change is intentional, it seems like QGIS would have to change?

Unless someone reverts that change. I made the corresponding change (r34284) in 7.0, but I wasn't expecting it to be backported to 6.4.

Ultimately, I don't know how important API stability is supposed to be in 6.4. I know that we've never made any effort to maintain ABI stability (in fact, we go out of our way to break it with the GIS_H_VERSION check).

comment:3 Changed 11 years ago by neteler

A rather simple solution could be to test the version as already done in src/plugins/grass/qgsgrassnewmapset.cpp:

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

The problem with backports is that commonly indication is lacking which change to backport. Usually it goes well, this time it has external impact.

Markus

comment:4 in reply to:  3 ; Changed 11 years ago by glynn

Replying to neteler:

A rather simple solution could be to test the version as already done in src/plugins/grass/qgsgrassnewmapset.cpp:

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

That won't distinguish between 6.4 as of a week ago and 6.4 as of today.

OTOH, it will work fine if external code only ever uses release versions.

comment:5 in reply to:  4 ; Changed 11 years ago by hamish

Replying to glynn:

Replying to neteler:

A rather simple solution could be to test the version as already done in src/plugins/grass/qgsgrassnewmapset.cpp:

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

That won't distinguish between 6.4 as of a week ago and 6.4 as of today.

OTOH, it will work fine if external code only ever uses release versions.

We can't be expected to guarantee that development snapshots will be free of changes, and folks who base stuff on random checkouts from dev branches should realize that.

where do we have to test/warn? qgis-grass plugin and the gdal(ogr)-grass plugin? jGRASS?

+1 for

src/plugins/grass/qgsgrassnewmapset.cpp:
#if GRASS_VERSION_MAJOR == 6 && GRASS_VERSION_MINOR >= 4

solution.

Hamish

comment:6 in reply to:  4 Changed 11 years ago by neteler

Replying to glynn:

Replying to neteler:

A rather simple solution could be to test the version as already done in src/plugins/grass/qgsgrassnewmapset.cpp:

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

That won't distinguish between 6.4 as of a week ago and 6.4 as of today.

This is not a real problem since we are in the development branch of GRASS 6. Source snapshots are already updated.

OTOH, it will work fine if external code only ever uses release versions.

... which should be the scope for the QGIS release.

Markus

comment:7 Changed 11 years ago by neteler

Cc: timlinux added

QGIS patch attached to work around the Vect_build() function call change. I dunno how to define those variables, though but that will be easy.

Markus

Changed 11 years ago by neteler

Attachment: qgsgrassprovider.diff added

QGIS patch

comment:8 in reply to:  5 ; Changed 11 years ago by glynn

Replying to hamish:

where do we have to test/warn? qgis-grass plugin and the gdal(ogr)-grass plugin? jGRASS?

What's the situation with grass-addons? Is there a specific release or branch which they are supposed to be compatible with, or does it vary from module to module?

comment:9 in reply to:  7 Changed 11 years ago by jef

Resolution: fixed
Status: newclosed

Replying to neteler:

QGIS patch attached to work around the Vect_build() function call change. I dunno how to define those variables, though but that will be easy.

applied in QGIS r9655

comment:10 Changed 11 years ago by jef

and https://trac.osgeo.org/qgis/changeset/9656 should be the correct fix pointing to the right TRAC.

comment:11 in reply to:  8 Changed 11 years ago by hamish

Replying to glynn:

What's the situation with grass-addons? Is there a specific release or branch which they are supposed to be compatible with, or does it vary from module to module?

AFAIK they are all grass 6.x, some of them use newer things like g.message, others not. Porting between grass 6.x should be trivial, so IMO there is no need to break it up further than it is.

I have just created grass5/ and grass7/ dirs, and eventually the current root dirs should be moved into a grass6/. Hopefully the presence of the new dirs should make it obvious to commiters where they should be putting stuff.

Hamish

comment:12 in reply to:  10 ; Changed 11 years ago by cgsbob

I just updated r9656, but I'm still having problems. I've attached to this ticket my hard coded fix. I can not find where GRASS_VERSION_MAJOR and GRASS_VERSION_MINOR are defined in qgis, so I added them to each problem file.

Replying to jef:

and https://trac.osgeo.org/qgis/changeset/9656 should be the correct fix pointing to the right TRAC.

comment:13 in reply to:  12 Changed 11 years ago by neteler

Replying to cgsbob:

I just updated r9656, but I'm still having problems. I've attached to this ticket my hard coded fix. I can not find where GRASS_VERSION_MAJOR and GRASS_VERSION_MINOR are defined in qgis, so I added them to each problem file.

Find attached a non-hardcoded patch to fix the outstanding problems.

Markus

Changed 11 years ago by neteler

Attachment: qgis_grass_api_prob.diff added

fix the fix

comment:14 Changed 11 years ago by neteler

Remaining things have been submitted to QGIS SVN and QGIS now compiles again against various GRASS versions incl. 6.4.svn and 7.svn.

Note: See TracTickets for help on using tickets.