From 49cfd390075ba1f92aa96bbf3d3510a813e0b068 Mon Sep 17 00:00:00 2001 From: Ben Boeckel 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 namespaces = this->FindNamespaces(gg, name); + auto exportInfo = this->FindBuildExportInfo(gg, name); + std::vector const& exportFiles = exportInfo.first; - int targetOccurrences = static_cast(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 cmExportBuildFileGenerator::FindNamespaces( - cmGlobalGenerator* gg, const std::string& name) +std::pair, std::string> +cmExportBuildFileGenerator::FindBuildExportInfo(cmGlobalGenerator* gg, + const std::string& name) { - std::vector namespaces; + std::vector exportFiles; + std::string ns; std::map& exportSets = gg->GetBuildExportSets(); @@ -305,31 +306,31 @@ std::vector cmExportBuildFileGenerator::FindNamespaces( std::vector 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 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 #include +#include #include class cmExportSet; @@ -64,7 +65,7 @@ protected: void ComplainAboutMissingTarget(cmGeneratorTarget* depender, cmGeneratorTarget* dependee, - int occurrences); + std::vector 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 FindNamespaces(cmGlobalGenerator* gg, - const std::string& name); + std::pair, std::string> FindBuildExportInfo( + cmGlobalGenerator* gg, const std::string& name); std::vector Targets; cmExportSet* ExportSet; -- cgit v0.12 From 27d6e51ae9b57657289a4db6092d8a47472b842e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Thu, 27 Jun 2019 08:50:57 -0400 Subject: Tests: add tests for export set error messages --- Tests/RunCMake/export/DependOnDoubleExport-result.txt | 1 + Tests/RunCMake/export/DependOnDoubleExport-stderr.txt | 13 +++++++++++++ Tests/RunCMake/export/DependOnDoubleExport.cmake | 7 +++++++ Tests/RunCMake/export/DependOnNotExport-result.txt | 1 + Tests/RunCMake/export/DependOnNotExport-stderr.txt | 6 ++++++ Tests/RunCMake/export/DependOnNotExport.cmake | 4 ++++ Tests/RunCMake/export/RunCMakeTest.cmake | 2 ++ 7 files changed, 34 insertions(+) create mode 100644 Tests/RunCMake/export/DependOnDoubleExport-result.txt create mode 100644 Tests/RunCMake/export/DependOnDoubleExport-stderr.txt create mode 100644 Tests/RunCMake/export/DependOnDoubleExport.cmake create mode 100644 Tests/RunCMake/export/DependOnNotExport-result.txt create mode 100644 Tests/RunCMake/export/DependOnNotExport-stderr.txt create mode 100644 Tests/RunCMake/export/DependOnNotExport.cmake diff --git a/Tests/RunCMake/export/DependOnDoubleExport-result.txt b/Tests/RunCMake/export/DependOnDoubleExport-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/export/DependOnDoubleExport-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt b/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt new file mode 100644 index 0000000..b78c7e4 --- /dev/null +++ b/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt @@ -0,0 +1,13 @@ +CMake Error in CMakeLists.txt: + export called with target "exported" which requires target "doubleexported" + that is not in this export set, but in multiple other export sets: + .*/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. + + +CMake Generate step failed. Build files cannot be regenerated correctly. diff --git a/Tests/RunCMake/export/DependOnDoubleExport.cmake b/Tests/RunCMake/export/DependOnDoubleExport.cmake new file mode 100644 index 0000000..8d108d7 --- /dev/null +++ b/Tests/RunCMake/export/DependOnDoubleExport.cmake @@ -0,0 +1,7 @@ +add_library(doubleexported INTERFACE) +install(TARGETS doubleexported EXPORT exportset) +export(TARGETS doubleexported FILE "${CMAKE_CURRENT_BINARY_DIR}/manual.cmake") +export(EXPORT exportset FILE "${CMAKE_CURRENT_BINARY_DIR}/exportset.cmake") +add_library(exported INTERFACE) +target_link_libraries(exported INTERFACE doubleexported) +export(TARGETS exported FILE "${CMAKE_CURRENT_BINARY_DIR}/exports.cmake") diff --git a/Tests/RunCMake/export/DependOnNotExport-result.txt b/Tests/RunCMake/export/DependOnNotExport-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/export/DependOnNotExport-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/export/DependOnNotExport-stderr.txt b/Tests/RunCMake/export/DependOnNotExport-stderr.txt new file mode 100644 index 0000000..80f5758 --- /dev/null +++ b/Tests/RunCMake/export/DependOnNotExport-stderr.txt @@ -0,0 +1,6 @@ +CMake Error in CMakeLists.txt: + export called with target "exported" which requires target "notexported" + that is not in any export set. + + +CMake Generate step failed. Build files cannot be regenerated correctly. diff --git a/Tests/RunCMake/export/DependOnNotExport.cmake b/Tests/RunCMake/export/DependOnNotExport.cmake new file mode 100644 index 0000000..06c1ad9 --- /dev/null +++ b/Tests/RunCMake/export/DependOnNotExport.cmake @@ -0,0 +1,4 @@ +add_library(notexported INTERFACE) +add_library(exported INTERFACE) +target_link_libraries(exported INTERFACE notexported) +export(TARGETS exported FILE "${CMAKE_CURRENT_BINARY_DIR}/exports.cmake") diff --git a/Tests/RunCMake/export/RunCMakeTest.cmake b/Tests/RunCMake/export/RunCMakeTest.cmake index 97a0ca6..4d2f217 100644 --- a/Tests/RunCMake/export/RunCMakeTest.cmake +++ b/Tests/RunCMake/export/RunCMakeTest.cmake @@ -10,3 +10,5 @@ run_cmake(ForbiddenToExportInterfaceProperties) run_cmake(ForbiddenToExportImportedProperties) run_cmake(ForbiddenToExportPropertyWithGenExp) run_cmake(ExportPropertiesUndefined) +run_cmake(DependOnNotExport) +run_cmake(DependOnDoubleExport) -- cgit v0.12