Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMakeLists.txt tries to override globally visible CMAKE_BUILD_TYPE through the cache #1081

Closed
andry81 opened this issue Mar 17, 2019 · 1 comment

Comments

@andry81
Copy link

andry81 commented Mar 17, 2019

This is incorrect behaviour because the Fmt CMakeList.txt could be called as a part of another CMakeLists.txt from parent directory and CMAKE_BUILD_TYPE is shared resource in that case.

You should not change globally visible variables from project potentially called from cmake add_subdirectory function because it is the task only for the most top level project (the root), but not a child project.

I suggest instead of this:

set(CMAKE_BUILD_TYPE Release CACHE STRING ${doc})

# Set the default CMAKE_BUILD_TYPE to Release.
# This should be done before the project command since the latter can set
# CMAKE_BUILD_TYPE itself (it does so for nmake).
if (NOT CMAKE_BUILD_TYPE)
  join(doc "Choose the type of build, options are: None(CMAKE_CXX_FLAGS or "
           "CMAKE_C_FLAGS used) Debug Release RelWithDebInfo MinSizeRel.")
  set(CMAKE_BUILD_TYPE Release CACHE STRING ${doc})
endif ()

Use this:

function(check_CMAKE_BUILD_TYPE_vs_multiconfig)
  get_property(GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
  if(NOT DEFINED GENERATOR_IS_MULTI_CONFIG)
    message(FATAL_ERROR "GENERATOR_IS_MULTI_CONFIG must be defined")
  endif()
  if((NOT GENERATOR_IS_MULTI_CONFIG AND NOT CMAKE_BUILD_TYPE) OR
     (GENERATOR_IS_MULTI_CONFIG AND CMAKE_BUILD_TYPE))
      message(FATAL_ERROR "CMAKE_BUILD_TYPE variable must not be set in case of a multiconfig generator presence and must be set if not: GENERATOR_IS_MULTI_CONFIG=`${GENERATOR_IS_MULTI_CONFIG}` CMAKE_BUILD_TYPE=`${CMAKE_BUILD_TYPE}`")
  endif()
endfunction()

As long as it calls only from the configure step, then it is enough.

The GENERATOR_IS_MULTI_CONFIG property is available from cmake 3.9+:
https://cmake.org/cmake/help/v3.9/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html

@vitaut
Copy link
Contributor

vitaut commented Mar 17, 2019

Good catch, thanks! Fixed in 53379df.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants