Merge topic 'export-name-safe-name'

142a85f9c1 cxxmodules: use filesystem-safe export names in filenames
4452d41488 cmGeneratorTarget: add method to get a filesystem-safe export name

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

Source/cmDyndepCollation.cxx

@@ -207,6 +207,7 @@ Json::Value CollationInformationExports(cmGeneratorTarget const* gt)
 {
   Json::Value tdi_exports = Json::arrayValue;
   std::string export_name = gt->GetExportName();
+  std::string fs_export_name = gt->GetFilesystemExportName();
 
   auto const& all_install_exports = gt->GetGlobalGenerator()->GetExportSets();
   for (auto const& exp : all_install_exports) {
@@ -232,6 +233,7 @@ Json::Value CollationInformationExports(cmGeneratorTarget const* gt)
 
       tdi_export_info["namespace"] = ns;
       tdi_export_info["export-name"] = export_name;
+      tdi_export_info["filesystem-export-name"] = fs_export_name;
       tdi_export_info["destination"] = dest;
       tdi_export_info["cxx-module-info-dir"] = cxxm_dir;
       tdi_export_info["export-prefix"] = export_prefix;
@@ -268,6 +270,7 @@ Json::Value CollationInformationExports(cmGeneratorTarget const* gt)
 
     tdi_export_info["namespace"] = ns;
     tdi_export_info["export-name"] = export_name;
+    tdi_export_info["filesystem-export-name"] = fs_export_name;
     tdi_export_info["destination"] = dest;
     tdi_export_info["cxx-module-info-dir"] = cxxm_dir;
     tdi_export_info["export-prefix"] = export_prefix;
@@ -315,6 +318,7 @@ struct CxxModuleBmiInstall
 struct CxxModuleExport
 {
   std::string Name;
+  std::string FilesystemName;
   std::string Destination;
   std::string Prefix;
   std::string CxxModuleInfoDir;
@@ -352,6 +356,7 @@ cmDyndepCollation::ParseExportInfo(Json::Value const& tdi)
       CxxModuleExport exp;
       exp.Install = tdi_export["install"].asBool();
       exp.Name = tdi_export["export-name"].asString();
+      exp.FilesystemName = tdi_export["filesystem-export-name"].asString();
       exp.Destination = tdi_export["destination"].asString();
       exp.Prefix = tdi_export["export-prefix"].asString();
       exp.CxxModuleInfoDir = tdi_export["cxx-module-info-dir"].asString();
@@ -426,8 +431,9 @@ bool cmDyndepCollation::WriteDyndepMetadata(
 
     std::string const export_dir =
       cmStrCat(exp.Prefix, '/', exp.CxxModuleInfoDir, '/');
-    std::string const property_file_path = cmStrCat(
-      export_dir, "target-", exp.Name, '-', export_info.Config, ".cmake");
+    std::string const property_file_path =
+      cmStrCat(export_dir, "target-", exp.FilesystemName, '-',
+               export_info.Config, ".cmake");
     properties = cm::make_unique<cmGeneratedFileStream>(property_file_path);
 
     // Set up the preamble.

Source/cmExportBuildFileGenerator.cxx

@@ -591,8 +591,8 @@ bool cmExportBuildFileGenerator::GenerateImportCxxModuleConfigTargetInclusion(
       continue;
     }
 
-    os << "include(\"${CMAKE_CURRENT_LIST_DIR}/target-" << tgt->GetExportName()
-       << '-' << config << ".cmake\")\n";
+    os << "include(\"${CMAKE_CURRENT_LIST_DIR}/target-"
+       << tgt->GetFilesystemExportName() << '-' << config << ".cmake\")\n";
   }
 
   return true;

Source/cmExportInstallFileGenerator.cxx

@@ -780,8 +780,8 @@ bool cmExportInstallFileGenerator::
       continue;
     }
 
-    auto prop_filename = cmStrCat("target-", tgt->GetExportName(), '-',
-                                  filename_config, ".cmake");
+    auto prop_filename = cmStrCat("target-", tgt->GetFilesystemExportName(),
+                                  '-', filename_config, ".cmake");
     prop_files.emplace_back(cmStrCat(dest, prop_filename));
     os << "include(\"${CMAKE_CURRENT_LIST_DIR}/" << prop_filename << "\")\n";
   }

Source/cmGeneratorTarget.cxx

@@ -369,6 +369,16 @@ std::string cmGeneratorTarget::GetExportName() const
   return this->GetName();
 }
 
