ign -> gz Migrate Ignition Headers : gz-math

[methylDragon]

See: gazebo-tooling/release-tools#784

[codecov]

Codecov Report

Merging #483 (12010bb) into ign-math6 (29e3f41) will not change coverage.
The diff coverage is 99.87%.

❗ Current head 12010bb differs from pull request most recent head 2b65247. Consider uploading reports for the commit 2b65247 to get more accurate results

@@            Coverage Diff             @@
##           ign-math6     #483   +/-   ##
==========================================
  Coverage      99.68%   99.68%           
==========================================
  Files             73       73           
  Lines           6913     6913           
==========================================
  Hits            6891     6891           
  Misses            22       22           

Impacted Files	Coverage Δ
include/gz/math/detail/Capsule.hh	96.49% <ø> (ø)
include/gz/math/detail/Cylinder.hh	100.00% <ø> (ø)
include/gz/math/detail/Ellipsoid.hh	100.00% <ø> (ø)
include/gz/math/detail/WellOrderedVector.hh	100.00% <ø> (ø)
include/gz/math/graph/Graph.hh	100.00% <ø> (ø)
include/gz/math/graph/GraphAlgorithms.hh	100.00% <ø> (ø)
include/gz/math/graph/Vertex.hh	100.00% <ø> (ø)
src/Angle.cc	100.00% <ø> (ø)
src/AxisAlignedBox.cc	100.00% <ø> (ø)
src/Color.cc	98.58% <ø> (ø)
... and 102 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[scpeters]

I've been looking at the abichecker CI failures, and I think it's a problem that the classes are defined in the gz namespace. I've checked the symbols using nm on a .so file from this branch, and there are no ignition:: symbols in the library, so I believe this approach breaks ABI.

We can use the namespace gz { using namespace ignition; } trick, but I think the class definitions need to remain in the ignition namespace

[methylDragon]

I've been looking at the abichecker CI failures, and I think it's a problem that the classes are defined in the gz namespace. I've checked the symbols using nm on a .so file from this branch, and there are no ignition:: symbols in the library, so I believe this approach breaks ABI.

We can use the namespace gz { using namespace ignition; } trick, but I think the class definitions need to remain in the ignition namespace

I got this to work, and I'll copy paste my notes regarding my approach here:

• To not break ABI, the headers need to be using the ignition namespace, and the ignition namespace needs to be the "primary" namespace. So I'm back to using reverse redirection (any gz namespaces will be using ignition )
  • To this end, I've put the namespace directives in config.hh for BOTH gz and ignition
• For msgs, since generation happens automatically with protoc, I can't force the headers to use ignition (it's directory dependent), so I can't migrate them to gz. msgs will hence not be migrated in Citadel (and Fortress)
• I've tried my best to migrate in-source namespace declarations to gz, but this cannot happen in a sweeping manner across the board, so there will be some interspersing of ignition and gz, but the diffs should be reduced, which still fits with the goal of making forward/backports easier. Some instances of these exceptions include:
  • Some packages don't use config.hh , so can't pick up the namespace redirections (common, plugin, fuel-tools)
  • Sometimes the redirections don't apply to namespace XXX declarations, so those aren't migrated
  • Some gz:: usages don't work (e.g. inside an explicitly ignition namespaced class)

[scpeters]

nice work on fixing the ABI checker.

how do you feel about squashing the last commit (0fc7ef2) into the second commit (9309be0)?

[methylDragon]

nice work on fixing the ABI checker.

how do you feel about squashing the last commit (0fc7ef2) into the second commit (9309be0)?

Once everything is ok, I'll squash all the commits into the second commit (leaving the first commit (the move) alone) for the rebase-merge!

[chapulina]

Great work, the general idea is ok to me, I just have some minor comments / questions

[chapulina]

Should this be

Suggested change

	gz::math::Angle angle;
	ignition::math::Angle angle;