Browse Source

export: Allow depending on targets exported multiple times

Fixes: #26556
Martin Duffy 10 months ago
parent
commit
c8997fc046

+ 24 - 15
Source/cmExportBuildFileGenerator.cxx

@@ -140,8 +140,8 @@ void cmExportBuildFileGenerator::HandleMissingTarget(
   if (!this->AppendMode) {
     auto const& exportInfo = this->FindExportInfo(dependee);
 
-    if (exportInfo.Files.size() == 1) {
-      std::string missingTarget = exportInfo.Namespace;
+    if (exportInfo.Namespaces.size() == 1 && exportInfo.Sets.size() == 1) {
+      std::string missingTarget = *exportInfo.Namespaces.begin();
 
       missingTarget += dependee->GetExportName();
       link_libs += missingTarget;
@@ -150,7 +150,7 @@ void cmExportBuildFileGenerator::HandleMissingTarget(
     }
     // We are not appending, so all exported targets should be
     // known here.  This is probably user-error.
-    this->ComplainAboutMissingTarget(depender, dependee, exportInfo.Files);
+    this->ComplainAboutMissingTarget(depender, dependee, exportInfo);
   }
   // Assume the target will be exported by another command.
   // Append it with the export namespace.
@@ -178,42 +178,51 @@ cmExportFileGenerator::ExportInfo cmExportBuildFileGenerator::FindExportInfo(
   cmGeneratorTarget const* target) const
 {
   std::vector<std::string> exportFiles;
-  std::string ns;
+  std::set<std::string> exportSets;
+  std::set<std::string> namespaces;
 
   auto const& name = target->GetName();
-  auto& exportSets =
+  auto& allExportSets =
     target->GetLocalGenerator()->GetGlobalGenerator()->GetBuildExportSets();
 
-  for (auto const& exp : exportSets) {
+  for (auto const& exp : allExportSets) {
     auto const& exportSet = exp.second;
     std::vector<TargetExport> targets;
     exportSet->GetTargets(targets);
     if (std::any_of(
           targets.begin(), targets.end(),
           [&name](TargetExport const& te) { return te.Name == name; })) {
+      exportSets.insert(exp.first);
       exportFiles.push_back(exp.first);
-      ns = exportSet->GetNamespace();
+      namespaces.insert(exportSet->GetNamespace());
     }
   }
 
-  return { exportFiles, exportFiles.size() == 1 ? ns : std::string{} };
+  return { exportFiles, exportSets, namespaces };
 }
 
 void cmExportBuildFileGenerator::ComplainAboutMissingTarget(
   cmGeneratorTarget const* depender, cmGeneratorTarget const* dependee,
-  std::vector<std::string> const& exportFiles) const
+  ExportInfo const& exportInfo) const
 {
   std::ostringstream e;
   e << "export called with target \"" << depender->GetName()
     << "\" which requires target \"" << dependee->GetName() << "\" ";
-  if (exportFiles.empty()) {
+  if (exportInfo.Sets.empty()) {
     e << "that is not in any export set.";
   } else {
-    e << "that is not in this export set, but in multiple other export sets: "
-      << cmJoin(exportFiles, ", ") << ".\n";
-    e << "An exported target cannot depend upon another target which is "
-         "exported multiple times. Consider consolidating the exports of the "
-         "\""
+    if (exportInfo.Sets.size() == 1) {
+      e << "that is not in this export set, but in another export set which "
+           "is "
+           "exported multiple times with different namespaces: ";
+    } else {
+      e << "that is not in this export set, but in multiple other export "
+           "sets: ";
+    }
+    e << cmJoin(exportInfo.Files, ", ") << ".\n"
+      << "An exported target cannot depend upon another target which is "
+         "exported in more than one export set or with more than one "
+         "namespace. Consider consolidating the exports of the \""
       << dependee->GetName() << "\" target to a single export.";
   }
 

+ 3 - 3
Source/cmExportBuildFileGenerator.h

@@ -79,9 +79,9 @@ protected:
                            cmGeneratorTarget const* depender,
                            cmGeneratorTarget* dependee) override;
 
-  void ComplainAboutMissingTarget(
-    cmGeneratorTarget const* depender, cmGeneratorTarget const* dependee,
-    std::vector<std::string> const& exportFiles) const;
+  void ComplainAboutMissingTarget(cmGeneratorTarget const* depender,
+                                  cmGeneratorTarget const* dependee,
+                                  ExportInfo const& exportInfo) const;
 
   void ComplainAboutDuplicateTarget(
     std::string const& targetName) const override;

+ 2 - 1
Source/cmExportFileGenerator.h

@@ -135,7 +135,8 @@ protected:
   struct ExportInfo
   {
     std::vector<std::string> Files;
-    std::string Namespace;
+    std::set<std::string> Sets;
+    std::set<std::string> Namespaces;
   };
 
   /** Find the set of export files and the unique namespace (if any) for a

+ 25 - 16
Source/cmExportInstallFileGenerator.cxx

@@ -249,8 +249,8 @@ void cmExportInstallFileGenerator::HandleMissingTarget(
 {
   auto const& exportInfo = this->FindExportInfo(dependee);
 
-  if (exportInfo.Files.size() == 1) {
-    std::string missingTarget = exportInfo.Namespace;
+  if (exportInfo.Namespaces.size() == 1 && exportInfo.Sets.size() == 1) {
+    std::string missingTarget = *exportInfo.Namespaces.begin();
 
     missingTarget += dependee->GetExportName();
     link_libs += missingTarget;
@@ -258,7 +258,7 @@ void cmExportInstallFileGenerator::HandleMissingTarget(
   } else {
     // All exported targets should be known here and should be unique.
     // This is probably user-error.
-    this->ComplainAboutMissingTarget(depender, dependee, exportInfo.Files);
+    this->ComplainAboutMissingTarget(depender, dependee, exportInfo);
   }
 }
 
@@ -266,13 +266,14 @@ cmExportFileGenerator::ExportInfo cmExportInstallFileGenerator::FindExportInfo(
   cmGeneratorTarget const* target) const
 {
   std::vector<std::string> exportFiles;
-  std::string ns;
+  std::set<std::string> exportSets;
+  std::set<std::string> namespaces;
 
   auto const& name = target->GetName();
-  auto& exportSets =
+  auto& allExportSets =
     target->GetLocalGenerator()->GetGlobalGenerator()->GetExportSets();
 
-  for (auto const& exp : exportSets) {
+  for (auto const& exp : allExportSets) {
     auto const& exportSet = exp.second;
     auto const& targets = exportSet.GetTargetExports();
 
@@ -280,35 +281,43 @@ cmExportFileGenerator::ExportInfo cmExportInstallFileGenerator::FindExportInfo(
                     [&name](std::unique_ptr<cmTargetExport> const& te) {
                       return te->TargetName == name;
                     })) {
+      exportSets.insert(exp.first);
       std::vector<cmInstallExportGenerator const*> const* installs =
         exportSet.GetInstallations();
       for (cmInstallExportGenerator const* install : *installs) {
         exportFiles.push_back(install->GetDestinationFile());
-        ns = install->GetNamespace();
+        namespaces.insert(install->GetNamespace());
       }
     }
   }
-
-  return { exportFiles, exportFiles.size() == 1 ? ns : std::string{} };
+  return { exportFiles, exportSets, namespaces };
 }
 
 void cmExportInstallFileGenerator::ComplainAboutMissingTarget(
   cmGeneratorTarget const* depender, cmGeneratorTarget const* dependee,
-  std::vector<std::string> const& exportFiles) const
+  ExportInfo const& exportInfo) const
 {
   std::ostringstream e;
   e << "install(" << this->IEGen->InstallSubcommand() << " \""
     << this->GetExportName() << "\" ...) "
     << "includes target \"" << depender->GetName()
     << "\" which requires target \"" << dependee->GetName() << "\" ";
-  if (exportFiles.empty()) {
+  if (exportInfo.Sets.empty()) {
     e << "that is not in any export set.";
   } else {
-    e << "that is not in this export set, but in multiple other export sets: "
-      << cmJoin(exportFiles, ", ") << ".\n";
-    e << "An exported target cannot depend upon another target which is "
-         "exported multiple times. Consider consolidating the exports of the "
-         "\""
+    if (exportInfo.Sets.size() == 1) {
+      e << "that is not in this export set, but in another export set which "
+           "is "
+           "exported multiple times with different namespaces: ";
+    } else {
+      e << "that is not in this export set, but in multiple other export "
+           "sets: ";
+    }
+    e << cmJoin(exportInfo.Files, ", ") << ".\n"
+      << "An exported target cannot depend upon another target which is "
+         "exported in more than one export set or with more than one "
+         "namespace. "
+         "Consider consolidating the exports of the \""
       << dependee->GetName() << "\" target to a single export.";
   }
   this->ReportError(e.str());

+ 3 - 3
Source/cmExportInstallFileGenerator.h

@@ -86,9 +86,9 @@ protected:
 
   void ReplaceInstallPrefix(std::string& input) const override;
 
-  void ComplainAboutMissingTarget(
-    cmGeneratorTarget const* depender, cmGeneratorTarget const* dependee,
-    std::vector<std::string> const& exportFiles) const;
+  void ComplainAboutMissingTarget(cmGeneratorTarget const* depender,
+                                  cmGeneratorTarget const* dependee,
+                                  ExportInfo const& exportInfo) const;
 
   void ComplainAboutDuplicateTarget(
     std::string const& targetName) const override;

+ 4 - 4
Source/cmExportPackageInfoGenerator.cxx

@@ -264,10 +264,10 @@ bool cmExportPackageInfoGenerator::NoteLinkedTarget(
     return true;
   }
 
-  // Target belongs to another export from this build.
+  // Target belongs to multiple namespaces or multiple export sets.
   auto const& exportInfo = this->FindExportInfo(linkedTarget);
-  if (exportInfo.Files.size() == 1) {
-    auto const& linkNamespace = exportInfo.Namespace;
+  if (exportInfo.Namespaces.size() == 1 && exportInfo.Sets.size() == 1) {
+    auto const& linkNamespace = *exportInfo.Namespaces.begin();
     if (!cmHasSuffix(linkNamespace, "::")) {
       target->Makefile->IssueMessage(
         MessageType::FATAL_ERROR,
@@ -293,7 +293,7 @@ bool cmExportPackageInfoGenerator::NoteLinkedTarget(
   }
 
   // cmExportFileGenerator::HandleMissingTarget should have complained about
-  // this already. (In fact, we probably shouldn't ever get here.)
+  // this already.
   return false;
 }
 

+ 1 - 1
Source/cmExportTryCompileFileGenerator.h

@@ -47,7 +47,7 @@ protected:
 
   ExportInfo FindExportInfo(cmGeneratorTarget const* /*target*/) const override
   {
-    return { {}, {} };
+    return { {}, {}, {} };
   }
 
   void PopulateProperties(cmGeneratorTarget const* target,

+ 3 - 0
Tests/RunCMake/PackageInfo/DependsMultiple.cmake

@@ -0,0 +1,3 @@
+project(DependsMultiple CXX)
+set(NAMESPACE foo::)
+include(DependsMultipleCommon.cmake)

+ 11 - 0
Tests/RunCMake/PackageInfo/DependsMultipleCommon.cmake

@@ -0,0 +1,11 @@
+add_library(foo foo.cxx)
+add_library(bar foo.cxx)
+target_link_libraries(bar foo)
+
+install(TARGETS foo EXPORT foo)
+install(EXPORT foo DESTINATION cmake NAMESPACE ${NAMESPACE})
+install(PACKAGE_INFO foo EXPORT foo DESTINATION cps)
+
+install(TARGETS bar EXPORT bar)
+install(EXPORT bar DESTINATION .)
+install(PACKAGE_INFO bar EXPORT bar DESTINATION cps)

+ 1 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-result.txt

@@ -0,0 +1 @@
+1

+ 19 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-stderr.txt

@@ -0,0 +1,19 @@
+CMake Error in CMakeLists.txt:
+  install\(EXPORT "bar" ...\) includes target "bar" which requires target "foo"
+  that is not in this export set, but in another export set which is exported
+  multiple times with different namespaces: cmake/foo.cmake, cps/foo.cps.
+
+  An exported target cannot depend upon another target which is exported in
+  more than one export set or with more than one namespace.  Consider
+  consolidating the exports of the "foo" target to a single export.
+
+
+CMake Error in CMakeLists.txt:
+  install\(PACKAGE_INFO "bar" ...\) includes target "bar" which requires target
+  "foo" that is not in this export set, but in another export set which is
+  exported multiple times with different namespaces: cmake/foo.cmake,
+  cps/foo.cps.
+
+  An exported target cannot depend upon another target which is exported in
+  more than one export set or with more than one namespace.  Consider
+  consolidating the exports of the "foo" target to a single export.

+ 3 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace.cmake

@@ -0,0 +1,3 @@
+project(DependsMultipleDifferentNamespace CXX)
+set(NAMESPACE "")
+include(DependsMultipleCommon.cmake)

+ 1 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-result.txt

@@ -0,0 +1 @@
+1

+ 18 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-stderr.txt

@@ -0,0 +1,18 @@
+CMake Error in CMakeLists.txt:
+  install\(EXPORT "bar" ...\) includes target "bar" which requires target "foo"
+  that is not in this export set, but in multiple other export sets:
+  cmake/foo.cmake, cps/foo.cps, cmake/foo-alt.cmake.
+
+  An exported target cannot depend upon another target which is exported in
+  more than one export set or with more than one namespace.  Consider
+  consolidating the exports of the "foo" target to a single export.
+
+
+CMake Error in CMakeLists.txt:
+  install\(PACKAGE_INFO "bar" ...\) includes target "bar" which requires target
+  "foo" that is not in this export set, but in multiple other export sets:
+  cmake/foo.cmake, cps/foo.cps, cmake/foo-alt.cmake.
+
+  An exported target cannot depend upon another target which is exported in
+  more than one export set or with more than one namespace.  Consider
+  consolidating the exports of the "foo" target to a single export.

+ 5 - 0
Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets.cmake

@@ -0,0 +1,5 @@
+project(DependsMultipleDifferentSets CXX)
+include(DependsMultipleCommon.cmake)
+
+install(TARGETS foo EXPORT foo-alt)
+install(EXPORT foo-alt DESTINATION cmake)

+ 3 - 0
Tests/RunCMake/PackageInfo/RunCMakeTest.cmake

@@ -21,6 +21,8 @@ run_cmake(ReferencesNonExportedTarget)
 run_cmake(ReferencesWronglyExportedTarget)
 run_cmake(ReferencesWronglyImportedTarget)
 run_cmake(ReferencesWronglyNamespacedTarget)
+run_cmake(DependsMultipleDifferentNamespace)
+run_cmake(DependsMultipleDifferentSets)
 
 # Test functionality
 run_cmake(Appendix)
@@ -31,3 +33,4 @@ run_cmake(MinimalVersion)
 run_cmake(LowerCaseFile)
 run_cmake(Requirements)
 run_cmake(TargetTypes)
+run_cmake(DependsMultiple)

+ 4 - 3
Tests/RunCMake/export/DependOnDoubleExport-stderr.txt

@@ -4,9 +4,10 @@ CMake Error in CMakeLists.txt:
   .*/Tests/RunCMake/export/DependOnDoubleExport-build/exportset.cmake,.*
   .*/Tests/RunCMake/export/DependOnDoubleExport-build/manual.cmake.
 +
-  An exported target cannot depend upon another target which is exported
-  multiple times.  Consider consolidating the exports of the "doubleexported"
-  target to a single export.
+  An exported target cannot depend upon another target which is exported in
+  more than one export set or with more than one namespace.  Consider
+  consolidating the exports of the "doubleexported" target to a single
+  export.
 
 
 CMake Generate step failed.  Build files cannot be regenerated correctly.