]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
refactor: Extract command execution code from hashutil to util function
authorJoel Rosdahl <joel@rosdahl.net>
Thu, 24 Jul 2025 11:18:40 +0000 (13:18 +0200)
committerJoel Rosdahl <joel@rosdahl.net>
Thu, 24 Jul 2025 14:12:52 +0000 (16:12 +0200)
src/ccache/core/exceptions.hpp
src/ccache/execute.cpp
src/ccache/hashutil.cpp
src/ccache/util/CMakeLists.txt
src/ccache/util/exec.cpp [new file with mode: 0644]
src/ccache/util/exec.hpp [new file with mode: 0644]
unittest/CMakeLists.txt
unittest/test_util_exec.cpp [new file with mode: 0644]

index da0e28808e482fece963a5f091e0592c7bec2210..2fc4dee4da033b183c011f2a5017eb6f7d8b8826 100644 (file)
@@ -49,15 +49,4 @@ class Fatal : public ErrorBase
   using ErrorBase::ErrorBase;
 };
 
-// Call a libc-style function (returns 0 on success and sets errno) and throw
-// Fatal on error.
-#define CHECK_LIB_CALL(function, ...)                                          \
-  {                                                                            \
-    int _result = function(__VA_ARGS__);                                       \
-    if (_result != 0) {                                                        \
-      throw core::Fatal(FMT(#function " failed: {}", strerror(_result)));      \
-    }                                                                          \
-  }                                                                            \
-  static_assert(true) /* allow semicolon after macro */
-
 } // namespace core
index 966252c56e11f8c46f9a549d320b1d21968d9f98..84d2c6d1d3899a2456c374f9d05b4f67a7d10348 100644 (file)
 #  include <signal.h> // NOLINT: sigaddset et al are defined in signal.h
 #endif
 
+// Call a libc-style function (returns 0 on success and sets errno) and throw
+// Fatal on error.
+#define CHECK_LIB_CALL(function, ...)                                          \
+  {                                                                            \
+    int _result = function(__VA_ARGS__);                                       \
+    if (_result != 0) {                                                        \
+      throw core::Fatal(FMT(#function " failed: {}", strerror(_result)));      \
+    }                                                                          \
+  }                                                                            \
+  static_assert(true) /* allow semicolon after macro */
+
 namespace fs = util::filesystem;
 
 #ifdef _WIN32
index 7b56f388f9ea5eebf91a0d308c4a7b1e52b3cf4c..df907b89c2a6eb8df43c701174c2d7eb5c4bdec8 100644 (file)
@@ -27,6 +27,7 @@
 #include <ccache/util/cpu.hpp>
 #include <ccache/util/direntry.hpp>
 #include <ccache/util/environment.hpp>
+#include <ccache/util/exec.hpp>
 #include <ccache/util/file.hpp>
 #include <ccache/util/filesystem.hpp>
 #include <ccache/util/format.hpp>
 #include <ccache/util/path.hpp>
 #include <ccache/util/string.hpp>
 #include <ccache/util/time.hpp>
-#include <ccache/util/wincompat.hpp>
 
 #ifdef INODE_CACHE_SUPPORTED
 #  include <ccache/inodecache.hpp>
 #endif
 
-#ifdef HAVE_SPAWN_H
-#  include <spawn.h>
-#endif
-
-#ifdef HAVE_UNISTD_H
-#  include <unistd.h>
-#endif
-
-#ifdef HAVE_SYS_WAIT_H
-#  include <sys/wait.h>
-#endif
-
 #ifdef HAVE_AVX2
 #  include <immintrin.h>
 #endif
@@ -357,107 +345,14 @@ hash_command_output(Hash& hash,
     }
   }
 
-  auto arg_vector = args.to_argv();
-  auto argv = const_cast<char* const*>(arg_vector.data());
-  LOG("Executing compiler check command {}",
-      util::format_argv_for_logging(argv));
-
-#ifdef _WIN32
-  PROCESS_INFORMATION pi;
-  memset(&pi, 0x00, sizeof(pi));
-  STARTUPINFO si;
-  memset(&si, 0x00, sizeof(si));
-
-  si.cb = sizeof(STARTUPINFO);
-
-  HANDLE pipe_out[2];
-  SECURITY_ATTRIBUTES sa = {sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE};
-  CreatePipe(&pipe_out[0], &pipe_out[1], &sa, 0);
-  SetHandleInformation(pipe_out[0], HANDLE_FLAG_INHERIT, 0);
-  si.hStdOutput = pipe_out[1];
-  si.hStdError = pipe_out[1];
-  si.hStdInput = GetStdHandle(STD_INPUT_HANDLE);
-  si.dwFlags = STARTF_USESTDHANDLES;
-
-  std::string commandline;
-  commandline = util::format_argv_as_win32_command_string(argv);
-  BOOL ret = CreateProcess(nullptr,
-                           const_cast<char*>(commandline.c_str()),
-                           nullptr,
-                           nullptr,
-                           1,
-                           0,
-                           nullptr,
-                           nullptr,
-                           &si,
-                           &pi);
-  CloseHandle(pipe_out[1]);
-  if (ret == 0) {
+  auto result = util::exec_to_string(args);
+  if (!result) {
+    LOG("Error executing compiler check command: {}", result.error());
     return false;
   }
-  int fd = _open_osfhandle((intptr_t)pipe_out[0], O_BINARY);
-  const auto compiler_check_result = hash.hash_fd(fd);
-  if (!compiler_check_result) {
-    LOG("Error hashing compiler check command output: {}",
-        compiler_check_result.error());
-  }
-  WaitForSingleObject(pi.hProcess, INFINITE);
-  DWORD exitcode;
-  GetExitCodeProcess(pi.hProcess, &exitcode);
-  CloseHandle(pipe_out[0]);
-  CloseHandle(pi.hProcess);
-  CloseHandle(pi.hThread);
-  if (exitcode != 0) {
-    LOG("Compiler check command returned {}", exitcode);
-    return false;
-  }
-  return bool(compiler_check_result);
-#else
-  int pipefd[2];
-  if (pipe(pipefd) == -1) {
-    throw core::Fatal(FMT("pipe failed: {}", strerror(errno)));
-  }
-
-  posix_spawn_file_actions_t fa;
-
-  CHECK_LIB_CALL(posix_spawn_file_actions_init, &fa);
-  CHECK_LIB_CALL(posix_spawn_file_actions_addclose, &fa, pipefd[0]);
-  CHECK_LIB_CALL(posix_spawn_file_actions_addclose, &fa, 0);
-  CHECK_LIB_CALL(posix_spawn_file_actions_adddup2, &fa, pipefd[1], 1);
-  CHECK_LIB_CALL(posix_spawn_file_actions_adddup2, &fa, pipefd[1], 2);
 
-  pid_t pid;
-  extern char** environ;
-  int result = posix_spawnp(&pid, argv[0], &fa, nullptr, argv, environ);
-
-  posix_spawn_file_actions_destroy(&fa);
-  close(pipefd[1]);
-
-  const auto hash_result = hash.hash_fd(pipefd[0]);
-  if (!hash_result) {
-    LOG("Error hashing compiler check command output: {}", hash_result.error());
-  }
-  close(pipefd[0]);
-
-  if (result != 0) {
-    LOG("posix_spawnp failed: {}", strerror(errno));
-    return false;
-  }
-
-  int status;
-  while ((result = waitpid(pid, &status, 0)) != pid) {
-    if (result == -1 && errno == EINTR) {
-      continue;
-    }
-    LOG("waitpid failed: {}", strerror(errno));
-    return false;
-  }
-  if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
-    LOG("Compiler check command returned {}", WEXITSTATUS(status));
-    return false;
-  }
-  return bool(hash_result);
-#endif
+  hash.hash(*result);
+  return true;
 }
 
 bool
index ba2f7632b3ad191e827ead2790fbc219e0d19097..b06df0b602144d3bdb4731f87b3628c4cfffb3c8 100644 (file)
@@ -8,6 +8,7 @@ set(
   direntry.cpp
   environment.cpp
   error.cpp
+  exec.cpp
   file.cpp
   filelock.cpp
   filesystem.cpp
diff --git a/src/ccache/util/exec.cpp b/src/ccache/util/exec.cpp
new file mode 100644 (file)
index 0000000..11cded4
--- /dev/null
@@ -0,0 +1,163 @@
+// Copyright (C) 2025 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#include "exec.hpp"
+
+#include <ccache/util/environment.hpp>
+#include <ccache/util/error.hpp>
+#include <ccache/util/fd.hpp>
+#include <ccache/util/file.hpp>
+#include <ccache/util/filesystem.hpp>
+#include <ccache/util/format.hpp>
+#include <ccache/util/logging.hpp>
+#include <ccache/util/path.hpp>
+#include <ccache/util/string.hpp>
+#include <ccache/util/wincompat.hpp>
+
+#ifdef HAVE_SPAWN_H
+#  include <spawn.h>
+#endif
+
+#ifdef HAVE_SYS_WAIT_H
+#  include <sys/wait.h>
+#endif
+
+#ifndef environ
+DLLIMPORT extern char** environ;
+#endif
+
+namespace fs = util::filesystem;
+
+// Call a libc-style function (returns 0 on success and sets errno) and return
+// tl::unexpected on error.
+#define CHECK_LIB_CALL(function, ...)                                          \
+  {                                                                            \
+    int _result = function(__VA_ARGS__);                                       \
+    if (_result != 0) {                                                        \
+      return tl::unexpected(FMT(#function " failed: {}", strerror(_result)));  \
+    }                                                                          \
+  }                                                                            \
+  static_assert(true) /* allow semicolon after macro */
+
+namespace util {
+
+tl::expected<std::string, std::string>
+exec_to_string(const Args& args)
+{
+  auto argv = args.to_argv();
+  LOG("Executing command: {}", format_argv_for_logging(argv.data()));
+
+  std::string output;
+
+#ifdef _WIN32
+  PROCESS_INFORMATION pi;
+  memset(&pi, 0x00, sizeof(pi));
+  STARTUPINFO si;
+  memset(&si, 0x00, sizeof(si));
+
+  si.cb = sizeof(STARTUPINFO);
+
+  HANDLE read_handle;
+  HANDLE write_handle;
+  SECURITY_ATTRIBUTES sa = {sizeof(SECURITY_ATTRIBUTES), nullptr, TRUE};
+  CreatePipe(&read_handle, &write_handle, &sa, 0);
+  SetHandleInformation(read_handle, HANDLE_FLAG_INHERIT, 0);
+  si.hStdOutput = write_handle;
+  si.hStdError = write_handle;
+  si.hStdInput = nullptr;
+  si.dwFlags = STARTF_USESTDHANDLES;
+
+  std::string commandline = format_argv_as_win32_command_string(argv.data());
+  BOOL ret = CreateProcess(nullptr,
+                           const_cast<char*>(commandline.c_str()),
+                           nullptr,
+                           nullptr,
+                           1, // inherit handles
+                           0, // no console window
+                           nullptr,
+                           nullptr,
+                           &si,
+                           &pi);
+  CloseHandle(write_handle);
+  if (ret == 0) {
+    CloseHandle(read_handle);
+    DWORD error = GetLastError();
+    return tl::unexpected(
+      FMT("CreateProcess failure: {} ({})", win32_error_message(error), error));
+  }
+  char buffer[4096];
+  DWORD bytes_read = 0;
+  while (ReadFile(read_handle, buffer, sizeof(buffer), &bytes_read, nullptr)
+         && bytes_read > 0) {
+    output.append(buffer, bytes_read);
+  }
+
+  CloseHandle(read_handle);
+  WaitForSingleObject(pi.hProcess, INFINITE);
+
+  DWORD exitcode;
+  GetExitCodeProcess(pi.hProcess, &exitcode);
+  CloseHandle(pi.hProcess);
+  CloseHandle(pi.hThread);
+  if (exitcode != 0) {
+    return tl::unexpected(FMT("Non-zero exit code: {}", exitcode));
+  }
+#else
+  int pipefd[2];
+  CHECK_LIB_CALL(pipe, pipefd);
+
+  posix_spawn_file_actions_t fa;
+  CHECK_LIB_CALL(posix_spawn_file_actions_init, &fa);
+  CHECK_LIB_CALL(posix_spawn_file_actions_addclose, &fa, pipefd[0]);
+  CHECK_LIB_CALL(posix_spawn_file_actions_addclose, &fa, 0);
+  CHECK_LIB_CALL(posix_spawn_file_actions_adddup2, &fa, pipefd[1], 1);
+  CHECK_LIB_CALL(posix_spawn_file_actions_adddup2, &fa, pipefd[1], 2);
+
+  pid_t pid;
+  auto argv_mutable = const_cast<char* const*>(argv.data());
+  int result = posix_spawnp(&pid, argv[0], &fa, nullptr, argv_mutable, environ);
+  int saved_errno = errno;
+  posix_spawn_file_actions_destroy(&fa);
+  close(pipefd[1]);
+
+  if (result != 0) {
+    close(pipefd[0]);
+    return tl::unexpected(
+      FMT("posix_spawnp failed: {}", strerror(saved_errno)));
+  }
+
+  read_fd(pipefd[0], [&](auto data) {
+    output.append(reinterpret_cast<const char*>(data.data()), data.size());
+  });
+
+  int status;
+  while ((result = waitpid(pid, &status, 0)) != pid) {
+    if (result == -1 && errno == EINTR) {
+      continue;
+    }
+    return tl::unexpected(FMT("waitpid failed: {}", strerror(errno)));
+  }
+  if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+    return tl::unexpected(FMT("Non-zero exit code: {}", WEXITSTATUS(status)));
+  }
+#endif
+
+  return output;
+}
+
+} // namespace util
diff --git a/src/ccache/util/exec.hpp b/src/ccache/util/exec.hpp
new file mode 100644 (file)
index 0000000..5c0cfee
--- /dev/null
@@ -0,0 +1,33 @@
+// Copyright (C) 2025 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#pragma once
+
+#include <ccache/util/args.hpp>
+
+#include <tl/expected.hpp>
+
+#include <string>
+
+namespace util {
+
+// Execute command in `args` and capture output (stdout and stderr combined),
+// similar to execvp(2)/popen(3).
+tl::expected<std::string, std::string> exec_to_string(const Args& args);
+
+} // namespace util
index 746c1e0569cdabf36972027cf1d20139905c62af..369ec9c55ab5aadceddbded0eadf4cfb456240b6 100644 (file)
@@ -25,6 +25,7 @@ set(
   test_util_direntry.cpp
   test_util_duration.cpp
   test_util_environment.cpp
+  test_util_exec.cpp
   test_util_expected.cpp
   test_util_file.cpp
   test_util_lockfile.cpp
diff --git a/unittest/test_util_exec.cpp b/unittest/test_util_exec.cpp
new file mode 100644 (file)
index 0000000..e0d1e80
--- /dev/null
@@ -0,0 +1,62 @@
+// Copyright (C) 2025 Joel Rosdahl and other contributors
+//
+// See doc/AUTHORS.adoc for a complete list of contributors.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation; either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along with
+// this program; if not, write to the Free Software Foundation, Inc., 51
+// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+#include "testutil.hpp"
+
+#include <ccache/util/exec.hpp>
+#include <ccache/util/file.hpp>
+#include <ccache/util/string.hpp>
+
+#include <doctest/doctest.h>
+
+using TestUtil::TestContext;
+
+TEST_CASE("util::exec_to_string")
+{
+  using util::exec_to_string;
+
+  TestContext test_context;
+
+  SUBCASE("stdout + stderr")
+  {
+#ifdef _WIN32
+    util::write_file("command.bat", "@echo off\r\necho fisk\r\necho sork>&2");
+    util::Args args{"command.bat"};
+#else
+    util::Args args{"sh", "-c", "echo fisk; echo sork >&2"};
+#endif
+    auto result = exec_to_string(args);
+    REQUIRE(result);
+#ifdef _WIN32
+    CHECK(*result == "fisk\r\nsork\r\n");
+#else
+    CHECK(*result == "fisk\nsork\n");
+#endif
+  }
+
+  SUBCASE("error")
+  {
+    auto result = exec_to_string({"doesnotexist"});
+    REQUIRE(!result);
+#ifdef _WIN32
+    CHECK(util::starts_with(result.error(), "CreateProcess failure: "));
+#else
+    CHECK(result.error() == "posix_spawnp failed: No such file or directory");
+#endif
+  }
+}