+std::string cmGeneratorTarget::GetFilesystemExportName() const
+{
+  auto fs_safe = this->GetExportName();
+  // First escape any `_` characters to avoid collisions.
+  cmSystemTools::ReplaceString(fs_safe, "_", "__");
+  // Escape other characters that are not generally filesystem-safe.
+  cmSystemTools::ReplaceString(fs_safe, ":", "_c");
+  return fs_safe;
+}
+
 cmValue cmGeneratorTarget::GetProperty(const std::string& prop) const
 {
   if (cmValue result =

Source/cmGeneratorTarget.h

@@ -99,6 +99,7 @@ public:
   cmStateEnums::TargetType GetType() const;
   const std::string& GetName() const;
   std::string GetExportName() const;
+  std::string GetFilesystemExportName() const;
 
   std::vector<std::string> GetPropertyKeys() const;
   //! Might return a nullptr if the property is not set or invalid

Tests/RunCMake/CXXModules/NinjaDependInfoExportFilesystemSafe-check.cmake

@@ -0,0 +1,39 @@
+include("${CMAKE_CURRENT_LIST_DIR}/check-json.cmake")
+
+if (RunCMake_GENERATOR_IS_MULTI_CONFIG)
+  set(have_file 0)
+  foreach (CXXModules_config IN ITEMS Release Debug RelWithDebInfo MinSizeRel)
+    if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/ninja-exports-public.dir/${CXXModules_config}/CXXDependInfo.json")
+      continue ()
+    endif ()
+    set(have_file 1)
+
+    set(CMAKE_BUILD_TYPE "${CXXModules_config}")
+
+    file(READ "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/ninja-exports-public.dir/${CXXModules_config}/CXXDependInfo.json" actual_contents)
+    file(READ "${CMAKE_CURRENT_LIST_DIR}/expect/NinjaDependInfoExportFilesystemSafe-public.json" expect_contents)
+    check_json("${actual_contents}" "${expect_contents}")
+
+    file(READ "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/ninja-exports-private.dir/${CXXModules_config}/CXXDependInfo.json" actual_contents)
+    file(READ "${CMAKE_CURRENT_LIST_DIR}/expect/NinjaDependInfoExportFilesystemSafe-private.json" expect_contents)
+    check_json("${actual_contents}" "${expect_contents}")
+  endforeach ()
+
+  if (NOT have_file)
+    list(APPEND RunCMake_TEST_FAILED
+      "No recognized build configurations found.")
+  endif ()
+else ()
+  set(CXXModules_config "${CXXModules_default_build_type}")
+  set(CMAKE_BUILD_TYPE "${CXXModules_default_build_type}")
+
+  file(READ "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/ninja-exports-public.dir/CXXDependInfo.json" actual_contents)
+  file(READ "${CMAKE_CURRENT_LIST_DIR}/expect/NinjaDependInfoExportFilesystemSafe-public.json" expect_contents)
+  check_json("${actual_contents}" "${expect_contents}")
+
+  file(READ "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/ninja-exports-private.dir/CXXDependInfo.json" actual_contents)
+  file(READ "${CMAKE_CURRENT_LIST_DIR}/expect/NinjaDependInfoExportFilesystemSafe-private.json" expect_contents)
+  check_json("${actual_contents}" "${expect_contents}")
+endif ()
+
+string(REPLACE ";" "\n  " RunCMake_TEST_FAILED "${RunCMake_TEST_FAILED}")

Tests/RunCMake/CXXModules/NinjaDependInfoExportFilesystemSafe.cmake

@@ -0,0 +1,84 @@
+# Fake out that we have dyndep; we only need to generate, not actually build
+# here.
+set(CMAKE_CXX_SCANDEP_SOURCE "")
+
+enable_language(CXX)
+
+if (NOT CMAKE_GENERATOR MATCHES "Ninja")
+  message(FATAL_ERROR
+    "This test requires a 'Ninja' generator to be used.")
+endif ()
+
+add_library(ninja-exports-public)
+target_sources(ninja-exports-public
+  PRIVATE
+    sources/module-impl.cxx
+    sources/module-internal-part-impl.cxx
+    sources/module-part-impl.cxx
+    sources/module-use.cxx
+  PUBLIC
+    FILE_SET modules TYPE CXX_MODULES
+    BASE_DIRS
+      "${CMAKE_CURRENT_SOURCE_DIR}/sources"
+    FILES
+      sources/module.cxx
+      sources/module-part.cxx
+    FILE_SET internal_partitions TYPE CXX_MODULES FILES
+      sources/module-internal-part.cxx)
+target_compile_features(ninja-exports-public
+  PRIVATE
+    cxx_std_20)
+set_property(TARGET ninja-exports-public
+  PROPERTY EXPORT_NAME "with::public_")
+
+install(TARGETS ninja-exports-public
+  EXPORT exp
+  FILE_SET modules
+    DESTINATION "lib/cxx"
+    COMPONENT "modules"
+  FILE_SET internal_partitions
+    DESTINATION "lib/cxx/internals"
+    COMPONENT "modules-internal")
+
+add_library(ninja-exports-private)
+target_sources(ninja-exports-private
+  PRIVATE
+    sources/module-impl.cxx
+    sources/module-internal-part-impl.cxx
+    sources/module-part-impl.cxx
+    sources/module-use.cxx
+  PRIVATE
+    FILE_SET modules TYPE CXX_MODULES
+    BASE_DIRS
+      "${CMAKE_CURRENT_SOURCE_DIR}/sources"
+    FILES
+      sources/module.cxx
+      sources/module-part.cxx
+    FILE_SET internal_partitions TYPE CXX_MODULES FILES
+      sources/module-internal-part.cxx)
+target_compile_features(ninja-exports-private
+  PRIVATE
+    cxx_std_20)
+set_property(TARGET ninja-exports-private
+  PROPERTY EXPORT_NAME "with::private_")
+
+install(TARGETS ninja-exports-private
+  EXPORT exp)
+
+# Test multiple build exports.
+export(EXPORT exp
+  FILE "${CMAKE_BINARY_DIR}/lib/cmake/export1/export1-targets.cmake"
+  NAMESPACE export1::
+  CXX_MODULES_DIRECTORY "cxx-modules")
+export(EXPORT exp
+  FILE "${CMAKE_BINARY_DIR}/lib/cmake/export2/export2-targets.cmake"
+  CXX_MODULES_DIRECTORY "cxx-modules")
+
+# Test multiple install exports.
+install(EXPORT exp
+  DESTINATION "lib/cmake/export1"
+  NAMESPACE export1::
+  CXX_MODULES_DIRECTORY "cxx-modules")
+install(EXPORT exp
+  DESTINATION "lib/cmake/export2"
+  CXX_MODULES_DIRECTORY "cxx-modules")

Tests/RunCMake/CXXModules/RunCMakeTest.cmake

@@ -109,6 +109,7 @@ run_cmake(ExportInstallCxxModules)
 if (RunCMake_GENERATOR MATCHES "Ninja")
   run_cmake(NinjaDependInfoFileSet)
   run_cmake(NinjaDependInfoExport)
+  run_cmake(NinjaDependInfoExportFilesystemSafe)
   run_cmake(NinjaDependInfoBMIInstall)
   run_cmake(NinjaForceResponseFile) # issue#25367
 elseif (RunCMake_GENERATOR MATCHES "Visual Studio")

Tests/RunCMake/CXXModules/expect/NinjaDependInfoExport-private.json

@@ -43,6 +43,7 @@
       "destination" : "lib/cmake/export1",
       "export-name" : "with-private",
       "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/d2e2673818fd2bd8c45c0e3ed0e38fcd",
+      "filesystem-export-name" : "with-private",
       "install" : true,
       "namespace" : "export1::"
     },
@@ -51,6 +52,7 @@
       "destination" : "lib/cmake/export2",
       "export-name" : "with-private",
       "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/28cd47cb4c96ad5cadaa3fb1b0201ae8",
+      "filesystem-export-name" : "with-private",
       "install" : true,
       "namespace" : ""
     },
@@ -59,6 +61,7 @@
       "destination" : "<BINARY_DIR>/lib/cmake/export1",
       "export-name" : "with-private",
       "export-prefix" : "<BINARY_DIR>/lib/cmake/export1",
+      "filesystem-export-name" : "with-private",
       "install" : false,
       "namespace" : "export1::"
     },
@@ -67,6 +70,7 @@
       "destination" : "<BINARY_DIR>/lib/cmake/export2",
       "export-name" : "with-private",
       "export-prefix" : "<BINARY_DIR>/lib/cmake/export2",
+      "filesystem-export-name" : "with-private",
       "install" : false,
       "namespace" : ""
     }

Tests/RunCMake/CXXModules/expect/NinjaDependInfoExport-public.json

@@ -43,6 +43,7 @@
       "destination" : "lib/cmake/export1",
       "export-name" : "with-public",
       "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/d2e2673818fd2bd8c45c0e3ed0e38fcd",
+      "filesystem-export-name" : "with-public",
       "install" : true,
       "namespace" : "export1::"
     },
@@ -51,6 +52,7 @@
       "destination" : "lib/cmake/export2",
       "export-name" : "with-public",
       "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/28cd47cb4c96ad5cadaa3fb1b0201ae8",
+      "filesystem-export-name" : "with-public",
       "install" : true,
       "namespace" : ""
     },
