From e388ed687af7f67b2c514d2b88bf5e5645862e25 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 10 Feb 2025 19:40:52 -0500 Subject: execute_process: Improve invocation of .cmd/.bat with spaces Extend the fix from commit 74c9d40876 (execute_process: Fix invocation of .cmd/.bat with spaces, 2025-01-31) to work without relying on conversion to a "short path", which may not exist. Instead, extending the `cmd /c` wrapper to `cmd /c call` seems to support spaces directly. Suggested-by: Alexandru Croitor Fixes: #26655 --- Help/command/execute_process.rst | 4 ++-- Source/cmExecuteProcessCommand.cxx | 11 +---------- Source/cmSystemTools.cxx | 17 +++++------------ Source/cmSystemTools.h | 19 +++++++------------ Tests/RunCMake/execute_process/RunCMakeTest.cmake | 6 +----- 5 files changed, 16 insertions(+), 41 deletions(-) diff --git a/Help/command/execute_process.rst b/Help/command/execute_process.rst index 510059a..e7729cc 100644 --- a/Help/command/execute_process.rst +++ b/Help/command/execute_process.rst @@ -54,8 +54,8 @@ Options: * .. versionchanged:: 4.0 On Windows platforms, if the command runs a ``.bat`` or ``.cmd`` script, - it is automatically executed through the command interpreter, ``cmd /c``. - However, paths with spaces may fail if a "short path" is not available. + it is automatically executed through the command interpreter with + ``cmd /c call ...``. No intermediate shell is used, so shell operators such as ``>`` are treated as normal arguments. diff --git a/Source/cmExecuteProcessCommand.cxx b/Source/cmExecuteProcessCommand.cxx index 750c09d..649b88f 100644 --- a/Source/cmExecuteProcessCommand.cxx +++ b/Source/cmExecuteProcessCommand.cxx @@ -166,16 +166,7 @@ bool cmExecuteProcessCommand(std::vector const& args, status.SetError(" given COMMAND argument with no value."); return false; } -#ifdef _WIN32 - cmsys::Status shortPathRes = cmSystemTools::MaybePrependCmdExe(cmd); - if (!shortPathRes) { - status.GetMakefile().IssueMessage( - MessageType::WARNING, - cmStrCat("Conversion of COMMAND:\n ", cmd[2], '\n', - "to a short path without spaces failed:\n ", - shortPathRes.GetString())); - } -#endif + cmSystemTools::MaybePrependCmdExe(cmd); } // Parse the timeout string. diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 8e9deb1..8036358 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -795,38 +795,31 @@ std::size_t cmSystemTools::CalculateCommandLineLengthLimit() return sz; } -cmsys::Status cmSystemTools::MaybePrependCmdExe( - std::vector& cmdLine) +void cmSystemTools::MaybePrependCmdExe(std::vector& cmdLine) { #if defined(_WIN32) && !defined(__CYGWIN__) - cmsys::Status status; if (!cmdLine.empty()) { std::string& applicationName = cmdLine.at(0); static cmsys::RegularExpression const winCmdRegex( "\\.([Bb][Aa][Tt]|[Cc][Mm][Dd])$"); cmsys::RegularExpressionMatch winCmdMatch; if (winCmdRegex.find(applicationName.c_str(), winCmdMatch)) { + // Wrap `.bat` and `.cmd` commands with `cmd /c call`. std::vector output; - output.reserve(cmdLine.size() + 2); + output.reserve(cmdLine.size() + 3); output.emplace_back(cmSystemTools::GetComspec()); output.emplace_back("/c"); + output.emplace_back("call"); // Convert the batch file path to use backslashes for cmd.exe to parse. std::replace(applicationName.begin(), applicationName.end(), '/', '\\'); - if (applicationName.find(' ') != std::string::npos) { - // Convert the batch file path to a short path to avoid spaces. - // Otherwise, cmd.exe may not handle arguments with spaces. - status = cmSystemTools::GetShortPath(applicationName, applicationName); - } - output.push_back(applicationName); + output.emplace_back(applicationName); std::move(cmdLine.begin() + 1, cmdLine.end(), std::back_inserter(output)); cmdLine = std::move(output); } } - return status; #else static_cast(cmdLine); - return cmsys::Status::Success(); #endif } diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 60a7095..e2720ca 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -235,23 +235,18 @@ public: std::string const& destination); /** - * According to the CreateProcessW documentation which is the underlying - * function for all RunProcess calls: + * According to the CreateProcessW documentation: * - * "To run a batch file, you must start the command interpreter; set" - * "lpApplicationName to cmd.exe and set lpCommandLine to the following" - * "arguments: /c plus the name of the batch file." + * To run a batch file, you must start the command interpreter; set + * lpApplicationName to cmd.exe and set lpCommandLine to the following + * arguments: /c plus the name of the batch file. * - * we should to take care of the correctness of the command line when - * attempting to execute the batch files. - * - * Also cmd.exe is unable to parse batch file names correctly if they - * contain spaces. This function uses cmSystemTools::GetShortPath - * conversion to suppress this behavior, and returns its status. + * Additionally, "cmd /c" does not always parse batch file names correctly + * if they contain spaces, but using "cmd /c call" seems to work. * * The function is noop on platforms different from the pure WIN32 one. */ - static cmsys::Status MaybePrependCmdExe(std::vector& cmdLine); + static void MaybePrependCmdExe(std::vector& cmdLine); /** * Run a single executable command diff --git a/Tests/RunCMake/execute_process/RunCMakeTest.cmake b/Tests/RunCMake/execute_process/RunCMakeTest.cmake index be2c652..9124afa 100644 --- a/Tests/RunCMake/execute_process/RunCMakeTest.cmake +++ b/Tests/RunCMake/execute_process/RunCMakeTest.cmake @@ -63,11 +63,7 @@ if(WIN32 OR CYGWIN) run_cmake_command(WindowsNoExtension-build ${CMAKE_COMMAND} --build . --config Debug --target RunScript) endif() -if(CMAKE_HOST_WIN32 - # By default, only C: has short paths enabled. - # Since querying with `fsutil 8dot3name query C:` - # requires admin, just test the drive letter. - AND RunCMake_SOURCE_DIR MATCHES "^[Cc]:") +if(CMAKE_HOST_WIN32) run_cmake_command(WindowsBatch ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/WindowsBatch.cmake) endif() -- cgit v0.12 From b902fbd0c68ce9eba308302dbec8adff6a1166e3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 11 Feb 2025 07:21:51 -0500 Subject: execute_process: Clarify when a Windows command interpreter is used We've always executed `.bat` and `.cmd` files through `cmd /c`, but previously it was implicit in undocumented `CreateProcessW` behavior. --- Help/command/execute_process.rst | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/Help/command/execute_process.rst b/Help/command/execute_process.rst index e7729cc..c2c7fe3 100644 --- a/Help/command/execute_process.rst +++ b/Help/command/execute_process.rst @@ -44,23 +44,31 @@ Options: CMake executes the child process using operating system APIs directly: - * On POSIX platforms, the command line is passed to the - child process in an ``argv[]`` style array. + * On POSIX platforms, the command line is passed to the child process + in an ``argv[]`` style array. No intermediate shell is executed, + so shell operators such as ``>`` are treated as normal arguments. * On Windows platforms, the command line is encoded as a string such that child processes using `CommandLineToArgvW`_ will decode the - original arguments. If the command runs a ``.bat`` or ``.cmd`` - script, it may receive arguments with extra quoting. + original arguments. - * .. versionchanged:: 4.0 - On Windows platforms, if the command runs a ``.bat`` or ``.cmd`` script, - it is automatically executed through the command interpreter with - ``cmd /c call ...``. + If the command runs a ``.exe``, ``.com``, or other executable, + no intermediate command interpreter is executed, so shell operators + such as ``>`` are treated as normal arguments. - No intermediate shell is used, so shell operators such as ``>`` - are treated as normal arguments. - (Use the ``INPUT_*``, ``OUTPUT_*``, and ``ERROR_*`` options to - redirect stdin, stdout, and stderr.) + If the command runs a ``.bat`` or ``.cmd`` script, it is executed + through the ``cmd`` command interpreter. The command interpreter + does not use `CommandLineToArgvW`_, so some arguments may be received + by the script with extra quoting. + + .. versionchanged:: 4.0 + ``.bat`` and ``.cmd`` scripts are now explicitly executed through the + command interpreter by prepending ``cmd /c call`` to the command line. + Previously, they were implicitly executed through ``cmd /c``, without + ``call``, by undocumented behavior of `CreateProcessW`_. + + Use the ``INPUT_*``, ``OUTPUT_*``, and ``ERROR_*`` options to + redirect stdin, stdout, and stderr. For **sequential execution** of multiple commands use multiple ``execute_process`` calls each with a single ``COMMAND`` argument. @@ -205,3 +213,4 @@ Options: :variable:`CMAKE_EXECUTE_PROCESS_COMMAND_ERROR_IS_FATAL` is ignored. .. _`CommandLineToArgvW`: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw +.. _`CreateProcessW`: https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw -- cgit v0.12