Skip to content
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

make installed code relocatable #490

Closed
dirk-thomas opened this Issue Jul 25, 2013 · 7 comments

Comments

Projects
None yet
2 participants
@dirk-thomas
Copy link
Member

dirk-thomas commented Jul 25, 2013

Installed code should not (or as few as possible) contain absolute path information. Currently a lot of code contains the CMAKE_INSTALL_PREFIX.

One goal would be to compile a workspace with DESTDIR=/tmp/destdir catkin_make_isolated --install and be able to build another workspace on top of that.
At best it should be possible to run everything from the installed workspace and run the tests of workspaces built on top.

The following things need to be updated to not use absolute paths:

  • env.sh references setup.sh, 9cfba6f
  • setup.SHELL references setup.sh, 9cfba6f
  • setup.sh references _setup_util.py,
    sourced shell scripts can not determine their own path therefore enable the user to override the default path via an environment variable, using CATKIN_SETUP_DIR, 9cfba6f
  • _setup_util.sh contains abs. path to prefix CPP,
    should be passed in from setup.sh, 9cfba6f
  • generated CMake config files, e.g. include directories, 74af67d
  • environment hooks,
    require context about their workspace from the outside, setup.sh need to provide that individually for each environment hook, using the environemt variable CATKIN_ENV_HOOK_WORKSPACE for that 44b3234
  • catkin_make_isolated must set the environment variable to override the base path when building with DESTDIR, 7282a4b

Changes required in other packages:

Message Generation:

  • the generated CMake files contain absolute paths to message/service files, ros/genmsg@cb45b13

All patches are committed to the relocatable branch of the respective repos. A file to checkout the modified repos using https://github.com/dirk-thomas/vcstool can be found here: https://gist.github.com/dirk-thomas/6092081

@ghost ghost assigned dirk-thomas Jul 25, 2013

dirk-thomas added a commit that referenced this issue Jul 26, 2013

dirk-thomas added a commit that referenced this issue Jul 26, 2013

dirk-thomas added a commit to ros/ros that referenced this issue Jul 26, 2013

dirk-thomas added a commit to ros/ros_comm that referenced this issue Jul 26, 2013

@herbrechtsmeier

This comment has been minimized.

Copy link

herbrechtsmeier commented Jul 29, 2013

@dirk-thomas Please correct the dynamic_reconfigure_BASE_DIR in extras.cmake.em of dynamic_reconfigure also.

dirk-thomas added a commit to ros/dynamic_reconfigure that referenced this issue Jul 29, 2013

@herbrechtsmeier

This comment has been minimized.

Copy link

herbrechtsmeier commented Jul 31, 2013

@dirk-thomas I have test the patches and they work. Nevertheless I would proposed to replace ${@PROJECT_NAME@_DIR}/../../../@(CATKIN_PACKAGE_BIN_DESTINATION) with a new global catkin variable ${@PROJECT_NAME@_BIN_DIR} which could be overwrite (passed) by the build environment to point to the native system root. Otherwise we get problems when somebody use the path for binaries instead of python scripts.

@dirk-thomas

This comment has been minimized.

Copy link
Member Author

dirk-thomas commented Jul 31, 2013

I am glad to hear that it works for your use case.

Regarding the variable: this variable should not be overwritten - not even for cross compilation. If a package requires compiled binaries to build itself / downstream stuff it needs to reference that using a separate variable pointing to a different directory. E.g. in the case where a package uses both (a Python script as well as a compiled binary) to build downstream stuff it will not have both of the executables in the same directory anyway.

Each package should consciously deal with this in its CMake extra file. catkin should not try to populate any package-specific variables. For the source/devel space catkin would not even be able to set a reasonable directory since it just does not know where the binaries in the package are.

@herbrechtsmeier

This comment has been minimized.

Copy link

herbrechtsmeier commented Aug 1, 2013

In that case the binaries need to use the find_binary function to get the real path of the binary.

As the long time goal it would be nice if catkin and the CMake extra files always use the find_binary function to get the real path of all executed scripts and binaries. This enables the cross build environment to use all binaries and as well scripts from the native system root if it doesn't install any binaries and scripts into the cross system root.

@dirk-thomas

This comment has been minimized.

Copy link
Member Author

dirk-thomas commented Aug 3, 2013

For packages which do not generate compiled binaries it can stay the way as it is with these patches. I don't see a reason to change that even in the long term.

For compiled binaries we should indeed use the find_* CMake functions. But that should handled by the first package which actually requires that. Currently no (wet) package requires compiled binaries during the build (as far as I know) and that was an important goal of the new buildsystem.

I will go ahead and merge all above mentioned patches. Releases for the separate packages will follow when ever each package has "enough" changes accumulated.

dirk-thomas added a commit to ros/ros that referenced this issue Aug 3, 2013

@herbrechtsmeier

This comment has been minimized.

Copy link

herbrechtsmeier commented Aug 3, 2013

As long as the build machine (native system root) and cross compiled machine (cross system root) use the same python version it doesn't matter which python script is used and its a design decision.

