Refactor LibraryManager to allow more control over library exclusion.

Now the set of library names to be excluded is a first class parameter
to the method and can be controlled externally.
Refs #0

Framework/Kernel/CMakeLists.txt

@@ -559,18 +559,18 @@ if ( ${CMAKE_PROJECT_NAME} STREQUAL "MantidFramework" )
   set ( MANTID_ROOT ${CMAKE_SOURCE_DIR}/.. )
 endif ()
 
-set ( PLUGINS "." )
-set ( QTPLUGINS "." )
+set ( FRAMEWORK_PLUGINS_DIR "." )
+set ( QT_PLUGINS_DIR "." )
 # %V will be replaced with the major version of Qt at runtime
 if ( MAKE_VATES )
-  set ( PV_PLUGINS "./plugins/paraview/qt%V" )
+  set ( PV_PLUGINS_DIR "./plugins/paraview/qt%V" )
   if ( MSVC )
     set (PARAVIEW_PYTHON_PATHS "${ParaView_DIR}/bin/$<$<CONFIG:Release>:Release>$<$<CONFIG:Debug>:Debug>;${ParaView_DIR}/lib/$<$<CONFIG:Release>:Release>$<$<CONFIG:Debug>:Debug>;${ParaView_DIR}/lib/site-packages;${ParaView_DIR}/lib/site-packages/vtk")
   else()
     set ( PARAVIEW_PYTHON_PATHS "${ParaView_DIR}/lib;${ParaView_DIR}/lib/site-packages;${ParaView_DIR}/lib/site-packages/vtk")
   endif()
 else ()
-  set ( PV_PLUGINS "" )
+  set ( PV_PLUGINS_DIR "" )
   set ( PARAVIEW_PYTHON_PATHS "")
 endif ()
 set ( IGNORE_PARAVIEW "0" )
@@ -618,7 +618,7 @@ endforeach()
 # These settings carry down to the installation configuration below
 if ( MPI_BUILD )
   set ( CONSOLEPATTERN "%H:%M:%S,%i [%N:%P] %p %s - %t")
-  set ( PV_PLUGINS "" )
+  set ( PV_PLUGINS_DIR "" )
   set ( IGNORE_PARAVIEW "1" )
 else ()
   option ( ENABLE_FILE_LOGGING "Enable logging to file for development builds. It is always enabled for packages" ON )
@@ -671,7 +671,7 @@ if ( NOT MPI_BUILD )
   set ( FILELOGGER_CHANNEL fileFilterChannel )
 endif ()
 set ( ENABLE_WEB_UPDATES 1 )
-set ( PLUGINS ${MANTID_ROOT}/plugins )
+set ( FRAMEWORK_PLUGINS_DIR ${MANTID_ROOT}/plugins )
 set ( PYTHONPLUGIN_DIRS "${PLUGINS}/python" )
 if(MAKE_VATES)
   if (APPLE)
@@ -723,13 +723,13 @@ set ( PYTHONSCRIPT_DIRS "${WITH_SEMICOLONS}" )
 # For a framework-only (e.g. MPI) build some of these are not relevant and should
 # be left empty to avoid warnings on starting Mantid
 if ( ${CMAKE_PROJECT_NAME} MATCHES "MantidFramework" )
-  set ( QTPLUGINS "" )
+  set ( QT_PLUGINS_DIR "" )
   set ( COLORMAPS_FOLDER "" )
-  set ( PV_PLUGINS "" )
+  set ( PV_PLUGINS_DIR "" )
   set ( IGNORE_PARAVIEW "1" )
 else ()
-  set ( QTPLUGINS "${MANTID_ROOT}/plugins/qt%V" )
-  set ( PV_PLUGINS "${MANTID_ROOT}/${PLUGINS_DIR}/${PVPLUGINS_SUBDIR}/qt%V" )
+  set ( QT_PLUGINS_DIR "${MANTID_ROOT}/plugins/qt%V" )
+  set ( PV_PLUGINS_DIR "${MANTID_ROOT}/${PLUGINS_DIR}/${PVPLUGINS_SUBDIR}/qt%V" )
   set ( COLORMAPS_FOLDER ${MANTID_ROOT}/colormaps )
 endif ()
 

Framework/Kernel/inc/MantidKernel/LibraryManager.h

@@ -5,18 +5,19 @@
 // Includes
 //----------------------------------------------------------------------
 #include <string>
-#include <map>
-#ifndef Q_MOC_RUN
-#include <boost/shared_ptr.hpp>
-#endif
+#include <unordered_map>
 
-#include "MantidKernel/SingletonHolder.h"
 #include "MantidKernel/DllConfig.h"
