Skip to content

Commit

Permalink
Replace rcpputils::fs with std::filesystem
Browse files Browse the repository at this point in the history
Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com>
  • Loading branch information
r7vme committed Mar 8, 2024
1 parent 87dbb4f commit 956252b
Show file tree
Hide file tree
Showing 33 changed files with 243 additions and 200 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -41,6 +40,8 @@
#include <sys/resource.h>
#endif

namespace fs = std::filesystem;

namespace rosbag2_compression
{

Expand Down Expand Up @@ -292,14 +293,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
Expand All @@ -318,10 +319,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -37,6 +37,8 @@

using namespace testing; // NOLINT

namespace fs = std::filesystem;

static constexpr const char * DefaultTestCompressor = "fake_comp";

class SequentialCompressionReaderTest : public Test
Expand All @@ -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_, {}, ""};
Expand Down Expand Up @@ -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;
Expand All @@ -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(_))
Expand All @@ -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_;
Expand Down Expand Up @@ -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);
Expand All @@ -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();

Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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>
Expand All @@ -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.
Expand All @@ -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()
Expand Down Expand Up @@ -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_;
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 1 addition & 3 deletions rosbag2_compression_zstd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
)
Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion rosbag2_compression_zstd/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@
#include <vector>

#include "logging.hpp"

#include "rcpputils/filesystem_helper.hpp"
#include "rcutils/types/rcutils_ret.h"

namespace rosbag2_compression_zstd
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()) {
Expand Down
Loading

0 comments on commit 956252b

Please sign in to comment.