Add ignition- deprecations and aliases, and gz- redirect

[methylDragon]

Description

This PR adds a deprecation warning to any calls of ign_find_package() with packages that have the ign- or ignition- prefix.

It then attempts to find the gz- prefixed version of the package first, before falling back to the originally requested package on failure-to-find, all with clear warnings/notices. (See big caveat below)

Additionally, it also creates ignition- aliases for the library names and include directories for any gz- prefixed libraries, so that they can be used as if they were ignition- prefixed (see Examples below).

Examples

Try out the example workspace here: https://github.com/methylDragon/ign-gz-cmake-package-alias

• You can try to see what happens if you remove gz-dummy_dependency and/or ign-dummy_dependency, and its impact on ign-dummy_dependent, which explicitly depends on ignition-dummy_dependency1

Run the commands with: colcon build --executor sequential
See caveat below for why.

Invocations

Example invocations (with `--executor sequential`)

If gz- prefixed library is present

If gz- prefixed library not present, but ignition- prefixed library is present

Aliases

Resolving ignition- prefix with explicit gz- import

This works!!!

Notice how you can still use ignition- prefixed variables despite importing gz-dummy_dependency1
(The fallback does not trigger for gz- -> ignition-, so this is gz-dummy_dependency1 truly being found and included!)

ign_find_package(  >>>  gz  <<<  -dummy_dependency1 VERSION 1.0.0 REQUIRED)

set(IGN_DUMMY_DEPENDENCY_VER ${ignition-dummy_dependency1_VERSION_MAJOR})

ign_add_executable(dependent_dummy_hello hello_exec.cpp)
target_link_libraries(dependent_dummy_hello ignition-dummy_dependency1)
target_include_directories(dependent_dummy_hello
  PRIVATE
  ${ignition-dummy_dependency${IGN_DUMMY_DEPENDENCY_VER}_INCLUDE_DIRS}
)

Resolving ignition- prefix with redirected gz- import

So does this, even if the ign_find_package(ignition-dummy_dependency1) call resolves it to gz-dummy_dependency1!!!:

# gz- redirection resolves this to gz-dummy_dependency1
ign_find_package(ignition-dummy_dependency1 VERSION 1.0.0 REQUIRED)

set(IGN_DUMMY_DEPENDENCY_VER ${ignition-dummy_dependency1_VERSION_MAJOR})

ign_add_executable(dependent_dummy_hello hello_exec.cpp)
target_link_libraries(dependent_dummy_hello ignition-dummy_dependency1)
target_include_directories(dependent_dummy_hello
  PRIVATE
  ${ignition-dummy_dependency${IGN_DUMMY_DEPENDENCY_VER}_INCLUDE_DIRS}
)

Caveats

This was only tested on the example workspace. The behavior with components is unknown.

Sequential Builds

[Only occurs with ign_find_package(ignition-XXX)]

Due to how colcon-cmake resolves dependencies (by looking at the exact strings in the CMake files), the gz- prefix redirection doesn't get picked up in the dependency resolution. Because of this, it is likely that the gz- prefixed library will continue to be building while the dependent library looks for it, causing the gz- library to not be able to be found and for the ignition- fallback to be triggered.

It is possible to force colcon to build things sequentially with --executor sequential, but this causes things to slow down since there will only be one core building at a time. The true solution will be to just not use the redirection and have the user replace all instances of ignition- prefixes with gz- prefixes for library names in their CMake files.

Windows Issues

[Only occurs with find_package(ignition-XXX) on WINDOWS]
If a package looks for ignition-XXX targets but the found library's project name is gz-XXX, it won't be found. This is expected behavior normally, but with the redirection, is an edge case. (In ign-cmake, the ignition-XXX library target is EXPLICITLY generated even for gz-XXX project names, and works on all platforms other than windows!)

The Windows CI is green because it is looking for the gz-XXX library. But observing the console logs, it is obvious the ignition- library target isn't installed properly.

