ign -> gz Namespace Migration : gz-sim #1496

methylDragon · 2022-05-18T09:43:18Z

methylDragon · 2022-05-23T18:37:56Z

A double free issue appears for new (compared to main) from this PR. But it might be due to/related to #1443

methylDragon · 2022-05-23T22:33:35Z

Two gz-sim tests are failing, but one necessitates no further actions:

101 - INTEGRATION_examples_build (Failed)
195 - INTEGRATION_log_system (Failed)

101 git clones gz-sensors's main branch as part of the test, which is still using ignition::msgs, causing a failing build, it should resolve itself once the gz-sensors namespace migration PR is merged to main

195: Trying to playback unsupported message type ignition.msgs.SerializedStateMap
195 needs an update of the .tlog, since the log is using ignition.msgs when the stack is meant to use gz.msgs

chapulina

Partial review

Migration.md

Changelog.md

Migration.md

src/SystemLoader.cc

include/gz/sim/Util.hh

tutorials/migrating_ardupilot_plugin.md

tutorials/migration_plugins.md

src/SystemInternal.hh

src/cmd/cmdgazebo.rb.in

Signed-off-by: methylDragon <[email protected]>

chapulina

include/gz/sim/config.hh.in

include/ignition/gazebo/config.hh

src/gui/plugins/entity_context_menu/EntityContextMenuPlugin.qml

src/gui/resources/GazeboDrawer.qml

chapulina · 2022-05-26T22:12:39Z

examples/worlds/actor.sdf

@@ -6,20 +6,20 @@
  <world name="actors">
    <plugin
      filename="ignition-gazebo-physics-system"
-      name="ignition::gazebo::systems::Physics">
+      name="gz::sim::systems::Physics">


I'm still compiling this, so just leaving a note here that we should check if ignition::gazebo still works.

Ok, I just tested it and it doesn't work. But the fix is simple, we need to add aliases like this to every system plugin:

diff --git a/src/systems/physics/Physics.cc b/src/systems/physics/Physics.cc index b9df0909b..451773597 100644 --- a/src/systems/physics/Physics.cc +++ b/src/systems/physics/Physics.cc @@ -3604,3 +3604,5 @@ IGNITION_ADD_PLUGIN(Physics, Physics::ISystemUpdate) IGNITION_ADD_PLUGIN_ALIAS(Physics, "gz::sim::systems::Physics") +// Deprecated, remove on version 8 +IGNITION_ADD_PLUGIN_ALIAS(Physics, "ignition::gazebo::systems::Physics")

You can check it worked either by changing a world to load the ignition::gazebo version of the plugin, or with the ign plugin CLI:

$ ign plugin -i -v -p install/lib/libignition-gazebo7-physics-system.so Loading plugin library file [install/lib/libignition-gazebo7-physics-system.so] Loader State Known Interfaces: 4 gz::sim::v7::ISystemUpdate gz::sim::v7::System gz::sim::v7::ISystemReset gz::sim::v7::ISystemConfigure Known Plugins: 1 [gz::sim::v7::systems::Physics] has 2 aliases: [gz::sim::systems::Physics] [ignition::gazebo::systems::Physics] implements 4 interfaces: gz::sim::v7::ISystemConfigure gz::sim::v7::ISystemReset gz::sim::v7::ISystemUpdate gz::sim::v7::System

Note the alias

5c21f04
:>

The specific steps I did are in the commit message

test/integration/deprecated_TEST.cc

Signed-off-by: methylDragon <[email protected]>

src/gui/Gui.cc

chapulina · 2022-05-27T22:43:22Z

examples/worlds/actor.sdf

@@ -6,20 +6,20 @@
  <world name="actors">
    <plugin
      filename="ignition-gazebo-physics-system"
-      name="ignition::gazebo::systems::Physics">
+      name="gz::sim::systems::Physics">


Ok, I just tested it and it doesn't work. But the fix is simple, we need to add aliases like this to every system plugin:

diff --git a/src/systems/physics/Physics.cc b/src/systems/physics/Physics.cc index b9df0909b..451773597 100644 --- a/src/systems/physics/Physics.cc +++ b/src/systems/physics/Physics.cc @@ -3604,3 +3604,5 @@ IGNITION_ADD_PLUGIN(Physics, Physics::ISystemUpdate) IGNITION_ADD_PLUGIN_ALIAS(Physics, "gz::sim::systems::Physics") +// Deprecated, remove on version 8 +IGNITION_ADD_PLUGIN_ALIAS(Physics, "ignition::gazebo::systems::Physics")

You can check it worked either by changing a world to load the ignition::gazebo version of the plugin, or with the ign plugin CLI:

