[PATCH] D107532: Get CPACK config variables from the cache to allow overriding via cache file

[Ben Boeckel via Phabricator via llvm-commits]

ben.boeckel requested changes to this revision.
ben.boeckel added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/CMakeLists.txt:232
 # Configure CPack.
-set(CPACK_PACKAGE_INSTALL_DIRECTORY "LLVM")
-set(CPACK_PACKAGE_VENDOR "LLVM")
-set(CPACK_PACKAGE_VERSION_MAJOR ${LLVM_VERSION_MAJOR})
-set(CPACK_PACKAGE_VERSION_MINOR ${LLVM_VERSION_MINOR})
-set(CPACK_PACKAGE_VERSION_PATCH ${LLVM_VERSION_PATCH})
-set(CPACK_PACKAGE_VERSION ${PACKAGE_VERSION})
-set(CPACK_RESOURCE_FILE_LICENSE "${CMAKE_CURRENT_SOURCE_DIR}/LICENSE.TXT")
-set(CPACK_NSIS_COMPRESSOR "/SOLID lzma \r\n SetCompressorDictSize 32")
+set(CPACK_PACKAGE_INSTALL_DIRECTORY "LLVM" CACHE STRING "")
+set(CPACK_PACKAGE_VENDOR "LLVM" CACHE STRING "")
----------------
Please either add docstrings for each of these or use `mark_as_advanced` to hide them from regular developer cache editing activities. Preferably both.


================
Comment at: llvm/CMakeLists.txt:234
+set(CPACK_PACKAGE_VENDOR "LLVM" CACHE STRING "")
+set(CPACK_PACKAGE_VERSION_MAJOR ${LLVM_VERSION_MAJOR} CACHE STRING "")
+set(CPACK_PACKAGE_VERSION_MINOR ${LLVM_VERSION_MINOR} CACHE STRING "")
----------------
There is also the "sticky cache" issue here. If I configure by build tree first with 13.0.0, this gets set to 13. If I then continue to use this build tree through to 14.0.0…it is still 13. I suggest, instead doing something like this:

    set(CPACK_PACKAGE_VERSION_MAJOR "<DEFAULT>" CACHE STRING "")
    if (CPACK_PACKAGE_VERSION_MAJOR STREQUAL "<DEFAULT>")
      set(CPACK_PACKAGE_VERSION_MAJOR "${LLVM_VERSION_MAJOR}")
    elseif (NOT CPACK_PACKAGE_VERSION_MAJOR STREQUAL LLVM_VERSION_MAJOR)
      # This condition is optional, but can help when manual settings are stale.
      message(AUTHOR_WARNING
        "Manually set `CPACK_PACKAGE_VERSION_MAJOR` does not match the "
        "detected major version of `${LLVM_VERSION_MAJOR}`; your cache may "
        "be out-of-date."
    endif ()

so that it actually acts like a default rather than an initial value that "sticks around" in cases where "no one" would want it to.  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107532/new/

https://reviews.llvm.org/D107532