A relatively significant chunk of time was devoted to trying to fix this, but no solutions were found as of yet... It seems to be something specific in how Windows is compiling the CMake.

(ignition-component_deps.lib is missing in /lib, and this is only the case on Windows. On other platforms, both generate appropriately.)

-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/bin/ignition-component_deps.dll
...
...
-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/lib/gz-component_deps.lib
-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/bin/gz-component_deps.dll

[methylDragon]

Ubuntu test is unstable because this introduced depreciation warnings.

If there's another approach that can get CI passing, I'm for it 👀

[scpeters]

Try out the example workspace here: https://github.com/methylDragon/ign-gz-cmake-package-alias

the example workspace only has packages that use ign_configure_project. I would expect most external users of our software are not using ign_configure_project. Is our priority for migration to support our internal packages or external cmake packages?

[methylDragon]

Try out the example workspace here: https://github.com/methylDragon/ign-gz-cmake-package-alias

the example workspace only has packages that use ign_configure_project. I would expect most external users of our software are not using ign_configure_project. Is our priority for migration to support our internal packages or external cmake packages?

My understanding is that we're adding the deprecation warnings for ign_find_package() calls, which downstream packages will use, pulling up variables from the internal packages that are packaged with variables set in ign_configure_project.

[methylDragon]

Since it's working for ubuntu and homebrew, and users are expected to use the gz- prefix of our internal libraries...
Target aliasing is a bonus at this stage. But a very welcome one.

Should we, in addition to merging in the deprecation warnings:

• Merge in the target aliasing (so you can grab the targets with gz- or ignition- on ubuntu) with the caveat that ignition- on windows will no longer work for any internal libraries with names changed to gz-
• Not merge aliasing in

In either case, we'll need to migrate the library names before merging the deprecation warning in, otherwise CI will turn unstable for every downstream package that uses ignition-cmake.

[chapulina]

I started updating the libraries to use the gz- project name making use of this PR. See

• CMake project names from ignition to gz gazebo-tooling/release-tools#718
• Rename CMake project to gz gz-utils#57
• Rename CMake project to gz gz-math#420

The gz- targets are being generated properly, but the include paths aren't working yet. Math isn't able to find includes from Utils. I think we need some more aliases.

Edit

Ah I forgot I had already done some changes on top of this PR. I'm testing with

• Rename CMake project to gz #244

[scpeters]

I think this file could be a bit simpler if it just includes Export.hh and defines new macros with different names. We did something like this with libsdformat:

• https://github.com/ignitionrobotics/sdformat/blob/sdf12/include/sdf/system_util.hh

[methylDragon]

Oh, I think this file shouldn't be here... It was placed here for testing the Windows CI (which still isn't resolved.)
Sorry about that! 🙇 I'll revert that specific change

[methylDragon]

The commit that introduced this file is now 💥

[scpeters]

