From 49cfd390075ba1f92aa96bbf3d3510a813e0b068 Mon Sep 17 00:00:00 2001
From: Ben Boeckel <ben.boeckel@kitware.com>
Date: Wed, 26 Jun 2019 13:48:59 -0400
Subject: cmExportBuildFileGenerator: improve error message

When an exported target depends on another exported target that is
included in multiple build export sets, the error message was woefully
unhelpful. Now, include information about what build exports the
dependent target was included in with instructions for fixing the
problem that are actually helpful.
---
 Source/cmExportBuildFileGenerator.cxx | 45 ++++++++++++++++++-----------------
 Source/cmExportBuildFileGenerator.h   |  7 +++---
 2 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/Source/cmExportBuildFileGenerator.cxx b/Source/cmExportBuildFileGenerator.cxx
index 3fe84a9..5800629 100644
--- a/Source/cmExportBuildFileGenerator.cxx
+++ b/Source/cmExportBuildFileGenerator.cxx
@@ -12,7 +12,6 @@
 #include "cmMessageType.h"
 #include "cmPolicies.h"
 #include "cmStateTypes.h"
-#include "cmSystemTools.h"
 #include "cmTarget.h"
 #include "cmTargetExport.h"
 #include "cmake.h"
@@ -259,11 +258,11 @@ void cmExportBuildFileGenerator::HandleMissingTarget(
     const std::string name = dependee->GetName();
     cmGlobalGenerator* gg =
       dependee->GetLocalGenerator()->GetGlobalGenerator();
-    std::vector<std::string> namespaces = this->FindNamespaces(gg, name);
+    auto exportInfo = this->FindBuildExportInfo(gg, name);
+    std::vector<std::string> const& exportFiles = exportInfo.first;
 
-    int targetOccurrences = static_cast<int>(namespaces.size());
-    if (targetOccurrences == 1) {
-      std::string missingTarget = namespaces[0];
+    if (exportFiles.size() == 1) {
+      std::string missingTarget = exportInfo.second;
 
       missingTarget += dependee->GetExportName();
       link_libs += missingTarget;
@@ -272,7 +271,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, targetOccurrences);
+    this->ComplainAboutMissingTarget(depender, dependee, exportFiles);
   }
   // Assume the target will be exported by another command.
   // Append it with the export namespace.
@@ -292,10 +291,12 @@ void cmExportBuildFileGenerator::GetTargets(
   targets = this->Targets;
 }
 
-std::vector<std::string> cmExportBuildFileGenerator::FindNamespaces(
-  cmGlobalGenerator* gg, const std::string& name)
+std::pair<std::vector<std::string>, std::string>
+cmExportBuildFileGenerator::FindBuildExportInfo(cmGlobalGenerator* gg,
+                                                const std::string& name)
 {
-  std::vector<std::string> namespaces;
+  std::vector<std::string> exportFiles;
+  std::string ns;
 
   std::map<std::string, cmExportBuildFileGenerator*>& exportSets =
     gg->GetBuildExportSets();
@@ -305,31 +306,31 @@ std::vector<std::string> cmExportBuildFileGenerator::FindNamespaces(
     std::vector<std::string> targets;
     exportSet->GetTargets(targets);
     if (std::find(targets.begin(), targets.end(), name) != targets.end()) {
-      namespaces.push_back(exportSet->GetNamespace());
+      exportFiles.push_back(exp.first);
+      ns = exportSet->GetNamespace();
     }
   }
 
-  return namespaces;
+  return std::make_pair(exportFiles, ns);
 }
 
 void cmExportBuildFileGenerator::ComplainAboutMissingTarget(
-  cmGeneratorTarget* depender, cmGeneratorTarget* dependee, int occurrences)
+  cmGeneratorTarget* depender, cmGeneratorTarget* dependee,
+  std::vector<std::string> const& exportFiles)
 {
-  if (cmSystemTools::GetErrorOccuredFlag()) {
-    return;
-  }
-
   std::ostringstream e;
   e << "export called with target \"" << depender->GetName()
     << "\" which requires target \"" << dependee->GetName() << "\" ";
-  if (occurrences == 0) {
-    e << "that is not in the export set.\n";
+  if (exportFiles.empty()) {
+    e << "that is not in any export set.";
   } else {
-    e << "that is not in this export set, but " << occurrences
-      << " times in others.\n";
+    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 "
+         "\""
+      << dependee->GetName() << "\" target to a single export.";
   }
-  e << "If the required target is not easy to reference in this call, "
-    << "consider using the APPEND option with multiple separate calls.";
 
   this->LG->GetGlobalGenerator()->GetCMakeInstance()->IssueMessage(
     MessageType::FATAL_ERROR, e.str(),
diff --git a/Source/cmExportBuildFileGenerator.h b/Source/cmExportBuildFileGenerator.h
index 0a1e755..e5b6597 100644
--- a/Source/cmExportBuildFileGenerator.h
+++ b/Source/cmExportBuildFileGenerator.h
@@ -11,6 +11,7 @@
 
 #include <iosfwd>
 #include <string>
+#include <utility>
 #include <vector>
 
 class cmExportSet;
@@ -64,7 +65,7 @@ protected:
 
   void ComplainAboutMissingTarget(cmGeneratorTarget* depender,
                                   cmGeneratorTarget* dependee,
-                                  int occurrences);
+                                  std::vector<std::string> const& namespaces);
 
   /** Fill in properties indicating built file locations.  */
   void SetImportLocationProperty(const std::string& config,
@@ -75,8 +76,8 @@ protected:
   std::string InstallNameDir(cmGeneratorTarget* target,
                              const std::string& config) override;
 
-  std::vector<std::string> FindNamespaces(cmGlobalGenerator* gg,
-                                          const std::string& name);
+  std::pair<std::vector<std::string>, std::string> FindBuildExportInfo(
+    cmGlobalGenerator* gg, const std::string& name);
 
   std::vector<std::string> Targets;
   cmExportSet* ExportSet;
-- 
cgit v0.12