From af81a3c31b206633742eb13d41c54a9bc807ffea Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 24 Mar 2013 21:18:17 +0100 Subject: [PATCH 1/2] install(EXPORT): Ensure clean INTERFACE_INCLUDE_DIRECTORIES Check that source and binary directories are not part of the INTERFACE_INCLUDE_DIRECTORIES for installed IMPORTED targets. This is limited to directories which do not contain generator expressions to evaluate. Such paths can only be checked at time of use of the imported target, which will be done in a follow up patch. --- Source/cmExportFileGenerator.cxx | 111 ++++++++++++++++++ Source/cmExportFileGenerator.h | 5 + Source/cmExportInstallFileGenerator.cxx | 3 +- Tests/ExportImport/Export/CMakeLists.txt | 18 ++- .../BinaryDirectoryInInterface-result.txt | 1 + .../BinaryDirectoryInInterface-stderr.txt | 6 + .../BinaryDirectoryInInterface.cmake | 11 ++ .../RelativePathInInterface-result.txt | 1 + .../RelativePathInInterface-stderr.txt | 5 + .../RelativePathInInterface.cmake | 11 ++ .../include_directories/RunCMakeTest.cmake | 3 + .../SourceDirectoryInInterface-result.txt | 1 + .../SourceDirectoryInInterface-stderr.txt | 6 + .../SourceDirectoryInInterface.cmake | 11 ++ Tests/RunCMake/include_directories/empty.cpp | 0 15 files changed, 189 insertions(+), 4 deletions(-) create mode 100644 Tests/RunCMake/include_directories/BinaryDirectoryInInterface-result.txt create mode 100644 Tests/RunCMake/include_directories/BinaryDirectoryInInterface-stderr.txt create mode 100644 Tests/RunCMake/include_directories/BinaryDirectoryInInterface.cmake create mode 100644 Tests/RunCMake/include_directories/RelativePathInInterface-result.txt create mode 100644 Tests/RunCMake/include_directories/RelativePathInInterface-stderr.txt create mode 100644 Tests/RunCMake/include_directories/RelativePathInInterface.cmake create mode 100644 Tests/RunCMake/include_directories/SourceDirectoryInInterface-result.txt create mode 100644 Tests/RunCMake/include_directories/SourceDirectoryInInterface-stderr.txt create mode 100644 Tests/RunCMake/include_directories/SourceDirectoryInInterface.cmake create mode 100644 Tests/RunCMake/include_directories/empty.cpp diff --git a/Source/cmExportFileGenerator.cxx b/Source/cmExportFileGenerator.cxx index 7fd038004..27ec56beb 100644 --- a/Source/cmExportFileGenerator.cxx +++ b/Source/cmExportFileGenerator.cxx @@ -24,6 +24,7 @@ #include "cmComputeLinkInformation.h" #include +#include //---------------------------------------------------------------------------- cmExportFileGenerator::cmExportFileGenerator() @@ -167,6 +168,116 @@ void cmExportFileGenerator::PopulateInterfaceProperty(const char *propName, } } +//---------------------------------------------------------------------------- +static bool isSubDirectory(const char* a, const char* b) +{ + return (cmSystemTools::ComparePath(a, b) || + cmSystemTools::IsSubDirectory(a, b)); +} + +//---------------------------------------------------------------------------- +static bool checkInterfaceDirs(const std::string &prepro, + cmTarget *target) +{ + const char* installDir = + target->GetMakefile()->GetSafeDefinition("CMAKE_INSTALL_PREFIX"); + const char* topSourceDir = target->GetMakefile()->GetHomeDirectory(); + const char* topBinaryDir = target->GetMakefile()->GetHomeOutputDirectory(); + + std::vector parts; + cmGeneratorExpression::Split(prepro, parts); + + const bool inSourceBuild = strcmp(topSourceDir, topBinaryDir) == 0; + + for(std::vector::iterator li = parts.begin(); + li != parts.end(); ++li) + { + if (cmGeneratorExpression::Find(*li) != std::string::npos) + { + continue; + } + if (strncmp(li->c_str(), "${_IMPORT_PREFIX}", 17) == 0) + { + continue; + } + if (!cmSystemTools::FileIsFullPath(li->c_str())) + { + cmOStringStream e; + e << "Target \"" << target->GetName() << "\" " + "INTERFACE_INCLUDE_DIRECTORIES property contains relative path:\n" + " \"" << *li << "\""; + target->GetMakefile()->IssueMessage(cmake::FATAL_ERROR, + e.str().c_str()); + return false; + } + if (isSubDirectory(li->c_str(), installDir)) + { + continue; + } + if (isSubDirectory(li->c_str(), topBinaryDir)) + { + cmOStringStream e; + e << "Target \"" << target->GetName() << "\" " + "INTERFACE_INCLUDE_DIRECTORIES property contains path:\n" + " \"" << *li << "\"\nwhich is prefixed in the build directory."; + target->GetMakefile()->IssueMessage(cmake::FATAL_ERROR, + e.str().c_str()); + return false; + } + if (!inSourceBuild) + { + if (isSubDirectory(li->c_str(), topSourceDir)) + { + cmOStringStream e; + e << "Target \"" << target->GetName() << "\" " + "INTERFACE_INCLUDE_DIRECTORIES property contains path:\n" + " \"" << *li << "\"\nwhich is prefixed in the source directory."; + target->GetMakefile()->IssueMessage(cmake::FATAL_ERROR, + e.str().c_str()); + return false; + } + } + } + return true; +} + +//---------------------------------------------------------------------------- +void cmExportFileGenerator::PopulateIncludeDirectoriesInterface( + cmTarget *target, + cmGeneratorExpression::PreprocessContext preprocessRule, + ImportPropertyMap &properties, + std::vector &missingTargets) +{ + assert(preprocessRule == cmGeneratorExpression::InstallInterface); + + const char *propName = "INTERFACE_INCLUDE_DIRECTORIES"; + const char *input = target->GetProperty(propName); + if (!input) + { + return; + } + if (!*input) + { + // Set to empty + properties[propName] = ""; + return; + } + + std::string prepro = cmGeneratorExpression::Preprocess(input, + preprocessRule); + if (!prepro.empty()) + { + this->ResolveTargetsInGeneratorExpressions(prepro, target, + missingTargets); + + if (!checkInterfaceDirs(prepro, target)) + { + return; + } + properties[propName] = prepro; + } +} + //---------------------------------------------------------------------------- void cmExportFileGenerator::PopulateInterfaceProperty(const char *propName, cmTarget *target, diff --git a/Source/cmExportFileGenerator.h b/Source/cmExportFileGenerator.h index 776be614d..9f958a234 100644 --- a/Source/cmExportFileGenerator.h +++ b/Source/cmExportFileGenerator.h @@ -107,6 +107,11 @@ protected: ImportPropertyMap &properties); void GenerateInterfaceProperties(cmTarget *target, std::ostream& os, const ImportPropertyMap &properties); + void PopulateIncludeDirectoriesInterface( + cmTarget *target, + cmGeneratorExpression::PreprocessContext preprocessRule, + ImportPropertyMap &properties, + std::vector &missingTargets); void SetImportLinkInterface(const char* config, std::string const& suffix, cmGeneratorExpression::PreprocessContext preprocessRule, diff --git a/Source/cmExportInstallFileGenerator.cxx b/Source/cmExportInstallFileGenerator.cxx index 8b8b8465f..746b0c836 100644 --- a/Source/cmExportInstallFileGenerator.cxx +++ b/Source/cmExportInstallFileGenerator.cxx @@ -120,8 +120,7 @@ bool cmExportInstallFileGenerator::GenerateMainFile(std::ostream& os) ImportPropertyMap properties; - this->PopulateInterfaceProperty("INTERFACE_INCLUDE_DIRECTORIES", - te, + this->PopulateIncludeDirectoriesInterface(te, cmGeneratorExpression::InstallInterface, properties, missingTargets); this->PopulateInterfaceProperty("INTERFACE_COMPILE_DEFINITIONS", diff --git a/Tests/ExportImport/Export/CMakeLists.txt b/Tests/ExportImport/Export/CMakeLists.txt index cdf67c2a7..be48483da 100644 --- a/Tests/ExportImport/Export/CMakeLists.txt +++ b/Tests/ExportImport/Export/CMakeLists.txt @@ -174,9 +174,14 @@ set_property(TARGET testSharedLibRequired set_property(TARGET testSharedLibRequired APPEND PROPERTY INCLUDE_DIRECTORIES "${CMAKE_CURRENT_BINARY_DIR}" ) +install(FILES + "${CMAKE_CURRENT_SOURCE_DIR}/testSharedLibRequired.h" + "${CMAKE_CURRENT_BINARY_DIR}/testsharedlibrequired_export.h" + DESTINATION include/testSharedLibRequired +) set_property(TARGET testSharedLibRequired APPEND PROPERTY - INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_BINARY_DIR}" - "${CMAKE_CURRENT_SOURCE_DIR}" + INTERFACE_INCLUDE_DIRECTORIES "$/include/testSharedLibRequired>" + "$" ) set_property(TARGET testSharedLibRequired APPEND PROPERTY @@ -205,6 +210,15 @@ set_property(TARGET testSharedLibDepends APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES $ ) +install(FILES + "${CMAKE_CURRENT_SOURCE_DIR}/testSharedLibDepends.h" + "${CMAKE_CURRENT_BINARY_DIR}/testsharedlibdepends_export.h" + DESTINATION include/testSharedLibDepends +) +set_property(TARGET testSharedLibDepends APPEND PROPERTY + INTERFACE_INCLUDE_DIRECTORIES "$/include/testSharedLibDepends>" + "$" +) set_property(TARGET testSharedLibDepends APPEND PROPERTY LINK_INTERFACE_LIBRARIES $<$,EXECUTABLE>:$> diff --git a/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-result.txt b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-stderr.txt b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-stderr.txt new file mode 100644 index 000000000..0d4379eb0 --- /dev/null +++ b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface-stderr.txt @@ -0,0 +1,6 @@ +CMake Error in CMakeLists.txt: + Target "testTarget" INTERFACE_INCLUDE_DIRECTORIES property contains path: + + ".*RunCMake/include_directories/BinaryDirectoryInInterface-build/foo" + + which is prefixed in the build directory. diff --git a/Tests/RunCMake/include_directories/BinaryDirectoryInInterface.cmake b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface.cmake new file mode 100644 index 000000000..875454029 --- /dev/null +++ b/Tests/RunCMake/include_directories/BinaryDirectoryInInterface.cmake @@ -0,0 +1,11 @@ + +project(BinaryDirectoryInInterface) + +add_library(testTarget "${CMAKE_CURRENT_SOURCE_DIR}/empty.cpp") +target_include_directories(testTarget INTERFACE "${CMAKE_CURRENT_BINARY_DIR}/foo") + +install(TARGETS testTarget EXPORT testTargets + DESTINATION lib +) + +install(EXPORT testTargets DESTINATION lib/cmake) diff --git a/Tests/RunCMake/include_directories/RelativePathInInterface-result.txt b/Tests/RunCMake/include_directories/RelativePathInInterface-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/include_directories/RelativePathInInterface-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/include_directories/RelativePathInInterface-stderr.txt b/Tests/RunCMake/include_directories/RelativePathInInterface-stderr.txt new file mode 100644 index 000000000..f6cdb532e --- /dev/null +++ b/Tests/RunCMake/include_directories/RelativePathInInterface-stderr.txt @@ -0,0 +1,5 @@ +CMake Error in CMakeLists.txt: + Target "testTarget" INTERFACE_INCLUDE_DIRECTORIES property contains + relative path: + + "foo" diff --git a/Tests/RunCMake/include_directories/RelativePathInInterface.cmake b/Tests/RunCMake/include_directories/RelativePathInInterface.cmake new file mode 100644 index 000000000..f2ce54ae2 --- /dev/null +++ b/Tests/RunCMake/include_directories/RelativePathInInterface.cmake @@ -0,0 +1,11 @@ + +project(RelativePathInInterface) + +add_library(testTarget "${CMAKE_CURRENT_SOURCE_DIR}/empty.cpp") +set_property(TARGET testTarget PROPERTY INTERFACE_INCLUDE_DIRECTORIES "foo") + +install(TARGETS testTarget EXPORT testTargets + DESTINATION lib +) + +install(EXPORT testTargets DESTINATION lib/cmake) diff --git a/Tests/RunCMake/include_directories/RunCMakeTest.cmake b/Tests/RunCMake/include_directories/RunCMakeTest.cmake index ddf268c1e..bd299fc9a 100644 --- a/Tests/RunCMake/include_directories/RunCMakeTest.cmake +++ b/Tests/RunCMake/include_directories/RunCMakeTest.cmake @@ -3,3 +3,6 @@ include(RunCMake) run_cmake(NotFoundContent) run_cmake(DebugIncludes) run_cmake(TID-bad-target) +run_cmake(SourceDirectoryInInterface) +run_cmake(BinaryDirectoryInInterface) +run_cmake(RelativePathInInterface) diff --git a/Tests/RunCMake/include_directories/SourceDirectoryInInterface-result.txt b/Tests/RunCMake/include_directories/SourceDirectoryInInterface-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/include_directories/SourceDirectoryInInterface-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/include_directories/SourceDirectoryInInterface-stderr.txt b/Tests/RunCMake/include_directories/SourceDirectoryInInterface-stderr.txt new file mode 100644 index 000000000..9346b994f --- /dev/null +++ b/Tests/RunCMake/include_directories/SourceDirectoryInInterface-stderr.txt @@ -0,0 +1,6 @@ +CMake Error in CMakeLists.txt: + Target "testTarget" INTERFACE_INCLUDE_DIRECTORIES property contains path: + + ".*RunCMake/include_directories/foo" + + which is prefixed in the source directory. diff --git a/Tests/RunCMake/include_directories/SourceDirectoryInInterface.cmake b/Tests/RunCMake/include_directories/SourceDirectoryInInterface.cmake new file mode 100644 index 000000000..c9a9c457b --- /dev/null +++ b/Tests/RunCMake/include_directories/SourceDirectoryInInterface.cmake @@ -0,0 +1,11 @@ + +project(SourceDirectoryInInterface) + +add_library(testTarget "${CMAKE_CURRENT_SOURCE_DIR}/empty.cpp") +target_include_directories(testTarget INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/foo") + +install(TARGETS testTarget EXPORT testTargets + DESTINATION lib +) + +install(EXPORT testTargets DESTINATION lib/cmake) diff --git a/Tests/RunCMake/include_directories/empty.cpp b/Tests/RunCMake/include_directories/empty.cpp new file mode 100644 index 000000000..e69de29bb From 28051f1150923804796b4e63e41f6906308788e0 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 24 Mar 2013 21:18:21 +0100 Subject: [PATCH 2/2] Report an error on IMPORTED targets with a faulty INTERFACE It is considered an error if the INTERFACE_INCLUDE_DIRECTORIES contains a directory which does not exist, which indicates a programmer error by the upstream, or a packaging error. One of the RunCMake.CompatibleInterface tests also needs to be updated due to this change. Non-existant includes were used in the test, but are not needed. --- Source/cmTarget.cxx | 31 +++++++++++++++++-- .../InterfaceString-builtin-prop.cmake | 2 -- .../ImportedTarget-result.txt | 1 + .../ImportedTarget-stderr.txt | 13 ++++++++ .../include_directories/ImportedTarget.cmake | 9 ++++++ .../include_directories/RunCMakeTest.cmake | 1 + 6 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 Tests/RunCMake/include_directories/ImportedTarget-result.txt create mode 100644 Tests/RunCMake/include_directories/ImportedTarget-stderr.txt create mode 100644 Tests/RunCMake/include_directories/ImportedTarget.cmake diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 61d4ce201..b4fdcd54f 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -131,11 +131,13 @@ public: SourceEntriesType SourceEntries; struct IncludeDirectoriesEntry { - IncludeDirectoriesEntry(cmsys::auto_ptr cge) - : ge(cge) + IncludeDirectoriesEntry(cmsys::auto_ptr cge, + const std::string &targetName = std::string()) + : ge(cge), TargetName(targetName) {} const cmsys::auto_ptr ge; std::vector CachedIncludes; + const std::string TargetName; }; std::vector IncludeDirectoriesEntries; std::vector LinkInterfaceIncludeDirectoriesEntries; @@ -2818,6 +2820,28 @@ static void processIncludeDirectories(cmTarget *tgt, for(std::vector::iterator li = entryIncludes.begin(); li != entryIncludes.end(); ++li) { + cmTarget *dependentTarget = + mf->FindTargetToUse((*it)->TargetName.c_str()); + + const bool fromImported = dependentTarget + && dependentTarget->IsImported(); + + if (fromImported && !cmSystemTools::FileExists(li->c_str())) + { + cmOStringStream e; + e << "Imported target \"" << (*it)->TargetName << "\" includes " + "non-existent path\n \"" << *li << "\"\nin its " + "INTERFACE_INCLUDE_DIRECTORIES. Possible reasons include:\n" + "* The path 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 references files it " + "does not provide.\n"; + tgt->GetMakefile()->IssueMessage(cmake::FATAL_ERROR, e.str().c_str()); + return; + } + if (testIsOff && !cmSystemTools::IsOff(li->c_str())) { cmSystemTools::ConvertToUnixSlashes(*li); @@ -2913,7 +2937,8 @@ std::vector cmTarget::GetIncludeDirectories(const char *config) it->Value + ",INTERFACE_INCLUDE_DIRECTORIES>"); this->Internal->CachedLinkInterfaceIncludeDirectoriesEntries.push_back( - new cmTargetInternals::IncludeDirectoriesEntry(cge)); + new cmTargetInternals::IncludeDirectoriesEntry(cge, + it->Value)); } } diff --git a/Tests/RunCMake/CompatibleInterface/InterfaceString-builtin-prop.cmake b/Tests/RunCMake/CompatibleInterface/InterfaceString-builtin-prop.cmake index 5221a129c..5772856cc 100644 --- a/Tests/RunCMake/CompatibleInterface/InterfaceString-builtin-prop.cmake +++ b/Tests/RunCMake/CompatibleInterface/InterfaceString-builtin-prop.cmake @@ -3,8 +3,6 @@ add_library(foo UNKNOWN IMPORTED) add_library(bar UNKNOWN IMPORTED) set_property(TARGET foo APPEND PROPERTY COMPATIBLE_INTERFACE_STRING INCLUDE_DIRECTORIES) -set_property(TARGET foo PROPERTY INTERFACE_INCLUDE_DIRECTORIES foo_inc) -set_property(TARGET bar PROPERTY INTERFACE_INCLUDE_DIRECTORIES bar_inc) add_executable(user main.cpp) set_property(TARGET user PROPERTY INCLUDE_DIRECTORIES bar_inc) diff --git a/Tests/RunCMake/include_directories/ImportedTarget-result.txt b/Tests/RunCMake/include_directories/ImportedTarget-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/include_directories/ImportedTarget-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/include_directories/ImportedTarget-stderr.txt b/Tests/RunCMake/include_directories/ImportedTarget-stderr.txt new file mode 100644 index 000000000..da2605297 --- /dev/null +++ b/Tests/RunCMake/include_directories/ImportedTarget-stderr.txt @@ -0,0 +1,13 @@ +CMake Error in CMakeLists.txt: + Imported target "imported" includes non-existent path + + "/does/not/exist" + + in its INTERFACE_INCLUDE_DIRECTORIES. Possible reasons include: + + \* The path was deleted, renamed, or moved to another location. + + \* An install or uninstall procedure did not complete successfully. + + \* The installation package was faulty and references files it does not + provide. diff --git a/Tests/RunCMake/include_directories/ImportedTarget.cmake b/Tests/RunCMake/include_directories/ImportedTarget.cmake new file mode 100644 index 000000000..e1a20b130 --- /dev/null +++ b/Tests/RunCMake/include_directories/ImportedTarget.cmake @@ -0,0 +1,9 @@ + +project(ImportedTarget) + +add_library(testTarget "${CMAKE_CURRENT_SOURCE_DIR}/empty.cpp") + +add_library(imported UNKNOWN IMPORTED) +set_property(TARGET imported PROPERTY INTERFACE_INCLUDE_DIRECTORIES "/does/not/exist") + +target_link_libraries(testTarget imported) diff --git a/Tests/RunCMake/include_directories/RunCMakeTest.cmake b/Tests/RunCMake/include_directories/RunCMakeTest.cmake index bd299fc9a..1caae5cfb 100644 --- a/Tests/RunCMake/include_directories/RunCMakeTest.cmake +++ b/Tests/RunCMake/include_directories/RunCMakeTest.cmake @@ -6,3 +6,4 @@ run_cmake(TID-bad-target) run_cmake(SourceDirectoryInInterface) run_cmake(BinaryDirectoryInInterface) run_cmake(RelativePathInInterface) +run_cmake(ImportedTarget)