From 5fb27cdb33d60a0ab374d6b22af4d7400bdc54dd Mon Sep 17 00:00:00 2001 From: JainAyush-01 Date: Thu, 22 Oct 2026 11:16:21 +0000 Subject: [PATCH 1/9] [core] Fix plugin lookup when Root.PluginPath has multiple directories When Root.PluginPath (or ROOT_PLUGIN_PATH) contains multiple colon-separated directories, TPluginManager::LoadHandlersFromPluginDirs loops over each directory to load handler macros for the requested base class. In commit e60640c52a (#18859), ConcatFileName was replaced by PrependPathName. Unlike ConcatFileName which returned a new heap-allocated string, PrependPathName modifies its second parameter in-place. Because `sbase` was passed directly: const char *p = gSystem->PrependPathName(d, sbase); the variable `sbase` was mutated in-place across iterations (e.g., from "TArchiveFile" to "/dir1/TArchiveFile", then to "/dir2//dir1/TArchiveFile"). Consequently, subsequent search directories received a mangled path and ROOT could not find its own built-in plugins whenever a custom plugin directory was prepended. Fix this by copying `sbase` to a temporary `TString temp = sbase;` on each iteration, matching the pattern used in the rest of TPluginManager.cxx. Fixes #23536. Added regression test in core/base/test/TPluginManagerTests.cxx. --- core/base/src/TPluginManager.cxx | 3 +- core/base/test/CMakeLists.txt | 2 + core/base/test/TPluginManagerTests.cxx | 73 ++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 1 deletion(-) create mode 100644 core/base/test/TPluginManagerTests.cxx diff --git a/core/base/src/TPluginManager.cxx b/core/base/src/TPluginManager.cxx index 9a20d9207be80..7e4e71ae2cc91 100644 --- a/core/base/src/TPluginManager.cxx +++ b/core/base/src/TPluginManager.cxx @@ -532,7 +532,8 @@ void TPluginManager::LoadHandlersFromPluginDirs(const char *base) } if (!skip) { if (sbase != "") { - const char *p = gSystem->PrependPathName(d, sbase); + TString temp = sbase; + const char *p = gSystem->PrependPathName(d, temp); LoadHandlerMacros(p); } else { void *dirp = gSystem->OpenDirectory(d); diff --git a/core/base/test/CMakeLists.txt b/core/base/test/CMakeLists.txt index c60aa85eb9992..9528a5c1af67a 100644 --- a/core/base/test/CMakeLists.txt +++ b/core/base/test/CMakeLists.txt @@ -36,3 +36,5 @@ configure_file(Foo.C Foo.C COPYONLY) ROOT_ADD_GTEST(IncludePathTest IncludePathTest.cxx LIBRARIES Core) ROOT_ADD_GTEST(UUIDTest UUIDTest.cxx LIBRARIES Core) + +ROOT_ADD_GTEST(CorePluginManagerTests TPluginManagerTests.cxx LIBRARIES Core) diff --git a/core/base/test/TPluginManagerTests.cxx b/core/base/test/TPluginManagerTests.cxx new file mode 100644 index 0000000000000..848db82804630 --- /dev/null +++ b/core/base/test/TPluginManagerTests.cxx @@ -0,0 +1,73 @@ +#include "gtest/gtest.h" + +#include "TPluginManager.h" +#include "TROOT.h" +#include "TEnv.h" +#include "TSystem.h" +#include "TUUID.h" + +#include + +// Regression test for https://github.com/root-project/root/issues/23536: +// TPluginManager::FindHandler must correctly resolve plugins across all directories +// when Root.PluginPath contains multiple colon-separated directory paths. +TEST(TPluginManager, MultiDirectoryPluginPath) +{ + // Save original Root.PluginPath + const char *origPluginPath = gEnv->GetValue("Root.PluginPath", (char *)nullptr); + TString savedPath = origPluginPath ? origPluginPath : ""; + + // Construct default plugin path ($ROOTSYS/etc/plugins) + TString defaultPath = "plugins"; + gSystem->PrependPathName(TROOT::GetEtcDir(), defaultPath); + + // Create a temporary custom plugin directory with a custom macro + TUUID uuid; + TString tempDir = gSystem->TempDirectory(); + TString customDir = TString::Format("test_plugins_%s", uuid.AsString()); + gSystem->PrependPathName(tempDir.Data(), customDir); + + TString baseSubdir = "TCustomPluginBase"; + gSystem->PrependPathName(customDir.Data(), baseSubdir); + ASSERT_EQ(0, gSystem->mkdir(baseSubdir.Data(), /*recursive=*/true)); + + TString macroFile = "P010_TCustomPlugin.C"; + gSystem->PrependPathName(baseSubdir.Data(), macroFile); + + { + std::ofstream f(macroFile.Data()); + ASSERT_TRUE(f.is_open()); + f << "void P010_TCustomPlugin()\n" + << "{\n" + << " gPluginMgr->AddHandler(\"TCustomPluginBase\", \".*\", \"TCustomClass\",\n" + << " \"CustomLib\", \"TCustomClass()\");\n" + << "}\n"; + } + + // Set multi-directory Root.PluginPath: custom directory prepended before default directory + TString multiPath = customDir + ":" + defaultPath; + gEnv->SetValue("Root.PluginPath", multiPath.Data()); + + // 1. Verify that the custom plugin from the first directory is resolved + TPluginHandler *hCustom = gPluginMgr->FindHandler("TCustomPluginBase", "custom://test"); + ASSERT_NE(hCustom, nullptr); + EXPECT_STREQ(hCustom->GetClass(), "TCustomClass"); + + // 2. Verify that built-in plugins from subsequent directories are resolved properly (not dropped or corrupted) + TPluginHandler *hArchive = gPluginMgr->FindHandler("TArchiveFile", "test.zip"); + ASSERT_NE(hArchive, nullptr); + EXPECT_STREQ(hArchive->GetClass(), "TZIPFile"); + + TPluginHandler *hSQL = gPluginMgr->FindHandler("TSQLServer", "sqlite://test.db"); + ASSERT_NE(hSQL, nullptr); + EXPECT_STREQ(hSQL->GetClass(), "TSQLiteServer"); + + // Clean up + gSystem->Unlink(macroFile.Data()); + gSystem->Unlink(baseSubdir.Data()); + gSystem->Unlink(customDir.Data()); + + if (!savedPath.IsNull()) { + gEnv->SetValue("Root.PluginPath", savedPath.Data()); + } +} From 4bbfaae4d4d9171516a614f57d57d305ee6ab0cf Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 29 Oct 2026 12:59:10 -0500 Subject: [PATCH 2/9] [core] Isolate multi-directory plugin regression test Use three temporary plugin directories and a fresh plugin manager so cached built-in handlers cannot hide path accumulation. Select the path separator for the platform and restore global state and remove temporary files in fixture teardown, including after fatal assertions. Validation: clang-format --dry-run --Werror and git diff --check passed. Runtime validation remains pending because VS Code CMake build and test tools failed and unit-test discovery found no tests. Assisted-by: Copilot SDK:GPT 6 astra --- core/base/test/TPluginManagerTests.cxx | 131 +++++++++++++++---------- 1 file changed, 78 insertions(+), 53 deletions(-) diff --git a/core/base/test/TPluginManagerTests.cxx b/core/base/test/TPluginManagerTests.cxx index 848db82804630..2b84dac61b4d3 100644 --- a/core/base/test/TPluginManagerTests.cxx +++ b/core/base/test/TPluginManagerTests.cxx @@ -1,73 +1,98 @@ #include "gtest/gtest.h" +#include "TEnv.h" #include "TPluginManager.h" #include "TROOT.h" -#include "TEnv.h" +#include "TString.h" #include "TSystem.h" #include "TUUID.h" #include +#include -// Regression test for https://github.com/root-project/root/issues/23536: -// TPluginManager::FindHandler must correctly resolve plugins across all directories -// when Root.PluginPath contains multiple colon-separated directory paths. -TEST(TPluginManager, MultiDirectoryPluginPath) -{ - // Save original Root.PluginPath - const char *origPluginPath = gEnv->GetValue("Root.PluginPath", (char *)nullptr); - TString savedPath = origPluginPath ? origPluginPath : ""; - - // Construct default plugin path ($ROOTSYS/etc/plugins) - TString defaultPath = "plugins"; - gSystem->PrependPathName(TROOT::GetEtcDir(), defaultPath); +namespace { - // Create a temporary custom plugin directory with a custom macro - TUUID uuid; - TString tempDir = gSystem->TempDirectory(); - TString customDir = TString::Format("test_plugins_%s", uuid.AsString()); - gSystem->PrependPathName(tempDir.Data(), customDir); - - TString baseSubdir = "TCustomPluginBase"; - gSystem->PrependPathName(customDir.Data(), baseSubdir); - ASSERT_EQ(0, gSystem->mkdir(baseSubdir.Data(), /*recursive=*/true)); - - TString macroFile = "P010_TCustomPlugin.C"; - gSystem->PrependPathName(baseSubdir.Data(), macroFile); +class TPluginManagerTest : public testing::Test { +protected: + TEnv fEnv; + TPluginManager fPluginManager; + TEnv *fSavedEnv = nullptr; + TPluginManager *fSavedPluginManager = nullptr; + std::vector fPaths; + TString fBase; + void SetUp() override { - std::ofstream f(macroFile.Data()); - ASSERT_TRUE(f.is_open()); - f << "void P010_TCustomPlugin()\n" - << "{\n" - << " gPluginMgr->AddHandler(\"TCustomPluginBase\", \".*\", \"TCustomClass\",\n" - << " \"CustomLib\", \"TCustomClass()\");\n" - << "}\n"; - } + // Initialize ROOT before replacing the globals used by interpreted plugin macros. + (void)gROOT->GetPluginManager(); + fSavedEnv = gEnv; + fSavedPluginManager = gPluginMgr; + gEnv = &fEnv; + gPluginMgr = &fPluginManager; - // Set multi-directory Root.PluginPath: custom directory prepended before default directory - TString multiPath = customDir + ":" + defaultPath; - gEnv->SetValue("Root.PluginPath", multiPath.Data()); + TUUID uuid; + TString id = uuid.AsString(); + id.ReplaceAll("-", "_"); + fBase = "TestPluginBase_" + id; + TString rootDir = "test_plugins_" + id; + gSystem->PrependPathName(gSystem->TempDirectory(), rootDir); + ASSERT_EQ(0, gSystem->mkdir(rootDir)); + fPaths.push_back(rootDir); - // 1. Verify that the custom plugin from the first directory is resolved - TPluginHandler *hCustom = gPluginMgr->FindHandler("TCustomPluginBase", "custom://test"); - ASSERT_NE(hCustom, nullptr); - EXPECT_STREQ(hCustom->GetClass(), "TCustomClass"); + TString pluginPath; + for (int i = 0; i < 3; ++i) { + TString dir = TString::Format("dir%d", i); + gSystem->PrependPathName(rootDir, dir); + ASSERT_EQ(0, gSystem->mkdir(dir)); + fPaths.push_back(dir); - // 2. Verify that built-in plugins from subsequent directories are resolved properly (not dropped or corrupted) - TPluginHandler *hArchive = gPluginMgr->FindHandler("TArchiveFile", "test.zip"); - ASSERT_NE(hArchive, nullptr); - EXPECT_STREQ(hArchive->GetClass(), "TZIPFile"); + TString baseDir = fBase; + gSystem->PrependPathName(dir, baseDir); + ASSERT_EQ(0, gSystem->mkdir(baseDir)); + fPaths.push_back(baseDir); - TPluginHandler *hSQL = gPluginMgr->FindHandler("TSQLServer", "sqlite://test.db"); - ASSERT_NE(hSQL, nullptr); - EXPECT_STREQ(hSQL->GetClass(), "TSQLiteServer"); + TString macroName = TString::Format("P010_TestPlugin%d_", i) + id; + TString macroFile = macroName + ".C"; + gSystem->PrependPathName(baseDir, macroFile); + fPaths.push_back(macroFile); + std::ofstream macro(macroFile.Data()); + ASSERT_TRUE(macro.is_open()); + macro << "#include \"TPluginManager.h\"\n" + << "void " << macroName << "()\n{\n" + << " gPluginMgr->AddHandler(\"" << fBase << "\", \"^test" << i << ":\", \"TestPlugin" << i + << "\", \"TestPluginLib\", \"TestPlugin" << i << "()\");\n}\n"; + macro.close(); + ASSERT_TRUE(macro.good()); - // Clean up - gSystem->Unlink(macroFile.Data()); - gSystem->Unlink(baseSubdir.Data()); - gSystem->Unlink(customDir.Data()); + if (i != 0) { +#ifdef R__WIN32 + pluginPath += ";"; +#else + pluginPath += ":"; +#endif + } + pluginPath += dir; + } + fEnv.SetValue("Root.PluginPath", pluginPath); + } + + void TearDown() override + { + gPluginMgr = fSavedPluginManager; + gEnv = fSavedEnv; + for (auto it = fPaths.rbegin(); it != fPaths.rend(); ++it) + EXPECT_EQ(0, gSystem->Unlink(*it)) << *it; + } +}; - if (!savedPath.IsNull()) { - gEnv->SetValue("Root.PluginPath", savedPath.Data()); +// Regression test for https://github.com/root-project/root/issues/23536. +TEST_F(TPluginManagerTest, MultiDirectoryPluginPath) +{ + for (int i = 0; i < 3; ++i) { + auto *handler = fPluginManager.FindHandler(fBase, TString::Format("test%d:example", i)); + ASSERT_NE(handler, nullptr) << "Plugin directory " << i; + EXPECT_STREQ(handler->GetClass(), TString::Format("TestPlugin%d", i).Data()); } } + +} // namespace From e338fc87a620816669d9815a0e5622e3ea762297 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 30 Oct 2026 12:43:25 -0500 Subject: [PATCH 3/9] [core] Fix path handling edge cases Avoid prefixing the dependency output path twice. Advance the parent component while reconstructing dictionary header paths, and stop at the filesystem root. Assisted-by: Copilot SDK:GPT 6 astra --- core/base/src/TSystem.cxx | 1 - core/metacling/src/TCling.cxx | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/core/base/src/TSystem.cxx b/core/base/src/TSystem.cxx index 9f8d8a83a23a2..bc8ee2454da3d 100644 --- a/core/base/src/TSystem.cxx +++ b/core/base/src/TSystem.cxx @@ -2619,7 +2619,6 @@ static void R__WriteDependencyFile(const TString & build_loc, const TString &dep } else { needToUnlinkTempFile = true; devnullfile = depfilename + ".stderr.tmp"; - gSystem->PrependPathName(build_loc, devnullfile); } } #endif diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 022cc1ddba473..a95f35412adf9 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -778,8 +778,12 @@ int TCling_GenerateDictionary(const std::vector &classes, while (dirbase.Length() && dirbase != "." && dirbase != "include" && dirbase != "inc" && dirbase != "prec_stl") { + TString parent = gSystem->GetDirName(dir); + if (parent == dir) + break; gSystem->PrependPathName(dirbase, header); - dir = gSystem->GetDirName(dir); + dir = parent; + dirbase = gSystem->BaseName(dir); } fileContent += TString("#include \"") + header + "\"\n"; } From 7888131cba03df56463900111a6730a52d630955 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 30 Oct 2026 14:38:43 -0500 Subject: [PATCH 4/9] [core] Test Windows dependency output path Exercise the ACLiC dependency generation path with a cmd.exe alias so that the nonstandard COMSPEC branch is covered without changing the actual shell behavior. Assisted-by: Copilot SDK:GPT 6 astra --- core/base/test/IncludePathTest.cxx | 41 +++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/core/base/test/IncludePathTest.cxx b/core/base/test/IncludePathTest.cxx index b5f0fbadb0fd2..e76acedfa68ee 100644 --- a/core/base/test/IncludePathTest.cxx +++ b/core/base/test/IncludePathTest.cxx @@ -1,5 +1,6 @@ #include "gtest/gtest.h" #include "TSystem.h" +#include "TUUID.h" #include @@ -14,15 +15,53 @@ TEST(TSystem, IncludePath) "-I \"C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.38.33130\\include\""); #endif +#ifdef WIN32 + struct ComspecGuard { + TString fOriginal; + TString fAlias; + + ~ComspecGuard() + { + gSystem->Setenv("COMSPEC", fOriginal); + gSystem->Unlink(fAlias); + } + }; + + const char *comspec = gSystem->Getenv("COMSPEC"); + ASSERT_NE(comspec, nullptr); + TString cmdPath = comspec; + if (cmdPath.BeginsWith("\"") && cmdPath.EndsWith("\"")) { + cmdPath.Remove(0, 1); + cmdPath.Chop(); + } + TString normalizedCmdPath = cmdPath; + normalizedCmdPath.ToLower(); + ASSERT_TRUE(normalizedCmdPath.EndsWith("cmd.exe")) << "This regression test needs cmd.exe as the process shell"; + + TUUID uuid; + TString alias = TString::Format("root_test_shell_%s.exe", uuid.AsString()); + gSystem->PrependPathName(gSystem->TempDirectory(), alias); + ComspecGuard comspecGuard{comspec, alias}; + ASSERT_EQ(0, gSystem->CopyFile(cmdPath, alias)); + + // Use a copied cmd.exe under a different name to exercise the temporary + // stderr-file path without changing the shell used to execute commands. + gSystem->Setenv("COMSPEC", alias); + ASSERT_EQ(0, gSystem->Exec("exit 0")); +#endif + testing::internal::CaptureStderr(); - gSystem->CompileMacro("Foo.C", "f"); + const auto compileResult = gSystem->CompileMacro("Foo.C", "f"); std::cerr.flush(); std::string errors = testing::internal::GetCapturedStderr(); + EXPECT_TRUE(compileResult); gSystem->Unload("Foo_C"); gSystem->CleanCompiledMacros(); #ifdef WIN32 EXPECT_TRUE((errors.find("cl : Command line warning D9002 : ignoring unknown option ") == std::string::npos)); + // This will need to be updated once we no longer rely on rmkdepend. + EXPECT_EQ(errors.find("Failed to run rmkdepend"), std::string::npos); gSystem->Exec("del Foo_C*"); #else EXPECT_TRUE((errors.find("c++: error: unrecognized command line option ") == std::string::npos)); From 5903bd3c374c956a524a9697624488a520642f82 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 30 Oct 2026 15:29:11 -0500 Subject: [PATCH 5/9] [core] Test dictionary header path traversal Exercise TCling_GenerateDictionary with an unknown class declared in a nested ROOT header path to cover directory traversal while generating a dictionary. Assisted-by: Copilot SDK:GPT 6 astra --- core/metacling/test/TClingTests.cxx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/core/metacling/test/TClingTests.cxx b/core/metacling/test/TClingTests.cxx index 08f846ee5f256..927dd09af0b99 100644 --- a/core/metacling/test/TClingTests.cxx +++ b/core/metacling/test/TClingTests.cxx @@ -4,6 +4,7 @@ #include "TInterpreter.h" #include "TROOT.h" #include "TSystem.h" +#include "TString.h" #include "gmock/gmock.h" @@ -63,6 +64,24 @@ TEST_F(TClingTests, GenerateDictionaryRegression) EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "")); EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "set")); + auto *sequentialExecutor = TClass::GetClass("ROOT::TSequentialExecutor"); + ASSERT_NE(sequentialExecutor, nullptr); + TString headerDir = gSystem->GetDirName(sequentialExecutor->GetDeclFileName()); + ASSERT_STREQ(gSystem->BaseName(headerDir), "ROOT"); + + ASSERT_TRUE(gInterpreter->Declare( + "#include \n" + "#include \n" + "int TCling_GenerateDictionary(const std::vector&, const std::vector&,\n" + " const std::vector&, const std::vector&);")); + TString includePath = gSystem->GetIncludePath(); + const auto result = + gInterpreter->ProcessLine("std::vector classes{\"std::list\"};" + "std::vector emptyvector, unknown{\"ROOT::TSequentialExecutor\"};" + "return TCling_GenerateDictionary(classes, emptyvector, emptyvector, unknown);"); + gSystem->SetIncludePath(includePath); + EXPECT_EQ(result, 0); + // FIXME: This makes the linkdef parser go in an infinite loop. //EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector>", "")); } From c3c62125071c2b9ce21cbae003da69a90bf7c9ab Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 3 Nov 2026 17:18:52 +0100 Subject: [PATCH 6/9] [core] Fix dictionary path regression on Windows Use a rootcling-generated test class with a stable nested include path so the dictionary path traversal test behaves consistently across platforms. Guard empty directory paths before calling BaseName and stop at drive roots when walking the declaration path. Assisted-by: Copilot SDK:GPT 6 astra --- core/metacling/src/TCling.cxx | 14 ++++++---- core/metacling/test/CMakeLists.txt | 7 +++++ .../test/TClingDictionaryPathProbeLinkDef.h | 9 ++++++ core/metacling/test/TClingTests.cxx | 28 +++++++++++++------ .../ROOT/TClingDictionaryPathProbe.hxx | 6 ++++ 5 files changed, 49 insertions(+), 15 deletions(-) create mode 100644 core/metacling/test/TClingDictionaryPathProbeLinkDef.h create mode 100644 core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index a95f35412adf9..9d93ae95e8a48 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -774,16 +774,18 @@ int TCling_GenerateDictionary(const std::vector &classes, if (cl && cl->GetDeclFileName()) { TString header = gSystem->BaseName(cl->GetDeclFileName()); TString dir = gSystem->GetDirName(cl->GetDeclFileName()); - TString dirbase(gSystem->BaseName(dir)); - while (dirbase.Length() && dirbase != "." - && dirbase != "include" && dirbase != "inc" - && dirbase != "prec_stl") { + TString dirbase; + if (!dir.IsNull()) + dirbase = gSystem->BaseName(dir); + while (dirbase.Length() && dirbase != "." && dirbase != "include" && dirbase != "inc" && + dirbase != "prec_stl") { TString parent = gSystem->GetDirName(dir); - if (parent == dir) + const Bool_t isDriveRoot = dir.Length() == 3 && dir[1] == ':' && (dir[2] == '/' || dir[2] == '\\'); + if (parent == dir || isDriveRoot) break; gSystem->PrependPathName(dirbase, header); dir = parent; - dirbase = gSystem->BaseName(dir); + dirbase = dir.IsNull() ? "" : gSystem->BaseName(dir); } fileContent += TString("#include \"") + header + "\"\n"; } diff --git a/core/metacling/test/CMakeLists.txt b/core/metacling/test/CMakeLists.txt index eaec1bfd31863..304031e7d5e30 100644 --- a/core/metacling/test/CMakeLists.txt +++ b/core/metacling/test/CMakeLists.txt @@ -29,6 +29,13 @@ ROOT_ADD_GTEST( RIO GTest::gmock ) +ROOT_GENERATE_DICTIONARY( + TClingDictionaryPathProbeDict + ${CMAKE_CURRENT_SOURCE_DIR}/include/ROOT/TClingDictionaryPathProbe.hxx + LINKDEF TClingDictionaryPathProbeLinkDef.h + OPTIONS -I${CMAKE_CURRENT_SOURCE_DIR}/include +) +target_sources(TClingTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/TClingDictionaryPathProbeDict.cxx) ROOT_ADD_GTEST(TClingLoadUnloadFile TClingLoadUnloadFileTests.cxx LIBRARIES Core RIO) diff --git a/core/metacling/test/TClingDictionaryPathProbeLinkDef.h b/core/metacling/test/TClingDictionaryPathProbeLinkDef.h new file mode 100644 index 0000000000000..c4eb829d790ab --- /dev/null +++ b/core/metacling/test/TClingDictionaryPathProbeLinkDef.h @@ -0,0 +1,9 @@ +#ifdef __CLING__ + +#pragma link off all globals; +#pragma link off all classes; +#pragma link off all functions; + +#pragma link C++ class TClingDictionaryPathProbe + ; + +#endif diff --git a/core/metacling/test/TClingTests.cxx b/core/metacling/test/TClingTests.cxx index 927dd09af0b99..b6c50cc4f01fa 100644 --- a/core/metacling/test/TClingTests.cxx +++ b/core/metacling/test/TClingTests.cxx @@ -64,26 +64,36 @@ TEST_F(TClingTests, GenerateDictionaryRegression) EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "")); EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "set")); - auto *sequentialExecutor = TClass::GetClass("ROOT::TSequentialExecutor"); - ASSERT_NE(sequentialExecutor, nullptr); - TString headerDir = gSystem->GetDirName(sequentialExecutor->GetDeclFileName()); - ASSERT_STREQ(gSystem->BaseName(headerDir), "ROOT"); + TString testDir = gSystem->GetDirName(__FILE__); + TString includeDir = "include"; + gSystem->PrependPathName(testDir, includeDir); + TString header = "ROOT/TClingDictionaryPathProbe.hxx"; + gSystem->PrependPathName(includeDir, header); + struct IncludePathGuard { + TString fPath; + ~IncludePathGuard() { gSystem->SetIncludePath(fPath); } + } includePathGuard{gSystem->GetIncludePath()}; + gSystem->AddIncludePath(TString::Format("-I \"%s\"", includeDir.Data())); + auto *probe = TClass::GetClass("TClingDictionaryPathProbe"); + ASSERT_NE(probe, nullptr); + ASSERT_NE(probe->GetDeclFileName(), nullptr); + TString declFile = probe->GetDeclFileName(); + declFile.ReplaceAll("\\", "/"); + EXPECT_STREQ(declFile, "ROOT/TClingDictionaryPathProbe.hxx"); ASSERT_TRUE(gInterpreter->Declare( "#include \n" "#include \n" "int TCling_GenerateDictionary(const std::vector&, const std::vector&,\n" " const std::vector&, const std::vector&);")); - TString includePath = gSystem->GetIncludePath(); const auto result = - gInterpreter->ProcessLine("std::vector classes{\"std::list\"};" - "std::vector emptyvector, unknown{\"ROOT::TSequentialExecutor\"};" + gInterpreter->ProcessLine("std::vector classes{\"std::list\"};" + "std::vector emptyvector, unknown{\"TClingDictionaryPathProbe\"};" "return TCling_GenerateDictionary(classes, emptyvector, emptyvector, unknown);"); - gSystem->SetIncludePath(includePath); EXPECT_EQ(result, 0); // FIXME: This makes the linkdef parser go in an infinite loop. - //EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector>", "")); + // EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector>", "")); } #if !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS) diff --git a/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx b/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx new file mode 100644 index 0000000000000..fe1160d77a45b --- /dev/null +++ b/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx @@ -0,0 +1,6 @@ +#ifndef ROOT_TClingDictionaryPathProbe +#define ROOT_TClingDictionaryPathProbe + +struct TClingDictionaryPathProbe {}; + +#endif From eda4e7f9714804c06acb7cc2e728f5787aefc541 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 5 Nov 2026 13:51:45 +0100 Subject: [PATCH 7/9] [core] Test generated Cling dictionary include path Inspect the generated AutoDict source directly so the regression test validates path reconstruction without depending on ACLiC compilation succeeding on every platform. Avoid Cling JIT in dictionary regression test Call the dictionary generator from compiled test code instead of materializing std::vector globals in a JIT module. Keep the test strict about ACLiC success and verify the generated header include. Not allowed to link again libCling Avoid linking Cling in dictionary regression test Invoke the internal dictionary helper through the interpreter, keeping vector objects local to a lambda so they do not require JIT static initialization. Check the interpreter error code and dictionary result. [NFC] white space Avoid copying cmd.exe in IncludePath test Use a quoted COMSPEC path to exercise the nonstandard-shell branch without copying the protected Windows system executable. [core] Test Windows shell handling Remove unsafe quoted COMSPEC test Do not run ACLiC with a quoted cmd.exe COMSPEC path: Windows process launch fails in that configuration. Retain coverage using the normal unquoted cmd.exe path. Assisted-by: Copilot SDK:GPT 6 astra --- core/base/test/IncludePathTest.cxx | 61 ++++++++++++++++++----------- core/metacling/test/TClingTests.cxx | 22 +++++++++-- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/core/base/test/IncludePathTest.cxx b/core/base/test/IncludePathTest.cxx index e76acedfa68ee..fdee3792f1ce3 100644 --- a/core/base/test/IncludePathTest.cxx +++ b/core/base/test/IncludePathTest.cxx @@ -1,9 +1,45 @@ #include "gtest/gtest.h" #include "TSystem.h" -#include "TUUID.h" +#include "TString.h" #include +#ifdef WIN32 +namespace { +struct ComspecGuard { + TString fOriginal; + + ~ComspecGuard() + { + gSystem->Setenv("COMSPEC", fOriginal); + } +}; + +} // namespace +#endif + +TEST(TSystem, ExecWithComspec) +{ +#ifdef WIN32 + ASSERT_TRUE(gSystem); + + const char *comspec = gSystem->Getenv("COMSPEC"); + ASSERT_NE(comspec, nullptr); + TString cmdPath = comspec; + if (cmdPath.BeginsWith("\"") && cmdPath.EndsWith("\"")) { + cmdPath.Remove(0, 1); + cmdPath.Chop(); + } + TString normalizedCmdPath = cmdPath; + normalizedCmdPath.ToLower(); + ASSERT_TRUE(normalizedCmdPath.EndsWith("cmd.exe")) << "This regression test needs cmd.exe as the process shell"; + + ComspecGuard comspecGuard{comspec}; + gSystem->Setenv("COMSPEC", cmdPath); + ASSERT_EQ(0, gSystem->Exec("exit 0")); +#endif +} + TEST(TSystem, IncludePath) { ASSERT_TRUE(gSystem); @@ -16,17 +52,6 @@ TEST(TSystem, IncludePath) #endif #ifdef WIN32 - struct ComspecGuard { - TString fOriginal; - TString fAlias; - - ~ComspecGuard() - { - gSystem->Setenv("COMSPEC", fOriginal); - gSystem->Unlink(fAlias); - } - }; - const char *comspec = gSystem->Getenv("COMSPEC"); ASSERT_NE(comspec, nullptr); TString cmdPath = comspec; @@ -38,16 +63,8 @@ TEST(TSystem, IncludePath) normalizedCmdPath.ToLower(); ASSERT_TRUE(normalizedCmdPath.EndsWith("cmd.exe")) << "This regression test needs cmd.exe as the process shell"; - TUUID uuid; - TString alias = TString::Format("root_test_shell_%s.exe", uuid.AsString()); - gSystem->PrependPathName(gSystem->TempDirectory(), alias); - ComspecGuard comspecGuard{comspec, alias}; - ASSERT_EQ(0, gSystem->CopyFile(cmdPath, alias)); - - // Use a copied cmd.exe under a different name to exercise the temporary - // stderr-file path without changing the shell used to execute commands. - gSystem->Setenv("COMSPEC", alias); - ASSERT_EQ(0, gSystem->Exec("exit 0")); + ComspecGuard comspecGuard{comspec}; + gSystem->Setenv("COMSPEC", cmdPath); #endif testing::internal::CaptureStderr(); diff --git a/core/metacling/test/TClingTests.cxx b/core/metacling/test/TClingTests.cxx index b6c50cc4f01fa..cfd07d3eb02f6 100644 --- a/core/metacling/test/TClingTests.cxx +++ b/core/metacling/test/TClingTests.cxx @@ -8,6 +8,8 @@ #include "gmock/gmock.h" +#include +#include #include #include #include @@ -81,17 +83,31 @@ TEST_F(TClingTests, GenerateDictionaryRegression) declFile.ReplaceAll("\\", "/"); EXPECT_STREQ(declFile, "ROOT/TClingDictionaryPathProbe.hxx"); + TString generatedDictionaryPath = "AutoDict_std__list_TClingDictionaryPathProbe__.cxx"; + gSystem->PrependPathName(gSystem->pwd(), generatedDictionaryPath); + // The public GenerateDictionary API passes no unknown classes, so use its helper to exercise this path. ASSERT_TRUE(gInterpreter->Declare( "#include \n" "#include \n" "int TCling_GenerateDictionary(const std::vector&, const std::vector&,\n" " const std::vector&, const std::vector&);")); + TInterpreter::EErrorCode error = TInterpreter::kNoError; const auto result = - gInterpreter->ProcessLine("std::vector classes{\"std::list\"};" - "std::vector emptyvector, unknown{\"TClingDictionaryPathProbe\"};" - "return TCling_GenerateDictionary(classes, emptyvector, emptyvector, unknown);"); + gInterpreter->ProcessLine("[] {" + "std::vector classes{\"std::list\"};" + "std::vector empty, unknown{\"TClingDictionaryPathProbe\"};" + "return TCling_GenerateDictionary(classes, empty, empty, unknown);" + "}()", + &error); + EXPECT_EQ(error, TInterpreter::kNoError); EXPECT_EQ(result, 0); + std::ifstream generatedDictionary(generatedDictionaryPath.Data()); + ASSERT_TRUE(generatedDictionary.is_open()); + const std::string generatedSource((std::istreambuf_iterator(generatedDictionary)), + std::istreambuf_iterator()); + EXPECT_NE(generatedSource.find("#include \"ROOT/TClingDictionaryPathProbe.hxx\""), std::string::npos); + // FIXME: This makes the linkdef parser go in an infinite loop. // EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector>", "")); } From 112a6c1cd7675e548f9a7b331b81c8007d026b4b Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 5 Nov 2026 22:59:07 +0100 Subject: [PATCH 8/9] [core] Remove unused 4th argument of TCling_GenerateDictionary. Remove related tests --- core/metacling/src/TCling.cxx | 32 ++----------- core/metacling/test/CMakeLists.txt | 8 ---- .../test/TClingDictionaryPathProbeLinkDef.h | 9 ---- core/metacling/test/TClingTests.cxx | 45 ------------------- .../ROOT/TClingDictionaryPathProbe.hxx | 6 --- 5 files changed, 4 insertions(+), 96 deletions(-) delete mode 100644 core/metacling/test/TClingDictionaryPathProbeLinkDef.h delete mode 100644 core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 9d93ae95e8a48..2816a504079df 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -716,8 +716,7 @@ static clang::ClassTemplateDecl* FindTemplateInNamespace(clang::Decl* decl) int TCling_GenerateDictionary(const std::vector &classes, const std::vector &headers, - const std::vector &fwdDecls, - const std::vector &unknown) + const std::vector &fwdDecls) { //This function automatically creates the "LinkDef.h" file for templated //classes then executes CompileMacro on it. @@ -769,27 +768,6 @@ int TCling_GenerateDictionary(const std::vector &classes, for (it = headers.begin(); it != headers.end(); ++it) { fileContent += "#include \"" + *it + "\"\n"; } - for (it = unknown.begin(); it != unknown.end(); ++it) { - TClass* cl = TClass::GetClass(it->c_str()); - if (cl && cl->GetDeclFileName()) { - TString header = gSystem->BaseName(cl->GetDeclFileName()); - TString dir = gSystem->GetDirName(cl->GetDeclFileName()); - TString dirbase; - if (!dir.IsNull()) - dirbase = gSystem->BaseName(dir); - while (dirbase.Length() && dirbase != "." && dirbase != "include" && dirbase != "inc" && - dirbase != "prec_stl") { - TString parent = gSystem->GetDirName(dir); - const Bool_t isDriveRoot = dir.Length() == 3 && dir[1] == ':' && (dir[2] == '/' || dir[2] == '\\'); - if (parent == dir || isDriveRoot) - break; - gSystem->PrependPathName(dirbase, header); - dir = parent; - dirbase = dir.IsNull() ? "" : gSystem->BaseName(dir); - } - fileContent += TString("#include \"") + header + "\"\n"; - } - } for (it = fwdDecls.begin(); it != fwdDecls.end(); ++it) { fileContent += "class " + *it + ";\n"; } @@ -843,8 +821,7 @@ int TCling_GenerateDictionary(const std::vector &classes, int TCling_GenerateDictionary(const std::string& className, const std::vector &headers, - const std::vector &fwdDecls, - const std::vector &unknown) + const std::vector &fwdDecls) { //This function automatically creates the "LinkDef.h" file for templated //classes then executes CompileMacro on it. @@ -852,7 +829,7 @@ int TCling_GenerateDictionary(const std::string& className, //if the file exist. std::vector classes; classes.push_back(className); - return TCling_GenerateDictionary(classes, headers, fwdDecls, unknown); + return TCling_GenerateDictionary(classes, headers, fwdDecls); } //______________________________________________________________________________ @@ -4977,8 +4954,7 @@ Int_t TCling::GenerateDictionary(const char* classes, const char* includes /* = } } // Generate the temporary dictionary file - return !TCling_GenerateDictionary(listClasses, listIncludes, - std::vector(), std::vector()); + return !TCling_GenerateDictionary(listClasses, listIncludes, std::vector()); } //////////////////////////////////////////////////////////////////////////////// diff --git a/core/metacling/test/CMakeLists.txt b/core/metacling/test/CMakeLists.txt index 304031e7d5e30..d7aa006cca17a 100644 --- a/core/metacling/test/CMakeLists.txt +++ b/core/metacling/test/CMakeLists.txt @@ -29,14 +29,6 @@ ROOT_ADD_GTEST( RIO GTest::gmock ) -ROOT_GENERATE_DICTIONARY( - TClingDictionaryPathProbeDict - ${CMAKE_CURRENT_SOURCE_DIR}/include/ROOT/TClingDictionaryPathProbe.hxx - LINKDEF TClingDictionaryPathProbeLinkDef.h - OPTIONS -I${CMAKE_CURRENT_SOURCE_DIR}/include -) -target_sources(TClingTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/TClingDictionaryPathProbeDict.cxx) - ROOT_ADD_GTEST(TClingLoadUnloadFile TClingLoadUnloadFileTests.cxx LIBRARIES Core RIO) if(pyroot) diff --git a/core/metacling/test/TClingDictionaryPathProbeLinkDef.h b/core/metacling/test/TClingDictionaryPathProbeLinkDef.h deleted file mode 100644 index c4eb829d790ab..0000000000000 --- a/core/metacling/test/TClingDictionaryPathProbeLinkDef.h +++ /dev/null @@ -1,9 +0,0 @@ -#ifdef __CLING__ - -#pragma link off all globals; -#pragma link off all classes; -#pragma link off all functions; - -#pragma link C++ class TClingDictionaryPathProbe + ; - -#endif diff --git a/core/metacling/test/TClingTests.cxx b/core/metacling/test/TClingTests.cxx index cfd07d3eb02f6..eb74313a03298 100644 --- a/core/metacling/test/TClingTests.cxx +++ b/core/metacling/test/TClingTests.cxx @@ -4,12 +4,9 @@ #include "TInterpreter.h" #include "TROOT.h" #include "TSystem.h" -#include "TString.h" #include "gmock/gmock.h" -#include -#include #include #include #include @@ -66,48 +63,6 @@ TEST_F(TClingTests, GenerateDictionaryRegression) EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "")); EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set", "set")); - TString testDir = gSystem->GetDirName(__FILE__); - TString includeDir = "include"; - gSystem->PrependPathName(testDir, includeDir); - TString header = "ROOT/TClingDictionaryPathProbe.hxx"; - gSystem->PrependPathName(includeDir, header); - struct IncludePathGuard { - TString fPath; - ~IncludePathGuard() { gSystem->SetIncludePath(fPath); } - } includePathGuard{gSystem->GetIncludePath()}; - gSystem->AddIncludePath(TString::Format("-I \"%s\"", includeDir.Data())); - auto *probe = TClass::GetClass("TClingDictionaryPathProbe"); - ASSERT_NE(probe, nullptr); - ASSERT_NE(probe->GetDeclFileName(), nullptr); - TString declFile = probe->GetDeclFileName(); - declFile.ReplaceAll("\\", "/"); - EXPECT_STREQ(declFile, "ROOT/TClingDictionaryPathProbe.hxx"); - - TString generatedDictionaryPath = "AutoDict_std__list_TClingDictionaryPathProbe__.cxx"; - gSystem->PrependPathName(gSystem->pwd(), generatedDictionaryPath); - // The public GenerateDictionary API passes no unknown classes, so use its helper to exercise this path. - ASSERT_TRUE(gInterpreter->Declare( - "#include \n" - "#include \n" - "int TCling_GenerateDictionary(const std::vector&, const std::vector&,\n" - " const std::vector&, const std::vector&);")); - TInterpreter::EErrorCode error = TInterpreter::kNoError; - const auto result = - gInterpreter->ProcessLine("[] {" - "std::vector classes{\"std::list\"};" - "std::vector empty, unknown{\"TClingDictionaryPathProbe\"};" - "return TCling_GenerateDictionary(classes, empty, empty, unknown);" - "}()", - &error); - EXPECT_EQ(error, TInterpreter::kNoError); - EXPECT_EQ(result, 0); - - std::ifstream generatedDictionary(generatedDictionaryPath.Data()); - ASSERT_TRUE(generatedDictionary.is_open()); - const std::string generatedSource((std::istreambuf_iterator(generatedDictionary)), - std::istreambuf_iterator()); - EXPECT_NE(generatedSource.find("#include \"ROOT/TClingDictionaryPathProbe.hxx\""), std::string::npos); - // FIXME: This makes the linkdef parser go in an infinite loop. // EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector>", "")); } diff --git a/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx b/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx deleted file mode 100644 index fe1160d77a45b..0000000000000 --- a/core/metacling/test/include/ROOT/TClingDictionaryPathProbe.hxx +++ /dev/null @@ -1,6 +0,0 @@ -#ifndef ROOT_TClingDictionaryPathProbe -#define ROOT_TClingDictionaryPathProbe - -struct TClingDictionaryPathProbe {}; - -#endif From ad45e962214cce138ed7ff85fd1764ad072e05e3 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 5 Nov 2026 23:03:33 +0100 Subject: [PATCH 9/9] [core] Remove unused 3rd argument (forward decls) of TCling_GenerateDictionary. --- core/metacling/src/TCling.cxx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/core/metacling/src/TCling.cxx b/core/metacling/src/TCling.cxx index 2816a504079df..453784279f04c 100644 --- a/core/metacling/src/TCling.cxx +++ b/core/metacling/src/TCling.cxx @@ -715,8 +715,7 @@ static clang::ClassTemplateDecl* FindTemplateInNamespace(clang::Decl* decl) // int TCling_GenerateDictionary(const std::vector &classes, - const std::vector &headers, - const std::vector &fwdDecls) + const std::vector &headers) { //This function automatically creates the "LinkDef.h" file for templated //classes then executes CompileMacro on it. @@ -768,9 +767,6 @@ int TCling_GenerateDictionary(const std::vector &classes, for (it = headers.begin(); it != headers.end(); ++it) { fileContent += "#include \"" + *it + "\"\n"; } - for (it = fwdDecls.begin(); it != fwdDecls.end(); ++it) { - fileContent += "class " + *it + ";\n"; - } fileContent += "#ifdef __CLING__ \n"; fileContent += "#pragma link C++ nestedclasses;\n"; fileContent += "#pragma link C++ nestedtypedefs;\n"; @@ -820,8 +816,7 @@ int TCling_GenerateDictionary(const std::vector &classes, } int TCling_GenerateDictionary(const std::string& className, - const std::vector &headers, - const std::vector &fwdDecls) + const std::vector &headers) { //This function automatically creates the "LinkDef.h" file for templated //classes then executes CompileMacro on it. @@ -829,7 +824,7 @@ int TCling_GenerateDictionary(const std::string& className, //if the file exist. std::vector classes; classes.push_back(className); - return TCling_GenerateDictionary(classes, headers, fwdDecls); + return TCling_GenerateDictionary(classes, headers); } //______________________________________________________________________________ @@ -4954,7 +4949,7 @@ Int_t TCling::GenerateDictionary(const char* classes, const char* includes /* = } } // Generate the temporary dictionary file - return !TCling_GenerateDictionary(listClasses, listIncludes, std::vector()); + return !TCling_GenerateDictionary(listClasses, listIncludes); } ////////////////////////////////////////////////////////////////////////////////