Skip to content

Commit

Permalink
Fix CMake policy warnings/changes introduced in 3.0.0 and higher
Browse files Browse the repository at this point in the history
  • Loading branch information
@dakcarto
dakcarto committed Dec 23, 2014
1 parent 247c3dd commit a4aaff5
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CMakeLists.txt
View file
Expand Up @@ -3,6 +3,9 @@ SET(CPACK_PACKAGE_VERSION_MINOR "7")
SET(CPACK_PACKAGE_VERSION_PATCH "0")
SET(COMPLETE_VERSION ${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH})
SET(RELEASE_NAME "Master")
IF (POLICY CMP0048) # in CMake 3.0.0+
CMAKE_POLICY (SET CMP0048 OLD) # keep PROJECT() from clearing VERSION variables
ENDIF (POLICY CMP0048)
SET(PROJECT_VERSION ${COMPLETE_VERSION})
PROJECT(qgis${PROJECT_VERSION})
IF (APPLE)
Expand Down Expand Up @@ -459,6 +462,9 @@ IF (WIN32)
ELSE (WIN32)

IF (APPLE)
IF (POLICY CMP0042) # in CMake 3.0.0+
SET (CMAKE_MACOSX_RPATH OFF) # otherwise ON by default
ENDIF (POLICY CMP0042)
# for Mac OS X, everything is put inside an application bundle
# save the root install prefix for the app later
SET (QGIS_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX})
Expand Down
4 changes: 3 additions & 1 deletion cmake/FindGDAL.cmake
View file
Expand Up @@ -13,7 +13,9 @@
#
# GDAL_INCLUDE_DIR = where to find headers


IF (POLICY CMP0053) # in CMake 3.1.0+
CMAKE_POLICY (SET CMP0053 OLD) # keep old-style @VAR@ expansion
ENDIF (POLICY CMP0053)
INCLUDE (@CMAKE_SOURCE_DIR@/cmake/MacPlistMacros.cmake)

IF(WIN32)
Expand Down
3 changes: 3 additions & 0 deletions cmake/FindGEOS.cmake
View file
Expand Up @@ -13,6 +13,9 @@
# GEOS_LIBRARY
#

IF (POLICY CMP0053) # in CMake 3.1.0+
CMAKE_POLICY (SET CMP0053 OLD) # keep old-style @VAR@ expansion
ENDIF (POLICY CMP0053)
INCLUDE (@CMAKE_SOURCE_DIR@/cmake/MacPlistMacros.cmake)

IF(WIN32)
Expand Down
2 changes: 1 addition & 1 deletion i18n/CMakeLists.txt
View file
Expand Up @@ -32,7 +32,7 @@ ADD_CUSTOM_TARGET (translations ALL
DEPENDS ${QM_FILES})

# first compile sources, then compile translations
ADD_DEPENDENCIES (translations qgis)
ADD_DEPENDENCIES (translations ${QGIS_APP_NAME})

INSTALL (FILES ${QM_FILES}
DESTINATION ${QGIS_DATA_DIR}/i18n)
10 changes: 10 additions & 0 deletions python/CMakeLists.txt
View file
Expand Up @@ -42,6 +42,12 @@ ADD_CUSTOM_TARGET(clean-staged-plugins
COMMAND ${CMAKE_COMMAND} -E remove_directory "${PYTHON_OUTPUT_DIRECTORY}/plugins"
)

IF(POLICY CMP0040) # in CMake 3.0.0+
# Skip 'TARGET signature of add_custom_command() must exist' warning, triggered by macro expansion
CMAKE_POLICY (PUSH) # see POP below (NOTE: must wrap related macros, which record policies)
CMAKE_POLICY (SET CMP0040 OLD) # temporary policy for staging/py_compile macros
ENDIF(POLICY CMP0040)

# Macro to byte-compile a target's staged Python resource(s)
MACRO(PY_COMPILE TARGET_NAME RESOURCE_PATHS)
IF(WITH_PY_COMPILE)
Expand All @@ -60,6 +66,10 @@ ADD_SUBDIRECTORY(console)
ADD_SUBDIRECTORY(pyplugin_installer)
ADD_SUBDIRECTORY(ext-libs)

IF(POLICY CMP0040)
CMAKE_POLICY (POP) # see PUSH above
ENDIF(POLICY CMP0040)

INCLUDE_DIRECTORIES(
${PYTHON_INCLUDE_PATH}
${SIP_INCLUDE_DIR}
Expand Down

3 comments on commit a4aaff5

@mathstuf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right way to handle policy warnings. The correct fix is to actually use the new behavior (the old behavior is there for backwards compatibility, not as a choice to keep using the old behavior). Policies may be removed in future versions of CMake, leaving only the new behavior.

@dakcarto
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @mathstuf, I agree, except for one point:

It is a clear choice to keep the old behavior until one has found the time to actually update the CMake code to reflect newer CMake changes, which often only affect a small percentage of QGIS developers who are using a newer CMake, e.g on macOS. We do have to keep the codebase compiling, but can't be waylaid rewriting CMake code every time someone decides to use the latest CMake.

Due to the wide array of platforms and OS versions supported, the minimum CMake for the project is still currently at 2.8.6! I'm guessing that would be a large amount of redundant if/then conditional CMake code.

Granted, with QGIS 3.0, it is an opportune time to fix these issues appropriately and decide upon a newer CMake minimum version. No one has yet taken on the task of refactoring large portions of working CMake code to update to a more modern approach. Developers usually work only on the CMake parts that affect their current work. Such a refactoring would take considerable effort. I intend to do so, albeit only for the macOS bundling routines, which were mostly written before the BundleUtilities were introduced into CMake (2.6.2).

Any guidance on even a partial refactoring would be greatly appreciated, like what bad practices (beyond policy handling) might we focus on updating to avoid deprecated CMake support in the near term?

@mathstuf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CMP0053 errors are easy: just replace @var@ with ${var}. CMP0042 just sets the default, so setting it yourself (should be) sufficient. CMP0048 does indeed require an if/then conditional (since older versions won't set the version variables properly). CMP0040 needs fixed since custom commands are ignored if the target doesn't exist (the policy is to raise an error instead of ignoring the command).

Please sign in to comment.