Here's a version that is more conservative. It doesn't change the install(EXPORT) behavior. install(TARGET) already supported genex, so basically this patch adds install(FILES) destination genex.
Perhaps we should update the Help to only mention install(FILES) destination instead of all variations of install? I don't have time to work on install(EXPORT) yet. I think it would still be good to have install(FILES) merged even if install(EXPORT) isn't done yet. Thanks! -----Original Message----- From: Brad King [mailto:brad.k...@kitware.com] Sent: Thursday, September 10, 2015 11:07 AM To: Robert Goulet <robert.gou...@autodesk.com> Cc: cmake-developers@cmake.org Subject: Re: Generator expressions for install destination On 09/09/2015 12:21 PM, Robert Goulet wrote: > Here's the patch to add generator expressions to the install command > DESTINATION option. Thanks for the update. >>This should not be needed if things are factored correctly. >>Everything in that block already passes "config" through as a parameter. > > None of the places where I use GetDestination, except in > cmInstallTargetGenerator, receives a config in parameter. > An ideally, the ConfigurationName member should not even exist, but > that will force all places to pass the config as a parameter. > Imho it's better to keep refactoring in a separate patch. I think such a separate refactoring patch should come first. I cannot review the logic with confidence because I don't know what implicitly depends on ConfigurationName being set and whether call paths leading to it set it correctly. Also, take a look at the install(EXPORT) case. I tried using a $<CONFIG> genex in the DESTINATION argument with your patch but it fails trying to create a directory with literal $<CONFIG> in its name (on Windows with a VS generator). Note that the install(EXPORT) command generates some files in the build tree packed away under CMakeFiles/Export and then adds cmake_install.cmake rules to install those. Generation of these files needs to expand the generator expression in time. Thanks, -Brad
install-dest-genex.patch
Description: install-dest-genex.patch
-- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers