Re: [cmake-developers] [CMake] 2.8.11-rc3 generator expression error
On 04/25/2013 03:34 AM, Stephen Kelly wrote: I haven't had time to investigate fully, but this patch should 'fix' the problem: [snip] I'll investigate later to see if it's the right fix and why. Great! I've turned that patch into this commit: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=de9d4a63 See commit message for an explanation that makes sense to me. Since these generator expressions are constructed and evaluated internally to CMake and never exposed it should be easy to adjust how this works in the future. Therefore I'm going to take this as the fix for 2.8.11-rc4 if it is clean on the test dashboard tonight. When you have time to investigate in more detail, please extend this topic with a test case covering target_link_libraries(B debug A) where A is a target instead of an external library name. Amazingly/apparently we do not have this in the test suite yet. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] 2.8.11-rc3 generator expression error
On 04/25/2013 11:02 AM, Stephen Kelly wrote: Brad King wrote: Great! I've turned that patch into this commit: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=de9d4a63 See commit message for an explanation that makes sense to me. The commit message is correct, but it's not the whole story. We also need to validate the it-Value as a target name acceptable to TARGET_PROPERTY. I've pushed the fix-multi-config-tll-include-dirs branch to my clone. That's a much more detailed explanation, thanks. Pre-evaluating the expression as Debug regardless of the configuration involved feels like a hack. Since this all occurs at generate time already anyway, why are we constructing a genex only to evaluate it immediately elsewhere in the generator code? Why can't GetIncludeDirectories just evaluate it-Value as a genex with the current configuration, and if it is a valid target name then lookup its INTERFACE_INCLUDE_DIRECTORIES and use that immediately? Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] 2.8.11-rc3 generator expression error
Brad King wrote: I've pushed the fix-multi-config-tll-include-dirs branch to my clone. That's a much more detailed explanation, thanks. Pre-evaluating the expression as Debug regardless of the configuration involved feels like a hack. Since this all occurs at generate time already anyway, why are we constructing a genex only to evaluate it immediately elsewhere in the generator code? Why can't GetIncludeDirectories just evaluate it-Value as a genex with the current configuration, and if it is a valid target name then lookup its INTERFACE_INCLUDE_DIRECTORIES and use that immediately? Good question. Reading the code, I can't see any reason it needs to be that way. It's possible that when I implemented that I was focussed on re-using the code in the processIncludeDirectories function. Changing it as you describe would probably make the caching easier too because the includes could be cached directly. This is an area where we had performance problems before when running cmake on the llvm repo. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [CMake] 2.8.11-rc3 generator expression error
On 4/25/2013 6:15 PM, Stephen Kelly wrote: Brad King wrote: Why can't GetIncludeDirectories just evaluate it-Value as a genex with the current configuration, and if it is a valid target name then lookup its INTERFACE_INCLUDE_DIRECTORIES and use that immediately? Good question. Reading the code, I can't see any reason it needs to be that way. I looked into implementing this and realized that we're not propagating usage requirements from the entire link closure, only from the direct dependencies. In an earlier design iteration the transitive requirements were brought in by evaluation of the interface properties themselves. Now that tll() doesn't touch the properties and the requirements are simply appended during generation we need to use the full link closure. I've drafted a change to convert cmTarget::GetIncludeDirectories and cmTarget::GetCompileDefinitions over to share a common internal usage requirements API that uses GetLinkInformation to get the transitive link closure and looks up the INTERFACE_* properties in every linked target. The caching is simplified because we never need to clear it since these are now only called during generation and never during configuration. I'll report back after doing more testing. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
