Deprecate public ign functions in favor of gz

[chapulina]

• Builds on top of
  • [ign ➡️ gz] Internal functions #247
  • Remove deprecated CMake functions #252
  • [ign -> gz] Migrate more internal variables  #263
• Part of CMake API from ign to gz gazebo-tooling/release-tools#719

Summary

Deprecate the ign_ functions that are used by downstream packages and replace them with gz_ counterparts. Warnings are commented out and will be enabled once all our libraries have migrated to gz_.

As far as I can tell, CMake doesn't provide a convenient way for a function to forward arguments to another. So I had to play a bit with the scopes to make sure all the values passed in and returned from a function worked correctly.

Here are the rules I came up with:

• If a function doesn't take any arguments and doesn't set or get any variables from the parent scope, it can be wrapped directly like this:

function(ign_XXX)
    gz_XXX()
endfunction()
function(gz_XXX)
  # Exactly the same as old ign_XXX
endfunction()

• If a function takes named arguments, those can be forwarded directly:

function(ign_XXX named_arg)
    gz_XXX(${named_arg})
endfunction()
function(gz_XXX)
  # Exactly the same as old ign_XXX
endfunction()

• If a function takes optional arguments parsed with cmake_parse_arguments, parse those at the old ign_ function and skip parsing them in the new function with the help of a skip_parsing variable.

function(ign_XXX named_arg)

    # Exactly same as in gz_ below
    set(options SOME_OPTION)
    set(oneValueArgs SOME_VALUE)
    set(multiValueArgs SOME_MULTIVALUE)
    cmake_parse_arguments(gz_install_all_headers "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

    set(gz_XXX_skip_parsing true)
    gz_XXX(${named_arg})
endfunction()
function(gz_XXX)

   # Skip parsing when called from ign_, otherwise parse as usual
    if (NOT gz_XXX_skip_parsing)
      set(options SOME_OPTION)
      set(oneValueArgs SOME_VALUE)
      set(multiValueArgs SOME_MULTIVALUE)
      cmake_parse_arguments(gz_install_all_headers "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
    endif()

  # The rest is exactly the same as old ign_XXX
endfunction()

• If a function sets values to the parent scope, those need to be bubbled up by the ign_ wrapper function:

function(ign_XXX)
    gz_XXX()

    set(${gz_XXX_RETURN_VALUE} ${${gz_XXX_RETURN_VALUE}} PARENT_SCOPE)
endfunction()
function(gz_XXX)
  # Exactly the same as old ign_XXX
  # Includes somewhere
  # set(${gz_XXX_RETURN_VALUE} ${calculated_value} PARENT_SCOPE)
endfunction()

Test it

1. Compile the stack using this branch, and verify that everything still works the same way
2. Uncomment some of the commented warnings, then compile the stack and verify that warnings show up
3. Update downstream packages to use gz_ instead of ign_ and verify that they work the same, without warnings

TODO

Keeping as draft until

• All ign_ functions have been deprecated
• Set up CI compiling the entire stack to verify point 1 above

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.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[methylDragon]

Problem

There's a small issue with the approach you're using for bubbling parsed variables down to a gz_ prefixed function.

If you have something like ign_A -> gz_A -> gz_B, the common skip_parsing variable causes gz_B to skip its parsing, which is not good (and breaks in the case of gz-gui!)

Solution

I'd recommend using a prefix to mangle the names. Like gz_b_skip_parsing.

Working Example

See working example here: https://github.com/gazebosim/gz-gui/pull/395/files#diff-befa40e675cae663c40a34346dbbffd61207a9fac4c557ecc9015786b8bc95c1

[chapulina]

I'd recommend using a prefix to mangle the names. Like gz_b_skip_parsing.

Thanks for catching that, updated this PR with this pattern

[chapulina]

This PR was getting too large. Started breaking it into smaller PRs:

• Deprecate ign_create_docs, ign_create_packages, ign_configure_build #253
• Deprecate ign_build_tests, ign_get_libsources_and_unittests, ign_get_sources, ign_install_all_headers #254
• Deprecate ign_manual_search, ign_create_packages, ign_create_core_library #255
• Deprecate ign_configure_project, ign_*_executable, ign_import_target #257
• Deprecate ign_find_package, ign_string_append, ign_build_error, ign_build_warning #258

[chapulina]

This PR now builds on top of

• [ign -> gz] Migrate more internal variables  #263

These are the functions and macros currently migrated by this PR:

ign_pkg_check_modules_quiet
ign_pkg_config_library_entry
ign_pkg_config_entry
ign_pkg_check_modules
ign_add_component
ign_target_interface_include_directories
ign_build_executables

There are still some to go.

[methylDragon]

Will approve once my local build works with this