Skip to content

Conversation

@boomanaiden154
Copy link
Contributor

When building just the runtimes (eg a patch only touches compiler-rt), we do not actually run any normal check targets. This ends up causing an empty ninja invocation, which builds more targets than necessary. Gate the ninja build for normal check-* targets under an if statement to fix this.

When building just the runtimes (eg a patch only touches compiler-rt),
we do not actually run any normal check targets. This ends up causing an
empty ninja invocation, which builds more targets than necessary. Gate
the ninja build for normal check-* targets under an if statement to fix
this.
@llvmbot llvmbot added the infrastructure Bugs about LLVM infrastructure label Nov 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-infrastructure

Author: Aiden Grossman (boomanaiden154)

Changes

When building just the runtimes (eg a patch only touches compiler-rt), we do not actually run any normal check targets. This ends up causing an empty ninja invocation, which builds more targets than necessary. Gate the ninja build for normal check-* targets under an if statement to fix this.


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

2 Files Affected:

  • (modified) .ci/monolithic-linux.sh (+5-3)
  • (modified) .ci/monolithic-windows.sh (+5-3)
diff --git a/.ci/monolithic-linux.sh b/.ci/monolithic-linux.sh
index 4a8418d7baa8c..ca619aa7e98a1 100755
--- a/.ci/monolithic-linux.sh
+++ b/.ci/monolithic-linux.sh
@@ -64,9 +64,11 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \
 
 start-group "ninja"
 
-# Targets are not escaped as they are passed as separate arguments.
-ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
-cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
+if [[ "${targets}" != "" ]]; then
+  # Targets are not escaped as they are passed as separate arguments.
+  ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
+  cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
+fi
 
 if [[ "${runtime_targets}" != "" ]]; then
   start-group "ninja Runtimes"
diff --git a/.ci/monolithic-windows.sh b/.ci/monolithic-windows.sh
index 7b926b87f3623..99e7758ce8d79 100755
--- a/.ci/monolithic-windows.sh
+++ b/.ci/monolithic-windows.sh
@@ -51,9 +51,11 @@ cmake -S "${MONOREPO_ROOT}"/llvm -B "${BUILD_DIR}" \
 
 start-group "ninja"
 
-# Targets are not escaped as they are passed as separate arguments.
-ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
-cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
+if [[ "${targets}" != "" ]]; then
+  # Targets are not escaped as they are passed as separate arguments.
+  ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
+  cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
+fi
 
 if [[ "${runtimes_targets}" != "" ]]; then
   start-group "ninja runtimes"

@github-actions
Copy link

🐧 Linux x64 Test Results

  • 193651 tests passed
  • 5829 tests skipped

# Targets are not escaped as they are passed as separate arguments.
ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
if [[ "${targets}" != "" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: use "-z" instead, i.e.

if [[ -z "${targets} ]] ; then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is stylistically consistent with the rest of the premerge bash scripts. If we prefer -z, we should probably switch over all at once in a follow up/base patch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I think we should switch to '-z' everywhere, but I'm ok with doing that in another PR.

# Targets are not escaped as they are passed as separate arguments.
ninja -C "${BUILD_DIR}" -k 0 ${targets} |& tee ninja.log
cp ${BUILD_DIR}/.ninja_log ninja.ninja_log
if [[ "${targets}" != "" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same suggestion here.

@boomanaiden154 boomanaiden154 merged commit b533712 into llvm:main Nov 18, 2025
13 checks passed
@boomanaiden154 boomanaiden154 deleted the only-build-runtimes-targets-ci branch November 18, 2025 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Bugs about LLVM infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants