[PATCH] D72947: [CMake] llvm/runtimes: Do not override LLVM_* variables with just-built LLVM configurations

[James Nagurne via Phabricator via llvm-commits]

JamesNagurne abandoned this revision.
JamesNagurne added a comment.

The problem here is the method we reset the variables in the cache back to their cached value.

  set(VAR1 "A;;B;;C")
  message(STATUS "VAR1 ${VAR1}")
  set(VAR2 ${VAR1})
  message(STATUS "VAR2 ${VAR2}")
  set(VAR3 "${VAR1})
  message(STATUS "VAR3 ${VAR3}")

Emits the following:
VAR1 A;;B;;C
VAR2 A;B;C
VAR3 A;;B;;C

This confuses me, but it seems to be the way cmake does things. A;;B;;C is a 5 element list, and when used as a raw list, empty elements are deleted.
The reason there is an issue here is because in some work that has happened since I've uploaded this changeset, we now utilize find_package(Python) where possible instead of include(FindPythonInterpreter).
The former sets an internal cache variable, _{PYTHON_PREFIX}_INTERPRETER_PROPERTIES, to a list of properties as a cache, and then indexes into it (https://github.com/Kitware/CMake/blob/v3.17.2/Modules/FindPython/Support.cmake#L1299), when called a second time.

My code as writtten will delete empty elements in this list, and cause cmake to error out.
The solution on our end is to set the variable as you see above for VAR3. This will preserve the empty elements, and doesn't seem to change the behavior of special values like ON/OFF or integers found in other cache variables.

However, I feel uncomfortable with having this particular solution committed to the upstream due to the dubious nature of using CACHE_VARIABLES and with my relative uncertainty of cmake semantics.
I'll retract this for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72947