diff --git a/include/rcpputils/filesystem_helper.hpp b/include/rcpputils/filesystem_helper.hpp index 68bb080d..8877df1e 100644 --- a/include/rcpputils/filesystem_helper.hpp +++ b/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_)); diff --git a/test/test_filesystem_helper.cpp b/test/test_filesystem_helper.cpp index a83047f0..8f4c7820 100644 --- a/test/test_filesystem_helper.cpp +++ b/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"); + } +}