Clean up usage of TSystem::PrependPathName

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);

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

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)

core/base/test/IncludePathTest.cxx

@@ -1,8 +1,45 @@
 #include "gtest/gtest.h"
 #include "TSystem.h"
+#include "TString.h"
 
 #include <string>
 
+#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);
@@ -14,15 +51,34 @@ TEST(TSystem, IncludePath)
       "-I \"C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.38.33130\\include\"");
 #endif
 
+#ifdef WIN32
+   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);
+#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));

core/base/test/TPluginManagerTests.cxx

@@ -0,0 +1,98 @@
+#include "gtest/gtest.h"
+
+#include "TEnv.h"
+#include "TPluginManager.h"
+#include "TROOT.h"
+#include "TString.h"
+#include "TSystem.h"
+#include "TUUID.h"
+
+#include <fstream>
+#include <vector>
+
+namespace {
+
+class TPluginManagerTest : public testing::Test {
+protected:
+   TEnv fEnv;
+   TPluginManager fPluginManager;
+   TEnv *fSavedEnv = nullptr;
+   TPluginManager *fSavedPluginManager = nullptr;
+   std::vector<TString> fPaths;
+   TString fBase;
+
+   void SetUp() override
+   {
+      // Initialize ROOT before replacing the globals used by interpreted plugin macros.
+      (void)gROOT->GetPluginManager();
+      fSavedEnv = gEnv;
+      fSavedPluginManager = gPluginMgr;
+      gEnv = &fEnv;
+      gPluginMgr = &fPluginManager;
+
+      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);
+
+      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);
+
+         TString baseDir = fBase;
+         gSystem->PrependPathName(dir, baseDir);
+         ASSERT_EQ(0, gSystem->mkdir(baseDir));
+         fPaths.push_back(baseDir);
+
+         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());
+
+         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;
+   }
+};
+
+// 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

core/metacling/src/TCling.cxx

@@ -715,9 +715,7 @@ static clang::ClassTemplateDecl* FindTemplateInNamespace(clang::Decl* decl)
 //
 
 int TCling_GenerateDictionary(const std::vector<std::string> &classes,
-                              const std::vector<std::string> &headers,
-                              const std::vector<std::string> &fwdDecls,
-                              const std::vector<std::string> &unknown)
+                              const std::vector<std::string> &headers)
 {
    //This function automatically creates the "LinkDef.h" file for templated
    //classes then executes CompileMacro on it.
@@ -769,24 +767,6 @@ int TCling_GenerateDictionary(const std::vector<std::string> &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(gSystem->BaseName(dir));
-            while (dirbase.Length() && dirbase != "."
-                   && dirbase != "include" && dirbase != "inc"
-                   && dirbase != "prec_stl") {
-               gSystem->PrependPathName(dirbase, header);
-               dir = gSystem->GetDirName(dir);
-            }
-            fileContent += TString("#include \"") + header + "\"\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";
@@ -836,17 +816,15 @@ int TCling_GenerateDictionary(const std::vector<std::string> &classes,
 }
 
 int TCling_GenerateDictionary(const std::string& className,
-                              const std::vector<std::string> &headers,
-                              const std::vector<std::string> &fwdDecls,
-                              const std::vector<std::string> &unknown)
+                              const std::vector<std::string> &headers)
 {
    //This function automatically creates the "LinkDef.h" file for templated
    //classes then executes CompileMacro on it.
    //The name of the file depends on the class name, and it's not generated again
    //if the file exist.
    std::vector<std::string> classes;
    classes.push_back(className);
-   return TCling_GenerateDictionary(classes, headers, fwdDecls, unknown);
+   return TCling_GenerateDictionary(classes, headers);
 }
 
 //______________________________________________________________________________
@@ -4971,8 +4949,7 @@ Int_t TCling::GenerateDictionary(const char* classes, const char* includes /* =
       }
    }
    // Generate the temporary dictionary file
-   return !TCling_GenerateDictionary(listClasses, listIncludes,
-      std::vector<std::string>(), std::vector<std::string>());
+   return !TCling_GenerateDictionary(listClasses, listIncludes);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

core/metacling/test/CMakeLists.txt

@@ -29,7 +29,6 @@ ROOT_ADD_GTEST(
   RIO
   GTest::gmock
 )
-
 ROOT_ADD_GTEST(TClingLoadUnloadFile TClingLoadUnloadFileTests.cxx LIBRARIES Core RIO)
 
 if(pyroot)

core/metacling/test/TClingTests.cxx

@@ -64,7 +64,7 @@ TEST_F(TClingTests, GenerateDictionaryRegression)
    EXPECT_TRUE(gInterpreter->GenerateDictionary("std::set<int>", "set"));
 
    // FIXME: This makes the linkdef parser go in an infinite loop.
-   //EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector<std::array<int, 5>>", ""));
+   // EXPECT_TRUE(gInterpreter->GenerateDictionary("std::vector<std::array<int, 5>>", ""));
 }
 
 #if !defined(_MSC_VER) || defined(R__ENABLE_BROKEN_WIN_TESTS)