Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/lang/io/include/sourcemeta/core/io.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,23 @@ SOURCEMETA_CORE_IO_EXPORT
auto hardlink_directory(const std::filesystem::path &source,
const std::filesystem::path &destination) -> void;

/// @ingroup io
///
/// Replace one directory with another, guaranteeing an atomic swap when
/// possible. Both directories must reside on the same filesystem and the
/// original path must not be a bare filename (it must have a parent
/// component). If the original does not exist, the replacement is simply
/// renamed into place.
///
/// ```cpp
/// #include <sourcemeta/core/io.h>
///
/// sourcemeta::core::atomic_directory_replace("/output", "/staging");
/// ```
SOURCEMETA_CORE_IO_EXPORT
auto atomic_directory_replace(const std::filesystem::path &original,
const std::filesystem::path &replacement) -> void;

/// @ingroup io
///
/// Flush an existing file to disk, beyond just to the operating system. For
Expand Down
58 changes: 57 additions & 1 deletion src/lang/io/io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,14 @@
#include <windows.h> // HANDLE, CreateFileW, FlushFileBuffers, CloseHandle
#else
#include <cerrno> // errno (for error codes)
#include <fcntl.h> // open, O_RDWR
#include <fcntl.h> // open, O_RDWR, AT_FDCWD
#include <unistd.h> // close, fsync
#if defined(__linux__)
#include <linux/fs.h> // RENAME_EXCHANGE
#include <stdio.h> // renameat2
#elif defined(__APPLE__)
#include <sys/stdio.h> // renameatx_np, RENAME_SWAP
#endif
#endif

namespace sourcemeta::core {
Expand Down Expand Up @@ -66,6 +72,56 @@ auto hardlink_directory(const std::filesystem::path &source,
}
}

auto atomic_directory_replace(const std::filesystem::path &original,
const std::filesystem::path &replacement)
-> void {
assert(std::filesystem::is_directory(replacement));
assert(!std::filesystem::exists(original) ||
std::filesystem::is_directory(original));
assert(!original.parent_path().empty());

if (!std::filesystem::exists(original)) {
std::filesystem::rename(replacement, original);
return;
}

// Atomic swap via renameat2 with RENAME_EXCHANGE
#if defined(__linux__)
if (renameat2(AT_FDCWD, replacement.c_str(), AT_FDCWD, original.c_str(),
RENAME_EXCHANGE) != 0) {
throw std::filesystem::filesystem_error{
"failed to atomically swap directories", replacement, original,
std::error_code{errno, std::generic_category()}};
}

std::filesystem::remove_all(replacement);
// Atomic swap via renameatx_np with RENAME_SWAP
#elif defined(__APPLE__)
if (renameatx_np(AT_FDCWD, replacement.c_str(), AT_FDCWD, original.c_str(),
RENAME_SWAP) != 0) {
throw std::filesystem::filesystem_error{
"failed to atomically swap directories", replacement, original,
std::error_code{errno, std::generic_category()}};
}

std::filesystem::remove_all(replacement);
#else
// Non-atomic fallback: two-rename approach with rollback.

// Note we cannot safely use the temporary directory of the system as it
// might be in another volume
TemporaryDirectory backup{original.parent_path(), ".backup-"};
std::filesystem::remove(backup.path());
std::filesystem::rename(original, backup.path());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

original.parent_path() can be empty for relative paths like "original", which may cause TemporaryDirectory (and its internal create_directories) to throw unexpectedly. Consider guarding this case so callers can safely use relative directory paths too.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

try {
std::filesystem::rename(replacement, original);
} catch (...) {
std::filesystem::rename(backup.path(), original);
throw;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the rollback std::filesystem::rename(backup.path(), original) throws inside this catch, the function can exit with original still moved aside, contradicting the “original is restored” guarantee in the header docs. Consider whether you want to handle/document rollback failure separately so the post-failure state is clear.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

#endif
}

auto flush(const std::filesystem::path &path) -> void {
#if defined(_WIN32)
HANDLE hFile =
Expand Down
3 changes: 2 additions & 1 deletion test/io/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ sourcemeta_googletest(NAMESPACE sourcemeta PROJECT core NAME io
io_read_file_test.cc
io_fileview_test.cc
io_temporary_test.cc
io_hardlink_directory_test.cc)
io_hardlink_directory_test.cc
io_atomic_directory_replace_test.cc)

target_link_libraries(sourcemeta_core_io_unit
PRIVATE sourcemeta::core::io)
Expand Down
116 changes: 116 additions & 0 deletions test/io/io_atomic_directory_replace_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
#include <gtest/gtest.h>

#include <sourcemeta/core/io.h>

#include <filesystem> // std::filesystem
#include <fstream> // std::ofstream, std::ifstream
#include <string> // std::string

TEST(IO_atomic_directory_replace, creates_when_original_absent) {
const sourcemeta::core::TemporaryDirectory workspace{
std::filesystem::path{BUILD_DIRECTORY}, ".test-atomic-"};
const auto original{workspace.path() / "original"};
const auto replacement{workspace.path() / "replacement"};

std::filesystem::create_directory(replacement);
std::ofstream{replacement / "file.txt"} << "hello";

EXPECT_FALSE(std::filesystem::exists(original));

sourcemeta::core::atomic_directory_replace(original, replacement);

EXPECT_TRUE(std::filesystem::exists(original));
EXPECT_TRUE(std::filesystem::is_directory(original));
EXPECT_FALSE(std::filesystem::exists(replacement));

std::ifstream stream{original / "file.txt"};
std::string content;
std::getline(stream, content);
EXPECT_EQ(content, "hello");
}

TEST(IO_atomic_directory_replace, replaces_existing_directory) {
const sourcemeta::core::TemporaryDirectory workspace{
std::filesystem::path{BUILD_DIRECTORY}, ".test-atomic-"};
const auto original{workspace.path() / "original"};
const auto replacement{workspace.path() / "replacement"};

std::filesystem::create_directory(original);
std::ofstream{original / "old.txt"} << "old";

std::filesystem::create_directory(replacement);
std::ofstream{replacement / "new.txt"} << "new";

sourcemeta::core::atomic_directory_replace(original, replacement);

EXPECT_TRUE(std::filesystem::exists(original));
EXPECT_FALSE(std::filesystem::exists(original / "old.txt"));
EXPECT_TRUE(std::filesystem::exists(original / "new.txt"));

std::ifstream stream{original / "new.txt"};
std::string content;
std::getline(stream, content);
EXPECT_EQ(content, "new");
}

TEST(IO_atomic_directory_replace, replacement_is_consumed) {
const sourcemeta::core::TemporaryDirectory workspace{
std::filesystem::path{BUILD_DIRECTORY}, ".test-atomic-"};
const auto original{workspace.path() / "original"};
const auto replacement{workspace.path() / "replacement"};

std::filesystem::create_directory(original);
std::filesystem::create_directory(replacement);
std::ofstream{replacement / "file.txt"} << "data";

sourcemeta::core::atomic_directory_replace(original, replacement);

EXPECT_FALSE(std::filesystem::exists(replacement));
}

TEST(IO_atomic_directory_replace, preserves_nested_structure) {
const sourcemeta::core::TemporaryDirectory workspace{
std::filesystem::path{BUILD_DIRECTORY}, ".test-atomic-"};
const auto original{workspace.path() / "original"};
const auto replacement{workspace.path() / "replacement"};

std::filesystem::create_directory(original);

std::filesystem::create_directories(replacement / "a" / "b");
std::ofstream{replacement / "root.txt"} << "root";
std::ofstream{replacement / "a" / "mid.txt"} << "mid";
std::ofstream{replacement / "a" / "b" / "deep.txt"} << "deep";

sourcemeta::core::atomic_directory_replace(original, replacement);

EXPECT_TRUE(std::filesystem::exists(original / "root.txt"));
EXPECT_TRUE(std::filesystem::exists(original / "a" / "mid.txt"));
EXPECT_TRUE(std::filesystem::exists(original / "a" / "b" / "deep.txt"));

std::ifstream stream{original / "a" / "b" / "deep.txt"};
std::string content;
std::getline(stream, content);
EXPECT_EQ(content, "deep");
}

TEST(IO_atomic_directory_replace, no_leftover_backup) {
const sourcemeta::core::TemporaryDirectory workspace{
std::filesystem::path{BUILD_DIRECTORY}, ".test-atomic-"};
const auto original{workspace.path() / "original"};
const auto replacement{workspace.path() / "replacement"};

std::filesystem::create_directory(original);
std::ofstream{original / "old.txt"} << "old";

std::filesystem::create_directory(replacement);
std::ofstream{replacement / "new.txt"} << "new";

sourcemeta::core::atomic_directory_replace(original, replacement);

for (const auto &entry :
std::filesystem::directory_iterator{workspace.path()}) {
const auto filename{entry.path().filename().string()};
EXPECT_FALSE(filename.starts_with(".backup-"))
<< "leftover backup directory found: " << filename;
}
}