From bb5905bb1342229c06cecee735322a8a28916b76 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 27 Nov 2014 01:04:33 +0100 Subject: cmTarget: Don't allow relative paths in INTERFACE_SOURCES Follow the pattern of checks that are made for INTERFACE_INCLUDE_DIRECTORIES. Existence is already checked by cmSourceFile::GetFullPath. Add a check to disallow relative paths in source directories. Otherwise code such as target_sources(lib1 INTERFACE foo.cpp) would fail if consumed by a target in a different directory. Unlike the INTERFACE_INCLUDE_DIRECTORIES behavior, we don't care whether the entry comes from an IMPORTED target or not. In the include directories case, the directory for a non-imported target might not exist yet but might be created. In the sources case, a file which does not yet exist in the filesystem must be explicitly marked with the GENERATED property. Adjust existing tests and add a new test for the error. --- Source/cmTarget.cxx | 26 +++++++++++++++++++--- Tests/ConfigSources/CMakeLists.txt | 6 ++--- Tests/RunCMake/TargetSources/OriginDebug.cmake | 2 +- .../RelativePathInInterface-result.txt | 1 + .../RelativePathInInterface-stderr.txt | 4 ++++ .../TargetSources/RelativePathInInterface.cmake | 6 +++++ Tests/RunCMake/TargetSources/RunCMakeTest.cmake | 1 + Tests/RunCMake/TargetSources/main.cpp | 5 +++++ Tests/SourcesProperty/CMakeLists.txt | 4 +++- 9 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 Tests/RunCMake/TargetSources/RelativePathInInterface-result.txt create mode 100644 Tests/RunCMake/TargetSources/RelativePathInInterface-stderr.txt create mode 100644 Tests/RunCMake/TargetSources/RelativePathInInterface.cmake create mode 100644 Tests/RunCMake/TargetSources/main.cpp diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index b476a27..ad1c83e 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -649,6 +649,8 @@ static bool processSources(cmTarget const* tgt, for (std::vector::const_iterator it = entries.begin(), end = entries.end(); it != end; ++it) { + cmLinkImplItem const& item = (*it)->LinkImplItem; + std::string const& targetName = item; std::vector entrySources; cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, config, @@ -667,11 +669,10 @@ static bool processSources(cmTarget const* tgt, i != entrySources.end(); ++i) { std::string& src = *i; - cmSourceFile* sf = mf->GetOrCreateSource(src); std::string e; - src = sf->GetFullPath(&e); - if(src.empty()) + std::string fullPath = sf->GetFullPath(&e); + if(fullPath.empty()) { if(!e.empty()) { @@ -681,6 +682,25 @@ static bool processSources(cmTarget const* tgt, } return contextDependent; } + + if (!targetName.empty() && !cmSystemTools::FileIsFullPath(src.c_str())) + { + cmOStringStream err; + if (!targetName.empty()) + { + err << "Target \"" << targetName << "\" contains relative " + "path in its INTERFACE_SOURCES:\n" + " \"" << src << "\""; + } + else + { + err << "Found relative path while evaluating sources of " + "\"" << tgt->GetName() << "\":\n \"" << src << "\"\n"; + } + tgt->GetMakefile()->IssueMessage(cmake::FATAL_ERROR, err.str()); + return contextDependent; + } + src = fullPath; } std::string usedSources; for(std::vector::iterator diff --git a/Tests/ConfigSources/CMakeLists.txt b/Tests/ConfigSources/CMakeLists.txt index c272257..748aad8 100644 --- a/Tests/ConfigSources/CMakeLists.txt +++ b/Tests/ConfigSources/CMakeLists.txt @@ -5,9 +5,9 @@ project(ConfigSources) add_library(iface INTERFACE) set_property(TARGET iface PROPERTY INTERFACE_SOURCES - iface_src.cpp - $<$:iface_debug_src.cpp> - $<$:does_not_exist.cpp> + "${CMAKE_CURRENT_SOURCE_DIR}/iface_src.cpp" + "$<$:${CMAKE_CURRENT_SOURCE_DIR}/iface_debug_src.cpp>" + "$<$:${CMAKE_CURRENT_SOURCE_DIR}/does_not_exist.cpp>" ) add_executable(ConfigSources diff --git a/Tests/RunCMake/TargetSources/OriginDebug.cmake b/Tests/RunCMake/TargetSources/OriginDebug.cmake index 5fe9ba7..d40a1d8 100644 --- a/Tests/RunCMake/TargetSources/OriginDebug.cmake +++ b/Tests/RunCMake/TargetSources/OriginDebug.cmake @@ -7,7 +7,7 @@ set(CMAKE_DEBUG_TARGET_PROPERTIES SOURCES) add_library(iface INTERFACE) set_property(TARGET iface PROPERTY INTERFACE_SOURCES - empty_1.cpp + "${CMAKE_CURRENT_SOURCE_DIR}/empty_1.cpp" ) add_library(OriginDebug empty_2.cpp) diff --git a/Tests/RunCMake/TargetSources/RelativePathInInterface-result.txt b/Tests/RunCMake/TargetSources/RelativePathInInterface-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetSources/RelativePathInInterface-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetSources/RelativePathInInterface-stderr.txt b/Tests/RunCMake/TargetSources/RelativePathInInterface-stderr.txt new file mode 100644 index 0000000..d47dd4d --- /dev/null +++ b/Tests/RunCMake/TargetSources/RelativePathInInterface-stderr.txt @@ -0,0 +1,4 @@ +CMake Error in CMakeLists.txt: + Target "iface" contains relative path in its INTERFACE_SOURCES: + + "empty_1.cpp" diff --git a/Tests/RunCMake/TargetSources/RelativePathInInterface.cmake b/Tests/RunCMake/TargetSources/RelativePathInInterface.cmake new file mode 100644 index 0000000..8bb6149 --- /dev/null +++ b/Tests/RunCMake/TargetSources/RelativePathInInterface.cmake @@ -0,0 +1,6 @@ + +add_library(iface INTERFACE) +target_sources(iface INTERFACE empty_1.cpp) + +add_executable(main main.cpp) +target_link_libraries(main iface) diff --git a/Tests/RunCMake/TargetSources/RunCMakeTest.cmake b/Tests/RunCMake/TargetSources/RunCMakeTest.cmake index 1d2eaec..c6d7f43 100644 --- a/Tests/RunCMake/TargetSources/RunCMakeTest.cmake +++ b/Tests/RunCMake/TargetSources/RunCMakeTest.cmake @@ -8,3 +8,4 @@ else() endif() run_cmake(CMP0026-LOCATION) +run_cmake(RelativePathInInterface) diff --git a/Tests/RunCMake/TargetSources/main.cpp b/Tests/RunCMake/TargetSources/main.cpp new file mode 100644 index 0000000..766b775 --- /dev/null +++ b/Tests/RunCMake/TargetSources/main.cpp @@ -0,0 +1,5 @@ + +int main() +{ + return 0; +} diff --git a/Tests/SourcesProperty/CMakeLists.txt b/Tests/SourcesProperty/CMakeLists.txt index 6c99e00..d1c35d8 100644 --- a/Tests/SourcesProperty/CMakeLists.txt +++ b/Tests/SourcesProperty/CMakeLists.txt @@ -4,7 +4,9 @@ cmake_minimum_required(VERSION 3.0) project(SourcesProperty) add_library(iface INTERFACE) -set_property(TARGET iface PROPERTY INTERFACE_SOURCES iface.cpp) +set_property(TARGET iface PROPERTY INTERFACE_SOURCES + "${CMAKE_CURRENT_SOURCE_DIR}/iface.cpp" +) add_executable(SourcesProperty main.cpp) target_link_libraries(SourcesProperty iface) -- cgit v0.12 From e1348056662b9ae0d399374b6726173d7bf0e9bb Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Thu, 27 Nov 2014 19:26:54 +0100 Subject: Export: Disallow export of targets with INTERFACE_SOURCES This can be allowed in the next release, but it needs to have some features present and tested such as * Ensuring that relative paths do not appear in the generated property. * Ensuring that paths to the source or build directories do not appear. * Generating a check in the file for CMake 3.1 or later so that the resulting property will be consumed. * Ensuring that any referenced targets are part of an export set and generating a check for them. * INSTALL_INTERFACE and BUILD_INTERFACE content. All of these checks are already done for INTERFACE_INCLUDE_DIRECTORIES, but it is too late to add them for INTERFACE_SOURCES for CMake 3.1. As the checks introduce some new error conditions, it is better to disallow exporting fully for this case and introduce proper error conditions later instead of policies. --- Source/cmExportBuildFileGenerator.cxx | 10 ++++++++++ Source/cmExportInstallFileGenerator.cxx | 11 +++++++++++ Tests/RunCMake/TargetSources/ExportBuild-result.txt | 1 + Tests/RunCMake/TargetSources/ExportBuild-stderr.txt | 1 + Tests/RunCMake/TargetSources/ExportBuild.cmake | 5 +++++ Tests/RunCMake/TargetSources/ExportInstall-result.txt | 1 + Tests/RunCMake/TargetSources/ExportInstall-stderr.txt | 1 + Tests/RunCMake/TargetSources/ExportInstall.cmake | 6 ++++++ Tests/RunCMake/TargetSources/RunCMakeTest.cmake | 2 ++ 9 files changed, 38 insertions(+) create mode 100644 Tests/RunCMake/TargetSources/ExportBuild-result.txt create mode 100644 Tests/RunCMake/TargetSources/ExportBuild-stderr.txt create mode 100644 Tests/RunCMake/TargetSources/ExportBuild.cmake create mode 100644 Tests/RunCMake/TargetSources/ExportInstall-result.txt create mode 100644 Tests/RunCMake/TargetSources/ExportInstall-stderr.txt create mode 100644 Tests/RunCMake/TargetSources/ExportInstall.cmake diff --git a/Source/cmExportBuildFileGenerator.cxx b/Source/cmExportBuildFileGenerator.cxx index 30a52d4..134ee98 100644 --- a/Source/cmExportBuildFileGenerator.cxx +++ b/Source/cmExportBuildFileGenerator.cxx @@ -68,6 +68,16 @@ bool cmExportBuildFileGenerator::GenerateMainFile(std::ostream& os) tei != this->Exports.end(); ++tei) { cmTarget* te = *tei; + if (te->GetProperty("INTERFACE_SOURCES")) + { + cmOStringStream e; + e << "Target \"" + << te->GetName() + << "\" has a populated INTERFACE_SOURCES property. This is not " + "currently supported."; + cmSystemTools::Error(e.str().c_str()); + return false; + } this->GenerateImportTargetCode(os, te); te->AppendBuildInterfaceIncludes(); diff --git a/Source/cmExportInstallFileGenerator.cxx b/Source/cmExportInstallFileGenerator.cxx index 89071c0..23180f1 100644 --- a/Source/cmExportInstallFileGenerator.cxx +++ b/Source/cmExportInstallFileGenerator.cxx @@ -123,6 +123,17 @@ bool cmExportInstallFileGenerator::GenerateMainFile(std::ostream& os) { cmTarget* te = (*tei)->Target; + if (te->GetProperty("INTERFACE_SOURCES")) + { + cmOStringStream e; + e << "Target \"" + << te->GetName() + << "\" has a populated INTERFACE_SOURCES property. This is not " + "currently supported."; + cmSystemTools::Error(e.str().c_str()); + return false; + } + requiresConfigFiles = requiresConfigFiles || te->GetType() != cmTarget::INTERFACE_LIBRARY; diff --git a/Tests/RunCMake/TargetSources/ExportBuild-result.txt b/Tests/RunCMake/TargetSources/ExportBuild-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportBuild-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetSources/ExportBuild-stderr.txt b/Tests/RunCMake/TargetSources/ExportBuild-stderr.txt new file mode 100644 index 0000000..0d65a55 --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportBuild-stderr.txt @@ -0,0 +1 @@ +CMake Error: Target "iface" has a populated INTERFACE_SOURCES property. This is not currently supported. diff --git a/Tests/RunCMake/TargetSources/ExportBuild.cmake b/Tests/RunCMake/TargetSources/ExportBuild.cmake new file mode 100644 index 0000000..b626aa6 --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportBuild.cmake @@ -0,0 +1,5 @@ + +add_library(iface INTERFACE) +target_sources(iface INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/empty_1.cpp") + +export(TARGETS iface FILE ${CMAKE_CURRENT_BINARY_DIR}/targets.cmake) diff --git a/Tests/RunCMake/TargetSources/ExportInstall-result.txt b/Tests/RunCMake/TargetSources/ExportInstall-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportInstall-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetSources/ExportInstall-stderr.txt b/Tests/RunCMake/TargetSources/ExportInstall-stderr.txt new file mode 100644 index 0000000..0d65a55 --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportInstall-stderr.txt @@ -0,0 +1 @@ +CMake Error: Target "iface" has a populated INTERFACE_SOURCES property. This is not currently supported. diff --git a/Tests/RunCMake/TargetSources/ExportInstall.cmake b/Tests/RunCMake/TargetSources/ExportInstall.cmake new file mode 100644 index 0000000..8e7c9f9 --- /dev/null +++ b/Tests/RunCMake/TargetSources/ExportInstall.cmake @@ -0,0 +1,6 @@ + +add_library(iface INTERFACE) +target_sources(iface INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/empty_1.cpp") + +install(TARGETS iface EXPORT exp) +install(EXPORT exp DESTINATION cmake) diff --git a/Tests/RunCMake/TargetSources/RunCMakeTest.cmake b/Tests/RunCMake/TargetSources/RunCMakeTest.cmake index c6d7f43..1b4ef0b 100644 --- a/Tests/RunCMake/TargetSources/RunCMakeTest.cmake +++ b/Tests/RunCMake/TargetSources/RunCMakeTest.cmake @@ -9,3 +9,5 @@ endif() run_cmake(CMP0026-LOCATION) run_cmake(RelativePathInInterface) +run_cmake(ExportBuild) +run_cmake(ExportInstall) -- cgit v0.12 From 8a75c7ef32af391cb45af889d266e2a77daa61d6 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 28 Nov 2014 18:46:12 +0100 Subject: Help: Document the export limitation of INTERFACE_SOURCES. --- Help/command/target_sources.rst | 4 ++++ Help/prop_tgt/INTERFACE_SOURCES.rst | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/Help/command/target_sources.rst b/Help/command/target_sources.rst index d6f148d..832240a 100644 --- a/Help/command/target_sources.rst +++ b/Help/command/target_sources.rst @@ -22,6 +22,10 @@ items will populate the :prop_tgt:`SOURCES` property of following arguments specify sources. Repeated calls for the same ```` append items in the order called. +Targets with :prop_tgt:`INTERFACE_SOURCES` may not be exported with the +:command:`export` or :command:`install(EXPORT)` commands. This limitation may be +lifted in a future version of CMake. + Arguments to ``target_sources`` may use "generator expressions" with the syntax ``$<...>``. See the :manual:`cmake-generator-expressions(7)` manual for available expressions. See the :manual:`cmake-buildsystem(7)` diff --git a/Help/prop_tgt/INTERFACE_SOURCES.rst b/Help/prop_tgt/INTERFACE_SOURCES.rst index fb28231..c8ca2e8 100644 --- a/Help/prop_tgt/INTERFACE_SOURCES.rst +++ b/Help/prop_tgt/INTERFACE_SOURCES.rst @@ -9,6 +9,10 @@ targets can add entries to their own :prop_tgt:`SOURCES` property such as ``$`` to use the sources specified in the interface of ``foo``. +Targets with ``INTERFACE_SOURCES`` may not be exported with the +:command:`export` or :command:`install(EXPORT)` commands. This limitation may be +lifted in a future version of CMake. + Contents of ``INTERFACE_SOURCES`` may use "generator expressions" with the syntax ``$<...>``. See the :manual:`cmake-generator-expressions(7)` manual for available expressions. See the :manual:`cmake-buildsystem(7)` -- cgit v0.12