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

Set gz tool name via GZ_CLI_EXECUTABLE_NAME #3368

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

scpeters
Copy link
Member

🎉 New feature

Enable co-installability of gazebo-classic and gz-sim by allowing gz executable to be renamed

Summary

Currently gazebo-classic and gz-tools cannot be installed side-by-side since both install a gz executable. This pull request adds a cmake variable to customize the name of the gazebo-classic CLI tool to allow a customized package with a different tool name.

Test it

To rename gz to gz11 (for example), invoke cmake with -DGZ_CLI_EXECUTABLE_NAME=gz11

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@scpeters scpeters requested a review from j-rivero February 13, 2024 05:40
@scpeters scpeters requested a review from traversaro as a code owner February 13, 2024 05:40
@@ -24,6 +24,10 @@ if (HAVE_DART)
link_directories(${DART_LIBRARY_DIRS})
endif()

if ("${GZ_CLI_EXECUTABLE_NAME}" STREQUAL "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does make sense to convert this to an option?

iff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
index a0ec46236e..4bbf71ddf8 100644
--- a/tools/CMakeLists.txt
+++ b/tools/CMakeLists.txt
@@ -24,9 +24,7 @@ if (HAVE_DART)
   link_directories(${DART_LIBRARY_DIRS})
 endif()
 
-if ("${GZ_CLI_EXECUTABLE_NAME}" STREQUAL "")
-  set(GZ_CLI_EXECUTABLE_NAME "gz")
-endif()
+option(GZ_CLI_EXECUTABLE_NAME "Name for the cli tool (default gz)" "gz")
 
 if(NOT WIN32)
   # gz_TEST and gz_log_TEST use fork(), that is not available on Windows

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had actually started with an option and then found out that options are only boolean

so I switched to this approach

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment, looks good.

@scpeters scpeters merged commit 6a9d39c into gazebo11 Feb 13, 2024
8 of 10 checks passed
@scpeters scpeters deleted the scpeters/cmake_gz_cli_executable_name branch February 13, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants