Merge topic 'ctest-remove-binary-dir-error'

07edab8ef2 ctest_empty_binary_directory: Report more detail in failure message

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: scivision <michael@scivision.dev>
Merge-request: !8959

Source/CTest/cmCTestEmptyBinaryDirectoryCommand.cxx

@@ -2,25 +2,27 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmCTestEmptyBinaryDirectoryCommand.h"
 
-#include <sstream>
-
 #include "cmCTestScriptHandler.h"
-
-class cmExecutionStatus;
+#include "cmExecutionStatus.h"
+#include "cmMakefile.h"
+#include "cmMessageType.h"
+#include "cmStringAlgorithms.h"
 
 bool cmCTestEmptyBinaryDirectoryCommand::InitialPass(
-  std::vector<std::string> const& args, cmExecutionStatus& /*unused*/)
+  std::vector<std::string> const& args, cmExecutionStatus& status)
 {
   if (args.size() != 1) {
     this->SetError("called with incorrect number of arguments");
     return false;
   }
 
-  if (!cmCTestScriptHandler::EmptyBinaryDirectory(args[0])) {
-    std::ostringstream ostr;
-    ostr << "problem removing the binary directory: " << args[0];
-    this->SetError(ostr.str());
-    return false;
+  std::string err;
+  if (!cmCTestScriptHandler::EmptyBinaryDirectory(args[0], err)) {
+    status.GetMakefile().IssueMessage(
+      MessageType::FATAL_ERROR,
+      cmStrCat("Did not remove the binary directory:\n ", args[0],
+               "\nbecause:\n ", err));
+    return true;
   }
 
   return true;

Source/CTest/cmCTestScriptHandler.cxx

@@ -672,9 +672,11 @@ int cmCTestScriptHandler::RunConfigurationDashboard()
 
   // clear the binary directory?
   if (this->EmptyBinDir) {
-    if (!cmCTestScriptHandler::EmptyBinaryDirectory(this->BinaryDir)) {
+    std::string err;
+    if (!cmCTestScriptHandler::EmptyBinaryDirectory(this->BinaryDir, err)) {
       cmCTestLog(this->CTest, ERROR_MESSAGE,
-                 "Problem removing the binary directory" << std::endl);
+                 "Problem removing the binary directory ("
+                   << err << "): " << this->BinaryDir << std::endl);
     }
   }
 
@@ -860,10 +862,12 @@ bool cmCTestScriptHandler::RunScript(cmCTest* ctest, cmMakefile* mf,
   return true;
 }
 
-bool cmCTestScriptHandler::EmptyBinaryDirectory(const std::string& sname)
+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;
   }
 
@@ -876,20 +880,24 @@ bool cmCTestScriptHandler::EmptyBinaryDirectory(const std::string& sname)
   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) {
-    if (TryToRemoveBinaryDirectoryOnce(sname)) {
+    status = TryToRemoveBinaryDirectoryOnce(sname);
+    if (status) {
       return true;
     }
     cmSystemTools::Delay(100);
   }
 
+  err = status.GetString();
   return false;
 }
 
-bool cmCTestScriptHandler::TryToRemoveBinaryDirectoryOnce(
+cmsys::Status cmCTestScriptHandler::TryToRemoveBinaryDirectoryOnce(
   const std::string& directoryPath)
 {
   cmsys::Directory directory;
@@ -907,18 +915,18 @@ bool cmCTestScriptHandler::TryToRemoveBinaryDirectoryOnce(
     bool isDirectory = cmSystemTools::FileIsDirectory(fullPath) &&
       !cmSystemTools::FileIsSymlink(fullPath);
 
+    cmsys::Status status;
     if (isDirectory) {
-      if (!cmSystemTools::RemoveADirectory(fullPath)) {
-        return false;
-      }
+      status = cmSystemTools::RemoveADirectory(fullPath);
     } else {
-      if (!cmSystemTools::RemoveFile(fullPath)) {
-        return false;
-      }
+      status = cmSystemTools::RemoveFile(fullPath);
+    }
+    if (!status) {
+      return status;
     }
   }
 
-  return static_cast<bool>(cmSystemTools::RemoveADirectory(directoryPath));
+  return cmSystemTools::RemoveADirectory(directoryPath);
 }
 
 cmDuration cmCTestScriptHandler::GetRemainingTimeAllowed()

Source/CTest/cmCTestScriptHandler.h

@@ -9,6 +9,8 @@
 #include <string>
 #include <vector>
 
+#include "cmsys/Status.hxx"
+
 #include "cmCTestGenericHandler.h"
 #include "cmDuration.h"
 
@@ -80,7 +82,7 @@ public:
   /*
    * Empty Binary Directory
    */
-  static bool EmptyBinaryDirectory(const std::string& dir);
+  static bool EmptyBinaryDirectory(const std::string& dir, std::string& err);
 
   /*
    * Write an initial CMakeCache.txt from the given contents.
@@ -139,7 +141,8 @@ private:
                        std::unique_ptr<cmCTestCommand> command);
 
   // Try to remove the binary directory once
-  static bool TryToRemoveBinaryDirectoryOnce(const std::string& directoryPath);
+  static cmsys::Status TryToRemoveBinaryDirectoryOnce(
+    const std::string& directoryPath);
 
   std::vector<std::string> ConfigurationScripts;
   std::vector<bool> ScriptProcessScope;