[cmake-developers] Review Request: Topic ExternalProject-independent-step-targets

[Daniele E. Domenichelli]

On 20/12/13 14:35, David Cole wrote:
> Existing calls to ExternalProject_Add must continue to work as they 
> were before your topic. As long as you leave *most* of them alone for 
> testing purposes, you should be ok. Ideally, for new functionality you 
> would write a new part of the test, with new calls to 
> ExternalProject_Add that test your new pieces.

Most of the calls work with no changes at all, the problem is that the
tests use ExternalProject_Add to setup vcs repositories. These
repositories are then cloned by another ExternalProject.

I added a line

   set_property(DIRECTORY PROPERTY EP_INDEPENDENT_STEP_TARGETS download update)

that make the download and update targets independent for all the
ExternalProjects, and that usually works, but that is obviously not
ok in this case, since the repository *must* be ready before you can
download from it and therefore the download step _is_ dependent.
This is in my opinion a corner case, that will not be used outside
the tests, since I believe that normally one will use the DEPENDS flag
for *build* dependencies, not for *download* dependencies (that usually
don't exist)

So the calls that should be updated are the ones that require the
repositories to be ready. For all the other calls to ExternalProject,
there won't be any difference.


My first fix was to use INDEPENDENT_STEP_TARGETS "" for these
ExternalProjects, overriding in this way the directory property and
therefore making them dependent. Since Brad didn't like it, what I am
now suggesting is to add the ExternalProject_Add_Step_Dependencies
method and to remove the line

    DEPENDS "SetupLocalXXXRepository"

from all the external projects cloning from a local repository (keeping
the DEPENDS flag for build dependencies) and to add instead one of
these lines (I'm not sure of which one yet):

    ExternalProject_Add_Step_Dependencies(${proj} download SetupLocalXXXRepository)
    ExternalProject_Add_Step_Dependencies(${proj} download SetupLocalXXXRepository-update)
    ExternalProject_Add_Step_Dependencies(${proj} download SetupLocalXXXRepository-download)

So that the dependency is explicitly set for the download step only.




> ExternalProject_Add is a tricky one, though, because adding many calls 
> to it can significantly increase the time required to run the test.

This will allow to test both the EP_INDEPENDENT_STEP_TARGETS property
and the ExternalProject_Add_Step_Dependencies without increasing the
execution time of the tests.



> What you don't want to do is change *all* of them and accidentally 
> *require* everyone to update their code to work with the new version of 
> CMake.
> 
> i.e. -- don't let a backwards compatibility break go unnoticed by 
> changing all the tests at the same time that you change underlying 
> CMake functionality.

As I said, I think this is a corner case, and the changes don't break
backwards compatibility, it is the EP_INDEPENDENT_STEP_TARGETS property
that I added to the test (since I would like to test it) that adds
extra complexity to the tests, and therefore requires some change to
these ExternalProject_Add calls. Removing the property, the tests will
work with no changes at all.


Anyway, if you prefer, I can just test the EP_INDEPENDENT_STEP_TARGETS
in another directory, but that means adding extra ExternalProjects, and
therefore extra execution time...



Cheers,
 Daniele