Merge topic 'cpp-modules-prep'

c5b56b35c2 cmInstallExportGenerator: expose the temporary directory
c107760417 cmNinjaTargetGenerator: support msvc-style deps discovery for scanning
64c15ec018 cmNinjaTargetGenerator: add flags for scanning based on the fileset type
aaa18f15cf cmTarget: add support for querying all file set names
cc4e19710d cmGlobalNinjaGenerator: use an extention based on the modmapfmt
97a68198c9 cmGlobalNinjaGenerator: escape `:` in module names
499009b79c cmTarget: avoid creating export entries if they don't exist
0513a1fe10 cmInstallGenerator: use CMake-private variables in generated code
...

Acked-by: Kitware Robot <[email protected]>
Tested-by: buildbot <[email protected]>
Merge-request: !7214

Help/dev/experimental.rst

@@ -36,7 +36,14 @@ For example, add code like the following to a test project:
 The tool specified by ``CMAKE_EXPERIMENTAL_CXX_SCANDEP_SOURCE`` is
 expected to process the translation unit, write preprocessor dependencies
 to the file specified by the ``<DEP_FILE>`` placeholder, and write module
-dependencies to the file specified by the ``<DYNDEP_FILE>`` placeholder.
+dependencies to the file specified by the ``<DYNDEP_FILE>`` placeholder. The
+``CMAKE_EXPERIMENTAL_CXX_SCANDEP_DEPFILE_FORMAT`` file may be set to ``msvc``
+for scandep rules which use ``msvc``-style dependency reporting.
+
+For tools which need to know the file set the source belongs to, the
+``CMAKE_EXPERIMENTAL_CXX_MODULE_SOURCE_TYPE_FLAG_<FILE_SET_TYPE>`` flag may
+be provided so that different source types can be distinguished prior to
+scanning.
 
 The module dependencies should be written in the format described
 by the `P1689r4`_ paper.

Source/cmFileSet.cxx

@@ -45,9 +45,12 @@ cmFileSetVisibility cmFileSetVisibilityFromName(cm::string_view name,
   if (name == "PRIVATE"_s) {
     return cmFileSetVisibility::Private;
   }
-  mf->IssueMessage(
-    MessageType::FATAL_ERROR,
-    cmStrCat("File set visibility \"", name, "\" is not valid."));
+  auto msg = cmStrCat("File set visibility \"", name, "\" is not valid.");
+  if (mf) {
+    mf->IssueMessage(MessageType::FATAL_ERROR, msg);
+  } else {
+    cmSystemTools::Error(msg);
+  }
   return cmFileSetVisibility::Private;
 }
 

Source/cmGlobalNinjaGenerator.cxx

@@ -2534,6 +2534,11 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
     }
   }
 
+  const char* module_ext = "";
+  if (arg_modmapfmt == "gcc") {
+    module_ext = ".gcm";
+  }
+
   // Extend the module map with those provided by this target.
   // We do this after loading the modules provided by linked targets
   // in case we have one of the same name that must be preferred.
@@ -2550,7 +2555,9 @@ bool cmGlobalNinjaGenerator::WriteDyndepFile(
         }
       } else {
         // Assume the module file path matches the logical module name.
-        mod = cmStrCat(module_dir, p.LogicalName);
+        std::string safe_logical_name = p.LogicalName;
+        cmSystemTools::ReplaceString(safe_logical_name, ":", "-");
+        mod = cmStrCat(module_dir, safe_logical_name, module_ext);
       }
       mod_files[p.LogicalName] = mod;
       tm[p.LogicalName] = mod;

Source/cmInstallExportGenerator.cxx

@@ -2,7 +2,6 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmInstallExportGenerator.h"
 
-#include <algorithm>
 #include <map>
 #include <sstream>
 #include <utility>
@@ -54,73 +53,36 @@ bool cmInstallExportGenerator::Compute(cmLocalGenerator* lg)
   return true;
 }
 
