windows: remove deprecated codecvt via UTF-8 narrow APIs #35704

kevkevinpal commented at 9:19 PM on July 12, 2026: contributor

Since #32380 the Windows process code page is UTF-8, so narrow APIs accept UTF-8 directly. Drop wstring_convert/codecvt and the related wide process calls (_wsystem, _wexecvp, CreateProcessW) in favor of ::system, _execvp, and CreateProcess.

This should be fine to remove since Bitcoin Core is now on C++20

DrahtBot commented at 9:20 PM on July 12, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35704.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	hebasto, hodlinator

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34995 (iwyu: Fix warnings in src/common and treat them as errors by hebasto)
#32387 (ipc: add windows support by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake commented at 7:54 AM on July 13, 2026: member

cc @hebasto

in src/util/exec.cpp:31 in d8e64c1e85

  26 | @@ -29,16 +27,16 @@ int ExecVp(const char* file, char* const argv[])
  27 |      return execvp(file, argv);
  28 |  #else
  29 |      std::vector<std::wstring> escaped_args;
  30 | -    std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> converter;
  31 |      for (char* const* arg_ptr{argv}; *arg_ptr; ++arg_ptr) {
  32 | -        subprocess::util::quote_argument(converter.from_bytes(*arg_ptr), escaped_args.emplace_back(), false);
  33 | +        subprocess::util::quote_argument(fs::u8path(*arg_ptr).wstring(), escaped_args.emplace_back(), false);

maflcko commented at 8:19 AM on July 13, 2026:

nit: Could use named args here for the bool literal while touching?

kevkevinpal commented at 3:16 PM on July 16, 2026:

Yup, makes sense, and thank you for the review! Updated in abb2983

hebasto commented at 8:26 AM on July 13, 2026: member

Concept ACK.

in src/common/system.cpp:56 in d8e64c1e85

  52 | @@ -53,7 +53,7 @@ void runCommand(const std::string& strCommand)
  53 |  #ifndef WIN32
  54 |      int nErr = ::system(strCommand.c_str());
  55 |  #else
  56 | -    int nErr = ::_wsystem(std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>,wchar_t>().from_bytes(strCommand).c_str());
  57 | +    int nErr = ::_wsystem(fs::u8path(strCommand).wstring().c_str());

hebasto commented at 3:03 PM on July 14, 2026:

This code was originally added to make runCommand handle UTF-8 command strings. Since #32380, the active code page is guaranteed to be UTF-8, so the narrow ::system() accepts UTF-8 directly and the wide conversion is no longer needed. Therefore, the correct solution here is to revert 23db9546c16c2be264cfc4f695f5738a2f5beeeb

kevkevinpal commented at 3:15 PM on July 16, 2026:

Thank you for the review. This should now be reverted in abb2983

hebasto commented at 3:40 PM on July 14, 2026: member

cc @ryanofsky

in src/util/exec.cpp:39 in d8e64c1e85

  37 |      new_argv.reserve(escaped_args.size() + 1);
  38 |      for (const auto& s : escaped_args) new_argv.push_back(s.c_str());
  39 |      new_argv.push_back(nullptr);
  40 | -    return _wexecvp(converter.from_bytes(file).c_str(), new_argv.data());
  41 | +    const std::wstring wfile{fs::u8path(file).wstring()};
  42 | +    return _wexecvp(wfile.c_str(), new_argv.data());

hebasto commented at 4:03 PM on July 14, 2026:

Given #32380, could we use a narrow execvp() here? Perhaps this will require templatizing subprocess::util::quote_argument.

kevkevinpal commented at 3:15 PM on July 16, 2026:

Thank you for the review, this should now be resolved in abb2983

kevkevinpal force-pushed on Jul 16, 2026

in src/common/system.cpp:56 in abb29836f5 outdated

  48 | @@ -50,11 +49,7 @@ std::string ShellEscape(const std::string& arg)
  49 |  void runCommand(const std::string& strCommand)
  50 |  {
  51 |      if (strCommand.empty()) return;
  52 | -#ifndef WIN32
  53 |      int nErr = ::system(strCommand.c_str());
  54 | -#else
  55 | -    int nErr = ::_wsystem(std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>,wchar_t>().from_bytes(strCommand).c_str());

hebasto commented at 2:37 PM on July 29, 2026:

The following check can also be removed now: https://github.com/bitcoin/bitcoin/blob/7e5952b0aa04429c88d8ad990f35862421c4fa9d/cmake/introspection.cmake#L27

kevkevinpal commented at 3:42 PM on August 2, 2026:

Thanks, this is updated in 00a9ef2

hebasto commented at 2:47 PM on July 29, 2026: member

Approach ACK abb29836f5e8e04f6abb2b83ce4a532f291026e1.

Have you considered rewriting Windows part of Popen::execute_process() using CreateProcess rather than CreateProcessW?

kevkevinpal force-pushed on Aug 2, 2026

kevkevinpal renamed this:
~~windows: replace deprecated codecvt with fs::u8path~~
windows: remove deprecated codecvt via UTF-8 narrow APIs
on Aug 2, 2026

kevkevinpal commented at 3:42 PM on August 2, 2026: contributor

Have you considered rewriting Windows part of Popen::execute_process() using CreateProcess rather than CreateProcessW?

No I had not but this makes sense, updated in 00a9ef2

DrahtBot added the label CI failed on Aug 2, 2026

kevkevinpal force-pushed on Aug 2, 2026

DrahtBot removed the label CI failed on Aug 2, 2026

hebasto commented at 2:11 PM on August 3, 2026: member

Almost ACK 00a9ef232af24f7d2649d992377aade751ba76b6. Going to build Windows "release" binaries in Guix and test them.

hebasto commented at 2:12 PM on August 3, 2026: member

Friendly ping @davidgumberg @hodlinator @Sjors @ryanofsky

hebasto approved

hebasto commented at 3:36 PM on August 3, 2026: member

ACK 00a9ef232af24f7d2649d992377aade751ba76b6.

Going to build Windows "release" binaries in Guix and test them.

Built bitcoin-00a9ef232af2-win64-unsigned.zip and tested its binaries on Windows 11.

DrahtBot added the label Needs rebase on Aug 4, 2026

windows: remove deprecated codecvt via UTF-8 narrow APIs

Drop wstring_convert/codecvt and the
related wide process calls (_wsystem, _wexecvp, CreateProcessW)
in favor of ::system, _execvp, and CreateProcess.

6b6d77cc84

kevkevinpal force-pushed on Aug 6, 2026

kevkevinpal commented at 11:22 PM on August 6, 2026: contributor

fixed conflict and rebased to 6b6d77c

DrahtBot removed the label Needs rebase on Aug 7, 2026

hebasto approved

hebasto commented at 11:02 AM on August 7, 2026: member

re-ACK 6b6d77cc84e4b08641bc2f3fd3c4cf2a22ffdddf, only rebased since my recent review.

in src/util/subprocess.h:165 in 6b6d77cc84

 161 | @@ -168,26 +162,29 @@ class OSError: public std::runtime_error
 162 |  namespace util
 163 |  {
 164 |  #ifdef WIN32
 165 | -  inline void quote_argument(const std::wstring &argument, std::wstring &command_line,
 166 | +  inline void quote_argument(const std::string &argument, std::string &command_line,

hodlinator commented at 12:48 PM on August 7, 2026:

nit: Could potentially switch to string_view here and at the call site to avoid potential heap allocation from string. (Builds successfully on Windows CI - https://github.com/hodlinator/bitcoin/actions/runs/31177630989/job/92863107266).

diff --git a/src/util/exec.cpp b/src/util/exec.cpp
index 69361fbc51..9a942eff58 100644
--- a/src/util/exec.cpp
+++ b/src/util/exec.cpp
@@ -28,7 +28,7 @@ int ExecVp(const char* file, char* const argv[])
 #else
     std::vector<std::string> escaped_args;
     for (char* const* arg_ptr{argv}; *arg_ptr; ++arg_ptr) {
-        subprocess::util::quote_argument(std::string{*arg_ptr}, escaped_args.emplace_back(), /*force=*/false);
+        subprocess::util::quote_argument(std::string_view{*arg_ptr}, escaped_args.emplace_back(), /*force=*/false);
     }
 
     std::vector<const char*> new_argv;
diff --git a/src/util/subprocess.h b/src/util/subprocess.h
index 5a64b116ad..8a7c5e5244 100644
--- a/src/util/subprocess.h
+++ b/src/util/subprocess.h
@@ -162,7 +162,7 @@ public:
 namespace util
 {
 #ifdef WIN32
-  inline void quote_argument(const std::string &argument, std::string &command_line,
+  inline void quote_argument(std::string_view argument, std::string &command_line,
                       bool force)
   {
     constexpr char quote = '"';

hodlinator commented at 1:19 PM on August 7, 2026: contributor

ACK 6b6d77cc84e4b08641bc2f3fd3c4cf2a22ffdddf

https://devblogs.microsoft.com/cppblog/c17-feature-removals-and-deprecations/ confirms that _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING is a way to silence warnings coming from using wstring conversion functions, which we no longer use.

fanquake merged this on Aug 7, 2026

fanquake closed this on Aug 7, 2026