2
\$\begingroup\$

I am new to CMake and got currently a setup of my Visual Studio solution working via CMake. My goal now is to have a professional CMake setup.

Here is my folder structure:

root_project
  |─── CMakeLists.txt
  |─── Project 1
  |      |─── build
  |      |      |─── Debug
  |      |      └─── Release
  |      |─── source
  |      |      |─── CMakeLists.txt
  |      |      └─── include
  |      |─── resource
  |      └─── header
  └─── Project 2
         |─── build
         |      |─── Debug
         |      └─── Release
         |─── source
         |      |─── CMakeLists.txt
         |      └─── include
         |─── resource
         └─── header
  • Project 2 depends on Project 1
  • Project 1 depends on external dependencies
  • "headers" is the folder for external header files

My CMakeLists.txt for the root project:

    # Specify the minimum version for CMake
cmake_minimum_required(VERSION 3.8.2)

set(CMAKE_DISABLE_IN_SOURCE_BUILD ON)
set(CMAKE_DISABLE_SOURCE_CHANGES ON)

# Project's name
project("root_project")

set_property(GLOBAL PROPERTY USE_FOLDERS ON)

IF(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR})
    message(FATAL_ERROR "In-source builds not allowed. Please make a new directory (called a build directory) and run CMake from there. You may need to remove CMakeCache.txt.")
ENDIF()

OPTION(BUILD_TESTS "Decides whether the unit tests will be built." ON)

# C/C++ languages required.
enable_language(C)
enable_language(CXX)

# Set the C++ Version
message("!REQUIRED! -- Supported features = ${cxx_std_14}")
message("Supported features = ${cxx_std_17}")

set(CMAKE_C_STANDARD 11)
set(CMAKE_C_STANDARD_REQUIRED ON)
set(CMAKE_C_EXTENSIONS OFF)

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

# Only allow 64bit architecture
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
    # 64bit
    message(STATUS "Running on x86-64 platform. Proceeding...")
ELSEIF(CMAKE_SIZEOF_VOID_P EQUAL 4)
    # 32bit
    message(FATAL_ERROR "Running on x86 platform. This is not supported. Aborting...")
ELSE()
    # unidentified architecture
    message(FATAL_ERROR "Running on unidentified architecture. This is not supported. Aborting...")
ENDIF()

# Abort when OpenGL is not found
IF(NOT OPENGL_FOUND)
    message(WARNING "Could not find OpenGL library.")
ENDIF()

IF(NOT VULKAN_FOUND)
    message(WARNING "Could not find Vulkan library.")
ENDIF()

# Set the output folder where the program will be created
set(EXECUTABLE_OUTPUT_PATH  ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR})
set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR})

message(STATUS "----------------------------------------")
message(STATUS "CMake Binary Dir:" ${CMAKE_BINARY_DIR})
message(STATUS "CMake Source Dir:" ${CMAKE_SOURCE_DIR})
message(STATUS "CMake CFG Dir:" ${CMAKE_CFG_INTDIR})
message(STATUS "CMake exe Dir:" ${EXECUTABLE_OUTPUT_PATH})
message(STATUS "CMake lib Dir:" ${LIBRARY_OUTPUT_PATH})

# Add the modules
add_subdirectory("Project 1")
add_subdirectory("Project 2")

The CMakeLists.txt for Project 1:

    # Specify the minimum version for CMake
cmake_minimum_required(VERSION 3.8.2)

project(Engine)

# Set the version number of the project here
set(VERSION_MAJOR "0")
set(VERSION_MINOR "1")
set(VERSION_PATCH "0")
set(VERSION ${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_PATCH})

find_package(OpenGL REQUIRED)

if(NOT OPENGL_FOUND)
    message(ERROR "OpenGL not found!")
endif()

include_directories(${GL_INCLUDE_DIRS})
link_directories(${GL_LIBRARY_DIRS})
add_definitions(${GL_DEFINITIONS})

set(HEADERS
    ${CMAKE_CURRENT_SOURCE_DIR}/include/Engine.h
)

