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

catkin_create_pkg should fill system deps and CATKIN_DEPENDS #18

Closed
tkruse opened this issue Nov 24, 2012 · 6 comments
Closed

catkin_create_pkg should fill system deps and CATKIN_DEPENDS #18

tkruse opened this issue Nov 24, 2012 · 6 comments

Comments

@tkruse
Copy link
Contributor

tkruse commented Nov 24, 2012

The new CATKIN_DEPENDS and DEPENDS cmake macro arg should be initialized by default

I imagine we could extend the create script so that this syntax:

catkin_create_pkg foo roscpp --system BZip2 --boost thread regex

creates this CMakeLists.txt snippets:

find_package(catkin REQUIRED COMPONENTS roscpp)
find_package(Boost REQUIRED COMPONENTS regex thread)
find_package(BZip2 REQUIRED)
catkin_package(
  # INCLUDE_DIRS include
  # CATKIN-DEPENDS roscpp
  # DEPENDS BZip2
  )
# target_link_libraries(rosbag 
#     ${catkin_LIBRARIES}
#     ${Boost_LIBRARIES} 
#     ${BZIP2_LIBRARIES})

Dirk to advise whether the above example is correct and makes sense.

I can create a pull request if others agree.

@wjwwood
Copy link
Contributor

wjwwood commented Nov 26, 2012

cc: @dirk-thomas

@dirk-thomas
Copy link
Member

Yes, the example makes sense. One thing which is missing is that Boost should also be passed as DEPENDS (like BZip2). I guess the template also has a line with include_directories() with all the variable from the found projects?

Another point may be that the name of the depending package might not be equal to the name which needs to be find_package()-ed. But adding it 1-to-1 is still a good thing in the template - the user can always modify it if necessary.

@tkruse
Copy link
Contributor Author

tkruse commented Nov 27, 2012

working on a pull request...

@tkruse
Copy link
Contributor Author

tkruse commented Nov 28, 2012

changes included in #19
now also suggests values for include_directories as Dirk suggested (and several other features)

@tkruse tkruse closed this as completed Nov 28, 2012
@tkruse tkruse reopened this Nov 28, 2012
@tkruse tkruse mentioned this issue Nov 28, 2012
@tkruse
Copy link
Contributor Author

tkruse commented Nov 28, 2012

on second thought, created a separate pull request #21 for this

@tkruse tkruse closed this as completed Jan 25, 2013
@tkruse
Copy link
Contributor Author

tkruse commented Jan 25, 2013

was implemented

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

No branches or pull requests

3 participants