Fix working with filesystem parent paths.

include/rcpputils/filesystem_helper.hpp

@@ -32,9 +32,8 @@
 /*! \file filesystem_helper.hpp
  * \brief Cross-platform filesystem helper functions and additional emulation of [std::filesystem](https://en.cppreference.com/w/cpp/filesystem).
  *
- * If std::filesystem is not available the necessary functions are emulated.
  * Note: Once std::filesystem is supported on all ROS2 platforms, this class
- * can be deprecated in favor of the built-in functionality.
+ * can be deprecated/removed in favor of the built-in functionality.
  */
 
 #ifndef RCPPUTILS__FILESYSTEM_HELPER_HPP_
@@ -104,10 +103,11 @@ class path
    * \param p A string path split by the platform's string path separator.
    */
   path(const std::string & p)  // NOLINT(runtime/explicit): this is a conversion constructor
-  : path_(p), path_as_vector_(split(p, kPreferredSeparator))
+  : path_(p)
   {
     std::replace(path_.begin(), path_.end(), '\\', kPreferredSeparator);
     std::replace(path_.begin(), path_.end(), '/', kPreferredSeparator);
+    path_as_vector_ = split(path_, kPreferredSeparator);
   }
 
   /**
@@ -220,7 +220,7 @@ class path
   bool is_absolute() const
   {
     return path_.size() > 0 &&
-           (path_.compare(0, 1, std::string(1, kPreferredSeparator)) == 0 ||
+           (path_[0] == kPreferredSeparator ||
            this->is_absolute_with_drive_letter());
   }
 
@@ -277,10 +277,13 @@ class path
 
     path parent;
     for (auto it = this->cbegin(); it != --this->cend(); ++it) {
-      if (!parent.empty() || it->empty()) {
-        parent /= *it;
-      } else {
+      if (parent.empty() && (!this->is_absolute() || this->is_absolute_with_drive_letter())) {
+        // This handles the case where we are dealing with a relative path or
+        // the Windows drive letter; in both cases we don't want a separator at
+        // the beginning, so just copy the piece directly.
         parent = *it;
+      } else {
+        parent /= *it;
       }
     }
     return parent;
@@ -356,7 +359,13 @@ class path
       this->path_ = other.path_;
       this->path_as_vector_ = other.path_as_vector_;
     } else {
-      this->path_ += kPreferredSeparator + other.string();
+      if (this->path_[this->path_.length() - 1] != kPreferredSeparator) {
+        // This ensures that we don't put duplicate separators into the path;
+        // this can happen, for instance, on absolute paths where the first
+        // item in the vector is the empty string.
+        this->path_ += kPreferredSeparator;
+      }
+      this->path_ += other.string();
       this->path_as_vector_.insert(
         std::end(this->path_as_vector_),
         std::begin(other.path_as_vector_), std::end(other.path_as_vector_));

test/test_filesystem_helper.cpp

@@ -392,3 +392,36 @@ TEST(TestFilesystemHelper, get_cwd)
   auto p = rcpputils::fs::current_path();
   EXPECT_EQ(expected_dir, p.string());
 }
+
+TEST(TestFilesystemHelper, parent_absolute_path)
+{
+  rcpputils::fs::path path("/home/foo/bar/baz");
+  if (is_win32) {
+    ASSERT_EQ(path.string(), "\\home\\foo\\bar\\baz");
+  } else {
+    ASSERT_EQ(path.string(), "/home/foo/bar/baz");
+  }
+
+  rcpputils::fs::path parent = path.parent_path();
+  if (is_win32) {
+    ASSERT_EQ(parent.string(), "\\home\\foo\\bar");
+  } else {
+    ASSERT_EQ(parent.string(), "/home/foo/bar");
+  }
+
+  rcpputils::fs::path grandparent = parent.parent_path();
+  if (is_win32) {
+    ASSERT_EQ(grandparent.string(), "\\home\\foo");
+  } else {
+    ASSERT_EQ(grandparent.string(), "/home/foo");
+  }
+
+  if (is_win32) {
+    rcpputils::fs::path win_drive_letter("C:\\home\\foo\\bar");
+    ASSERT_EQ(win_drive_letter.string(), "C:\\home\\foo\\bar");
+    rcpputils::fs::path win_parent = win_drive_letter.parent_path();
+    ASSERT_EQ(win_parent.string(), "C:\\home\\foo");
+    rcpputils::fs::path win_grandparent = win_parent.parent_path();
+    ASSERT_EQ(win_grandparent.string(), "C:\\home");
+  }
+}