From cacf4aaf762e44e7eee123f81949cc04ebb2f200 Mon Sep 17 00:00:00 2001
From: Pierre-Alain Loizeau <p.-a.loizeau@gsi.de>
Date: Mon, 15 Jan 2024 14:33:45 +0000
Subject: [PATCH] [CI] Overlaps exceptions and CI, fix for #2931 and sub-issues

---
 .gitlab-ci.yml                    |  7 ++++
 macro/geometry/CMakeLists.txt     | 50 ++++++++++++++++-----------
 macro/geometry/check_overlaps.C   | 35 +++++++++++++++----
 scripts/check-geo-hash-changes.sh | 56 +++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 25 deletions(-)
 create mode 100755 scripts/check-geo-hash-changes.sh

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9b25477915..b7c1bf0323 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -221,6 +221,12 @@ FileLicenceCheck:
     - if [[ -n $RAW_DATA_PATH ]]; then
     -   echo "export RAW_DATA_PATH=$RAW_DATA_PATH" >> Dart.cfg
     - fi
+    # Need to encapsulate the command to add GEO_HASH_CHANGE flag in Dart.cfg probably due to a long-standing
+    # feature/bug of gitlab shell runners w/ cmd result assignment when the return value is 0
+    # see https://gitlab.com/gitlab-org/gitlab-runner/-/issues/1779 and 3897
+    - if [[ -n $CHECK_GEO_HASH_CHANGE ]]; then
+    -   scripts/check-geo-hash-changes.sh $CI_MERGE_REQUEST_PROJECT_URL $CI_MERGE_REQUEST_TARGET_BRANCH_NAME
+    - fi
     - echo "export BUILDDIR=$PWD/build" >> Dart.cfg
     - echo "export SOURCEDIR=$PWD" >> Dart.cfg
     - if [[ -n $EXTRA_PATH ]]; then
@@ -257,6 +263,7 @@ FileLicenceCheck:
     BASE_PATH: "/opt/cbmsoft"
     SIMPATH: "$BASE_PATH/fairsoft_$FAIRSOFT_VERSION/installation"
     FAIRROOTPATH: "$BASE_PATH/fairroot_${FAIRROOT_VERSION}-fairsoft_${FAIRSOFT_VERSION}"
+    CHECK_GEO_HASH_CHANGE: "1"
 
 .apptainer_tag: &apptainer_tag
   stage: build
diff --git a/macro/geometry/CMakeLists.txt b/macro/geometry/CMakeLists.txt
index 5a8c146dfb..de20dac5c9 100644
--- a/macro/geometry/CMakeLists.txt
+++ b/macro/geometry/CMakeLists.txt
@@ -30,25 +30,37 @@ file(GLOB setup_files
      ${CBMROOT_SOURCE_DIR}/geometry/setup/setup_*.C
     )
 
-foreach(setup IN LISTS setup_files)
-    string(REPLACE ${CBMROOT_SOURCE_DIR}/geometry/setup/setup_ "" setup ${setup})
-    string(REPLACE .C "" setup ${setup})
-    # Message("Test: found setup tag ${setup}")
+SET( CHECK_SETUPS FALSE )
+If(${CBM_TEST_MODEL} MATCHES Nightly OR ${CBM_TEST_MODEL} MATCHES Weekly)
+  SET( CHECK_SETUPS TRUE )
+  Message( STATUS "Enabling tests on geometry setups consistency (materials and overlaps) for Nightly/Weekly build")
+elseIf(DEFINED ENV{GEO_HASH_CHANGE})
+  set(GEO_HASH_CHANGE $ENV{GEO_HASH_CHANGE})
+  If(GEO_HASH_CHANGE)
+    SET( CHECK_SETUPS TRUE )
+    Message( STATUS "Change in geo repo hash => Enabling tests on geometry setups consistency (materials and overlaps)")
+  endIf()
+endIf()
 
