colcon / colcon-cargo

An extension for colcon-core to support Rust projects built with Cargo
http://colcon.readthedocs.io
Apache License 2.0
30 stars 20 forks source link

Example package.xml building with colcon-cargo? #5

Open ruffsl opened 4 years ago

ruffsl commented 4 years ago

I'm trying to write a pure Rust package and build it in a colcon workspace. However, I can't seem to add a basic package.xml without colcon thinking it somehow deals with Cmake:

colcon build --symlink-install --packages-select bbr_cli
Starting >>> bbr_cli 
--- stderr: bbr_cli
CMake Error: The source directory "/home/ruffsl/ws/bbr_ros2/src/bbr_ros2/bbr_cli" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.
---
Failed   <<< bbr_cli    [ Exited with code 1 ]

Summary: 0 packages finished [0.22s]
  1 package failed: bbr_cli
  1 package had stderr output: bbr_cli

Having the colcon-cargo extension installed, the cargo project seems to build fine without a package.xml but if I add a skeleton package.xml like so, I get the error above:

<?xml version="1.0"?>
<package format="2">
  <name>bbr_cli</name>
  <version>0.0.1</version>
  <description>Command Line Interface for BBR</description>
  <maintainer email="roxfoxpox@gmail.com">Ruffin White</maintainer>
  <license>Apache License 2.0</license>

</package>

cc @lelongg

lelongg commented 4 years ago

colcon use different extensions to build different packages type. Extensions are tried according to their priority. If an extension identify a package, it will be used to build the package and extensions with less priority won't be triggered.

In this case, I believe that colcon-ros has higher priority than colcon-cargo and colcon-ros identify any package containing a package.xml file as its own, so colcon-cargo is ignored when such a file is present (https://github.com/colcon/colcon-ros/blob/7e51402a233e44350d0e957524bba7f8746d6b1d/colcon_ros/package_identification/ros.py#L28).

I think that the solution would be to add a new _buildtype, such as _amentcargo, to colcon-ros in order to correctly deal with ROS packages containing Rust code.

ruffsl commented 4 years ago

I think that the solution would be to add a new build_type, such as ament_cargo, to colcon-ros in order to correctly deal with ROS packages containing Rust code.

That's what I was thinking as well, but I wasn't sure where to start. Is there a document that details the requirements for adding a new ament build type? The ament_cmake repo is huge, and it looks like the ament_python repo is deprecated, so I could use some guidance on mimicking this for cargo.

cc @dirk-thomas

dirk-thomas commented 4 years ago

ament_cmake is a separate build type since CMake is being invoked with ament_cmake specific arguments, e.g. https://github.com/colcon/colcon-ros/blob/7e51402a233e44350d0e957524bba7f8746d6b1d/colcon_ros/task/ament_cmake/build.py#L58-L60

Same for ament_python: e.g. https://github.com/colcon/colcon-ros/blob/7e51402a233e44350d0e957524bba7f8746d6b1d/colcon_ros/task/ament_python/build.py#L38-L41

The question is if a ROS-specific cargo package needs any specific arguments / logic compared to a vanilla cargo package? If yes, this need to be implemented in a separate task (if that should go into colcon-ros or better be contributed through a separate extension is a different question).

nnmm commented 3 years ago

if that should go into colcon-ros or better be contributed through a separate extension is a different question

Could somebody elaborate? My reading of https://github.com/colcon/colcon-ros/issues/24 was that separate extensions are preferred now. Context: I'm planning to rewrite a prototype for cargo support in colcon-ros into a separate extension.

esteve commented 3 years ago

@nnmm separate extensions are the recommended way. See for example https://github.com/colcon/colcon-ros-grade which leverages https://github.com/colcon/colcon-grade to support ament_gradle packages (Gradle packages that use the ROS and ament infrastructure for resolving dependencies and accesss the ament index)