+#include "MantidKernel/LibraryWrapper.h"
+#include "MantidKernel/SingletonHolder.h"
+
+namespace Poco {
+class File;
+class Path;
+}
 
 namespace Mantid {
 namespace Kernel {
-class LibraryWrapper;
-
 /**
 Class for opening shared libraries.
 
@@ -46,8 +47,9 @@ Code Documentation is available at: <http://doxygen.mantidproject.org>
 */
 class MANTID_KERNEL_DLL LibraryManagerImpl {
 public:
-  // opens all suitable libraries on a given path
-  int OpenAllLibraries(const std::string &, bool isRecursive = false);
+  enum LoadLibraries { Recursive, NonRecursive };
+  int openLibraries(const std::string &, LoadLibraries loadingBehaviour,
+                    const std::vector<std::string> &excludes);
   LibraryManagerImpl(const LibraryManagerImpl &) = delete;
   LibraryManagerImpl &operator=(const LibraryManagerImpl &) = delete;
 
@@ -56,17 +58,20 @@ private:
 
   /// Private Constructor
   LibraryManagerImpl();
-
   /// Private Destructor
-  virtual ~LibraryManagerImpl() = default;
+  ~LibraryManagerImpl() = default;
 
+  /// Load libraries from the given Poco::File path
+  /// Private so Poco::File doesn't leak to the public interface
+  int openLibraries(const Poco::File &, LoadLibraries loadingBehaviour,
+                    const std::vector<std::string> &excludes);
   /// Load a given library
-  bool loadLibrary(const std::string &filepath);
+  bool loadLibrary(Poco::Path filepath);
   /// Returns true if the library is to be loaded
-  bool skip(const std::string &filename);
+  bool skipLibrary(const std::string &filename,
+                   const std::vector<std::string> &excludes);
   /// Storage for the LibraryWrappers.
-  std::map<const std::string, boost::shared_ptr<Mantid::Kernel::LibraryWrapper>>
-      OpenLibs;
+  std::unordered_map<std::string, LibraryWrapper> m_openedLibs;
 };
 
 EXTERN_MANTID_KERNEL template class MANTID_KERNEL_DLL

Framework/Kernel/src/ConfigService.cpp

@@ -194,7 +194,7 @@ ConfigServiceImpl::ConfigServiceImpl()
   // Fill the list of possible relative path keys that may require conversion to
   // absolute paths
   m_ConfigPaths.emplace("mantidqt.python_interfaces_directory", true);
-  m_ConfigPaths.emplace("plugins.directory", true);
+  m_ConfigPaths.emplace("framework.plugins.directory", true);
   m_ConfigPaths.emplace("pvplugins.directory", false);
   m_ConfigPaths.emplace("mantidqt.plugins.directory", false);
   m_ConfigPaths.emplace("instrumentDefinition.directory", true);

Framework/Kernel/src/LibraryManager.cpp

-
 #include "MantidKernel/ConfigService.h"
 #include "MantidKernel/DllOpen.h"
 #include "MantidKernel/LibraryManager.h"
@@ -9,8 +8,6 @@
 #include <Poco/File.h>
 #include <Poco/DirectoryIterator.h>
 #include <boost/algorithm/string.hpp>
-#include <boost/make_shared.hpp>
-#include <unordered_set>
 
 namespace Mantid {
 namespace Kernel {
@@ -20,73 +17,89 @@ Logger g_log("LibraryManager");
 }
 
 /// Constructor
-LibraryManagerImpl::LibraryManagerImpl() {
+LibraryManagerImpl::LibraryManagerImpl() : m_openedLibs() {
   g_log.debug() << "LibraryManager created.\n";
 }
 
-/** Opens all suitable DLLs on a given path.
-*  @param filePath :: The filepath to the directory where the libraries are.
-*  @param isRecursive :: Whether to search subdirectories.
-*  @return The number of libraries opened.
-*/
-int LibraryManagerImpl::OpenAllLibraries(const std::string &filePath,
-                                         bool isRecursive) {
+/**
+ * Opens suitable DLLs on a given path.
+ *  @param filePath The filepath to the directory where the libraries are.
+ *  @param loadingBehaviour Control how libraries are searched for
+ *  @param excludes If not empty then each string is considered as a substring
+ * to search within each library to be opened. If the substring is found then
+ * the library is not opened.
+ *  @return The number of libraries opened.
+ */
+int LibraryManagerImpl::openLibraries(
+    const std::string &filePath, LoadLibraries loadingBehaviour,
+    const std::vector<std::string> &excludes) {
   g_log.debug() << "Opening all libraries in " << filePath << "\n";
-  int libCount = 0;
-  // validate inputs
-  Poco::File libPath;
   try {
-    libPath = Poco::File(filePath);
+    return openLibraries(Poco::File(filePath), loadingBehaviour, excludes);
+  } catch (std::exception &exc) {
+    g_log.debug() << "Error occurred while opening libraries: " << exc.what()
+                  << "\n";
+    return 0;
   } catch (...) {
-    return libCount;
+    g_log.error() << "An unknown error occurred while opening libraries.";
+    return 0;
   }
+}
+
+//-------------------------------------------------------------------------
+// Private members
+//-------------------------------------------------------------------------
+/**
+ * Opens suitable DLLs on a given path.
+ *  @param filePath A Poco::File object pointing to a directory where the
+ * libraries are.
+ *  @param loadingBehaviour Control how libraries are searched for
+ *  @param excludes If not empty then each string is considered as a substring
+ * to search within each library to be opened. If the substring is found then
+ * the library is not opened.
+ *  @return The number of libraries opened.
+ */
+int LibraryManagerImpl::openLibraries(
+    const Poco::File &libPath,
+    LibraryManagerImpl::LoadLibraries loadingBehaviour,
+    const std::vector<std::string> &excludes) {
+  int libCount(0);
   if (libPath.exists() && libPath.isDirectory()) {
-    DllOpen::addSearchDirectory(filePath);
-    // Iteratate over the available files
+    DllOpen::addSearchDirectory(libPath.path());
+    // Iterate over the available files
     Poco::DirectoryIterator end_itr;
     for (Poco::DirectoryIterator itr(libPath); itr != end_itr; ++itr) {
-      const Poco::Path &item = itr.path();
-      if (item.isDirectory()) {
-        if (isRecursive) {
-          libCount += OpenAllLibraries(item.toString());
-        }
-      } else {
-        if (skip(item.toString()))
+      const Poco::File &item = *itr;
+      if (item.isFile()) {
+        if (skipLibrary(itr.path().getFileName(), excludes))
           continue;
-        if (loadLibrary(item.toString())) {
+        if (loadLibrary(itr.path())) {
           ++libCount;
         }
+      } else if (loadingBehaviour == LoadLibraries::Recursive) {
+        // it must be a directory
+        libCount += openLibraries(item, LoadLibraries::Recursive, excludes);
       }
     }
   } else {
-    g_log.error("In OpenAllLibraries: " + filePath + " must be a directory.");
+    g_log.error("In OpenAllLibraries: " + libPath.path() +
+                " must be a directory.");
   }
-
   return libCount;
 }
 
-//-------------------------------------------------------------------------
-// Private members
-//-------------------------------------------------------------------------
 /**
  * Returns true if the name contains one of the strings given in the
- * 'plugins.exclude' variable. Each string from the variable is
+ * exclude list. Each string from the variable is
  * searched for with the filename so an exact match is not necessary. This
  * avoids having to specify prefixes and suffixes for different platforms,
  * i.e. 'plugins.exclude = MantidKernel' will exclude libMantidKernel.so
- * @param filename :: A string giving the filename/file path
+ * @param filename A string giving the filename/file path
+ * @param excludes A list of substrings to exclude library from loading
  * @return True if the library should be skipped
  */
-bool LibraryManagerImpl::skip(const std::string &filename) {
-  static std::unordered_set<std::string> excludes;
-  static bool initialized(false);
-  if (!initialized) {
-    std::string excludeStr =
-        ConfigService::Instance().getString("plugins.exclude");
-    boost::split(excludes, excludeStr, boost::is_any_of(":;"),
-                 boost::token_compress_on);
-    initialized = true;
-  }
+bool LibraryManagerImpl::skipLibrary(const std::string &filename,
+                                     const std::vector<std::string> &excludes) {
   bool skipme(false);
   for (const auto &exclude : excludes) {
     if (filename.find(exclude) != std::string::npos) {
@@ -99,31 +112,26 @@ bool LibraryManagerImpl::skip(const std::string &filename) {
 
 /**
 * Load a library
-* @param filepath :: The full path to a library as a string
+* @param filepath :: A Poco::File The full path to a library as a string
 */
-bool LibraryManagerImpl::loadLibrary(const std::string &filepath) {
+bool LibraryManagerImpl::loadLibrary(Poco::Path filepath) {
   // Get the name of the library.
-  std::string libName =
-      DllOpen::ConvertToLibName(Poco::Path(filepath).getFileName());
+  std::string libName = DllOpen::convertToLibName(filepath.getFileName());
   if (libName.empty())
     return false;
-  // The wrapper will unload the library when it is deleted
-  auto dlwrap = boost::make_shared<LibraryWrapper>();
-  std::string libNameLower = boost::algorithm::to_lower_copy(libName);
-
-  // Check that a libray with this name has not already been loaded
-  if (OpenLibs.find(libNameLower) == OpenLibs.end()) {
-    Poco::Path directory(filepath);
-    directory.makeParent();
-    if (g_log.is(Poco::Message::PRIO_DEBUG)) {
-      g_log.debug() << "Trying to open library: " << libName << " from "
-                    << directory.toString() << " ...";
-    }
-    // Try to open the library
-    if (dlwrap->OpenLibrary(libName, directory.toString())) {
+  // Check that a library with this name has not already been loaded
+  if (m_openedLibs.find(boost::algorithm::to_lower_copy(libName)) ==
+      m_openedLibs.end()) {
+    filepath.makeParent();
+    // Try to open the library. The wrapper will unload the library when it
+    // is deleted
+    LibraryWrapper dlwrap;
+    if (dlwrap.openLibrary(libName, filepath.toString())) {
       // Successfully opened, so add to map
-      g_log.debug("Opened library: " + libName + ".\n");
-      OpenLibs.emplace(libName, dlwrap);
+      if (g_log.is(Poco::Message::PRIO_DEBUG)) {
+        g_log.debug("Opened library: " + libName + ".\n");
+      }
+      m_openedLibs.emplace(libName, std::move(dlwrap));
       return true;
     } else {
       return false;

Framework/Properties/Mantid.properties.template

@@ -22,20 +22,20 @@ Q.convention = Inelastic
 # Set of PyQt interfaces to add to the Interfaces menu, separated by a space.  Interfaces are seperated from their respective categories by a "/".
 mantidqt.python_interfaces = Direct/DGS_Reduction.py Direct/DGSPlanner.py Direct/PyChop.py SANS/ORNL_SANS.py Utility/TofConverter.py Reflectometry/ISIS_Reflectometry_Old.py Diffraction/Powder_Diffraction_Reduction.py Utility/FilterEvents.py Diffraction/HFIR_Powder_Diffraction_Reduction.py Diffraction/HFIR_4Circle_Reduction.py Utility/QECoverage.py SANS/ISIS_SANS_v2_experimental.py Muon/Frequency_Domain_Analysis.py
 
-
+# Directory containing the above startup scripts
 mantidqt.python_interfaces_directory = @MANTID_ROOT@/scripts
 
-# Where to find mantid plugin libraries
-plugins.directory = @PLUGINS@
+# Where to find mantid framework plugins
+framework.plugins.directory = @FRAMEWORK_PLUGINS_DIR@
 
 # Libraries to skip. The strings are searched for when loading libraries so they don't need to be exact
-plugins.exclude = Qt5
+framework.plugins.exclude = Qt4;Qt5
 
 # Where to find mantid paraview plugin libraries
-pvplugins.directory = @PV_PLUGINS@
+pvplugins.directory = @PV_PLUGINS_DIR@
 
 # Where to find Mantid Qt plugin libraries
-mantidqt.plugins.directory = @QTPLUGINS@
+mantidqt.plugins.directory = @QT_PLUGINS_DIR@
 
 # Where to find python plugins
 python.plugins.directories = @PYTHONPLUGIN_DIRS@

docs/source/concepts/PropertiesFile.rst

@@ -87,9 +87,15 @@ Directory Properties
 |                                      | that Mantid requires to function correctly.       |                                     |
 |                                      | **WARNING:** Do not alter the default value.      |                                     |
 +--------------------------------------+---------------------------------------------------+-------------------------------------+
-| ``plugins.directory``                | The path to the directory that contains the       | ``../Plugins``                      |
+| ``framework.plugins.directory``      | The path to the directory that contains the       | ``../plugins``                      |
 |                                      | Mantid plugin libraries                           |                                     |
 +--------------------------------------+---------------------------------------------------+-------------------------------------+
+| ``framework.plugins.exclude``        | A list of substrings to allow libraries to be     | ``Qt4;Qt5``                         |
+|                                      | skipped                                           |                                     |
++--------------------------------------+---------------------------------------------------+-------------------------------------+
+| ``mantidqt.plugins.directory``       | The path to the directory containing the          | ``../plugins/qtX``                  |
+|                                      | Mantid Qt-based plugin libraries                  |                                     |
++--------------------------------------+---------------------------------------------------+-------------------------------------+
 | ``requiredpythonscript.directories`` | A list of directories containing Python scripts   | N/A                                 |
 |                                      | that Mantid requires to function correctly.       |                                     |
 |                                      | **WARNING:** Do not alter the default value.      |                                     |