has find_package(ign-*) ever worked? I thought we always had to spell it out as 1ignition-*. If that is the case, I would simplify this to only cover the case of ignition-` to avoid confusion

[methylDragon]

841fc7c

[scpeters]

the PACKAGE_NAME_BAK variable is confusing. can we use GZ_PACKAGE_NAME in this code path for the gz-* name and leave PACKAGE_NAME unchanged until we've determined which package name is to be used?

[methylDragon]

841fc7c

[scpeters]

I split out a subset of these changes into #246 since I think it is easier to review than the aliasing and redirections

[traversaro]

Windows Issues

[Only occurs with find_package(ignition-XXX) on WINDOWS] If a package looks for ignition-XXX targets but the found library's project name is gz-XXX, it won't be found. This is expected behavior normally, but with the redirection, is an edge case. (In ign-cmake, the ignition-XXX library target is EXPLICITLY generated even for gz-XXX project names, and works on all platforms other than windows!)

The Windows CI is green because it is looking for the gz-XXX library. But observing the console logs, it is obvious the ignition- library target isn't installed properly.

A relatively significant chunk of time was devoted to trying to fix this, but no solutions were found as of yet... It seems to be something specific in how Windows is compiling the CMake.

(ignition-component_deps.lib is missing in /lib, and this is only the case on Windows. On other platforms, both generate appropriately.)

-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/bin/ignition-component_deps.dll
...
...
-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/lib/gz-component_deps.lib
-- Installing: C:/Jenkins/workspace/ignition_cmake-ci-pr_any-windows7-amd64/ws/ign-cmake/build/install/Release/bin/gz-component_deps.dll

I did not checked in depth, but whenever a .dll is produced without the corresponding .lib that means that that library is not exporting any symbol. I think this is happening due to the logic in https://github.com/ignitionrobotics/ign-cmake/pull/239/files#diff-955d1f97e2360702839ba9277cc71b9797e65d417d5cb88f383fec0e53ad5124R1045 . In particular, there there is a library called ignition-*** something that is being compiled using exactly the same source code of the gz-*** library. The problem is that the library uses the export header generated in https://github.com/ignitionrobotics/ign-cmake/blob/5b0a826cbc07ab30fbddb2efd26194fa253b7317/cmake/IgnUtils.cmake#L1556 assuming that the library name is gz-***, not ignition-** . A possible solution is to copy the value of the DEFINE_SYMBOL target property from gz-*** target to the ignition-*** target, something like:

get_target_property(_GZ_DEFINE_SYMBOL ${PROJECT_LIBRARY_TARGET_NAME} DEFINE_SYMBOL )
set_target_property(${IGN_LIBRARY_TARGET_NAME} PROPERTIES DEFINE_SYMBOL ${_GZ_DEFINE_SYMBOL})

to ensure that everything works as expected.

[chapulina]

I believe parts of this PR have been moved to other PRs.

This PR adds a deprecation warning to any calls of ign_find_package() with packages that have the ign- or ignition- prefix.
It then attempts to find the gz- prefixed version of the package first, before falling back to the originally requested package on failure-to-find, all with clear warnings/notices.

After iterating a bit more on all these packages, I think this isn't worth it anymore.

What this is proposing is to have ign_find_package(ignition-math7) work as if it were doing ign_find_package(gz-math7), for backwards compatibility. This would be useful if we had users already doing ign_find_package(ignition-math7), but we don't. They're all doing ign_find_package(ignition-math6). So users will need to change that 6 to 7 anyway, and at this point they might as well change ignition to gz.

So I believe there's little value in providing this alias.

Additionally, it also creates ignition- aliases for the library names and include directories for any gz- prefixed libraries, so that they can be used as if they were ignition- prefixed (see Examples below).

I believe we're addressing all of these downstream at each library, so gz-cmake probably won't need to deal with it. But I can be missing some important detail.

[methylDragon]

I believe parts of this PR have been moved to other PRs.

This PR adds a deprecation warning to any calls of ign_find_package() with packages that have the ign- or ignition- prefix.
It then attempts to find the gz- prefixed version of the package first, before falling back to the originally requested package on failure-to-find, all with clear warnings/notices.

After iterating a bit more on all these packages, I think this isn't worth it anymore.

What this is proposing is to have ign_find_package(ignition-math7) work as if it were doing ign_find_package(gz-math7), for backwards compatibility. This would be useful if we had users already doing ign_find_package(ignition-math7), but we don't. They're all doing ign_find_package(ignition-math6). So users will need to change that 6 to 7 anyway, and at this point they might as well change ignition to gz.

So I believe there's little value in providing this alias.

Additionally, it also creates ignition- aliases for the library names and include directories for any gz- prefixed libraries, so that they can be used as if they were ignition- prefixed (see Examples below).

I believe we're addressing all of these downstream at each library, so gz-cmake probably won't need to deal with it. But I can be missing some important detail.

You're right! I think the main reason for this alias was to help make sure things didn't break when we're migrating the project names, but due to the need to build with a single core (from the colcon dependency resolution issue)... we're kind of out of luck.. So I'll close the PR!