-void cmInstallExportGenerator::ComputeTempDir()
+std::string cmInstallExportGenerator::TempDirCalculate() const
 {
   // Choose a temporary directory in which to generate the import
   // files to be installed.
-  this->TempDir = cmStrCat(this->LocalGenerator->GetCurrentBinaryDirectory(),
-                           "/CMakeFiles/Export");
+  std::string path = cmStrCat(
+    this->LocalGenerator->GetCurrentBinaryDirectory(), "/CMakeFiles/Export");
   if (this->Destination.empty()) {
-    return;
-  }
-  this->TempDir += "/";
-
-  // Enforce a maximum length.
-  bool useMD5 = false;
-#if defined(_WIN32) || defined(__CYGWIN__)
-  std::string::size_type const max_total_len = 250;
-#else
-  std::string::size_type const max_total_len = 1000;
-#endif
-  // Will generate files of the form "<temp-dir>/<base>-<config>.<ext>".
-  std::string::size_type const len = this->TempDir.size() + 1 +
-    this->FileName.size() + 1 + this->GetMaxConfigLength();
-  if (len < max_total_len) {
-    // Keep the total path length below the limit.
-    std::string::size_type const max_len = max_total_len - len;
-    if (this->Destination.size() > max_len) {
-      useMD5 = true;
-    }
-  } else {
-    useMD5 = true;
+    return path;
   }
-  if (useMD5) {
-    // Replace the destination path with a hash to keep it short.
+
 #ifndef CMAKE_BOOTSTRAP
-    this->TempDir += cmSystemTools::ComputeStringMD5(this->Destination);
+  path += '/';
+  // Replace the destination path with a hash to keep it short.
+  path += cmSystemTools::ComputeStringMD5(this->Destination);
 #endif
-  } else {
-    std::string dest = this->Destination;
-    // Avoid unix full paths.
-    if (dest[0] == '/') {
-      dest[0] = '_';
-    }
-    // Avoid windows full paths by removing colons.
-    std::replace(dest.begin(), dest.end(), ':', '_');
-    // Avoid relative paths that go up the tree.
-    cmSystemTools::ReplaceString(dest, "../", "__/");
-    // Avoid spaces.
-    std::replace(dest.begin(), dest.end(), ' ', '_');
-    this->TempDir += dest;
-  }
+
+  return path;
 }
 
-size_t cmInstallExportGenerator::GetMaxConfigLength() const
+void cmInstallExportGenerator::ComputeTempDir()
 {
-  // Always use at least 8 for "noconfig".
-  size_t len = 8;
-  if (this->ConfigurationTypes->empty()) {
-    if (this->ConfigurationName.size() > 8) {
-      len = this->ConfigurationName.size();
-    }
-  } else {
-    for (std::string const& c : *this->ConfigurationTypes) {
-      if (c.size() > len) {
-        len = c.size();
-      }
-    }
+  this->TempDir = this->TempDirCalculate();
+}
+
+std::string cmInstallExportGenerator::GetTempDir() const
+{
+  if (this->TempDir.empty()) {
+    return this->TempDirCalculate();
   }
-  return len;
+  return this->TempDir;
 }
 
 void cmInstallExportGenerator::GenerateScript(std::ostream& os)

Source/cmInstallExportGenerator.h

@@ -4,7 +4,6 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
-#include <cstddef>
 #include <iosfwd>
 #include <memory>
 #include <string>
@@ -50,6 +49,7 @@ public:
   std::string const& GetDestination() const { return this->Destination; }
   std::string GetDestinationFile() const;
   std::string GetFileName() const { return this->FileName; }
+  std::string GetTempDir() const;
 
 protected:
   void GenerateScript(std::ostream& os) override;
@@ -57,8 +57,8 @@ protected:
   void GenerateScriptActions(std::ostream& os, Indent indent) override;
   void GenerateImportFile(cmExportSet const* exportSet);
   void GenerateImportFile(const char* config, cmExportSet const* exportSet);
+  std::string TempDirCalculate() const;
   void ComputeTempDir();
-  size_t GetMaxConfigLength() const;
 
   cmExportSet* const ExportSet;
   std::string const FilePermissions;

Source/cmInstallGenerator.cxx

@@ -41,10 +41,10 @@ void cmInstallGenerator::CheckCMP0082(bool& haveSubdirectoryInstall,
 void cmInstallGenerator::AddInstallRule(
   std::ostream& os, std::string const& dest, cmInstallType type,
   std::vector<std::string> const& files, bool optional /* = false */,
-  const char* permissions_file /* = 0 */,
-  const char* permissions_dir /* = 0 */, const char* rename /* = 0 */,
-  const char* literal_args /* = 0 */, Indent indent,
-  const char* files_var /* = 0 */)
+  const char* permissions_file /* = nullptr */,
+  const char* permissions_dir /* = nullptr */,
+  const char* rename /* = nullptr */, const char* literal_args /* = nullptr */,
+  Indent indent, const char* files_var /* = nullptr */)
 {
   // Use the FILE command to install the file.
   std::string stype;
@@ -91,21 +91,28 @@ void cmInstallGenerator::AddInstallRule(
       os << "\")\n";
     }
     if (files_var) {
-      os << indent << "foreach(_f IN LISTS " << files_var << ")\n";
-      os << indent.Next() << "get_filename_component(_fn \"${_f}\" NAME)\n";
+      os << indent << "foreach(_cmake_abs_file IN LISTS " << files_var
+         << ")\n";
+      os << indent.Next()
+         << "get_filename_component(_cmake_abs_file_name "
+            "\"${_cmake_abs_file}\" NAME)\n";
       os << indent.Next() << "list(APPEND CMAKE_ABSOLUTE_DESTINATION_FILES \""
-         << dest << "/${_fn}\")\n";
+         << dest << "/${_cmake_abs_file_name}\")\n";
       os << indent << "endforeach()\n";
+      os << indent << "unset(_cmake_abs_file_name)\n";
+      os << indent << "unset(_cmake_abs_file)\n";
     }
     os << indent << "if(CMAKE_WARN_ON_ABSOLUTE_INSTALL_DESTINATION)\n";
-    os << indent.Next() << "message(WARNING \"ABSOLUTE path INSTALL "
-       << "DESTINATION : ${CMAKE_ABSOLUTE_DESTINATION_FILES}\")\n";
+    os << indent.Next()
+       << "message(WARNING \"ABSOLUTE path INSTALL "
+          "DESTINATION : ${CMAKE_ABSOLUTE_DESTINATION_FILES}\")\n";
     os << indent << "endif()\n";
 
     os << indent << "if(CMAKE_ERROR_ON_ABSOLUTE_INSTALL_DESTINATION)\n";
-    os << indent.Next() << "message(FATAL_ERROR \"ABSOLUTE path INSTALL "
-       << "DESTINATION forbidden (by caller): "
-       << "${CMAKE_ABSOLUTE_DESTINATION_FILES}\")\n";
+    os << indent.Next()
+       << "message(FATAL_ERROR \"ABSOLUTE path INSTALL "
+          "DESTINATION forbidden (by caller): "
+          "${CMAKE_ABSOLUTE_DESTINATION_FILES}\")\n";
     os << indent << "endif()\n";
   }
   std::string absDest = ConvertToAbsoluteDestination(dest);

Source/cmNinjaTargetGenerator.cxx

@@ -21,6 +21,7 @@
 
 #include "cmComputeLinkInformation.h"
 #include "cmCustomCommandGenerator.h"
+#include "cmFileSet.h"
 #include "cmGeneratedFileStream.h"
 #include "cmGeneratorExpression.h"
 #include "cmGeneratorTarget.h"
@@ -28,6 +29,7 @@
 #include "cmLocalGenerator.h"
 #include "cmLocalNinjaGenerator.h"
 #include "cmMakefile.h"
+#include "cmMessageType.h"
 #include "cmNinjaNormalTargetGenerator.h"
 #include "cmNinjaUtilityTargetGenerator.h"
 #include "cmOutputConverter.h"
@@ -39,6 +41,7 @@
 #include "cmStateTypes.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
+#include "cmTarget.h"
 #include "cmValue.h"
 #include "cmake.h"
 
@@ -252,6 +255,55 @@ std::string cmNinjaTargetGenerator::ComputeFlagsForObject(
       flags, genexInterpreter.Evaluate(pchOptions, COMPILE_OPTIONS));
   }
 
+  if (this->NeedCxxModuleSupport(language, config)) {
+    auto const& path = source->GetFullPath();
+    auto const* tgt = this->GeneratorTarget->Target;
+
+    std::string file_set_type;
+
+    for (auto const& name : tgt->GetAllFileSetNames()) {
+      auto const* file_set = tgt->GetFileSet(name);
+      if (!file_set) {
+        this->GetMakefile()->IssueMessage(
+          MessageType::INTERNAL_ERROR,
+          cmStrCat("Target `", tgt->GetName(),
+                   "` is tracked to have file set `", name,
+                   "`, but it was not found."));
+        continue;
+      }
+
+      auto fileEntries = file_set->CompileFileEntries();
+      auto directoryEntries = file_set->CompileDirectoryEntries();
+      auto directories = file_set->EvaluateDirectoryEntries(
+        directoryEntries, this->LocalGenerator, config, this->GeneratorTarget);
+
+      std::map<std::string, std::vector<std::string>> files;
+      for (auto const& entry : fileEntries) {
+        file_set->EvaluateFileEntry(directories, files, entry,
+                                    this->LocalGenerator, config,
+                                    this->GeneratorTarget);
+      }
+
+      for (auto const& it : files) {
+        for (auto const& filename : it.second) {
+          if (filename == path) {
+            file_set_type = file_set->GetType();
+            break;
+          }
+        }
+      }
+
+      if (!file_set_type.empty()) {
+        std::string source_type_var = cmStrCat(
+          "CMAKE_EXPERIMENTAL_CXX_MODULE_SOURCE_TYPE_FLAG_", file_set_type);
+        cmMakefile* mf = this->GetMakefile();
+        if (cmValue source_type_flag = mf->GetDefinition(source_type_var)) {
+          this->LocalGenerator->AppendFlags(flags, *source_type_flag);
+        }
+      }
+    }
+  }
+
   return flags;
 }
 
