build: Fix `::_wsystem` check #25425

The ::_wsystem check has been introduced in bitcoin/bitcoin#15457, and it is broken.

An excerpt from config.log for ./autogen.sh && ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site on master (a09033e22c4f072c86a1885dd476f3059e5416d1):

configure:29111: checking for ::_wsystem
configure:29125: x86_64-w64-mingw32-g++-posix -std=c++17 -o conftest.exe -pipe -std=c++17 -O2  -I/home/hebasto/git/bitcoin/depends/x86_64-w64-mingw32/include/  -L/home/hebasto/git/bitcoin/depends/x86_64-w64-mingw32/lib  conftest.cpp -lssp -liphlpapi -lshlwapi -lws2_32 -ladvapi32 -luuid -loleaut32 -lole32 -lcomctl32 -lshell32 -lwinmm -lcomdlg32 -lgdi32 -luser32 -lkernel32  >&5
conftest.cpp: In function 'int main()':
conftest.cpp:81:15: error: '::_wsystem' has not been declared
   81 |  int nErr = ::_wsystem("");
      |               ^~~~~~~~
configure:29125: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "Bitcoin Core"
| #define PACKAGE_TARNAME "bitcoin"
| #define PACKAGE_VERSION "23.99.0"
| #define PACKAGE_STRING "Bitcoin Core 23.99.0"
| #define PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues"
| #define PACKAGE_URL "https://bitcoincore.org/"
| #define HAVE_CXX17 1
| #define HAVE_STDIO_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_STRING_H 1
| #define HAVE_INTTYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_UNISTD_H 1
| #define STDC_HEADERS 1
| #define LT_OBJDIR ".libs/"
| #define USE_ASM 1
| #define HAVE_CLMUL 1
| #define ENABLE_SSE41 1
| #define ENABLE_AVX2 1
| #define ENABLE_X86_SHANI 1
| #define HAVE_LIBKERNEL32 1
| #define HAVE_LIBUSER32 1
| #define HAVE_LIBGDI32 1
| #define HAVE_LIBCOMDLG32 1
| #define HAVE_LIBWINMM 1
| #define HAVE_LIBSHELL32 1
| #define HAVE_LIBCOMCTL32 1
| #define HAVE_LIBOLE32 1
| #define HAVE_LIBOLEAUT32 1
| #define HAVE_LIBUUID 1
| #define HAVE_LIBADVAPI32 1
| #define HAVE_LIBWS2_32 1
| #define HAVE_LIBSHLWAPI 1
| #define HAVE_LIBIPHLPAPI 1
| #define HAVE_PTHREAD_PRIO_INHERIT 1
| #define HAVE_PTHREAD 1
| #define _FILE_OFFSET_BITS 64
| #define HAVE_DECL_STRERROR_R 0
| #define HAVE_LIBSSP 1
| #define HAVE_STDIO_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_UNISTD_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_DECL_GETIFADDRS 0
| #define HAVE_DECL_FREEIFADDRS 0
| #define HAVE_DECL_FORK 0
| #define HAVE_DECL_SETSID 0
| #define HAVE_DECL_PIPE2 0
| #define HAVE_DECL_LE16TOH 0
| #define HAVE_DECL_LE32TOH 0
| #define HAVE_DECL_LE64TOH 0
| #define HAVE_DECL_HTOLE16 0
| #define HAVE_DECL_HTOLE32 0
| #define HAVE_DECL_HTOLE64 0
| #define HAVE_DECL_BE16TOH 0
| #define HAVE_DECL_BE32TOH 0
| #define HAVE_DECL_BE64TOH 0
| #define HAVE_DECL_HTOBE16 0
| #define HAVE_DECL_HTOBE32 0
| #define HAVE_DECL_HTOBE64 0
| #define HAVE_DECL_BSWAP_16 0
| #define HAVE_DECL_BSWAP_32 0
| #define HAVE_DECL_BSWAP_64 0
| #define HAVE_BUILTIN_CLZL 1
| #define HAVE_BUILTIN_CLZLL 1
| #define HAVE_DEFAULT_VISIBILITY_ATTRIBUTE 1
| #define HAVE_DLLEXPORT_ATTRIBUTE 1
| #define HAVE_FDATASYNC 0
| #define HAVE_O_CLOEXEC 0
| /* end confdefs.h.  */
| 
| int
| main (void)
| {
|  int nErr = ::_wsystem("");
| 
|   ;
|   return 0;
| }
configure:29130: result: no

See https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem

Friendly ping @Sjors :)

I wonder, what is the effect of this? Apparently it was working fine without the check?

I wonder, what is the effect of this? Apparently it was working fine without the check?

At least for the mingw32 host, the HAVE_SYSTEM macro is defined due to the positive result of the std::system check.

At least for the mingw32 host, the HAVE_SYSTEM macro is defined due to the positive result of the std::system check.

Ah yes. Sneaky.

Ideally it'd make sense to have separate defines for the different functions, then one that gets set if either is available. E.g.

HAVE_SYSTEM
HAVE_WSYSTEM
HAVE_ANY_SYSTEM

But it's probably overkill.

I have no idea how I even wrote this back then. I also can't test it, because I haven't tried building for iOs in ages.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25218 (refactor: introduce generic 'Result' classes and connect them to CreateTransaction and GetNewDestination by furszy)

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.

I have no idea how I even wrote this back then. I also can't test it, because I haven't tried building for iOs in ages.

I suspect that works correctly as iOS has neither _wsystem() nor system(). I don't think this particular mistake ever caused a real problem (there is apparently no OS that has _wsystem but not system?) , still, it's better to fix it.

Code review ACK b5f6a4650334d58245b45eace57f2bc23467ffc7

win64 build in CI:

checking for std::system... yes
checking for ::_wsystem... yes

linux build:

checking for std::system... yes
checking for ::_wsystem... no