@dirk-thomas thanks for your work. We will update our recipes in meta-ros after you have apply the different patches.

dirk-thomas added a commit that referenced this issue Aug 3, 2013

Merge pull request #494 from ros/relocatable
make catkin packages relocatable (#490)

dirk-thomas added a commit to ros/genmsg that referenced this issue Aug 3, 2013

Merge pull request #32 from ros/relocatable
resolve path of message file and include dirs in installspace at runtime (ros/catkin#490)

dirk-thomas added a commit to ros/gencpp that referenced this issue Aug 3, 2013

Merge pull request #5 from ros/relocatable
resolve gencpp path in installspace at runtime (ros/catkin#490)

dirk-thomas added a commit to ros/genlisp that referenced this issue Aug 3, 2013

Merge pull request #5 from ros/relocatable
resolve genlisp path in installspace at runtime (ros/catkin#490)

dirk-thomas added a commit to ros/genpy that referenced this issue Aug 3, 2013

Merge pull request #16 from ros/relocatable
resolve genpy path in installspace at runtime (ros/catkin#490)

dirk-thomas added a commit to ros/ros that referenced this issue Aug 3, 2013

Merge pull request #29 from ros/relocatable
resolve rosunit path in installspace at runtime without find_program(), update env hooks to use runtime context (ros/catkin#490)

dirk-thomas added a commit to ros/ros_comm that referenced this issue Aug 3, 2013

Merge pull request #268 from ros/relocatable
resolve roslaunch/rostest paths in installspace at runtime without find_program() (ros/catkin#490)

dirk-thomas added a commit to ros/dynamic_reconfigure that referenced this issue Aug 3, 2013

Merge pull request #14 from ros/relocatable
resolve dynamic_reconfigure path in installspace at runtime (ros/catkin#490)

dirk-thomas added a commit to ros/ros that referenced this issue Aug 4, 2013

moesenle added a commit to ros/roslisp that referenced this issue Aug 5, 2013

Merge pull request #10 from ros/relocatable
resolve roslisp path in installspace at runtime (ros/catkin#490)
@dirk-thomas

This comment has been minimized.

Copy link
Member Author

dirk-thomas commented Aug 5, 2013

All repos listed in this ticket has been updated to be relocatable - therefore closing this ticket.

In case other repos use CMAKE_INSTALL_PREFIX in the CMake extra files please fill further tickets against these repos directly.

@dirk-thomas dirk-thomas closed this Aug 5, 2013

severin-lemaignan pushed a commit to severin-lemaignan/robotpkg that referenced this issue Aug 18, 2014

Anthony Mallet
[middleware/ros-genmsg] Update to indigo (0.5.3)
Changes since 0.4.20:

0.5.3 (2014-07-10)
------------------
* escape messages to avoid CMake warning (`#49
<https://github.com/ros/genmsg/issues/49>`_)

0.5.2 (2014-05-07)
------------------
* refactor to generate pkg-msg-paths.cmake via configure_file() instead of empy
(`#43 <https://github.com/ros/genmsg/issues/43>`_)
* fix python 3 compatibility (`#45 <https://github.com/ros/genmsg/issues/45>`_)
* remove debug message introduced in 0.5.1 (`#42
<https://github.com/ros/genmsg/issues/42>`_)

0.5.1 (2014-03-04)
------------------
* add check for changed message dependencies (`#41
<https://github.com/ros/genmsg/issues/41>`_)

0.5.0 (2014-02-25)
------------------
* remove usage of debug_message() (`#40
<https://github.com/ros/genmsg/issues/40>`_)

0.4.24 (2014-01-07)
-------------------
* python 3 compatibility (`#36 <https://github.com/ros/genmsg/issues/36>`_,
`#37 <https://github.com/ros/genmsg/issues/37>`_)
* add support for ROS_LANG_DISABLE env variable (`ros/ros#39
<https://github.com/ros/ros/issues/39>`_)
* fix installation of __init__.py from devel space (`#38
<https://github.com/ros/genmsg/issues/38>`_)

0.4.23 (2013-09-17)
-------------------
* fix installation of __init__.py file for packages where name differs from
project name (`#34 <https://github.com/ros/genmsg/issues/34>`_)
* rename variable 'config' to not collide with global variable (`#33
<https://github.com/ros/genmsg/issues/33>`_)
* fix service files variable to only contain package relative paths (`#32
<https://github.com/ros/genmsg/issues/32>`_)

0.4.22 (2013-08-21)
-------------------
* make genmsg relocatable (`ros/catkin#490
<https://github.com/ros/catkin/issues/490>`_)
* add warning in case generate_messages() is invoked without any messages and
services (`#31 <https://github.com/ros/genmsg/issues/31>`_)
* check if files have been generated before trying to install them (`#31
<https://github.com/ros/genmsg/issues/31>`_)

0.4.21 (2013-07-03)
-------------------
* check for CATKIN_ENABLE_TESTING to enable configure without tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.