From a2be068c75890a9b6723c0bbb2d32a108cb84eed Mon Sep 17 00:00:00 2001 From: Alex Neundorf Date: Sat, 12 Nov 2011 18:12:07 +0100 Subject: [PATCH 1/2] install(EXPORT): Enforce existence of imported target files Typical Config.cmake files for find_package() rely only on the files generated by install(EXPORT). They might be wrong, for whatever reasons, like people manually deleted files, projects were packaged wrong by distributions, whatever. To protect against this, add checks that the file locations we are importing actually exist on disk. Alex --- Source/cmExportFileGenerator.cxx | 66 +++++++++++++++++++++++++ Source/cmExportFileGenerator.h | 5 ++ Source/cmExportInstallFileGenerator.cxx | 30 +++++++---- Source/cmExportInstallFileGenerator.h | 4 +- 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index 9e5c91efc..777737323 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -368,3 +368,69 @@ cmExportFileGenerator os << " )\n" << "\n"; } + + +//---------------------------------------------------------------------------- +void +cmExportFileGenerator::GenerateImportedFileCheckLoop(std::ostream& os) +{ + // Add code which verifies at cmake time that the file which is being + // imported actually exists on disk. This should in theory always be theory + // case, but still when packages are split into normal and development + // packages this might get broken (e.g. the Config.cmake could be part of + // the non-development package, something similar happened to me without + // on SUSE with a mysql pkg-config file, which claimed everything is fine, + // but the development package was not installed.). + os << "# Loop over all imported files and verify that they actually exist\n" + "FOREACH(target ${_IMPORT_CHECK_TARGETS} )\n" + " FOREACH(file ${_IMPORT_CHECK_FILES_FOR_${target}} )\n" + " IF(NOT EXISTS \"${file}\" )\n" + " MESSAGE(FATAL_ERROR \"The imported target \\\"${target}\\\" " + "references the file \\\"${file}\\\", but this file does not exist. " + "There are multiple possible reasons:\n" + " * The file \\\"${file}\\\" has been manually " + "deleted, renamed or moved to another location.\n" + " * A previous install or uninstall procedure did not complete " + " successfully.\n" + " * The installation package was faulty, and contained\n" + "\\\"${CMAKE_CURRENT_LIST_FILE}\\\"\n" + "but not\n" + "\\\"${file}\\\"\n" + "which must always be installed together.\\n\"\n" + " )\n" + " ENDIF()\n" + " ENDFOREACH()\n" + " UNSET(_IMPORT_CHECK_FILES_FOR_${target})\n" + "ENDFOREACH()\n" + "UNSET(_IMPORT_CHECK_TARGETS)\n" + "\n"; +} + + +//---------------------------------------------------------------------------- +void +cmExportFileGenerator +::GenerateImportedFileChecksCode(std::ostream& os, cmTarget* target, + ImportPropertyMap const& properties, + const std::set& importedLocations) +{ + // Construct the imported target name. + std::string targetName = this->Namespace; + targetName += target->GetName(); + + os << "LIST(APPEND _IMPORT_CHECK_TARGETS " << targetName << " )\n" + "LIST(APPEND _IMPORT_CHECK_FILES_FOR_" << targetName << " "; + + for(std::set::const_iterator li = importedLocations.begin(); + li != importedLocations.end(); + ++li) + { + ImportPropertyMap::const_iterator pi = properties.find(*li); + if (pi != properties.end()) + { + os << "\"" << pi->second << "\" "; + } + } + + os << ")\n\n"; +} diff --git a/Source/cmExportFileGenerator.h b/Source/cmExportFileGenerator.h index 05f73a247..f271e5570 100644 --- a/Source/cmExportFileGenerator.h +++ b/Source/cmExportFileGenerator.h @@ -56,6 +56,11 @@ protected: void GenerateImportPropertyCode(std::ostream& os, const char* config, cmTarget* target, ImportPropertyMap const& properties); + void GenerateImportedFileChecksCode(std::ostream& os, cmTarget* target, + ImportPropertyMap const& properties, + const std::set& importedLocations); + void GenerateImportedFileCheckLoop(std::ostream& os); + // Collect properties with detailed information about targets beyond // their location on disk. diff --git a/Source/cmExportInstallFileGenerator.cxx b/Source/cmExportInstallFileGenerator.cxx index 23ff5fb12..da14dd787 100644 --- a/Source/cmExportInstallFileGenerator.cxx +++ b/Source/cmExportInstallFileGenerator.cxx @@ -167,16 +167,18 @@ cmExportInstallFileGenerator // Collect import properties for this target. cmTargetExport* te = *tei; ImportPropertyMap properties; + std::set importedLocations; + this->SetImportLocationProperty(config, suffix, te->ArchiveGenerator, + properties, importedLocations); + this->SetImportLocationProperty(config, suffix, te->LibraryGenerator, + properties, importedLocations); this->SetImportLocationProperty(config, suffix, - te->ArchiveGenerator, properties); - this->SetImportLocationProperty(config, suffix, - te->LibraryGenerator, properties); - this->SetImportLocationProperty(config, suffix, - te->RuntimeGenerator, properties); - this->SetImportLocationProperty(config, suffix, - te->FrameworkGenerator, properties); - this->SetImportLocationProperty(config, suffix, - te->BundleGenerator, properties); + te->RuntimeGenerator, properties, + importedLocations); + this->SetImportLocationProperty(config, suffix, te->FrameworkGenerator, + properties, importedLocations); + this->SetImportLocationProperty(config, suffix, te->BundleGenerator, + properties, importedLocations); // If any file location was set for the target add it to the // import file. @@ -194,9 +196,13 @@ cmExportInstallFileGenerator // Generate code in the export file. this->GenerateImportPropertyCode(os, config, te->Target, properties); + this->GenerateImportedFileChecksCode(os, te->Target, properties, + importedLocations); } } + this->GenerateImportedFileCheckLoop(os); + // Cleanup the import prefix variable. if(!this->ImportPrefix.empty()) { @@ -211,7 +217,9 @@ void cmExportInstallFileGenerator ::SetImportLocationProperty(const char* config, std::string const& suffix, cmInstallTargetGenerator* itgen, - ImportPropertyMap& properties) + ImportPropertyMap& properties, + std::set& importedLocations + ) { // Skip rules that do not match this configuration. if(!(itgen && itgen->InstallsForConfig(config))) @@ -249,6 +257,7 @@ cmExportInstallFileGenerator // Store the property. properties[prop] = value; + importedLocations.insert(prop); } else { @@ -291,6 +300,7 @@ cmExportInstallFileGenerator // Store the property. properties[prop] = value; + importedLocations.insert(prop); } } diff --git a/Source/cmExportInstallFileGenerator.h b/Source/cmExportInstallFileGenerator.h index 8c8fb4445..fb678e891 100644 --- a/Source/cmExportInstallFileGenerator.h +++ b/Source/cmExportInstallFileGenerator.h @@ -75,7 +75,9 @@ protected: void SetImportLocationProperty(const char* config, std::string const& suffix, cmInstallTargetGenerator* itgen, - ImportPropertyMap& properties); + ImportPropertyMap& properties, + std::set& importedLocations + ); void ComplainAboutImportPrefix(cmInstallTargetGenerator* itgen); From 5c03438661c285a7cc5f1e691198ac4e8a68a8f6 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 17 Nov 2011 16:39:24 -0500 Subject: [PATCH 2/2] install(EXPORT): Improve target import failure message format --- Source/cmExportFileGenerator.cxx | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index 777737323..c4f5dfb50 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -385,19 +385,16 @@ cmExportFileGenerator::GenerateImportedFileCheckLoop(std::ostream& os) "FOREACH(target ${_IMPORT_CHECK_TARGETS} )\n" " FOREACH(file ${_IMPORT_CHECK_FILES_FOR_${target}} )\n" " IF(NOT EXISTS \"${file}\" )\n" - " MESSAGE(FATAL_ERROR \"The imported target \\\"${target}\\\" " - "references the file \\\"${file}\\\", but this file does not exist. " - "There are multiple possible reasons:\n" - " * The file \\\"${file}\\\" has been manually " - "deleted, renamed or moved to another location.\n" - " * A previous install or uninstall procedure did not complete " - " successfully.\n" - " * The installation package was faulty, and contained\n" - "\\\"${CMAKE_CURRENT_LIST_FILE}\\\"\n" - "but not\n" - "\\\"${file}\\\"\n" - "which must always be installed together.\\n\"\n" - " )\n" + " MESSAGE(FATAL_ERROR \"The imported target \\\"${target}\\\"" + " references the file\n" + " \\\"${file}\\\"\n" + "but this file does not exist. Possible reasons include:\n" + "* The file was deleted, renamed, or moved to another location.\n" + "* An install or uninstall procedure did not complete successfully.\n" + "* The installation package was faulty and contained\n" + " \\\"${CMAKE_CURRENT_LIST_FILE}\\\"\n" + "but not all the files it references.\n" + "\")\n" " ENDIF()\n" " ENDFOREACH()\n" " UNSET(_IMPORT_CHECK_FILES_FOR_${target})\n"