[ign-common3] Miscellaneous Cleanups

examples/CMakeLists.txt

@@ -1,17 +1,18 @@
 cmake_minimum_required(VERSION 2.8 FATAL_ERROR)
 
 # Find the ignition-common library
-find_package(ignition-common3 QUIET REQUIRED COMPONENTS events profiler)
+set(IGN_COMMON_VER 3)
+find_package(ignition-common${IGN_COMMON_VER} QUIET REQUIRED COMPONENTS events profiler)
 
 add_executable(assert_example assert_example.cc)
-target_link_libraries(assert_example ignition-common3::core)
+target_link_libraries(assert_example ignition-common${IGN_COMMON_VER}::core)
 
 add_executable(console_example console.cc)
-target_link_libraries(console_example ignition-common3::core)
+target_link_libraries(console_example ignition-common${IGN_COMMON_VER}::core)
 
 add_executable(events_example events.cc)
-target_link_libraries(events_example ignition-common3::events)
+target_link_libraries(events_example ignition-common${IGN_COMMON_VER}::events)
 
 add_executable(profiler_example profiler.cc)
-target_link_libraries(profiler_example ignition-common3::profiler)
+target_link_libraries(profiler_example ignition-common${IGN_COMMON_VER}::profiler)
 target_compile_definitions(profiler_example PUBLIC "IGN_PROFILER_ENABLE=1")

include/ignition/common/StringUtils.hh

@@ -33,6 +33,30 @@ namespace ignition
     std::vector<std::string> IGNITION_COMMON_VISIBLE Split(
         const std::string &_orig, char _delim);
 
+    /// \brief Join a sequence of strings with a delimiter
+    ///
+    /// Note that this will skip any empty entries in the vector,
+    /// and will also not prepend or append the delimiter to the
+    /// final output string
+    ///
+    /// \param[in] _orig The input sequence of strings
+    /// \param[in] _delim a string delimiter to join the string with
+    /// \returns a single string composed of strings joined with the delimiter
+    std::string IGNITION_COMMON_VISIBLE Join(
+        const std::vector<std::string> &_orig, const std::string &_delim);
+
+    /// \brief Join a sequence of strings with a delimiter
+    ///
+    /// Note that this will skip any empty entries in the vector,
+    /// and will also not prepend or append the delimiter to the
+    /// final output string
+    ///
+    /// \param[in] _orig The input sequence of strings
+    /// \param[in] _delim a character to join the string with
+    /// \returns a single string composed of strings joined with the delimiter
+    std::string IGNITION_COMMON_VISIBLE Join(
+        const std::vector<std::string> &_orig, char _delim);
+
     /// \brief return true if string starts with another string
     /// \param[in] _s1 the string to check
     /// \param[in] _s2 the possible prefix

src/Filesystem_TEST.cc

@@ -25,19 +25,17 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#include "ignition/common/Util.hh"
+
 // The symlink tests should always work on UNIX systems
 #define BUILD_SYMLINK_TESTS
 
 /////////////////////////////////////////////////
 bool create_and_switch_to_temp_dir(std::string &_new_temp_path)
 {
   std::string tmppath;
-  const char *tmp = getenv("TMPDIR");
-  if (tmp)
-  {
-    tmppath = std::string(tmp);
-  }
-  else
+
+  if (!ignition::common::env("TMPDIR", tmppath))
   {
     tmppath = std::string("/tmp");
   }

src/StringUtils.cc

@@ -38,6 +38,30 @@ namespace ignition
       return pieces;
     }
 
+    //////////////////////////////////////////////////
+    std::string Join(const std::vector<std::string> &_orig,
+                     const std::string &_delim)
+    {
+      std::string ret = "";
+      for (size_t ii = 0; ii < _orig.size(); ++ii)
+      {
+        if (_orig[ii].empty())
+          continue;
+
+        ret += _orig[ii];
+        if (ii < _orig.size() - 1)
+        {
+          ret += _delim;
+        }
+      }
+      return ret;
+    }
+    //////////////////////////////////////////////////
+    std::string Join(const std::vector<std::string> &_orig, char _delim)
+    {
+      return Join(_orig, std::string(1, _delim));
+    }
+
     //////////////////////////////////////////////////
     bool StartsWith(const std::string &_s1, const std::string &_s2)
     {

src/StringUtils_TEST.cc

@@ -70,6 +70,59 @@ TEST(StringUtils, SplitOneDelimiterAtEnd)
   EXPECT_EQ("", pieces[1]);
 }
 
