From bbff50a4de706998414c2404bc9997d6d2fd2226 Mon Sep 17 00:00:00 2001 From: Axel Naumann Date: Fri, 18 Aug 2023 16:01:17 +0200 Subject: [PATCH] [ci] Move logging to global, pass docker info in: Simplifies script and allows to capture (and log) essential config values. Some more drive-by code fixes. --- .../workflows/root-ci-config/build_root.py | 208 +++++++++--------- .../workflows/root-ci-config/build_utils.py | 89 +++++--- .github/workflows/root-ci.yml | 18 +- 3 files changed, 166 insertions(+), 149 deletions(-) diff --git a/.github/workflows/root-ci-config/build_root.py b/.github/workflows/root-ci-config/build_root.py index 9f7c6e2203a0d..f62dbd53146f0 100755 --- a/.github/workflows/root-ci-config/build_root.py +++ b/.github/workflows/root-ci-config/build_root.py @@ -23,17 +23,13 @@ import openstack from build_utils import ( - cmake_options_from_dict, die, - download_latest, github_log_group, - print_info, load_config, - print_shell_log, subprocess_with_log, - upload_file, - print_warning, + upload_file ) +import build_utils S3CONTAINER = 'ROOT-build-artifacts' # Used for uploads S3URL = 'https://s3.cern.ch/swift/v1/' + S3CONTAINER # Used for downloads @@ -51,40 +47,19 @@ def main(): # openstack.enable_logging(debug=True) - # accumulates commands executed so they can be displayed as a script on build failure - shell_log = "" - # used when uploading artifacts, calculate early since build times are inconsistent yyyy_mm_dd = datetime.datetime.today().strftime('%Y-%m-%d') - # it is difficult to use boolean flags from github actions, use strings to convey - # true/false for boolean arguments instead. - parser = argparse.ArgumentParser() - parser.add_argument("--platform", default="centos8", help="Platform to build on") - parser.add_argument("--incremental", default="false", help="Do incremental build") - parser.add_argument("--buildtype", default="Release", help="Release|Debug|RelWithDebInfo") - parser.add_argument("--coverage", default="false", help="Create Coverage report in XML") - parser.add_argument("--base_ref", default=None, help="Ref to target branch") - parser.add_argument("--head_ref", default=None, help="Ref to feature branch; it may contain a : part") - parser.add_argument("--architecture", default=None, help="Windows only, target arch") - parser.add_argument("--repository", default="https://github.com/root-project/root.git", - help="url to repository") - - args = parser.parse_args() - - # Set argument to True if matched - args.incremental = args.incremental.lower() in ('yes', 'true', '1', 'on') - args.coverage = args.coverage.lower() in ('yes', 'true', '1', 'on') + args = parse_args() - if not args.base_ref: - die(os.EX_USAGE, "base_ref not specified") + build_utils.log = build_utils.Tracer(args.image, args.dockeropts) pull_request = args.head_ref and args.head_ref != args.base_ref if not pull_request: - print_info("head_ref same as base_ref, assuming non-PR build") + build_utils.print_info("head_ref same as base_ref, assuming non-PR build") - shell_log = cleanup_previous_build(shell_log) + cleanup_previous_build() # Load CMake options from .github/workflows/root-ci-config/buildconfig/[platform].txt this_script_dir = os.path.dirname(os.path.abspath(__file__)) @@ -95,7 +70,7 @@ def main(): **load_config(f'{this_script_dir}/buildconfig/{args.platform}.txt') } - options = cmake_options_from_dict(options_dict) + options = build_utils.cmake_options_from_dict(options_dict) if WINDOWS: options = "-Thost=x64 " + options @@ -115,25 +90,25 @@ def main(): if args.incremental: try: - shell_log += download_artifacts(obj_prefix, shell_log) + download_artifacts(obj_prefix) except Exception as err: - print_warning(f'Failed to download: {err}') + build_utils.print_warning(f'Failed to download: {err}') args.incremental = False - shell_log = git_pull(args.repository, args.base_ref, shell_log) + git_pull(args.repository, args.base_ref) if pull_request: - shell_log = rebase(args.base_ref, args.head_ref, shell_log) + rebase(args.base_ref, args.head_ref) if not WINDOWS: - shell_log = show_node_state(shell_log, options) + show_node_state() + + build(options, args.buildtype) - shell_log = build(options, args.buildtype, shell_log) # Build artifacts should only be uploaded for full builds, and only for # "official" branches (master, v?-??-??-patches), i.e. not for pull_request # We also want to upload any successful build, even if it fails testing # later on. - if not pull_request and not args.incremental and not args.coverage: archive_and_upload(yyyy_mm_dd, obj_prefix) @@ -145,95 +120,122 @@ def main(): extra_ctest_flags += "--repeat until-pass:3 " extra_ctest_flags += "--build-config " + args.buildtype - num_failed_test, shell_log = run_ctest(shell_log, extra_ctest_flags) + ctest_returncode = run_ctest(extra_ctest_flags) if args.coverage: - shell_log = create_coverage_xml(shell_log) + create_coverage_xml() + + if ctest_returncode != 0: + die(msg=f"TEST FAILURE: ctest exited with code {ctest_returncode}") + + print_trace() + + +def parse_args(): + # it is difficult to use boolean flags from github actions, use strings to convey + # true/false for boolean arguments instead. + parser = argparse.ArgumentParser() + parser.add_argument("--platform", help="Platform to build on") + parser.add_argument("--image", default=None, help="Container image, if any") + parser.add_argument("--dockeropts", default=None, help="Extra docker options, if any") + parser.add_argument("--incremental", default="false", help="Do incremental build") + parser.add_argument("--buildtype", default="Release", help="Release|Debug|RelWithDebInfo") + parser.add_argument("--coverage", default="false", help="Create Coverage report in XML") + parser.add_argument("--base_ref", default=None, help="Ref to target branch") + parser.add_argument("--head_ref", default=None, help="Ref to feature branch; it may contain a : part") + parser.add_argument("--architecture", default=None, help="Windows only, target arch") + parser.add_argument("--repository", default="https://github.com/root-project/root.git", + help="url to repository") + + args = parser.parse_args() + + # Set argument to True if matched + args.incremental = args.incremental.lower() in ('yes', 'true', '1', 'on') + args.coverage = args.coverage.lower() in ('yes', 'true', '1', 'on') - if num_failed_test != 0: - die(msg=f"TEST FAILURE: {num_failed_test} tests failed", log=shell_log) + if not args.base_ref: + die(os.EX_USAGE, "base_ref not specified") - print_shell_log(shell_log, args.platform) + return args + + +@github_log_group("To reproduce") +def print_trace(): + build_utils.log.print() @github_log_group("Clean up from previous runs") -def cleanup_previous_build(shell_log): +def cleanup_previous_build(): # runners should never have root permissions but be on the safe side - if WORKDIR == "" or WORKDIR == "/": - die(1, "WORKDIR not set", "") + if WORKDIR in ("", "/"): + die(1, "WORKDIR not set") if WINDOWS: # windows os.environ['COMSPEC'] = 'powershell.exe' - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" $ErrorActionPreference = 'Stop' if (Test-Path {WORKDIR}) {{ Remove-Item -Recurse -Force -Path {WORKDIR} }} New-Item -Force -Type directory -Path {WORKDIR} - """, shell_log) + """) else: # mac/linux/POSIX - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" rm -rf {WORKDIR} mkdir -p {WORKDIR} - """, shell_log) + """) if result != 0: - die(result, "Failed to clean up previous artifacts", shell_log) - - return shell_log + die(result, "Failed to clean up previous artifacts") @github_log_group("Pull/clone branch") -def git_pull(repository: str, branch: str, shell_log: str): +def git_pull(repository: str, branch: str): returncode = 1 - for attempts in range(5): + for _ in range(5): if returncode == 0: break if os.path.exists(f"{WORKDIR}/src/.git"): - returncode, shell_log = subprocess_with_log(f""" + returncode = subprocess_with_log(f""" cd '{WORKDIR}/src' git checkout {branch} git fetch git reset --hard @{{u}} - """, shell_log) + """) else: - returncode, shell_log = subprocess_with_log(f""" + returncode = subprocess_with_log(f""" git clone --branch {branch} --single-branch {repository} "{WORKDIR}/src" - """, shell_log) + """) if returncode != 0: - die(returncode, f"Failed to pull {branch}", shell_log) - - return shell_log + die(returncode, f"Failed to pull {branch}") @github_log_group("Download previous build artifacts") -def download_artifacts(obj_prefix: str, shell_log: str): +def download_artifacts(obj_prefix: str): try: - tar_path, shell_log = download_latest(S3URL, obj_prefix, WORKDIR, shell_log) + tar_path = build_utils.download_latest(S3URL, obj_prefix, WORKDIR) print(f"\nExtracting archive {tar_path}") with tarfile.open(tar_path) as tar: tar.extractall(WORKDIR) - shell_log += f'\ntar -xf {tar_path}\n' + build_utils.log.add(f'\ntar -xf {tar_path}\n') except Exception as err: - print_warning("failed to download/extract:", err) + build_utils.print_warning("failed to download/extract:", err) shutil.rmtree(f'{WORKDIR}/src', ignore_errors=True) shutil.rmtree(f'{WORKDIR}/build', ignore_errors=True) raise err - return shell_log - @github_log_group("Node state") -def show_node_state(shell_log: str, options: str) -> str: - result, shell_log = subprocess_with_log(""" +def show_node_state() -> None: + result = subprocess_with_log(""" which cmake cmake --version which c++ || true @@ -243,21 +245,19 @@ def show_node_state(shell_log: str, options: str) -> str: sw_vers || true uptime || true df || true - """, shell_log) + """) if result != 0: - print_warning("Failed to extract node state") - - return shell_log + build_utils.print_warning("Failed to extract node state") -# Just return the number of test failures instead of `die()`-ing; report test@github_log_group("Run tests") -def run_ctest(shell_log: str, extra_ctest_flags: str) -> str: - num_failed_test, shell_log = subprocess_with_log(f""" +# Just return the exit code in case of test failures instead of `die()`-ing; report test@github_log_group("Run tests") +def run_ctest(extra_ctest_flags: str) -> int: + ctest_result = subprocess_with_log(f""" cd '{WORKDIR}/build' ctest --output-on-failure --parallel {os.cpu_count()} --output-junit TestResults.xml {extra_ctest_flags} - """, shell_log) + """) - return num_failed_test, shell_log + return ctest_result @github_log_group("Archive and upload") @@ -279,49 +279,47 @@ def archive_and_upload(archive_name, prefix): @github_log_group("Build") -def build(options, buildtype, shell_log): +def build(options, buildtype): generator_flags = "-- '-verbosity:minimal'" if WINDOWS else "" if not os.path.isdir(f'{WORKDIR}/build'): - result, shell_log = subprocess_with_log(f"mkdir {WORKDIR}/build", shell_log) + result = subprocess_with_log(f"mkdir {WORKDIR}/build") if result != 0: - die(result, "Failed to create build directory", shell_log) + die(result, "Failed to create build directory") if not os.path.exists(f'{WORKDIR}/build/CMakeCache.txt'): - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" cmake -S '{WORKDIR}/src' -B '{WORKDIR}/build' {options} -DCMAKE_BUILD_TYPE={buildtype} - """, shell_log) + """) if result != 0: - die(result, "Failed cmake generation step", shell_log) + die(result, "Failed cmake generation step") else: # Print CMake cached config - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" cmake -S '{WORKDIR}/src' -B '{WORKDIR}/build' -N -L - """, shell_log) + """) if result != 0: - die(result, "Failed cmake cache print step", shell_log) + die(result, "Failed cmake cache print step") - shell_log += f"\nBUILD OPTIONS: {options}" + print(f"\nBUILD OPTIONS: {options}") - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" cmake --build '{WORKDIR}/build' --config '{buildtype}' --parallel '{os.cpu_count()}' {generator_flags} - """, shell_log) + """) if result != 0: - die(result, "Failed to build", shell_log) - - return shell_log + die(result, "Failed to build") @github_log_group("Rebase") -def rebase(base_ref, head_ref, shell_log) -> str: +def rebase(base_ref, head_ref) -> None: head_ref_src, _, head_ref_dst = head_ref.partition(":") head_ref_dst = head_ref_dst or "__tmp" # rebase fails unless user.email and user.name is set - result, shell_log = subprocess_with_log(f""" + result = subprocess_with_log(f""" cd '{WORKDIR}/src' git config user.email "rootci@root.cern" @@ -330,25 +328,21 @@ def rebase(base_ref, head_ref, shell_log) -> str: git fetch origin {head_ref_src}:{head_ref_dst} git checkout {head_ref_dst} git rebase {base_ref} - """, shell_log) + """) if result != 0: - die(result, "Rebase failed", shell_log) - - return shell_log + die(result, "Rebase failed") @github_log_group("Create Test Coverage in XML") -def create_coverage_xml(shell_log: str) -> str: - result, shell_log = subprocess_with_log(f""" +def create_coverage_xml() -> None: + result = subprocess_with_log(f""" cd '{WORKDIR}/build' gcovr --cobertura-pretty --gcov-ignore-errors=no_working_dir_found --merge-mode-functions=merge-use-line-min -r ../src ../build -o cobertura-cov.xml - """, shell_log) + """) if result != 0: - die(result, "Failed to create test coverage", shell_log) - - return shell_log + die(result, "Failed to create test coverage") if __name__ == "__main__": diff --git a/.github/workflows/root-ci-config/build_utils.py b/.github/workflows/root-ci-config/build_utils.py index 40a3e4f847db2..87d989b43a577 100755 --- a/.github/workflows/root-ci-config/build_utils.py +++ b/.github/workflows/root-ci-config/build_utils.py @@ -7,11 +7,47 @@ import textwrap from functools import wraps from http import HTTPStatus -from typing import Callable, Dict, Tuple +from typing import Callable, Dict from openstack.connection import Connection from requests import get +class Tracer: + """ + Trace command invocations and print them to reproduce builds. + """ + + image = "" + docker_opts = [] + trace = "" + + def __init__(self, image: str, docker_opts: str): + self.image = image + if docker_opts: + self.docker_opts = docker_opts.split(' ') + if '--rm' in self.docker_opts: + self.docker_opts.remove('--rm') + + def add(self, command: str) -> None: + self.trace += '\n(\n' + textwrap.dedent(command.strip()) + '\n)' + + def print(self) -> None: + if self.trace != "": + print("""\ +###################################### +# To replicate build locally # +###################################### +""") + if self.image: + print(f"""\ +Grab the image: +$ docker run -it {self.image} {' '.join(self.docker_opts)} +Then: +""") + print(self.trace) + + +log = Tracer("", "") def github_log_group(title: str): """ decorator that places function's stdout/stderr output in a @@ -23,9 +59,9 @@ def wrapper(*args, **kwargs): try: result = func(*args, **kwargs) - except Exception as e: + except Exception as exc: print("::endgroup::") - raise e + raise exc print("::endgroup::") @@ -57,7 +93,7 @@ def print_error(*values, **kwargs): print_fancy("Fatal error: ", *values, sgr=31, **kwargs) -def subprocess_with_log(command: str, log="") -> Tuple[int, str]: +def subprocess_with_log(command: str) -> int: """Runs in shell and appends to log""" print_fancy(textwrap.dedent(command), sgr=1) @@ -71,33 +107,18 @@ def subprocess_with_log(command: str, log="") -> Tuple[int, str]: print("\033[0m", end='') - return (result.returncode, - log + '\n(\n' + textwrap.dedent(command.strip()) + '\n)') + log.add(command) + return result.returncode -def die(code: int = 1, msg: str = "", log: str = "") -> None: - print_error(f"({code}) {msg}") - - print_shell_log(log) - - sys.exit(code) -def print_shell_log(log: str, image: str) -> None: - if log != "": - shell_log = f"""\ -###################################### -# To replicate build locally # -###################################### - -For Linux, grab the image: -$ docker run --rm -it registry.cern.ch/root-ci/{image}:buildready -Then: +def die(code: int = 1, msg: str = "") -> None: + print_error(f"({code}) {msg}") -{log} -""" + log.print() - print(shell_log) + sys.exit(code) def load_config(filename) -> dict: @@ -178,20 +199,20 @@ def download_file(url: str, dest: str) -> None: if not os.path.exists(parent_dir): os.makedirs(parent_dir) - with open(dest, 'wb') as f, get(url, timeout=300) as r: - f.write(r.content) + with open(dest, 'wb') as fout, get(url, timeout=300) as req: + fout.write(req.content) -def download_latest(url: str, prefix: str, destination: str, shell_log: str) -> Tuple[str, str]: +def download_latest(url: str, prefix: str, destination: str) -> str: """Downloads latest build artifact starting with , and returns the file path to the downloaded file and shell_log.""" # https://docs.openstack.org/api-ref/object-store/#show-container-details-and-list-objects - with get(f"{url}/?prefix={prefix}&format=json", timeout=20) as r: - if r.status_code == HTTPStatus.NO_CONTENT or r.content == b'[]': + with get(f"{url}/?prefix={prefix}&format=json", timeout=20) as req: + if req.status_code == HTTPStatus.NO_CONTENT or req.content == b'[]': raise Exception(f"No object found with prefix: {prefix}") - result = json.loads(r.content) + result = json.loads(req.content) artifacts = [x['name'] for x in result if 'content_type' in x] latest = max(artifacts) @@ -199,8 +220,8 @@ def download_latest(url: str, prefix: str, destination: str, shell_log: str) -> download_file(f"{url}/{latest}", f"{destination}/artifacts.tar.gz") if os.name == 'nt': - shell_log += f"\nInvoke-WebRequest {url}/{latest} -OutFile {destination}\\artifacts.tar.gz" + log.add(f"\nInvoke-WebRequest {url}/{latest} -OutFile {destination}\\artifacts.tar.gz") else: - shell_log += f"\nwget -x -O {destination}/artifacts.tar.gz {url}/{latest}\n" + log.add(f"\nwget -x -O {destination}/artifacts.tar.gz {url}/{latest}\n") - return f"{destination}/artifacts.tar.gz", shell_log + return f"{destination}/artifacts.tar.gz" diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index b489f125b673e..495e37660d479 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -278,8 +278,8 @@ jobs: name: ${{ matrix.image }} ${{ matrix.config }} ${{ join( matrix.overrides, ', ' ) }} container: - image: registry.cern.ch/root-ci/${{ matrix.image }}:buildready - options: '--shm-size=1g -m 16g --security-opt label=disable --rm' + image: registry.cern.ch/root-ci/${{ matrix.image }}:buildready # ALSO UPDATE BELOW! + options: '--shm-size=1g -m 16g --security-opt label=disable --rm' # ALSO UPDATE BELOW! env: OS_APPLICATION_CREDENTIAL_ID: '7f5b64a265244623a3a933308569bdba' OS_APPLICATION_CREDENTIAL_SECRET: ${{ secrets.OS_APPLICATION_CREDENTIAL_SECRET }} @@ -308,26 +308,26 @@ jobs: if: ${{ matrix.overrides != NaN }} env: OVERRIDES: ${{ join( matrix.overrides, ' ') }} - FILE: .github/workflows/root-ci-config/buildconfig/${{ matrix.image }}.txt + CONFIGFILE: '.github/workflows/root-ci-config/buildconfig/${{ matrix.image }}.txt' shell: bash run: | set -x - echo '' >> "$FILE" + echo '' >> "$CONFIGFILE" for ENTRY in $OVERRIDES; do KEY=$( echo "$ENTRY" | cut -d '=' -f 1 ) # Add entry to file if not exists, otherwise replace - if grep -q "$KEY=" "$FILE"; then - sed -i "s/$KEY=.*\$/$ENTRY/" "$FILE" + if grep -q "$KEY=" "$CONFIGFILE"; then + sed -i "s/$KEY=.*\$/$ENTRY/" "$CONFIGFILE" else - echo "$ENTRY" >> "$FILE" + echo "$ENTRY" >> "$CONFIGFILE" fi done - cat "$FILE" || true + cat "$CONFIGFILE" || true - uses: root-project/gcc-problem-matcher-improved@main with: @@ -341,6 +341,8 @@ jobs: run: ".github/workflows/root-ci-config/build_root.py --buildtype ${{ matrix.config }} --platform ${{ matrix.image }} + --image registry.cern.ch/root-ci/${{ matrix.image }}:buildready + --dockeropts '--shm-size=1g -m 16g --security-opt label=disable --rm' --incremental $INCREMENTAL --base_ref ${{ github.base_ref }} --head_ref refs/pull/${{ github.event.pull_request.number }}/head:${{ github.event.pull_request.head.ref }}