HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Portable CMake script

Submitted by: @import:stackexchange-codereview··
0
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

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