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

Relocatable Gazebo installation #626

Open
stefanbuettner opened this issue Feb 12, 2021 · 14 comments
Open

Relocatable Gazebo installation #626

stefanbuettner opened this issue Feb 12, 2021 · 14 comments
Labels
enhancement New feature or request help wanted We accept pull requests!

Comments

@stefanbuettner
Copy link

Desired behavior

Being able to move an Ignition installation which was compiled from source to another destination.

I want to package the Ignition projects into Conan packages. The relocatability is needed for pre-built conan packages, because they are installed into the user's home folders. This changes the absolute installation path.

Alternatives considered

  1. Searched for already existing conan packages. There is only ignition-cmake and ignition-math, probably from this effort which seems to be abandoned.
  2. Tried to fix it using Conan's deploy() method to remap the paths in the yaml config files.
  3. Don't pre-build the packages. We've tried that. But it's annoying if a user has to wait for 30mins for all the projects to download and compile while the workstation becomes virtually unusable due to the high CPU and memory usage.

Problem description

The problem seems to be hard coded paths in various locations.

Package config files

The library paths are hard coded into the packages' configuration files. E.g. in share/ignition/msgs5.yaml

--- # Subcommands available inside ignition-transport.
format: 1.0.0
library_name: ignition-msgs
library_version: 5.3.0
library_path: /home/sb/.conan/data/ignition-msgs/5.3.0/ar/stable/package/75506859fd8f6284fac51eeb3592dfae9546ff1c/lib/ruby/ignition/cmdmsgs5
commands:
    - msg   : Print information about messages.
---

The source code

E.g. the default location of the config files is hard coded:
Defined in the config.hh
And used as a sole source for the installedConfig in the Gui, Ign, and the server.

There might be additional issues elsewhere, which did not pop up, yet, e.g. cmake issues which were addressed for example in ignition-cmake, ignition-tools, and gazebo.

Implementation suggestion

  • Use additional environment variables which can override the hard coded paths.
  • Change the paths to package/installation relative paths and add mechanisms to determine the final paths during runtime.

Additional context

I'm working on conan packages for the Ignition projects in our company. Building the individual projects went fine. However, bringing them together, did not.

The two approaches I tried:

Build every package independently

  1. Create a conan package for every ignition project (cmake, math, msgs, etc.).
  2. Bundle them in a meta conan package
    From the installation instructions it seems that the packages need to be packaged into one combined destination (--merge-install). That's when I tried the second approach (build everything in one package). Due to the problems I encountered there, I came back to this approach because I actually prefer it because it reflects the modularity of the Ignition project and makes updates in the individual packages easier.

Build everything in one package

I tried to use colcon in one single conan package. Works, except that one uses the very good cmake support of Conan and that there is no equivalent colcon support. I didn't follow the idea of passing the appropriate cmake arguments to colcon via the --cmake-args, because I went back to the other approach.

@stefanbuettner stefanbuettner added the enhancement New feature or request label Feb 12, 2021
@traversaro
Copy link
Contributor

Change the paths to package/installation relative paths and add mechanisms to determine the final paths during runtime.

If we want to go down in this direction (that I indeed think is quite flexible), an interesting resource is:

The library paths are hard coded into the packages' configuration files. E.g. in share/ignition/msgs5.yaml

Related downstream issue:

Just FYI @JShep1 conda is dealing automatically in package post-processing to fix some of the issues mentioned by @stefanbuettner, see https://docs.conda.io/projects/conda-build/en/latest/resources/make-relocatable.html for the details. However, fixing the problems directly in the libraries as proposed by @stefanbuettner would mean that relocation would work correctly also for package managers that don't have powerful automatic handling of relocatability as conda, i.e. conan or vcpkg.

@chapulina chapulina added the help wanted We accept pull requests! label Feb 12, 2021
@stefanbuettner
Copy link
Author

Thanks for the fast reply, @traversaro.

I don't know which scope you have in mind for a solution. Fixing the issues on a demand basis or realizing a resource location system for ruby and c++?

For fixing the issue regarding the yaml config files which are used by ruby scripts, the approach <yaml-config-path> + <tool-path-relative-to-yaml-file> would do it, right? tool-path-relative-to-yaml-file would be saved in the yaml config. Or is this too simple?

Regarding the defaultConfig which is copied in c++ code, additional environment variables could also do the trick. E.g.

  1. Use the hard coded paths as default
  2. If some env var is set, use this as package/shared data/whatever root and compute the resource path
  3. If path cannot be found, raise an error mentioning the env var.

Yet, an approach which works without environment variables would be more convenient. But as far as I understood, Resourceful only supports Linux and MacOS, not Windows (because binreloc only supports these). Extending it, as suggested by Resourceful's README, could be an option, even though it doesn't sound straightforward. Gazebo is officially supporting Windows, isn't it?

@traversaro
Copy link
Contributor

traversaro commented Feb 15, 2021

I don't know which scope you have in mind for a solution. Fixing the issues on a demand basis or realizing a resource location system for ruby and c++?

To be completely honest, I did not have anything in mind, as personally speaking nowadays for platforms in which repeatability is important I am mostly focusing on conda, that handles this issue automatically. I provided just a few links that could be helpful for the discussion. However, I will comment on what for me could be reasonable direction, hopefully to help the work of ignition maintainers.

For fixing the issue regarding the yaml config files which are used by ruby scripts, the approach + would do it, right? tool-path-relative-to-yaml-file would be saved in the yaml config. Or is this too simple?

I think that should work fine, unless I am missing something on the Ruby side. Probably it would be important to make sure that the tool-path-relative-to-yaml-file is not hardcoded but rather automatically configured by the build tool (similar to what done in gazebosim/gz-cmake#129 (comment) for pkg-config .pc files). This would probably not work if the tool and the yaml file on Windows are installed on different drives, but I think this is really a niche use case that can be safely ignored.

Regarding the defaultConfig which is copied in c++ code, additional environment variables could also do the trick. E.g.

Yes, I think that using environment variables is the strategy that Gazebo historically has used (and that is used in Ignition has well), and definitely the simplest solution. I linked the Morgan's presentation because it explains a lot of aspects that I ignored on relocatability and I found it cool, but not relying on environment variables may be more complex then it seems. For example, probably it would not work at all if libraries are compiled as static, that is a common workflow when C++ libraries are used for self-contained applications.

@traversaro
Copy link
Contributor

traversaro commented Nov 15, 2022

I had the problem of making libraries relocatable in some other projects, so I created a simple CMake/C++ module to handle this : https://github.com/ami-iit/reloc-cpp . Ideally, we could use that or something similar to start to substitute any direct use of CMAKE_INSTALL_PREFIX in Gazebo libraries, even if probably we also need to understand how to handle this in bazel.

@mjcarroll
Copy link
Contributor

Is it just the generated files that cause relocatability issues? If that's the case, maybe those things are better handled with saner defaults that can be overridden by environment variables?

@traversaro
Copy link
Contributor

Is it just the generated files that cause relocatability issues?

The install prefix used at build time is hardcoded in:

  • All the generated config.hh file
  • All the generated <proj><projnum>.yaml files that are used by gz-tools
  • All the binary code that included and used the macros defined in config.hh

From the Windows/Conda point of view, the most problematic is the last type of hardcoding, as it is the one that can't be easily patched once the package has been built. I am not sure what you mean by "saner defaults", but yes, in theory if all the behaviour could be overridden by environment variables we could use environment variables. However, this is not the case at the moment, and the high number of env variables from different projects with slightly different semantics (see conda-forge/libignition-gazebo-feedstock#52 (comment)) does not make this an easy task.
Anyhow, strictly speaking having a way to query CMAKE_INSTALL_PREFIX at runtime for shared libraries is not an alternative to environment variables, as environment variables may be necessary in the following cases:

  • For resources/plugins installed by downstream projects in an installation prefix different from the one used by Gazebo Sim
  • For static libraries, in which it is not possible to query the CMAKE_INSTALL_PREFIX at runtime.

@traversaro
Copy link
Contributor

I started working on the binary relocatability aspect, this are the first (working) commits:

Some projects (gz-sensors, gz-fuel_tools, gz-launch) are still missing, but the overall logic can be already seen in the changes for these projects, so comments are welcome.

@traversaro
Copy link
Contributor

traversaro commented Jan 16, 2023

I started working on the binary relocatability aspect, this are the first (working) commits:

* [traversaro/ign-cmake@746a2f0](https://github.com/traversaro/ign-cmake/commit/746a2f04fee37885f99ab57a148ae9046904f712)

* [traversaro/ign-physics@f79843b](https://github.com/traversaro/ign-physics/commit/f79843b2b842ffbd7a7f69d206e4af34b52b9006)

* [traversaro/ign-rendering@b8dccb3](https://github.com/traversaro/ign-rendering/commit/b8dccb323584d386edd5580c1d614835cac8428a)

* [traversaro@0ac1a4e](https://github.com/traversaro/ign-gazebo/commit/0ac1a4ebb61bdae808e7eb02312d7d1403b5c91a)

Some projects (gz-sensors, gz-fuel_tools, gz-launch) are still missing, but the overall logic can be already seen in the changes for these projects, so comments are welcome.

I iterated a bit on this changes, and I opened the first two PRs:

@traversaro
Copy link
Contributor

Note to self: I forgot sdformat, we can do that after Harmonic is released.

@traversaro
Copy link
Contributor

traversaro commented Aug 30, 2023

Hi @MirkoFerrati, long time no see!

I saw you are working on https://github.com/canonical/gazebo_snap, so I am mentioning you here as you may be interested in this discussion, just in case you never saw it.

@MirkoFerrati
Copy link

Hey @traversaro ! Previously our snapcraft was replacing the needed paths directly in the source code using sed during the snap creation process.
When moving to Fortress I realized most of the harcoded paths in the source code were supporting an additional env-var so in the end this is what I ended up doing.
https://github.com/canonical/gazebo_snap/blob/2ec189ac7c8b964d97e4f79e529740f0db96d51e/snap/snapcraft.yaml#L213-L232
I probably used the changes mentioned here, didn't I? If yes, I am glad those changes were made! :)
Anything I can help with?

@traversaro
Copy link
Contributor

The idea of this issue is basically to avoid the need to define any env variable to relocate the gz-* packages, the packages would be relocatable out of the box if the libraries are compiled as shared and GZ_ENABLE_RELOCATABLE_INSTALL CMake option is enabled. This is done in part for convenience and in part because the env variable do not permit to override all hardcoded variables (see conda-forge/libignition-gazebo-feedstock#52 (comment), in particular GZ_SIM_GUI_CONFIG_PATH and GZ_SIM_SERVER_CONFIG_PATH cannot be overrided by env variables, unless I got something wrong).

However, if the existing environment variable override mechanism works for the snap, that's great, probably you do not need anything new!

@MirkoFerrati
Copy link

Indeed I just remembered I could not remove these 2 lines https://github.com/canonical/gazebo_snap/blob/9dbe05ede2530ef0ea43b69b1581a1d0d64b871e/scripts/override_pull.sh#L20-L21
I'll update my PR as soon as the new mechanism works, it will clean up a lot of the snapcraft.yaml!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted We accept pull requests!
Projects
None yet
Development

No branches or pull requests

5 participants