subprocess: Backport upstream changes #32358

1. 
   hebasto commented at 4:21 pm on April 27, 2025: member

   Most of these changes were developed during work on #29868 and #32342 and have since been upstreamed.

   As they are now merged, this PR backports them to our src/util/subprocess.h header.

   Required for #29868.

2. subprocess: Fix string_arg when used with rref

   When passing in a rvalue reference, compiler
   considers it ambiguous between std::string and
   std::string&&. Making one of them take a lvalue
   reference makes compilers correctly pick the right
   one depending on whether the passed in value binds
   to a rvalue or lvalue reference.
   
   Github-Pull: arun11299/cpp-subprocess#110
   Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33

   9888d0f511
3. subprocess: Get Windows return code in wait()

   Currently, wait() returns 0 on windows regardless
   of the actual return code of processes.
   
   Github-Pull: arun11299/cpp-subprocess#109
   Rebased-From: 04b015a8e52ead4d8bb5f0eb486419c77e418a17

   9e8992bf8b
4. subprocess: Fix cross-compiling with mingw toolchain

   Github-Pull: arun11299/cpp-subprocess#99
   Rebased-From: 34033d03deacfdba892a708b7d8092b4d9e5e889

   7ecb4e430a
5. 
   DrahtBot commented at 4:21 pm on April 27, 2025: 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/32358.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	laanwj, theStack

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

6. subprocess: Avoid leaking POSIX name aliases beyond `subprocess.h`

   The commit a32c0f3df4b6bcd1d2e93f19e8f380bb890cd507 introduced code to
   silence MSVC's "warning C4996: The POSIX name for this item is
   deprecated."
   
   However, it exhibits several issues:
   1. The aliases may leak into code outside the `subprocess.hpp` header.
   2. They are unnecessarily applied when using the MinGW-w64 toolchain.
   3. The fix is incomplete: downstream projects still see C4996 warnings.
   4. The implementation lacks documentation.
   
   This change addresses all of the above shortcomings.
   
   Github-Pull: arun11299/cpp-subprocess#112
   Rebased-From: 778543b2f2ca7f5d1c4f0241b635bbb265d750dd
   
   Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>

   df7a52241f
7. subprocess: Do not escape double quotes for command line arguments on Windows

   * refactor: Guard `util::quote_argument()` with `#ifdef __USING_WINDOWS__`
   
   The `util::quote_argument()` function is specific to Windows and is used
   in code already guarded by `#ifdef __USING_WINDOWS__`.
   
   * Do not escape double quotes for command line arguments on Windows
   
   This change fixes the handling of double quotes and aligns the behavior
   with Python's `Popen` class. For example:
   ```
   >py -3
   >>> import subprocess
   >>> p = subprocess.Popen("cmd.exe /c dir \"C:\\Program Files\"", stdout=subprocess.PIPE, text=True)
   >>> print(f"Captured stdout:\n{stdout}")
   ```
   
   Currently, the same command line processed by the `quote_argument()`
   function looks like `cmd.exe /c dir "\"C:\Program" "Files\""`, which is
   broken.
   
   With this change, it looks correct: `cmd.exe /c dir "C:\Program Files"`.
   
   Github-Pull: arun11299/cpp-subprocess#113
   Rebased-From: ed313971c04ac10dc006104aba07d016ffc6542a

   bf8230d525
8. subprocess: Proper implementation of wait() on Windows

   This commit makes sure:
   1. WaitForSingleObject returns with expected
   code before proceeding.
   2. Process handle is properly closed.
   
   Github-Pull: arun11299/cpp-subprocess#116
   Rebased-From: 625a8775791e62736f20f3fa3e6cc4f1b24aa89a

   e25ec7300c
9. hebasto force-pushed on Apr 27, 2025
10. laanwj added the label Upstream on Apr 28, 2025
11. 
    laanwj commented at 6:57 am on April 28, 2025: member

    Concept ACK
12. subprocess: check and handle fcntl(F_GETFD) failure

    Add missing error check for fcntl(fd, F_GETFD, 0) in set_clo_on_exec.
    Raise OSError on failure to align with existing FD_SETFD behavior.
    This improves robustness in subprocess setup and error visibility.
    
    Github-Pull: arun11299/cpp-subprocess#117
    Rebased-From: 9974ff69cdd5fc1a2722cb63f006df9308628b35

    7e80030a95
13. 
    hebasto commented at 4:18 pm on April 28, 2025: member

    Added https://github.com/arun11299/cpp-subprocess/pull/117, which is upstreamed #32342.
14. 
    theStack commented at 5:36 pm on April 28, 2025: contributor

    Concept ACK

Contributors
hebasto DrahtBot laanwj theStack

Labels
Upstream