colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

Add support for just configuring cmake projects #119

Closed kunaltyagi closed 1 year ago

kunaltyagi commented 2 years ago

Fixes #116

  1. Made the new argument the penultimate one so as to not interfere with other PRs
  2. Don't really know how to handle rc
  3. Put the log messages after identifying the project name (can also not use it, but it felt proper to use the name)
kunaltyagi commented 2 years ago

@dirk-thomas PTAL

Can we run the CI at least in the meantime?

kunaltyagi commented 2 years ago

Oh yeah, completely forgot I made this PR 😆 and felt the need again. Would there be any progress on this? 😅

dirk-thomas commented 1 year ago

@kunaltyagi Sorry, I am not a maintainer of this repository anymore.

kunaltyagi commented 1 year ago

You've been pivotal in a ton of projects and it's been a privilege using them.

Paging @cottsay (hope this time I get it correct 😂) instead

imcelroy commented 1 year ago

@kunaltyagi @cottsay Any news about this? This feature would be a great addition!

kunaltyagi commented 1 year ago

Seems like Scott is quite busy (elsewhere). Lack of activity in a while makes me think this PR wouldn't get merged in a hurry.

That's a bit sad since CMake is a key builder for ROS2.

cottsay commented 1 year ago

Please take a look at #124.

I'm trying to avoid adding new arguments for such a narrow use case, so I'm proposing we expand the capability of an existing one to support this scenario.

kunaltyagi commented 1 year ago

That PR seems to be orthogonal. If there are no cmake-targets, it performs install, and if there are cmake-targets, it builds them.

This PR is to stop cmake right after configuration completes, before building anything.

cottsay commented 1 year ago

If there are no cmake-targets, it performs install

This is not true in #124, and was not true with the previous behavior either. Manually specifying targets (including no targets) suppresses the default additional install invocation.

kunaltyagi commented 1 year ago

So, in order to just configure, not build, the invocation would be:

colcon build --cmake-targets

?

cottsay commented 1 year ago

So, in order to just configure, not build, the invocation would be:

colcon build --cmake-targets

That's correct. Normal arguments could appear after --cmake-targets, e.x. colcon build --cmake-targets --event-handler console_direct+. In mixins, defaults files, and meta files, it would look like this:

  ...
  "cmake_targets": [],
  ...

As stated in the PR, the goal is to allow more than one custom target to be invoked, but it was trivial to support this scenario under that feature as well, without adding an additional command line argument to the already overwhelming list of possible arguments to colcon build.

If you're looking for something that explicitly references only configuring, we could create a mixin that specifies the empty target list so that you could invoke the build with --mixin configure-only or something like that.

kunaltyagi commented 1 year ago

Perfect :)

cottsay commented 1 year ago

Perfect :)

Thanks for your consideration, @kunaltyagi, and for pushing this issue forward. It would be helpful if you could provide feedback on #124, especially if you'd like to see it merged swiftly.

kunaltyagi commented 1 year ago
okvik commented 1 year ago

I have this feature implemented in my own copy of colcon-cmake and find it indispensable. In particular, for having cmake generate compile-commands.json for easy browsing of huge source sets without having to build them.

IMO the proposed reuse of cmake-targets is arcane UI that mixes up the stages of regular cmake usage. Having to document this oddity and adding a mixin to try and hide it's arcanity does worse for the volume of colcon build --help than just adding a straightforward self-documenting flag. </bikeshedding>