Merge topic 'try_compile-unique-bindir'

aa9220d3a0 try_compile: Add keyword-dispatched signature
d1befe5515 cmSystemTools: Add MakeTempDirectory

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Acked-by: Matthew Woehlke <matthew.woehlke@kitware.com>
Merge-request: !7579

5 files changed, 220 insertions, 24 deletions

diff --git a/Source/cmCoreTryCompile.cxx b/Source/cmCoreTryCompile.cxx
index 7b9dc2e..0a6c359 100644
--- a/Source/cmCoreTryCompile.cxx
+++ b/Source/cmCoreTryCompile.cxx
@@ -31,6 +31,7 @@
 #include "cmake.h"
 
 namespace {
+constexpr const char* unique_binary_directory = "CMAKE_BINARY_DIR_USE_MKDTEMP";
 constexpr size_t lang_property_start = 0;
 constexpr size_t lang_property_size = 4;
 constexpr size_t pie_property_start = 4;
@@ -122,13 +123,9 @@ ArgumentParser::Continue TryCompileCompileDefs(Arguments& args,
     .Bind(#lang "_STANDARD_REQUIRED"_s, TryCompileLangProp)                   \
     .Bind(#lang "_EXTENSIONS"_s, TryCompileLangProp)
 
-auto const TryCompileArgParser =
+auto const TryCompileBaseArgParser =
   cmArgumentParser<Arguments>{}
     .Bind(0, &Arguments::CompileResultVariable)
-    .Bind(1, &Arguments::BinaryDirectory)
-    .Bind(2, &Arguments::SourceDirectoryOrFile)
-    .Bind(3, &Arguments::ProjectName)
-    .Bind(4, &Arguments::TargetName)
     .Bind("SOURCES"_s, &Arguments::Sources)
     .Bind("CMAKE_FLAGS"_s, &Arguments::CMakeFlags)
     .Bind("COMPILE_DEFINITIONS"_s, TryCompileCompileDefs,
@@ -136,7 +133,6 @@ auto const TryCompileArgParser =
     .Bind("LINK_LIBRARIES"_s, &Arguments::LinkLibraries)
     .Bind("LINK_OPTIONS"_s, &Arguments::LinkOptions)
     .Bind("__CMAKE_INTERNAL"_s, &Arguments::CMakeInternal)
-    .Bind("OUTPUT_VARIABLE"_s, &Arguments::OutputVariable)
     .Bind("COPY_FILE"_s, &Arguments::CopyFileTo)
     .Bind("COPY_FILE_ERROR"_s, &Arguments::CopyFileError)
     .BIND_LANG_PROPS(C)
@@ -147,8 +143,31 @@ auto const TryCompileArgParser =
     .BIND_LANG_PROPS(OBJCXX)
   /* keep semicolon on own line */;
 
-auto const TryRunArgParser =
+auto const TryCompileArgParser =
+  cmArgumentParser<Arguments>{ TryCompileBaseArgParser }.Bind(
+    "OUTPUT_VARIABLE"_s, &Arguments::OutputVariable)
+  /* keep semicolon on own line */;
+
+auto const TryCompileOldArgParser =
   cmArgumentParser<Arguments>{ TryCompileArgParser }
+    .Bind(1, &Arguments::BinaryDirectory)
+    .Bind(2, &Arguments::SourceDirectoryOrFile)
+    .Bind(3, &Arguments::ProjectName)
+    .Bind(4, &Arguments::TargetName)
+  /* keep semicolon on own line */;
+
+auto const TryRunArgParser =
+  cmArgumentParser<Arguments>{ TryCompileBaseArgParser }
+    .Bind("COMPILE_OUTPUT_VARIABLE"_s, &Arguments::CompileOutputVariable)
+    .Bind("RUN_OUTPUT_VARIABLE"_s, &Arguments::RunOutputVariable)
+    .Bind("RUN_OUTPUT_STDOUT_VARIABLE"_s, &Arguments::RunOutputStdOutVariable)
+    .Bind("RUN_OUTPUT_STDERR_VARIABLE"_s, &Arguments::RunOutputStdErrVariable)
+    .Bind("WORKING_DIRECTORY"_s, &Arguments::RunWorkingDirectory)
+    .Bind("ARGS"_s, &Arguments::RunArgs)
+  /* keep semicolon on own line */;
+
+auto const TryRunOldArgParser =
+  cmArgumentParser<Arguments>{ TryCompileOldArgParser }
     .Bind("COMPILE_OUTPUT_VARIABLE"_s, &Arguments::CompileOutputVariable)
     .Bind("RUN_OUTPUT_VARIABLE"_s, &Arguments::RunOutputVariable)
     .Bind("RUN_OUTPUT_STDOUT_VARIABLE"_s, &Arguments::RunOutputStdOutVariable)
@@ -161,10 +180,10 @@ auto const TryRunArgParser =
 }
 
 Arguments cmCoreTryCompile::ParseArgs(
-  cmRange<std::vector<std::string>::const_iterator> args, bool isTryRun)
+  const cmRange<std::vector<std::string>::const_iterator>& args,
+  const cmArgumentParser<Arguments>& parser,
+  std::vector<std::string>& unparsedArguments)
 {
-  std::vector<std::string> unparsedArguments;
-  const auto& parser = (isTryRun ? TryRunArgParser : TryCompileArgParser);
   auto arguments = parser.Parse(args, &unparsedArguments, 0);
   if (!arguments.MaybeReportError(*(this->Makefile)) &&
       !unparsedArguments.empty()) {
@@ -174,6 +193,26 @@ Arguments cmCoreTryCompile::ParseArgs(
     }
     this->Makefile->IssueMessage(MessageType::AUTHOR_WARNING, m);
   }
+  return arguments;
+}
+
+Arguments cmCoreTryCompile::ParseArgs(
+  cmRange<std::vector<std::string>::const_iterator> args, bool isTryRun)
+{
+  std::vector<std::string> unparsedArguments;
+  if (cmHasLiteralPrefix(*(++args.begin()), "SOURCE")) {
+    // New signature.
+    auto arguments =
+      this->ParseArgs(args, isTryRun ? TryRunArgParser : TryCompileArgParser,
+                      unparsedArguments);
+    arguments.BinaryDirectory = unique_binary_directory;
+    return arguments;
+  }
+
+  // Old signature.
+  auto arguments = this->ParseArgs(
+    args, isTryRun ? TryRunOldArgParser : TryCompileOldArgParser,
+    unparsedArguments);
   // For historical reasons, treat some empty-valued keyword
   // arguments as if they were not specified at all.
   if (arguments.OutputVariable && arguments.OutputVariable->empty()) {
@@ -210,6 +249,7 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
   // which signature were we called with ?
   this->SrcFileSignature = true;
 
+  bool useUniqueBinaryDirectory = false;
   std::string sourceDirectory;
   std::string projectName;
   std::string targetName;
@@ -230,7 +270,17 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
     targetName = targetNameBuf;
   }
 
-  if (arguments.BinaryDirectory && !arguments.BinaryDirectory->empty()) {
+  if (!arguments.BinaryDirectory || arguments.BinaryDirectory->empty()) {
+    this->Makefile->IssueMessage(MessageType::FATAL_ERROR,
+                                 "No <bindir> specified.");
+    return false;
+  }
+  if (*arguments.BinaryDirectory == unique_binary_directory) {
+    // leave empty until we're ready to create it, so we don't try to remove
+    // a non-existing directory if we abort due to e.g. bad arguments
+    this->BinaryDirectory.clear();
+    useUniqueBinaryDirectory = true;
+  } else {
     if (!cmSystemTools::FileIsFullPath(*arguments.BinaryDirectory)) {
       this->Makefile->IssueMessage(
         MessageType::FATAL_ERROR,
@@ -244,10 +294,6 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
     if (this->SrcFileSignature) {
       this->BinaryDirectory += "/CMakeFiles/CMakeTmp";
     }
-  } else {
-    this->Makefile->IssueMessage(MessageType::FATAL_ERROR,
-                                 "No <bindir> specified.");
-    return false;
   }
 
   std::vector<std::string> targets;
@@ -331,7 +377,14 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
     }
   }
   // make sure the binary directory exists
-  cmSystemTools::MakeDirectory(this->BinaryDirectory);
+  if (useUniqueBinaryDirectory) {
+    this->BinaryDirectory =
+      cmStrCat(this->Makefile->GetHomeOutputDirectory(),
+               "/CMakeFiles/CMakeScratch/TryCompile-XXXXXX");
+    cmSystemTools::MakeTempDirectory(this->BinaryDirectory);
+  } else {
+    cmSystemTools::MakeDirectory(this->BinaryDirectory);
+  }
 
   // do not allow recursive try Compiles
   if (this->BinaryDirectory == this->Makefile->GetHomeOutputDirectory()) {
@@ -635,7 +688,7 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
       fprintf(fout, " \"%s\"", si.c_str());
 
       // Add dependencies on any non-temporary sources.
-      if (si.find("CMakeTmp") == std::string::npos) {
+      if (!IsTemporary(si)) {
         this->Makefile->AddCMakeDependFile(si);
       }
     }
@@ -914,17 +967,23 @@ bool cmCoreTryCompile::TryCompileCode(Arguments& arguments,
   return res == 0;
 }
 
+bool cmCoreTryCompile::IsTemporary(std::string const& path)
+{
+  return ((path.find("CMakeTmp") != std::string::npos) ||
+          (path.find("CMakeScratch") != std::string::npos));
+}
+
 void cmCoreTryCompile::CleanupFiles(std::string const& binDir)
 {
   if (binDir.empty()) {
     return;
   }
 
-  if (binDir.find("CMakeTmp") == std::string::npos) {
+  if (!IsTemporary(binDir)) {
     cmSystemTools::Error(
       "TRY_COMPILE attempt to remove -rf directory that does not contain "
-      "CMakeTmp:" +
-      binDir);
+      "CMakeTmp or CMakeScratch: \"" +
+      binDir + "\"");
     return;
   }
 
@@ -970,6 +1029,10 @@ void cmCoreTryCompile::CleanupFiles(std::string const& binDir)
       }
     }
   }
+
+  if (binDir.find("CMakeScratch") != std::string::npos) {
+    cmSystemTools::RemoveADirectory(binDir);
+  }
 }
 
 void cmCoreTryCompile::FindOutputFile(const std::string& targetName,
diff --git a/Source/cmCoreTryCompile.h b/Source/cmCoreTryCompile.h
index deefe57..b483f0f 100644
--- a/Source/cmCoreTryCompile.h
+++ b/Source/cmCoreTryCompile.h
@@ -67,14 +67,22 @@ public:
                       bool isTryRun);
 
   /**
-   * This is the core code for try compile. It is here so that other
-   * commands, such as TryRun can access the same logic without
-   * duplication.
+   * This is the core code for try compile. It is here so that other commands,
+   * such as TryRun can access the same logic without duplication.
+   *
+   * This function requires at least two \p arguments and will crash if given
+   * fewer.
    */
   bool TryCompileCode(Arguments& arguments,
                       cmStateEnums::TargetType targetType);
 
   /**
+   * Returns \c true if \p path resides within a CMake temporary directory,
+   * otherwise returns \c false.
+   */
+  static bool IsTemporary(std::string const& path);
+
+  /**
    * This deletes all the files created by TryCompileCode.
    * This way we do not have to rely on the timing and
    * dependencies of makefiles.
@@ -94,4 +102,10 @@ public:
   std::string FindErrorMessage;
   bool SrcFileSignature = false;
   cmMakefile* Makefile;
+
+private:
+  Arguments ParseArgs(
+    const cmRange<std::vector<std::string>::const_iterator>& args,
+    const cmArgumentParser<Arguments>& parser,
+    std::vector<std::string>& unparsedArguments);
 };
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index 3c4e709..8e77afa 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -11,6 +11,11 @@
 // NOLINTNEXTLINE(bugprone-reserved-identifier)
 #  define _XOPEN_SOURCE 700
 #endif
+#if defined(__APPLE__)
+// Restore Darwin APIs removed by _POSIX_C_SOURCE.
+// NOLINTNEXTLINE(bugprone-reserved-identifier)
+#  define _DARWIN_C_SOURCE
+#endif
 
 #include "cmSystemTools.h"
 
@@ -88,7 +93,6 @@
 #  include <unistd.h>
 
 #  include <sys/time.h>
-#  include <sys/types.h>
 #endif
 
 #if defined(_WIN32) &&                                                        \
@@ -1001,6 +1005,93 @@ void cmSystemTools::InitializeLibUV()
 #endif
 }
 
+#if defined(_WIN32)
+#  include <random>
+
+#  include <wctype.h>
+#  ifdef _MSC_VER
+using mode_t = cmSystemTools::SystemTools::mode_t;
+#  endif
+#else
+#  include <sys/stat.h>
+#endif
+
+inline int Mkdir(const char* dir, const mode_t* mode)
+{
+#if defined(_WIN32)
+  int ret = _wmkdir(cmSystemTools::ConvertToWindowsExtendedPath(dir).c_str());
+  if (ret == 0 && mode)
+    cmSystemTools::SystemTools::SetPermissions(dir, *mode);
+  return ret;
+#else
+  return mkdir(dir, mode ? *mode : 0777);
+#endif
+}
+
+cmsys::Status cmSystemTools::MakeTempDirectory(std::string& path,
+                                               const mode_t* mode)
+{
+  if (path.empty()) {
+    return cmsys::Status::POSIX(EINVAL);
+  }
+  return cmSystemTools::MakeTempDirectory(&path.front(), mode);
+}
+
+cmsys::Status cmSystemTools::MakeTempDirectory(char* path, const mode_t* mode)
+{
+  if (!path) {
+    return cmsys::Status::POSIX(EINVAL);
+  }
+
+  // verify that path ends with "XXXXXX"
+  const auto l = std::strlen(path);
+  if (!cmHasLiteralSuffix(cm::string_view{ path, l }, "XXXXXX")) {
+    return cmsys::Status::POSIX(EINVAL);
+  }
+
+  // create parent directories
+  auto* sep = path;
+  while ((sep = strchr(sep, '/'))) {
+    // all underlying functions use C strings,
+    // so temporarily end the string here
+    *sep = '\0';
+    Mkdir(path, mode);
+
+    *sep = '/';
+    ++sep;
+  }
+
+#ifdef _WIN32
+  const int nchars = 36;
+  const char chars[nchars + 1] = "abcdefghijklmnopqrstuvwxyz0123456789";
+
+  std::random_device rd;
+  std::mt19937 rg{ rd() };
+  std::uniform_int_distribution<int> dist{ 0, nchars - 1 };
+
+  for (auto tries = 100; tries; --tries) {
+    for (auto n = l - 6; n < l; ++n) {
+      path[n] = chars[dist(rg)];
+    }
+    if (Mkdir(path, mode) == 0) {
+      return cmsys::Status::Success();
+    } else if (errno != EEXIST) {
+      return cmsys::Status::POSIX_errno();
+    }
+  }
+  return cmsys::Status::POSIX(EAGAIN);
+#else
+  if (mkdtemp(path)) {
+    if (mode) {
+      chmod(path, *mode);
+    }
+  } else {
+    return cmsys::Status::POSIX_errno();
+  }
+  return cmsys::Status::Success();
+#endif
+}
+
 #ifdef _WIN32
 namespace {
 bool cmMoveFile(std::wstring const& oldname, std::wstring const& newname,
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 48bbe23..b02a977 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -4,6 +4,10 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#if !defined(_WIN32)
+#  include <sys/types.h>
+#endif
+
 #include <cstddef>
 #include <functional>
 #include <map>
@@ -151,6 +155,27 @@ public:
     Failure,
   };
 
+#if defined(_MSC_VER)
+  /** Visual C++ does not define mode_t. */
+  using mode_t = unsigned short;
+#endif
+
+  /**
+   * Make a new temporary directory.  The path must end in "XXXXXX", and will
+   * be modified to reflect the name of the directory created.  This function
+   * is similar to POSIX mkdtemp (and is implemented using the same where that
+   * function is available).
+   *
+   * This function can make a full path even if none of the directories existed
+   * prior to calling this function.
+   *
+   * Note that this function may modify \p path even if it does not succeed.
+   */
+  static cmsys::Status MakeTempDirectory(char* path,
+                                         const mode_t* mode = nullptr);
+  static cmsys::Status MakeTempDirectory(std::string& path,
+                                         const mode_t* mode = nullptr);
+
   /** Copy a file. */
   static bool CopySingleFile(const std::string& oldname,
                              const std::string& newname);
diff --git a/Source/cmake.cxx b/Source/cmake.cxx
index b905caa..281e63f 100644
--- a/Source/cmake.cxx
+++ b/Source/cmake.cxx
@@ -2139,6 +2139,9 @@ int cmake::ActualConfigure()
   this->UpdateConversionPathTable();
   this->CleanupCommandsAndMacros();
 
+  cmSystemTools::RemoveADirectory(this->GetHomeOutputDirectory() +
+                                  "/CMakeFiles/CMakeScratch");
+
   int res = this->DoPreConfigureChecks();
   if (res < 0) {
     return -2;