[cmake-developers] Target usage requirements and conventions

[Stephen Kelly]

Brad King wrote:

> On 5/1/2012 5:33 PM, Stephen Kelly wrote:
>> I think Alex' suggestion solves the problem anyway:
> 
> Yes.
> 
>> target_use_package(Foo IMPORTED_TARGETS Bar)
> 
> There is no reason that targets built in the same project can't provide
> package interfaces just like imported targets.  I think this can be
> just "TARGETS", or just "TARGET" if we allow only one package per call.

I would like to be able to specify multiple arguments to one invokation of 
it. I probably wouldn't allow

target_use_package(Foo TARGETS Bar Bat PACKAGES Blub Blur)

but instead require that to be 

target_use_package(Foo TARGETS Bar Bat)
target_use_package(Foo PACKAGES Blub Blur)

for clarity.

> Would this one ever imply find_package?

I don't think it would need to, if we have a way to be explicit about it 
with PACKAGES or so.

> 
>> target_use_package(Foo VIA_PREFIX Bar )
> 
> The name here should indicate that the prefix is for variables and
> not an installation prefix or something.  Perhaps one of
> 
>   (VAR|VARIABLE)_(PREFIX|GROUP|NAMESPACE)

Yes, I think VARIABLE_PREFIX would be fine.

> 
> ?  Would this one ever imply find_package to set the variables?

I think better than what you suggest below:

target_use_package(Foo FIND_PACKAGE Bar)

would be:

target_use_package(Foo VARIABLE_PREFIX Bar FIND_PACKAGE NO_MODULE)

That is, make the FIND_PACKAGE a indicator that the package should be found 
first, and the arguments following it should be passed to the find_package 
command. 

I don't think this would be good API for the TARGETS mode above, because the 
names of targets do not necessarily correspond to the name of the package 
they are in. For the prefix though, it is the convention set forth in the 
readme.txt (which might need to be extended with Foo_COMPILE_DEFINITIONS 
etc).

> 
>> target_use_package(Foo PACKAGE Bar)
> 
> There is debate elsewhere in this thread over whether this would
> imply find_package(Bar).  Perhaps we should say that "PACKAGE"
> means use an already-found package and error if not found already.

I think that would be ok, yes.

> Then add
> 
>   target_use_package(Foo FIND_PACKAGE Bar)

Or 

target_use_package(Foo PACKAGE Bar FIND_PACKAGE)

> 
> to mean find-if-necessary.  Then arguments after "Bar" would be
> passed to the implied find_package for version and all the other
> options.  Either Module mode or Config mode could detect whether
> there is enough information to define the Bar package, and then
> do so.  See below.
> 
> This brings up a fundamental question for this design discussion.
> Are usage requirements associated with packages, with targets, or
> with either/both?

Both, I think. They are associated with targets because properties on 
targets are what needs to be propagated as required for usage, and because 
targets will get usage requirements from other targets. 

They are associated with packages for compatibility with packages which do 
not create exported targets, but which do have conventional prefixes on 
variables.

> 
>>> We will likely need to make packages something first-class on the
>>> C++ side.  If find_package(Bar) detects that the files it loaded
>>> define sufficient information for a package it should construct a
>>> C++ object to represent the package.  This event should establish
>>> the package.  A loose sequence of variable set()s is not explicit
>>> enough.
>>
>> Right.
> 
> How might find_package detect/define a first-class package object?
> Just look for the right variables?  Perhaps we could add an (optional)
> explicit package declaration:
> 
>   cmake_package(Bar LIBRARIES ... DEFINITIONS ...)
> 
> that can be either in the Find module or in the package configuration
> file.  This would allow more detailed information to be set.

Maybe. Where would this get us that convention in the right variables would 
not?

I can imagine someone looking at that and thinking that that's the way to 
notify users of my package/config file how to use it, and disregard the 
actual variable name conventions:

FooConfig.cmake:

set(THE_FOO_LIBRARIES foo1 foo2)

cmake_package(Foo LIBRARIES ${THE_FOO_LIBRARIES})

