search

colcon / colcon-cmake

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

Remove configuration sniffing #90

Closed rotu closed 3 years ago

rotu commented 3 years ago

colcon test currently can run with the wrong build type. If CMAKE_BUILD_TYPE is unset, the cache line will be CMAKE_BUILD_TYPE:STRING=, which this logic previously assumed means 'Release'. That is not a safe assumption, and in multi-config setups the configuration may be influenced by CMAKE_DEFAULT_BUILD_TYPE, CMAKE_DEFAULT_CONFIGS, CTEST_CONFIGURATION_TYPE, or DEFAULT_CTEST_CONFIGURATION_TYPE.

dirk-thomas commented 3 years ago

Have you verified that this change still works on Windows with Visual Studio which is the case why this logic was added in the first place?

rotu commented 3 years ago

Working on it. How is CI normally run against ROS2 for colcon PRs?

dirk-thomas commented 3 years ago

How is CI normally run against ROS2 for colcon PRs?

The jobs on ci.ros2.org have a parameter CI_COLCON_BRANCH to pass a branch name in the colcon repositories. Specifying a fork isn't possible atm though.

rotu commented 3 years ago

Running CI here: https://github.com/rotu/colcon-cmake/actions/runs/222969761

dirk-thomas commented 3 years ago

Running CI here: https://github.com/rotu/colcon-cmake/actions/runs/222969761

@rotu The referenced build failed. Any update on this?

rotu commented 3 years ago

Unfortunately, no. I recommend using your regular CI --- it's clear that my attempt at using GitHub CI for this is a little half-baked.

dirk-thomas commented 3 years ago

Please see https://ci.ros2.org/job/ci_windows/12408/ which doesn't specify a build type. The test invocations fail since -C is not specified anymore:

Test not available without configuration. (Missing "-C "?)

dirk-thomas commented 3 years ago

I will go ahead and close this ticket for now since the approach doesn't work. Please feel free to iterate on this to support Ninja better. Either the PR can be reopened when the branch was revised or a new PR can be opened.