set(SOURCES
    ${CMAKE_CURRENT_SOURCE_DIR}/Engine.cpp
)

set(HEADERS_WIN32

)

set(SOURCES_WIN32

)

set(HEADERS_LINUX

)

set(SOURCES_LINUX

)

source_group(headers FILES ${HEADERS} ${HEADERS_WIN32} ${HEADERS_LINUX})
source_group(sources FILES ${SOURCES} ${SOURCES_WIN32} ${HEADERS_LINUX})

if(WIN32)
add_executable(DarkEngine WIN32
    ${HEADERS}
    ${SOURCES}
    ${HEADERS_WIN32}
    ${SOURCES_WIN32}
)
ELSEIF(UNIX AND NOT APPLE)
add_library(DarkEngine
    ${HEADERS}
    ${SOURCES}
    ${HEADERS_LINUX}
    ${HEADERS_LINUX}
)
ELSE()
# The system is not supported
message(FATAL_ERROR "System not supported.")
ENDIF()

target_link_libraries(DarkEngine ${GL_LIBRARY})

The CMakeLists.txt for Project 2:

    # Specify the minimum version for CMake
cmake_minimum_required(VERSION 3.8.2)

project(ProjectX)

# Set the version number of the project here
set(VERSION_MAJOR "0")
set(VERSION_MINOR "1")
set(VERSION_PATCH "0")
set(VERSION ${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_PATCH})

set(HEADERS

)

set(SOURCES

)

set(HEADERS_WIN32

)

set(SOURCES_WIN32

)

set(HEADERS_LINUX

)

set(SOURCES_LINUX

)

source_group(headers FILES ${HEADERS} ${HEADERS_WIN32} ${HEADERS_LINUX})
source_group(sources FILES ${SOURCES} ${SOURCES_WIN32} ${HEADERS_LINUX})

if(WIN32)
add_executable(Game
    ${HEADERS}
    ${SOURCES}
    ${HEADERS_WIN32}
    ${SOURCES_WIN32}
)
ELSEIF(UNIX AND NOT APPLE)
add_executable(Game
    ${HEADERS}
    ${SOURCES}
    ${HEADERS_LINUX}
    ${HEADERS_LINUX}
)
ELSE()
# The system is not supported
message(FATAL_ERROR "System not supported.")
ENDIF()

target_link_libraries(DarkEngine Game)
\$\endgroup\$
  • 2
    \$\begingroup\$ CMake still makes me chuckle. The original developers looked at make and said this is too hard we can make it better. Now its more complicated and still has a tenth of the functionality. :-) Great for small projects and getting things off the ground. Not much use professionally. \$\endgroup\$ – Martin York Jul 28 '17 at 6:42
  • \$\begingroup\$ PS: There is no language C/C++ these are distinct languages pick one don't mix them together with enable_language(C) enable_language(CXX) \$\endgroup\$ – Martin York Jul 28 '17 at 6:45
  • 2
    \$\begingroup\$ @LokiAstari: You're quite mistaken about CMake. The purpose is to enable simplified cross-platform builds using native tools, not to create a new make. Some systems supported by CMake don't even use make. As for "not much use professionally", you should look at the list of projects using CMake. \$\endgroup\$ – Edward Jul 28 '17 at 12:03
  • \$\begingroup\$ @Edward: It was a little tounge in cheek (yes it is used professionally). But saying it was not trying to create a new make when the purpose of make is simplified cross-platform builds. I would be interested to know of systems that support CMake where make is not supported. \$\endgroup\$ – Martin York Jul 28 '17 at 16:56
  • 1
    \$\begingroup\$ See the list of generators for CMake. It generates not only files for make but also specific make variants such as Watcom WMake, non-make systems such as Ninja and IDEs that don't use make, such as Visual Studio and Xcode. Is make available for such systems? Sure. But CMake allows developers to use their own accustomed tools which may or may not be make-based. \$\endgroup\$ – Edward Jul 28 '17 at 17:09
1
\$\begingroup\$

I see some things that may help you improve your code.

Understand the risk of undocumented directives

Although they have been in the code for years, both of these directives are undocumented:

