Ardent patch1 release (#98)

* remove references to plugin_tool from CMakeLists (#93)

* move pluginlib in its own folder (port 83 to ros2 branch) (#95)

* move pluginlib in its own folder (port 83 to ros2 branch)

* fix most linter errors

* forward port of #88

Continue loading classes on error

* vs2015 doesnt support __has_include, VS2015 and 2017 have both <files… (#96)

* windows contains older version of std filesystem

* either <experimental/filesystem> or <filesystem> will work ... use experimental to be namespace-consistent

* vs2015 doesnt support __has_include, VS2015 and 2017 have both <filesystem> and <experimental/filesystem> but use std::experimental::filesystem in both cases

* adjust library search to work on windows, warn about lib prefix (#97)

CMakeLists.txt → pluginlib/CMakeLists.txt

@@ -19,18 +19,6 @@ find_package(TinyXML2 REQUIRED)  # provided by tinyxml2 upstream, or tinyxml2_ve
 ament_export_dependencies(ament_index_cpp class_loader tinyxml2_vendor TinyXML2)
 ament_export_include_directories(include)
 
-# target_include_directories(plugin_tool PUBLIC
-#   include
-#   ${ament_index_cpp_INCLUDE_DIRS}
-#   ${class_loader_INCLUDE_DIRS}
-#   ${tinyxml2_INCLUDE_DIRS}
-# )
-# target_link_libraries(plugin_tool
-#   ${ament_index_cpp_LIBRARIES}
-#   ${class_loader_LIBRARIES}
-#   ${tinyxml2_LIBRARIES}
-# )
-
 install(
   DIRECTORY cmake
   DESTINATION share/${PROJECT_NAME}
@@ -70,7 +58,7 @@ install(
 #   endif()
 
 #   include(CheckCXXCompilerFlag)
-#   CHECK_CXX_COMPILER_FLAG("-std=c++11" COMPILER_SUPPORTS_CXX11)
+#   check_cxx_compiler_flag("-std=c++11" COMPILER_SUPPORTS_CXX11)
 #   if(COMPILER_SUPPORTS_CXX11)
 #     catkin_add_gtest(${PROJECT_NAME}_unique_ptr_test test/unique_ptr_test.cpp)
 #     if(TARGET ${PROJECT_NAME}_unique_ptr_test)

cmake/pluginlib_export_plugin_description_file.cmake → pluginlib/cmake/pluginlib_export_plugin_description_file.cmake

@@ -69,7 +69,7 @@ set(__PLUGINLIB_PLUGIN_CATEGORIES "")
 #   pluginlib_export_plugin_description_file(rviz "plugin_description.xml")
 #
 # This macro assumes the package name is PROJECT_NAME.
-# 
+#
 # :param plugin_category: category for the type of plugin described; used at
 #   runtime to locate all description files for that kind of plugin
 # :type plugin_category: string

cmake/pluginlib_package_hook.cmake → pluginlib/cmake/pluginlib_package_hook.cmake

@@ -12,7 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# install the ament resource for the plugin descriptions exported by this package
+# install the ament resource for the plugin descriptions exported by
+# this package
 list(REMOVE_DUPLICATES __PLUGINLIB_PLUGIN_CATEGORIES)
 foreach(plugin_category ${__PLUGINLIB_PLUGIN_CATEGORIES})
   # this assumes PROJECT_NAME is the package name

include/pluginlib/class_loader_imp.hpp → pluginlib/include/pluginlib/class_loader_imp.hpp

@@ -316,7 +316,13 @@ std::map<std::string, ClassDesc> ClassLoader<T>::determineAvailableClasses(
   for (std::vector<std::string>::const_iterator it = plugin_xml_paths.begin();
     it != plugin_xml_paths.end(); ++it)
   {
-    processSingleXMLPluginFile(*it, classes_available);
+    try {
+      processSingleXMLPluginFile(*it, classes_available);
+    } catch (const pluginlib::InvalidXMLException & e) {
+      RCUTILS_LOG_ERROR_NAMED("pluginlib.ClassLoader",
+        "Skipped loading plugin with error: %s.",
+        e.what());
+    }
   }
 
   RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader", "Exiting determineAvailableClasses()...");
@@ -370,11 +376,13 @@ std::vector<std::string> ClassLoader<T>::getAllLibraryPathsToTry(
   //
   //   - <prefix for rviz>/lib/librviz_default_plugins.so
   //   - <prefix for rviz>/lib64/librviz_default_plugins.so
+  //   - <prefix for rviz>/bin/rviz_default_plugins.dll
   //   - <prefix for rviz>/lib/rviz/librviz_default_plugins.so
   //   - <prefix for rviz>/lib64/rviz/librviz_default_plugins.so
   //
   // The extension, e.g. `.so`, might be different based on the operating
   // system, e.g. it might be `.dylib` on macOS or `.dll` on Windows.
+  // Similarly, the library might have the `lib` prefix added or removed.
   // Also, the library name might have a `d` added if the library is built
   // debug, depending on the system.
 
@@ -384,39 +392,77 @@ std::vector<std::string> ClassLoader<T>::getAllLibraryPathsToTry(
 
   const std::string path_separator = getPathSeparator();
 
-  std::vector<std::string> all_paths;
+  std::vector<std::string> all_paths;  // result of all pairs to search
+
   std::string package_prefix = ament_index_cpp::get_package_prefix(exporting_package_name);
-  std::vector<std::string> all_paths_without_extension = {
+
+  // Setup the directories to look in.
+  std::vector<std::string> all_search_paths = {
     // for now just try lib and lib64 (and their respective "libexec" directories)
     package_prefix + path_separator + "lib",
     package_prefix + path_separator + "lib64",
+    package_prefix + path_separator + "bin",  // also look in bin, for dll's on Windows
     package_prefix + path_separator + "lib" + path_separator + exporting_package_name,
     package_prefix + path_separator + "lib64" + path_separator + exporting_package_name,
+    package_prefix + path_separator + "bin" + path_separator + exporting_package_name,
   };
+
+  // Prepare to setup the relative file paths.
   bool debug_library_suffix = (0 == class_loader::systemLibrarySuffix().compare(0, 1, "d"));
   std::string non_debug_suffix;
   if (debug_library_suffix) {
     non_debug_suffix = class_loader::systemLibrarySuffix().substr(1);
   } else {
     non_debug_suffix = class_loader::systemLibrarySuffix();
   }
-  std::string library_name_with_extension = library_name + non_debug_suffix;
   std::string stripped_library_name = stripAllButFileFromPath(library_name);
-  std::string stripped_library_name_with_extension = stripped_library_name + non_debug_suffix;
 
-  for (const auto & current_path : all_paths_without_extension) {
-    all_paths.push_back(current_path + path_separator + library_name_with_extension);
-    all_paths.push_back(current_path + path_separator + stripped_library_name_with_extension);
+  std::string library_name_alternative;  // either lib<library> or <library> without lib prefix
+  const char * lib_prefix = "lib";
+  if (library_name.rfind(lib_prefix, 0) == 0) {
+    library_name_alternative = library_name.substr(strlen(lib_prefix));
+    RCUTILS_LOG_WARN_NAMED("pluginlib.ClassLoader",
+      "given plugin name '%s' should be '%s' for better portability",
+      library_name.c_str(),
+      library_name_alternative.c_str());
+  } else {
+    library_name_alternative = lib_prefix + library_name;
+  }
+  std::string stripped_library_name_alternative = stripAllButFileFromPath(library_name_alternative);
+
+  // Setup the relative file paths to pair with the search directories above.
+  std::vector<std::string> all_relative_library_paths = {
+    library_name + non_debug_suffix,
+    library_name_alternative + non_debug_suffix,
+    stripped_library_name + non_debug_suffix,
+    stripped_library_name_alternative + non_debug_suffix,
+  };
+  std::vector<std::string> all_relative_debug_library_paths = {
+    library_name + class_loader::systemLibrarySuffix(),
+    library_name_alternative + class_loader::systemLibrarySuffix(),
+    stripped_library_name + class_loader::systemLibrarySuffix(),
+    stripped_library_name_alternative + class_loader::systemLibrarySuffix(),
+  };
+
+  for (auto && current_search_path : all_search_paths) {
+    for (auto && current_library_path : all_relative_library_paths) {
+      all_paths.push_back(current_search_path + path_separator + current_library_path);
+    }
     // We're in debug mode, try debug libraries as well
     if (debug_library_suffix) {
-      all_paths.push_back(
-        current_path + path_separator + library_name + class_loader::systemLibrarySuffix());
-      all_paths.push_back(
-        current_path + path_separator + stripped_library_name +
-        class_loader::systemLibrarySuffix());
+      for (auto && current_library_path : all_relative_debug_library_paths) {
+        all_paths.push_back(current_search_path + path_separator + current_library_path);
+      }
     }
   }
 
+  for (auto && path : all_paths) {
+    RCUTILS_LOG_DEBUG_NAMED("pluginlib.ClassLoader",
+      "[search path for '%s']: '%s'",
+      library_name.c_str(),
+      path.c_str());
+  }
+
   return all_paths;
 }
 
@@ -670,15 +716,15 @@ void ClassLoader<T>::processSingleXMLPluginFile(
   tinyxml2::XMLElement * config = document.RootElement();
   if (NULL == config) {
     throw pluginlib::InvalidXMLException(
-            "XML Document has no Root Element. "
-            "This likely means the XML is malformed or missing.");
+            "XML Document '" + xml_file +
+            "' has no Root Element. This likely means the XML is malformed or missing.");
     return;
   }
   if (!(strcmp(config->Value(), "library") == 0 ||
     strcmp(config->Value(), "class_libraries") == 0))
   {
     throw pluginlib::InvalidXMLException(
-            "The XML document given to add must have either \"library\" or "
+            "The XML document '" + xml_file + "' given to add must have either \"library\" or "
             "\"class_libraries\" as the root tag");
     return;
   }

include/pluginlib/exceptions.hpp → pluginlib/include/pluginlib/exceptions.hpp

@@ -47,7 +47,7 @@ class PluginlibException : public std::runtime_error
   : std::runtime_error(error_desc) {}
 };
 
-/// An exception class thrown when pluginlib is unable to load a plugin XML file.
+/// Thrown when pluginlib is unable to load a plugin XML file.
 /**
  * \class InvalidXMLException
  */
@@ -58,7 +58,7 @@ class InvalidXMLException : public PluginlibException
   : PluginlibException(error_desc) {}
 };
 
-/// An exception class thrown when pluginlib is unable to load the library associated with a given plugin.
+/// Thrown when pluginlib is unable to load the library associated with a given plugin.
 /**
  * \class LibraryLoadException
  */
@@ -69,7 +69,7 @@ class LibraryLoadException : public PluginlibException
   : PluginlibException(error_desc) {}
 };
 
-/// An exception class thrown when pluginlib is unable to instantiate a class loader.
+/// Thrown when pluginlib is unable to instantiate a class loader.
 /**
  * \class ClassLoaderException
  */
@@ -80,7 +80,7 @@ class ClassLoaderException : public PluginlibException
   : PluginlibException(error_desc) {}
 };
 
-/// An exception class thrown when pluginlib is unable to unload the library associated with a given plugin.
+/// Thrown when pluginlib is unable to unload the library associated with a given plugin.
 /**
  * \class LibraryUnloadException
  */
@@ -91,7 +91,7 @@ class LibraryUnloadException : public PluginlibException
   : PluginlibException(error_desc) {}
 };
 
-/// An exception class thrown when pluginlib is unable to create the class associated with a given plugin.
+/// Thrown when pluginlib is unable to create the class associated with a given plugin.
 /**
  * \class CreateClassException
  */

include/pluginlib/impl/filesystem_helper.hpp → pluginlib/include/pluginlib/impl/filesystem_helper.hpp

@@ -35,9 +35,22 @@
 #ifndef PLUGINLIB__IMPL__FILESYSTEM_HELPER_HPP_
 #define PLUGINLIB__IMPL__FILESYSTEM_HELPER_HPP_
 
-#if defined(__has_include)
-#if __has_include(<filesystem>)
-# include <filesystem>
+
+#if _MSC_VER >= 1900
+# include <experimental/filesystem>
+namespace pluginlib
+{
+namespace impl
+{
+namespace fs = std::experimental::filesystem;
+}  // namespace impl
+}  // namespace pluginlib
+
+# define PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
+
+#elif defined(__has_include)
+# if __has_include(<filesystem>)
+#  include <filesystem>
 
 namespace pluginlib
 {
@@ -47,9 +60,9 @@ namespace fs = std::filesystem;
 }  // namespace impl
 }  // namespace pluginlib
 
-#define PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
-#elif __has_include(<experimental/filesystem>)
-# include <experimental/filesystem>
+#  define PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
+# elif __has_include(<experimental/filesystem>)
+#  include <experimental/filesystem>
 
 namespace pluginlib
 {
@@ -59,14 +72,15 @@ namespace fs = std::experimental::filesystem;
 }  // namespace impl
 }  // namespace pluginlib
 
-#define PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
-#endif
+#  define PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
+# endif
 #endif
 
 // The standard library does not provide it, so emulate it.
 #ifndef PLUGINLIB__IMPL__FILESYSYEM_HELPER__HAS_STD_FILESYSTEM
 
 #include <string>
+#include <vector>
 
 #ifdef _WIN32
 #define CLASS_LOADER_IMPL_OS_DIRSEP '\\'

include/pluginlib/impl/split.hpp → pluginlib/include/pluginlib/impl/split.hpp

@@ -40,7 +40,8 @@ namespace impl
 {
 
 inline std::vector<std::string>
-split(const std::string & input, const std::string & regex) {
+split(const std::string & input, const std::string & regex)
+{
   std::regex re(regex);
   // the -1 will cause this to return the stuff between the matches, see the submatch argument:
   //   http://en.cppreference.com/w/cpp/regex/regex_token_iterator/regex_token_iterator

test/test_plugins.cpp → pluginlib/test/test_plugins.cpp

@@ -29,7 +29,7 @@
 
 #include <pluginlib/class_list_macros.hpp>
 #include "./test_base.h"
-#include "./test_plugins.h"
+#include "test_plugins.h"  // NOLINT
 
 PLUGINLIB_EXPORT_CLASS(test_plugins::Foo, test_base::Fubar)
 PLUGINLIB_EXPORT_CLASS(test_plugins::Bar, test_base::Fubar)

test/utest.cpp → pluginlib/test/utest.cpp

@@ -29,7 +29,8 @@
 
 #include <gtest/gtest.h>
 
-#include <pluginlib/class_loader.hpp>
+#include <memory>
+#include <pluginlib/class_loader.hpp>  // NOLINT
 
 #include "./test_base.h"
 
@@ -123,7 +124,8 @@ TEST(PluginlibTest, brokenXML) {
   try {
     pluginlib::ClassLoader<test_base::Fubar> test_loader("pluginlib", "test_base::Fubar",
       "plugin_test");
-  } catch (pluginlib::InvalidXMLException & ex) {
+    test_loader.createInstance("pluginlib/foo");
+  } catch (pluginlib::PluginlibException & ex) {
     SUCCEED();
     return;
   }