Add GitLab CI foldable sections to build script#1049
Add GitLab CI foldable sections to build script#1049adrienbernede wants to merge 15 commits intodevelopfrom
Conversation
Implement collapsible sections in build_and_test.sh to improve CI log readability. Adds helper functions section_start() and section_end() to manage GitLab CI section markers, and wraps all major build phases (dependencies, cmake config, build, test, install) in foldable sections.
Enhance the collapsible section helpers with comprehensive timing information: - Track script start time and calculate total elapsed time - Add format_time() function to display durations in HH:MM:SS format - Store section start times for elapsed time calculation per section - Display current time, total elapsed, and section elapsed in section headers/footers Each section now shows: - section_start: current time | total elapsed | section title - section_end: current time | total elapsed | section duration This provides clear visibility into CI job performance and helps identify bottlenecks.
Implement stack-based nested GitLab CI sections with automatic ID management: - Add section_id_stack array to track section hierarchy - Auto-generate unique section IDs using counters - Add visual indentation based on nesting level - section_end now pops from stack (no ID parameter needed) Replace all timed_message calls with nested collapsible sections: - Module loading - Dependencies (with nested sub-sections for Spack setup, registry, build, push) - Cleaning working directory - CMake configuration - Build (with nested verbose rebuild and install sections) - Tests (with nested XML processing and install test sections) This provides hierarchical organization of CI logs with consistent timing information at all levels, making it easier to drill down into specific phases and identify performance bottlenecks.
adrienbernede
force-pushed
the
woptim/ci-logs-and-errors
branch
from
a4ae5a5 to
06db578
Compare
|
@davidbeckingsale This has been verified and is ready for review. |
There was a problem hiding this comment.
Pull request overview
This pull request adds GitLab CI collapsible section support to the build and test script to improve log readability in CI environments. The changes replace the simple timed_message() function with more sophisticated section_start() and section_end() helpers that support nesting and provide detailed timing information.
Changes:
- Introduced
section_start()andsection_end()helper functions with nesting support and timing calculations - Wrapped all major build phases (dependencies, cmake config, build, install, test) in collapsible GitLab CI sections
- Updated echo statements to use a consistent
[Information]/[Error]/[Warning]prefix format
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
scripts/gitlab/build_and_test.sh
Outdated
| @@ -264,9 +355,12 @@ then | |||
| then | |||
| echo "[Error]: Failure(s) while running CTest" && exit 1 | |||
There was a problem hiding this comment.
The error handling on lines 347 and 356 exits the script while inside the "tests" section (started on line 333, closed on line 358). These exits will leave the section unclosed, potentially causing issues with GitLab CI log parsing. Consider calling section_end before exiting, or setting up a trap handler to ensure sections are properly closed on script exit.
| section_start "spack_setup" "Spack setup and environment" "collapsed" | ||
| ${uberenv_cmd} --setup-and-env-only --spec="${spec}" ${prefix_opt} | ||
| section_end | ||
|
|
||
| if [[ -n ${ci_registry_token} ]] | ||
| then | ||
| timed_message "GitLab registry as Spack Buildcache" | ||
| section_start "registry_setup" "GitLab registry as Spack Buildcache" "collapsed" | ||
| ${spack_cmd} -D ${spack_env_path} mirror add --unsigned --oci-username-variable ci_registry_user --oci-password-variable ci_registry_token gitlab_ci oci://${ci_registry_image} | ||
| section_end | ||
| fi | ||
|
|
||
| timed_message "Spack build of dependencies" | ||
| section_start "spack_build" "Spack build of dependencies" "collapsed" | ||
| ${uberenv_cmd} --skip-setup-and-env --spec="${spec}" ${prefix_opt} | ||
| section_end | ||
|
|
||
| if [[ -n ${ci_registry_token} && ${push_to_registry} == true ]] | ||
| then | ||
| timed_message "Push dependencies to buildcache" | ||
| section_start "buildcache_push" "Push dependencies to buildcache" "collapsed" | ||
| ${spack_cmd} -D ${spack_env_path} buildcache push --only dependencies gitlab_ci |
There was a problem hiding this comment.
The uberenv and spack commands on lines 194, 200, 205, and 211 are executed within nested sections (inside the "dependencies" parent section) without explicit error handling. If any of these commands fail, the script will exit due to set -o errexit on line 16, potentially leaving both the nested section and the parent "dependencies" section unclosed. This could cause issues with GitLab CI log parsing. Consider setting up a trap handler to ensure all open sections are properly closed on script exit, or add explicit error handling to close sections before exiting.
| tree Testing | ||
| xsltproc -o junit.xml ${project_dir}/scripts/radiuss-spack-configs/utilities/ctest-to-junit.xsl Testing/*/Test.xml |
There was a problem hiding this comment.
The tree and xsltproc commands on lines 350-351 are executed without error handling. If either command fails, the script will exit due to set -o errexit without calling section_end for the "tests" section, leaving it unclosed. Consider adding explicit error handling or using || true if these commands are non-critical, to ensure the section is properly closed on failure.
| local current_time=$(date -d @${timestamp} --rfc-3339=seconds) | ||
| local total_elapsed=$((timestamp - script_start_time)) | ||
| local total_elapsed_formatted=$(date -d @${total_elapsed} -u +%H:%M:%S) | ||
|
|
||
| # Store section start time for later calculation | ||
| section_start_times[${section_id}]=${timestamp} | ||
|
|
||
| # Push section ID onto stack | ||
| section_id_stack+=("${section_id}") | ||
|
|
||
| echo "${section_indent}~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~" | ||
| echo "${section_indent}~ TIME | TOTAL | SECTION " | ||
| echo "${section_indent}~ ${current_time} | ${total_elapsed_formatted} | ${section_title}" | ||
| echo -e "\e[0Ksection_start:${timestamp}:${section_id}[collapsed=${collapsed}]\r\e[0K${section_indent}~ ${section_title}" | ||
|
|
||
| # Increase indentation for nested sections | ||
| section_indent="${section_indent} " | ||
| } | ||
|
|
||
| section_end () | ||
| { | ||
| # Pop section ID from stack | ||
| if [[ ${#section_id_stack[@]} -eq 0 ]]; then | ||
| echo "[Warning]: section_end called with empty stack" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Decrease indentation before displaying | ||
| section_indent="${section_indent% }" | ||
|
|
||
| local stack_index=$((${#section_id_stack[@]} - 1)) | ||
| local section_id="${section_id_stack[$stack_index]}" | ||
| unset section_id_stack[$stack_index] | ||
|
|
||
| local timestamp=$(date +%s) | ||
| local current_time=$(date -d @${timestamp} --rfc-3339=seconds) | ||
| local total_elapsed=$((timestamp - script_start_time)) | ||
| local total_elapsed_formatted=$(date -d @${total_elapsed} -u +%H:%M:%S) | ||
|
|
||
| # Calculate section elapsed time | ||
| local section_start=${section_start_times[${section_id}]:-${timestamp}} | ||
| local section_elapsed=$((timestamp - section_start)) | ||
| local section_elapsed_formatted=$(date -d @${section_elapsed} -u +%H:%M:%S) |
There was a problem hiding this comment.
The date command with -d @${timestamp} syntax (lines 69, 71, 104, 106, 111) is a GNU date extension and may not work on non-GNU systems (e.g., macOS, BSD). Since this is a GitLab CI script likely running on Linux, this is probably acceptable, but consider documenting this GNU date dependency or adding a compatibility check if the script might run on other systems.
scripts/gitlab/build_and_test.sh
Outdated
| @@ -283,14 +377,15 @@ then | |||
| if ! make; then | |||
| echo "[Error]: Running make for using-with-cmake test" && exit 1 | |||
There was a problem hiding this comment.
The error handling on lines 368, 374, and 378 exits the script while inside the "install_test" section (started on line 360). These exits will leave the section unclosed, potentially causing issues with GitLab CI log parsing. Consider calling section_end before exiting, or setting up a trap handler to ensure sections are properly closed on script exit.
|
|
||
| local stack_index=$((${#section_id_stack[@]} - 1)) | ||
| local section_id="${section_id_stack[$stack_index]}" | ||
| unset section_id_stack[$stack_index] |
There was a problem hiding this comment.
Using unset section_id_stack[$stack_index] on line 101 creates a "hole" in the array rather than properly removing the last element. While this works for the immediate use case, it could cause issues if the array is iterated over later. Consider using section_id_stack=("${section_id_stack[@]:0:$stack_index}") or unset 'section_id_stack[-1]' (bash 4.3+) to properly remove the last element without leaving a hole in the array indices.
| unset section_id_stack[$stack_index] | |
| section_id_stack=("${section_id_stack[@]:0:$stack_index}") |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Implement collapsible sections in build_and_test.sh to improve CI log
readability. Adds helper functions section_start() and section_end()
to manage GitLab CI section markers, and wraps all major build phases
(dependencies, cmake config, build, test, install) in foldable
sections.