-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle the no-target case. #403
Conversation
It is done through a COMPUTE_TARGETS_LIST in package-config.cmake Targets are gathered by going through the root and the subdirectories of the root and collect the BUILDSYSTEM_TARGET property. From this list, standard targets are removed. Then if there is no user target the <package-name>Target.cmake file is not generated and not included in the Config.cmake.in file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot we let user #398 say that this package has no target ?
It is simple to implement, and fairly simple to use. Something that says that you want a Config.cmake file without a Targets.cmake file.
I am not sure this will cover all cases anyway. There are packages which have targets that aren't meant to be exported.
|
||
COMPUTE_TARGETS_LIST() | ||
if(TARGETS_LIST_LENGTH) | ||
SET(VALID_TARGETS_EXPORT_NAME "SET(VALID_TARGETS_EXPORT_NAME TRUE)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the scope of variable VALID_TARGETS_EXPORT_NAME
in the configured Config.cmake ? Will it interfere with previous included config files ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
I agree with @jmirabel I think it's much simpler to add an option to opt-out of targets installation (and checking for the file existence before including in the generated Config.cmake) than trying to list all targets in the project. Plus this listing doesn't check that any targets are actually exported so it's prone to be a partial solution at best. |
But then how do you handle the case where some packages are optional ? The user has to handle both cases ? |
This indeed would be a better solution to avoid the problem with the scope of TARGETS_LIST_LENGTH. |
I think I understand where you need it. On your side, it is going to be a There may be a cleverer way of doing this but I think letting the user handle it is preferable over relying on targets being created or not. |
Closing in favor of #408 |
Goal
Handling package with no targets (typically data packages)
Issue #398
Principle
It is done through a COMPUTE_TARGETS_LIST in package-config.cmake
Targets are gathered by going through the root and the subdirectories
of the root and collect the BUILDSYSTEM_TARGET property for directory.
From this list, standard targets are removed.
Then if there is no user target the Target.cmake file
is not generated and not included in the Config.cmake.in file.
Cons:
There is a hard coded list of targets which are filtered out. They are generated probably by a call to CTest or equivalent. That is probably the weaker point of this proposal. However I did not find the source of such targets. And they do not appear in the list of targets generated by the INSTALL(export) call.
With respect to what it is intended here I believe that there is no impact on most of our use cases.
Tests:
It was tested over talos_data (https://github.com/olivier-stasse/talos_data/tree/topic/no-target-optional) and jrl-walkgen.
To make sure it does not affect usual packages it was tested over dynamic-graph:
https://github.com/olivier-stasse/dynamic-graph/tree/Mac
And together with dynamc-graph-python:
https://github.com/olivier-stasse/dynamic-graph-python/tree/topic/no-target-optional