Re: [CMake] How to add dependencies to ExternalProject_Add()

2011-03-28 Thread Rolf Eike Beer 
I wrote:
> David Cole wrote:

> > I am not sure I like this patch, or not. I'm on the fence.
> > 
> > I would recommend just using:
> >   ExternalProject_Add(xyz ...)
> >   add_dependencies(xyz mylib)
> > 
> > to add non-ExternalProject dependencies. Just to make it clear that the
> > dependencies are target-level dependencies and not file dependencies.
> 
> This only "sort-of" works. If I delete the library from the build try
> (only the .so) and rebuild the external project does not get relinked.
> Also the documentation of ExternalProject_Add says:
> 
>   [DEPENDS projects...]   # Targets on which the project depends
> 
> No word that these need to be file-level dependencies. In fact since
> ExternalProject.cmake unconditionally uses the _EP_STAMP_DIR property of
> the target this will only work with ExternalProject targets at all (not
> even file level dependencies) which looks just wrong to me.

Here is the patch in case you are going to apply it.

Eike>From 788e6ca699ad1f48fbfa20d8d9d715b3fc732e46 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer 
Date: Mon, 28 Mar 2011 23:21:14 +0200
Subject: [PATCH] ExternalProject: make DEPENDS work with "normal" targets

ExternalProject_Add currently expects it's dependencies to have the property
_EP_STAMP_DIR set which is only true for other targets created by it. Now
it is first checked if that property is actually present and is only taken
as generated by ExternalProject if it does.
---
 Modules/ExternalProject.cmake |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 390b8f9..1aecbfa 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1266,8 +1266,14 @@ function(_ep_add_configure_command name)
   set(file_deps)
   get_property(deps TARGET ${name} PROPERTY _EP_DEPENDS)
   foreach(dep IN LISTS deps)
+# Find out if this dependency is itself an external target or not.
+# If it doesn't have _EP_STAMP_DIR we assume it's a normal target.
 get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
-list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+if(dep_stamp_dir)
+  list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+else(dep_stamp_dir)
+  list(APPEND file_deps ${dep})
+endif(dep_stamp_dir)
   endforeach()
 
   get_property(cmd_set TARGET ${name} PROPERTY _EP_CONFIGURE_COMMAND SET)
-- 
1.7.3.2



signature.asc
Description: This is a digitally signed message part.
___
Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake

Re: [CMake] How to add dependencies to ExternalProject_Add()

2011-03-28 Thread Rolf Eike Beer 
> On Mon, Mar 28, 2011 at 10:48 AM, Rolf Eike Beer  wrote:
>
>> > I came up with this simple diff which makes everything work smoothly
>> for
>> > me:
>>
>> This also works for me and should properly detect if the dependency is
>> itself an external target or not:
>>
>> diff --git a/Modules/ExternalProject.cmake
>> b/Modules/ExternalProject.cmake
>> index 3de6b7e..90b23ce 100644
>> --- a/Modules/ExternalProject.cmake
>> +++ b/Modules/ExternalProject.cmake
>> @@ -1261,8 +1261,14 @@ function(_ep_add_configure_command name)
>>set(file_deps)
>>   get_property(deps TARGET ${name} PROPERTY _EP_DEPENDS)
>>   foreach(dep IN LISTS deps)
>> +# Find out if this dependency is itself an external target or not.
>> +# If it doesn't have _EP_STAMP_DIR we assume it's a normal target.
>>  get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
>> -list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
>> +if(dep_stamp_dir)
>> +  list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
>> +else(dep_stamp_dir)
>> +  list(APPEND file_deps ${dep})
>> +endif(dep_stamp_dir)
>>endforeach()
>>
>>   get_property(cmd_set TARGET ${name} PROPERTY _EP_CONFIGURE_COMMAND
>> SET)

> I am not sure I like this patch, or not. I'm on the fence.
>
> I would recommend just using:
>
>   ExternalProject_Add(xyz ...)
>   add_dependencies(xyz mylib)
>
> to add non-ExternalProject dependencies. Just to make it clear that the
> dependencies are target-level dependencies and not file dependencies.

This only "sort-of" works. If I delete the library from the build try
(only the .so) and rebuild the external project does not get relinked.
Also the documentation of ExternalProject_Add says:

  [DEPENDS projects...]   # Targets on which the project depends

No word that these need to be file-level dependencies. In fact since
ExternalProject.cmake unconditionally uses the _EP_STAMP_DIR property of
the target this will only work with ExternalProject targets at all (not
even file level dependencies) which looks just wrong to me.

