Bug report #3215

qgsclipper code causes trunk to fail to build on OSX

Added by John Tull almost 10 years ago. Updated almost 10 years ago.

Status:Closed
Priority:Low
Assignee:Jürgen Fischer
Category:Build/Install
Affected QGIS version: Regression?:No
Operating System:OS X Easy fix?:No
Pull Request or Patch supplied: Resolution:fixed
Crashes QGIS or corrupts data: Copied to github as #:13275

Description

In the current trunk, there is a problem with either cmath or isnan, perhaps a non-portable use of isnan, that makes qgis fail to build on OS X systems. This is the current error during the make process:

[  8%] Building CXX object src/core/CMakeFiles/qgis_core.dir/qgsclipper.cpp.o
In file included from /Users/jctull/sources/qgis/trunk/src/core/qgsclipper.cpp:21:
/Users/jctull/sources/qgis/trunk/src/core/qgsclipper.h: In static member function ‘static [[QgsPoint]] QgsClipper::intersect(double, double, double, double, [[QgsClipper]]::Boundary)’:
/Users/jctull/sources/qgis/trunk/src/core/qgsclipper.h:271: error: call of overloaded ‘abs(double&)’ is ambiguous
/usr/include/stdlib.h:146: note: candidates are: int abs(int)
/usr/include/c++/4.2.1/cstdlib:143: note:                 long int std::abs(long int)
/usr/include/c++/4.2.1/cstdlib:174: note:                 long long int +gnu_cxx::abs(long long int)
/Users/jctull/sources/qgis/trunk/src/core/qgsclipper.h:271: error: call of overloaded ‘abs(double&)’ is ambiguous
/usr/include/stdlib.h:146: note: candidates are: int abs(int)
/usr/include/c++/4.2.1/cstdlib:143: note:                 long int std::abs(long int)
/usr/include/c++/4.2.1/cstdlib:174: note:                 long long int +gnu_cxx::abs(long long int)
maker2: *** [src/core/CMakeFiles/qgis_core.dir/qgsclipper.cpp.o] Error 1
maker1: *** [src/core/CMakeFiles/qgis_core.dir/all] Error 2
make: *** [all] Error 2

bugtest3215.diff Magnifier - OS X isnan error fix. (2.95 KB) John Tull, 2010-11-18 10:54 AM

math.diff Magnifier - does qgis still build on osx with the attached patch? (81.1 KB) Jürgen Fischer, 2010-11-18 04:07 PM

History

#1 Updated by John Tull almost 10 years ago

I have verified that this bug was introduced with http://trac.osgeo.org/qgis/changeset/14554
Reverting to 06305bc4 (SVN r14554) builds fine.

#2 Updated by John Tull almost 10 years ago

...although, simply removing the code from 14554 in trunk does not get the code to build. In fact, I jumped the gun. 358e4349 (SVN r14555) builds also... If I can pinpoint the code change that leads to the crash (accurately) I will report it here.

#3 Updated by John Tull almost 10 years ago

Ok, it is f275cd2e (SVN r14602) that causes the failure. It looks like the includes were moved around a bit, and that is somehow negatively affecting OSX.

#4 Updated by John Tull almost 10 years ago

I've attached a patch that works for me. This needs to be worked on to define changes specific to OSX, but should suffice for testing purposes. I'm not sure if this will work for systems older than Snow Leopard, so please test.

#5 Updated by John Tull almost 10 years ago

I updated the patch to test for Q_OS_MACX and use std::isnan or std::isinf, so this should be a usable patch for qgis. This still needs testing from older OS's and, possibly, ppc builds.

#6 Updated by Jürgen Fischer almost 10 years ago

  • Status changed from Open to Closed
  • Resolution set to fixed

2a3a087e (SVN r14712) should also fix it. Looks like Qt has what we need.

#7 Updated by William Kyngesburye almost 10 years ago

There are more places in Qgis that use abs(). Maybe should be changed for consistency? After 2a3a087e (SVN r14712) I found:

  • core/composer/qgscomposeritem.cpp
  • core/qgscoordinatereferencesystem.cpp
  • core/qgsmaprender.cpp (acually fabs())
  • core/qgspoint.cpp (right above changes made in 2a3a087e (SVN r14712))
  • core/spatialindex/geometry/LineSegment.cc (though this seems to be independent of Qt/Qgis)
  • core/spatialindex/rtree/Node.cc (same as previous)
  • gui/qgsannotationitem.cpp
  • plugins/georeferencer/qgsgeoreftransform.cpp
  • plugin/grass/qgsgrassselect.cpp
  • plugin/grass/qtermwidget/TerminalDisplay.cpp

For older OS X versions, if it's in Qt I think it should be OK. I can test back to 10.5 only. Qt "official" binaries support back to 10.4, but the C++ version is almost identical to 10.5 (4.0.x), if that means anything.

#8 Updated by John Tull almost 10 years ago

The math.diff patch applied to 8868c952 (SVN r14713) worked fine for me. Thanks for sorting all of this out!

Also available in: Atom PDF