ros2-rust / cargo-ament-build

5 stars 9 forks source link

Distribute cargo-ament-build as a PyPI package #12

Open esteve opened 1 week ago

esteve commented 1 week ago

Distributing cargo-ament-build as a crate is fine, but makes it more difficult for the colcon extensions to be properly package since they require cargo-ament-build, but we can't specify that dependency anywhere. This complicates things more when uploading the extensions to PackageCloud.

https://github.com/PyO3/maturin seems to support packaging Rust crates as PyPI packages (thanks @stelzo for the link). cargo-ament-build could be compiled natively via cargo-dist and then packaged as a script in the scripts folder inside the PyPI archive. This might be bending the rules of what a PyPI package is, so any feedback would be great.

esteve commented 1 week ago

@jhdcs @luca-della-vedova @maspe36 @stelzo, thoughts?

stelzo commented 1 week ago

Maybe it would be nice to ship the binary inside one of the colcon packages directly without creating a package for cargo-ament-build. This would better fit the semantics of the scripts folder. It is a little closer to something like numpy. Shipping prebuilt libraries could also be seen as a binary dependency imo.

luca-della-vedova commented 6 days ago

I admit I am not very familiar with the details of how this package can be used with pure cargo and without colcon-ros-cargo but a part of me wonders if this really needs to be a Rust package in the first place. Since the main (only? not sure) use of this binary is as a subprocess of a python script colcon-ros-cargo part of me feels that distribution would be a lot simpler if this was just a Python executable, but I admit I have never tried to run it standalone.

maspe36 commented 3 days ago

Off hand, I don't see anything wrong with shipping cargo-ament-build as a PyPI package. In fact, one of the maintainers of maturin has done exactly this a number of times

Considering the ROS ecosystem, this seems like a great way to offer symmetry for users.

Our instructions could look something like this

pip3 install colcon-cargo colcon-ros-cargo cargo-ament-build

Now to @luca-della-vedova's point, maybe it doesn't make sense to even write this component in Rust. It would probably be a pain to maintain something like cargo-manifest ourselves though.

luca-della-vedova commented 2 days ago

Now to @luca-della-vedova's point, maybe it doesn't make sense to even write this component in Rust. It would probably be a pain to maintain something like cargo-manifest ourselves though.

Fair enough, there are toml crates but not really any cargo.toml crate, my hunch is that it wouldn't really be a ton of work since this crate is fairly simple but I admit haven't looked at it much. My point was really to get support for ROS users a bit more straightforward, we should be able to release both colcon-cargo and colcon-ros-cargo to the buildfarm "soon" :tm: so users can install them through apt. However, debian packages can only depend on other debian packages. Assuming we get this on rosdep, releasing it on pypi will be more of a side step (users can install it through rosdep install) not in the direction of full support (it still won't be installed when users do apt install python3-colcon-ros-cargo), we will still need full Rust support on the buildfarm to release this which has a much more unclear state right now.

esteve commented 17 hours ago

My point was really to get support for ROS users a bit more straightforward, we should be able to release both colcon-cargo and colcon-ros-cargo to the buildfarm "soon" ™️ so users can install them through apt.

Correct me if I'm wrong, but the colcon extensions are not released to the ROS buildfarm. They are built locally and then uploaded to Dirk's packagecloud (https://packagecloud.io/dirk-thomas/colcon). So in our case, we'd add support for maturin in cargo-ament-build, upload it to PyPI and then also generate Debians (I guess with stdeb?).

AFAIK, packagecloud does not build packages, just stores them, so there's no worry about it supporting Rust or not. Given that the PyPI package will already contain a binary version of cargo-ament-build (built via cargo-dist and saved as part of the release on GitHub), we can just upload such package as is to packagecloud.

esteve commented 11 hours ago

I've submitted #13 to add support for ARM builds with cargo-dist and Buildjet, but I don't really know how maturin and cargo-dist work together, or does maturin replace cargo-dist entirely? @stelzo do you know more about this?

luca-della-vedova commented 1 hour ago

Correct me if I'm wrong, but the colcon extensions are not released to the ROS buildfarm. They are built locally and then uploaded to Dirk's packagecloud (https://packagecloud.io/dirk-thomas/colcon). So in our case, we'd add support for maturin in cargo-ament-build, upload it to PyPI and then also generate Debians (I guess with stdeb?).

AFAIK, packagecloud does not build packages, just stores them, so there's no worry about it supporting Rust or not. Given that the PyPI package will already contain a binary version of cargo-ament-build (built via cargo-dist and saved as part of the release on GitHub), we can just upload such package as is to packagecloud.

Ah yes sorry indeed colcon is built locally and uploaded to packagecloud. I am still worried about distributing compiled binaries with Pypi / packagecloud though. Currently all the packages that are distributed are pure Python packages hence we don't need to worry about which platform they are running on. If we distribute debians with binaries / Pypi packages with precompiled binaries how would that work with different architectures? I.e. as you brought up in your last comment about supporting aarch64, I don't see it being currently done in the colcon packagecloud (although I imagine it should be possible to distribute multiple debian packages depending on the architecture) and I'm not sure how it can be done in Pypi either