set(CMAKE_DISABLE_IN_SOURCE_BUILD ON)
set(CMAKE_DISABLE_SOURCE_CHANGES ON)

That means that they could disappear without warning in future versions of CMake. Their use also means that instead of your nice message telling users not to do an in-source build, they see this:

CMake Error at /usr/share/cmake/Modules/CMakeDetermineSystem.cmake:174 (file):
  file attempted to write a file:
  /home/edward/foo/CMakeFiles/CMakeOutput.log into a source
  directory.
Call Stack (most recent call first):
  CMakeLists.txt:8 (project)


CMake Error: CMAKE_C_COMPILER not set, after EnableLanguage
CMake Error: CMAKE_CXX_COMPILER not set, after EnableLanguage
-- Configuring incomplete, errors occurred!

Be cautious when using global properties

The top level file sets USE_FOLDERS ON. That's only effective when used in conjunction with the FOLDER property of individual targets, but no targets in this project have that property set. I'd omit it from the top level for that reason.

Make sure your comments don't mislead

Immediately after this comment:

# Set the C++ Version

Is this code:

message("!REQUIRED! -- Supported features = ${cxx_std_14}")
message("Supported features = ${cxx_std_17}")

set(CMAKE_C_STANDARD 11)
set(CMAKE_C_STANDARD_REQUIRED ON)
set(CMAKE_C_EXTENSIONS OFF)

None of that actually sets the C++ version so I'd either move the comment to where it doesn't lie or better, simply delete the comment.

Don't rely on nonstandard environment variables

These two lines rely on nonstandard environment variables:

message("!REQUIRED! -- Supported features = ${cxx_std_14}")
message("Supported features = ${cxx_std_17}")

I would advise either omitting those lines (because they don't print anything useful on machines without those variables set) or creating a warning if the variables don't exist. My vote would be to delete.

Understand the limitations of some target compilers

The top level CMake file includes this line:

set(CMAKE_CXX_STANDARD_REQUIRED ON)

According to the CMake documentation, "For compilers that have no notion of a standard level, such as MSVC, this has no effect." That's a limitation of MSVC and not of CMake, but if, as it appears, MSVC is one of the target environments, you should be aware of this limitation. I'd still leave this line in place, however, because it serves as useful documentation even if it has no effect for some compilers.

Don't rule out cross-compiling

The code includes these lines:

# Only allow 64bit architecture
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
        # 64bit
        message(STATUS "Running on x86-64 platform. Proceeding...")

It's OK to make the check, but your message is not really technically accurate. In fact, it's possible to do a build for a 64-bit target on a 32-bit machine set up for cross compiling, so rather than "Running on" the message would be more technically accurate if it said "Compiling for".

Don't test variables that haven't been set

The top level CMake file has this:

# Abort when OpenGL is not found
IF(NOT OPENGL_FOUND)
        message(WARNING "Could not find OpenGL library.")
ENDIF()

However, because the FindOpenGL module has not yet been invoked, that variable will be unset even on machines which have it installed. Instead, the way to do this is like this:

FIND_PACKAGE(OpenGL REQUIRED)

Requiring a specific version (or "version x or newer") is also supported by FIND_PACKAGE, so you should consider that if, for example, the code uses features which don't exist in old versions.

Don't clutter the output

The top level CMake file includes this:

# Set the output folder where the program will be created
set(EXECUTABLE_OUTPUT_PATH  ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR})
set(LIBRARY_OUTPUT_PATH ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR})

message(STATUS "----------------------------------------")
message(STATUS "CMake Binary Dir:" ${CMAKE_BINARY_DIR})
message(STATUS "CMake Source Dir:" ${CMAKE_SOURCE_DIR})
message(STATUS "CMake CFG Dir:" ${CMAKE_CFG_INTDIR})
message(STATUS "CMake exe Dir:" ${EXECUTABLE_OUTPUT_PATH})
message(STATUS "CMake lib Dir:" ${LIBRARY_OUTPUT_PATH})

