fix: preserve files during filesystem copies

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- Filesystem copies preserve their source when copying onto itself and leave
+  existing destinations intact when reading or writing fails.
+
 - PDF Identity-H/V fonts retain two-byte character codes even when their
   text-extraction map declares a conflicting codespace, preserving glyphs and
   spacing.

src/odr/internal/common/filesystem.cpp

@@ -141,11 +141,9 @@ bool SystemFilesystem::copy(const AbsPath &from, const AbsPath &to) {
 
 std::shared_ptr<abstract::File>
 SystemFilesystem::copy(const abstract::File &from, const AbsPath &to) {
-  // `create_file` and `open` translate `to` themselves
-  const auto istream = from.stream();
-  const auto ostream = create_file(to);
-
-  util::stream::pipe(*istream, *ostream);
+  util::file::write_atomic(
+      to_system_path_(to).string(),
+      [&](std::ostream &out) { util::stream::pipe(*from.stream(), out); });
 
   return open(to);
 }

test/src/internal/common/filesystem_test.cpp

@@ -4,13 +4,15 @@
 #include <odr/internal/common/file.hpp>
 #include <odr/internal/common/path.hpp>
 #include <odr/internal/common/random.hpp>
+#include <odr/internal/common/temporary_file.hpp>
 #include <odr/internal/util/file_util.hpp>
 
 #include <gtest/gtest.h>
 
 #include <array>
 #include <filesystem>
 #include <memory>
+#include <sstream>
 #include <string>
 #include <system_error>
 #include <vector>
@@ -45,6 +47,45 @@ filesystem_of(const std::vector<std::string> &files,
 
 } // namespace
 
+TEST(Filesystem, copying_a_file_over_itself_preserves_its_bytes) {
+  const MemoryFile source(std::string(8193, 'x'));
+  const auto file = TemporaryDiskFileFactory::system_default().copy(source);
+  SystemFilesystem disk(file.disk_path()->parent());
+  const AbsPath target("/" + file.name());
+
+  const auto copied = disk.copy(file, target);
+
+  ASSERT_NE(copied, nullptr);
+  EXPECT_EQ(MemoryFile(*copied).content(), source.content());
+}
+
+TEST(Filesystem, a_failed_copy_preserves_the_destination) {
+  class UnreadableFile final : public abstract::File {
+  public:
+    odr::FileLocation location() const noexcept override {
+      return odr::FileLocation::memory;
+    }
+    std::size_t size() const override { return 11; }
+    std::string name() const override { return {}; }
+    std::optional<AbsPath> disk_path() const override { return std::nullopt; }
+    std::optional<std::string_view> memory_data() const override {
+      return std::nullopt;
+    }
+    std::unique_ptr<std::istream> stream() const override {
+      auto result = std::make_unique<std::istringstream>("replacement");
+      result->setstate(std::ios::badbit);
+      return result;
+    }
+  } source;
+  const auto file =
+      TemporaryDiskFileFactory::system_default().copy(MemoryFile("original"));
+  SystemFilesystem disk(file.disk_path()->parent());
+  const AbsPath target("/" + file.name());
+
+  EXPECT_THROW(disk.copy(source, target), std::ios_base::failure);
+  EXPECT_EQ(MemoryFile(file).content(), "original");
+}
+
 TEST(Filesystem, copied_walkers_advance_independently) {
   struct Directory final {
     std::filesystem::path path = std::filesystem::temp_directory_path() /