[core] Add test for ROOT_LIBRARY_PATH.

core/base/src/TPluginManager.cxx

@@ -97,6 +97,8 @@ TFile etc. functionality.
 #include "TObjArray.h"
 #include "ThreadLocalStorage.h"
 
+#include <ROOT/FoundationUtils.hxx>
+
 #include <memory>
 #include <sstream>
 
@@ -513,11 +515,7 @@ void TPluginManager::LoadHandlersFromPluginDirs(const char *base)
       plugindirs = "plugins";
       gSystem->PrependPathName(TROOT::GetEtcDir(), plugindirs);
    }
-#ifdef WIN32
-   TObjArray *dirs = plugindirs.Tokenize(";");
-#else
-   TObjArray *dirs = plugindirs.Tokenize(":");
-#endif
+   TObjArray *dirs = plugindirs.Tokenize(TString(ROOT::FoundationUtils::GetEnvPathSeparator()));
    TString d;
    for (Int_t i = 0; i < dirs->GetEntriesFast(); i++) {
       d = ((TObjString*)dirs->At(i))->GetString();

core/base/src/TROOT.cxx

@@ -2934,18 +2934,14 @@ const char *TROOT::GetMacroPath()
    TString &macroPath = ROOT::GetMacroPath();
 
    if (macroPath.Length() == 0) {
+      const TString sep(ROOT::FoundationUtils::GetEnvPathSeparator());
       macroPath = gEnv->GetValue("Root.MacroPath", (char*)nullptr);
-#if defined(R__WIN32)
-      macroPath.ReplaceAll("; ", ";");
-#else
-      macroPath.ReplaceAll(": ", ":");
-#endif
+      // Drop the blank that may follow a separator in the rootrc value, so that
+      // "Root.MacroPath: .: $(HOME)/macros" does not yield a path element
+      // starting with a space.
+      macroPath.ReplaceAll(sep + " ", sep);
       if (macroPath.Length() == 0)
-#if !defined(R__WIN32)
-         macroPath = ".:" + TROOT::GetMacroDir();
-#else
-         macroPath = ".;" + TROOT::GetMacroDir();
-#endif
+         macroPath = "." + sep + TROOT::GetMacroDir();
    }
 
    return macroPath;

core/base/test/CMakeLists.txt

@@ -26,7 +26,8 @@ ROOT_ADD_GTEST(CoreEnvTests TEnvTests.cxx LIBRARIES Core)
 
 ROOT_ADD_GTEST(CoreErrorTests TErrorTests.cxx LIBRARIES Core)
 
-ROOT_ADD_GTEST(CoreSystemTests TSystemTests.cxx LIBRARIES Core)
+# INCLUDE_DIRS: for ROOT/FoundationUtils.hxx
+ROOT_ADD_GTEST(CoreSystemTests TSystemTests.cxx LIBRARIES Core INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/core/foundation/res)
 
 ROOT_ADD_GTEST(CoreCryptoRandomTest CryptoRandomTest.cxx LIBRARIES Core)
 

core/base/test/TSystemTests.cxx

@@ -2,13 +2,18 @@
 
 #include "TSystem.h"
 #include "TString.h"
+#include "TROOT.h"
+
+#include <ROOT/FoundationUtils.hxx>
 
 #include <string>
 #include <cstdio>
 #include <cstdint>
 #include <fstream>
 #include <streambuf>
 
+static const char kPathSep = ROOT::FoundationUtils::GetEnvPathSeparator();
+
 TEST(TSystem, TempFile)
 {
    TString fname = "root_test_";
@@ -31,6 +36,63 @@ TEST(TSystem, TempFile)
    gSystem->Unlink(fname);
 }
 
+// Count occurrences of `dir` as a full path component of `path`.
+static int CountPathComponent(const TString &path, const TString &dir)
+{
+   int count = 0;
+   TString token;
+   Ssiz_t from = 0;
+   while (path.Tokenize(token, from, TString::Format("%c", kPathSep)))
+      if (token == dir)
+         ++count;
+   return count;
+}
+
+// Exercise the interplay of Get/Add/SetDynamicPath as a single scenario:
+// the dynamic path is process-global state, so the steps are order dependent.
+TEST(TSystem, DynamicPath)
+{
+   const TString defaultPath = gSystem->GetDynamicPath();
+   // The ROOT library directory is always part of the default path.
+   EXPECT_TRUE(defaultPath.Contains(TROOT::GetLibDir()))
+      << "default path: " << defaultPath;
+
+   // AddDynamicPath appends the directory (at the end) ...
+   // (the directories do not need to exist; use names that cannot already be
+   // part of the default path)
+   const TString extraDir1 = TString::Format("%s/root-dynpath-gtest-1", gSystem->TempDirectory());
+   gSystem->AddDynamicPath(extraDir1);
+   TString path = gSystem->GetDynamicPath();
+   EXPECT_TRUE(path.EndsWith(extraDir1)) << "path: " << path;
+   EXPECT_EQ(1, CountPathComponent(path, extraDir1)) << "path: " << path;
+   // ... and keeps the rest of the path intact.
+   EXPECT_TRUE(path.BeginsWith(defaultPath)) << "path: " << path;
+
+   // Appended directories accumulate in order.
+   const TString extraDir2 = TString::Format("%s/root-dynpath-gtest-2", gSystem->TempDirectory());
+   gSystem->AddDynamicPath(extraDir2);
+   path = gSystem->GetDynamicPath();
+   EXPECT_TRUE(path.EndsWith(TString::Format("%s%c%s", extraDir1.Data(), kPathSep, extraDir2.Data())))
+      << "path: " << path;
+
+   // AddDynamicPath(nullptr) is a no-op.
+   gSystem->AddDynamicPath(nullptr);
+   EXPECT_STREQ(path, gSystem->GetDynamicPath());
+
+   // SetDynamicPath freezes the path to exactly the given value.
+   const TString userPath = TString::Format("%s%c%s", extraDir2.Data(), kPathSep, extraDir1.Data());
+   gSystem->SetDynamicPath(userPath);
+   EXPECT_STREQ(userPath, gSystem->GetDynamicPath());
+
+   // SetDynamicPath(nullptr) resets to the default: the explicitly set value
+   // and the previously appended directories are gone, the ROOT library
+   // directory is back.
+   gSystem->SetDynamicPath(nullptr);
+   path = gSystem->GetDynamicPath();
+   EXPECT_TRUE(path.Contains(TROOT::GetLibDir())) << "path: " << path;
+   EXPECT_EQ(0, CountPathComponent(path, extraDir2)) << "path: " << path;
+}
+
 TEST(TSystem, TempFileSuffix)
 {
    TString fname = "root_suffix_test_";

core/clingutils/src/TClingUtils.cxx

@@ -5208,11 +5208,7 @@ void ROOT::TMetaUtils::SetPathsForRelocatability(std::vector<std::string>& cling
    if (!envInclPath)
       return;
 
-#ifdef _WIN32
-   constexpr char kPathSep = ';';
-#else
-   constexpr char kPathSep = ':';
-#endif
+   const char kPathSep = ROOT::FoundationUtils::GetEnvPathSeparator();
 
    std::istringstream envInclPathsStream(envInclPath);
    std::string inclPath;

core/metacling/src/TCling.cxx

@@ -5937,11 +5937,7 @@ Int_t TCling::LoadLibraryMap(const char* rootmapfile)
    TString ldpath = gSystem->GetDynamicPath();
    if (ldpath != fRootmapLoadPath) {
       fRootmapLoadPath = ldpath;
-#ifdef WIN32
-      TObjArray* paths = ldpath.Tokenize(";");
-#else
-      TObjArray* paths = ldpath.Tokenize(":");
-#endif
+      TObjArray *paths = ldpath.Tokenize(TString(ROOT::FoundationUtils::GetEnvPathSeparator()));
       TString d;
       for (Int_t i = 0; i < paths->GetEntriesFast(); i++) {
          d = ((TObjString *)paths->At(i))->GetString();

core/rint/src/TTabCom.cxx

@@ -148,15 +148,13 @@
 #include "Rstrstream.h"
 #include "strlcpy.h"
 
+#include <ROOT/FoundationUtils.hxx>
+
 #define BUF_SIZE 1024 // must be smaller than/equal to fgLineBufSize in Getline.cxx and
                       // lineBufSize in cppcompleter.py
 #define IfDebug(x)  if(gDebug==TTabCom::kDebug) x
 
-#ifdef R__WIN32
-const char kDelim = ';';
-#else
-const char kDelim = ':';
-#endif
+const char kDelim = ROOT::FoundationUtils::GetEnvPathSeparator();
 
 
 // ----------------------------------------------------------------------------

core/unix/src/TUnixSystem.cxx

@@ -4552,19 +4552,33 @@ int TUnixSystem::UnixSend(int sock, const void *buffer, int length, int flag)
 /// path" | sed 's/.*=//g' | awk '//{print $1}')` This might be useful in scenarios, where ROOT is instantiated many
 /// times.
 
-static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFALSE)
+static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFALSE,
+                               const char *addpath = nullptr)
 {
    static TString dynpath_full;
    static std::atomic<bool> initialized(kFALSE);
    static std::atomic<bool> seenCling(kFALSE);
 
    // If we have not seen Cling but the result has been initialized and gCling
    // is still nullptr, the result won't change.
-   if (newpath == nullptr && !reset && (seenCling || (initialized && gCling == nullptr)))
+   if (newpath == nullptr && addpath == nullptr && !reset &&
+       (seenCling || (initialized && gCling == nullptr)))
       return dynpath_full;
 
    R__LOCKGUARD2(gSystemMutex);
 
+   // Directories appended via `addpath` (TUnixSystem::AddDynamicPath). They are
+   // kept separately from the automatically assembled part so that they survive
+   // a re-assembly (e.g. the deferred insertion of the Cling provided parts).
+   static TString addedPaths;
+
+   if (reset) {
+      addedPaths = "";
+      // Re-arm the deferred insertion of the Cling provided parts (it is set
+      // again below if gCling is already available).
+      seenCling = kFALSE;
+   }
+
    if (newpath) {
       dynpath_full = newpath;
       // Don't erase the user given path at the next call.
@@ -4579,6 +4593,23 @@ static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFA
       return dynpath_full;
    }
 
+   if (addpath && *addpath) {
+      // Record the extra directory and append it to the current value. Contrary
+      // to a path set explicitly via SetDynamicPath (`newpath`), this must not
+      // set seenCling: the automatically assembled part can still be updated
+      // (to insert the Cling provided parts) and the extra directories are then
+      // re-appended below.
+      addedPaths += ":";
+      addedPaths += addpath;
+      if (initialized) {
+         if (!dynpath_full.EndsWith(":"))
+            dynpath_full += ":";
+         dynpath_full += addpath;
+      }
+      // Fall through: if the path was not yet assembled, or if the Cling
+      // provided parts can now be inserted, do it now (extrapath included).
+   }
+
    // Another thread might have updated this. Even-though this is executed at the
    // start of the process, we might get there if the user is explicitly
    // 'resetting' the value.
@@ -4668,7 +4699,7 @@ static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFA
    #endif
    }
 
-   if (!initialized || (!seenCling && gCling)) {
+   if (reset || !initialized || (!seenCling && gCling)) {
       dynpath_full = dynpath_envpart;
       if (!dynpath_full.EndsWith(":")) dynpath_full += ":";
       if (gCling) {
@@ -4678,6 +4709,7 @@ static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFA
          seenCling = kTRUE;
       }
       dynpath_full += dynpath_syspart;
+      dynpath_full += addedPaths;  // entries carry a leading ':'
       initialized = kTRUE;
 
       if (gDebug > 0) std::cout << "dynpath = " << dynpath_full.Data() << std::endl;
@@ -4692,10 +4724,7 @@ static const char *DynamicPath(const char *newpath = nullptr, Bool_t reset = kFA
 void TUnixSystem::AddDynamicPath(const char *path)
 {
    if (path) {
-      TString oldpath = DynamicPath(nullptr, kFALSE);
-      oldpath.Append(":");
-      oldpath.Append(path);
-      DynamicPath(oldpath);
+      DynamicPath(nullptr, kFALSE, path);
    }
 }
 

core/winnt/src/TWinNTSystem.cxx

@@ -356,17 +356,45 @@ namespace {
 
    /////////////////////////////////////////////////////////////////////////////
    /// Get shared library search path.
-
-   static const char *DynamicPath(const char *newpath = 0, Bool_t reset = kFALSE)
+   /// The path is assembled from the ROOT_LIBRARY_PATH environment variable,
+   /// the PATH environment variable, the `Root.DynamicPath` resource of
+   /// .rootrc and the ROOT library directory. Directories can be appended via
+   /// `addpath` (used by TWinNTSystem::AddDynamicPath). The path may first be
+   /// assembled before gEnv is available (TWinNTSystem::Init() adds the
+   /// directory of libCore.dll); in that case it is re-assembled - keeping the
+   /// appended directories - as soon as gEnv exists, so that the
+   /// `Root.DynamicPath` resource is taken into account.
+   /// A path set explicitly through `newpath` (TWinNTSystem::SetDynamicPath)
+   /// is used verbatim and is never amended.
+
+   static const char *DynamicPath(const char *newpath = 0, Bool_t reset = kFALSE,
+                                  const char *addpath = 0)
    {
       static TString dynpath;
+      static TString addedPaths;        // directories appended via `addpath`
+      static Bool_t userpath = kFALSE; // dynpath explicitly set via SetDynamicPath()
+      static Bool_t sawEnv = kFALSE;   // dynpath was assembled with gEnv available
 
-      if (reset || newpath) {
+      if (reset) {
          dynpath = "";
+         addedPaths = "";
+         userpath = kFALSE;
+         sawEnv = kFALSE;
       }
       if (newpath) {
          dynpath = newpath;
-      } else if (dynpath == "") {
+         userpath = kTRUE;
+      } else if (addpath && *addpath) {
+         if (!dynpath.IsNull()) {
+            dynpath += ";"; dynpath += addpath;
+         }
+         addedPaths += ";"; addedPaths += addpath;
+      }
+      if (!userpath && (dynpath.IsNull() || (!sawEnv && gEnv))) {
+         // (Re)assemble the path. A path assembled while gEnv was not yet
+         // available misses the Root.DynamicPath resource, so it is rebuilt
+         // here once gEnv exists.
+         sawEnv = (gEnv != nullptr);
          dynpath = gSystem->Getenv("ROOT_LIBRARY_PATH");
          TString rdynpath = gEnv ? gEnv->GetValue("Root.DynamicPath", (char*)0) : "";
          rdynpath.ReplaceAll("; ", ";");  // in case DynamicPath was extended
@@ -384,9 +412,10 @@ namespace {
                dynpath += ";";
             dynpath += rdynpath;
          }
-      }
-      if (!dynpath.Contains(TROOT::GetLibDir())) {
-         dynpath += ";"; dynpath += TROOT::GetLibDir();
+         if (!dynpath.Contains(TROOT::GetLibDir())) {
+            dynpath += ";"; dynpath += TROOT::GetLibDir();
+         }
+         dynpath += addedPaths;  // entries carry a leading ';'
       }
 
       return dynpath;
@@ -4073,10 +4102,7 @@ Int_t TWinNTSystem::RedirectOutput(const char *file, const char *mode,
 void TWinNTSystem::AddDynamicPath(const char *dir)
 {
    if (dir) {
-      TString oldpath = DynamicPath(0, kFALSE);
-      oldpath.Append(";");
-      oldpath.Append(dir);
-      DynamicPath(oldpath);
+      DynamicPath(0, kFALSE, dir);
    }
 }
 

roottest/root/core/dynamicpath/CMakeLists.txt

@@ -0,0 +1,13 @@
+# Check the way the dynamic (shared library) search path is assembled from
+# ROOT_LIBRARY_PATH, the system loader path and the .rootrc resource.
+#
+# This used to be implemented by the shell script `test_dynpath_setup.sh`; it
+# is now driven by the CMake script `dynamicPathSetup.cmake` so that the test
+# also runs on Windows.
+
+ROOTTEST_ADD_TEST(DynPathSetup
+                  COMMAND ${CMAKE_COMMAND}
+                          -DROOT_EXE=${ROOT_root_CMD}
+                          -DWORKDIR=${CMAKE_CURRENT_BINARY_DIR}/rootlibpath_test
+                          -P ${CMAKE_CURRENT_SOURCE_DIR}/dynamicPathSetup.cmake
+                  ENVIRONMENT ROOTENV_NO_HOME=1)