From c8997fc046178b91090b85fd6a6371d0f0ef9e47 Mon Sep 17 00:00:00 2001 From: Martin Duffy Date: Thu, 26 Dec 2024 09:35:19 -0500 Subject: [PATCH] export: Allow depending on targets exported multiple times Fixes: #26556 --- Source/cmExportBuildFileGenerator.cxx | 39 +++++++++++------- Source/cmExportBuildFileGenerator.h | 6 +-- Source/cmExportFileGenerator.h | 3 +- Source/cmExportInstallFileGenerator.cxx | 41 +++++++++++-------- Source/cmExportInstallFileGenerator.h | 6 +-- Source/cmExportPackageInfoGenerator.cxx | 8 ++-- Source/cmExportTryCompileFileGenerator.h | 2 +- .../PackageInfo/DependsMultiple.cmake | 3 ++ .../PackageInfo/DependsMultipleCommon.cmake | 11 +++++ ...pendsMultipleDifferentNamespace-result.txt | 1 + ...pendsMultipleDifferentNamespace-stderr.txt | 19 +++++++++ .../DependsMultipleDifferentNamespace.cmake | 3 ++ .../DependsMultipleDifferentSets-result.txt | 1 + .../DependsMultipleDifferentSets-stderr.txt | 18 ++++++++ .../DependsMultipleDifferentSets.cmake | 5 +++ Tests/RunCMake/PackageInfo/RunCMakeTest.cmake | 3 ++ .../export/DependOnDoubleExport-stderr.txt | 7 ++-- 17 files changed, 130 insertions(+), 46 deletions(-) create mode 100644 Tests/RunCMake/PackageInfo/DependsMultiple.cmake create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleCommon.cmake create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-result.txt create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-stderr.txt create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace.cmake create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-result.txt create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-stderr.txt create mode 100644 Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets.cmake diff --git a/Source/cmExportBuildFileGenerator.cxx b/Source/cmExportBuildFileGenerator.cxx index 7bd60e8cb9..35f45b0439 100644 --- a/Source/cmExportBuildFileGenerator.cxx +++ b/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 exportFiles; - std::string ns; + std::set exportSets; + std::set 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 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 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."; } diff --git a/Source/cmExportBuildFileGenerator.h b/Source/cmExportBuildFileGenerator.h index dfd0416191..dcfd75732d 100644 --- a/Source/cmExportBuildFileGenerator.h +++ b/Source/cmExportBuildFileGenerator.h @@ -79,9 +79,9 @@ protected: cmGeneratorTarget const* depender, cmGeneratorTarget* dependee) override; - void ComplainAboutMissingTarget( - cmGeneratorTarget const* depender, cmGeneratorTarget const* dependee, - std::vector const& exportFiles) const; + void ComplainAboutMissingTarget(cmGeneratorTarget const* depender, + cmGeneratorTarget const* dependee, + ExportInfo const& exportInfo) const; void ComplainAboutDuplicateTarget( std::string const& targetName) const override; diff --git a/Source/cmExportFileGenerator.h b/Source/cmExportFileGenerator.h index accf14c29b..0daa016c8c 100644 --- a/Source/cmExportFileGenerator.h +++ b/Source/cmExportFileGenerator.h @@ -135,7 +135,8 @@ protected: struct ExportInfo { std::vector Files; - std::string Namespace; + std::set Sets; + std::set Namespaces; }; /** Find the set of export files and the unique namespace (if any) for a diff --git a/Source/cmExportInstallFileGenerator.cxx b/Source/cmExportInstallFileGenerator.cxx index e42dea8a21..b89c55383e 100644 --- a/Source/cmExportInstallFileGenerator.cxx +++ b/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 exportFiles; - std::string ns; + std::set exportSets; + std::set 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 const& te) { return te->TargetName == name; })) { + exportSets.insert(exp.first); std::vector 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 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()); diff --git a/Source/cmExportInstallFileGenerator.h b/Source/cmExportInstallFileGenerator.h index f33ecc112b..8ef0eacdef 100644 --- a/Source/cmExportInstallFileGenerator.h +++ b/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 const& exportFiles) const; + void ComplainAboutMissingTarget(cmGeneratorTarget const* depender, + cmGeneratorTarget const* dependee, + ExportInfo const& exportInfo) const; void ComplainAboutDuplicateTarget( std::string const& targetName) const override; diff --git a/Source/cmExportPackageInfoGenerator.cxx b/Source/cmExportPackageInfoGenerator.cxx index 034f14562c..79cfb14d62 100644 --- a/Source/cmExportPackageInfoGenerator.cxx +++ b/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; } diff --git a/Source/cmExportTryCompileFileGenerator.h b/Source/cmExportTryCompileFileGenerator.h index 58d1f5ee4f..20c331c58b 100644 --- a/Source/cmExportTryCompileFileGenerator.h +++ b/Source/cmExportTryCompileFileGenerator.h @@ -47,7 +47,7 @@ protected: ExportInfo FindExportInfo(cmGeneratorTarget const* /*target*/) const override { - return { {}, {} }; + return { {}, {}, {} }; } void PopulateProperties(cmGeneratorTarget const* target, diff --git a/Tests/RunCMake/PackageInfo/DependsMultiple.cmake b/Tests/RunCMake/PackageInfo/DependsMultiple.cmake new file mode 100644 index 0000000000..314c07430c --- /dev/null +++ b/Tests/RunCMake/PackageInfo/DependsMultiple.cmake @@ -0,0 +1,3 @@ +project(DependsMultiple CXX) +set(NAMESPACE foo::) +include(DependsMultipleCommon.cmake) diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleCommon.cmake b/Tests/RunCMake/PackageInfo/DependsMultipleCommon.cmake new file mode 100644 index 0000000000..388ce48d80 --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-result.txt b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-stderr.txt b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace-stderr.txt new file mode 100644 index 0000000000..cfaf949262 --- /dev/null +++ b/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. diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace.cmake b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace.cmake new file mode 100644 index 0000000000..a874757f2b --- /dev/null +++ b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentNamespace.cmake @@ -0,0 +1,3 @@ +project(DependsMultipleDifferentNamespace CXX) +set(NAMESPACE "") +include(DependsMultipleCommon.cmake) diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-result.txt b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-result.txt new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-stderr.txt b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets-stderr.txt new file mode 100644 index 0000000000..9c97a75229 --- /dev/null +++ b/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. diff --git a/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets.cmake b/Tests/RunCMake/PackageInfo/DependsMultipleDifferentSets.cmake new file mode 100644 index 0000000000..57bccbb86d --- /dev/null +++ b/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) diff --git a/Tests/RunCMake/PackageInfo/RunCMakeTest.cmake b/Tests/RunCMake/PackageInfo/RunCMakeTest.cmake index e90c371125..6618ff4ca0 100644 --- a/Tests/RunCMake/PackageInfo/RunCMakeTest.cmake +++ b/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) diff --git a/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt b/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt index a8849399ed..c79b74e1fe 100644 --- a/Tests/RunCMake/export/DependOnDoubleExport-stderr.txt +++ b/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.