Pull requests

#3 Declined
Repository
cbuehler cbuehler
Branch
default
Repository
Coin3D Coin3D
Branch
default

Use CMake build system

Author
  1. Christian Bühler
Reviewers
Description

Tested with Windows 7 / MSVC 2013 and Arch Linux / gcc 4.8.2 (together with pull request #3). Could someone please test on Mac (Fabien Spindler?)?

Does not include all the functionality of Autotools yet, but the .dll/.so gets compiled.

Comments (16)

  1. Amit Aronovitch

    Hi Christian,

    I get the same issue with gcc 4.8.1. Fix seems OK.

    However - it seems from the comment in the source that this include statement was left out on purpose. include/Inventor/SbBasic.h, line 100:

    // cc_debugerror_post() is not attempted resolved before the template is // used, hence the missing Inventor/errors/SoDebugError.h #include. This // "trick" does only work portably for functions in the global namespace.

    I do hope the project owners get responsive again, but since there seem to be no activity in the last year, I suggest we try helping each other and get a single working branch integrating the 3 existing pull requests.

    Can you help dig up further to see what they were trying to do, and why it stopped working now? Perhaps we can fix that in a way that would preserve the original intention? (Maybe recent versions of gcc do try to instatiate that template in cases where they did not before? Or maybe the NDEBUG macro should get defined somehow, etc.)

  2. Christian Bühler author

    Hello Amit,

    I stumbled upon the problem only when I tried to build a package for Arch Linux. I do not know much about the internals of Coin. When doing research about the problem, I found out that Debian has a fix for this since a long time (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=667375), and as I did not see any upstream report of this, I simply created this pull request.

    So I guess I am not of much help to find out the original intention.

  3. Roy Walmsley

    Hi Christian,

    I can see where you are coming from, even though I am not yet (I am trying Kubuntu) a Linux user. However, what worries me somewhat is that the SbBasic.h include file is a low level file. SoDebugError.h, and debugerror.h (where the function is actually defined) are the higher level files that include SbBasic.h.

    So are you guys sure that this is the change you want? I can check it out in Windows / Visual Studio, but Is it going to be OK on a Mac?

    Roy

  4. Gerhard R

    Hi everyone,

    I don't think the original 'trick' described in the comment will work on modern C++ compilers anymore. The C++ standard requires compilers to resolve all names in templates that do not depend on template parameters, even if the template is not instantiated. Therefore, new compilers will complain about the code.

    The same error comes up on latest clang (LLVM) compiler on Mac OSX.

    An alternative to the header file inclusion could be a forward declaration. This would not add additional headers to parse.

    cheers, Gerhard

  5. Christian Bühler author

    I tried the variant with a forward declaration, without success. I put this in SbBasic.h:

    COIN_DLL_API void cc_debugerror_post(const char * source, const char * format, ...);
    

    and got

     g++ -DHAVE_CONFIG_H -I../../include -I../../include -I../../src -I../../src -D_FORTIFY_SOURCE=2 -D_REENTRANT -DCOIN_DEBUG=1 -DCOIN_INTERNAL -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -g -fvar-tracking-assignments -g -fvar-tracking-assignments -W -Wall -Wno-unused -Wno-multichar -Woverloaded-virtual -fno-builtin -finline-functions -Wreturn-type -Wchar-subscripts -Wparentheses -MT document.lo -MD -MP -MF .deps/document.Tpo -c document.cpp  -fPIC -DPIC -o .libs/document.o
    In file included from ../../include/Inventor/errors/SoError.h:36:0,
                     from ../../include/Inventor/errors/SoDebugError.h:36,
                     from ../../src/coindefs.h:95,
                     from document.cpp:47:
    ../../include/Inventor/SbBasic.h:100:19: error: previous declaration of 'void cc_debugerror_post(const char*, const char*, ...)' with 'C++' linkage
     COIN_DLL_API void cc_debugerror_post(const char * source, const char * format, ...);
                       ^
    In file included from ../../include/Inventor/errors/SoDebugError.h:37:0,
                     from ../../src/coindefs.h:95,
                     from document.cpp:47:
    ../../include/Inventor/C/errors/debugerror.h:68:83: error: conflicts with new declaration with 'C' linkage
     COIN_DLL_API void cc_debugerror_post(const char * source, const char * format, ...);
                                                                                       ^
    

    So I still think we should add the include.

  6. Bastiaan Veelo

    Since the function declaration is only needed #ifndef NDEBUG, we could wrap the #include with this #ifndef, and maybe move it down to where the comment was (row 100).

    Bastiaan.

  7. Christian Bühler author

    I accidentally updated this pull request to include the changes for CMake and cannot find a way to undo it. It originally only included the fix for missing include file on current GCC. Please let me know if i should create seperate pull requests, or of this is fine.

  8. Fabien Spindler

    I've tried to build with CMake on OSX from the version available on https://bitbucket.org/cbuehler/coin and I come to some errors related to : - COIN_DEBUG wich is not set properly to 0 or 1 as it should be - OpenGL headers need to be set specifically for OSX. They are in OpenGL/gl.h rather then GL/gl.h - GL/glx.h comes only with X11 nor with OpenGL framework. Disabled on OSX

    I've created a PR with the changes https://bitbucket.org/cbuehler/coin/pull-request/1/added-material-to-be-able-to-unsinstall/diff

    I haven't tested but I suspect that the FindCoin3D.cmake file provided with CMake is no more able to find coin. Having a look inside it searches in /Library/Frameworks/Inventor.framework... I don't know if it works on other plaforms then OSX

    Here the errors I encounter before the changes:

    $ cmake -DCMAKE_VERBOSE_MAKEFILE=ON .. [ 0%] Building CXX object src/CMakeFiles/Coin.dir/xml/document.cpp.o cd /Users/fspindle/poub/coin-cmake/coin/build-cmake/src && /usr/bin/c++ -DCoin_EXPORTS -DYY_NO_UNISTD_H -DCOIN_DEBUG="\$<CONFIG:Debug>" -DCOIN_INTERNAL -DGLX_GLXEXT_PROTOTYPES -DHAVE_CONFIG_H -fPIC -I/Users/fspindle/poub/coin-cmake/coin/src -I/Users/fspindle/poub/coin-cmake/coin/include -I/Users/fspindle/poub/coin-cmake/coin/include/Inventor/annex -I/Users/fspindle/poub/coin-cmake/coin/build-cmake -o CMakeFiles/Coin.dir/xml/document.cpp.o -c /Users/fspindle/poub/coin-cmake/coin/src/xml/document.cpp In file included from /Users/fspindle/poub/coin-cmake/coin/src/xml/document.cpp:47: /Users/fspindle/poub/coin-cmake/coin/src/coindefs.h:93:5: error: ':' without preceding '?'

    After modifying the CMakeLists.txt I come to the next error indicating that OpenGL headers are not found. They are in OpenGL/gl.h...

    [ 1%] Building CXX object src/CMakeFiles/Coin.dir/actions/SoAction.cpp.o cd /Users/fspindle/poub/coin-cmake/coin/build-cmake/src && /usr/bin/c++ -DCoin_EXPORTS -DYY_NO_UNISTD_H -DCOIN_DEBUG=0 -DCOIN_INTERNAL -DGLX_GLXEXT_PROTOTYPES -DHAVE_CONFIG_H -fPIC -I/Users/fspindle/poub/coin-cmake/coin/src -I/Users/fspindle/poub/coin-cmake/coin/include -I/Users/fspindle/poub/coin-cmake/coin/include/Inventor/annex -I/Users/fspindle/poub/coin-cmake/coin/build-cmake -o CMakeFiles/Coin.dir/actions/SoAction.cpp.o -c /Users/fspindle/poub/coin-cmake/coin/src/actions/SoAction.cpp In file included from /Users/fspindle/poub/coin-cmake/coin/include/Inventor/system/gl.h:47, from /Users/fspindle/poub/coin-cmake/coin/src/actions/SoAction.cpp:222: /Users/fspindle/poub/coin-cmake/coin/build-cmake/Inventor/system/gl-headers.h:47:19: error: GL/gl.h: No such file or directory /Users/fspindle/poub/coin-cmake/coin/build-cmake/Inventor/system/gl-headers.h:48:20: error: GL/glu.h: No such file or directory /Users/fspindle/poub/coin-cmake/coin/build-cmake/Inventor/system/gl-headers.h:49:22: error: GL/glext.h: No such file or directory In file included from /Users/fspindle/poub/coin-cmake/coin/src/actions/SoAction.cpp:233: /Users/fspindle/poub/coin-cmake/coin/src/profiler/SoNodeProfiling.h: In member function ‘void SoNodeProfiling::postTraversal(SoAction*)’: /Users/fspindle/poub/coin-cmake/coin/src/profiler/SoNodeProfiling.h:95: error: ‘glFinish’ was not declared in this scope

    Here the next error related to glx that doesn't exist in the OpenGL framework

    [ 46%] Building CXX object src/CMakeFiles/Coin.dir/glue/gl.cpp.o cd /Users/fspindle/soft/third-party/Coin3D/coin-build/src && /usr/bin/c++ -DCoin_EXPORTS -DYY_NO_UNISTD_H -DCOIN_DEBUG=0 -DCOIN_INTERNAL -DGLX_GLXEXT_PROTOTYPES -DHAVE_CONFIG_H -fPIC -I/Users/fspindle/soft/third-party/Coin3D/coin/src -I/Users/fspindle/soft/third-party/Coin3D/coin/include -I/Users/fspindle/soft/third-party/Coin3D/coin/include/Inventor/annex -I/Users/fspindle/soft/third-party/Coin3D/coin-build -o CMakeFiles/Coin.dir/glue/gl.cpp.o -c /Users/fspindle/soft/third-party/Coin3D/coin/src/glue/gl.cpp /Users/fspindle/soft/third-party/Coin3D/coin/src/glue/gl.cpp:232:20: error: GL/glx.h: No such file or directory /Users/fspindle/soft/third-party/Coin3D/coin/src/glue/gl.cpp: In function ‘void* coin_gl_current_context()’: /Users/fspindle/soft/third-party/Coin3D/coin/src/glue/gl.cpp:5188: error: ‘glXGetCurrentContext’ was not declared in this scope