There are a few problems with this in my view. First, unless I'm debugging the CMake build system itself, I probably don't care about these variables and printing them only clutters the output. Second, the two variable that are set are deeply suspicious to me. They aren't referenced anywhere else but they appear to be an attempt to put generated files in a specific place. I'd recommend against doing this generically (not least because your usage is not supported by makefile generators) and choosing the destination per target only if absolutely necessary. Generally, users will be less confused if you let CMake determine where to put the binaries. Since you're already requiring version 3.8.2 or better, CMake will already do what you want without special variable tricks.

Use the correct variable names

In the Project 1 CMake file, we have these lines:

find_package(OpenGL REQUIRED)
if(NOT OPENGL_FOUND)
        message(ERROR "OpenGL not found!")
endif()
include_directories(${GL_INCLUDE_DIRS})
link_directories(${GL_LIBRARY_DIRS})
add_definitions(${GL_DEFINITIONS})

First, the if clause is redundant because the find_package will already have exited with an appropriate error message if OpenGL was not found because you've marked it as REQUIRED. Second, the FindOpenGL module sets variables that start with OPENGL_ and not GL_. I have no idea what GL_DEFINITIONS might be. If you want to link with only GL (and not GLU) just use OPENGL_gl_LIBRARY as mentioned in the documentation. Also instead of link_directories, you should instead use target_link_libraries.

Rethink your code structure

Rather than having the build system keep track of which headers are for Windows and which are for Linux, I'd recommend instead that you use defined macros within the source code for this purpose. That way, it will be easier to keep common code common and help remind developers that if they add a feature or fix for one platform, that it probably ought to also be applied to the other platform(s).

Also, instead of writing this:

add_library(DarkEngine
        ${HEADERS}
        ${SOURCES}
        ${HEADERS_LINUX}
        ${HEADERS_LINUX}
    )

I'd prefer to see the files listed. Forcing the developer to look up another set of variables is annoying. Just list them here.

Never put paths in header definitions

This is absolutely a no-no on pretty much any system:

set(HEADERS
        ${CMAKE_CURRENT_SOURCE_DIR}/include/Engine.h
)

Instead, you should tell the system where to look for headers and then simply list them by file name without the path. Use target_include_directories to tell CMake where to look for include files.

\$\endgroup\$
  • \$\begingroup\$ 1. For the header definitions, should I just tell cmake to look-up into a new subdictionary with a new CMakeLists file or how should I achieve this? 2. I used the GL_DEFINITIONS etc. because I don't want to have anything else as the bare OpenGL, so now GLU etc. And the OPENGL_LIBRARIES definition would for example link against OpenGL and GLU. 3. How should I use the FOLDER property in combination with the USE_FOLDERS ON property? 4. I am also wondering which is the best place and command to start the top-level CMake to setup the whole solution. \$\endgroup\$ – ShadowDragon Jul 28 '17 at 17:12
  • \$\begingroup\$ I've updated my answer to try to address 1 and 2. For 3, I don't personally use the FOLDER property, but you can set it like this: set_target_properties (DarkEngine PROPERTIES FOLDER Engine-projects) or wherever you were intending to have it show up. For 4, where it's located now is fine. I usually put a little README file at the top that instructs users to do mkdir build && cd build && cmake .. && make or whatever the equivalent instructions are on your platform. \$\endgroup\$ – Edward Jul 28 '17 at 18:02
  • \$\begingroup\$ Wouldn't your way to solve number 4 result into it that CMake puts everything in the build folder of the root project instead of the build folder for each specific project? \$\endgroup\$ – ShadowDragon Jul 28 '17 at 22:09
  • \$\begingroup\$ Also using target_link_libraries results into this error: CMake Error at Engine/CMakeLists.txt:18 (target_link_libraries): Cannot specify link libraries for target "opengl32" which is not built by this project. \$\endgroup\$ – ShadowDragon Jul 28 '17 at 22:23
  • \$\begingroup\$ Read the docs for target_link_libraries and you'll see that the first argument is the target that your project is building. The error code you show indicates that you're specifying opengl32 as the target which is not correct for the reason shown in that message. \$\endgroup\$ – Edward Jul 28 '17 at 22:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.