From 2eb294aa76cb197022472f726b0ce8098939d508 Mon Sep 17 00:00:00 2001 From: GernotMaier Date: Sun, 4 Oct 2026 18:37:30 +0200 Subject: [PATCH 1/3] Reject interpreter errors when loading the analysis ROOT library --- release_tests/utilities/parameters.C | 11 ++++--- tests/test_review_library_loading.py | 43 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 tests/test_review_library_loading.py diff --git a/release_tests/utilities/parameters.C b/release_tests/utilities/parameters.C index 5b2f35d..f554b92 100644 --- a/release_tests/utilities/parameters.C +++ b/release_tests/utilities/parameters.C @@ -12,6 +12,7 @@ #include #include #include +#include "TInterpreter.h" /* * Helper for ROOT macros: load libVAnaSum from environment. @@ -56,8 +57,9 @@ bool loadVAnaSumLibrary() { // Use gROOT->ProcessLine to properly load the library and its dictionaries string loadCmd = "R__LOAD_LIBRARY(" + iLibPath + ");"; - Longptr_t result = gROOT->ProcessLine( loadCmd.c_str() ); - if( result >= 0 ) + Int_t error = TInterpreter::kNoError; + Longptr_t result = gROOT->ProcessLine( loadCmd.c_str(), &error ); + if( error == TInterpreter::kNoError && result >= 0 ) { iLibraryLoaded = true; return true; @@ -65,8 +67,9 @@ bool loadVAnaSumLibrary() } // Try with just the library name - Longptr_t result = gROOT->ProcessLine( "R__LOAD_LIBRARY(libVAnaSum.so);" ); - if( result >= 0 ) + Int_t error = TInterpreter::kNoError; + Longptr_t result = gROOT->ProcessLine( "R__LOAD_LIBRARY(libVAnaSum.so);", &error ); + if( error == TInterpreter::kNoError && result >= 0 ) { iLibraryLoaded = true; return true; diff --git a/tests/test_review_library_loading.py b/tests/test_review_library_loading.py new file mode 100644 index 0000000..0197aa2 --- /dev/null +++ b/tests/test_review_library_loading.py @@ -0,0 +1,43 @@ +"""Check actual ROOT library loading failures, retries and successful caching.""" + +import json +import os +from pathlib import Path +import subprocess +import tempfile + + +helper = Path(__file__).resolve().parents[1] / "release_tests/utilities/parameters.C" +with tempfile.TemporaryDirectory() as temporary: + directory = Path(temporary) + library = directory / "valid-library.so" + subprocess.run(["g++", "-shared", "-fPIC", "-x", "c++", "-", "-o", str(library)], + input='extern "C" int review_library_marker() { return 42; }\n', + text=True, check=True) + invalid = directory / "invalid-library.so" + invalid.write_text("This is not a shared library.\n") + environment = os.environ.copy() + for variable in ("EVNDISPSYS", "EVNDISP", "VERITAS_VANASUM_LIBRARY"): + environment.pop(variable, None) + for candidate in (directory / "missing-library.so", invalid): + macro = directory / "test_library_load.C" + macro.write_text(f""" +#include {json.dumps(str(helper))} +#include +void test_library_load() +{{ + gSystem->SetDynamicPath({json.dumps(str(directory))}); + gSystem->Setenv("VERITAS_VANASUM_LIBRARY", {json.dumps(str(candidate))}); + if (loadVAnaSumLibrary()) {{ std::cerr << "Failed load reported success" << std::endl; gSystem->Exit(1); }} + if (loadVAnaSumLibrary()) {{ std::cerr << "Failed load reported success" << std::endl; gSystem->Exit(1); }} // failure must not populate the success cache + gSystem->Setenv("VERITAS_VANASUM_LIBRARY", {json.dumps(str(library))}); + if (!loadVAnaSumLibrary()) {{ std::cerr << "Successful load reported failure" << std::endl; gSystem->Exit(1); }} // a failed first attempt permits a real retry + gSystem->Setenv("VERITAS_VANASUM_LIBRARY", "missing-after-success.so"); + if (!loadVAnaSumLibrary()) {{ std::cerr << "Successful load reported failure" << std::endl; gSystem->Exit(1); }} // an actually loaded library stays cached +}} +""") + result = subprocess.run(["root", "-l", "-b", "-q", str(macro)], env=environment, + cwd=directory, text=True, capture_output=True) + assert result.returncode == 0, result.stdout + result.stderr + +print("PASS actual ROOT missing/invalid library, uncached failure, successful retry and cache") From 467a7b7e5394d9835f4a9d65151568af4e500b2c Mon Sep 17 00:00:00 2001 From: GernotMaier Date: Sun, 4 Oct 2026 19:45:15 +0200 Subject: [PATCH 2/3] no tests needed --- tests/test_review_library_loading.py | 43 ---------------------------- 1 file changed, 43 deletions(-) delete mode 100644 tests/test_review_library_loading.py diff --git a/tests/test_review_library_loading.py b/tests/test_review_library_loading.py deleted file mode 100644 index 0197aa2..0000000 --- a/tests/test_review_library_loading.py +++ /dev/null @@ -1,43 +0,0 @@ -"""Check actual ROOT library loading failures, retries and successful caching.""" - -import json -import os -from pathlib import Path -import subprocess -import tempfile - - -helper = Path(__file__).resolve().parents[1] / "release_tests/utilities/parameters.C" -with tempfile.TemporaryDirectory() as temporary: - directory = Path(temporary) - library = directory / "valid-library.so" - subprocess.run(["g++", "-shared", "-fPIC", "-x", "c++", "-", "-o", str(library)], - input='extern "C" int review_library_marker() { return 42; }\n', - text=True, check=True) - invalid = directory / "invalid-library.so" - invalid.write_text("This is not a shared library.\n") - environment = os.environ.copy() - for variable in ("EVNDISPSYS", "EVNDISP", "VERITAS_VANASUM_LIBRARY"): - environment.pop(variable, None) - for candidate in (directory / "missing-library.so", invalid): - macro = directory / "test_library_load.C" - macro.write_text(f""" -#include {json.dumps(str(helper))} -#include -void test_library_load() -{{ - gSystem->SetDynamicPath({json.dumps(str(directory))}); - gSystem->Setenv("VERITAS_VANASUM_LIBRARY", {json.dumps(str(candidate))}); - if (loadVAnaSumLibrary()) {{ std::cerr << "Failed load reported success" << std::endl; gSystem->Exit(1); }} - if (loadVAnaSumLibrary()) {{ std::cerr << "Failed load reported success" << std::endl; gSystem->Exit(1); }} // failure must not populate the success cache - gSystem->Setenv("VERITAS_VANASUM_LIBRARY", {json.dumps(str(library))}); - if (!loadVAnaSumLibrary()) {{ std::cerr << "Successful load reported failure" << std::endl; gSystem->Exit(1); }} // a failed first attempt permits a real retry - gSystem->Setenv("VERITAS_VANASUM_LIBRARY", "missing-after-success.so"); - if (!loadVAnaSumLibrary()) {{ std::cerr << "Successful load reported failure" << std::endl; gSystem->Exit(1); }} // an actually loaded library stays cached -}} -""") - result = subprocess.run(["root", "-l", "-b", "-q", str(macro)], env=environment, - cwd=directory, text=True, capture_output=True) - assert result.returncode == 0, result.stdout + result.stderr - -print("PASS actual ROOT missing/invalid library, uncached failure, successful retry and cache") From 63c3115e8d5f5672747ffd3fcf0655354835d064 Mon Sep 17 00:00:00 2001 From: Gernot Maier Date: Sun, 4 Oct 2026 19:52:41 +0200 Subject: [PATCH 3/3] Change error type to TInterpreter::EErrorCode Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- release_tests/utilities/parameters.C | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_tests/utilities/parameters.C b/release_tests/utilities/parameters.C index f554b92..a1adc04 100644 --- a/release_tests/utilities/parameters.C +++ b/release_tests/utilities/parameters.C @@ -57,7 +57,7 @@ bool loadVAnaSumLibrary() { // Use gROOT->ProcessLine to properly load the library and its dictionaries string loadCmd = "R__LOAD_LIBRARY(" + iLibPath + ");"; - Int_t error = TInterpreter::kNoError; + TInterpreter::EErrorCode error = TInterpreter::kNoError; Longptr_t result = gROOT->ProcessLine( loadCmd.c_str(), &error ); if( error == TInterpreter::kNoError && result >= 0 ) {