@@ -534,6 +586,7 @@ std::string GetScanCommand(const std::string& cmakeCmd, const std::string& tdi,
 // not perform explicit preprocessing too.
 cmNinjaRule GetScanRule(
   std::string const& ruleName, std::string const& ppFileName,
+  std::string const& deptype,
   cmRulePlaceholderExpander::RuleVariables const& vars,
   const std::string& responseFlag, const std::string& flags,
   cmRulePlaceholderExpander* const rulePlaceholderExpander,
@@ -542,8 +595,13 @@ cmNinjaRule GetScanRule(
 {
   cmNinjaRule rule(ruleName);
   // Scanning always uses a depfile for preprocessor dependencies.
-  rule.DepType = ""; // no deps= for multiple outputs
-  rule.DepFile = "$DEP_FILE";
+  if (deptype == "msvc"_s) {
+    rule.DepType = deptype;
+    rule.DepFile = "";
+  } else {
+    rule.DepType = ""; // no deps= for multiple outputs
+    rule.DepFile = "$DEP_FILE";
+  }
 
   cmRulePlaceholderExpander::RuleVariables scanVars;
   scanVars.CMTargetName = vars.CMTargetName;
@@ -647,6 +705,9 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang,
       cmSystemTools::GetCMakeCommand(), cmLocalGenerator::SHELL);
 
   if (needDyndep) {
+    const auto& scanDepType = this->GetMakefile()->GetSafeDefinition(
+      cmStrCat("CMAKE_EXPERIMENTAL_", lang, "_SCANDEP_DEPFILE_FORMAT"));
+
     // Rule to scan dependencies of sources that need preprocessing.
     {
       std::vector<std::string> scanCommands;
@@ -674,10 +735,10 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang,
                                                  "$DYNDEP_INTERMEDIATE_FILE"));
       }
 
-      auto scanRule =
-        GetScanRule(scanRuleName, ppFileName, vars, responseFlag, flags,
-                    rulePlaceholderExpander.get(), this->GetLocalGenerator(),
-                    std::move(scanCommands), config);
+      auto scanRule = GetScanRule(
+        scanRuleName, ppFileName, scanDepType, vars, responseFlag, flags,
+        rulePlaceholderExpander.get(), this->GetLocalGenerator(),
+        std::move(scanCommands), config);
 
       scanRule.Comment =
         cmStrCat("Rule for generating ", lang, " dependencies.");
@@ -705,9 +766,10 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang,
       scanCommands.emplace_back(
         GetScanCommand(cmakeCmd, tdi, lang, "$in", "$out"));
 
-      auto scanRule = GetScanRule(
-        scanRuleName, "", vars, "", flags, rulePlaceholderExpander.get(),
-        this->GetLocalGenerator(), std::move(scanCommands), config);
+      auto scanRule =
+        GetScanRule(scanRuleName, "", scanDepType, vars, "", flags,
+                    rulePlaceholderExpander.get(), this->GetLocalGenerator(),
+                    std::move(scanCommands), config);
 
       // Write the rule for generating dependencies for the given language.
       scanRule.Comment = cmStrCat("Rule for generating ", lang,
@@ -1197,7 +1259,8 @@ cmNinjaBuild GetScanBuildStatement(const std::string& ruleName,
     scanBuild.Variables["PREPROCESSED_OUTPUT_FILE"] = ppFileName;
   }
 
-  // Scanning always uses a depfile for preprocessor dependencies.
+  // Scanning always provides a depfile for preprocessor dependencies. This
+  // variable is unused in `msvc`-deptype scanners.
   std::string const& depFileName = cmStrCat(scanBuild.Outputs.front(), ".d");
   scanBuild.Variables["DEP_FILE"] =
     lg->ConvertToOutputFormat(depFileName, cmOutputConverter::SHELL);

Source/cmTarget.cxx

@@ -1291,7 +1291,12 @@ void cmTarget::AddInstallIncludeDirectories(cmTargetExport const& te,
 cmStringRange cmTarget::GetInstallIncludeDirectoriesEntries(
   cmTargetExport const& te) const
 {
-  return cmMakeRange(this->impl->InstallIncludeDirectoriesEntries[&te]);
+  auto i = this->impl->InstallIncludeDirectoriesEntries.find(&te);
+  if (i == this->impl->InstallIncludeDirectoriesEntries.end()) {
+    decltype(i->second) empty;
+    return cmMakeRange(empty);
+  }
+  return cmMakeRange(i->second);
 }
 
 cmBTStringRange cmTarget::GetIncludeDirectoriesEntries() const
@@ -2540,6 +2545,17 @@ std::string cmTarget::GetInterfaceFileSetsPropertyName(const std::string& type)
   return "";
 }
 
+std::vector<std::string> cmTarget::GetAllFileSetNames() const
+{
+  std::vector<std::string> result;
+
+  for (auto const& it : this->impl->FileSets) {
+    result.push_back(it.first);
+  }
+
+  return result;
+}
+
 std::vector<std::string> cmTarget::GetAllInterfaceFileSets() const
 {
   std::vector<std::string> result;

Source/cmTarget.h

@@ -292,6 +292,7 @@ public:
                                                  const std::string& type,
                                                  cmFileSetVisibility vis);
 
+  std::vector<std::string> GetAllFileSetNames() const;
   std::vector<std::string> GetAllInterfaceFileSets() const;
 
   static std::string GetFileSetsPropertyName(const std::string& type);

Tests/RunCMake/AndroidMK/AndroidMK-check.cmake

@@ -26,5 +26,5 @@ compare_file_to_expected(
 "${RunCMake_BINARY_DIR}/AndroidMK-build/Android.mk"
 "${RunCMake_TEST_SOURCE_DIR}/expectedBuildAndroidMK.txt")
 compare_file_to_expected(
-"${RunCMake_BINARY_DIR}/AndroidMK-build/CMakeFiles/Export/share/ndk-modules/Android.mk"
+"${RunCMake_BINARY_DIR}/AndroidMK-build/CMakeFiles/Export/c8a72b7cccded047a31c221a6b84dd48/Android.mk"
 "${RunCMake_TEST_SOURCE_DIR}/expectedInstallAndroidMK.txt")

Tests/RunCMake/FileAPI/codemodel-v2-data/directories/top.json

@@ -496,7 +496,7 @@
             "type": "export",
             "destination": "lib/cmake/foo",
             "paths": [
-                "^CMakeFiles/Export/lib/cmake/foo/FooTargets\\.cmake$"
+                "^CMakeFiles/Export/22ecfa717ccadd33cf3e4bcbabcbde6b/FooTargets\\.cmake$"
             ],
             "isExcludeFromAll": null,
             "isForAllComponents": null,

Tests/RunCMake/install/EXPORT-TargetTwice-check.cmake

@@ -1,4 +1,4 @@
-set(pkg1_cmake "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/Export/pkg1/pkg1.cmake")
+set(pkg1_cmake "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/Export/59965f5e1aafdb63698f8ae505daf864/pkg1.cmake")
 file(STRINGS "${pkg1_cmake}" pkg1_includes REGEX INTERFACE_INCLUDE_DIRECTORIES)
 set(pkg1_expect [[INTERFACE_INCLUDE_DIRECTORIES "\${_IMPORT_PREFIX}/pkg1/inc"]])
 if(NOT pkg1_includes MATCHES "${pkg1_expect}")
@@ -8,7 +8,7 @@ It does not match:
   ${pkg1_expect}")
 endif()
 
-set(pkg2_cmake "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/Export/pkg2/pkg2.cmake")
+set(pkg2_cmake "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/Export/72c00a5f9d34b6649110956cfc9f27e6/pkg2.cmake")
 file(STRINGS "${pkg2_cmake}" pkg2_includes REGEX INTERFACE_INCLUDE_DIRECTORIES)
 set(pkg2_expect [[INTERFACE_INCLUDE_DIRECTORIES "\${_IMPORT_PREFIX}/pkg2/inc"]])
 if(NOT pkg2_includes MATCHES "${pkg2_expect}")