util: use stronger-guarantee rename method #24308

Use std::filesystem::rename() instead of std::rename(). We rely on the destination to be overwritten if it exists, but std::rename()'s behavior is implementation-defined in this case.

This is a rebase of #20435 by vasild.

Approach ACK ee822d85d6de7db85416190cf843ad74147519cf.

We rely on the destination to be overwritten if it exists

Add a test for this assumption?

ACK ee822d85d6de7db85416190cf843ad74147519cf

Add a test for this assumption?

<details> <summary>Unit test for RenameOver()</summary>

commit 23074edf054845b1b74c43618cf7a8cc58aa5518 (HEAD -> pull/24308_1644480965_ee822d85d6__20435_rebased)
Parent: ee822d85d6de7db85416190cf843ad74147519cf
Author:     Vasil Dimov <vd@FreeBSD.org>
AuthorDate: Thu Feb 10 14:11:35 2022 +0100
Commit:     Vasil Dimov <vd@FreeBSD.org>
CommitDate: Thu Feb 10 14:14:58 2022 +0100

    test: add a unit test for RenameOver()

diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp
index 1256395849..8a5f025e5f 100644
--- a/src/test/fs_tests.cpp
+++ b/src/test/fs_tests.cpp
@@ -115,7 +115,40 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
         BOOST_CHECK(p1 != p2);
         BOOST_CHECK(p2 != p3);
         BOOST_CHECK(p1 != p3);
     }
 }
 
-BOOST_AUTO_TEST_SUITE_END()
\ No newline at end of file
+BOOST_AUTO_TEST_CASE(rename)
+{
+    const fs::path tmpfolder{m_args.GetDataDirBase()};
+
+    const fs::path path1{GetUniquePath(tmpfolder)};
+    const fs::path path2{GetUniquePath(tmpfolder)};
+
+    const std::string path1_contents{"1111"};
+    const std::string path2_contents{"2222"};
+
+    {
+        std::ofstream file{path1};
+        file << path1_contents;
+    }
+
+    {
+        std::ofstream file{path2};
+        file << path2_contents;
+    }
+
+    // Rename path1 -> path2.
+    BOOST_CHECK(RenameOver(path1, path2));
+
+    BOOST_CHECK(!fs::exists(path1));
+
+    {
+        std::ifstream file{path2};
+        std::string contents;
+        file >> contents;
+        BOOST_CHECK_EQUAL(contents, path1_contents);
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

</details>

Concept ACK. Checked the relevant documentation that the behavior is what we want for RenameOver:

Moves or renames the filesystem object identified by <code>old_p</code> to <code>new_p</code> as if by the POSIX <a rel="nofollow" class="external text" href="http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html"><code>rename</code></a>

- https://en.cppreference.com/w/cpp/filesystem/rename

If the link named by the new argument exists, it shall be removed and old renamed to new.

- https://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html

Also, it is a nice cleanup, no more win32-specific code here.

review ACK ee822d85d6de7db85416190cf843ad74147519cf

(Also reviewed the unit test in #24308#pullrequestreview-878832906 but didn't run it)

It seems to break Windows builds:

Add a test for this assumption?

Unit test for RenameOver()

Btw, it fails for mingw build: https://cirrus-ci.com/task/6670169794674688