refactor: Simplify code for executing Windows processes

diff --git a/src/ccache/execute.cpp b/src/ccache/execute.cpp
index d9d222e4fb2e63e279a294e2019c87880e383ebe..80e28a723aebf6e67b47b480b3f27bbbde255bcd 100644 (file)
--- a/src/ccache/execute.cpp
+++ b/src/ccache/execute.cpp
@@ -56,8 +56,7 @@
 namespace fs = util::filesystem;
 
 #ifdef _WIN32
-static int win32execute(const char* path,
-                        const char* const* argv,
+static int win32execute(const char* const* argv,
                         int doreturn,
                         int fd_stdout,
                         int fd_stderr,
@@ -71,8 +70,7 @@ execute(Context& ctx,
 {
   LOG("Executing {}", util::format_argv_for_logging(argv));
 
-  return win32execute(argv[0],
-                      argv,
+  return win32execute(argv,
                       1,
                       fd_out.release(),
                       fd_err.release(),
@@ -82,7 +80,7 @@ execute(Context& ctx,
 void
 execute_noreturn(const char* const* argv, const fs::path& temp_dir)
 {
-  win32execute(argv[0], argv, 0, -1, -1, util::pstr(temp_dir).c_str());
+  win32execute(argv, 0, -1, -1, util::pstr(temp_dir).c_str());
 }
 
 std::string
@@ -114,8 +112,7 @@ win32getshell(const std::string& path)
 }
 
 int
-win32execute(const char* path,
-             const char* const* argv,
+win32execute(const char* const* argv,
              int doreturn,
              int fd_stdout,
              int fd_stderr,
@@ -198,11 +195,6 @@ win32execute(const char* path,
   STARTUPINFO si;
   memset(&si, 0x00, sizeof(si));
 
-  std::string sh = win32getshell(path);
-  if (!sh.empty()) {
-    path = sh.c_str();
-  }
-
   si.cb = sizeof(STARTUPINFO);
   if (fd_stdout != -1) {
     si.hStdOutput = (HANDLE)_get_osfhandle(fd_stdout);
@@ -225,10 +217,9 @@ win32execute(const char* path,
     }
   }
 
-  std::string args = util::format_argv_as_win32_command_string(argv, sh);
-  std::string full_path = util::add_exe_suffix(path);
-  fs::path tmp_file_path;
+  std::string args = util::format_argv_as_win32_command_string(argv);
 
+  fs::path tmp_file_path;
   DEFER([&] {
     if (!tmp_file_path.empty()) {
       util::remove(tmp_file_path);
@@ -238,12 +229,17 @@ win32execute(const char* path,
   if (args.length() > 8192) {
     auto tmp_file = util::value_or_throw<core::Fatal>(
       util::TemporaryFile::create(FMT("{}/cmd_args", temp_dir)));
-    args = util::format_argv_as_win32_command_string(argv + 1, sh, true);
+    args = util::format_argv_as_win32_command_string(argv + 1, true);
     util::write_fd(*tmp_file.fd, args.data(), args.length());
-    args = FMT(R"("{}" "@{}")", full_path, tmp_file.path);
+    args = FMT(R"("{}" "@{}")", argv[0], tmp_file.path);
     tmp_file_path = tmp_file.path;
     LOG("Arguments from {}", tmp_file.path);
   }
+
+  std::string sh = win32getshell(argv[0]);
+  if (!sh.empty()) {
+    args = FMT(R"("{}" {})", sh, args);
+  }
   BOOL ret = CreateProcess(nullptr,
                            const_cast<char*>(args.c_str()),
                            nullptr,
@@ -261,7 +257,7 @@ win32execute(const char* path,
   if (ret == 0) {
     DWORD error = GetLastError();
     LOG("failed to execute {}: {} ({})",
-        full_path,
+        argv[0],
         util::win32_error_message(error),
         error);
     return -1;
@@ -272,8 +268,8 @@ win32execute(const char* path,
       TerminateProcess(pi.hProcess, 1);
 
       DWORD error = GetLastError();
-      LOG("failed to assign process to job object {}: {} ({})",
-          full_path,
+      LOG("failed to assign process to job object for {}: {} ({})",
+          argv[0],
           util::win32_error_message(error),
           error);
       return -1;

diff --git a/src/ccache/hashutil.cpp b/src/ccache/hashutil.cpp
index 880bedee5e6db92a9406d9770751e26123b56c50..c8bed7e545f4505df2350d7e20866cedb45dc2a2 100644 (file)
--- a/src/ccache/hashutil.cpp
+++ b/src/ccache/hashutil.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2009-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2009-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -367,6 +367,13 @@ hash_command_output(Hash& hash,
     using_cmd_exe = false;
   }
   Args args = Args::from_string(adjusted_command);
+  {
+    auto full_path =
+      find_executable_in_path(args[0], util::getenv_path_list("PATH")).string();
+    if (!full_path.empty()) {
+      args[0] = full_path;
+    }
+  }
 #else
   Args args = Args::from_string(command);
 #endif
@@ -388,16 +395,6 @@ hash_command_output(Hash& hash,
   STARTUPINFO si;
   memset(&si, 0x00, sizeof(si));
 
-  auto path =
-    find_executable_in_path(args[0], util::getenv_path_list("PATH")).string();
-  if (path.empty()) {
-    path = args[0];
-  }
-  std::string sh = win32getshell(path);
-  if (!sh.empty()) {
-    path = sh;
-  }
-
   si.cb = sizeof(STARTUPINFO);
 
   HANDLE pipe_out[2];
@@ -409,14 +406,18 @@ hash_command_output(Hash& hash,
   si.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
   si.dwFlags = STARTF_USESTDHANDLES;
 
-  std::string win32args;
+  std::string commandline;
   if (using_cmd_exe) {
-    win32args = adjusted_command; // quoted
+    commandline = adjusted_command; // quoted
   } else {
-    win32args = util::format_argv_as_win32_command_string(argv, sh);
+    commandline = util::format_argv_as_win32_command_string(argv);
+    std::string sh = win32getshell(args[0]);
+    if (!sh.empty()) {
+      commandline = FMT(R"("{}" {})", sh, commandline);
+    }
   }
-  BOOL ret = CreateProcess(path.c_str(),
-                           const_cast<char*>(win32args.c_str()),
+  BOOL ret = CreateProcess(nullptr,
+                           const_cast<char*>(commandline.c_str()),
                            nullptr,
                            nullptr,
                            1,

diff --git a/src/ccache/util/string.cpp b/src/ccache/util/string.cpp
index 1597647ec2baf1e4eabe1ca78763647238235fac..54874ffb662857509865ae485ee177d93569fac7 100644 (file)
--- a/src/ccache/util/string.cpp
+++ b/src/ccache/util/string.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -51,14 +51,12 @@ namespace util {
 
 std::string
 format_argv_as_win32_command_string(const char* const* argv,
-                                    const std::string& prefix,
                                     bool escape_backslashes)
 {
   std::string result;
-  size_t i = 0;
-  const char* arg = prefix.empty() ? argv[i++] : prefix.c_str();
 
-  do {
+  for (size_t i = 0; argv[i]; ++i) {
+    const char* arg = argv[i];
     int bs = 0;
     result += '"';
     for (size_t j = 0; arg[j]; ++j) {
@@ -88,7 +86,7 @@ format_argv_as_win32_command_string(const char* const* argv,
       --bs;
     }
     result += "\" ";
-  } while ((arg = argv[i++]));
+  }
 
   result.resize(result.length() - 1);
   return result;

diff --git a/src/ccache/util/string.hpp b/src/ccache/util/string.hpp
index b1c3ae261cdb046416c90823b7400cfcf42eedb5..f78ee085113cf2b595498e736d16ce669af5a9a4 100644 (file)
--- a/src/ccache/util/string.hpp
+++ b/src/ccache/util/string.hpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -50,7 +50,6 @@ bool ends_with(std::string_view string, std::string_view suffix);
 // is not at the end of `argv[i]` either.
 std::string
 format_argv_as_win32_command_string(const char* const* argv,
-                                    const std::string& prefix,
                                     bool escape_backslashes = false);
 
 // Format `argv` as a simple string for logging purposes. That is, the result is

diff --git a/unittest/test_util_string.cpp b/unittest/test_util_string.cpp
index 6714d93e0670b3505b7c39aa5b5e02f127c3f841..25fbb8b855f5e7b1e5e6963b18f8ef1c81d2294c 100644 (file)
--- a/unittest/test_util_string.cpp
+++ b/unittest/test_util_string.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Joel Rosdahl and other contributors
+// Copyright (C) 2021-2025 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -43,34 +43,30 @@ TEST_CASE("util::format_argv_as_win32_command_string")
 {
   {
     const char* const argv[] = {"a", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "") == R"("a")");
-  }
-  {
-    const char* const argv[] = {"a", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "p") == R"("p" "a")");
+    CHECK(util::format_argv_as_win32_command_string(argv) == R"("a")");
   }
   {
     const char* const argv[] = {"a", "b c", "\"d\"", "'e'", "\\\"h", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "")
+    CHECK(util::format_argv_as_win32_command_string(argv)
           == R"("a" "b c" "\"d\"" "'e'" "\\\"h")");
   }
   {
     const char* const argv[] = {"a\\b\\c", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "") == R"("a\b\c")");
+    CHECK(util::format_argv_as_win32_command_string(argv) == R"("a\b\c")");
   }
   {
     const char* const argv[] = {"a\\b\\c", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "", true)
+    CHECK(util::format_argv_as_win32_command_string(argv, true)
           == R"("a\\b\\c")");
   }
   {
     const char* const argv[] = {R"(a\b \"c\" \)", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "")
+    CHECK(util::format_argv_as_win32_command_string(argv)
           == R"("a\b \\\"c\\\" \\")");
   }
   {
     const char* const argv[] = {R"(a\b \"c\" \)", nullptr};
-    CHECK(util::format_argv_as_win32_command_string(argv, "", true)
+    CHECK(util::format_argv_as_win32_command_string(argv, true)
           == R"("a\\b \\\"c\\\" \\")");
   }
 }