catkin / catkin_tools

Command line tools for working with catkin
Apache License 2.0
164 stars 146 forks source link

build: can't find download script at build time #102

Closed trainman419 closed 10 years ago

trainman419 commented 10 years ago

(This works with the traditional catkin_make)

I have a package which contains python scripts which can download large data files at build time, and I use it from other packages that need this, such as for compiled perception descriptors and for large graphical textures. (catkin has a similar set of macros, but they're only usable for downloading test data).

Following the example from catkin, we have the following cmake snippet in our CMakeLists.txt:

set(stdr_common_EXTRAS_DIR ${PROJECT_SOURCE_DIR}/cmake PARENT_SCOPE)

And our CFG_EXTRAS file has:

if(NOT stdr_common_EXTRAS_DIR)
  set(stdr_common_EXTRAS_DIR ${stdr_common_DIR})
endif()

The result is that, in builds with catkin_make, stdr_common_EXTRAS_DIR is set to the source directory when building in devel space, and is set to the share directory when the package is installed.

We then have a macro which other packages can use to call the download utility program at build time:

add_custom_target(${ARG_TARGET} COMMAND ${stdr_common_EXTRAS_DIR}/download_checkmd5.py ${url} ${output} ${ARG_MD5} ${required} VERBATIM)

When attempting the same build with catkin build, I get the following error:

make[2]: /home/hendrix/hydro/junior/devel/stdr_common/share/stdr_common/cmake/download_checkmd5.py: Command not found

It looks like stdr_common_EXTRAS_DIR isn't being set properly because it isn't used from the same build space. Is there a better way to do this with catkin_tools or a recommended workaround?

trainman419 commented 10 years ago

I was able to patch this by adding more logic to the detection of stdr_common_EXTRAS_DIR in my CFG_EXTRAS file:

if(NOT stdr_common_EXTRAS_DIR)
  if(stdr_common_SOURCE_PREFIX)
    set(stdr_common_EXTRAS_DIR "${stdr_common_SOURCE_PREFIX}/cmake")
  else()
    set(stdr_common_EXTRAS_DIR ${stdr_common_DIR})
  endif()
endif()

Does that seem like a reasonable solution, or is there a better way?

wjwwood commented 10 years ago

I think the appropriate thing to do is to have two cfg_extras files, one for devel space and one for install space, example:

https://github.com/ros/cmake_modules/tree/0.3-devel/cmake

Or a single cfg_extras file with conditional logic for each case:

https://github.com/ros/ros/blob/indigo-devel/core/roslib/cmake/roslib-extras.cmake.em

The latter would look something like this for you:

# generated from stdr_common/cmake/stdr_common-extras.cmake.em

@[if DEVELSPACE]@
# set path to source space
set(stdr_common_EXTRAS_DIR "@(CMAKE_CURRENT_SOURCE_DIR)/cmake")
@[else]@
# set path to installspace
set(stdr_common_EXTRAS_DIR "${stdr_common_DIR}")
@[end if]@

Both of these approaches achieve the same thing as your patch, but I think they are more obvious to read.

wjwwood commented 10 years ago

Can this be closed?

trainman419 commented 10 years ago

Yes; that approach works for me. Thanks!

wjwwood commented 10 years ago

Cool, thanks for closing.