Skip to content

Support discovery of TinyXML2 by pkgconfig #5697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fedegiova
Copy link

@fedegiova fedegiova commented Mar 10, 2025

Allow to discovery TinyXML2 by pkg-config is a -config.cmake file is not available.

Since tinyxml2 is built with meson in Yocto it's configuration gets deployed by pkg-config (because meson cannot generate CMake target files). In this case, the current FindTinyXML2.cmake will find out the path of tinyxml2.h and of tinyxml2.so by looking into the recipe-sysroot folder, which path in going to get expanded into the fastrtps-target.cmake generated when fastdds is built. The end result is that we're going to have a system-wide deployed config mode package that points to a directory that won't (probably) not exist anymore when other dependent targets are built.

Since it's not possible to force meson to produce a tinyxml2-target.cmake file, the most reasonable solution is trying to look for the library by pkg-config.

With this patch applied, the generated fastrtps-target.cmake will have a reference to the target PkgConfig::TinyXML2 instead of a full path.

Note that embedding TinyXML2 without symbol leaking it's not possible since they declare interfaces with default visibility. Which means that while creating an image we must have one and only one version of TinyXML2 around. Which basically makes impossible to use the bundled one in thirdparty

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • ❌ Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • ❌ Any new/modified methods have been properly documented using Doxygen.
  • ❌ Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • ❌ Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • ❌ Changes are API compatible.
  • ❌ New feature has been added to the versions.md file (if applicable).
  • ❌ New feature has been documented/Current behavior is correctly described in the documentation.
  • ❌ Applicable backports have been included in the description.

Reviewer Checklist

  • The PR has a milestone assigned.
  • The title and description correctly express the PR's purpose.
  • Check contributor checklist is correct.
  • If this is a critical bug fix, backports to the critical-only supported branches have been requested.
  • Check CI results: changes do not issue any warning.
  • Check CI results: failing tests are unrelated with the changes.

Allow to discovery TinyXML2 by pkg-config is a `-config.cmake` file is not available.

Since tinyxml2 is built with meson in Yocto it's configuration gets deployed by `pkg-config` (because meson cannot generate CMake target files). In this case, the current `FindTinyXML2.cmake` will find out the path of `tinyxml2.h` and of `tinyxml2.so` by looking into the `recipe-sysroot` folder, which path in going to get expanded into the `fastrtps-target.cmake` generated when fastdds is built. The end result is that we're going to have a system-wide deployed config mode package that points to a directory that won't (probably) not exist anymore when other dependent targets are built.

Since it's not possible to force meson to produce a `tinyxml2-target.cmake` file, the most reasonable solution is trying to look for the library by pkg-config.

With this patch applied, the generated `fastrtps-target.cmake` will have a reference to the target `PkgConfig::TinyXML2` instead of a full path.
@fedegiova fedegiova force-pushed the tinxyml2-pkgconfig branch from e3add06 to fea2dbe Compare March 10, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant