Ticket #364 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

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
Platform: Linux CPU: x86-64

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

qgsgrassprovider.diff Download (0.9 KB) - added by neteler 3 years ago.
QGIS patch
qgis_grass_api_prob.diff Download (1.9 KB) - added by neteler 3 years ago.
fix the fix

Change History

follow-up: ↓ 2   Changed 3 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 );

in reply to: ↑ 1   Changed 3 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).

follow-up: ↓ 4   Changed 3 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

in reply to: ↑ 3 ; follow-ups: ↓ 5 ↓ 6   Changed 3 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.

in reply to: ↑ 4 ; follow-up: ↓ 8   Changed 3 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

in reply to: ↑ 4   Changed 3 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

follow-up: ↓ 9   Changed 3 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 3 years ago by neteler

QGIS patch

in reply to: ↑ 5 ; follow-up: ↓ 11   Changed 3 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?

in reply to: ↑ 7   Changed 3 years ago by jef

  • status changed from new to closed
  • resolution set to fixed

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

follow-up: ↓ 12   Changed 3 years ago by jef

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

in reply to: ↑ 8   Changed 3 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

in reply to: ↑ 10 ; follow-up: ↓ 13   Changed 3 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.

in reply to: ↑ 12   Changed 3 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 3 years ago by neteler

fix the fix

  Changed 3 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.