Eike
___
Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake

Re: [CMake] How to add dependencies to ExternalProject_Add()

2011-03-28 Thread David Cole 
On Mon, Mar 28, 2011 at 10:48 AM, Rolf Eike Beer  wrote:

> > I came up with this simple diff which makes everything work smoothly for
> > me:
>
> This also works for me and should properly detect if the dependency is
> itself an external target or not:
>
> diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
> index 3de6b7e..90b23ce 100644
> --- a/Modules/ExternalProject.cmake
> +++ b/Modules/ExternalProject.cmake
> @@ -1261,8 +1261,14 @@ function(_ep_add_configure_command name)
>set(file_deps)
>   get_property(deps TARGET ${name} PROPERTY _EP_DEPENDS)
>   foreach(dep IN LISTS deps)
> +# Find out if this dependency is itself an external target or not.
> +# If it doesn't have _EP_STAMP_DIR we assume it's a normal target.
>  get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
> -list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
> +if(dep_stamp_dir)
> +  list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
> +else(dep_stamp_dir)
> +  list(APPEND file_deps ${dep})
> +endif(dep_stamp_dir)
>endforeach()
>
>   get_property(cmd_set TARGET ${name} PROPERTY _EP_CONFIGURE_COMMAND SET)
>
> Eike
> ___
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.cmake.org/mailman/listinfo/cmake
>


I am not sure I like this patch, or not. I'm on the fence.

I would recommend just using:

  ExternalProject_Add(xyz ...)
  add_dependencies(xyz mylib)

to add non-ExternalProject dependencies. Just to make it clear that the
dependencies are target-level dependencies and not file dependencies.
___
Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake

Re: [CMake] How to add dependencies to ExternalProject_Add()

2011-03-28 Thread Rolf Eike Beer 
> I came up with this simple diff which makes everything work smoothly for
> me:

This also works for me and should properly detect if the dependency is
itself an external target or not:

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 3de6b7e..90b23ce 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1261,8 +1261,14 @@ function(_ep_add_configure_command name)
   set(file_deps)
   get_property(deps TARGET ${name} PROPERTY _EP_DEPENDS)
   foreach(dep IN LISTS deps)
+# Find out if this dependency is itself an external target or not.
+# If it doesn't have _EP_STAMP_DIR we assume it's a normal target.
 get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
-list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+if(dep_stamp_dir)
+  list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+else(dep_stamp_dir)
+  list(APPEND file_deps ${dep})
+endif(dep_stamp_dir)
   endforeach()

   get_property(cmd_set TARGET ${name} PROPERTY _EP_CONFIGURE_COMMAND SET)

Eike
___
Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake

[CMake] How to add dependencies to ExternalProject_Add()

2011-03-28 Thread Rolf Eike Beer 
I have an external project that should depend on a library I build in my
usual project. I do an export(TARGETS) on that library and pass that file
into the build of the external project and all goes fine.

But when I go to our build machine which will do "make -j 5" it breaks
because the external project get's build before the internal library.
Silly me ;)

So I did

ExternalProject_Add(external
DEPENDS mylib
   PREFIX "${CMAKE_CURRENT_BINARY_DIR}/build_external"
  ...
 )

Which of course doesn't work:

make[2]: *** No rule to make target `/mylib-done', needed by
`build_external/src/exernal-stamp/external-configure'.  Stop.

I wonder what is the supposed way to pass this sort of dependencies? Looks
like that one only works if the DEPENDS is itself an external project?

I came up with this simple diff which makes everything work smoothly for me:

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 3de6b7e..e4b7121 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -1261,8 +1261,12 @@ function(_ep_add_configure_command name)
   set(file_deps)
   get_property(deps TARGET ${name} PROPERTY _EP_DEPENDS)
   foreach(dep IN LISTS deps)
-get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
-list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+if(TARGET ${dep})
+  list(APPEND file_deps ${dep})
+else(TARGET ${dep})
+  get_property(dep_stamp_dir TARGET ${dep} PROPERTY _EP_STAMP_DIR)
+  list(APPEND file_deps ${dep_stamp_dir}${cfgdir}/${dep}-done)
+endif(TARGET ${dep})
   endforeach()

   get_property(cmd_set TARGET ${name} PROPERTY _EP_CONFIGURE_COMMAND SET)

Am I getting something totally wrong here? Should I open a bug report with
a properly annotated patch for inclusion?

Eike
___
Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake