diff --git a/lix/libutil/file-system.cc b/lix/libutil/file-system.cc index 0e688ec20..8702968a7 100644 --- a/lix/libutil/file-system.cc +++ b/lix/libutil/file-system.cc @@ -824,22 +824,23 @@ void moveFile(const Path & oldName, const Path & newName) try { fs::rename(oldName, newName); } catch (fs::filesystem_error & e) { // NOLINT(lix-foreign-exceptions) - auto oldPath = fs::path(oldName); - auto newPath = fs::path(newName); - // For the move to be as atomic as possible, copy to a temporary - // directory try { + if (e.code().value() != EXDEV) { + throw SysError(e.code().value(), "failed to move %s to %s", oldName, newName); + } + auto oldPath = fs::path(oldName); + auto newPath = fs::path(newName); + // For the move to be as atomic as possible, copy to a temporary + // directory fs::path temp = createTempSubdir(newPath.parent_path(), "rename-tmp"); Finally removeTemp = [&]() { fs::remove(temp); }; auto tempCopyTarget = temp / "copy-target"; - if (e.code().value() == EXDEV) { - fs::remove(newPath); - printTaggedWarning("Can’t rename %s as %s, copying instead", oldName, newName); - copy(fs::directory_entry(oldPath), tempCopyTarget, {.deleteAfter = true}); - fs::rename(tempCopyTarget, newPath); - } + fs::remove(newPath); + printTaggedWarning("Can’t rename %s as %s, copying instead", oldName, newName); + copy(fs::directory_entry(oldPath), tempCopyTarget, {.deleteAfter = true}); + fs::rename(tempCopyTarget, newPath); } catch (fs::filesystem_error & e) { // NOLINT(lix-foreign-exceptions) - throw Error("failed to move %s to %s: %s", oldName, newName, e.what()); + throw SysError(e.code().value(), "failed to move %s to %s", oldName, newName); } } } diff --git a/tests/unit/libutil/tests.cc b/tests/unit/libutil/tests.cc index 34ced9f18..064d03b56 100644 --- a/tests/unit/libutil/tests.cc +++ b/tests/unit/libutil/tests.cc @@ -1,3 +1,4 @@ +#include "lix/libstore/temporary-dir.hh" #include "lix/libutil/c-calls.hh" #include "lix/libutil/file-system.hh" #include "lix/libutil/processes.hh" @@ -210,6 +211,31 @@ namespace nix { ASSERT_FALSE(pathExists("/schnitzel/darmstadt/pommes")); } + /* ---------------------------------------------------------------------------- + * moveFile + * --------------------------------------------------------------------------*/ + + TEST(moveFile, handlesErrors) + { + auto tmpDir = createTempDir(); + AutoDelete _delete(tmpDir); + + auto source = tmpDir + "/source"; + auto target = tmpDir + "/target"; + + // xdev should cause copies, everything else must throw. this is a small selection. + ASSERT_THROW(moveFile(source, target), Error); // ENOENT + createDirs(source); + createSymlink(target, target); + ASSERT_THROW(moveFile(source, target), Error); // ENOTDIR + ASSERT_THROW(moveFile(target, source), Error); // EISDIR + ASSERT_THROW(moveFile(target + "/foo", source), Error); // ELOOP + + deletePath(target); + createDirs(target + "/snafu"); + ASSERT_THROW(moveFile(source, target), Error); // ENOTEMPTY or EEXIST + } + /* ---------------------------------------------------------------------------- * AutoCloseFD::guessOrInventPath * --------------------------------------------------------------------------*/