Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Updating ruby script to use standalone executable #1489

Merged
merged 12 commits into from
Jan 23, 2025

Conversation

sauk2
Copy link
Contributor

@sauk2 sauk2 commented Oct 14, 2024

🎉 New feature

Related to gazebosim/gz-tools#7

Summary

Updated the ruby script to use the standalone executable.

I also tried to resolve the issue of building the package because of the FrameSemantics class. You can check the error in ci of your branch.

I updated the CMakeLists and am not sure if the following is the right approach ...

  add_library(gz STATIC gz.cc ../FrameSemantics.cc)

Test it

Test any of the SDF commands

  gz sdf --help

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.

sauk2 and others added 4 commits December 6, 2024 18:12
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey azeey force-pushed the gz_sdf_executable branch from 2be0d2d to 615048c Compare December 7, 2024 00:12
azeey added 2 commits December 6, 2024 18:23
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@azeey
Copy link
Collaborator

azeey commented Jan 7, 2025

@sauk2 have you had a chance to push this further?

sauk2 added 2 commits January 9, 2025 01:05
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
@sauk2
Copy link
Contributor Author

sauk2 commented Jan 8, 2025

@sauk2 have you had a chance to push this further?

@azeey Do take a look, I have made some changes to the tests.

src/gz_TEST.cc Outdated
{
return " --force-version " + std::string(SDF_VERSION_FULL);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason for removing this? The --foce-version argument is handled by gz-tools (see https://github.com/gazebosim/gz-tools/blob/2b228e5b956f1e966053dd860374670573580b41/src/gz.in#L244). We need to make sure it still works with the change to using executables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I added it back and the tests should pass now!

sauk2 added 2 commits January 9, 2025 23:41
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
@sauk2 sauk2 requested a review from azeey January 9, 2025 16:12
--inertial-stats
-h --help
--force-version
--versions
-v --version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert?

Copy link
Contributor Author

@sauk2 sauk2 Jan 11, 2025

Choose a reason for hiding this comment

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

The following test failed when --force-version and --versions were kept in.

sdformat/src/gz_TEST.cc

Lines 2071 to 2073 in 181c0ef

/// \brief Check help message and bash completion script for consistent flags
TEST(HelpVsCompletionFlags, GZ_UTILS_TEST_DISABLED_ON_WIN32(SDF))
{

I noticed those tags were removed in bash_completion of gz-transport so I did the same here. Let me know what you think.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, I think we'll need to keep it here. Otherwise, it would not show up in the bash completions. I think it'd be better to modify the HelpVsCompletionFlags test to make it pass in this case.

Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
@sauk2 sauk2 requested a review from azeey January 11, 2025 18:21
Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Other than the --force-versions issue, it LGTM!

--inertial-stats
-h --help
--force-version
--versions
-v --version
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, I think we'll need to keep it here. Otherwise, it would not show up in the bash completions. I think it'd be better to modify the HelpVsCompletionFlags test to make it pass in this case.

Signed-off-by: Saurabh Kamat <kamatsaurabh01@gmail.com>
@sauk2
Copy link
Contributor Author

sauk2 commented Jan 22, 2025

Other than the --force-versions issue, it LGTM!

Added dummy flags in the executable to make that HelpVsCompletionFlags test pass.

@azeey azeey merged commit 891e490 into gazebosim:scpeters/gz_sdf_executable Jan 23, 2025
7 checks passed
@sauk2 sauk2 deleted the gz_sdf_executable branch February 6, 2025 14:17
@sauk2 sauk2 mentioned this pull request Feb 25, 2025
9 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants