Revert "Reapply "[CI] Migrate to runtimes build""

[Keenuts]

Looks like reverting this commit solves the broken presubmit.

Fixes #145703

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Nathan Gauër (Keenuts)

Changes

Looks like reverting this commit solves the broken presubmit.

Fixes #145703

---

Full diff: https://github.com/llvm/llvm-project/pull/145742.diff

4 Files Affected:

• (modified) .ci/compute_projects.py (+45-75)
• (modified) .ci/compute_projects_test.py (+4-51)
• (modified) .ci/monolithic-linux.sh (+3-10)
• (modified) .github/workflows/premerge.yaml (+1-2)

diff --git a/.ci/compute_projects.py b/.ci/compute_projects.py
index 80e4e0221b3d7..86733af1a275a 100644
--- a/.ci/compute_projects.py
+++ b/.ci/compute_projects.py
@@ -49,7 +49,8 @@
     },
     "lld": {"bolt", "cross-project-tests"},
     # TODO(issues/132795): LLDB should be enabled on clang changes.
-    "clang": {"clang-tools-extra", "cross-project-tests"},
+    "clang": {"clang-tools-extra", "compiler-rt", "cross-project-tests"},
+    "clang-tools-extra": {"libc"},
     "mlir": {"flang"},
     # Test everything if ci scripts are changed.
     ".ci": {
@@ -74,16 +75,7 @@
 
 # This mapping describes runtimes that should be tested when the key project is
 # touched.
-DEPENDENT_RUNTIMES_TO_TEST = {
-    "clang": {"compiler-rt"},
-    "clang-tools-extra": {"libc"},
-    ".ci": {"compiler-rt", "libc"},
-}
-DEPENDENT_RUNTIMES_TO_TEST_NEEDS_RECONFIG = {
-    "llvm": {"libcxx", "libcxxabi", "libunwind"},
-    "clang": {"libcxx", "libcxxabi", "libunwind"},
-    ".ci": {"libcxx", "libcxxabi", "libunwind"},
-}
+DEPENDENT_RUNTIMES_TO_TEST = {"clang": {"libcxx", "libcxxabi", "libunwind"}}
 
 EXCLUDE_LINUX = {
     "cross-project-tests",  # TODO(issues/132796): Tests are failing.
@@ -112,6 +104,9 @@
     "cross-project-tests",
     "flang",
     "libc",
+    "libcxx",
+    "libcxxabi",
+    "libunwind",
     "lldb",
     "openmp",
     "polly",
@@ -138,35 +133,21 @@
     "polly": "check-polly",
 }
 
-RUNTIMES = {"libcxx", "libcxxabi", "libunwind", "compiler-rt", "libc"}
+RUNTIMES = {"libcxx", "libcxxabi", "libunwind"}
 
 
-def _add_dependencies(projects: Set[str], runtimes: Set[str]) -> Set[str]:
+def _add_dependencies(projects: Set[str]) -> Set[str]:
     projects_with_dependents = set(projects)
     current_projects_count = 0
     while current_projects_count != len(projects_with_dependents):
         current_projects_count = len(projects_with_dependents)
         for project in list(projects_with_dependents):
-            if project in PROJECT_DEPENDENCIES:
-                projects_with_dependents.update(PROJECT_DEPENDENCIES[project])
-    for runtime in runtimes:
-        if runtime in PROJECT_DEPENDENCIES:
-            projects_with_dependents.update(PROJECT_DEPENDENCIES[runtime])
+            if project not in PROJECT_DEPENDENCIES:
+                continue
+            projects_with_dependents.update(PROJECT_DEPENDENCIES[project])
     return projects_with_dependents
 
 
-def _exclude_projects(current_projects: Set[str], platform: str) -> Set[str]:
-    if platform == "Linux":
-        to_exclude = EXCLUDE_LINUX
-    elif platform == "Windows":
-        to_exclude = EXCLUDE_WINDOWS
-    elif platform == "Darwin":
-        to_exclude = EXCLUDE_MAC
-    else:
-        raise ValueError(f"Unexpected platform: {platform}")
-    return current_projects.difference(to_exclude)
-
-
 def _compute_projects_to_test(modified_projects: Set[str], platform: str) -> Set[str]:
     projects_to_test = set()
     for modified_project in modified_projects:
@@ -184,14 +165,25 @@ def _compute_projects_to_test(modified_projects: Set[str], platform: str) -> Set
             ):
                 continue
             projects_to_test.add(dependent_project)
-    projects_to_test = _exclude_projects(projects_to_test, platform)
+    if platform == "Linux":
+        for to_exclude in EXCLUDE_LINUX:
+            if to_exclude in projects_to_test:
+                projects_to_test.remove(to_exclude)
+    elif platform == "Windows":
+        for to_exclude in EXCLUDE_WINDOWS:
+            if to_exclude in projects_to_test:
+                projects_to_test.remove(to_exclude)
+    elif platform == "Darwin":
+        for to_exclude in EXCLUDE_MAC:
+            if to_exclude in projects_to_test:
+                projects_to_test.remove(to_exclude)
+    else:
+        raise ValueError("Unexpected platform.")
     return projects_to_test
 
 
-def _compute_projects_to_build(
-    projects_to_test: Set[str], runtimes: Set[str]
-) -> Set[str]:
-    return _add_dependencies(projects_to_test, runtimes)
+def _compute_projects_to_build(projects_to_test: Set[str]) -> Set[str]:
+    return _add_dependencies(projects_to_test)
 
 
 def _compute_project_check_targets(projects_to_test: Set[str]) -> Set[str]:
@@ -202,34 +194,24 @@ def _compute_project_check_targets(projects_to_test: Set[str]) -> Set[str]:
     return check_targets
 
 
-def _compute_runtimes_to_test(modified_projects: Set[str], platform: str) -> Set[str]:
+def _compute_runtimes_to_test(projects_to_test: Set[str]) -> Set[str]:
     runtimes_to_test = set()
-    for modified_project in modified_projects:
-        if modified_project in DEPENDENT_RUNTIMES_TO_TEST:
-            runtimes_to_test.update(DEPENDENT_RUNTIMES_TO_TEST[modified_project])
-    return _exclude_projects(runtimes_to_test, platform)
-
-
-def _compute_runtimes_to_test_needs_reconfig(
-    modified_projects: Set[str], platform: str
-) -> Set[str]:
-    runtimes_to_test = set()
-    for modified_project in modified_projects:
-        if modified_project in DEPENDENT_RUNTIMES_TO_TEST_NEEDS_RECONFIG:
-            runtimes_to_test.update(
-                DEPENDENT_RUNTIMES_TO_TEST_NEEDS_RECONFIG[modified_project]
-            )
-    return _exclude_projects(runtimes_to_test, platform)
+    for project_to_test in projects_to_test:
+        if project_to_test in DEPENDENT_RUNTIMES_TO_TEST:
+            runtimes_to_test.update(DEPENDENT_RUNTIMES_TO_TEST[project_to_test])
+        if project_to_test in DEPENDENT_RUNTIMES_TO_BUILD:
+            runtimes_to_test.update(DEPENDENT_RUNTIMES_TO_BUILD[project_to_test])
+    return runtimes_to_test
 
 
-def _compute_runtimes_to_build(
-    runtimes_to_test: Set[str], modified_projects: Set[str], platform: str
-) -> Set[str]:
-    runtimes_to_build = set(runtimes_to_test)
-    for modified_project in modified_projects:
-        if modified_project in DEPENDENT_RUNTIMES_TO_BUILD:
-            runtimes_to_build.update(DEPENDENT_RUNTIMES_TO_BUILD[modified_project])
-    return _exclude_projects(runtimes_to_build, platform)
+def _compute_runtime_check_targets(projects_to_test: Set[str]) -> Set[str]:
+    check_targets = set()
+    for project_to_test in projects_to_test:
+        if project_to_test not in DEPENDENT_RUNTIMES_TO_TEST:
+            continue
+        for runtime_to_test in DEPENDENT_RUNTIMES_TO_TEST[project_to_test]:
+            check_targets.add(PROJECT_CHECK_TARGETS[runtime_to_test])
+    return check_targets
 
 
 def _get_modified_projects(modified_files: list[str]) -> Set[str]:
@@ -253,19 +235,10 @@ def _get_modified_projects(modified_files: list[str]) -> Set[str]:
 def get_env_variables(modified_files: list[str], platform: str) -> Set[str]:
     modified_projects = _get_modified_projects(modified_files)
     projects_to_test = _compute_projects_to_test(modified_projects, platform)
-    runtimes_to_test = _compute_runtimes_to_test(modified_projects, platform)
-    runtimes_to_test_needs_reconfig = _compute_runtimes_to_test_needs_reconfig(
-        modified_projects, platform
-    )
-    runtimes_to_build = _compute_runtimes_to_build(
-        runtimes_to_test | runtimes_to_test_needs_reconfig, modified_projects, platform
-    )
-    projects_to_build = _compute_projects_to_build(projects_to_test, runtimes_to_build)
+    projects_to_build = _compute_projects_to_build(projects_to_test)
     projects_check_targets = _compute_project_check_targets(projects_to_test)
-    runtimes_check_targets = _compute_project_check_targets(runtimes_to_test)
-    runtimes_check_targets_needs_reconfig = _compute_project_check_targets(
-        runtimes_to_test_needs_reconfig
-    )
+    runtimes_to_build = _compute_runtimes_to_test(projects_to_test)
+    runtimes_check_targets = _compute_runtime_check_targets(projects_to_test)
     # We use a semicolon to separate the projects/runtimes as they get passed
     # to the CMake invocation and thus we need to use the CMake list separator
     # (;). We use spaces to separate the check targets as they end up getting
@@ -275,9 +248,6 @@ def get_env_variables(modified_files: list[str], platform: str) -> Set[str]:
         "project_check_targets": " ".join(sorted(projects_check_targets)),
         "runtimes_to_build": ";".join(sorted(runtimes_to_build)),
         "runtimes_check_targets": " ".join(sorted(runtimes_check_targets)),
-        "runtimes_check_targets_needs_reconfig": " ".join(
-            sorted(runtimes_check_targets_needs_reconfig)
-        ),
     }
 
 
diff --git a/.ci/compute_projects_test.py b/.ci/compute_projects_test.py
index 5efac26366981..2c8015656b2d0 100644
--- a/.ci/compute_projects_test.py
+++ b/.ci/compute_projects_test.py
@@ -26,10 +26,6 @@ def test_llvm(self):
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -50,10 +46,6 @@ def test_llvm_windows(self):
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -74,10 +66,6 @@ def test_llvm_mac(self):
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -87,21 +75,17 @@ def test_clang(self):
         )
         self.assertEqual(
             env_variables["projects_to_build"],
-            "clang;clang-tools-extra;lld;llvm",
+            "clang;clang-tools-extra;compiler-rt;lld;llvm",
         )
         self.assertEqual(
             env_variables["project_check_targets"],
-            "check-clang check-clang-tools",
+            "check-clang check-clang-tools check-compiler-rt",
         )
         self.assertEqual(
-            env_variables["runtimes_to_build"], "compiler-rt;libcxx;libcxxabi;libunwind"
+            env_variables["runtimes_to_build"], "libcxx;libcxxabi;libunwind"
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "check-compiler-rt",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -120,10 +104,6 @@ def test_clang_windows(self):
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -135,7 +115,6 @@ def test_bolt(self):
         self.assertEqual(env_variables["project_check_targets"], "check-bolt")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_lldb(self):
         env_variables = compute_projects.get_env_variables(
@@ -145,7 +124,6 @@ def test_lldb(self):
         self.assertEqual(env_variables["project_check_targets"], "check-lldb")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_mlir(self):
         env_variables = compute_projects.get_env_variables(
@@ -157,7 +135,6 @@ def test_mlir(self):
         )
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_flang(self):
         env_variables = compute_projects.get_env_variables(
@@ -167,7 +144,6 @@ def test_flang(self):
         self.assertEqual(env_variables["project_check_targets"], "check-flang")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_invalid_subproject(self):
         env_variables = compute_projects.get_env_variables(
@@ -177,7 +153,6 @@ def test_invalid_subproject(self):
         self.assertEqual(env_variables["project_check_targets"], "")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_top_level_file(self):
         env_variables = compute_projects.get_env_variables(["README.md"], "Linux")
@@ -185,7 +160,6 @@ def test_top_level_file(self):
         self.assertEqual(env_variables["project_check_targets"], "")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_exclude_runtiems_in_projects(self):
         env_variables = compute_projects.get_env_variables(
@@ -195,7 +169,6 @@ def test_exclude_runtiems_in_projects(self):
         self.assertEqual(env_variables["project_check_targets"], "")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_exclude_docs(self):
         env_variables = compute_projects.get_env_variables(
@@ -205,7 +178,6 @@ def test_exclude_docs(self):
         self.assertEqual(env_variables["project_check_targets"], "")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_exclude_gn(self):
         env_variables = compute_projects.get_env_variables(
@@ -215,7 +187,6 @@ def test_exclude_gn(self):
         self.assertEqual(env_variables["project_check_targets"], "")
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
     def test_ci(self):
         env_variables = compute_projects.get_env_variables(
@@ -230,15 +201,10 @@ def test_ci(self):
             "check-bolt check-clang check-clang-tools check-flang check-lld check-lldb check-llvm check-mlir check-polly",
         )
         self.assertEqual(
-            env_variables["runtimes_to_build"],
-            "compiler-rt;libc;libcxx;libcxxabi;libunwind",
+            env_variables["runtimes_to_build"], "libcxx;libcxxabi;libunwind"
         )
         self.assertEqual(
             env_variables["runtimes_check_targets"],
-            "check-compiler-rt check-libc",
-        )
-        self.assertEqual(
-            env_variables["runtimes_check_targets_needs_reconfig"],
             "check-cxx check-cxxabi check-unwind",
         )
 
@@ -252,19 +218,6 @@ def test_lldb(self):
             env_variables["runtimes_to_build"], "libcxx;libcxxabi;libunwind"
         )
         self.assertEqual(env_variables["runtimes_check_targets"], "")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
-
-    def test_clang_tools_extra(self):
-        env_variables = compute_projects.get_env_variables(
-            ["clang-tools-extra/CMakeLists.txt"], "Linux"
-        )
-        self.assertEqual(
-            env_variables["projects_to_build"], "clang;clang-tools-extra;lld;llvm"
-        )
-        self.assertEqual(env_variables["project_check_targets"], "check-clang-tools")
-        self.assertEqual(env_variables["runtimes_to_build"], "libc")
-        self.assertEqual(env_variables["runtimes_check_targets"], "check-libc")
-        self.assertEqual(env_variables["runtimes_check_targets_needs_reconfig"], "")
 
 
 if __name__ == "__main__":
diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index 89447963b8528..1a4ccab783f0c 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -52,7 +52,6 @@ projects="${1}"
 targets="${2}"
 runtimes="${3}"
 runtime_targets="${4}"
-runtime_targets_needs_reconfig="${5}"
 
 lit_args="-v --xunit-xml-output ${BUILD_DIR}/test-results.xml --use-unique-output-file-name --timeout=1200 --time-tests"
 
@@ -89,15 +88,9 @@ echo "--- ninja"
 # Targets are not escaped as they are passed as separate arguments.
 ninja -C "${BUILD_DIR}" -k 0 ${targets}
 
-if [[ "${runtime_targets}" != "" ]]; then
-  echo "--- ninja runtimes"
-
-  ninja -C "${BUILD_DIR}" ${runtime_targets}
-fi
-
 # Compiling runtimes with just-built Clang and running their tests
 # as an additional testing for Clang.
-if [[ "${runtime_targets_needs_reconfig}" != "" ]]; then
+if [[ "${runtimes_targets}" != "" ]]; then
   echo "--- cmake runtimes C++26"
 
   cmake \
@@ -107,7 +100,7 @@ if [[ "${runtime_targets_needs_reconfig}" != "" ]]; then
 
   echo "--- ninja runtimes C++26"
 
-  ninja -C "${BUILD_DIR}" ${runtime_targets_needs_reconfig}
+  ninja -C "${BUILD_DIR}" ${runtime_targets}
 
   echo "--- cmake runtimes clang modules"
 
@@ -118,5 +111,5 @@ if [[ "${runtime_targets_needs_reconfig}" != "" ]]; then
 
   echo "--- ninja runtimes clang modules"
 
-  ninja -C "${BUILD_DIR}" ${runtime_targets_needs_reconfig}
+  ninja -C "${BUILD_DIR}" ${runtime_targets}
 fi
diff --git a/.github/workflows/premerge.yaml b/.github/workflows/premerge.yaml
index 4435a3e905768..709b6d03d94c3 100644
--- a/.github/workflows/premerge.yaml
+++ b/.github/workflows/premerge.yaml
@@ -56,12 +56,11 @@ jobs:
           echo "Running project checks targets: ${project_check_targets}"
           echo "Building runtimes: ${runtimes_to_build}"
           echo "Running runtimes checks targets: ${runtimes_check_targets}"
-          echo "Running runtimes checks requiring reconfiguring targets: ${runtimes_check_targets_needs_reconfig}"
 
           export CC=/opt/llvm/bin/clang
           export CXX=/opt/llvm/bin/clang++
 
-          ./.ci/monolithic-linux.sh "${projects_to_build}" "${project_check_targets}" "${runtimes_to_build}" "${runtimes_check_targets}" "${runtimes_check_targets_needs_reconfig}"
+          ./.ci/monolithic-linux.sh "${projects_to_build}" "${project_check_targets}" "${runtimes_to_build}" "${runtimes_check_targets}"
       - name: Upload Artifacts
         uses: actions/upload-artifact@65c4c4a1ddee5b72f698fdd19549f0f0fb45cf08 # v4.6.0
         with:

[boomanaiden154]

I don’t really understand how this fixes it. The error is with clang-tools-extra which was built before this patch too. This patch just changes the runtime builds shouldn’t impact the check-clang-tools target.

I would also prefer to avoid reverting this if possible. The tight interaction between clang and libcxx made this a pain to land and to reland (three libcxx patches and one LWG issue required).

It’s also a race condition (seemingly), so how have you validated that everything works as expected?

[Keenuts]

I don’t really understand how this fixes it. The error is with clang-tools-extra which was built before this patch too. This patch just changes the runtime builds shouldn’t impact the check-clang-tools target.

I don't think this fixes it, but seems like this hides it (not great)

I would also prefer to avoid reverting this if possible. The tight interaction between clang and libcxx made this a pain to land and to reland (three libcxx patches and one LWG issue required).

It’s also a race condition (seemingly), so how have you validated that everything works as expected?

Yes, the more I dig the more I think this is an actual race condition. I was able to reproduce on an older LLVM commit, but not all the time.