Improve ign tool support on macOS

[scpeters]

The ign gazebo command does not have full support on macOS (see #25 and #44). If System Integrity Protection is not disabled, the system version of ruby at /usr/bin/ruby cannot be used, though the brew version of ruby can be used as a workaround. Additionally, the GUI interface window does not work on macOS. This pull request enables UNIT_ign_TEST on macOS and fixes it to use the brew version of ruby (which was added as a dependency to the homebrew formulae in osrf/homebrew-simulation#1221). It also moves the configuration of environment variables from the test to the cmake code.

I've also added some extra console error messages when macOS is detected (from the .dylib suffix on library files):

$ ign gazebo
Library error for [/Users/ignition/ws/install/ignition-gazebo4/lib/libignition-gazebo4-ign.4.0.0.dylib]: can't load /Users/ignition/ws/install/ignition-gazebo4/lib/libignition-gazebo4-ign.4.0.0.dylib

If this script was executed with /usr/bin/ruby, this error may be caused by
macOS System Integrity Protection. One workaround is to use a different
version of ruby, for example:
    brew install ruby
and add the following line to your shell profile:
    export PATH=/usr/local/opt/ruby/bin:$PATH
If you are using a colcon workspace, please ensure that the setup script
has properly set the DYLD_LIBRARY_PATH environment variables.

# explicitly invoking with brew's ruby and without -g or -s
$ /usr/local/opt/ruby/bin/ruby $(which ign) gazebo
`ign gazebo` currently only works with the -s argument on macOS.

# explicitly invoking with brew's ruby and with -g
$ /usr/local/opt/ruby/bin/ruby $(which ign) gazebo -g
`ign gazebo` currently only works with the -s argument on macOS.

[scpeters]

ign tests passing on macOS: https://build.osrfoundation.org/job/ignition_gazebo-ci-pr_any-homebrew-amd64/4218/testReport/(root)/CmdLine/

[chapulina]

Great to give more direction to macOS users!

[chapulina]

Interesting, I don't think we've been setting environment variables for tests from CMake.

[scpeters]

I learned it from Dirk; it's not universal, but we have done it in a few places:

• https://github.com/ignitionrobotics/ign-cmake/blob/ignition-cmake2_2.6.0/examples/CMakeLists.txt#L180-L186
• https://github.com/osrf/sdformat/blob/sdformat10_10.0.0/cmake/SDFUtils.cmake#L192-L197
• https://github.com/osrf/gazebo/blob/gazebo11_11.3.0/cmake/GazeboTestUtils.cmake#L69-L77

I think it's a bit cleaner to set them from cmake. One drawback is that executing the binaries directly (like ./UNIT_ign_TEST) may not work, but it's easy enough to use ctest -R UNIT_ign_TEST -VV once you know to do so

[chapulina]

One drawback is that executing the binaries directly (like ./UNIT_ign_TEST) may not work

Oh no, that sounds like a regression. We have that workflow documented in a few places, it would be unfortunate not to support it anymore.

[chapulina]

As I mentioned during the meeting, I don't see the advantage of moving the environment variables to CMake.

It prevents a common workflow, and it hides the environment configuration from anyone who's debugging the tests or looking into them to understand how the code works. Especially for variables like IGN_GAZEBO_SYSTEM_PLUGIN_PATH, which are set by the user at runtime, I think it's valuable to explicitly set them during the tests.

But I'm not going to block this PR on this, I'll leave it up to you.

[scpeters]

I pointed to 3 examples in our code where it's already being used, so this isn't a completely new thing, but you're right that the docs need to be updated to handle this case. I'll merge this then open a pull request to update the docs

[chapulina]

As I mentioned during the meeting, I don't see the advantage of moving the environment variables to CMake.

It prevents a common workflow, and it hides the environment configuration from anyone who's debugging the tests or looking into them to understand how the code works. Especially for variables like IGN_GAZEBO_SYSTEM_PLUGIN_PATH, which are set by the user at runtime, I think it's valuable to explicitly set them during the tests.

But I'm not going to block this PR on this, I'll leave it up to you.

[chapulina]

@scpeters , I think you moved this back to "In Progress" because a resolution to gazebosim/gz-tools#7 may help here, right? I think we could get this in as is, and make those improvements later if we can. Let me know if I misunderstood something. I think this is a good intermediate state and better than the current state without it.