[cmake-developers] Review Request: Topic ExternalProject_labels

[Daniele E. Domenichelli]

On 29/04/14 15:13, David Cole wrote:
> You could do the same thing with set_property calls in 
> your own code immediately after an ExternalProject_Add.

You would have to do it immediately after each ExternalProject_Add_Step
call.

I agree that you could do it, but you will have to add several extra
lines of code while, in normal cases you could just set the
"USE_FOLDERS" global property and get all your external projects
organized in subfolders. (Or for the LABELS property just configure the
ctest script for subprojects)
I don't see many other use cases for this...


> If we go to the trouble to actually incorporate this into 
> ExternalProject, shouldn't we make it flexible, so that callers can 
> pass in LABELS of their choice, and a FOLDER name of their choice? 
> Simply hard-coding it to the name of the project, and the string 
> "ExternalProjectTargets" may make it possible to organize, but not 
> everybody will be happy with the resulting names and organization.

Maybe... but CMake already uses "fixed" target to group targets
(CMakePredefinedTargets and CTestDashboardTargets), therefore I don't
see a big issue in hardcoding the folder name...
Using ExternalProjectTargets/<project_name> as folder seems to me a good
default, all the targets for each external project will be grouped in
the same folder.

About the LABELS property, I don't see a reason not to add a label with
the same name of the project to each project target.

I see both patches as "better defaults than not having one". As you said
it is possible to use set_property later to replace the FOLDER property
or to replace/append LABELS if the user don't like the default value...



> I think if we're going to add something like this to EP, it should be 
> flexible enough to allow LABELS and FOLDER values to be passed in from 
> the caller.

I think that the syntax for ExternalProject is already quite complicated
and adding extra arguments that most users will not use, and that can be
replaced with set_property, if you really don't like the default values,
will just make it harder to use, without actually adding a real value.

Anyway, if you still believe that these values should be configurable, I
can modify it, just let me know.



Cheers,
 Daniele