Add script for finding TinyXML.

[adolfo-rt]

No description provided.

[wjwwood]

@adolfo-rt This will likely require an install rule be added, unless this folder is glob selected.

I know you were hoping to get this merged in directly, but it will likely fire up another round of debate about where these should live.

Since several packages currently use tinyxml and should reuse one FindTinyXML.cmake, I would be in favor of allowing this module to be added.

[wjwwood]

But as I mentioned before, a separate package to hold these kind of modules would probably be a better solution.

[wjwwood]

ros-controls/ros_control@8a55e3b#commitcomment-3698782

[dirk-thomas]

Since catkin does not require tinyxml this module should not be part of catkin. The topic has been discussed before on the mailing list: https://groups.google.com/forum/?hl=en#!msg/ros-sig-buildsystem/_AJ3ZaR-41A/WpcuZQ9R6g0J

There are multiple other options - all with their own pros and cons:

• keep the logic together with the package which uses it, which would not make it really reusable
• contribute the module upstream, might take longer but would be best considering long-term
• place it in a new package like cmake_modules to collect more CMake modules in the future. This will keep the burden of actively maintaining the code.

[wjwwood]

@dirk-thomas is right, I would suggest putting it in the package locally for now, and we can take your contributed module and put it into a new package which serves as a repository for such files.

[wjwwood]

I created a package here: https://github.com/ros/cmake_modules

[davetcoleman]

I've documented the current method for including TinyXML on the wiki:
http://www.ros.org/wiki/tinyxml

I think adding a custom cmake file to every package that needs tinyxml (a lot) is very inconvenient and duplicates a lot of functionality. What we really need is:

contribute the module upstream, might take longer but would be best considering long-term

[davetcoleman]

Maybe someone who can speak of CMake more elequently than I could add a ticket for TinyXML:
http://sourceforge.net/p/tinyxml/feature-requests/?source=navbar

There's already a issue for using CMake as the build system, but all we really want is for a FindTinyXML.cmake file to be installed with TinyXML, right?

[wjwwood]

• Including FindTinyXML.cmake in your package
  • Helps you now
• Put a FindTinyXML.cmake in https://github.com/ros/cmake_modules and have people depend on that
  • Helps reduce the overhead of putting a copy in your own package
• Contributing the module to CMake (upstream)
  • Takes the longest (years to get the correct version of CMake on all supported OS's), but is the best solution.

Like everything, to go from one solution level to the next requires someone to push for it.

[wjwwood]

@davetcoleman Yes, it doesn't have to use CMake as the build system in order to provide a FindTinyXML.cmake file, it could also provide a pkg-config file.

[wjwwood]

I would point out, that like the CMake contribution model, we would need to wait for a new version of tinyxml, which includes the file we need, to be available on all supported platforms. I think that the timeline for that is about a year at the least.

[davetcoleman]

Put a FindTinyXML.cmake in https://github.com/ros/cmake_modules and have people depend on that

How would they depend on that? Just downloading it manually into their package or some better way?

@adolfo-rt will you add the FindTinyXML.cmake to the new repo?

[wjwwood]

You just have to <build_depend> on it. Then you will be able to find_package any Modules that it installed.

[wjwwood]

My suggestion, after reading a bit, would be to have the cmake_modules package install <lower-case-package-name>-config.cmake files in <prefix>/cmake/. @dirk-thomas do you have thoughts on this?

[wjwwood]

We will have to create a config-extra file for the package so that it can expose these properly in the devel space.

[wjwwood]

I am referencing this: http://www.cmake.org/cmake/help/v2.8.8/cmake.html#command:find_package

[wjwwood]

I have setup the catkin package and added the tinyxml rule you proposed here:

https://github.com/ros/cmake_modules

Can I get a review of the package @dirk-thomas

[adolfo-rt]

Thanks for pushing the script on the cmake_modules package. I think it's the best compromise right now.

I think the file naming is incorrect according my understanding of CMake's convention. The find_package has two modes, module and config (see link from @wjwwood's previous post):

• Config mode is associated to xxx-config.cmake or XxxConfig.cmake files and is intended for packages built with CMake and "...attempts to locate a configuration file provided by the package". These files don't need to do any find_path or find_library, but normally know where everything is, and just give you the right variables. Details here. A more complex example (written by me ;-) is here, here and here).
• Module mode is associated to FindXxx.cmake files and is intended for packages not built with CMake. As the name implies, the script has to find an existing installation of the package, hence the calls to find_path or find_library... Details here.

In config mode, if the xxx-config.cmake script is found, you have the certainty that the package is installed. Things that can fail at this point are advanced features like missing optional COMPONENTs or the VERSION being older than what we require. Conversely, in module mode, the existence of a FindXxx.cmake script has nothig to do with the existence of a package installation, and the script is subject to not finding anything.

The above being said, I propose to rename tinyxml-config.cmake to FindTinyXML.cmake.

[wjwwood]

@adolfo-rt, I think @dirk-thomas and I came to the same conclusion, but we were trying to figure out some other details on how to distribute this stuff and what rules to make about what we accept. We would like to make them as consistent as possible.

[wjwwood]

@adolfo-rt @dirk-thomas Should it be FindTinyXML.cmake and the variables like TinyXML_INCLUDE_DIRS? Currently the variables are like TINYXML_INCLUDE_DIRS.

[wjwwood]

I updated the repository to use/recommend Modules over config files and I updated the CONTRIBUTING.md file (which shows up when a pull request is being opened):

https://github.com/ros/cmake_modules/blob/master/CONTRIBUTING.md

[wjwwood]

Where should we install these modules to?

• <prefix>/cmake/Modules
• <prefix>/share/cmake_modules/cmake/Modules

Thoughts?

[wjwwood]

I think we will install our CMake modules to a namespaced folder which is not on the CMAKE_MODULES_PATH by default, that way we won't collide with CMake in the future. When you find_package(cmake_modules) it will prepend its Modules folder to the CMAKE_MODULES_PATH.

[dirk-thomas]

+1

[wjwwood]

Ok, I updated the package to reflect this stuff, @dirk-thomas can you review it?

ros/cmake_modules@d7eb9a2

[wjwwood]

@adolfo-rt @davetcoleman Can you guys read and comment on the contribution guide for the new package?

https://github.com/ros/cmake_modules/blob/master/CONTRIBUTING.md

[dirk-thomas]

Looks great +1

[davetcoleman]

Good documentation +1

[wjwwood]

This little test package works:

https://github.com/ros/cmake_modules/tree/master/tests/test_find_tinyxml

[adolfo-rt]

@adolfo-rt @dirk-thomas Should it be FindTinyXML.cmake and the variables like TinyXML_INCLUDE_DIRS? Currently the variables are like TINYXML_INCLUDE_DIRS.

Yes, you got that right.

[adolfo-rt]

@adolfo-rt @davetcoleman Can you guys read and comment on the contribution guide for the new package?

It's concise and the wording is clear. I like it!.

I submitted a PR with minimal changes. It's great that we didn't loose momentum and got this done. My thanks to all of you.

[wjwwood]

I have released the package into Hydro and Groovy, added it to the doc jobs, and I put a note on its wiki page pointing to the readme for usage.

[davetcoleman]

So I should watch for an update to the catkin package for this to take effect in public? Then we can remove the custom cmake file from ros_control, correct?

[wjwwood]

@davetcoleman Depends on what you want. If most people are using ros_control from debs then it would be ok, because the ros_control deb in public will get there at the same time or after cmake_modules will. On the other hand, the from source people will have to get cmake_modules from source and add it to their workspace until it is in public. So it's up to you.

[davetcoleman]

Ah, so its a new package called cmake_modules then? One that I should add to the package.xml dependencies?

[wjwwood]

This sounds like a good test for the documentation:

https://github.com/ros/cmake_modules/blob/master/README.md#usage