-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Add option for verbose output from run-buildbot
. Disable by default.
#144704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The run-buildbot script with verbose output generates thousands of lines of output about copying headers around. This output makes it very difficult to find the actual CI failures among the noise. The original idea behind verbose output was that it would help debug CI failures. I believe the non-verbose CMake output is (A) Sufficient to debug any issues you encounter, given that the cmake failure and log files are uploaded as artifacts. (B) It would be much easier to debug CI failures if the logs were shorter. Verbose output from the buildbot script can still be enabled using `-v` or `--verbose`
@llvm/pr-subscribers-libcxx Author: Eric (EricWF) ChangesThe run-buildbot script with verbose output generates thousands of lines of output about copying headers around. This output makes it very difficult to find the actual CI failures among the noise. The original idea behind verbose output was that it would help debug CI failures. I believe the non-verbose CMake output is Verbose output from the buildbot script can still be enabled using Full diff: https://github.com/llvm/llvm-project/pull/144704.diff 1 Files Affected:
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index d8b23be9a0323..01c5a46e6de99 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -22,6 +22,8 @@ ${PROGNAME} [options] <BUILDER>
[-h|--help] Display this help and exit.
+[-v|--verbose] Enable verbose output.
+
--llvm-root <DIR> Path to the root of the LLVM monorepo. By default, we try
to figure it out based on the current working directory.
@@ -48,12 +50,18 @@ if [[ $# == 0 ]]; then
exit 0
fi
+VERBOSE_FLAG=''
+
while [[ $# -gt 0 ]]; do
case ${1} in
-h|--help)
usage
exit 0
;;
+ -v|--verbose)
+ VERBOSE_FLAG='-v'
+ shift
+ ;;
--llvm-root)
MONOREPO_ROOT="${2}"
shift; shift
@@ -168,25 +176,25 @@ function generate-cmake-android() {
function check-runtimes() {
step "Building libc++ test dependencies"
- ${NINJA} -vC "${BUILD_DIR}" cxx-test-depends
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" cxx-test-depends
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
step "Running the libc++abi tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxxabi
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxxabi
step "Running the libunwind tests"
- ${NINJA} -vC "${BUILD_DIR}" check-unwind
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-unwind
}
# TODO: The goal is to test this against all configurations. We should also move
# this to the Lit test suite instead of being a separate CMake target.
function check-abi-list() {
step "Running the libc++ ABI list test"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx-abilist || (
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx-abilist || (
error "Generating the libc++ ABI list after failed check"
- ${NINJA} -vC "${BUILD_DIR}" generate-cxx-abilist
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" generate-cxx-abilist
false
)
}
@@ -233,7 +241,7 @@ function test-armv7m-picolibc() {
"${@}"
step "Installing compiler-rt"
- ${NINJA} -vC "${BUILD_DIR}/compiler-rt" install
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}/compiler-rt" install
# Move compiler-rt libs into the same directory as all the picolib objects.
mv "${INSTALL_DIR}/lib/armv7m-unknown-none-eabi"/* "${INSTALL_DIR}/lib"
@@ -256,7 +264,7 @@ check-generated-output)
# Reject patches that forgot to re-run the generator scripts.
step "Making sure the generator scripts were run"
set +x # Printing all the commands below just creates extremely confusing output
- ${NINJA} -vC "${BUILD_DIR}" libcxx-generate-files
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" libcxx-generate-files
git diff | tee ${BUILD_DIR}/generated_output.patch
git ls-files -o --exclude-standard | tee ${BUILD_DIR}/generated_output.status
! grep -q '^--- a' ${BUILD_DIR}/generated_output.patch || false
@@ -400,13 +408,13 @@ bootstrapping-build)
-DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests"
step "Running the libc++ and libc++abi tests"
- ${NINJA} -vC "${BUILD_DIR}" check-runtimes
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-runtimes
step "Installing libc++ and libc++abi to a fake location"
- ${NINJA} -vC "${BUILD_DIR}" install-runtimes
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" install-runtimes
step "Running the LLDB libc++ data formatter tests"
- ${NINJA} -vC "${BUILD_DIR}" lldb-api-test-deps
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" lldb-api-test-deps
${BUILD_DIR}/bin/llvm-lit -sv --param dotest-args='--category libc++' "${MONOREPO_ROOT}/lldb/test/API"
ccache -s
@@ -552,7 +560,7 @@ apple-configuration)
step "Running tests against Apple-configured libc++"
# TODO: It would be better to run the tests against the fake-installed version of libc++ instead
- xcrun --sdk macosx ninja -vC "${BUILD_DIR}/${arch}" check-cxx check-cxxabi check-cxx-abilist
+ xcrun --sdk macosx ninja ${VERBOSE_FLAG} -C "${BUILD_DIR}/${arch}" check-cxx check-cxxabi check-cxx-abilist
;;
apple-system|apple-system-hardened)
clean
@@ -595,13 +603,13 @@ apple-system|apple-system-hardened)
-DCMAKE_INSTALL_NAME_DIR="/usr/lib/system"
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}/cxx" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}/cxx" check-cxx
step "Running the libc++abi tests"
- ${NINJA} -vC "${BUILD_DIR}/cxx" check-cxxabi
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}/cxx" check-cxxabi
step "Running the libunwind tests"
- ${NINJA} -vC "${BUILD_DIR}/unwind" check-unwind
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}/unwind" check-unwind
;;
aarch64)
clean
@@ -659,13 +667,13 @@ clang-cl-dll)
# setting when cmake and the test driver does the right thing automatically.
generate-cmake-libcxx-win -DLIBCXX_TEST_PARAMS="enable_experimental=False"
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
;;
clang-cl-static)
clean
generate-cmake-libcxx-win -DLIBCXX_ENABLE_SHARED=OFF
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
;;
clang-cl-no-vcruntime)
clean
@@ -676,14 +684,14 @@ clang-cl-no-vcruntime)
generate-cmake-libcxx-win -DLIBCXX_TEST_PARAMS="enable_experimental=False" \
-DLIBCXX_TEST_CONFIG="llvm-libc++-shared-no-vcruntime-clangcl.cfg.in"
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
;;
clang-cl-debug)
clean
generate-cmake-libcxx-win -DLIBCXX_TEST_PARAMS="enable_experimental=False" \
-DCMAKE_BUILD_TYPE=Debug
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
;;
clang-cl-static-crt)
clean
@@ -692,7 +700,7 @@ clang-cl-static-crt)
generate-cmake-libcxx-win -DLIBCXX_ENABLE_SHARED=OFF \
-DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
;;
mingw-dll)
clean
@@ -738,7 +746,7 @@ mingw-incomplete-sysroot)
# Only test that building succeeds; there's not much extra value in running
# the tests here, as it would be equivalent to the mingw-dll config above.
step "Building the runtimes"
- ${NINJA} -vC "${BUILD_DIR}"
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}"
;;
aix)
clean
@@ -775,7 +783,7 @@ android-ndk-*)
-DLIBCXX_TEST_PARAMS="${PARAMS}" \
-DLIBCXXABI_TEST_PARAMS="${PARAMS}"
check-abi-list
- ${NINJA} -vC "${BUILD_DIR}" install-cxx install-cxxabi
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" install-cxx install-cxxabi
# Start the emulator and make sure we can connect to the adb server running
# inside of it.
@@ -788,9 +796,9 @@ android-ndk-*)
adb shell mkdir -p /data/local/tmp/adb_run
adb push "${BUILD_DIR}/lib/libc++_shared.so" /data/local/tmp/libc++/libc++_shared.so
step "Running the libc++ tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxx
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxx
step "Running the libc++abi tests"
- ${NINJA} -vC "${BUILD_DIR}" check-cxxabi
+ ${NINJA} ${VERBOSE_FLAG} -C "${BUILD_DIR}" check-cxxabi
;;
#################################################################
# Insert vendor-specific internal configurations below.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this buys us that much. Compare for example:
Without verbose (your patch): https://github.com/llvm/llvm-project/actions/runs/15734663895/job/44362585390?pr=144704
Before your patch: https://github.com/llvm/llvm-project/actions/runs/15674865342/job/44354780295?pr=144310
If you go under "Build and test", we already collapse into sections so it looks like this before expansion:
Then, when you expand e.g. Running libc++ tests
, in both cases you get this:
This is not super succinct but it shows how the test suite is set up, and in particular there is no noise like headers being installed or similar.
The difference is when you expand Building libc++ test dependencies
, where you get the super verbose output before your patch, and not much information after your patch. In particular, we lose the command-lines used to compile each translation unit in the library, which is likely of interest for someone expanding that section of the CI logs.
So, TLDR, since we already have collapsed-by-default sections in the CI logs, I'm not convinced this patch buys us much, but we do lose some useful information. WDYT?
If you download the raw logs for a job before and after, the new log file will be about ~4x smaller. (1M vs 275K). But I'm happy to drop this. |
The run-buildbot script with verbose output generates thousands of lines of output about copying headers around. This output makes it very difficult to find the actual CI failures among the noise.
The original idea behind verbose output was that it would help debug CI failures.
I believe the non-verbose CMake output is
(A) Sufficient to debug any issues you encounter, given that the cmake failure and log files are uploaded as artifacts.
(B) It would be much easier to debug CI failures if the logs were shorter.
Verbose output from the buildbot script can still be enabled using
-v
or--verbose