[cmake-developers] Setting includes, defines and other usage requirements with one command

Stephen Kelly steveire at gmail.com
Thu Jan 31 11:35:15 EST 2013


Brad King wrote:

> On 01/31/2013 10:23 AM, Stephen Kelly wrote:
>> I left it as $<LINKED> so far so that it is a shorter string, and because
>> the implementation is somewhat simpler.
> 
> Ahh, I see you were using $<LINKED:...> even for already-defined targets.
> I think that is fine during processing of the same project to keep the
> strings shorter.  We still need to keep it out of the exported interface
> so that we can change it later without having to keep the expression
> implementation around to support old export files.  This way it never
> outlives the CMake process that generated it.

Fair enough. I've squashed that patch into the commit that introduces the 
LINKED genex.

> 
>> Do you think it needs to be done before merging to next?
> 
> De-duplication of $<LINKED> references can be added later but I'd still
> like to see it before the 2.8.11 release so that it doesn't become a
> behavior change later.

Ok, I can look into that.

> 
>> Anything else to do before merging it?
> 
> Please add a test case for the $<INSTALL_PREFIX> evaluation error.
> 

Done.

> 
> What is the purpose of the else case here?
> 
> +  std::string value = !isGenex ? "$<LINKED:" + lib + ">"
> +                               : "$<$<TARGET_DEFINED:" + lib + ">:" +
> +                                   "$<TARGET_PROPERTY:" + lib +
> +                                   ",INTERFACE_" + property + ">"
> +                                 ">";
> 
> If the input is already a genex isn't the author responsible for
> making sure it is valid?

Yes, a valid genex for linking. Consider this:

 add_library(foo ...)
 add_library(bar ...)
 target_link_libraries(foo $<$<CONFIG:Debug>:bar>)

The genex for the INCLUDE_DIRECTORIES of foo needs to be (excuse the python-
style concats):

 "$<$<TARGET_DEFINED:" + "$<$<CONFIG:Debug>:bar>" + ">:" +
                       "$<TARGET_PROPERTY:" + "$<$<CONFIG:Debug>:bar>" +
                              ",INTERFACE_" + "INCLUDE_DIRECTORIES" + ">"
                                 ">"

because the result of the genex could either be "" or "bar".

> The documentation of <package>_NO_INTERFACES and <package>_INTERFACES
> reference ${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION but the corresponding
> <package>_FIND_VERSION isn't documented until further down.  Please move
> the new docs to the bottom, but still above the NO_POLICY_SCOPE line.
> 
> Also the current wording of the documentation makes it sound like 2.8.11
> is at fault for introducing incompatibility.  There is no need to mention
> the version of CMake there.  It is the version of the upstream that
> starts using the new features that causes a problem.
> 
> It should be clear that the upstream is responsible for adding the example
> code to set the <package>_NO_INTERFACES variable in their package config
> file when it exposes the new interfaces.  Also <package>_INTERFACES is to
> be set by the downstream and used by that code in the upstream.

Ok. Hopefully that's more clear now.

> 
> 
> Then merge for testing!

Done.

Thanks,

Steve.





More information about the cmake-developers mailing list