Skip to content

Commit df08250

Browse files
committed
Merge bitcoin/bitcoin#24331: util: Revert back MoveFileExW call for MinGW-w64
dc01cbc test: Add fs_tests/rename unit test (Hennadii Stepanov) d4999d4 util: Revert back MoveFileExW call for MinGW-w64 (Hennadii Stepanov) Pull request description: Unfortunately, bitcoin/bitcoin#24308 introduced a [regression](bitcoin/bitcoin#24308 (comment)) for mingw builds. The root of the problem is a broken implementation of [`std::filesystem::rename`](https://en.cppreference.com/w/cpp/filesystem/rename). In particular, the expected behavior > If `old_p` is a non-directory file, then `new_p` must be ... existing non-directory file: `new_p` _is first deleted_... fails with the "File exists" error. This PR reverts back the `MoveFileExW` call, and adds the [suggested](bitcoin/bitcoin#24308 (review)) unit test. ACKs for top commit: vasild: ACK dc01cbc Tree-SHA512: c8e5a98844cfa32bec0ad67a1aaa58fe2efd0c5474d3e83490211985b110f83245758a742dcaa0a933a192ab66a7f11807e0c53ae69260b7dd02fc99f6d03849
2 parents 9761192 + dc01cbc commit df08250

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

src/test/fs_tests.cpp

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,38 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream)
118118
}
119119
}
120120

121-
BOOST_AUTO_TEST_SUITE_END()
121+
BOOST_AUTO_TEST_CASE(rename)
122+
{
123+
const fs::path tmpfolder{m_args.GetDataDirBase()};
124+
125+
const fs::path path1{GetUniquePath(tmpfolder)};
126+
const fs::path path2{GetUniquePath(tmpfolder)};
127+
128+
const std::string path1_contents{"1111"};
129+
const std::string path2_contents{"2222"};
130+
131+
{
132+
std::ofstream file{path1};
133+
file << path1_contents;
134+
}
135+
136+
{
137+
std::ofstream file{path2};
138+
file << path2_contents;
139+
}
140+
141+
// Rename path1 -> path2.
142+
BOOST_CHECK(RenameOver(path1, path2));
143+
144+
BOOST_CHECK(!fs::exists(path1));
145+
146+
{
147+
std::ifstream file{path2};
148+
std::string contents;
149+
file >> contents;
150+
BOOST_CHECK_EQUAL(contents, path1_contents);
151+
}
152+
fs::remove(path2);
153+
}
154+
155+
BOOST_AUTO_TEST_SUITE_END()

src/util/system.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,9 +1062,20 @@ void ArgsManager::LogArgs() const
10621062

10631063
bool RenameOver(fs::path src, fs::path dest)
10641064
{
1065+
#ifdef __MINGW64__
1066+
// This is a workaround for a bug in libstdc++ which
1067+
// implements std::filesystem::rename with _wrename function.
1068+
// This bug has been fixed in upstream:
1069+
// - GCC 10.3: 8dd1c1085587c9f8a21bb5e588dfe1e8cdbba79e
1070+
// - GCC 11.1: 1dfd95f0a0ca1d9e6cbc00e6cbfd1fa20a98f312
1071+
// For more details see the commits mentioned above.
1072+
return MoveFileExW(src.wstring().c_str(), dest.wstring().c_str(),
1073+
MOVEFILE_REPLACE_EXISTING) != 0;
1074+
#else
10651075
std::error_code error;
10661076
fs::rename(src, dest, error);
10671077
return !error;
1078+
#endif
10681079
}
10691080

10701081
/**

0 commit comments

Comments
 (0)