From e36a05fd7f7d008c4c1e75ebf46eac3072ef71b1 Mon Sep 17 00:00:00 2001 From: Robert Goulet Date: Tue, 11 Aug 2015 15:18:39 -0400 Subject: [PATCH 1/2] cmTarget: Detect and diagnose recursion in GetOutputInfo --- Source/cmTarget.cxx | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index d3170e4ae..d2d4c671f 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -66,6 +66,8 @@ struct cmTarget::OutputInfo std::string OutDir; std::string ImpDir; std::string PdbDir; + bool empty() const + { return OutDir.empty() && ImpDir.empty() && PdbDir.empty(); } }; //---------------------------------------------------------------------------- @@ -2609,19 +2611,35 @@ cmTarget::OutputInfo const* cmTarget::GetOutputInfo( config_upper = cmSystemTools::UpperCase(config); } typedef cmTargetInternals::OutputInfoMapType OutputInfoMapType; - OutputInfoMapType::const_iterator i = + OutputInfoMapType::iterator i = this->Internal->OutputInfoMap.find(config_upper); if(i == this->Internal->OutputInfoMap.end()) { + // Add empty info in map to detect potential recursion. OutputInfo info; + OutputInfoMapType::value_type entry(config_upper, info); + i = this->Internal->OutputInfoMap.insert(entry).first; + + // Compute output directories. this->ComputeOutputDir(config, false, info.OutDir); this->ComputeOutputDir(config, true, info.ImpDir); if(!this->ComputePDBOutputDir("PDB", config, info.PdbDir)) { info.PdbDir = info.OutDir; } - OutputInfoMapType::value_type entry(config_upper, info); - i = this->Internal->OutputInfoMap.insert(entry).first; + + // Now update the previously-prepared map entry. + i->second = info; + } + else if(i->second.empty()) + { + // An empty map entry indicates we have been called recursively + // from the above block. + this->Makefile->GetCMakeInstance()->IssueMessage( + cmake::FATAL_ERROR, + "Target '" + this->GetName() + "' OUTPUT_DIRECTORY depends on itself.", + this->GetBacktrace()); + return 0; } return &i->second; } From d25819bc2623b5144ffc57b694500993ac5759b4 Mon Sep 17 00:00:00 2001 From: Robert Goulet Date: Tue, 11 Aug 2015 15:19:03 -0400 Subject: [PATCH 2/2] Add generator expression support to OUTPUT_DIRECTORY target properties If {ARCHIVE,LIBRARY,RUNTIME}_OUTPUT_DIRECTORY is set with a genex then do not add the per-config subdirectory on multi-config generators. This will allow projects to use $ to place the per-config part of the directory path somewhere other than the end. --- .../ARCHIVE_OUTPUT_DIRECTORY_CONFIG.rst | 3 +++ .../LIBRARY_OUTPUT_DIRECTORY_CONFIG.rst | 3 +++ .../RUNTIME_OUTPUT_DIRECTORY_CONFIG.rst | 3 +++ Help/prop_tgt/XXX_OUTPUT_DIRECTORY.txt | 7 ++++-- Help/release/dev/OUTPUT_DIRECTORY-genex.rst | 7 ++++++ Source/cmTarget.cxx | 17 +++++++++++-- Tests/ExportImport/Export/CMakeLists.txt | 17 +++++++++++-- Tests/ExportImport/Export/testExe4.c | 24 +++++++++++++++++++ Tests/ExportImport/Import/A/CMakeLists.txt | 12 ++++++++++ Tests/ExportImport/Import/A/imp_testExe1.c | 3 ++- .../GeneratorExpression/RunCMakeTest.cmake | 1 + .../TARGET_FILE-recursion-result.txt | 1 + .../TARGET_FILE-recursion-stderr.txt | 4 ++++ .../TARGET_FILE-recursion.cmake | 3 +++ 14 files changed, 98 insertions(+), 7 deletions(-) create mode 100644 Help/release/dev/OUTPUT_DIRECTORY-genex.rst create mode 100644 Tests/ExportImport/Export/testExe4.c create mode 100644 Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake diff --git a/Help/prop_tgt/ARCHIVE_OUTPUT_DIRECTORY_CONFIG.rst b/Help/prop_tgt/ARCHIVE_OUTPUT_DIRECTORY_CONFIG.rst index 29991ebf8..12f8bb7ef 100644 --- a/Help/prop_tgt/ARCHIVE_OUTPUT_DIRECTORY_CONFIG.rst +++ b/Help/prop_tgt/ARCHIVE_OUTPUT_DIRECTORY_CONFIG.rst @@ -11,3 +11,6 @@ per-configuration subdirectory to the specified directory. This property is initialized by the value of the :variable:`CMAKE_ARCHIVE_OUTPUT_DIRECTORY_` variable if it is set when a target is created. + +Contents of ``ARCHIVE_OUTPUT_DIRECTORY_`` may use +:manual:`generator expressions `. diff --git a/Help/prop_tgt/LIBRARY_OUTPUT_DIRECTORY_CONFIG.rst b/Help/prop_tgt/LIBRARY_OUTPUT_DIRECTORY_CONFIG.rst index 6fc014220..28dd404df 100644 --- a/Help/prop_tgt/LIBRARY_OUTPUT_DIRECTORY_CONFIG.rst +++ b/Help/prop_tgt/LIBRARY_OUTPUT_DIRECTORY_CONFIG.rst @@ -11,3 +11,6 @@ per-configuration subdirectory to the specified directory. This property is initialized by the value of the :variable:`CMAKE_LIBRARY_OUTPUT_DIRECTORY_` variable if it is set when a target is created. + +Contents of ``LIBRARY_OUTPUT_DIRECTORY_`` may use +:manual:`generator expressions `. diff --git a/Help/prop_tgt/RUNTIME_OUTPUT_DIRECTORY_CONFIG.rst b/Help/prop_tgt/RUNTIME_OUTPUT_DIRECTORY_CONFIG.rst index c10034666..94fb2776a 100644 --- a/Help/prop_tgt/RUNTIME_OUTPUT_DIRECTORY_CONFIG.rst +++ b/Help/prop_tgt/RUNTIME_OUTPUT_DIRECTORY_CONFIG.rst @@ -11,3 +11,6 @@ per-configuration subdirectory to the specified directory. This property is initialized by the value of the :variable:`CMAKE_RUNTIME_OUTPUT_DIRECTORY_` variable if it is set when a target is created. + +Contents of ``RUNTIME_OUTPUT_DIRECTORY_`` may use +:manual:`generator expressions `. diff --git a/Help/prop_tgt/XXX_OUTPUT_DIRECTORY.txt b/Help/prop_tgt/XXX_OUTPUT_DIRECTORY.txt index 0b3d31c29..3ae54484a 100644 --- a/Help/prop_tgt/XXX_OUTPUT_DIRECTORY.txt +++ b/Help/prop_tgt/XXX_OUTPUT_DIRECTORY.txt @@ -1,8 +1,11 @@ Output directory in which to build |XXX| target files. This property specifies the directory into which |xxx| target files -should be built. Multi-configuration generators (VS, Xcode) append a -per-configuration subdirectory to the specified directory. +should be built. The property value may use +:manual:`generator expressions `. +Multi-configuration generators (VS, Xcode) append a per-configuration +subdirectory to the specified directory unless a generator expression +is used. This property is initialized by the value of the variable |CMAKE_XXX_OUTPUT_DIRECTORY| if it is set when a target is created. diff --git a/Help/release/dev/OUTPUT_DIRECTORY-genex.rst b/Help/release/dev/OUTPUT_DIRECTORY-genex.rst new file mode 100644 index 000000000..8b839c038 --- /dev/null +++ b/Help/release/dev/OUTPUT_DIRECTORY-genex.rst @@ -0,0 +1,7 @@ +OUTPUT_DIRECTORY-genex +---------------------- + +* The :prop_tgt:`ARCHIVE_OUTPUT_DIRECTORY`, + :prop_tgt:`LIBRARY_OUTPUT_DIRECTORY`, and + :prop_tgt:`RUNTIME_OUTPUT_DIRECTORY` target properties learned to + support :manual:`generator expressions `. diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index d2d4c671f..a94c2ddaa 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -3520,7 +3520,10 @@ bool cmTarget::ComputeOutputDir(const std::string& config, if(const char* config_outdir = this->GetProperty(configProp)) { // Use the user-specified per-configuration output directory. - out = config_outdir; + cmGeneratorExpression ge; + cmsys::auto_ptr cge = + ge.Parse(config_outdir); + out = cge->Evaluate(this->Makefile, config); // Skip per-configuration subdirectory. conf = ""; @@ -3528,7 +3531,17 @@ bool cmTarget::ComputeOutputDir(const std::string& config, else if(const char* outdir = this->GetProperty(propertyName)) { // Use the user-specified output directory. - out = outdir; + cmGeneratorExpression ge; + cmsys::auto_ptr cge = + ge.Parse(outdir); + out = cge->Evaluate(this->Makefile, config); + + // Skip per-configuration subdirectory if the value contained a + // generator expression. + if (out != outdir) + { + conf = ""; + } } else if(this->GetType() == cmTarget::EXECUTABLE) { diff --git a/Tests/ExportImport/Export/CMakeLists.txt b/Tests/ExportImport/Export/CMakeLists.txt index df3f178e1..7fdfaa847 100644 --- a/Tests/ExportImport/Export/CMakeLists.txt +++ b/Tests/ExportImport/Export/CMakeLists.txt @@ -105,6 +105,19 @@ target_link_libraries(testLib4 add_executable(testExe3 testExe3.c) set_property(TARGET testExe3 PROPERTY MACOSX_BUNDLE 1) +# Test _OUTPUT_DIRECTORY[_] properties with generator expressions +add_executable(testExe4 testExe4.c) +target_link_libraries(testExe4 testExe1lib) +set_property(TARGET testLib7 PROPERTY ARCHIVE_OUTPUT_DIRECTORY_DEBUG testLib7D-$) +set_property(TARGET testLib7 PROPERTY ARCHIVE_OUTPUT_DIRECTORY_RELEASE testLib7R-$) +set_property(TARGET testLib7 PROPERTY ARCHIVE_OUTPUT_DIRECTORY testLib7-$) +set_property(TARGET testLib5 PROPERTY LIBRARY_OUTPUT_DIRECTORY_DEBUG testLib5D-$) +set_property(TARGET testLib5 PROPERTY LIBRARY_OUTPUT_DIRECTORY_RELEASE testLib5R-$) +set_property(TARGET testLib5 PROPERTY LIBRARY_OUTPUT_DIRECTORY testLib5-$) +set_property(TARGET testExe4 PROPERTY RUNTIME_OUTPUT_DIRECTORY_DEBUG testExe4D-$) +set_property(TARGET testExe4 PROPERTY RUNTIME_OUTPUT_DIRECTORY_RELEASE testExe4R-$) +set_property(TARGET testExe4 PROPERTY RUNTIME_OUTPUT_DIRECTORY testExe4-$) + # Test cyclic dependencies. add_library(testLibCycleA STATIC testLibCycleA1.c testLibCycleA2.c testLibCycleA3.c) @@ -450,7 +463,7 @@ install(FILES # Install and export from install tree. install( TARGETS - testExe1 testLib1 testLib2 testExe2 testLib3 testLib4 testExe3 + testExe1 testLib1 testLib2 testExe2 testLib3 testLib4 testExe3 testExe4 testExe2lib testLib4lib testLib4libdbg testLib4libopt testLib6 testLib7 testLibCycleA testLibCycleB @@ -511,7 +524,7 @@ export(TARGETS testExe1 testLib1 testLib2 testLib3 NAMESPACE bld_ FILE ExportBuildTree.cmake ) -export(TARGETS testExe2 testLib4 testLib5 testLib6 testLib7 testExe3 testExe2lib +export(TARGETS testExe2 testLib4 testLib5 testLib6 testLib7 testExe3 testExe4 testExe2lib testLib4lib testLib4libdbg testLib4libopt testLibCycleA testLibCycleB testLibPerConfigDest diff --git a/Tests/ExportImport/Export/testExe4.c b/Tests/ExportImport/Export/testExe4.c new file mode 100644 index 000000000..731057edc --- /dev/null +++ b/Tests/ExportImport/Export/testExe4.c @@ -0,0 +1,24 @@ +#include + +int main(int argc, const char* argv[]) +{ + if(argc < 2) + { + fprintf(stderr, "Must specify output file.\n"); + return 1; + } + { + FILE* f = fopen(argv[1], "w"); + if(f) + { + fprintf(f, "int generated_by_testExe4() { return 0; }\n"); + fclose(f); + } + else + { + fprintf(stderr, "Error writing to %s\n", argv[1]); + return 1; + } + } + return 0; +} diff --git a/Tests/ExportImport/Import/A/CMakeLists.txt b/Tests/ExportImport/Import/A/CMakeLists.txt index a74bad102..0f56495d5 100644 --- a/Tests/ExportImport/Import/A/CMakeLists.txt +++ b/Tests/ExportImport/Import/A/CMakeLists.txt @@ -19,11 +19,17 @@ add_custom_command( COMMAND exp_testExe3 ${Import_BINARY_DIR}/exp_generated3.c DEPENDS exp_testExe3 ) +add_custom_command( + OUTPUT ${Import_BINARY_DIR}/exp_generated4.c + COMMAND exp_testExe4 ${Import_BINARY_DIR}/exp_generated4.c + DEPENDS exp_testExe4 + ) add_executable(imp_testExe1 imp_testExe1.c ${Import_BINARY_DIR}/exp_generated.c ${Import_BINARY_DIR}/exp_generated3.c + ${Import_BINARY_DIR}/exp_generated4.c ) # Try linking to a library imported from the install tree. @@ -53,11 +59,17 @@ add_custom_command( COMMAND bld_testExe3 ${Import_BINARY_DIR}/bld_generated3.c DEPENDS bld_testExe3 ) +add_custom_command( + OUTPUT ${Import_BINARY_DIR}/bld_generated4.c + COMMAND bld_testExe4 ${Import_BINARY_DIR}/bld_generated4.c + DEPENDS bld_testExe4 + ) add_executable(imp_testExe1b imp_testExe1.c ${Import_BINARY_DIR}/bld_generated.c ${Import_BINARY_DIR}/bld_generated3.c + ${Import_BINARY_DIR}/bld_generated4.c ) # Try linking to a library imported from the build tree. diff --git a/Tests/ExportImport/Import/A/imp_testExe1.c b/Tests/ExportImport/Import/A/imp_testExe1.c index 56cdd2c0a..0a74309a3 100644 --- a/Tests/ExportImport/Import/A/imp_testExe1.c +++ b/Tests/ExportImport/Import/A/imp_testExe1.c @@ -1,5 +1,6 @@ extern int generated_by_testExe1(); extern int generated_by_testExe3(); +extern int generated_by_testExe4(); extern int testLib2(); extern int testLib3(); extern int testLib4(); @@ -24,5 +25,5 @@ int main() return (testLib2() + generated_by_testExe1() + testLib3() + testLib4() + testLib5() + testLib6() + testLib7() + testLibCycleA1() + testLibPerConfigDest() - + generated_by_testExe3() + testLib4lib() + testLib4libcfg()); + + generated_by_testExe3() + generated_by_testExe4() + testLib4lib() + testLib4libcfg()); } diff --git a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake index cba3941df..21fc8518e 100644 --- a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake +++ b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake @@ -25,6 +25,7 @@ run_cmake(COMPILE_LANGUAGE-add_executable) run_cmake(COMPILE_LANGUAGE-add_library) run_cmake(COMPILE_LANGUAGE-add_test) run_cmake(COMPILE_LANGUAGE-unknown-lang) +run_cmake(TARGET_FILE-recursion) run_cmake(ImportedTarget-TARGET_PDB_FILE) if(LINKER_SUPPORTS_PDB) diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-result.txt b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-stderr.txt b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-stderr.txt new file mode 100644 index 000000000..5b15526e7 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at TARGET_FILE-recursion.cmake:[0-9]+ \(add_executable\): + Target 'empty1' OUTPUT_DIRECTORY depends on itself. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake new file mode 100644 index 000000000..7633be1b3 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake @@ -0,0 +1,3 @@ +enable_language(C) +add_executable(empty1 empty.c) +set_property(TARGET empty1 PROPERTY RUNTIME_OUTPUT_DIRECTORY $)