+/////////////////////////////////////////////////
+TEST(Join, Simple)
+{
+  {
+    std::string delim = " ";
+    auto pieces = std::vector<std::string>{"Hello", "World"};
+    auto joined = common::Join(pieces, delim);
+    EXPECT_EQ("Hello World", joined);
+  }
+
+  {
+    std::string delim = "x";
+    auto pieces = std::vector<std::string>{"Hello", "World"};
+    auto joined = common::Join(pieces, delim);
+    EXPECT_EQ("HelloxWorld", joined);
+  }
+
+  {
+    std::string delim = "x";
+    auto pieces = std::vector<std::string>{
+      "foo", "bar", "baz", "bing"};
+    auto joined = common::Join(pieces, delim);
+    EXPECT_EQ("fooxbarxbazxbing", joined);
+  }
+}
+
+/////////////////////////////////////////////////
+TEST(Join, EmptyDelim)
+{
+  std::string delim = "";
+  auto pieces = std::vector<std::string>{"Hello", "World"};
+  auto joined = common::Join(pieces, delim);
+  EXPECT_EQ("HelloWorld", joined);
+}
+
+/////////////////////////////////////////////////
+TEST(Join, EmptyPart)
+{
+  std::string delim = " ";
+  auto pieces = std::vector<std::string>{"Hello", "", "World"};
+  auto joined = common::Join(pieces, delim);
+  EXPECT_EQ("Hello World", joined);
+}
+
+/////////////////////////////////////////////////
+TEST(Join, EmptyParts)
+{
+  std::string delim = "x";
+  auto pieces = std::vector<std::string>{};
+  auto joined = common::Join(pieces, delim);
+  EXPECT_EQ("", joined);
+}
+
 /////////////////////////////////////////////////
 TEST(StartsWith, NotInString)
 {

src/SystemPaths_TEST.cc

@@ -24,6 +24,7 @@
 #include <cstdio>
 
 #include "ignition/common/Util.hh"
+#include "ignition/common/StringUtils.hh"
 #include "ignition/common/SystemPaths.hh"
 
 #ifdef _WIN32
@@ -32,24 +33,19 @@
 
 using namespace ignition;
 
+const char kPluginPath[] = "IGN_PLUGIN_PATH";
+const char kFilePath[] = "IGN_FILE_PATH";
+
 class SystemPathsFixture : public ::testing::Test
 {
   // Documentation inherited
   public: virtual void SetUp()
     {
-      this->backupPluginPath = "IGN_PLUGIN_PATH=";
-
-      if (getenv("IGN_PLUGIN_PATH"))
-        this->backupPluginPath += getenv("IGN_PLUGIN_PATH");
-
-      putenv(const_cast<char*>("IGN_PLUGIN_PATH="));
-
-      this->backupFilePath = "IGN_FILE_PATH=";
+      common::env(kPluginPath, this->backupPluginPath);
+      common::unsetenv(kPluginPath);
 
-      if (getenv("IGN_FILE_PATH"))
-        this->backupFilePath += getenv("IGN_FILE_PATH");
-
-      putenv(const_cast<char*>("IGN_FILE_PATH="));
+      common::env(kFilePath, this->backupFilePath);
+      common::unsetenv(kFilePath);
 
 #ifdef _WIN32
       this->filesystemRoot = "C:\\";
@@ -61,8 +57,8 @@ class SystemPathsFixture : public ::testing::Test
   // Documentation inherited
   public: virtual void TearDown()
     {
-      putenv(const_cast<char*>(this->backupPluginPath.c_str()));
-      putenv(const_cast<char*>(this->backupFilePath.c_str()));
+      common::setenv(kPluginPath, this->backupPluginPath);
+      common::setenv(kFilePath, this->backupFilePath);
     }
 
   /// \brief Backup of plugin paths to be restored after the test
@@ -75,21 +71,18 @@ class SystemPathsFixture : public ::testing::Test
   public: std::string filesystemRoot;
 };
 
+std::string SystemPathsJoin(const std::vector<std::string> &_paths)
+{
+  return common::Join(_paths, common::SystemPaths::Delimiter());
+}
+
 /////////////////////////////////////////////////
 TEST_F(SystemPathsFixture, SystemPaths)
 {
-  common::SystemPaths paths;
-  std::string env_str("IGN_PLUGIN_PATH=/tmp/plugin");
-  env_str += ignition::common::SystemPaths::Delimiter();
-  env_str += "/test/plugin/now";
-
-  // The char* passed to putenv will continue to be stored after the lifetime
-  // of this function, so we should not pass it a pointer which has an automatic
-  // lifetime.
-  static char env[1024];
-  snprintf(env, sizeof(env), "%s", env_str.c_str());
-  putenv(env);
+  common::setenv(kPluginPath,
+      SystemPathsJoin({"/tmp/plugin", "/test/plugin/now"}));
 
+  common::SystemPaths paths;
   const std::list<std::string> pathList3 = paths.PluginPaths();
   EXPECT_EQ(static_cast<unsigned int>(2), pathList3.size());
   EXPECT_STREQ("/tmp/plugin/", pathList3.front().c_str());
@@ -103,19 +96,12 @@ TEST_F(SystemPathsFixture, SystemPaths)
 /////////////////////////////////////////////////
 TEST_F(SystemPathsFixture, FileSystemPaths)
 {
-  std::string env_str("IGN_FILE_PATH=/tmp/file");
-  env_str += ignition::common::SystemPaths::Delimiter();
-  env_str += "/test/file/now";
-
-  // The char* passed to putenv will continue to be stored after the lifetime
-  // of this function, so we should not pass it a pointer which has an automatic
-  // lifetime.
-  static char env[1024];
-  snprintf(env, sizeof(env), "%s", env_str.c_str());
-  putenv(env);
+  std::vector<std::string> filePaths {"/tmp/file",
+                                      "/test/file/now"};
+  common::setenv(kFilePath, SystemPathsJoin(filePaths));
 
   common::SystemPaths paths;
-  EXPECT_EQ("IGN_FILE_PATH", paths.FilePathEnv());
+  EXPECT_EQ(kFilePath, paths.FilePathEnv());
   const std::list<std::string> pathList3 = paths.FilePaths();
   EXPECT_EQ(static_cast<unsigned int>(2), pathList3.size());
   EXPECT_STREQ("/tmp/file/", pathList3.front().c_str());
@@ -136,12 +122,10 @@ TEST_F(SystemPathsFixture, FileSystemPaths)
 
   EXPECT_EQ("", paths.FindFile("no_such_file"));
   EXPECT_EQ("", paths.FindFile("test_f1"));
-  env_str += ignition::common::SystemPaths::Delimiter();
-  env_str += "test_dir1";
 
-  snprintf(env, sizeof(env), "%s", env_str.c_str());
-  putenv(env);
-  paths.SetFilePathEnv("IGN_FILE_PATH");
+  filePaths.push_back("test_dir1");
+  common::setenv(kFilePath, SystemPathsJoin(filePaths));
+  paths.SetFilePathEnv(kFilePath);
   EXPECT_EQ(file1, paths.FindFile("test_f1")) << paths.FindFile("test_f1");
   EXPECT_EQ(file1, paths.FindFile("model://test_f1"));
 }
@@ -285,19 +269,14 @@ TEST_F(SystemPathsFixture, FindFileURI)
   EXPECT_EQ(file2, sp.FindFileURI("osrf://test_f2"));
 
   // URI + env var
-  static char env[1024];
-  snprintf(env, sizeof(env), "%s", ("IGN_FILE_PATH=" + dir1).c_str());
-  putenv(env);
-
-  sp.SetFilePathEnv("IGN_FILE_PATH");
-  EXPECT_EQ("IGN_FILE_PATH", sp.FilePathEnv());
+  common::setenv(kFilePath, dir1);
+  sp.SetFilePathEnv(kFilePath);
+  EXPECT_EQ(kFilePath, sp.FilePathEnv());
   EXPECT_EQ(file1, sp.FindFileURI("anything://test_f1"));
   EXPECT_NE(file2, sp.FindFileURI("anything://test_f2"));
 
   std::string newEnv{"IGN_NEW_FILE_PATH"};
-  snprintf(env, sizeof(env), "%s", (newEnv + "=" + dir2).c_str());
-  putenv(env);
-
+  common::setenv(newEnv, dir2);
   sp.SetFilePathEnv(newEnv);
   EXPECT_EQ(newEnv, sp.FilePathEnv());
   EXPECT_NE(file1, sp.FindFileURI("anything://test_f1"));
@@ -422,20 +401,10 @@ TEST_F(SystemPathsFixture, NormalizeDirectoryPath)
 //////////////////////////////////////////////////
 TEST_F(SystemPathsFixture, PathsFromEnv)
 {
-  std::string env_str = "IGN_PLUGIN_PATH=/tmp/plugin";
-  env_str += ignition::common::SystemPaths::Delimiter();
-  env_str += "/test/plugin/now/";
-  env_str += ignition::common::SystemPaths::Delimiter();
-  env_str += "/tmp/plugin";
-
-  // The char* passed to putenv will continue to be stored after the lifetime
-  // of this function, so we should not pass it a pointer which has an automatic
-  // lifetime.
-  static char env[1024];
-  snprintf(env, sizeof(env), "%s", env_str.c_str());
-  putenv(env);
-
-  auto paths = ignition::common::SystemPaths::PathsFromEnv("IGN_PLUGIN_PATH");
+  common::setenv(kPluginPath,
+      SystemPathsJoin({"/tmp/plugin", "/test/plugin/now/", "/tmp/plugin"}));
+
+  auto paths = ignition::common::SystemPaths::PathsFromEnv(kPluginPath);
 
   EXPECT_EQ(paths.size(), 2u);