try_compile: Add keyword-dispatched signature

Introduce a new signature for try_compile (and try_run) which removes
the `bindir` argument and requires the SOURCES tag. This will eventually
allow us to add other ways of providing sources, but also allows us to
change the behavior without breaking compatibility.

The old signature uses a special, but non-unique temporary location
inside the specified `bindir`, which conventionally is just the
project's build directory. The new signature unconditionally uses the a
unique temporary directory which is unconditionally within the project's
build directory (which is no longer separately specified). This ensures
that successive runs do not overwrite previous runs, will simplify
debugging, and should also, eventually, allow us to execute multiple
trials in parallel.

3 files changed, 103 insertions, 23 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/cmake.cxx b/Source/cmake.cxx
index 9b2b119..c792d4a 100644
--- a/Source/cmake.cxx
+++ b/Source/cmake.cxx
@@ -2130,6 +2130,9 @@ int cmake::ActualConfigure()
   this->UpdateConversionPathTable();
   this->CleanupCommandsAndMacros();
 
+  cmSystemTools::RemoveADirectory(this->GetHomeOutputDirectory() +
+                                  "/CMakeFiles/CMakeScratch");
+
   int res = this->DoPreConfigureChecks();
   if (res < 0) {
     return -2;