-    # --- Test setup materials
-    # --- Check that materials in a loaded geometry (1 evt tra run) match corresponding single det geo files,
-    #     using check_materials.C
-    set(testname geo_check_mats_${setup})
-    add_test(${testname} ${MACRO_DIR}/examine_materials.sh \"${setup}\")
-    set_tests_properties(${testname} PROPERTIES
-        TIMEOUT ${timeOutTime}
-        FAIL_REGULAR_EXPRESSION "FAILED;issues with materials were detected"
-        PASS_REGULAR_EXPRESSION "SUCCESS;Excluding air and dummy materials, no material errors were detected"
-        FIXTURES_REQUIRED geo_cleanup
-        FIXTURES_SETUP fixt_check_mats_${setup}
-    )
+If(CHECK_SETUPS)
+    ForEach(setup IN LISTS setup_files)
+        string(REPLACE ${CBMROOT_SOURCE_DIR}/geometry/setup/setup_ "" setup ${setup})
+        string(REPLACE .C "" setup ${setup})
+        # Message("Test: found setup tag ${setup}")
+
+        # --- Test setup materials
+        # --- Check that materials in a loaded geometry (1 evt tra run) match corresponding single det geo files,
+        #     using check_materials.C
+        set(testname geo_check_mats_${setup})
+        add_test(${testname} ${MACRO_DIR}/examine_materials.sh \"${setup}\")
+        set_tests_properties(${testname} PROPERTIES
+            TIMEOUT ${timeOutTime}
+            FAIL_REGULAR_EXPRESSION "FAILED;issues with materials were detected"
+            PASS_REGULAR_EXPRESSION "SUCCESS;Excluding air and dummy materials, no material errors were detected"
+            FIXTURES_REQUIRED geo_cleanup
+            FIXTURES_SETUP fixt_check_mats_${setup}
+        )
 
-    if(${CBM_TEST_MODEL} MATCHES Nightly OR ${CBM_TEST_MODEL} MATCHES Weekly)
         set(testname geo_setup_overlaps_${setup})
         add_test(${testname} ${MACRO_DIR}/check_overlaps.sh \"data/examine_${setup}\")
         set_tests_properties(${testname} PROPERTIES
@@ -58,8 +70,8 @@ foreach(setup IN LISTS setup_files)
             FIXTURES_REQUIRED fixt_check_mats_${setup}
             FIXTURES_SETUP fixt_check_overlaps_${setup}
         )
-    endIf()
-endforeach(setup IN LISTS setup_files)
+    endForEach(setup IN LISTS setup_files)
+endIf()
 # ============================================================================
 
 # Example of usage to check overlaps in aligned setup
diff --git a/macro/geometry/check_overlaps.C b/macro/geometry/check_overlaps.C
index f822d74d8f..704a1af65c 100644
--- a/macro/geometry/check_overlaps.C
+++ b/macro/geometry/check_overlaps.C
@@ -148,6 +148,8 @@ namespace geo_ci
 }  // namespace geo_ci
 /**********************************************************************************************************************/
 
+/// Disable clang formatting to keep easier exceptions lists reading (and better comparison to error output)
+/* clang-format off */
 std::vector<std::string> mcbm_pipevac_bmon_overlaps = {
   "cave/pipe_v19b_0/vacu20_1 overlapping cave/tof_v19d_mcbm_0/tof_v19d_mcbmStand_1/module_5_0",
   "cave/pipe_v19b_0/vacu20_1 overlapping cave/tof_v19e_mcbm_0/tof_v19e_mcbmStand_1/module_5_0",
@@ -156,7 +158,8 @@ std::vector<std::string> mcbm_pipevac_bmon_overlaps = {
   "cave/pipe_v19f_0/vacu20_1 overlapping cave/tof_v21d_mcbm_0/tof_v21d_mcbmStand_1/module_5_0",
   "cave/pipe_v19f_0/vacu20_1 overlapping cave/tof_v21f_mcbm_0/tof_v21f_mcbmStand_1/module_5_0",
   "cave/pipe_v19f_0/vacu20_1 overlapping cave/tof_v22a_mcbm_0/tof_v22a_mcbmStand_1/module_5_0",
-  "cave/pipe_v19f_0/vacu20_1 overlapping cave/tof_v21j_mcbm_0/tof_v21j_mcbmStand_1/module_5_0"};
+  "cave/pipe_v19f_0/vacu20_1 overlapping cave/tof_v21j_mcbm_0/tof_v21j_mcbmStand_1/module_5_0"
+};
 
 std::vector<std::pair<std::string, std::string>> mcbm_dets_overlaps = {
   {"gas_box extruded by: gas_box/counter_0", "between gas_box and gas_box/counter_0 (internal to mTOF v19e)"},
@@ -164,7 +167,8 @@ std::vector<std::pair<std::string, std::string>> mcbm_dets_overlaps = {
   {"HalfLadder12d_Module04 extruded by: HalfLadder12d_Module04/Sensor04_1",
    ": module 4 sticking out of ladder 12 (internal to mSTS v22f)"},
   {"tof_v21j_mcbmStand/module_2_0 overlapping tof_v21j_mcbmStand/module_9_0",
-   "between modules 2 and 9 (internal to mTOF v21j)"}};
+   "between modules 2 and 9 (internal to mTOF v21j)"}
+};
 
 std::vector<std::pair<std::string, std::string>> mcbm_dets_overlaps_sampling = {
   {"tof_v19d_mcbmStand: node module_8_0 overlapping module_8_0", "between module 8 and itself (internal to mTOF v19d)"},
@@ -177,20 +181,39 @@ std::vector<std::pair<std::string, std::string>> mcbm_dets_overlaps_sampling = {
   {"gas_box: node counter_5 overlapping counter_6", "between counters 5 and 6 (internal to mTOF v19e)"},
   {"gas_box: node counter_6 overlapping counter_7", "between counters 6 and 7 (internal to mTOF v19e)"},
   {"counter: node Gap_3 overlapping tof_glass_4", "between gap and glass (internal to mTOF v21f)"},
-  {"tof_v21j_mcbmStand: node module_2_0 overlapping module_9_0", "between modules 2 and 9 (internal to mTOF v21j)"}};
+  {"tof_v21j_mcbmStand: node module_2_0 overlapping module_9_0", "between modules 2 and 9 (internal to mTOF v21j)"}
+};
 
 std::vector<std::string> cbm_pipevac_bmon_overlaps = {
   "cave/pipe_v20a_1m_0/pipevac7_0 overlapping cave/tof_v20b_1m_0/module_5_0",
-  "cave/pipe_v20a_1m_0/pipe7_0 overlapping cave/tof_v20b_1m_0/module_5_0"};
+  "cave/pipe_v20a_1m_0/pipe7_0 overlapping cave/tof_v20b_1m_0/module_5_0"
+};
 
 std::vector<std::pair<std::string, std::string>> cbm_dets_overlaps = {
   {"cave/magnet_container_0 overlapping cave/rich_v17a_3e_0/rich_container_288", "between rich v17a_3e and magnet"},
-  {"cave/magnet_container_0 overlapping cave/rich_v17a_1e_0/rich_container_290", "between rich v17a_1e and magnet"}};
+  {"cave/magnet_container_0 overlapping cave/rich_v17a_1e_0/rich_container_290", "between rich v17a_1e and magnet"},
+  {"cave/magnet_container_0 overlapping cave/rich_v21a_0/rich_container_1", "between rich v21a & magnet v21a, DEC21"},
+  {"cave/pipe_v21i_0/conical_beam_pipe_1/conus_volume_1 overlapping cave/rich_v23a_0/rich_v23a_1/rich_cont_1",
+   "between rich v23_a and cone, known beampipe fix pending"},
+  {"cave/pipe_v21i_0/vacuum_conical_beam_pipe_1/vacuum_conus_volume_1 overlapping cave/rich_v23a_0/rich_v23a_1/rich_cont_1",
+   "between rich v23_a and cone, known beamtime fix pending"}
+};
 
 std::vector<std::pair<std::string, std::string>> cbm_dets_overlaps_sampling = {
   {"cave: node magnet_container_0 overlapping rich_v17a_3e_0/rich_container_288", "between rich v17a_3e and magnet"},
   {"cave: node magnet_container_0 overlapping rich_v17a_1e_0/rich_container_290", "between rich v17a_1e and magnet"},
-  {"belt_assembly_1: node belt_part3_156 overlapping belt_part4_157", "between parts of belt in ???? (s300e)"}};
+  {"cave: node magnet_container_0 overlapping rich_v21a_0/rich_container_1", "between rich v21a & magnet v21a, DEC21"},
+  {"belt_assembly_1: node belt_part3_156 overlapping belt_part4_157", "between parts of belt in RICH (s300e)"},
+  {"belt: node belt2_1 overlapping belt4_1", "between parts of belt in RICH v23"},
+  {"shielding_box: node shielding_box_1 overlapping shieling_wing_1", "between shielding box parts (RICH internal)"},
+  {"MVDscripted: node station_S0_1/heatsink_S0_1/heatsinkpart_2_2 overlapping top_bottom_plate_2",
+   "between heatsink and bottom plate (internal to MVD)"},
+  {"conical_beam_pipe: node rich_much_downstream_flange_1 overlapping conus_volume_1",  // Fix in next beampipe geo
+   "between R/M flange and cone (beampipe internal), known fix pending"}
+};
+
+/// Re-enable clang formatting after exceptions lists filling
+/* clang-format on */
 
 bool expected_mcbm(TGeoOverlap* ov)
 {
diff --git a/scripts/check-geo-hash-changes.sh b/scripts/check-geo-hash-changes.sh
new file mode 100755
index 0000000000..c9b104eafd
--- /dev/null
+++ b/scripts/check-geo-hash-changes.sh
@@ -0,0 +1,56 @@
+# Copyright (C) 2023 Facility for Antiproton and Ion Research in Europe, Darmstadt
+# SPDX-License-Identifier: GPL-3.0-only
+# First commited by Pierre-Alain Loizeau
+
+# This script will check if the Geometry repository installation script (externals) hold any modification relative to
+# the "upstream" repository.
+# The result is stored as an export to the variable GEO_HASH_CHANGE in the Dart.cfg file
+#
+# upstream is used here in the CBM workflow and CBM CI meaning
+# => designating the target branch of an MR or the official master branch
+# => Not your own fork in most use cases and the script is mostly meant to be auto-called
+# => Be careful if calling by hand connect_upstream_repo as it updates the remote called upstream if different!!!
+#    This is typically visible by a long list of "[new branch]" when this new remote if fetched!
+
+if [[ $# -ne 2 ]]; then
+  echo "Missing argument! please call this script with both the upstream URL and the upstream branch:"
+  echo "./scripts/check-geo-hash-changes.sh <url> <branch>"
+  echo ""
+  echo "upstream is used here in the CBM workflow and CBM CI meaning"
+  echo "=> designating the target branch of an MR or the official master branch"
+  echo "=> Not your own fork in most use cases and the script is mostly meant to be auto-called"
+  echo "=> Be careful if calling by hand connect_upstream_repo: it updates the remote called upstream if different!!!"
+  echo ""
+  echo "Examples: "
+  echo "./scripts/check-geo-hash-changes.sh https://git.cbm.gsi.de/computing/cbmroot.git master # https without login"
+  echo "./scripts/check-geo-hash-changes.sh git@git.cbm.gsi.de:computing/cbmroot.git master     # ssh with key login"
+  echo "./scripts/check-geo-hash-changes.sh $CI_MERGE_REQUEST_PROJECT_URL $CI_MERGE_REQUEST_TARGET_BRANCH_NAME # MR CI"
+  exit -1
+fi
+
+# Copied from connect_upstream_repo.sh to avoid replacing an existing upstream remote!
+upstream_there=$(git remote -v | grep upstream)
+if [ $? -eq 0 ]; then
+  upstream_there=$(git remote -v | grep upstream | grep $1)
+  if [ $? -ne 0 ]; then
+    echo "Remote link upstream already exist and points to a different repo, stopping here!! (not changing local conf.)"
+    echo "Existing: "
+    git remote -v | grep upstream
+    echo "Parameter: "
+    echo "upstream	$1"
+    exit -1
+  fi
+fi
+
+scripts/connect_upstream_repo.sh $1
+git fetch upstream
+
+BASE_COMMIT=upstream/$2
+
+GEO_HASH_CHANGE=`git diff --name-only $BASE_COMMIT | grep external/InstallGeometry.cmake | wc -l`
+echo "export GEO_HASH_CHANGE=$GEO_HASH_CHANGE" >> Dart.cfg
+if [[ $GEO_HASH_CHANGE -eq 0 ]]; then
+  echo "No changes found to Geometry repository install script"
+else
+  echo "Changes found to Geometry repository install script, assuming hash was bumped or target repo/branch changed"
+fi
-- 
GitLab