Skip to content

Conversation

@Rechi
Copy link
Member

@Rechi Rechi commented Nov 3, 2025

TinyXML-2 can be built with meson, which doesn't generate the CMake config files.

fixes #337

TinyXML-2 can be built with meson, which doesn't generate the CMake config files.
@garbear garbear requested a review from Copilot November 3, 2025 09:30
Copy link
Member

@garbear garbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for adding this file!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a CMake find module for the TinyXML2 library to enable the project to locate and link against TinyXML2 in various installation scenarios (system-wide CONFIG-based installation, pkg-config, or manual paths).

  • Adds fallback mechanisms for finding TinyXML2 using CONFIG mode, pkg-config, and manual path search
  • Creates an imported target tinyxml2::tinyxml2 for modern CMake usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +4
# FindTinyXML2
# -----------
# Finds the TinyXML2 library
#
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation header is incomplete compared to other find modules in the project (FindUDEV.cmake and FindSdl2.cmake). It should follow the .rst: format and document the exported variables and imported targets. Add documentation listing TINYXML2_FOUND, TINYXML2_INCLUDE_DIR, TINYXML2_LIBRARY, TINYXML2_VERSION, and the tinyxml2::tinyxml2 imported target.

Suggested change
# FindTinyXML2
# -----------
# Finds the TinyXML2 library
#
#.rst:
# FindTinyXML2
# -----------
#
# Find the TinyXML2 library.
#
# Imported Targets
# ^^^^^^^^^^^^^^^
# tinyxml2::tinyxml2
# The TinyXML2 library target, if found.
#
# Result Variables
# ^^^^^^^^^^^^^^^
# TINYXML2_FOUND
# True if the library was found.
# TINYXML2_INCLUDE_DIR
# The directory containing tinyxml2.h.
# TINYXML2_LIBRARY
# The path to the TinyXML2 library.
# TINYXML2_VERSION
# The version of TinyXML2 found, if available.
#
# This module will define the imported target tinyxml2::tinyxml2 if found.
#
#.rst:

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wanna grab this AI spew? It doesn't hurt and is kinda nice to have the summary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will pick some parts from FindUDEV.cmake. I haven't looked at the file before.

Comment on lines 26 to 29
add_library(tinyxml2 INTERFACE)
target_include_directories(tinyxml2 SYSTEM INTERFACE ${TINYXML2_INCLUDE_DIR})
target_link_libraries(tinyxml2 INTERFACE ${TINYXML2_LIBRARY})
add_library(tinyxml2::tinyxml2 ALIAS tinyxml2)
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using an INTERFACE library with an ALIAS creates a non-relocatable target. This pattern differs from FindUDEV.cmake which uses UNKNOWN IMPORTED library type with set_target_properties. Consider using add_library(tinyxml2::tinyxml2 UNKNOWN IMPORTED) and set_target_properties() to set IMPORTED_LOCATION and INTERFACE_INCLUDE_DIRECTORIES, or guard the target creation with if(NOT TARGET tinyxml2::tinyxml2) to avoid conflicts with CONFIG mode.

Suggested change
add_library(tinyxml2 INTERFACE)
target_include_directories(tinyxml2 SYSTEM INTERFACE ${TINYXML2_INCLUDE_DIR})
target_link_libraries(tinyxml2 INTERFACE ${TINYXML2_LIBRARY})
add_library(tinyxml2::tinyxml2 ALIAS tinyxml2)
add_library(tinyxml2::tinyxml2 UNKNOWN IMPORTED)
set_target_properties(tinyxml2::tinyxml2 PROPERTIES
IMPORTED_LOCATION "${TINYXML2_LIBRARY}"
INTERFACE_INCLUDE_DIRECTORIES "${TINYXML2_INCLUDE_DIR}"
)

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be updated?

@Rechi Rechi force-pushed the cmake/fixTinyXML2 branch from 221e404 to b5ecdb0 Compare November 3, 2025 10:36
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.

Build of 22.0.5-Piers fails when building against “meson built tinyxml-11.0.0”

3 participants