patterncppMinor
Portable CMake script
Viewed 0 times
cmakescriptportable
Problem
I'm trying to write a portable CMake script for a simple Qt application. The target platforms are Win and Mac OSX. But as you can see, it's quite a monster already.
Are there any CMake profs? Can you see any essentially wrong approaches used in this script? I bet there are a few.
```
CMAKE_MINIMUM_REQUIRED( VERSION 2.8.6 )
##################################################################################################################################
MACRO( CHOOSE_QT path )
FILE( GLOB QTROOTS "${path}/bin" )
FIND_PROGRAM( QT_QMAKE_EXECUTABLE NAMES qmake qmake4 qmake-qt4 qmake-mac PATHS ${QTROOTS} )
ENDMACRO( CHOOSE_QT path )
MACRO( ADD_FILES_TO_FILTER rootFilterName rootFilterPath files )
FOREACH( curFile ${files} )
FILE( RELATIVE_PATH curFilter "${CMAKE_CURRENT_SOURCE_DIR}/${rootFilterPath}" "${CMAKE_CURRENT_SOURCE_DIR}/${curFile}" )
FILE( RELATIVE_PATH test "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${curFile}" )
GET_FILENAME_COMPONENT( curFilter ${curFilter} PATH )
SET( curFilter "${rootFilterName}/${curFilter}" )
STRING( REPLACE "/" "\\\\" curFilter ${curFilter} )
SOURCE_GROUP( ${curFilter} FILES ${curFile} )
ENDFOREACH( curFile )
ENDMACRO( ADD_FILES_TO_FILTER rootFilterName rootFilterPath files )
MACRO( TO_RELATIVE_PATHS filePaths )
SET( resPaths "" )
FOREACH( curPath ${${filePaths}} )
FILE( RELATIVE_PATH relPath ${CMAKE_CURRENT_SOURCE_DIR} ${curPath} )
SET( resPaths ${resPaths} ${relPath} )
ENDFOREACH( curPath )
SET( ${filePaths} ${resPaths} )
ENDMACRO( TO_RELATIVE_PATHS filePaths )
MACRO( COPY_TO_BUNDLE resourcePath bundlePath )
LIST( APPEND BUNDLE_COPY_RESOURCES ${resourcePath} )
SET_SOURCE_FILES_PROPERTIES( ${resourcePath} PROPERTIES MACOSX_PACKAGE_LOCATION ${bundlePath} )
ENDMACRO( COPY_TO_BUNDLE )
MACRO( ADD_FRAMEWORK fwname fwpath appname )
TARGET_LINK_LIBRARIES( ${appname} ${fwpath}/${fwname} )
MESSAGE( STATUS "Framew
Are there any CMake profs? Can you see any essentially wrong approaches used in this script? I bet there are a few.
```
CMAKE_MINIMUM_REQUIRED( VERSION 2.8.6 )
##################################################################################################################################
MACRO( CHOOSE_QT path )
FILE( GLOB QTROOTS "${path}/bin" )
FIND_PROGRAM( QT_QMAKE_EXECUTABLE NAMES qmake qmake4 qmake-qt4 qmake-mac PATHS ${QTROOTS} )
ENDMACRO( CHOOSE_QT path )
MACRO( ADD_FILES_TO_FILTER rootFilterName rootFilterPath files )
FOREACH( curFile ${files} )
FILE( RELATIVE_PATH curFilter "${CMAKE_CURRENT_SOURCE_DIR}/${rootFilterPath}" "${CMAKE_CURRENT_SOURCE_DIR}/${curFile}" )
FILE( RELATIVE_PATH test "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_SOURCE_DIR}/${curFile}" )
GET_FILENAME_COMPONENT( curFilter ${curFilter} PATH )
SET( curFilter "${rootFilterName}/${curFilter}" )
STRING( REPLACE "/" "\\\\" curFilter ${curFilter} )
SOURCE_GROUP( ${curFilter} FILES ${curFile} )
ENDFOREACH( curFile )
ENDMACRO( ADD_FILES_TO_FILTER rootFilterName rootFilterPath files )
MACRO( TO_RELATIVE_PATHS filePaths )
SET( resPaths "" )
FOREACH( curPath ${${filePaths}} )
FILE( RELATIVE_PATH relPath ${CMAKE_CURRENT_SOURCE_DIR} ${curPath} )
SET( resPaths ${resPaths} ${relPath} )
ENDFOREACH( curPath )
SET( ${filePaths} ${resPaths} )
ENDMACRO( TO_RELATIVE_PATHS filePaths )
MACRO( COPY_TO_BUNDLE resourcePath bundlePath )
LIST( APPEND BUNDLE_COPY_RESOURCES ${resourcePath} )
SET_SOURCE_FILES_PROPERTIES( ${resourcePath} PROPERTIES MACOSX_PACKAGE_LOCATION ${bundlePath} )
ENDMACRO( COPY_TO_BUNDLE )
MACRO( ADD_FRAMEWORK fwname fwpath appname )
TARGET_LINK_LIBRARIES( ${appname} ${fwpath}/${fwname} )
MESSAGE( STATUS "Framew
Solution
My CMake-fu is a bit rusty - probably as old as this question -, but I still see some things that could be trivially improved:
-
CMake doesn't require
-
The remark above also applies for
-
Your capitalization is not consistent. You sometimes use
-
There are too many occurrences of
-
You should remove commented out code. If you need an old version of your code, you should simply rely on revision control software. If you have comments like "comment out to do something", it generally means that the "something" in question should be a command line option.
-
CMake doesn't require
else() and endif() to contain the original if() expression anymore. It seems that this restriction disappeared a long time ago, so you can simplify your code by leaving empty else() and endif() expressions. Your indentation already does a great job to document where an if() is ending.-
The remark above also applies for
endmacro() and endforeach and more generally any endxxx() instruction. Note that it works even with your version: you left the endmacro() empty for ADD_FRAMEWORK.-
Your capitalization is not consistent. You sometimes use
FOREACH while you sometimes use foreach. I know that CMake is not really case-sensitive, but being consistent doesn't hurt and helps case-sensitive search in code editors.-
There are too many occurrences of
"${CMAKE_CURRENT_SOURCE_DIR}/cxx/thirdparty. You should define a ${THIRD_PARTY_DIR} variable or something to having repeating the full path over and over. It will avoid many problems if the path changes. It seems that there isn't a portable way to make it work for both WIN32 and APPLE, but doing it for WIN32 would already help.-
You should remove commented out code. If you need an old version of your code, you should simply rely on revision control software. If you have comments like "comment out to do something", it generally means that the "something" in question should be a command line option.
Context
StackExchange Code Review Q#18706, answer score: 9
Revisions (0)
No revisions yet.