CMake version mismatch

[golpa]

The CMakeLists.txt files specifies that it requires cmake version 2.8.0. However, the cmake command add_compile_options() on lines 22-24 and 27 was added to CMake version 2.8.12 and later.

CentOS 7, for example, uses cmake version 2.8.11 and fails with:

-- The CXX compiler identification is GNU 4.8.3
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
CMake Error at CMakeLists.txt:22 (add_compile_options):
  Unknown CMake command "add_compile_options".

One option to support cmake version 2.8.0 and up is to replace line 22 - 24 with set(CMAKE_CXX_FLAGS "-pedantic -Wall -Wextra") (untested). Similar solution should work for line 27.

[guruofquality]

Might also add the following:

########################################################################
# Provide add_compile_options() when not available
########################################################################
if(CMAKE_VERSION VERSION_LESS "2.8.12")
    function(add_compile_options)
        add_definitions(${ARGN})
    endfunction(add_compile_options)
endif()

[golpa]

You're right that's a better way even though the CMake docs say you should only use add_definitions() for prepocessor definitions.

[martinrotter]

Problem is that one should almost always use "add_definitions" function.

https://bitbucket.org/skunkos/rssguard/src/88bbe648a0de89a3db669f66eb536f6b4012bb87/CMakeLists.txt?at=master&fileviewer=file-view-default#CMakeLists.txt-215

[aaronpburke]

FYI, if you opt for the "set(CMAKE_CXX_FLAGS ...)" option, please be aware that setting it in this manner will override any settings coming from parent CMakeLists.txt files (e.g., if the project is included as part of a larger project - a very likely scenario due to the library nature of this project). In this scenario, use the following syntax: set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -myarg)

add_definitions(...) is for preprocessor definitions, whereas add_compile_options is intended for just about everything else (CMake version requirements notwithstanding).

On Thu, Oct 22, 2015 at 9:06 PM, martinrotter notifications@github.com wrote:

Problem is that one should almost always use "add_definitions" function.

https://bitbucket.org/skunkos/rssguard/src/88bbe648a0de89a3db669f66eb536f6b4012bb87/CMakeLists.txt?at=master&fileviewer=file-view-default#CMakeLists.txt-215

— Reply to this email directly or view it on GitHub https://github.com/beltoforion/muparserx/issues/64#issuecomment-150461051 .

[guruofquality]

set(CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS} -myarg)

Agreed, that would be an acceptable replacement. Although, I have always put the quotes in when doing it this way (to keep it a string). Otherwise CMake thinks its a list and adds semicolons:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -myarg")

add_definitions(...) is for preprocessor definitions, whereas add_compile_options is intended for just about everything else (CMake version requirements notwithstanding).

For better or for worse, I have used add_definitions() for years for flags before add_compile_options() came out. I never really liked using flags for something called definitions. But it always worked as intended. ;-)

[beltoforion]

I can't addredd this issue right now help would be appreciated

[guruofquality]

The issue is that older cmake's don't have add_compile_options(). Here is my patch, it uses add_definitions() in place of the missing add_compile_options().

Here's the docs if anyone is concerned, add_definitions() totally supports compiler flags, and it always has: https://cmake.org/cmake/help/v3.0/command/add_definitions.html

There's 100 ways to handle this. I think this keeps things simple and clean. And it will resolve @golpa's issue, and things will continue to build just fine.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1e44fb5..02c2f8e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,13 +19,13 @@ set(MUPARSERX_VERSION ${CMAKE_MATCH_1})
 # Compiler specific flags
 ########################################################################
 if(CMAKE_COMPILER_IS_GNUCXX)
-    add_compile_options(-pedantic)
-    add_compile_options(-Wall)
-    add_compile_options(-Wextra)
+    add_definitions(-pedantic)
+    add_definitions(-Wall)
+    add_definitions(-Wextra)
 endif(CMAKE_COMPILER_IS_GNUCXX)

 if(MSVC)                                                                                                                                                                                            
-    add_compile_options(/MP) #multi-core build                                                                                                                                                      
+    add_definitions(/MP) #multi-core build                                                                                                                                                          
 endif(MSVC)                                                                                                                                                                                         

 ######################################################################## 

[guruofquality]

Actually, while we are at it, it needs c++11 flag now (just tested):

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1e44fb5..ee86cf2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -19,13 +19,15 @@ set(MUPARSERX_VERSION ${CMAKE_MATCH_1})
 # Compiler specific flags
 ########################################################################
 if(CMAKE_COMPILER_IS_GNUCXX)
-    add_compile_options(-pedantic)
-    add_compile_options(-Wall)
-    add_compile_options(-Wextra)
+    add_definitions(-pedantic)
+    add_definitions(-Wall)
+    add_definitions(-Wextra)
+    add_definitions(-std=c++11)
+
 endif(CMAKE_COMPILER_IS_GNUCXX)

 if(MSVC)
-    add_compile_options(/MP) #multi-core build
+    add_definitions(/MP) #multi-core build
 endif(MSVC)

 ########################################################################

Let me know if you prefer a pull request.

[beltoforion]

Thanks for the response., I'd appreciate a pull request. Currently i don't even have cmake installed

[guruofquality]

I took care of the build issue in this pull request: https://github.com/beltoforion/muparserx/pull/69 This pull also adds cmake project config files to use muparserx in a client cmake project.

[beltoforion]

Thank you very much for helping me out her, i pulled it.