Script for Header Migration

[methylDragon]

Description

This is related to #711
And is an extended scripted version of gazebosim/gz-utils#51

It should:

• Move header files to gz/* and add redirect headers to ignition/*
• Replace all #include <ignition/*> statements in header files with #include <gz/*>
• Create the associated Export.hh and <lib>.hh header files
• Replace all #include <ignition/*> statements in src, test, and examples source code with #include <gz/*>

Examples

With the script placed into the root of ign-utils, on the main branch and an empty header_migration branch, run ./header_migration_script.sh

PR generated from this script: gazebosim/gz-utils#53

Further Work

Caveats

Generated Files

This script can't handle generated files because it has no knowledge of them, and to get that functionality, it'll need to be able to intelligently go up and down the directory tree, looking into CMake files and the like.

There is too much variance between the packages to do this quickly (and robustly), so for those cases, users will have to manually do the migration. These are some of the common edge cases:

• Export.hh and <lib>.<lib.hh files have to be manually added since they're usually dynamically generated by ign-cmake
  • Export Example | lib Example
• And also manually installed in CMakeLists.txt files (that usually have to be added)
  • Sometimes, whole migrated packages have to have extra CMake files added to ensure that they install properly
  • Example

Typically, the way you'd surface these issues is, once you've done the migration with the script, install the unmigrated version alongside the migrated version (across two workspaces), then use meld to run a directory diff on:

• The top level directory: To check if the appropriate redirection headers are installed!
  • migrated_ws/install/ignition-<lib>/include/ignition/<lib> vs;
  • unmigrated_ws/install/ignition-<lib>/include/ignition/<lib>
• The target directory: Tto check if the migrated headers are installed!
  • migrated_ws/install/ignition-<lib>/include/ignition/<lib>/gz vs;
  • unmigrated_ws/install/ignition-<lib>/include/ignition/<lib>/ignition

Greedy Matching

There are a couple of instances where the script will match greedily and change something that shouldn't be changed. This is because I am erring on the side of editing something (so it is visible to a user) rather than missing something.

I think the script gets it right in the ballpark of 90% correct, but there are about 10% of cases that are false positive matches that require users to manually discard changes for.

To aid in this, the script has multiple stopping and review points for a user to open the git diffs on a program like GitKraken to double-check any changes made.

Already Migrated Files

(This is sort of a variant of greedy matching)

Be aware that the script will greedily convert already migrated gz headers into redirection headers! Please check to make sure it doesn't do that!

Unrelated To This PR

• Remove the REPLACE_IGNITION_INCLUDE_PATH parameter override once its no longer needed
• Add using namespace ignition to the ignition/ headers
• Add portable #warning equivalent to ignition/ headers
• Replace all #include <ignition/*> statements in downstream code
• Change ign-cmake3's default value of PROJECT_INCLUDE_DIR from ignition/${IGN_DESIGNATION} to gz/${IGN_DESIGNATION} (or use ${GZ_DESIGNATION} if that has been defined) in IgnConfigureProject.cmake

[scpeters]

are you planning to match the pattern of commits used in gazebosim/gz-utils#51? I think it's helpful to be strategic about the commits because the overall diff is very hard to read

[methylDragon]

The script is currently in a state where it reproduces all the changes made by gazebosim/gz-utils#51 except making the Export.hh and utils.hh!

I'll be implementing that (though the utils.hh will require command line input to resolve properly.)

Then I'll also work on replacing the relevant statements in src, test, and examples. (This will mean I'll also have to migrate the headers in cli/include, which I deliberately skipped for now so I could get the script doing what was in gazebosim/gz-utils#51 first.

---

N.B.: I'll split the commits accordingly when it's ready to start implementing git commit functionality to follow the pattern in the pull request.

[scpeters]

I just noticed the following header file with Ignition in the file name. We'll probably have to deprecate that manually

• https://github.com/ignitionrobotics/ign-utils/blob/ign-utils1/cli/include/ignition/utils/cli/IgnitionFormatter.hpp

[methylDragon]

I just noticed the following header file with Ignition in the file name. We'll probably have to deprecate that manually

• https://github.com/ignitionrobotics/ign-utils/blob/ign-utils1/cli/include/ignition/utils/cli/IgnitionFormatter.hpp

I'll write functionality to migrate the file with redirection, but not the class name. Let's do class and variable name migration in a separate set of PRs so things are cleaner?

[scpeters]

there's a lot happening so it's a bit hard to follow this script. I would imagine the first commit could be done with code like the following:

git checkout -b migrate_ign_headers

# First commit: rename folders
git mv include/ignition include/gz
for d in $(ls -d */include/ignition)
do
  git mv ${d} ${d}/../gz
done
git commit -a --signoff -m "git mv include/ignition include/gz"

[methylDragon]

there's a lot happening so it's a bit hard to follow this script. I would imagine the first commit could be done with code like the following:

git checkout -b migrate_ign_headers

# First commit: rename folders
git mv include/ignition include/gz
for d in $(ls -d */include/ignition)
do
  git mv ${d} ${d}/../gz
done
git commit -a --signoff -m "git mv include/ignition include/gz"

I was using git mv, just that I ended up running git reset to get the diffs to show up (since they'll only show up on unstaged changes), which caused the index to get reset.

That is no longer the case, and history should be preserved! 🎉

daad715
0a46de1

[j-rivero]

Seems like the script is in active development. If that is the case please mark it as Draft.

Before looking at the code, the script is placed under jenkins-scripts/tools but I believe that it does not have anything to do with Jenkins. We can create a new subdir in the root of the repo named something like code-repo-scripts to host the scripts designed to work with repository code. My intention is to move source_changelog into it but that will require a different PR.

[j-rivero]

Maybe adding a set -e to stop on errors at the begging can help to detect hidden problems.

[methylDragon]

Good call! cc67a0b

[methylDragon]

Seems like the script is in active development. If that is the case please mark it as Draft.

Before looking at the code, the script is placed under jenkins-scripts/tools but I believe that it does not have anything to do with Jenkins. We can create a new subdir in the root of the repo named something like code-repo-scripts to host the scripts designed to work with repository code. My intention is to move source_changelog into it but that will require a different PR.

I agree! I couldn't really find another place to put this into...

[chapulina]

We can create a new subdir in the root of the repo named something like code-repo-scripts to host the scripts designed to work with repository code. My intention is to move source_changelog into it but that will require a different PR.

I just saw this comment. On #677 I created the source-repo-scripts directory and moved source_changelog there.

[methylDragon]

In the end it was still better to manually copy and move from the install space (too many edge cases), and it's unlikely we'll need to keep using this functionality, so I'm closing this :(