Use std::filesystem instead of rcpputils::fs (#1576)

* Remove rcpputils::fs dependency from rosbag2_cpp Signed-off-by: Kenta Yonekura <[email protected]> * Remove rcpputils::fs dependency from tests Signed-off-by: Kenta Yonekura <[email protected]> * Replace rcpputils::fs with std::filesystem Signed-off-by: Roman Sokolkov <[email protected]> * Use .generic_string() in tests Signed-off-by: Roman Sokolkov <[email protected]> * Revert remove_all from fixture Signed-off-by: Roman Sokolkov <[email protected]> --------- Signed-off-by: Kenta Yonekura <[email protected]> Signed-off-by: Roman Sokolkov <[email protected]> Co-authored-by: Kenta Yonekura <[email protected]>
ros2 · Mar 25, 2024 · 415ddda · 415ddda
1 parent 3032800
commit 415ddda
Show file tree

Hide file tree

Showing 34 changed files with 333 additions and 291 deletions.
diff --git a/rosbag2_compression/src/rosbag2_compression/sequential_compression_reader.cpp b/rosbag2_compression/src/rosbag2_compression/sequential_compression_reader.cpp
@@ -14,19 +14,21 @@
 
 #include "rosbag2_compression/sequential_compression_reader.hpp"
 
+#include <filesystem>
 #include <memory>
 #include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/compression_options.hpp"
 
 #include "logging.hpp"
 
+namespace fs = std::filesystem;
+
 namespace rosbag2_compression
 {
 SequentialCompressionReader::SequentialCompressionReader(
@@ -67,17 +69,17 @@ void SequentialCompressionReader::preprocess_current_file()
      * Because we have no way to check whether the bag was written correctly,
      * check for the existence of the prefixed file as a fallback.
      */
-    const rcpputils::fs::path base{base_folder_};
-    const rcpputils::fs::path relative{get_current_file()};
+    const fs::path base{base_folder_};
+    const fs::path relative{get_current_file()};
     const auto resolved = base / relative;
-    if (!resolved.exists()) {
+    if (!fs::exists(resolved)) {
       const auto base_stripped = relative.filename();
       const auto resolved_stripped = base / base_stripped;
       ROSBAG2_COMPRESSION_LOG_DEBUG_STREAM(
         "Unable to find specified bagfile " << resolved.string() <<
           ". Falling back to checking for " << resolved_stripped.string());
       rcpputils::require_true(
-        resolved_stripped.exists(),
+        fs::exists(resolved_stripped),
         "Unable to resolve relative file path either as a V3 or V4 relative path");
       *current_file_iterator_ = resolved_stripped.string();
     }

diff --git a/rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp b/rosbag2_compression/src/rosbag2_compression/sequential_compression_writer.cpp
@@ -17,16 +17,15 @@
 #include <algorithm>
 #include <chrono>
 #include <cstring>
+#include <filesystem>
 #include <functional>
 #include <memory>
 #include <stdexcept>
 #include <string>
+#include <system_error>
 #include <utility>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
-
-#include "rcutils/filesystem.h"
 
 #include "rosbag2_cpp/info.hpp"
 
@@ -41,6 +40,8 @@
 #include <sys/resource.h>
 #endif
 
+namespace fs = std::filesystem;
+
 namespace rosbag2_compression
 {
 
@@ -294,14 +295,14 @@ void SequentialCompressionWriter::compress_file(
   BaseCompressorInterface & compressor,
   const std::string & file_relative_to_bag)
 {
-  using rcpputils::fs::path;
-
-  const auto file_relative_to_pwd = path(base_folder_) / file_relative_to_bag;
+  const auto file_relative_to_pwd = fs::path(base_folder_) / file_relative_to_bag;
   ROSBAG2_COMPRESSION_LOG_INFO_STREAM("Compressing file: " << file_relative_to_pwd.string());
 
-  if (file_relative_to_pwd.exists() && file_relative_to_pwd.file_size() > 0u) {
+  if (fs::exists(file_relative_to_pwd) &&
+    fs::file_size(file_relative_to_pwd) > 0u)
+  {
     const auto compressed_uri = compressor.compress_uri(file_relative_to_pwd.string());
-    const auto relative_compressed_uri = path(compressed_uri).filename();
+    const auto relative_compressed_uri = fs::path(compressed_uri).filename();
     {
       // After we've compressed the file, replace the name in the file list with the new name.
       // Must search for the entry because other threads may have changed the order of the vector
@@ -320,10 +321,11 @@ void SequentialCompressionWriter::compress_file(
       }
     }
 
-    if (!rcpputils::fs::remove(file_relative_to_pwd)) {
+    if (std::error_code ec;!fs::remove(file_relative_to_pwd, ec)) {
       ROSBAG2_COMPRESSION_LOG_ERROR_STREAM(
         "Failed to remove original pre-compressed bag file: \"" <<
-          file_relative_to_pwd.string() << "\". This should never happen - but execution " <<
+          file_relative_to_pwd.string() << "\"." << ec.message() <<
+          "This should never happen - but execution " <<
           "will not be halted because the compressed output was successfully created.");
     }
   } else {

diff --git a/rosbag2_compression/test/rosbag2_compression/fake_decompressor.cpp b/rosbag2_compression/test/rosbag2_compression/fake_decompressor.cpp
@@ -12,19 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <filesystem>
 #include <string>
 
 #include "pluginlib/class_list_macros.hpp"
 
-#include "rcpputils/filesystem_helper.hpp"
-
 #include "fake_decompressor.hpp"
 
+namespace fs = std::filesystem;
+
 std::string FakeDecompressor::decompress_uri(const std::string & uri)
 {
-  auto uri_path = rcpputils::fs::path{uri};
-  const auto decompressed_path = rcpputils::fs::remove_extension(uri_path);
-  return decompressed_path.string();
+  auto uri_path = fs::path{uri};
+  const auto decompressed_path = uri_path.replace_extension();
+  return decompressed_path.generic_string();
 }
 
 void FakeDecompressor::decompress_serialized_bag_message(

diff --git a/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_reader.cpp b/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_reader.cpp
@@ -14,14 +14,14 @@
 
 #include <gmock/gmock.h>
 
+#include <filesystem>
 #include <fstream>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/sequential_compression_reader.hpp"
 
@@ -37,6 +37,8 @@
 
 using namespace testing;  // NOLINT
 
+namespace fs = std::filesystem;
+
 static constexpr const char * DefaultTestCompressor = "fake_comp";
 
 class SequentialCompressionReaderTest : public Test
@@ -48,10 +50,10 @@ class SequentialCompressionReaderTest : public Test
     converter_factory_{std::make_shared<StrictMock<MockConverterFactory>>()},
     metadata_io_{std::make_unique<NiceMock<MockMetadataIo>>()},
     storage_serialization_format_{"rmw1_format"},
-    tmp_dir_{rcpputils::fs::temp_directory_path() / bag_name_},
+    tmp_dir_{fs::temp_directory_path() / bag_name_},
     converter_options_{"", storage_serialization_format_}
   {
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
     storage_options_.uri = tmp_dir_.string();
     topic_with_type_ = rosbag2_storage::TopicMetadata{
       0U, "topic", "test_msgs/BasicTypes", storage_serialization_format_, {}, ""};
@@ -92,7 +94,7 @@ class SequentialCompressionReaderTest : public Test
   void initialize_dummy_storage_files()
   {
     // Initialize some dummy files so that they can be found
-    rcpputils::fs::create_directories(tmp_dir_);
+    fs::create_directories(tmp_dir_);
     for (auto relative : metadata_.relative_file_paths) {
       std::ofstream output((tmp_dir_ / relative).string());
       output << "Fake storage data" << std::endl;
@@ -104,9 +106,9 @@ class SequentialCompressionReaderTest : public Test
     auto decompressor = std::make_unique<NiceMock<MockDecompressor>>();
     ON_CALL(*decompressor, decompress_uri).WillByDefault(
       [](auto uri) {
-        auto path = rcpputils::fs::path(uri);
-        EXPECT_TRUE(path.exists());
-        return rcpputils::fs::remove_extension(path).string();
+        auto path = fs::path(uri);
+        EXPECT_TRUE(fs::exists(path));
+        return path.replace_extension().generic_string();
       });
     auto compression_factory = std::make_unique<NiceMock<MockCompressionFactory>>();
     ON_CALL(*compression_factory, create_decompressor(_))
@@ -126,7 +128,7 @@ class SequentialCompressionReaderTest : public Test
   std::string storage_serialization_format_;
   rosbag2_storage::TopicMetadata topic_with_type_;
   const std::string bag_name_ = "SequentialCompressionReaderTest";
-  rcpputils::fs::path tmp_dir_;
+  fs::path tmp_dir_;
   rosbag2_storage::StorageOptions storage_options_;
   rosbag2_storage::BagMetadata metadata_;
   rosbag2_cpp::ConverterOptions converter_options_;
@@ -254,7 +256,7 @@ TEST_F(SequentialCompressionReaderTest, throws_on_incorrect_filenames)
 {
   for (auto & relative_file_path : metadata_.relative_file_paths) {
     relative_file_path = (
-      rcpputils::fs::path(bag_name_) / (relative_file_path + ".something")).string();
+      fs::path(bag_name_) / (relative_file_path + ".something")).string();
   }
   auto reader = create_reader();
   EXPECT_THROW(reader->open(storage_options_, converter_options_), std::invalid_argument);
@@ -264,7 +266,7 @@ TEST_F(SequentialCompressionReaderTest, can_find_prefixed_filenames)
 {
   // By prefixing the bag name, this imitates the V3 filename logic
   for (auto & relative_file_path : metadata_.relative_file_paths) {
-    relative_file_path = (rcpputils::fs::path(bag_name_) / relative_file_path).string();
+    relative_file_path = (fs::path(bag_name_) / relative_file_path).string();
   }
   auto reader = create_reader();
 
@@ -278,7 +280,7 @@ TEST_F(SequentialCompressionReaderTest, can_find_prefixed_filenames_in_renamed_b
   // was recorded using V3 logic, then the directory was moved to be a new name - this is the
   // use case the V4 relative path logic was intended to fix
   for (auto & relative_file_path : metadata_.relative_file_paths) {
-    relative_file_path = (rcpputils::fs::path("OtherBagName") / relative_file_path).string();
+    relative_file_path = (fs::path("OtherBagName") / relative_file_path).string();
   }
   auto reader = create_reader();
 

diff --git a/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp b/rosbag2_compression/test/rosbag2_compression/test_sequential_compression_writer.cpp
@@ -14,14 +14,14 @@
 
 #include <gmock/gmock.h>
 
+#include <filesystem>
 #include <fstream>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "rcpputils/asserts.hpp"
-#include "rcpputils/filesystem_helper.hpp"
 
 #include "rosbag2_compression/compression_options.hpp"
 #include "rosbag2_compression/sequential_compression_writer.hpp"
@@ -47,6 +47,8 @@
 
 using namespace testing;  // NOLINT
 
+namespace fs = std::filesystem;
+
 static constexpr const char * DefaultTestCompressor = "fake_comp";
 
 class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
@@ -57,12 +59,12 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
     storage_{std::make_shared<NiceMock<MockStorage>>()},
     converter_factory_{std::make_shared<StrictMock<MockConverterFactory>>()},
     metadata_io_{std::make_unique<NiceMock<MockMetadataIo>>()},
-    tmp_dir_{rcpputils::fs::temp_directory_path() / "SequentialCompressionWriterTest"},
+    tmp_dir_{fs::temp_directory_path() / "SequentialCompressionWriterTest"},
     tmp_dir_storage_options_{},
     serialization_format_{"rmw_format"}
   {
     tmp_dir_storage_options_.uri = tmp_dir_.string();
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
     ON_CALL(*storage_factory_, open_read_write(_)).WillByDefault(Return(storage_));
     EXPECT_CALL(*storage_factory_, open_read_write(_)).Times(AtLeast(0));
     // intercept the metadata write so we can analyze it.
@@ -79,7 +81,7 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
 
   ~SequentialCompressionWriterTest() override
   {
-    rcpputils::fs::remove_all(tmp_dir_);
+    fs::remove_all(tmp_dir_);
   }
 
   void initializeFakeFileStorage()
@@ -139,7 +141,7 @@ class SequentialCompressionWriterTest : public TestWithParam<uint64_t>
   std::shared_ptr<StrictMock<MockConverterFactory>> converter_factory_;
   std::unique_ptr<MockMetadataIo> metadata_io_;
 
-  rcpputils::fs::path tmp_dir_;
+  fs::path tmp_dir_;
   rosbag2_storage::StorageOptions tmp_dir_storage_options_;
   rosbag2_storage::BagMetadata intercepted_write_metadata_;
   std::vector<rosbag2_storage::BagMetadata> v_intercepted_update_metadata_;
@@ -212,7 +214,7 @@ TEST_F(SequentialCompressionWriterTest, open_succeeds_on_supported_compression_f
     kDefaultCompressionQueueThreadsPriority};
   initializeWriter(compression_options);
 
-  auto tmp_dir = rcpputils::fs::temp_directory_path() / "path_not_empty";
+  auto tmp_dir = fs::temp_directory_path() / "path_not_empty";
   auto storage_options = rosbag2_storage::StorageOptions();
   storage_options.uri = tmp_dir.string();
 

diff --git a/rosbag2_compression_zstd/CMakeLists.txt b/rosbag2_compression_zstd/CMakeLists.txt
@@ -23,7 +23,6 @@ endif()
 
 find_package(ament_cmake REQUIRED)
 find_package(pluginlib REQUIRED)
-find_package(rcpputils REQUIRED)
 find_package(rosbag2_compression REQUIRED)
 find_package(zstd_vendor REQUIRED)
 find_package(zstd REQUIRED)
@@ -37,7 +36,6 @@ target_include_directories(${PROJECT_NAME}
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
   $<INSTALL_INTERFACE:include/${PROJECT_NAME}>)
 target_link_libraries(${PROJECT_NAME}
-  rcpputils::rcpputils
   rosbag2_compression::rosbag2_compression
   zstd::zstd
 )
@@ -63,7 +61,7 @@ ament_export_libraries(${PROJECT_NAME})
 ament_export_targets(export_${PROJECT_NAME})
 
 # order matters here, first vendor, then zstd
-ament_export_dependencies(rcpputils rosbag2_compression zstd_vendor zstd)
+ament_export_dependencies(rosbag2_compression zstd_vendor zstd)
 
 
 if(BUILD_TESTING)

diff --git a/rosbag2_compression_zstd/package.xml b/rosbag2_compression_zstd/package.xml
@@ -15,7 +15,6 @@
   <buildtool_depend>ament_cmake</buildtool_depend>
 
   <depend>pluginlib</depend>
-  <depend>rcpputils</depend>
   <depend>rcutils</depend>
   <depend>rosbag2_compression</depend>
   <depend>zstd_vendor</depend>

diff --git a/rosbag2_compression_zstd/src/rosbag2_compression_zstd/compression_utils.hpp b/rosbag2_compression_zstd/src/rosbag2_compression_zstd/compression_utils.hpp
@@ -23,8 +23,7 @@
 #include <vector>
 
 #include "logging.hpp"
-
-#include "rcpputils/filesystem_helper.hpp"
+#include "rcutils/types/rcutils_ret.h"
 
 namespace rosbag2_compression_zstd
 {

diff --git a/rosbag2_compression_zstd/src/rosbag2_compression_zstd/zstd_compressor.cpp b/rosbag2_compression_zstd/src/rosbag2_compression_zstd/zstd_compressor.cpp
@@ -19,8 +19,6 @@
 #include <string>
 #include <vector>
 
-#include "rcpputils/filesystem_helper.hpp"
-
 #include "compression_utils.hpp"
 #include "rosbag2_compression_zstd/zstd_compressor.hpp"
 #include "rosbag2_storage/ros_helper.hpp"

diff --git a/rosbag2_compression_zstd/src/rosbag2_compression_zstd/zstd_decompressor.cpp b/rosbag2_compression_zstd/src/rosbag2_compression_zstd/zstd_decompressor.cpp
@@ -15,15 +15,16 @@
 #include <algorithm>
 #include <chrono>
 #include <cstdio>
+#include <filesystem>
 #include <sstream>
 #include <string>
 #include <vector>
 
-#include "rcpputils/filesystem_helper.hpp"
-
 #include "compression_utils.hpp"
 #include "rosbag2_compression_zstd/zstd_decompressor.hpp"
 
+namespace fs = std::filesystem;
+
 namespace rosbag2_compression_zstd
 {
 ZstdDecompressor::ZstdDecompressor()
@@ -45,8 +46,8 @@ ZstdDecompressor::~ZstdDecompressor()
 std::string ZstdDecompressor::decompress_uri(const std::string & uri)
 {
   const auto start = std::chrono::high_resolution_clock::now();
-  const auto uri_path = rcpputils::fs::path{uri};
-  const auto decompressed_uri = rcpputils::fs::remove_extension(uri_path).string();
+  auto uri_path = fs::path{uri};
+  auto decompressed_uri = uri_path.replace_extension().generic_string();
 
   std::ifstream input(uri, std::ios::in | std::ios::binary);
   if (!input.is_open()) {