Refactoring of CPM policies causes a regression in policy propagation

[robertmaynard]

After deploying CPM 0.35.4 I found a regression in policy propagation.

Here is a small reproducer of the issue:

Root CMakeLists.txt

cmake_minimum_required(VERSION 3.10.0 FATAL_ERROR)
project(cpm_reproducer)

function(cpm_init)
  include(${CMAKE_CURRENT_SOURCE_DIR}/cpm.cmake)
endfunction()
cpm_init()

add_cpm_subdir(src)

cpm.cmake:

macro(set_policies )
  # the policy allows us to change options without caching
  cmake_policy(SET CMP0077 NEW)
  set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)

  # the policy allows us to change set(CACHE) without caching
  if(POLICY CMP0118)
    cmake_policy(SET CMP0118 NEW)
    set(CMAKE_POLICY_DEFAULT_CMP0118 NEW)
  endif()
endmacro()
set_policies()

function(add_cpm_subdir dir)
  # set_policies()
  add_subdirectory(${dir})
endfunction()

and the src/CMakeLists.txt

message(STATUS "before src cmake_minimum_required")
cmake_policy(GET CMP0077 77_state)
cmake_policy(GET CMP0118 126_state)
message(STATUS "CMP0077 ${77_state}")
message(STATUS "CMP0118 ${126_state}")

cmake_minimum_required(VERSION 3.1.0 FATAL_ERROR)

message(STATUS "after src cmake_minimum_required")
cmake_policy(GET CMP0077 77_state)
cmake_policy(GET CMP0118 126_state)
message(STATUS "CMP0077 ${77_state}")
message(STATUS "CMP0118 ${126_state}")

The behavior we see is that src/CMakeLists.txt will reset to 3.1.0 policy settings since the variables CMAKE_POLICY_DEFAULT_CMP0077 and CMAKE_POLICY_DEFAULT_CMP0126 are not set in the scope the are called from.

So while CPM.cmake behaves as expected the consuming project doesn't which means that options and set(CACHE) variables will ignore the provided values we propagate.

Proposed Solution:

We refactor the CPM policies to set in a macro, which we call when CPM.cmake is included and also when CPMAddPackage is called.

[TheLartians]

Would it work as expected if we set the default policies with the PARENT_SCOPE flag?

[iboB]

@TheLartians I reproduced the issue with @robertmaynard's code and it does get resolved when the policies are set with PARENT_SCOPE.

I think this is the better solution

[robertmaynard]

I don't like the PARENT_SCOPE solution for a couple of reasons:

• We will have to determine if CPM is already in the root project scope. If so we can't use PARENT_SCOPE.
• It only resolves the issue when the inclusion of CPM occurs at most 1 scope below the usage of CPMAddPackage. If we have two nested sibling directories, and one uses a function to bring in CPM we now have 2 levels of scope we need to punch through.

Due to the fact that we could have abitrary levels of scope between the inclusion of CPM and the usage of the functions the safest approach is setting the policies at each invocation of CPMAddPackage.

[TheLartians]

The arbitrary inclusion level is actually good point, especially as I see some projects import the CPM.cmake script from other CMake subdirectories.

At this point I'd be happy to go with @robertmaynard's suggestion as it seems to be the most consistent solution in CMake where functions are globally available but variables exist only in the local scope.

A disadvantage would be that its impossible to change policies back to old without modifying the dependency's CMakeLists, but as discussed before it hopefully shouldn't cause much trouble.

[iboB]

It hadn't occurred to me that including CPM.cmake from a subdirectory is an actual use case. But in this case, does it make sense to call set_policies upon inclusion? It seems to me that it doesn't. If we go ahead and call set_policies in CPMAddPackage, I think we can safely go back to @robertmaynard original idea and only call it there. Thus the policies will be set in packages and not in the main script, but that will also be true if CPM.cmake is included in a subdir

[robertmaynard]

I will draft up a PR to fix this, but I think I will keep the policies being setup for CPM.cmake itself as well. That will ensure that CPM options and cache variables behave as people expect for the latest version of CMake ( local variables with same name aren't removed when a cache variable is constructed ).

[TheLartians]

Thanks!