@@ -59,6 +61,7 @@
       "destination" : "<BINARY_DIR>/lib/cmake/export1",
       "export-name" : "with-public",
       "export-prefix" : "<BINARY_DIR>/lib/cmake/export1",
+      "filesystem-export-name" : "with-public",
       "install" : false,
       "namespace" : "export1::"
     },
@@ -67,6 +70,7 @@
       "destination" : "<BINARY_DIR>/lib/cmake/export2",
       "export-name" : "with-public",
       "export-prefix" : "<BINARY_DIR>/lib/cmake/export2",
+      "filesystem-export-name" : "with-public",
       "install" : false,
       "namespace" : ""
     }

Tests/RunCMake/CXXModules/expect/NinjaDependInfoExportFilesystemSafe-private.json

@@ -0,0 +1,83 @@
+{
+  "bmi-installation": null,
+  "compiler-id": "<IGNORE>",
+  "compiler-frontend-variant": "<IGNORE>",
+  "compiler-simulate-id": "<IGNORE>",
+  "config": "<CONFIG>",
+  "cxx-modules": {
+    "CMakeFiles/ninja-exports-private.dir<CONFIG_DIR>/sources/module-internal-part.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": null,
+      "name": "internal_partitions",
+      "relative-directory": "sources",
+      "source": "<SOURCE_DIR>/sources/module-internal-part.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PRIVATE"
+    },
+    "CMakeFiles/ninja-exports-private.dir<CONFIG_DIR>/sources/module-part.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": null,
+      "name": "modules",
+      "relative-directory": "",
+      "source": "<SOURCE_DIR>/sources/module-part.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PRIVATE"
+    },
+    "CMakeFiles/ninja-exports-private.dir<CONFIG_DIR>/sources/module.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": null,
+      "name": "modules",
+      "relative-directory": "",
+      "source": "<SOURCE_DIR>/sources/module.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PRIVATE"
+    }
+  },
+  "dir-cur-bld": "<BINARY_DIR>",
+  "dir-cur-src": "<SOURCE_DIR>",
+  "dir-top-bld": "<BINARY_DIR>",
+  "dir-top-src": "<SOURCE_DIR>",
+  "exports": [
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "lib/cmake/export1",
+      "export-name" : "with::private_",
+      "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/d2e2673818fd2bd8c45c0e3ed0e38fcd",
+      "filesystem-export-name" : "with_c_cprivate__",
+      "install" : true,
+      "namespace" : "export1::"
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "lib/cmake/export2",
+      "export-name" : "with::private_",
+      "filesystem-export-name" : "with_c_cprivate__",
+      "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/28cd47cb4c96ad5cadaa3fb1b0201ae8",
+      "install" : true,
+      "namespace" : ""
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "<BINARY_DIR>/lib/cmake/export1",
+      "export-name" : "with::private_",
+      "export-prefix" : "<BINARY_DIR>/lib/cmake/export1",
+      "filesystem-export-name" : "with_c_cprivate__",
+      "install" : false,
+      "namespace" : "export1::"
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "<BINARY_DIR>/lib/cmake/export2",
+      "export-name" : "with::private_",
+      "export-prefix" : "<BINARY_DIR>/lib/cmake/export2",
+      "filesystem-export-name" : "with_c_cprivate__",
+      "install" : false,
+      "namespace" : ""
+    }
+  ],
+  "include-dirs": [],
+  "language": "CXX",
+  "forward-modules-from-target-dirs": [],
+  "linked-target-dirs": [],
+  "module-dir": "<BINARY_DIR>/CMakeFiles/ninja-exports-private.dir<CONFIG_DIR>"
+}

