From d21a6a0fcf2bb197b7f724a7f28c1b741c914738 Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Wed, 13 Nov 2024 10:42:20 +0100 Subject: [PATCH 1/3] [ci] build with clingtest flag active if interpreter folder was touched Fixes https://github.com/root-project/root/issues/15230 [ci] move clingtest to a non-apple-silicon machine hahnjo: On Apple Silicon, we (still) cannot catch exceptions from interpreted code and therefore many (all?) NullDeref tests fail. --- .github/workflows/root-ci.yml | 45 +++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index 5fdba8a1c0099..a22716599d766 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -77,7 +77,36 @@ concurrency: cancel-in-progress: true jobs: + git_diff: + name: Detect changes in Cling + # https://www.meziantou.net/executing-github-actions-jobs-or-steps-only-when-specific-files-change.htm + runs-on: ubuntu-latest + # Declare output for next jobs + outputs: + interpreter_changed: ${{ steps.check_file_changed.outputs.interpreter_changed }} + steps: + - uses: actions/checkout@v4 + with: + # Checkout as many commits as needed for the diff + fetch-depth: 2 + - shell: pwsh + id: check_file_changed + run: | + # Diff HEAD with the latest commit of master + $diff = git diff --name-only HEAD^ HEAD + # (works with HEAD^ as the last commit fetched is virtual merge commit by GH which shows the full diff for the PR.) + + # Check if a file under interpreter/ has changed + $SourceDiff = $diff | Where-Object { $_ -match '^interpreter/' } + $HasDiff = $SourceDiff.Length -gt 0 + + # Set the output named "interpreter_changed" + Write-Host "::set-output name=interpreter_changed::$HasDiff" + #echo "interpreter_changed=$HasDiff" >> $GITHUB_OUTPUT + echo "interpreter_changed=$HasDiff" + build-macos: + needs: [ git_diff ] # For any event that is not a PR, the CI will always run. In PRs, the CI # can be skipped if the tag [skip-ci] or [skip ci] is written in the title. if: | @@ -95,18 +124,19 @@ jobs: # Common configs: {Release,Debug,RelWithDebInfo) # Build options: https://root.cern/install/build_from_source/#all-build-options include: - - platform: mac13 + - platform: mac13 arch: ARM64 overrides: ["LLVM_ENABLE_ASSERTIONS=On", "builtin_zlib=ON"] - platform: mac14 arch: X64 - overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"] + overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20", "clingtest=${{needs.git_diff.outputs.interpreter_changed == 'true' && 'ON' || 'OFF'}}"] - platform: mac15 arch: ARM64 overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"] - platform: mac-beta arch: ARM64 overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"] + # https://stackoverflow.com/questions/76292948/github-action-boolean-input-with-default-value runs-on: # Using '[self-hosted, ..., ...]' does not work for some reason :) - self-hosted @@ -115,7 +145,7 @@ jobs: - ${{ matrix.platform }} name: | - ${{ matrix.platform }} ${{ matrix.arch }} + ${{ matrix.platform }} ${{ matrix.arch }} ${{ (github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' && join( matrix.overrides, ', ' )) || '' }} steps: @@ -338,6 +368,7 @@ jobs: build-linux: + needs: [ git_diff ] # For any event that is not a PR, the CI will always run. In PRs, the CI # can be skipped if the tag [skip-ci] or [skip ci] is written in the title. if: | @@ -350,6 +381,11 @@ jobs: strategy: fail-fast: false matrix: + # https://github.com/orgs/community/discussions/26253 + # https://stackoverflow.com/questions/65384420/how-do-i-make-a-github-action-matrix-element-conditional + # https://stackoverflow.com/questions/68994484/how-to-skip-a-configuration-of-a-matrix-with-github-actions + # https://github.com/joaomcteixeira/python-project-skeleton/pull/31/files + # Specify image + (optional) build option overrides # # Available images: https://github.com/root-project/root-ci-images @@ -363,7 +399,8 @@ jobs: - image: alma8 overrides: ["LLVM_ENABLE_ASSERTIONS=On"] - image: alma9 - overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug"] + overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug", "clingtest=${{needs.git_diff.outputs.interpreter_changed == 'true' && 'ON' || 'OFF'}}"] + # https://stackoverflow.com/questions/76292948/github-action-boolean-input-with-default-value - image: ubuntu22 overrides: ["imt=Off", "LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug"] - image: ubuntu2404 From 91ea2f9f2e60d2a8365fa3965e7f56282f6b7c0e Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Sat, 30 Nov 2024 22:22:02 +0100 Subject: [PATCH 2/3] [ci] add also clingtest flag when building in windows, activate on both if touched --- .github/workflows/root-ci-config/build_root.py | 6 ++++-- .github/workflows/root-ci.yml | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/root-ci-config/build_root.py b/.github/workflows/root-ci-config/build_root.py index 89b90091d4f2e..878e69066715d 100755 --- a/.github/workflows/root-ci-config/build_root.py +++ b/.github/workflows/root-ci-config/build_root.py @@ -75,9 +75,9 @@ def main(): # file below overwrites values from above **load_config(f'{this_script_dir}/buildconfig/{args.platform}.txt') } - + if WINDOWS and pull_request: + options_dict["clingtest"] = 'on' if args.clingtest else 'off' options = build_utils.cmake_options_from_dict(options_dict) - if WINDOWS: options = "-Thost=x64 " + options @@ -195,6 +195,7 @@ def parse_args(): 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("--clingtest", default="false", help="Build with -Dclingtest=ON for Windows PR CI") parser.add_argument("--coverage", default="false", help="Create Coverage report in XML") parser.add_argument("--sha", default=None, help="sha that triggered the event") parser.add_argument("--base_ref", default=None, help="Ref to target branch") @@ -210,6 +211,7 @@ def parse_args(): # Set argument to True if matched args.incremental = args.incremental.lower() in ('yes', 'true', '1', 'on') + args.clingtest = args.clingtest.lower() in ('yes', 'true', '1', 'on') args.coverage = args.coverage.lower() in ('yes', 'true', '1', 'on') args.binaries = args.binaries.lower() in ('yes', 'true', '1', 'on') diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index a22716599d766..92198aaa3cb66 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -247,6 +247,7 @@ jobs: build-windows: + needs: [ git_diff ] # For any event that is not a PR, the CI will always run. In PRs, the CI # can be skipped if the tag [skip-ci] or [skip ci] is written in the title. if: | @@ -301,6 +302,7 @@ jobs: run: "C:\\setenv.bat ${{ matrix.target_arch }} && python .github/workflows/root-ci-config/build_root.py --buildtype ${{ matrix.config }} + --clingtest ${{ needs.git_diff.outputs.interpreter_changed }} --platform windows10 --incremental $INCREMENTAL --base_ref ${{ github.base_ref }} @@ -344,6 +346,7 @@ jobs: run: "C:\\setenv.bat ${{ matrix.target_arch }} && python .github/workflows/root-ci-config/build_root.py --buildtype ${{ matrix.config }} + --clingtest ${{ needs.git_diff.outputs.interpreter_changed }} --platform windows10 --incremental false --base_ref ${{ github.ref_name }} From 3db45e06d7b925a2d04b3ef6c015b083350aec43 Mon Sep 17 00:00:00 2001 From: ferdymercury Date: Sat, 30 Nov 2024 22:33:31 +0100 Subject: [PATCH 3/3] [to-remove] dummy change to trigger clingtest=ON --- interpreter/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interpreter/CMakeLists.txt b/interpreter/CMakeLists.txt index ca59036be90ae..350f8c3204513 100644 --- a/interpreter/CMakeLists.txt +++ b/interpreter/CMakeLists.txt @@ -1,4 +1,4 @@ -# Do not depend on CMake scripts from the rest of the ROOT build + # Do not depend on CMake scripts from the rest of the ROOT build set(CMAKE_MODULE_PATH "") #--- Check if we need to build llvm and clang ------------------------------------------------------