[bootstrap] Allow passing options to sub-builds for all targets

[ldionne]

Summary

This patch makes it possible to pass a CMake option to one of the runtimes
for all targets being built by using RUNTIMES_<project>_<OPTION>. For
example, one can pass RUNTIMES_LIBCXX_TEST_CONFIG=<config> to the
bootstrapping build, and LIBCXX_TEST_CONFIG=<config> will be passed
to the runtimes sub-build as a result.

This is useful for customizing a sub-build for all targets. Note that
we could also simply allow passing through options without prefixing
them with RUNTIMES_ but this tries to be consistent with what already
exists for per-target passthroughs.

[ldionne]

This is a strawman proposal. The end goal I'd like to achieve is being able to pass options to sub-builds without worrying about setting the right target. In other words, I want to pass roughly LIBCXX_TEST_CONFIG=<config> instead of having to do RUNTIMES_x86_64-apple-darwin20.xx.yy_LIBCXX_TEST_CONFIG=<config> for each target I'm building.

I don't mind how we achieve this, and if @phosek thinks this is not the right approach, I am not married to it.

[ldionne]

Poke CI.

[phosek]

I like this!

This is useful for customizing a sub-build for all targets. Note that
we could also simply allow passing through options without prefixing
them with RUNTIMES_ but this tries to be consistent with what already
exists for per-target passthroughs.

I'd be fine with that, but in that case I'd prefer changing the per-target passthrough to be more consistent and the question is what would that look like.

What I could come up with is <project>_<option>_<target> or <project>_<target>_<option>, so LIBCXX_TEST_CONFIG_x86_64-linux-gnu=<config> or LIBCXX_x86_64-linux-gnu_TEST_CONFIG=<config> but there might be other ways.

llvm/runtimes/CMakeLists.txt

350–356

We should turn this into a function to avoid duplication but that can be done in a follow up change.

[ldionne]

In D121822#3390948, @phosek wrote:

What I could come up with is <project>_<option>_<target> or <project>_<target>_<option>, so LIBCXX_TEST_CONFIG_x86_64-linux-gnu=<config> or LIBCXX_x86_64-linux-gnu_TEST_CONFIG=<config> but there might be other ways.

Hmm, I think I like LIBCXX_TEST_CONFIG_x86_64-linux-gnu best. It is closest to what CMake already does for various options like CMAKE_<LANG>_FLAGS_<CONFIG> (e.g. CMAKE_CXX_FLAGS_Release).

In that case, I will amend this patch so it accepts options in the form LIBCXX_TEST_CONFIG as-is, and we can work on moving the per-target option passthrough to the new convention separately.

[ldionne]

Allow passing e.g. LIBCXX_TEST_CONFIG directly.

[ldionne]

Rebase onto main -- that should fix the bootstrapping CI failure.

[ldionne]

I'm going to land this. What kind of migration story do you want for the per-target properties (which should be handled as a separate effort)?

[phosek]

This actually broke our build. We pass LIBCXX_<option> to the default build on Darwin and RUNTIME_<target>_LIBCXX_<option> to the per-target build, but with this change, the LIBCXX_<option> now affects the per-target builds as well which is undesirable.

We need a way to pass options to Darwin builds. D117263 is an attempt at that but that change hasn't landed yet. Would it be fine with you if I reverted your change while we figure out a solution?

In D121822#3397391, @ldionne wrote:

I'm going to land this. What kind of migration story do you want for the per-target properties (which should be handled as a separate effort)?

I'd like to use this opportunity to land D117263 so you can do LIBCXX_TEST_CONFIG_foo and not necessarily just LIBCXX_TEST_CONFIG_x86_64-linux-gnu. I'll update that change to support the new scheme.

We then need to decide if we want to continue supporting the current scheme RUNTIMES_<target>_<project>_<option> scheme. If not, then we need to start emitting a warning and mention that we'll deprecate that scheme in the next release.

[ldionne]

@phosek Thanks for reverting. Just to make sure I understand, you're going to roll this change into D117263 so I don't have anything to pick up?

[phosek]

In D121822#3399848, @ldionne wrote:

@phosek Thanks for reverting. Just to make sure I understand, you're going to roll this change into D117263 so I don't have anything to pick up?

Yes that's the plan.

libcxx/utils/ci/run-buildbot

${CMAKE} \
           -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
           -DLLVM_ENABLE_PROJECTS="clang" \
           -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" \
           -DLLVM_RUNTIME_TARGETS="$(c++ --print-target-triple)" \
           -DLLVM_TARGETS_TO_BUILD="host" \
           -DRUNTIMES_BUILD_ALLOW_DARWIN=ON \
           -DLLVM_ENABLE_ASSERTIONS=ON \
           -C "${MONOREPO_ROOT}/libcxx/cmake/caches/Generic-modules.cmake" \
-          -DRUNTIMES_LIBUNWIND_TEST_CONFIG="llvm-libunwind-shared.cfg.in"
+          -DLIBUNWIND_TEST_CONFIG="llvm-libunwind-shared.cfg.in"
 
     echo "+++ Running the libc++ and libc++abi tests"
     ${NINJA} -C "${BUILD_DIR}" check-runtimes
 
     echo "--- Installing libc++ and libc++abi to a fake location"
     ${NINJA} -C "${BUILD_DIR}" install-runtimes
 ;;
 legacy-test-config)

llvm/runtimes/CMakeLists.txt

foreach(variableName ${variableNames})
       if("${out}" EQUAL 0)
         string(REPLACE "RUNTIMES_${name}_" "" new_name ${variableName})
         string(REPLACE ";" "|" new_value "${${variableName}}")
         list(APPEND ${name}_extra_args "-D${new_name}=${new_value}")
       endif()
     endforeach()
   endif()
 
+  # If we're passed any option like <project>_FOO, pass it through to the sub-builds.
+  foreach(runtime ${LLVM_ENABLE_RUNTIMES})
+    string(TOUPPER "${runtime}" runtime_upper)
+    foreach(variableName ${variableNames})
+      if (variableName MATCHES "^${runtime_upper}_.+")
+        string(REPLACE ";" "|" escaped_value "${${variableName}}")
+        list(APPEND ${name}_extra_args "-D${variableName}=${escaped_value}")
+      endif()
+    endforeach()
+  endforeach()
+
   if(NOT RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES AND NOT RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES)
     string(REPLACE ";" "|" LLVM_ENABLE_RUNTIMES_PASSTHROUGH "${LLVM_ENABLE_RUNTIMES}")
     list(APPEND ${name}_extra_args -DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
   endif()
 
   llvm_ExternalProject_Add(runtimes-${name}
                            ${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
                            DEPENDS ${${name}_deps}