WIP: Implement gz sdf as standalone executable #1465

scpeters · 2024-07-29T23:10:23Z

🎉 New feature

Summary

This is a work-in-progress to implement the gz sdf functionality in a standalone executable (similar to gazebosim/gz-transport#216). The standalone executable has added alongside the existing approach. Once the standalone executable is complete and functioning, the old approach should be removed.

This is not targeting the first release of Gazebo Ionic.

Test it

Use UNIT_gz_TEST as the guide.

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.

Signed-off-by: Steve Peters <[email protected]>

scpeters · 2024-10-03T18:37:08Z

this is in progress. as of d823820, the src/gz.cc and src/gz.hh files are duplicated into the src/cmd folder and a standalone executable is built and installed into libexec but not yet called by the src/cmd/cmdsdformat.in ruby file. At the time, I found it easier to test this way, but now that Ionic has been released, it may be a good time to update the ruby file to invoke the standalone executable (see cmdplugin.rb.in) so that UNIT_gz_TEST will invoke the standalone executable. That test is pretty thorough and should be able to guide most of the conversion effort

sauk2 · 2024-10-14T19:20:00Z

this is in progress. as of d823820, the src/gz.cc and src/gz.hh files are duplicated into the src/cmd folder and a standalone executable is built and installed into libexec but not yet called by the src/cmd/cmdsdformat.in ruby file. At the time, I found it easier to test this way, but now that Ionic has been released, it may be a good time to update the ruby file to invoke the standalone executable (see cmdplugin.rb.in) so that UNIT_gz_TEST will invoke the standalone executable. That test is pretty thorough and should be able to guide most of the conversion effort

@scpeters I raised a PR into this branch with the pending changes. If you could take a look at #1489

azeey · 2024-12-07T00:50:02Z

src/cmd/gz.cc

Not sure why we're adding these files when they're already in src.

I had initially added them separately while prototyping so that I could run the standalone executable side-by-side with the ruby script. My goal was to eventually match the file layout used by gz-transport with gz.* in src/cmd/:

https://github.com/gazebosim/gz-transport/tree/gz-transport14/src/cmd

Okay, moving them here makes sense to be consistent, but we should move gz_TEST.cc here as well then right?

Also, it would be best if we can git mv the files to preserve history.

* Fixed cmake to find source file for FrameSemantics * Updated exe location in ruby script * Add a dependency on gz-sdformat-sdf to the sdf_descriptions target * Updated cmd line arguments in executable tests * Added dummy flags to make tests pass --------- Signed-off-by: Saurabh Kamat <[email protected]> Signed-off-by: Addisu Z. Taddese <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]>

azeey

@scpeters I've merged #1489 into this PR. I've a few more comments, but I think this is almost there.

azeey · 2025-01-23T23:44:04Z

src/cmd/CMakeLists.txt

+      PROPERTIES
+      ENVIRONMENT
+        "GZ_CONFIG_PATH=${CMAKE_BINARY_DIR}/test/conf/$<CONFIG>"
+  #      "GZ_CONFIG_PATH=${CMAKE_BINARY_DIR}/test/conf/$<CONFIG>;LD_LIBRARY_PATH=\"$<TARGET_FILE_DIR:${PROJECT_LIBRARY_TARGET_NAME}>\":$ENV{LD_LIBRARY_PATH}"


Remove commented out code?

azeey · 2025-01-23T23:45:51Z

src/cmd/sdf_main.cc

+  app.add_flag("--force-version", "Use a specific library version.");
+  app.add_flag("--versions", "Show the available versions.");


Can you add a comment that these flags are actually handled by gz-tools?

azeey · 2025-01-23T23:46:58Z

src/cmd/gz.hh

+/// \brief External hook to execute 'gz sdf -k' from the command line.
+/// \param[in] _path Path to the file to validate.
+/// \return Zero on success, negative one otherwise.
+extern "C" SDFORMAT_VISIBLE int cmdCheck(const char *_path);


Now that we're calling these functions from C++, we don't need the extern "C" anymore.

azeey · 2025-01-23T23:47:30Z

src/cmd/gz.hh

+
+#include <sdf/sdf_config.h>
+#include "sdf/system_util.hh"
+


Add namespace sdf

azeey · 2025-01-23T23:50:22Z

src/cmd/gz.cc

Okay, moving them here makes sense to be consistent, but we should move gz_TEST.cc here as well then right?

Also, it would be best if we can git mv the files to preserve history.

azeey · 2025-01-23T23:50:40Z

src/cmd/sdf_main.cc

@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2021 Open Source Robotics Foundation


Suggested change

* Copyright (C) 2021 Open Source Robotics Foundation

* Copyright (C) 2025 Open Source Robotics Foundation

Implement gz sdf as standalone executable

d823820

Signed-off-by: Steve Peters <[email protected]>

github-actions bot added the 🏛️ ionic Gazebo Ionic label Jul 29, 2024

scpeters mentioned this pull request Jul 29, 2024

Consider binaries to implement the functionality offered by gz gazebosim/gz-tools#7

Open

scpeters changed the base branch from main to sdf15 October 3, 2024 18:14

Merge branch 'sdf15' into scpeters/gz_sdf_executable

ce14de5

Merge branch 'sdf15' into scpeters/gz_sdf_executable

aafefa1

azeey reviewed Dec 7, 2024

View reviewed changes

sauk2 and others added 2 commits January 23, 2025 17:40

Merge branch 'sdf15' into scpeters/gz_sdf_executable

f4fc058

azeey reviewed Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Implement gz sdf as standalone executable #1465

WIP: Implement gz sdf as standalone executable #1465

scpeters commented Jul 29, 2024

scpeters commented Oct 3, 2024

sauk2 commented Oct 14, 2024

azeey Dec 7, 2024

scpeters Dec 7, 2024

azeey Jan 23, 2025

azeey left a comment

azeey Jan 23, 2025

azeey Jan 23, 2025

azeey Jan 23, 2025

azeey Jan 23, 2025

azeey Jan 23, 2025

azeey Jan 23, 2025

		app.add_flag("--force-version", "Use a specific library version.");
		app.add_flag("--versions", "Show the available versions.");

		@@ -0,0 +1,199 @@
		/*
		* Copyright (C) 2021 Open Source Robotics Foundation

	* Copyright (C) 2021 Open Source Robotics Foundation
	* Copyright (C) 2025 Open Source Robotics Foundation


		#include <sdf/sdf_config.h>
		#include "sdf/system_util.hh"

WIP: Implement gz sdf as standalone executable #1465

Are you sure you want to change the base?

WIP: Implement gz sdf as standalone executable #1465

Conversation

scpeters commented Jul 29, 2024

🎉 New feature

Summary

Test it

Checklist

scpeters commented Oct 3, 2024

sauk2 commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment