Merge topic 'ctest-empty-binary'

bcb396996c cmCTestEmptyBinaryDirectoryCommand: turn into a free function
9a5525cae7 cmCTestSleepCommand: implement as free function
f7ed0bd2f3 cmCTestCommand: remove CTestScriptHandler member
1440b05bf1 cmMakefile: invoke callback after command execution

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !9869

Source/CTest/cmCTestBuildCommand.cxx

@@ -24,7 +24,6 @@ std::unique_ptr<cmCommand> cmCTestBuildCommand::Clone()
 {
   auto ni = cm::make_unique<cmCTestBuildCommand>();
   ni->CTest = this->CTest;
-  ni->CTestScriptHandler = this->CTestScriptHandler;
   return std::unique_ptr<cmCommand>(std::move(ni));
 }
 

Source/CTest/cmCTestCommand.h

@@ -5,7 +5,6 @@
 #include "cmCommand.h"
 
 class cmCTest;
-class cmCTestScriptHandler;
 
 /** \class cmCTestCommand
  * \brief A superclass for all commands added to the CTestScriptHandler
@@ -17,12 +16,5 @@ class cmCTestScriptHandler;
 class cmCTestCommand : public cmCommand
 {
 public:
-  cmCTestCommand()
-  {
-    this->CTest = nullptr;
-    this->CTestScriptHandler = nullptr;
-  }
-
-  cmCTest* CTest;
-  cmCTestScriptHandler* CTestScriptHandler;
+  cmCTest* CTest = nullptr;
 };

Source/CTest/cmCTestConfigureCommand.h

@@ -29,7 +29,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestConfigureCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestCoverageCommand.h

@@ -32,7 +32,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestCoverageCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestEmptyBinaryDirectoryCommand.cxx

@@ -2,22 +2,98 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCTestEmptyBinaryDirectoryCommand.h"
 
-#include "cmCTestScriptHandler.h"
+#include "cmsys/Directory.hxx"
+
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmStringAlgorithms.h"
+#include "cmSystemTools.h"
 
-bool cmCTestEmptyBinaryDirectoryCommand::InitialPass(
-  std::vector<std::string> const& args, cmExecutionStatus& status)
+namespace {
+
+// Try to remove the binary directory once
+cmsys::Status TryToRemoveBinaryDirectoryOnce(const std::string& directoryPath)
+{
+  cmsys::Directory directory;
+  directory.Load(directoryPath);
+
+  // Make sure that CMakeCache.txt is deleted last.
+  for (unsigned long i = 0; i < directory.GetNumberOfFiles(); ++i) {
+    std::string path = directory.GetFile(i);
+
+    if (path == "." || path == ".." || path == "CMakeCache.txt") {
+      continue;
+    }
+
+    std::string fullPath = cmStrCat(directoryPath, "/", path);
+
+    bool isDirectory = cmSystemTools::FileIsDirectory(fullPath) &&
+      !cmSystemTools::FileIsSymlink(fullPath);
+
+    cmsys::Status status;
+    if (isDirectory) {
+      status = cmSystemTools::RemoveADirectory(fullPath);
+    } else {
+      status = cmSystemTools::RemoveFile(fullPath);
+    }
+    if (!status) {
+      return status;
+    }
+  }
+
+  return cmSystemTools::RemoveADirectory(directoryPath);
+}
+
+/*
+ * Empty Binary Directory
+ */
+bool EmptyBinaryDirectory(const std::string& sname, std::string& err)
+{
+  // try to avoid deleting root
+  if (sname.size() < 2) {
+    err = "path too short";
+    return false;
+  }
+
+  // consider non existing target directory a success
+  if (!cmSystemTools::FileExists(sname)) {
+    return true;
+  }
+
+  // try to avoid deleting directories that we shouldn't
+  std::string check = cmStrCat(sname, "/CMakeCache.txt");
+
+  if (!cmSystemTools::FileExists(check)) {
+    err = "path does not contain an existing CMakeCache.txt file";
+    return false;
+  }
+
+  cmsys::Status status;
+  for (int i = 0; i < 5; ++i) {
+    status = TryToRemoveBinaryDirectoryOnce(sname);
+    if (status) {
+      return true;
+    }
+    cmSystemTools::Delay(100);
+  }
+
+  err = status.GetString();
+  return false;
+}
+
+} // namespace
+
+bool cmCTestEmptyBinaryDirectoryCommand(std::vector<std::string> const& args,
+                                        cmExecutionStatus& status)
 {
   if (args.size() != 1) {
-    this->SetError("called with incorrect number of arguments");
+    status.SetError("called with incorrect number of arguments");
     return false;
   }
 
   std::string err;
-  if (!cmCTestScriptHandler::EmptyBinaryDirectory(args[0], err)) {
+  if (!EmptyBinaryDirectory(args[0], err)) {
     status.GetMakefile().IssueMessage(
       MessageType::FATAL_ERROR,
       cmStrCat("Did not remove the binary directory:\n ", args[0],

Source/CTest/cmCTestEmptyBinaryDirectoryCommand.h

@@ -5,42 +5,9 @@
 #include "cmConfigure.h" // IWYU pragma: keep
 
 #include <string>
-#include <utility>
 #include <vector>
 
-#include <cm/memory>
-
-#include "cmCTestCommand.h"
-#include "cmCommand.h"
-
 class cmExecutionStatus;
 
-/** \class cmCTestEmptyBinaryDirectory
- * \brief Run a ctest script
- *
- * cmLibrarysCommand defines a list of executable (i.e., test)
- * programs to create.
- */
-class cmCTestEmptyBinaryDirectoryCommand : public cmCTestCommand
-{
-public:
-  cmCTestEmptyBinaryDirectoryCommand() {}
-
-  /**
-   * This is a virtual constructor for the command.
-   */
-  std::unique_ptr<cmCommand> Clone() override
-  {
-    auto ni = cm::make_unique<cmCTestEmptyBinaryDirectoryCommand>();
-    ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
-    return std::unique_ptr<cmCommand>(std::move(ni));
-  }
-
-  /**
-   * This is called when the command is first encountered in
-   * the CMakeLists.txt file.
-   */
-  bool InitialPass(std::vector<std::string> const& args,
-                   cmExecutionStatus& status) override;
-};
+bool cmCTestEmptyBinaryDirectoryCommand(std::vector<std::string> const& args,
+                                        cmExecutionStatus& status);

Source/CTest/cmCTestMemCheckCommand.h

@@ -30,7 +30,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestMemCheckCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestRunScriptCommand.h

@@ -33,7 +33,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestRunScriptCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestScriptHandler.cxx

@@ -12,8 +12,6 @@
 
 #include <cm3p/uv.h>
 
-#include "cmsys/Directory.hxx"
-
 #include "cmCTest.h"
 #include "cmCTestBuildCommand.h"
 #include "cmCTestCommand.h"
@@ -36,19 +34,12 @@
 #include "cmState.h"
 #include "cmStateDirectory.h"
 #include "cmStateSnapshot.h"
-#include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 #include "cmUVHandlePtr.h"
 #include "cmUVProcessChain.h"
 #include "cmValue.h"
 #include "cmake.h"
 
-#ifdef _WIN32
-#  include <windows.h>
-#else
-#  include <unistd.h>
-#endif
-
 cmCTestScriptHandler::cmCTestScriptHandler() = default;
 
 void cmCTestScriptHandler::Initialize()
@@ -108,7 +99,6 @@ void cmCTestScriptHandler::AddCTestCommand(
   std::string const& name, std::unique_ptr<cmCTestCommand> command)
 {
   command->CTest = this->CTest;
-  command->CTestScriptHandler = this;
   this->CMake->GetState()->AddBuiltinCommand(name, std::move(command));
 }
 
@@ -229,20 +219,21 @@ void cmCTestScriptHandler::CreateCMake()
       }
     });
 
+  cmState* state = this->CMake->GetState();
   this->AddCTestCommand("ctest_build", cm::make_unique<cmCTestBuildCommand>());
   this->AddCTestCommand("ctest_configure",
                         cm::make_unique<cmCTestConfigureCommand>());
   this->AddCTestCommand("ctest_coverage",
                         cm::make_unique<cmCTestCoverageCommand>());
-  this->AddCTestCommand("ctest_empty_binary_directory",
-                        cm::make_unique<cmCTestEmptyBinaryDirectoryCommand>());
+  state->AddBuiltinCommand("ctest_empty_binary_directory",
+                           cmCTestEmptyBinaryDirectoryCommand);
   this->AddCTestCommand("ctest_memcheck",
                         cm::make_unique<cmCTestMemCheckCommand>());
   this->AddCTestCommand("ctest_read_custom_files",
                         cm::make_unique<cmCTestReadCustomFilesCommand>());
   this->AddCTestCommand("ctest_run_script",
                         cm::make_unique<cmCTestRunScriptCommand>());
-  this->AddCTestCommand("ctest_sleep", cm::make_unique<cmCTestSleepCommand>());
+  state->AddBuiltinCommand("ctest_sleep", cmCTestSleepCommand);
   this->AddCTestCommand("ctest_start", cm::make_unique<cmCTestStartCommand>());
   this->AddCTestCommand("ctest_submit",
                         cm::make_unique<cmCTestSubmitCommand>());
@@ -341,15 +332,6 @@ int cmCTestScriptHandler::ReadInScript(const std::string& total_script_arg)
   return 0;
 }
 
-void cmCTestScriptHandler::SleepInSeconds(unsigned int secondsToWait)
-{
-#if defined(_WIN32)
-  Sleep(1000 * secondsToWait);
-#else
-  sleep(secondsToWait);
-#endif
-}
-
 // run a specific script
 int cmCTestScriptHandler::RunConfigurationScript(
   const std::string& total_script_arg, bool pscope)
@@ -391,73 +373,6 @@ bool cmCTestScriptHandler::RunScript(cmCTest* ctest, cmMakefile* mf,
   return true;
 }
 
-bool cmCTestScriptHandler::EmptyBinaryDirectory(const std::string& sname,
-                                                std::string& err)
-{
-  // try to avoid deleting root
-  if (sname.size() < 2) {
-    err = "path too short";
-    return false;
-  }
-
-  // consider non existing target directory a success
-  if (!cmSystemTools::FileExists(sname)) {
-    return true;
-  }
-
-  // try to avoid deleting directories that we shouldn't
-  std::string check = cmStrCat(sname, "/CMakeCache.txt");
-
-  if (!cmSystemTools::FileExists(check)) {
-    err = "path does not contain an existing CMakeCache.txt file";
-    return false;
-  }
-
-  cmsys::Status status;
-  for (int i = 0; i < 5; ++i) {
-    status = TryToRemoveBinaryDirectoryOnce(sname);
-    if (status) {
-      return true;
-    }
-    cmSystemTools::Delay(100);
-  }
-
-  err = status.GetString();
-  return false;
-}
-
-cmsys::Status cmCTestScriptHandler::TryToRemoveBinaryDirectoryOnce(
-  const std::string& directoryPath)
-{
-  cmsys::Directory directory;
-  directory.Load(directoryPath);
-
-  for (unsigned long i = 0; i < directory.GetNumberOfFiles(); ++i) {
-    std::string path = directory.GetFile(i);
-
-    if (path == "." || path == ".." || path == "CMakeCache.txt") {
-      continue;
-    }
-
-    std::string fullPath = cmStrCat(directoryPath, "/", path);
-
-    bool isDirectory = cmSystemTools::FileIsDirectory(fullPath) &&
-      !cmSystemTools::FileIsSymlink(fullPath);
-
-    cmsys::Status status;
-    if (isDirectory) {
-      status = cmSystemTools::RemoveADirectory(fullPath);
-    } else {
-      status = cmSystemTools::RemoveFile(fullPath);
-    }
-    if (!status) {
-      return status;
-    }
-  }
-
-  return cmSystemTools::RemoveADirectory(directoryPath);
-}
-
 cmDuration cmCTestScriptHandler::GetRemainingTimeAllowed()
 {
   if (!this->Makefile) {

Source/CTest/cmCTestScriptHandler.h

@@ -9,8 +9,6 @@
 #include <string>
 #include <vector>
 
-#include "cmsys/Status.hxx"
-
 #include "cmCTestGenericHandler.h"
 #include "cmDuration.h"
 
@@ -45,15 +43,9 @@ public:
                         const std::string& script, bool InProcess,
                         int* returnValue);
 
-  /*
-   * Empty Binary Directory
-   */
-  static bool EmptyBinaryDirectory(const std::string& dir, std::string& err);
-
   /*
    * Some elapsed time handling functions
    */
-  static void SleepInSeconds(unsigned int secondsToWait);
   void UpdateElapsedTime();
 
   /**
@@ -84,10 +76,6 @@ private:
   void AddCTestCommand(std::string const& name,
                        std::unique_ptr<cmCTestCommand> command);
 
-  // Try to remove the binary directory once
-  static cmsys::Status TryToRemoveBinaryDirectoryOnce(
-    const std::string& directoryPath);
-
   std::vector<std::string> ConfigurationScripts;
   std::vector<bool> ScriptProcessScope;
 

Source/CTest/cmCTestSleepCommand.cxx

@@ -2,42 +2,34 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCTestSleepCommand.h"
 
+#include <chrono>
 #include <cstdlib>
+#include <thread>
 
-#include "cmCTestScriptHandler.h"
+#include "cmExecutionStatus.h"
 
-class cmExecutionStatus;
-
-bool cmCTestSleepCommand::InitialPass(std::vector<std::string> const& args,
-                                      cmExecutionStatus& /*unused*/)
+bool cmCTestSleepCommand(std::vector<std::string> const& args,
+                         cmExecutionStatus& status)
 {
-  if (args.empty()) {
-    this->SetError("called with incorrect number of arguments");
-    return false;
-  }
-
   // sleep for specified seconds
-  unsigned int time1 = atoi(args[0].c_str());
   if (args.size() == 1) {
-    cmCTestScriptHandler::SleepInSeconds(time1);
-    // update the elapsed time since it could have slept for a while
-    this->CTestScriptHandler->UpdateElapsedTime();
+    unsigned int duration = atoi(args[0].c_str());
+    std::this_thread::sleep_for(std::chrono::seconds(duration));
     return true;
   }
 
   // sleep up to a duration
   if (args.size() == 3) {
+    unsigned int time1 = atoi(args[0].c_str());
     unsigned int duration = atoi(args[1].c_str());
     unsigned int time2 = atoi(args[2].c_str());
     if (time1 + duration > time2) {
       duration = (time1 + duration - time2);
-      cmCTestScriptHandler::SleepInSeconds(duration);
-      // update the elapsed time since it could have slept for a while
-      this->CTestScriptHandler->UpdateElapsedTime();
+      std::this_thread::sleep_for(std::chrono::seconds(duration));
     }
     return true;
   }
 
-  this->SetError("called with incorrect number of arguments");
+  status.SetError("called with incorrect number of arguments");
   return false;
 }

Source/CTest/cmCTestSleepCommand.h

@@ -5,42 +5,9 @@
 #include "cmConfigure.h" // IWYU pragma: keep
 
 #include <string>
-#include <utility>
 #include <vector>
 
-#include <cm/memory>
-
-#include "cmCTestCommand.h"
-#include "cmCommand.h"
-
 class cmExecutionStatus;
 
-/** \class cmCTestSleep
- * \brief Run a ctest script
- *
- * cmLibrarysCommand defines a list of executable (i.e., test)
- * programs to create.
- */
-class cmCTestSleepCommand : public cmCTestCommand
-{
-public:
-  cmCTestSleepCommand() {}
-
-  /**
-   * This is a virtual constructor for the command.
-   */
-  std::unique_ptr<cmCommand> Clone() override
-  {
-    auto ni = cm::make_unique<cmCTestSleepCommand>();
-    ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
-    return std::unique_ptr<cmCommand>(std::move(ni));
-  }
-
-  /**
-   * This is called when the command is first encountered in
-   * the CMakeLists.txt file.
-   */
-  bool InitialPass(std::vector<std::string> const& args,
-                   cmExecutionStatus& status) override;
-};
+bool cmCTestSleepCommand(std::vector<std::string> const& args,
+                         cmExecutionStatus& status);

Source/CTest/cmCTestStartCommand.h

@@ -33,7 +33,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestStartCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     ni->CreateNewTag = this->CreateNewTag;
     ni->Quiet = this->Quiet;
     return std::unique_ptr<cmCommand>(std::move(ni));

Source/CTest/cmCTestSubmitCommand.cxx

@@ -29,7 +29,6 @@ std::unique_ptr<cmCommand> cmCTestSubmitCommand::Clone()
 {
   auto ni = cm::make_unique<cmCTestSubmitCommand>();
   ni->CTest = this->CTest;
-  ni->CTestScriptHandler = this->CTestScriptHandler;
   return std::unique_ptr<cmCommand>(std::move(ni));
 }
 

Source/CTest/cmCTestTestCommand.h

@@ -32,7 +32,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestTestCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestUpdateCommand.h

@@ -29,7 +29,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestUpdateCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/CTest/cmCTestUploadCommand.h

@@ -32,7 +32,6 @@ public:
   {
     auto ni = cm::make_unique<cmCTestUploadCommand>();
     ni->CTest = this->CTest;
-    ni->CTestScriptHandler = this->CTestScriptHandler;
     return std::unique_ptr<cmCommand>(std::move(ni));
   }
 

Source/cmMakefile.cxx

@@ -486,10 +486,6 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
     return result;
   }
 
-  if (this->ExecuteCommandCallback) {
-    this->ExecuteCommandCallback();
-  }
-
   // Place this call on the call stack.
   cmMakefileCall stack_manager(this, lff, std::move(deferId), status);
   static_cast<void>(stack_manager);
@@ -545,6 +541,10 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
     }
   }
 
+  if (this->ExecuteCommandCallback) {
+    this->ExecuteCommandCallback();
+  }
+
   return result;
 }