diff --git a/include/tmp/entry b/include/tmp/entry index fad2285..50f2302 100644 --- a/include/tmp/entry +++ b/include/tmp/entry @@ -42,7 +42,8 @@ public: native_handle_type native_handle() const noexcept; /// Moves the managed path recursively to a given target, releasing - /// ownership of the managed path + /// ownership of the managed path; behaves like `std::filesystem::rename` + /// even when moving between filesystems /// @note The target path parent is created when this function is called /// @param to A path to the target file or directory /// @throws std::filesystem::filesystem_error if cannot move the owned path diff --git a/src/entry.cpp b/src/entry.cpp index 65ba0f3..f82a03b 100644 --- a/src/entry.cpp +++ b/src/entry.cpp @@ -117,27 +117,54 @@ entry::native_handle_type entry::native_handle() const noexcept { void entry::move(const fs::path& to) { std::error_code ec; + if (fs::exists(to)) { + if (!fs::is_directory(*this) && fs::is_directory(to)) { + ec = std::make_error_code(std::errc::is_a_directory); + throw_move_error(to, ec); + } + + if (fs::is_directory(*this) && !fs::is_directory(to)) { + ec = std::make_error_code(std::errc::not_a_directory); + throw_move_error(to, ec); + } + } + create_parent(to, ec); if (ec) { throw_move_error(to, ec); } - fs::rename(*this, to, ec); - if (ec == std::errc::cross_device_link) { - if (fs::is_regular_file(*this) && fs::is_directory(to)) { - ec = std::make_error_code(std::errc::is_a_directory); - throw_move_error(to, ec); - } + bool copying = false; +#ifdef _WIN32 + // On Windows, the underlying `MoveFileExW` fails when moving a directory + // between drives; in that case we copy the directory manually + copying = fs::is_directory(*this) && path().root_name() != to.root_name(); + if (copying) { + fs::copy(*this, to, copy_options, ec); + } else { + fs::rename(*this, to, ec); + } +#else + // On POSIX-compliant systems, the underlying `rename` function may return + // `EXDEV` if the implementation does not support links between file systems; + // so we try to rename the file, and if we fail with `EXDEV`, move it manually + fs::rename(*this, to, ec); + copying = ec == std::errc::cross_device_link; + if (copying) { fs::remove_all(to); fs::copy(*this, to, copy_options, ec); } +#endif if (ec) { throw_move_error(to, ec); } - remove(*this); + if (copying) { + remove(*this); + } + pathobject.clear(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8e526ce..e00dcd0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,10 +2,11 @@ project(${PROJECT_NAME}.test) find_package(GTest) -add_executable(${PROJECT_NAME} directory.cpp file.cpp) +add_executable(${PROJECT_NAME} entry.cpp directory.cpp file.cpp) target_link_libraries(${PROJECT_NAME} tmp::tmp GTest::gtest_main) target_compile_definitions(${PROJECT_NAME} - PRIVATE LABEL="com.github.bugdea1er.tmp") + PRIVATE LABEL="com.github.bugdea1er.tmp" + BUILD_DIR="${CMAKE_CURRENT_BINARY_DIR}") # On some platforms (e.g. Windows) CMake doesn't write load paths properly # This solution to put outputs in the same directory is good enough diff --git a/tests/directory.cpp b/tests/directory.cpp index 09ddd63..57a5767 100644 --- a/tests/directory.cpp +++ b/tests/directory.cpp @@ -120,7 +120,7 @@ TEST(directory, list) { /// Tests that destructor removes a directory TEST(directory, destructor) { - fs::path path = fs::path(); + fs::path path; entry::native_handle_type handle; { directory tmpdir = directory(); @@ -172,27 +172,6 @@ TEST(directory, move_assignment) { EXPECT_TRUE(native_handle_is_valid(fst.native_handle())); } -/// Tests directory moving -TEST(directory, move) { - fs::path path = fs::path(); - entry::native_handle_type handle; - - fs::path to = fs::temp_directory_path() / "non-existing" / "parent"; - { - directory tmpdir = directory(); - path = tmpdir; - handle = tmpdir.native_handle(); - - tmpdir.move(to); - } - - EXPECT_FALSE(fs::exists(path)); - EXPECT_TRUE(fs::exists(to)); - EXPECT_FALSE(native_handle_is_valid(handle)); - - fs::remove_all(fs::temp_directory_path() / "non-existing"); -} - /// Tests directory swapping TEST(directory, swap) { directory fst = directory(); diff --git a/tests/entry.cpp b/tests/entry.cpp new file mode 100644 index 0000000..748be64 --- /dev/null +++ b/tests/entry.cpp @@ -0,0 +1,233 @@ +#include +#include +#include + +#include "utils.hpp" + +#include + +#include +#include +#include +#include + +namespace tmp { +namespace { + +namespace fs = std::filesystem; + +/// Returns a temporary file containing `Hello world!` +file test_file() { + file tmpfile = file(); + tmpfile.write("Hello, world!"); + + return tmpfile; +} + +/// Returns a temporary directory with a file containing `Hello world!` +directory test_directory() { + directory tmpdir = directory(); + std::ofstream(tmpdir / "file") << "Hello, world!"; + + return tmpdir; +} +} // namespace + +/// Tests that moving a temporary file to itself does nothing +TEST(entry, move_file_to_self) { + fs::path path; + entry::native_handle_type handle; + + { + file tmpfile = test_file(); + path = tmpfile; + handle = tmpfile.native_handle(); + + tmpfile.move(tmpfile); + } + + EXPECT_TRUE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(path); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + fs::remove_all(path); +} + +/// Tests moving a temporary file to existing non-directory file +TEST(entry, move_file_to_existing_file) { + fs::path path; + entry::native_handle_type handle; + + fs::path to = fs::path(BUILD_DIR) / "move_file_to_existing_test"; + std::ofstream(to / "file") << "Goodbye, world!"; + + { + file tmpfile = test_file(); + path = tmpfile; + handle = tmpfile.native_handle(); + + tmpfile.move(to); + } + + EXPECT_TRUE(fs::exists(to)); + EXPECT_FALSE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(to); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + fs::remove_all(to); +} + +/// Tests moving a temporary file to an existing directory +TEST(entry, move_file_to_existing_directory) { + fs::path directory = fs::path(BUILD_DIR) / "existing_directory"; + fs::create_directories(directory); + + EXPECT_THROW(test_file().move(directory), fs::filesystem_error); + + fs::remove_all(directory); +} + +/// Tests moving a temporary file to a non-existing file +TEST(entry, move_file_to_non_existing_file) { + fs::path path; + entry::native_handle_type handle; + + fs::path parent = fs::path(BUILD_DIR) / "non-existing1"; + fs::path to = parent / "path"; + + { + file tmpfile = test_file(); + path = tmpfile; + handle = tmpfile.native_handle(); + + tmpfile.move(to); + } + + EXPECT_TRUE(fs::exists(to)); + EXPECT_FALSE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(to); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + fs::remove_all(parent); +} + +/// Tests moving a temporary file to a non-existing directory +TEST(entry, move_file_to_non_existing_directory) { + fs::path parent = fs::path(BUILD_DIR) / "non-existing2"; + fs::path to = parent / "path/"; + + EXPECT_THROW(test_file().move(to), fs::filesystem_error); + + fs::remove_all(parent); +} + +/// Tests that moving a temporary directory to itself does nothing +TEST(entry, move_directory_to_self) { + fs::path path; + entry::native_handle_type handle; + + { + directory tmpdir = test_directory(); + path = tmpdir; + handle = tmpdir.native_handle(); + + tmpdir.move(tmpdir); + } + + EXPECT_TRUE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(path / "file"); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + fs::remove_all(path); +} + +/// Tests moving a temporary directory to existing directory +TEST(entry, move_directory_to_existing_directory) { + fs::path path; + entry::native_handle_type handle; + + fs::path to = fs::path(BUILD_DIR) / "move_directory_to_existing_test"; + std::ofstream(to / "file2") << "Goodbye, world!"; + + { + directory tmpdir = test_directory(); + path = tmpdir; + handle = tmpdir.native_handle(); + + tmpdir.move(to); + } + + EXPECT_TRUE(fs::exists(to)); + EXPECT_FALSE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(to / "file"); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + EXPECT_FALSE(fs::exists(to / "file2")); + + fs::remove_all(to); +} + +/// Tests moving a temporary directory to an existing file +TEST(entry, move_directory_to_existing_file) { + fs::path to = fs::path(BUILD_DIR) / "existing_file"; + std::ofstream(to) << "Goodbye, world!"; + + EXPECT_THROW(test_directory().move(to), fs::filesystem_error); + + fs::remove_all(to); +} + +/// Tests moving a temporary directory to a non-existing path +TEST(entry, move_directory_to_non_existing_path) { + fs::path path; + entry::native_handle_type handle; + + fs::path parent = fs::path(BUILD_DIR) / "non-existing3"; + fs::path to = parent / "path"; + + { + directory tmpdir = test_directory(); + path = tmpdir; + handle = tmpdir.native_handle(); + + tmpdir.move(to); + } + + EXPECT_TRUE(fs::exists(to)); + EXPECT_FALSE(fs::exists(path)); + EXPECT_FALSE(native_handle_is_valid(handle)); + + { + auto stream = std::ifstream(to / "file"); + auto content = std::string(std::istreambuf_iterator(stream), {}); + EXPECT_EQ(content, "Hello, world!"); + } + + fs::remove_all(parent); +} +} // namespace tmp diff --git a/tests/file.cpp b/tests/file.cpp index 81dec4b..4b05fd5 100644 --- a/tests/file.cpp +++ b/tests/file.cpp @@ -340,7 +340,7 @@ TEST(file, output_stream_append_text) { /// Tests that destructor removes a file TEST(file, destructor) { - fs::path path = fs::path(); + fs::path path; entry::native_handle_type handle; { file tmpfile = file(); @@ -392,27 +392,6 @@ TEST(file, move_assignment) { EXPECT_TRUE(native_handle_is_valid(fst.native_handle())); } -/// Tests file moving -TEST(file, move) { - fs::path path = fs::path(); - entry::native_handle_type handle; - - fs::path to = fs::temp_directory_path() / "non-existing" / "parent"; - { - file tmpfile = file(); - path = tmpfile; - handle = tmpfile.native_handle(); - - tmpfile.move(to); - } - - EXPECT_FALSE(fs::exists(path)); - EXPECT_TRUE(fs::exists(to)); - EXPECT_FALSE(native_handle_is_valid(handle)); - - fs::remove_all(fs::temp_directory_path() / "non-existing"); -} - /// Tests file swapping TEST(file, swap) { file fst = file();