Skip to content

Check non-uppercase variable when missing import target#30

Closed
f0reachARR wants to merge 2 commits intoros2:rollingfrom
f0reachARR:fix/uppercase-variable
Closed

Check non-uppercase variable when missing import target#30
f0reachARR wants to merge 2 commits intoros2:rollingfrom
f0reachARR:fix/uppercase-variable

Conversation

@f0reachARR
Copy link
Copy Markdown

Description

This PR fixes the error message Unable to extract the library file path from , which occurs under specific conditions.

This issue happens when the following two conditions are met:

  • TinyXML2 is located by a CMake module other than the FindTinyXML2.cmake in tinyxml2_vendor.
  • That CMake module sets the variable TinyXML2_LIBRARY instead of the expected uppercase TINYXML2_LIBRARY.

This edge case was identified in tier4/scenario_simulator_v2#1604. Specifically, the transitive dependency mrt_cmake_modules provides its own FindTinyXML2.cmake, which triggers this problem.

To resolve this, I have modified the logic to check for non-uppercase variables (including TinyXML2_LIBRARY) as a fallback if TINYXML2_LIBRARY is not defined.

Is this a user-facing behavior change?

This does not affect almost use cases, but it requires careful review to ensure compatibility.

Did you use Generative AI?

No

Additional Information

Signed-off-by: f0reachARR <f0reach@f0reach.me>
Signed-off-by: f0reachARR <f0reach@f0reach.me>
@f0reachARR f0reachARR force-pushed the fix/uppercase-variable branch from 59479f2 to 08519ed Compare January 8, 2026 09:10
@f0reachARR f0reachARR marked this pull request as ready for review January 8, 2026 09:57
@asymingt
Copy link
Copy Markdown
Member

Hi @f0reachARR 👋 It looks like you are using a non-standard cmake module to search for tinyxml2. As I understand it, the distribution-provided FindTinyXML2.cmake file typically provides upper-case variables, which I recognize is against the developer standard but has been in practice for a while. To avoid ROS packages becoming a catch-all for edge-cases, could I suggest we first try and have this inconsistency fixed upstream? One idea is for the mrt_cmake_modules version to provider both cases for wider compatibility.

@f0reachARR
Copy link
Copy Markdown
Author

f0reachARR commented Jan 20, 2026

Thanks for reply and suggestion! That makes sense: it's better to avoid to handle edge-cases in ROS packages.
I'll look into proposing a fix to mrt_cmake_modules to provide both cases.

Thanks!

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.

2 participants