Use ign-cmake.

[osrf-migration]

Original report (archived issue) by Nate Koenig (Bitbucket: Nathan Koenig).

---

Switch this project to use ign-cmake. This will likely require moving osrf/sdf to ignition/sdf

[scpeters]

This would be a very big change

• We would probably want to move from the osrf' GitHub org to ignitionrobotics. This would require communication and agreement but shouldn't be too disruptive overall
• the cmake project name would have to change to start with ignition-
• the code for auto-generating the all-inclusive header file assumes that headers have ignition/ at the start of the include path, so the include paths would all change from sdf/* to ignition/sdf/* or ignition/sdformat/* or something
• this repository holds both the specification (SDFormat) and our reference parsing library (libsdformat). If we are making big changes, we should consider if we want to split these two into separate repositories so we can factor that in to the overall migration plan

[chapulina]

We would probably want to move from the osrf' GitHub org to ignitionrobotics.

This has been brought up several times and could happen regardless of whether we use ign-cmake or not. Some advantages:

• Help with visibility into the SDFormat repo. The osrf org is quite bloated and it's hard to find what's important and active. While the ignitionrobotics org is more focused.
• SDFormat is being released as part of Ignition collections, so it makes sense to move it closer to the rest of the libraries
• It would make a lot of our infra easier, we have to check in several places if org == "osrf" just for sdformat
• SDF tickets would automatically land in https://github.com/orgs/ignitionrobotics/projects/3 and be tracked by the Ignition team

The transition should also be seamless for both users and tools, because GitHub is pretty good about redirecting.

[chapulina]

This would be a very big change

The first point has been addressed. Not sure if the other 3 would be easy changes in ign-cmake so we can keep this project's name.

[scpeters]

This would be a very big change

The first point has been addressed. Not sure if the other 3 would be easy changes in ign-cmake so we can keep this project's name.

I imagine that we could add a NO_IGN_PREFIX option to ign-cmake2 that would allow more flexibility in the cmake project name.

[scpeters]

• the cmake project name would have to change to start with ignition-
• the code for auto-generating the all-inclusive header file assumes that headers have ignition/ at the start of the include path, so the include paths would all change from sdf/* to ignition/sdf/* or ignition/sdformat/* or something

I've opened gazebosim/gz-cmake#190 to add options to ign_configure_project to address these concerns

I expect some more work may be needed, but it's not as difficult as I thought it would be

[scpeters]

here's a list of changes needed in ign-cmake to support libsdformat:

• configurable cmake project name (sdformatN instead of ignition-sdformatN) Project option: NO_IGNITION_PREFIX gz-cmake#191
• configurable include prefix (sdf/*.hh instead of ignition/sdformat/*.hh) Project option: REPLACE_IGNITION_INCLUDE_PATH gz-cmake#190
• configurable INCLUDE_INSTALL_DIR_POSTFIX (include/sdformat-N/ instead of include/ignition/sdformatN)
• find module for urdfdom

The following may not be needed:

• configurable macro prefix in Export.hh (SDF_VISIBLE instead of IGN_SDFORMAT_VISIBLE) prototype in scpeters/ign-cmake@e610e66
• configurable master header file name (sdf.hh instead of sdformat.hh)
• configurable master header file install path (sibling to other header files instead of in parent folder)
• configurable config.hh header file name (sdf_config.h instead of config.hh)

[scpeters]

I think we could work around the issues about the master header file and config header file without changes to ign-cmake. Once the first 4 issues are resolved, we should have the following header files

• include/sdformat-N/sdformat.hh
• include/sdformat-N/sdf/config.hh

I think we could then have trivial header files manually included in sdformat's cmake code:

• include/sdformat-N/sdf/sdf.hh that just includes sdformat.hh
• include/sdformat-N/sdf/sdf_config.h that just includes sdf/config.hh

@koonpeng what do you think?

[adlarkin]

I think we could work around the issues about the master header file and config header file without changes to ign-cmake ... we could then have trivial header files manually included in sdformat's cmake code

That's not a bad idea. I think that we should try that to see if it works. If not, then we can see how to change ign-cmake to fix this.

[j-rivero]

here's a list of changes needed in ign-cmake to support libsdformat:

Nothing to argue about the first two points. About the following points maybe is not a bad idea to follow what Steve is proposing and push sdformat into a migration that approach it to have more "standard" naming and file names. Sounds to me like we should not try to push ignition-cmake to fit into sdformat but working more in the opposite.

configurable macro prefix in Export.hh (SDF_VISIBLE instead of IGN_SDFORMAT_VISIBLE) prototype in scpeters/ign-cmake@e610e66

Since the visible macro keyword can be decided by the project, not 100% sure that is a good idea but we could just reuse the NO_IGNITION_PREFIX option to default to ${PROJECT_NAME}_VISIBLE?

configurable INCLUDE_INSTALL_DIR_POSTFIX (include/sdformat-N/ instead of include/ignition/sdformatN)

Are the changes in INCLUDE_DIR not enough to make this happen?

configurable master header file name (sdf.hh instead of sdformat.hh)

+1 to Steve proposal for migrating/supporting both without changing ign-cmake.

configurable master header file install path (sibling to other header files instead of in parent folder)

May I have an example of this?

configurable config.hh header file name (sdf_config.h instead of config.hh)

+1 to Steve proposal for migrating/supporting both without changing ign-cmake.

[koonpeng]

I think we could work around the issues about the master header file and config header file without changes to ign-cmake. Once the first 4 issues are resolved, we should have the following header files

• include/sdformat-N/sdformat.hh
• include/sdformat-N/sdf/config.hh

I think we could then have trivial header files manually included in sdformat's cmake code:

• include/sdformat-N/sdf/sdf.hh that just includes sdformat.hh
• include/sdformat-N/sdf/sdf_config.h that just includes sdf/config.hh

I like a "redirection" header approach as well, should we also deprecate sdf.hh and sdf_config.h? Would this https://stackoverflow.com/questions/68306977/how-can-i-deprecate-a-c-header work?

configurable INCLUDE_INSTALL_DIR_POSTFIX (include/sdformat-N/ instead of include/ignition/sdformatN)

Are the changes in INCLUDE_DIR not enough to make this happen?

INCLUDE_DIR only affects the source path that the compiler checks (and also used to build part of the install include path). But the installed path is still always fixed with ignition/ prefix. https://github.com/scpeters/ign-cmake/blob/787cebd91c0ba16562ea6d545b86d5a87d90775c/cmake/IgnPackaging.cmake#L106.

Actually this also affects the data dir, so we should probably add an option for that too, or see if we can generalized it to something like INSTALL_POSTFIX_PATH.

[scpeters]

configurable macro prefix in Export.hh (SDF_VISIBLE instead of IGN_SDFORMAT_VISIBLE) prototype in scpeters/ign-cmake@e610e66

Since the visible macro keyword can be decided by the project, not 100% sure that is a good idea but we could just reuse the NO_IGNITION_PREFIX option to default to ${PROJECT_NAME}_VISIBLE?

I've just been trying to identify what is needed to enable an already released version of libsdformat to use ign_configure_project and still pass the API/ABI checker. If we change the export macros, it shouldn't matter to downstream code, but I think the checker would complain. Although, now that I think about it, we could use the same trick I mentioned for sdf_config.h and config.hh: create another header file that maps the ${EXPORT_BASE}_VISIBLE macros to SDF_VISIBLE. So I think it may be helpful to match the old macro names, but I'm not sure we need to modify ing-cmake to support it.

[scpeters]

configurable INCLUDE_INSTALL_DIR_POSTFIX (include/sdformat-N/ instead of include/ignition/sdformatN)

Are the changes in INCLUDE_DIR not enough to make this happen?

If you don't make INCLUDE_INSTALL_DIR_POSTFIX configurable and then wanted to start using ign-cmake2, then the path of the installed include files would change. This wouldn't be a problem if you do a fresh build against it because the pkg-config and cmake config would point to the right place, but I imagine the API checker will complain that the header files are in a new location. It's not recommended, but I suppose it's possible someone has #include <sdformat-9.6/sdf/sdf.hh>, which would break if the headers moved to ignition/sdformat9/sdf/sdf.hh.

We'd also have to update the debian metadata, but that is easily done

If we only want to make this change on main, then I think we don't need to make INCLUDE_INSTALL_DIR_POSTFIX configurable

[scpeters]

configurable master header file install path (sibling to other header files instead of in parent folder)

May I have an example of this?

in ign-math, the master header can be used with #include <ignition/math.hh>

in sdformat, we use #include <sdf/sdf.hh>, while I think the auto-generated header would be accessible from #include <sdformat.hh>. I had initially thought we should make the location of the auto-generated header file configurable, but we can instead simply add an sdf/sdf.hh file that contains: #include "../sdformat.hh"

[scpeters]

* find module for urdfdom

I just added a find module for urdfdom to the list of things needed in ign-cmake

[koonpeng]

If you don't make INCLUDE_INSTALL_DIR_POSTFIX configurable and then wanted to start using ign-cmake2, then the path of the installed include files would change. This wouldn't be a problem if you do a fresh build against it because the pkg-config and cmake config would point to the right place, but I imagine the API checker will complain that the header files are in a new location. It's not recommended, but I suppose it's possible someone has #include <sdformat-9.6/sdf/sdf.hh>, which would break if the headers moved to ignition/sdformat9/sdf/sdf.hh.

I feel there probably are some projects out there using #include <sdformat-N/sdf/sdf.hh>, there's also a chance that someone is using other build systems and they are not properly converting the metadata from pkg-config or cmake to the build system's format.

There's also chances that it would break other components of the toolchain, for example, a project might have their static analysis tool configured to ignore warnings from a specific directory, or they might have configured their ide to look at specific directory to provide intellisense etc.

[koonpeng]

Since the visible macro keyword can be decided by the project, not 100% sure that is a good idea but we could just reuse the NO_IGNITION_PREFIX option to default to ${PROJECT_NAME}_VISIBLE?

From what I understand, the visibility macro is considered an implementation detail, downstream package's should not be using sdformat's visibility macros. So we should be able to change all the macros to use the new names without breaking api/abi. Unless maybe if sdformat supports some plugin interfaces, which expects external plugins to use those visibility macros.

[j-rivero]

configurable macro prefix in Export.hh (SDF_VISIBLE instead of IGN_SDFORMAT_VISIBLE) prototype in scpeters/ign-cmake@e610e66

Since the visible macro keyword can be decided by the project, not 100% sure that is a good idea but we could just reuse the NO_IGNITION_PREFIX option to default to ${PROJECT_NAME}_VISIBLE?

I've just been trying to identify what is needed to enable an already released version of libsdformat to use ign_configure_project and still pass the API/ABI checker. If we change the export macros, it shouldn't matter to downstream code, but I think the checker would complain.

Not sure about this one too: the visibility keywords are defines that should turn into visibility modifiers at pre-processing time. We could change them and generate the same pre-processor instructions, that could make API checker happy. I'm a bit more sure about that these changes would not affect ABI.

Although, now that I think about it, we could use the same trick I mentioned for sdf_config.h and config.hh: create another header file that maps the ${EXPORT_BASE}_VISIBLE macros to SDF_VISIBLE. So I think it may be helpful to match the old macro names, but I'm not sure we need to modify ing-cmake to support it.

Not a bad idea if we need it.

[j-rivero]

configurable INCLUDE_INSTALL_DIR_POSTFIX (include/sdformat-N/ instead of include/ignition/sdformatN)

Are the changes in INCLUDE_DIR not enough to make this happen?

If you don't make INCLUDE_INSTALL_DIR_POSTFIX configurable and then wanted to start using ign-cmake2, then the path of the installed include files would change. This wouldn't be a problem if you do a fresh build against it because the pkg-config and cmake config would point to the right place, but I imagine the API checker will complain that the header files are in a new location. It's not recommended, but I suppose it's possible someone has #include <sdformat-9.6/sdf/sdf.hh>, which would break if the headers moved to ignition/sdformat9/sdf/sdf.hh.

We'd also have to update the debian metadata, but that is easily done

If we only want to make this change on main, then I think we don't need to make INCLUDE_INSTALL_DIR_POSTFIX configurable

This last option was the one I was thinking on, making changes in main.

To support transition, I would prefer not to add options to ign-cmake if we can handle it in the software being migrated since I consider this (migrating a released software keeping API/ABI compatibility) as non usual way of using ign-cmake. I could be wrong. We can discuss this with the rest of the team.

If we decided to go with changes in sdformat released versions, the trick of having duplicate .hh for new and old location (with the latest one just including new ones) could help.

[j-rivero]

If you don't make INCLUDE_INSTALL_DIR_POSTFIX configurable and then wanted to start using ign-cmake2, then the path of the installed include files would change. This wouldn't be a problem if you do a fresh build against it because the pkg-config and cmake config would point to the right place, but I imagine the API checker will complain that the header files are in a new location. It's not recommended, but I suppose it's possible someone has #include <sdformat-9.6/sdf/sdf.hh>, which would break if the headers moved to ignition/sdformat9/sdf/sdf.hh.

I feel there probably are some projects out there using #include <sdformat-N/sdf/sdf.hh>, there's also a chance that someone is using other build systems and they are not properly converting the metadata from pkg-config or cmake to the build system's format.

There's also chances that it would break other components of the toolchain, for example, a project might have their static analysis tool configured to ignore warnings from a specific directory, or they might have configured their ide to look at specific directory to provide intellisense etc.

+1 let's keep old files in place, although they could be changed in released versions to just include new headers.

[scpeters]

I've opened a draft pull request targeting sdf10 in #780

[scpeters]

If you don't make INCLUDE_INSTALL_DIR_POSTFIX configurable and then wanted to start using ign-cmake2, then the path of the installed include files would change. This wouldn't be a problem if you do a fresh build against it because the pkg-config and cmake config would point to the right place, but I imagine the API checker will complain that the header files are in a new location. It's not recommended, but I suppose it's possible someone has #include <sdformat-9.6/sdf/sdf.hh>, which would break if the headers moved to ignition/sdformat9/sdf/sdf.hh.

I feel there probably are some projects out there using #include <sdformat-N/sdf/sdf.hh>, there's also a chance that someone is using other build systems and they are not properly converting the metadata from pkg-config or cmake to the build system's format.
There's also chances that it would break other components of the toolchain, for example, a project might have their static analysis tool configured to ignore warnings from a specific directory, or they might have configured their ide to look at specific directory to provide intellisense etc.

+1 let's keep old files in place, although they could be changed in released versions to just include new headers.

I've made a pull request targeting sdf10, and the abi-checker does not complain about the new location of the header files. Now that I think about it, the header files are currently prefixed with both major and minor (<prefix>/include/sdformat-10.6). As long as we bump the minor when releasing this change, I think it should be no different to change to /include/ignition/sdformat10instead of/include/sdformat-10.7`.

[scpeters]

Closed by #780.

[osrf-triage]

This issue has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1