Open stefanbuettner opened 3 years ago
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.
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.
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?
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 https://github.com/ignitionrobotics/ign-cmake/pull/129#discussion_r529025102 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.
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.
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?
Is it just the generated files that cause relocatability issues?
The install prefix used at build time is hardcoded in:
config.hh
file<proj><projnum>.yaml
files that are used by gz-toolsconfig.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 https://github.com/conda-forge/libignition-gazebo-feedstock/pull/52#issuecomment-1285233271) 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:
CMAKE_INSTALL_PREFIX
at runtime.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.
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:
Note to self: I forgot sdformat, we can do that after Harmonic is released.
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.
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?
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 https://github.com/conda-forge/libignition-gazebo-feedstock/pull/52#issuecomment-1285233271, 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!
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!
Missing libraries for the binary relocatability (ignoring textual non-relocatable files)
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
ignition-cmake
andignition-math
, probably from this effort which seems to be abandoned.deploy()
method to remap the paths in the yaml config files.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
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
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
--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.