$ ign plugin -i -v -p install/lib/libignition-gazebo7-physics-system.so Loading plugin library file [install/lib/libignition-gazebo7-physics-system.so] Loader State Known Interfaces: 4 gz::sim::v7::ISystemUpdate gz::sim::v7::System gz::sim::v7::ISystemReset gz::sim::v7::ISystemConfigure Known Plugins: 1 [gz::sim::v7::systems::Physics] has 2 aliases: [gz::sim::systems::Physics] [ignition::gazebo::systems::Physics] implements 4 interfaces: gz::sim::v7::ISystemConfigure gz::sim::v7::ISystemReset gz::sim::v7::ISystemUpdate gz::sim::v7::System

Note the alias

src/ServerConfig.cc

Find: IGNITION_ADD_PLUGIN_ALIAS\((.*),$ Replace: // TODO(CH3): Deprecated, remove on version 8 IGNITION_ADD_PLUGIN_ALIAS($1, "ignition::gazebo::$1") IGNITION_ADD_PLUGIN_ALIAS($1, ----------- Find: IGNITION_ADD_PLUGIN_ALIAS(.*)gz::sim::(.*) Replace: IGNITION_ADD_PLUGIN_ALIAS$1gz::sim::$2 // TODO(CH3): Deprecated, remove on version 8 IGNITION_ADD_PLUGIN_ALIAS$1ignition::gazebo::$2 ----------- And manual replacements for IGNITION_ADD_PLUGIN_ALIAS($ Signed-off-by: methylDragon <[email protected]>

Co-authored-by: Louise Poubel <[email protected]> Signed-off-by: methylDragon <[email protected]>

Signed-off-by: methylDragon <[email protected]>

Signed-off-by: Louise Poubel <[email protected]>

chapulina

Looks good to me with 🟢 CI! I pushed a few changes toward the end:

Recorded updated rolling shapes log for test
Support logs recorded with ignition messages if possible. The levels_log test is still using the old messages, mainly because I didn't know how to record a new log and would probably need to tweak the test. That log was recorded with Blueprint as has been working until now, it would be a shame to stop supporting it instead of doing the 2-line change I did.
I fixed the plugin aliases, it looks like at some point some of them got messed up and failed to compile, while others had lost the systems namespace.
Updated more instances of ~/.gz path, leaving ~/.ignition/fuel untouched for now

codecov · 2022-05-29T16:31:07Z

Codecov Report

Merging #1496 (197b029) into main (aad2019) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1496   +/-   ##
=======================================
  Coverage   35.01%   35.01%           
=======================================
  Files          44       44           
  Lines        2356     2356           
=======================================
  Hits          825      825           
  Misses       1531     1531

Impacted Files	Coverage Δ
src/msgs/peer_info.pb.cc	`45.58% <0.00%> (ø)`
src/msgs/simulation_step.pb.cc	`51.06% <0.00%> (ø)`
src/msgs/performer_affinity.pb.cc	`27.12% <0.00%> (ø)`
src/gui/plugins/copy_paste/moc_CopyPaste.cpp	`7.40% <0.00%> (ø)`
src/gui/plugins/entity_tree/moc_EntityTree.cpp	`6.25% <0.00%> (ø)`
src/gui/plugins/modules/moc_EntityContextMenu.cpp	`23.07% <0.00%> (ø)`
...plugins/transform_control/moc_TransformControl.cpp	`2.77% <0.00%> (ø)`
...tion-gazebo7-gui_autogen/3YJK5W5UP7/qrc_gazebo.cpp	`100.00% <0.00%> (ø)`
...ins/component_inspector/moc_ComponentInspector.cpp	`9.27% <0.00%> (ø)`
...n-gazebo7-gui_autogen/EWIEGA46WW/moc_GuiRunner.cpp	`27.58% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aad2019...197b029. Read the comment docs.

chapulina · 2022-05-29T18:03:03Z

No new test failures or warnings! Merging 🚀

azeey · 2022-09-19T21:26:14Z

src/ServerPrivate.cc

@@ -59,7 +59,7 @@ struct LoggingPlugin
  public: static std::string &RecordPluginName()
  {
    static std::string recordPluginName =
-      "ignition::gazebo::systems::LogRecord";
+      "gz::sim::systems::LogRecord";


Just noticed this while testing something else. This change in together with the logic in

gz-sim/src/ServerPrivate.cc

Line 214 in 59b76c2

name == LoggingPlugin::RecordPluginName())

means sdf fileds with ignition::gazebo::systems::LogRecord won't work in Garden. Can you confirm @methylDragon ?

You're right actually :o
It might be better to do a suffix comparison/regex match for LogRecord$ then :/

methylDragon requested review from mabelzhang, iche033, azeey and chapulina as code owners May 18, 2022 09:43

methylDragon mentioned this pull request May 18, 2022

C++ namespaces, directories and header guards from ignition to gz gazebo-tooling/release-tools#711

Closed

chapulina added 🌱 garden Ignition Garden ign to gz Renaming Ignition to Gazebo. labels May 18, 2022

chapulina added the needs upstream release Blocked by a release of an upstream library label May 18, 2022

methylDragon force-pushed the namespace_migration branch 13 times, most recently from b7de1b3 to cf2a4bb Compare May 20, 2022 12:20

methylDragon force-pushed the namespace_migration branch 2 times, most recently from a2647ff to daff093 Compare May 25, 2022 05:54

chapulina suggested changes May 25, 2022

View reviewed changes

methylDragon force-pushed the namespace_migration branch 2 times, most recently from b1bc54f to 5f6d2b8 Compare May 25, 2022 23:20

methylDragon added 3 commits May 26, 2022 15:03

Migrate GZ_GAZEBO -> GZ_SIM for header guards

dfa25aa

Signed-off-by: methylDragon <[email protected]>

Migrate Gazebo's -> Gazebo Sim's

009344d

Signed-off-by: methylDragon <[email protected]>

Wording changes

0dfe036

Signed-off-by: methylDragon <[email protected]>

methylDragon force-pushed the namespace_migration branch from 5f6d2b8 to 0dfe036 Compare May 26, 2022 22:10

chapulina suggested changes May 26, 2022

View reviewed changes

methylDragon added 3 commits May 26, 2022 15:44

Fix GZ_SIM_SERVER_CONFIG_PATH C++ macro

a5493d3

Signed-off-by: methylDragon <[email protected]>

Gazebo -> Gazebo Sim

e00612f

Signed-off-by: methylDragon <[email protected]>

Fix deprecated test namespaces

fa1601b

Signed-off-by: methylDragon <[email protected]>

methylDragon force-pushed the namespace_migration branch from 06b9a96 to fa1601b Compare May 26, 2022 22:45

chapulina suggested changes May 27, 2022

View reviewed changes

chapulina reviewed May 27, 2022

View reviewed changes

src/ServerConfig.cc Outdated Show resolved Hide resolved

methylDragon and others added 3 commits May 27, 2022 16:32

Fix buggy macro IGN_GAZEBO_SERVER_CONFIG_PATH

5b19d0b

Co-authored-by: Louise Poubel <[email protected]> Signed-off-by: methylDragon <[email protected]>

Gazebo GUI/Server -> Gazebo Sim GUI/Server

f73e7a5

Signed-off-by: methylDragon <[email protected]>

methylDragon force-pushed the namespace_migration branch from e79792f to f73e7a5 Compare May 27, 2022 23:40

chapulina added 6 commits May 28, 2022 08:06

fix build

b548c3e

Signed-off-by: Louise Poubel <[email protected]>

sort out ~/.gz directory

4512c38

Signed-off-by: Louise Poubel <[email protected]>

Revert env vars that need to be tick-tocked

2b41b35

Signed-off-by: Louise Poubel <[email protected]>

standardize and fix plugin aliases

0c27fb0

Signed-off-by: Louise Poubel <[email protected]>

support old logs, fix build

e0a84e9

Signed-off-by: Louise Poubel <[email protected]>

restore README

197b029

Signed-off-by: Louise Poubel <[email protected]>

chapulina approved these changes May 29, 2022

View reviewed changes

chapulina merged commit c7e25be into main May 29, 2022

chapulina deleted the namespace_migration branch May 29, 2022 18:03

chapulina removed the needs upstream release Blocked by a release of an upstream library label May 29, 2022

j-rivero mentioned this pull request Sep 16, 2022

Rename python library as gz.sim7 #1716

Merged

3 tasks

azeey reviewed Sep 19, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ign -> gz Namespace Migration : gz-sim #1496

ign -> gz Namespace Migration : gz-sim #1496

methylDragon commented May 18, 2022

methylDragon commented May 23, 2022

methylDragon commented May 23, 2022 •

edited

Loading

chapulina left a comment

chapulina left a comment

chapulina May 26, 2022

chapulina May 27, 2022

methylDragon May 27, 2022

chapulina May 27, 2022

chapulina left a comment

codecov bot commented May 29, 2022 •

edited

Loading

chapulina commented May 29, 2022

azeey Sep 19, 2022

methylDragon Sep 19, 2022

ign -> gz Namespace Migration : gz-sim #1496

ign -> gz Namespace Migration : gz-sim #1496

Conversation

methylDragon commented May 18, 2022

methylDragon commented May 23, 2022

methylDragon commented May 23, 2022 • edited Loading

chapulina left a comment

Choose a reason for hiding this comment

chapulina left a comment

Choose a reason for hiding this comment

chapulina May 26, 2022

Choose a reason for hiding this comment

chapulina May 27, 2022

Choose a reason for hiding this comment

methylDragon May 27, 2022

Choose a reason for hiding this comment

chapulina May 27, 2022

Choose a reason for hiding this comment

chapulina left a comment

Choose a reason for hiding this comment

codecov bot commented May 29, 2022 • edited Loading

Codecov Report

chapulina commented May 29, 2022

azeey Sep 19, 2022

Choose a reason for hiding this comment

methylDragon Sep 19, 2022

Choose a reason for hiding this comment

methylDragon commented May 23, 2022 •

edited

Loading

codecov bot commented May 29, 2022 •

edited

Loading