Skip to content
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

Handling package-local CMake modules #950

Closed
zultron opened this issue Jul 19, 2018 · 6 comments
Closed

Handling package-local CMake modules #950

zultron opened this issue Jul 19, 2018 · 6 comments
Labels

Comments

@zultron
Copy link

zultron commented Jul 19, 2018

When a ROS package uses the "Foo" project with no upstream-provided CMake module, the author may wish to write e.g. cmake/FindFoo.cmake and then find_package(Foo).

However, the "CMake coding standards" document says simply, "Do not set CMAKE_MODULE_PATH" without suggesting the correct way to do this.

Others who have asked this question haven't found satisfactory answers, such as this question on ROS Answers (the OP ended up answering his own question by setting CMAKE_MODULE_PATH, contradictory to coding standards).

Anyway, other experienced ROS developers seem unaware of and violate this standard, such as here. (I understand cmake_modules makes this an obsolete example, and the line could simply be removed, but....)

The Indigo Migration guide suggests that CMake modules might be contributed to the ROS cmake_modules package. However, that package's maintainers wouldn't be interested in modules for an unpublished package, and .rosinstalling a locally-forked version with FindFoo.cmake is a lot of overhead.

So what is the correct way to find a CMake module that's local to a package? Is this something missing from catkin, something missing from the docs, or something best accomplished some other way?

@sloretz
Copy link
Contributor

sloretz commented Jul 19, 2018

Hi @zultron, is that link to the right question? I don't see CMAKE_CXX_FLAGS on that page.

@zultron
Copy link
Author

zultron commented Jul 19, 2018

Argh, really dumb copy'n'paste error. I'll fix the original description to CMAKE_MODULE_PATH!

@sloretz
Copy link
Contributor

sloretz commented Jul 19, 2018

Ah, I should have figured that out from the context.

I wasn't able to find the rationale for avoiding CMAKE_MODULE_PATH either. It was added in #216. @tkruse do you remember?

An alternative to using CMAKE_MODULE_PATH is to create your own package that installs the find module (install rule in cmake_modules). For example, create a package foobar_cmake_module which installs Findfoobar.cmake. Then add <build_depend>foobar_cmake_module</build_depend> to the package.xml of the package which needs to find it.

@dirk-thomas
Copy link
Member

Without further information why this "rule" was added to the docs I suggest to remove it since I don't see a reason this usage shouldn't be "allowed". See #951.

@zultron
Copy link
Author

zultron commented Aug 3, 2018

I submitted a new issue in fkie/catkin_lint#55 in case someone over there wants to weigh in over here. If #951 is merged, catkin lint will need the equivalent update.

I agree with @dirk-thomas. If there's no known reason to disallow modifying CMAKE_MODULE_PATH, relaxing this standard reduces confusion and effort for ROS package authors.

If there does turn out to be a good reason for it, @sloretz's suggestion to create a special package is as elegant a workaround as I can imagine in cases where contributing new rules to the ROS cmake_modules package is impractical. Thanks!

@zultron
Copy link
Author

zultron commented Aug 3, 2018

Since #951 is merged, I went ahead and submitted PR fkie/catkin_lint#56, reflecting the change in catkin lint.

Thanks, @dirk-thomas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants