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

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

Open
esteve opened this issue Sep 10, 2024 · 11 comments
Open

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

esteve opened this issue Sep 10, 2024 · 11 comments

Comments

@esteve
Copy link
Contributor

esteve commented Sep 10, 2024

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
Copy link
Contributor Author

esteve commented Sep 10, 2024

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

@stelzo
Copy link

stelzo commented Sep 10, 2024

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
Copy link
Collaborator

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
Copy link

maspe36 commented Sep 14, 2024

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
Copy link
Collaborator

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" ™️ 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
Copy link
Contributor Author

esteve commented Sep 17, 2024

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
Copy link
Contributor Author

esteve commented Sep 17, 2024

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
Copy link
Collaborator

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

@esteve
Copy link
Contributor Author

esteve commented Sep 18, 2024

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

PyPI supports wheels, which are platform and architecture-dependent binary packages, maturin takes care of building the binary part and uploading it to PyPI as the appropriate wheel. See for example https://pypi.org/project/ruff/#files, Ruff is a Python linter written in Rust, but distributed as a PyPI package that can be installed via pipx / pip.

@esteve
Copy link
Contributor Author

esteve commented Sep 18, 2024

I've submitted #14 as a test, it generated a bunch of wheel files, for example the one I'm attaching (I'm not sure if you everyone can see the log files from the CI run). Unzip it and then install the whl file with pipx and it'll be available in your PATH:

$ pipx install ./cargo_ament_build-0.1.8-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 
  installed package cargo-ament-build 0.1.8, installed using Python 3.11.2
  These apps are now globally available
    - cargo-ament-build
done! ✨ 🌟 ✨

$ cargo ament-build
Error in cargo-ament-build

Caused by:
    the '--install-base' option must be set

$ which cargo-ament-build
/home/esteve/.local/bin/cargo-ament-build

wheels-linux-x86_64.zip

@stelzo
Copy link

stelzo commented Sep 30, 2024

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?

Maturin uses cross-compilation and reaches much more targets. Probably needed for PyPI. The binaries are somewhere in the pipeline but there is no easy integration for both tools in the same project. Ruff does use the binaries from maturin in cargo-dist but it does not look very easy nor maintainable imo.
I would keep the tools the way you initialized them, building the binaries multiple times with the respective tool. Maturin publishes to PyPI and cargo-dist to GitHub when pushing a tag. I ran the changes (without buildjet because it is not set up for my repo) and it worked.

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

4 participants