Then again, cmake_package could also ensure that the correct conventional 
variables are set by actually setting Foo_LIBRARIES to be the contents of 
THE_FOO_LIBRARIES in that case, ensuring the possibility of using the 
variables conventionally.

That also opens up the question of whether it should overwrite Foo_LIBRARIES 
if it already exists, or warn about that.

I also think the 'Foo' would be redundant. It should be this to avoid 
mistakes:

cmake_package(LIBRARIES ${THE_FOO_LIBRARIES})

CMake already knows the name of the package, right?

> 
>>> We need to settle on a generator-expression syntax for per-config
>>> conditions.  The $<CONFIG?Debug:...>  syntax is sufficient for
>>> discussion but needs more thorough discussion before it is final.
>>
>> In particular, I think it might be necessary to set multiple conditions
>> in such expressions.
> 
> Good point.  That could get ugly with my originally proposed syntax :(
> I'll have to think about it more.
> 
>> set_target_property(Foo INTERFACE_INCLUDE_DIRS
>>    "$<INCLUDE_MODE=Build:${CMAKE_SOURCE_DIR}/src/foo/debugstuff>"
>>    "$<CONFIG=Debug?INCLUDE_MODE=Build:${CMAKE_SOURCE_DIR}/src/foo>"
>>    "$<INCLUDE_MODE=Install:${CMAKE_INSTALL_PREFIX}/mylib>"
>> )
> 
> I don't think we should have something like INCLUDE_MODE.  There will
> be much more about a package that varies between build tree and install
> tree.  Currently when I write a project that can be used from either
> tree I create separate package configuration files and separate target
> files, one with export() and one with install(EXPORT).  Essentially
> each one is its own complete package that doesn't care about the other.

Yes, that is the case I was thinking about with the above example. In this 
case, the imported target created by export() would create a 
INTERFACE_INCLUDE_DIRS (or some better name) property on the imported target 
with the value: 

  "${CMAKE_SOURCE_DIR}/src/foo/debugstuff"
  "$<CONFIG=Debug:${CMAKE_SOURCE_DIR}/src/foo>"

and in the one created by install(EXPORT), it would have the value:

  "$<INCLUDE_MODE=Install:${CMAKE_INSTALL_PREFIX}/mylib>"

> 
> I'd prefer that the properties set directly on the target in the build
> tree affect only its usage requirements for use from the build tree.

Hmm, I proposed it this way because for example if Foo uses QtCore and 
includes QString in its headers, and QtCore is installed at /opt/qt5/, then 
/opt/qt5/ needs to be in the include_directories of Bar (which uses Foo).

Thinking about it again, the directories at configure time of Foo are not 
necessarily the same as at configure time of Bar, so indeed maybe that's not 
the case. 

We probably need some way to say 'use the include directories of the Foo 
target', or '... Foo package' without specifying them. I think that's 
already possible with the link libraries with a recent patch from Alex?

> The export() command would transfer this information to the imported
> target it generates.

Yes.

> 
> We need a way to separately specify the install-tree usage requirements
> to be associated with its imported targets.  Currently I don't like our
> target properties like INSTALL_RPATH that are set on the build-tree
> target but affect the install-tree file. 

Yes. RPATH was another 'dimension' I listed in an unpublished draft of the 
wiki page, but I took it out. It's essentially just a special case of 
LINK_FLAGS. The implementation of it could be a bit messy.

> It breaks with the fact that
> our install(TARGETS) interface allows the same target file to be put
> in multiple install locations.  Perhaps we can fix that mistake along
> with the new interface.  New options could be added to the install
> command's TARGETS mode to specify usage requirements for the targets.
>
> Perhaps a new install(PACKAGE) command?  We have export(PACKAGE)
> already which could be extended for this purpose.

Maybe that would work, yes.

> 
>> Yes, I think we'll need to first have a list of 'dimensions' that are
>> relevant....Is there anything missing from the list I made?
> 
> It looks good to me but I don't think we'll know for sure until it is
> implemented and new cases are encountered ;)

Right, so I guess we can start prototyping. :)

Thanks,

Steve.