[cmake] Make LIT_COMMAND configurable and improve fallback support

[mgorny]

Summary

Make LIT_COMMAND configurable, use source tree only when actually available and extend the default search to other common executable names 'lit.py' and 'lit', in order to increase uniformity between all LLVM projects and support using installed lit.

Changing the conditional used to determine whether in-tree or external lit is being used covers the case when LLVM_MAIN_SRC_DIR is defined but does not exist (anymore). In this case, the functions falls back to looking for installed lit rather than attempting to use a non-existing path. The same conditional is used in clang already.

Making LIT_COMMAND a cache variable in case the source tree variant is used serves two purposes. Firstly, it increases uniformity between the two branches since find_program() implicitly makes LIT_COMMAND a cache variable. Secondly, it allows overriding the lit executable used to run the tests when the LLVM source tree is provided. Gentoo is planning to use this to use installed (and byte-compiled) lit instead of re-compiling it in every LLVM project.

Extending default search is meant to increase uniformity between different LLVM projects. The 'lit.py' name is already used by a few of them, and 'lit' is the name used by utils/lit/setup.py when installing.

[mgorny]

Updated: fixed find_program() use.

[beanz]

LGTM!

[mgorny]

Thanks for the review. Committing in a minute.

[mgorny]

@beanz, it seems that the commit has caused quite a huge breakage due to typo in the default. I've fixed it already but it seems that buildbots are sticking to the old value due to CMakeCache.txt. Any idea how I could force them to reset?

[mgorny]

Since there seems to be no clean way of resetting CMakeCache.txt for all buildbots, I have reverted the original change. If I'm not mistaken, this should cause the CMakeCache.txt files to be cleaned up implicitly since LIT_COMMAND will no longer be a cache variable. After all bots rebuild cleanly, it should be possible to reintroduce the change, with the correct default value.

I've updated the patch for that. The difference is that the default needs to be semicolon-separated rather than space-separated (the non-cache 'set' variant implicitly converts ARGN to a semicolon-separated list).

[mgorny]

I think the bots are clean now. @beanz, would it be ok for me to commit the updated patch? I'm going to do one more round of local testing in the meantime.

[beanz]

CMake has no method for cache invalidation. Reverting your patch will not have cleaned up the caches.

[mgorny]

In D25076#561118, @beanz wrote:

CMake has no method for cache invalidation. Reverting your patch will not have cleaned up the caches.

Damn it. Any idea how to proceed then?

[mgorny]

@beanz, ok reading CMake docs I think FORCE could help there. That is:

1. Commit the patch adding FORCE to the cache variable. This will force resetting the value on buildbots where it is set. Since in this branch of code, it currently isn't a cache variable, no people should be affected by forcing the value.
2. Wait for all bots to rebuild it.
3. Remove the FORCE to make variable behave normally.

I don't like the idea of committing workarounds for buildbots like this but I guess the only alternative would be to require manual action for every buildbot.

[beanz]

In the past when I have had these issues I've written temporary workaround code that has inspected the variable for the bad value, then used unset to remove it from the cache and follow the normal flow from there.

Following that model you end up with a block of code that you can put a TODO comment above that says to remove it, and when it should be safe to remove.

[mgorny]

Thanks for the suggestion, @beanz. I've updated the patch appropriately, and confirmed locally that it corrects the previous value successfully.

[beanz]

LGTM!

[mgorny]

Thanks again. Crossing fingers and waiting for bots to build.

llvm/trunk/cmake/modules/AddLLVM.cmake

 # management functions.
 function(add_lit_target target comment)
   cmake_parse_arguments(ARG "" "" "PARAMS;DEPENDS;ARGS" ${ARGN})
   set(LIT_ARGS "${ARG_ARGS} ${LLVM_LIT_ARGS}")
   separate_arguments(LIT_ARGS)
   if (NOT CMAKE_CFG_INTDIR STREQUAL ".")
     list(APPEND LIT_ARGS --param build_mode=${CMAKE_CFG_INTDIR})
   endif ()
-  if (LLVM_MAIN_SRC_DIR)
-    set (LIT_COMMAND ${PYTHON_EXECUTABLE} ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
+  if (EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
+    # reset cache after erraneous r283029
+    # TODO: remove this once all buildbots run
+    if (LIT_COMMAND STREQUAL "${PYTHON_EXECUTABLE} ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py")
+      unset(LIT_COMMAND CACHE)
+    endif()
+    set (LIT_COMMAND "${PYTHON_EXECUTABLE};${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py"
+         CACHE STRING "Command used to spawn llvm-lit")
   else()
-    find_program(LIT_COMMAND llvm-lit)
+    find_program(LIT_COMMAND NAMES llvm-lit lit.py lit)
   endif ()
   list(APPEND LIT_COMMAND ${LIT_ARGS})
   foreach(param ${ARG_PARAMS})
     list(APPEND LIT_COMMAND --param ${param})
   endforeach()
   if (ARG_UNPARSED_ARGUMENTS)
     add_custom_target(${target}
       COMMAND ${LIT_COMMAND} ${ARG_UNPARSED_ARGUMENTS}