#36620 test: Add case for libstdc++ bug on Windows

ReviewabilityReviewsAgreementSize
#36620 hebasto test: Add case for libstdc++ bug on WindowsReady0Positive0 + 26 tests
  • Detect an upstream Windows libstdc++ bug that fails when overwriting existing files
  • Protect wallet migration code that relies on overwriting backup files across affected GCC toolchains
  • Ready for review
  • No review verdicts recorded
  • Support because adding the test seems harmless (maflcko)
  • Questions whether testing an upstream compiler bug is useful when user impact is low (fanquake, theuni)
  • Suggests rejecting affected compilers at configure or making migration not overwrite (maflcko, theuni)
  • 0 lines added or modified outside tests
  • 26 lines added or modified in tests
  • 0 lines removed in total
  • 1 files, 1 commits
Test infrastructure#33 of 54
P3 · test coverage
  • P3 because it adds unit test coverage for an upstream Windows compiler bug during file copying
  • The benefit is marginal because real-world user impact from this toolchain bug is practically nonexistent
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds a unit test in `fs_tests` verifying that `fs::copy_file` with `copy_options::overwrite_existing` succeeds. The test exercises a specific upstream libstdc++ bug on Windows where the call fails with a `File exists` error.

Problem: Wallet migration code (`BerkeleyRODatabase::Backup()`) relies on `fs::copy_file` successfully overwriting an existing file. In certain GCC toolchains targeting Windows (GCC 12.x, GCC < 13.4, and GCC < 14.3), libstdc++ fails with `filesystem error: cannot copy file: File exists`, but unit tests lacked coverage to detect this toolchain bug.

Evidence of importance:

  • Commit message cites upstream GCC patches fixing the bug (017e3f89b081, 35c98149a5c7, 8b3e47b4099f).
  • PR description links to a CI failure demonstrating the test failing under GCC 14.2.0 cross-compilation for Windows: `std::filesystem::__cxx11::filesystem_error: filesystem error: cannot copy file: File exists`.
  • PR description points directly to `BerkeleyRODatabase::Backup()` in `src/wallet/migrate.cpp` as relying on this behavior.

Risk of a mistake: low (tests). Touches only unit tests in fs_tests; a failure only affects test execution on buggy toolchains.

Changed since the previous assessment: No functional changes to the patch; the previously missing diff is now provided.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: response to maflcko and theuni regarding alternative approaches such as fixing migration not to overwrite 2026-10-26

Agreement

Positive

support without stated reasons, no open objection (maflcko)

Thread notes: Added maflcko's suggestion in c:6420696121 that migration shouldn't overwrite, which also contributes to the discussion started in c:6371922580.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)suggestion, styleaddress Ralph's comment from the previous PRfixed · fix pushed · author confirmed addressedansweredyes2026-10-23: 'Could address the Ralph comment?'
Status from: 2026-10-23: hebasto: 'Done.'
fanquake (member)question, usefulnesswhy this bug specifically and which users in what environments are expected to hit thisanswered · author noted users in uncontrolled build environments meeting minimum requirements could hit itansweredyes2026-10-23: 'Can you elaborate on the motivation here? Why this bug specifically? Which users (compiling in what currently used environments) do we expect to hit this?'
Status from: 2026-10-23: hebasto: 'Even after #36042, users can hit this bug in a build environment we don’t control'
maflcko (member)suggestion, approachreject g++-mingw versions less than 14.3 during configure instead of adding the testopenansweredno2026-10-23: 'An alternative could be to reject g++-mingw less than 14.3 during configure?'
fanquake (member)suggestion, usefulnessexplain the utility of adding a test for a compiler bug given end-users rarely run tests or hit runtime issuesanswered · author conceded there is no known real-life scenario where users would hit the bugansweredyes2026-10-23: 'It's not clear that adding a test here is that useful, unless you expect cross-compiling end-users are running the unit test binary on their systems before running bitcoind?'
Status from: 2026-10-23: hebasto: 'I have to admit that I can’t see a real-life scenario in which the bug affects users.'
theuni (member)suggestion, usefulnesspursue alternatives such as blocking the compiler or removing copy_options rather than adding a test that fails where expectedopen · author deferred by asking achow101 if removing copy_options is viableansweredyes2026-10-23: 'Either way, adding a test that fails where we expect it to fail doesn't seem very helpful to me.'
fanquake (member)question, scopewhether libstdc++ should be patched in Guix and backported to point releases if the issue is serious enough for a unit testanswered · author explained that real-world impact is practically nonexistentansweredyes2026-10-23: 'if this is serious enough to warrant a unit test now, shouldn't we be patching the compiler/libstdc++ in Guix and doing a point release with the fix?'
Status from: 2026-10-23: hebasto: 'I have to admit that I can’t see a real-life scenario in which the bug affects users.'
maflcko (member)suggestion, approachfix migration not to overwrite existing files instead of relying on copy_file with overwrite_existingopenansweredno2026-10-26: 'It seems like a bugfix, because migration shouldn't overwrite?'

Support:

  • maflcko (member) (2026-10-23): Seems harmless to add the test. [not substantive]

Participants:

  • fanquake (member, question, 3 statements 2026-10-23 to 2026-10-23): questioned the motivation, runtime impact, and usefulness of adding a test for a compiler bug
  • maflcko (member, support, 3 statements 2026-10-23 to 2026-10-26): noted adding the test seems harmless, suggested rejecting affected compilers at configure time, and suggested migration shouldn't overwrite
  • theuni (member, neutral, 1 statements 2026-10-23 to 2026-10-23): questioned the usefulness of the test and suggested alternative approaches

Files

26 lines under test/bench/ci.

  • src/test/fs_tests.cpp +26/-0

Card

The short text the ranking pass compares across PRs.

Adds a unit test in fs_tests to verify that fs::copy_file with overwrite_existing works correctly. Certain GCC libstdc++ releases targeting Windows (such as GCC 14.2.0) throw a filesystem error claiming the file exists even when overwrite_existing is specified. Wallet migration relies on this function during BerkeleyRODatabase::Backup, and the new test ensures affected toolchains fail CI fs_tests.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-27T01:28, confidence high, input hash dbb8c9244c6e18d3
thread read through 2026-10-26 (12 statements), head 37c2ad655b
labels: Tests

Categories

Test infrastructure

P3 · test coverage

Adds a unit test in fs_tests to detect an upstream libstdc++ toolchain bug on Windows. The PR description notes that BerkeleyRODatabase::Backup() in src/wallet/migrate.cpp relies on fs::copy_file overwriting existing files, which fails under GCC 12.x to 14.2 cross-compilation with a 'File exists' error.

Membership: Matches 'Tests' label and modifies unit test suite in 'src/test/fs_tests.cpp'.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0