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

Attachment: 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

Reply via email to