gazebosim / gz-tools

Command line tools for the Gazebo libraries.
https://gazebosim.org
Apache License 2.0
15 stars 18 forks source link

Consider binaries to implement the functionality offered by `gz` #7

Open osrf-migration opened 4 years ago

osrf-migration commented 4 years ago

Original report (archived issue) by Carlos Agüero (Bitbucket: caguero, GitHub: caguero).


Right now, ign is a ruby script that offers several subcommands (topic, msg, etc.). Each subcommand is implemented by a function inside a versioned Ignition shared library. ign also keeps track of multiple versions of the same library. This approach has some issues, in particular we have detected:

  1. Difficulty for using gdb when running ign. Imagine for example running ign gazebo …
  2. Portability issues. E.g.: On Windows, there are ABI problems interacting from Ruby (compiled with MinGW) and an Ignition shared library (compiled with Visual Studio).

An idea for mitigating these issues might be to move the functionality from a shared library to a binary (e.g.: igntopic). In this case, ign should invoke the right external binary. The solution to (1) is to run gdb with the binary directly without using ign. (2) probably won’t happen again as we wouldn’t need to interact with a shared library anymore, just executing an external binary which looks less involved.

On the other hand, each Ignition library should do some refactor and expose a binary (or a collection of binaries) that capture all functionality to be executed from the command line.

chapulina commented 3 years ago

ign-tools itself should not be limited to loading shared libraries right now. This is being done at each library's Ruby file, for example cmdtransport.rb.in.

To use executables in ign-transport for example, one would need to:

scpeters commented 3 years ago

ign-tools itself should not be limited to loading shared libraries right now. This is being done at each library's Ruby file, for example cmdtransport.rb.in.

To use executables in ign-transport for example, one would need to:

  • Install executables that provide the command line functionality, like topic and service
  • Call those from the Ruby script instead of loading a shared lib
  • It may be interesting to move the command line argument parsing from Ruby to the executables

we are doing this in https://github.com/ignitionrobotics/ign-transport/pull/216 using the cli component of ign-utils

chapulina commented 3 years ago

Let's close this issue once all libraries have migrated to binaries.

chapulina commented 3 years ago
traversaro commented 3 years ago
2\. Portability issues. E.g.: On Windows, there are ABI problems interacting from Ruby (compiled with MinGW) and an Ignition shared library (compiled with Visual Studio).

I guess anyone that tried this on Windows know this, but just to record this for future readers: another problem that is present in Windows when using shell such as Command Prompt or Powershell is that bash-based shebangs do not work, so even if ign.rb is part of the path, just typing ign does not execute any command, and you need to run ruby %CONDA_PREFIX%\Library\bin\ign.rb or something like that.

traversaro commented 6 months ago
2\. Portability issues. E.g.: On Windows, there are ABI problems interacting from Ruby (compiled with MinGW) and an Ignition shared library (compiled with Visual Studio).

I guess anyone that tried this on Windows know this, but just to record this for future readers: another problem that is present in Windows when using shell such as Command Prompt or Powershell is that bash-based shebangs do not work, so even if ign.rb is part of the path, just typing ign does not execute any command, and you need to run ruby %CONDA_PREFIX%\Library\bin\ign.rb or something like that.

For historical reference, that was eventually fixed in https://github.com/gazebosim/gz-tools/pull/73 .

sauk2 commented 1 month ago

@azeey I would like to work on this issue!

scpeters commented 1 month ago

@azeey I would like to work on this issue!

As a starting point, I would recommend porting the fix from https://github.com/gazebosim/gz-plugin/pull/131 to other libraries that already have standalone executable commands like gz-transport and gz-launch. If the windows tests just work after that fix, I would say enable them, but if there's a bunch of failures I think it's worth merging the fix to the ruby file on its own.

A follow up task could be completing the conversion for gz sdf that is in progress in https://github.com/gazebosim/sdformat/pull/1465.

What do you think @azeey?