Tests/RunCMake/CXXModules/expect/NinjaDependInfoExportFilesystemSafe-public.json

@@ -0,0 +1,83 @@
+{
+  "bmi-installation": null,
+  "compiler-id": "<IGNORE>",
+  "compiler-frontend-variant": "<IGNORE>",
+  "compiler-simulate-id": "<IGNORE>",
+  "config": "<CONFIG>",
+  "cxx-modules": {
+    "CMakeFiles/ninja-exports-public.dir<CONFIG_DIR>/sources/module-internal-part.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": "lib/cxx/internals",
+      "name": "internal_partitions",
+      "relative-directory": "sources",
+      "source": "<SOURCE_DIR>/sources/module-internal-part.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PUBLIC"
+    },
+    "CMakeFiles/ninja-exports-public.dir<CONFIG_DIR>/sources/module-part.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": "lib/cxx",
+      "name": "modules",
+      "relative-directory": "",
+      "source": "<SOURCE_DIR>/sources/module-part.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PUBLIC"
+    },
+    "CMakeFiles/ninja-exports-public.dir<CONFIG_DIR>/sources/module.cxx<OBJEXT>": {
+      "bmi-only": false,
+      "destination": "lib/cxx",
+      "name": "modules",
+      "relative-directory": "",
+      "source": "<SOURCE_DIR>/sources/module.cxx",
+      "type": "CXX_MODULES",
+      "visibility": "PUBLIC"
+    }
+  },
+  "dir-cur-bld": "<BINARY_DIR>",
+  "dir-cur-src": "<SOURCE_DIR>",
+  "dir-top-bld": "<BINARY_DIR>",
+  "dir-top-src": "<SOURCE_DIR>",
+  "exports": [
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "lib/cmake/export1",
+      "export-name" : "with::public_",
+      "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/d2e2673818fd2bd8c45c0e3ed0e38fcd",
+      "filesystem-export-name" : "with_c_cpublic__",
+      "install" : true,
+      "namespace" : "export1::"
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "lib/cmake/export2",
+      "export-name" : "with::public_",
+      "export-prefix" : "<BINARY_DIR>/CMakeFiles/Export/28cd47cb4c96ad5cadaa3fb1b0201ae8",
+      "filesystem-export-name" : "with_c_cpublic__",
+      "install" : true,
+      "namespace" : ""
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "<BINARY_DIR>/lib/cmake/export1",
+      "export-name" : "with::public_",
+      "export-prefix" : "<BINARY_DIR>/lib/cmake/export1",
+      "filesystem-export-name" : "with_c_cpublic__",
+      "install" : false,
+      "namespace" : "export1::"
+    },
+    {
+      "cxx-module-info-dir" : "cxx-modules",
+      "destination" : "<BINARY_DIR>/lib/cmake/export2",
+      "export-name" : "with::public_",
+      "export-prefix" : "<BINARY_DIR>/lib/cmake/export2",
+      "filesystem-export-name" : "with_c_cpublic__",
+      "install" : false,
+      "namespace" : ""
+    }
+  ],
+  "include-dirs": [],
+  "language": "CXX",
+  "forward-modules-from-target-dirs": [],
+  "linked-target-dirs": [],
+  "module-dir": "<BINARY_DIR>/CMakeFiles/ninja-exports-public.dir<CONFIG_DIR>"
+}