Closed methylDragon closed 2 years ago
@methylDragon I believe I've addressed the major concerns raised at https://github.com/colcon/colcon-core/issues/493 which should get us moving in here.
Currently the tools in here are well tested and known to work (and provided there're no glitches in future Vitis releases, they should remain as such). I like your recommendations above though, and believe this should be implemented but I'd like to prioritize the overall release process of hardware acceleration packages (tracked at https://github.com/ros2-gbp/ros2-gbp-github-org/pull/22#issuecomment-1073811491) before diving into these refinements, specially, because it'll need to be tested from a functionality perspective, which will take some additional time.
For now, I've assigned you to the Recommendations
item in that list so that if you have bandwidth, can help send PRs implementing them. If not, these recommendations should be implemented in future releases regardless.
- [ ] Create publish-python.yaml file to define the release process
I believe https://github.com/ros-acceleration/colcon-acceleration/commit/6d4ddbaaee5e66d67743e8e299c35e3f547507b5 does it. Please assess @methylDragon.
All the changes so far are good! The colcon team mentioned that, for the README, it would be good to specify colcon workspace instead of ROS workspace, since that's more technically correct and less ROS specific.
But this should only happen if the package can be used without ROS, which it looks like it can? Correct me if I'm wrong :o
There's internal support for allowing the package release if the name is renamed to colcon-hardware-acceleration
.
Remember to change the PRs, and the internal name references in any source files!
@methylDragon https://github.com/colcon/colcon-core/issues/493#issuecomment-1081739573, let me know if we're good or I'm missing something.
Remember to change the PRs, and the internal name references in any source files!
I believe I picked all source changes, any additional action needed?
There's some comments with ROS 2 instead of colcon, but I'm ok with them. I'll ping the team!
There's some comments with ROS 2 instead of colcon, but I'm ok with them. I'll ping the team!
Feel free to PR things out if I forget something please!
Got it, though it might mean another release tag needs to be made, haha...
Let's see what the team thinks of using acceleration
as a verb in the meantime.
Accepted #14, created a new tag after changes and pushed appropriately to rolling
and humble
branches.
As a quick comment, from the documentation provided, my understanding is that the main
branch should be maintained as the release branch, is this correct @methylDragon? If so, should PRs like #14 be submitted against development branches like rolling
(instead of to main
)?
Let me know your thoughts about this so that I clarify it to the team internally. Thanks!
Hey @vmayoral , let me answer your query in a separate comment.
For now, for this repo, the acceleration colcon team has been created in https://github.com/colcon (you should accept the invite.)
Then, migrate this repo over there so we can proceed with the release, as per: https://github.com/colcon/colcon-core/issues/493
Accepted #14, created a new tag after changes and pushed appropriately to
rolling
andhumble
branches.As a quick comment, from the documentation provided, my understanding is that the
main
branch should be maintained as the release branch, is this correct @methylDragon? If so, should PRs like #14 be submitted against development branches likerolling
(instead of tomain
)?Let me know your thoughts about this so that I clarify it to the team internally. Thanks!
Specifically for colcon
packages, there's no relation to ROS releases, so rolling
and humble
branches shouldn't exist for this repo.
As for the difference between master
and rolling
/humble
/etc. branches for ROS 2 packages, I need to confirm internally, but my understanding is that master is for the next release, and the individual branches are for development AND release tagging for the specific distro they are targeting. So for each branch the source is separately maintained, though sometimes backports occur.
tl;dr:
This means that as you support more and more distros, you need to be upkeeping more and more releases (until you decide to make them EOL.)
But I am only about 60% sure on this. If you take a look at the other packages out in the ecosystem, it's kind of a jungle. But my understanding seems to check out for rclc
and rclcpp
at least. There isn't really a convention, just what the package maintainers prefer.
Could I double check which documentation you are referring to?
I might have accidentally phrased it vaguely, apologies for that :X
Could I double check which documentation you are referring to?
I might have accidentally phrased it vaguely, apologies for that :X
I had a quick look again and couldn't find the source of my assumption. Sorry. I've been going back and forth between older docs and your newly provided guidelines, so it might be somewhere there (or maybe I just simply misunderstood something). Regardless, thanks for clarifying things above!
I'll stick to the following for now since it seems the simplest approach (and the one I see in other colcon extensions):
Specifically for colcon packages, there's no relation to ROS releases, so rolling and humble branches shouldn't exist for this repo.
With this clarified, I went ahead and migrated code to https://github.com/colcon/colcon-hardware-acceleration.
Cathing up with https://github.com/colcon/colcon-core/issues/493#issuecomment-1086233533, and reacted accordingly deleting the newly created repo and transferring ownership as recommended. Will look now into the guidelines provided there as well.
I've already created a packagecloud account, and I can use my existing PyPI to do the release. Let me give this one a try, I haven't done it before either :eyes:
I've already created a packagecloud account, and I can use my existing PyPI to do the release. Let me give this one a try, I haven't done it before either 👀
All right, I was just doing the same :) but I'll stop it and let you push it. For the sake of human-transfer-learning, do you think you could drop some notes on how the process went from your side 😄?
No worries ;) I'm already writing internal notes. I'll pass them to you when done, though no promises on 100% accuracy or correctness..
Oh, you might want to update the CHANGELOG. @vmayoral
Oh, you might want to update the CHANGELOG. @vmayoral
Will do.
Quick question, @nuclearsandwich suggested in https://github.com/colcon/colcon-core/issues/493#issuecomment-1086233533 to add more maintainers (nuclearsandwich in PyPI). Is this something we need to add to https://github.com/colcon/colcon-hardware-acceleration/blob/main/setup.cfg#L8-L11 or something you set within packagecloud?
Oh, you might want to update the CHANGELOG. @vmayoral
https://github.com/colcon/colcon-hardware-acceleration/pull/15
Perfect timing. I was trying to run the releases and hit a small snag, and it's a perfect time to pass it on to you :eyes:
Since the publish-python.yaml
specifies ros-acceleration/colcon-hardware-acceleration
as the packagecloud repository, you'll need to do the release for that one. You need to make a packagecloud account specifically called ros-acceleration
.
The PyPI package is up already. https://pypi.org/project/colcon-hardware-acceleration/0.6.1/ I'll add you as maintainer on the maintainer email listed on this package.
Then, follow these modified steps, augmenting the [Release Guide](https://colcon.readthedocs.io/en/released/developer/release.html):
1. Install deps
curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg
echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null
sudo apt update
sudo apt install gh
sudo apt install git ruby-full python3-yaml debhelper dh-python fakeroot python3-all
pip install wheel twine stdeb
pip install --upgrade requests
sudo gem install package_cloud
Place the bin
and publish_python
directories in the root of the colcon package you want to release.
3. Configure authentication
# For PyPI
keyring set https://upload.pypi.org/legacy/ <username>
# Generate GPG key (use 4096 bits)
gpg --full-generate-key
# For packagecloud
# Get token from https://packagecloud.io/api_token and add it
# Configure gh
gh auth login
4. Do the release
# Build the release artifacts (without uploading them)
./bin/publish-python
# Build and upload the release artifacts (to PyPI and Pakagecloud)
./bin/publish-python -upload
# Target a particular platform
# wheel:pypi, stdeb:packagecloud
./bin/publish-python -upload <TARGET>
5. Make PR
In the case the Debian package is used by the ROS community you need to open a ticket in the GitHub repository ros-infrastructure/reprepro-updater. The pull request should update the two files colcon.ubuntu.upstream.yaml and colcon.debian.upstream.yaml file with a bumped version number for the package.
Sorry, I can't add you on PyPI by email (it's only possible via username..), pass me your PyPI account username instead!
A few comments of my adventure for completeness following your indications @methylDragon:
I'm using an Ubuntu 20.04
OS in my workstation with latest packages installed and updated (as of today). You guys may want to consider adding some of this to official documentation (and/or dockerizing the whole process).
I stopped at Hurdle 4
above and didn't continue further. @methylDragon feel free to provide support if necessary but given the hurdles, my suggestion would be to either :one: switch to https://github.com/colcon/colcon-hardware-acceleration/blob/main/publish-python.yaml#L9 and have someone with a working publish-python
do it, or 2️⃣ provide support to make it work.
I need to confirm internally, but my understanding is that master is for the next release, and the individual branches are for development AND release tagging for the specific distro they are targeting. So for each branch the source is separately maintained, though sometimes backports occur.
There is no rule or requirement regarding the branch structure of repositories. The ROS 2 core packages use the convention where the default branch (usually master
) is the development branch for Rolling and any stable distributions which Rolling is still compatible with. When making incompatible changes on Rolling, the previous branch state is generally named after the stable distribution it targets (i.e. galactic
) and maintained as the development branch for Galactic.
I'm already writing internal notes. I'll pass them to you when done, though no promises on 100% accuracy or correctness..
I think adding developer docs to a repository with notes for making releases is a good idea. Please feel free to open pull requests for any improvements or suggestions for the colcon release docs: https://github.com/colcon/colcon.readthedocs.org/blob/main/developer/release.rst and if you want review on either let me know.
Quick question, @nuclearsandwich suggested in colcon/colcon-core#493 (comment) to add more maintainers (nuclearsandwich in PyPI).
I've been invited and accepted on PyPI. I don't expect to use the release access there unless prompted by the primary maintenance team or if there is no activity from the maintenance team.
Is this something we need to add to https://github.com/colcon/colcon-hardware-acceleration/blob/main/setup.cfg#L8-L11 or something you set within packagecloud?
I don't need to be listed in the setup.cfg, you can consider me a "releaser" rather than a "maintainer" and a backstop releaser at that. If I start performing active maintenance on this project I'll update the setup.cfg accordingly.
Since the
publish-python.yaml
specifiesros-acceleration/colcon-hardware-acceleration
as the packagecloud repository, you'll need to do the release for that one. You need to make a packagecloud account specifically calledros-acceleration
.
I think there might be some confusion or disconnect about packagecloud. I was expecting that we'd add the colcon-hardware-acceleration team to the main colcon packagecloud repository so that they could push releases there rather than setting up a separate repository. That plan is somewhat forestalled by the fact that none of the currently active colcon maintainers have the ability to add new maintainers to the repository. We're working on that.
I'm using an Ubuntu 20.04 OS in my workstation with latest packages installed and updated (as of today). You guys may want to consider adding some of this to official documentation (and/or dockerizing the whole process).
Did you see the setup instructions in the publish-python README? I think at least 1 & 2 are covered by those setup instructions, which are linked to in the release instructions although the link could be rephrased to indicate that the "details" include mandatory setup instructions. I'll take care of that.
Hey @vmayoral , could you double check if all the dependency steps I posted managed to run. At least for hurdles 1 and 2 the steps should have resolved them.
Regardless, the PyPI package is already up, and I'm still available if any updates need to be made.
For packagecloud, my mistake; let's wait on the main colcon packagecloud repo, though for now if you need either the public or internal teams to use colcon-hardware-acceleration
, it should already be usable with pip install colcon-hardware-acceleration
, since it's already on PyPI.
PS: Could you sign up for PyPI and pass me your username so I can add you in as an owner on the repo?
Additionally: I think the colcon team wants to prepare to add you to the packagecloud org once they are able to, so you should also sign up for packagecloud and pass me your username for that too!
PS: Could you sign up for PyPI and pass me your username so I can add you in as an owner on the repo?
ros-acceleration
Additionally: I think the colcon team wants to prepare to add you to the packagecloud org once they are able to, so you should also sign up for packagecloud and pass me your username for that too!
ros-acceleration
Did you see the setup instructions in the publish-python README? I think at least 1 & 2 are covered by those setup instructions, which are linked to in the release instructions although the link could be rephrased to indicate that the "details" include mandatory setup instructions. I'll take care of that.
Thanks @nuclearsandwich and @methylDragon, that indeed addresses 1 and 2. I was probably distracted. I went ahead an reproduced this again in a clean container and that worked just fine. Hurdles 3 and 4 above seem to be related to my workstation setup, so can be discarded safely.
Let me know if you need anything else from my side.
PyPI collaborator invite sent! I've added you as owner. @vmayoral
PyPI collaborator invite sent! I've added you as owner. @vmayoral
Accepted the invite. I'm in now! Thanks.
Thanks to @cottsay the 0.7.0 version of this package has been built and pushed to packagecloud, and from there is being imported into the ROS repositories via https://github.com/ros-infrastructure/reprepro-updater/pull/156
The package will be available in the ROS and ROS 2 repositories once those imports complete and the subsequent upload jobs are triggered which should be within the next few minutes.
@methylDragon, @vmayoral while direct access to packagecloud for the colcon-hardware-acceleration team is pending, please create new issues when you need packagecloud uploads for subsequent releases and assign myself and @cottsay.
I see it on http://repo.ros2.org/status_page/rolling_default.html and http://repo.ros2.org/status_page/humble_default.html !! Thanks so much for the help! :birthday: :tada:
[STATUS: LIVE]
Description
This issue tracks the status for the release of this repository into the
colcon
org and making it available for installation as.deb
via the ROS 2 repositories.It will detail the steps that need to be taken, and what recommended changes are good to have.
Release Process
Steps
publish-python.yaml
file to define the release processcolcon-core
colcon/colcon-core
to request for this repo to be transferred into thecolcon
org (https://github.com/colcon/colcon-core/issues/493)colcon
team to review package name and general approachcolcon
workspaces!)Recommendations
pytest
separate and independent from this repo that should be fixed more generallycolcon
workspaces.Python notes
flake8
passing on thisPopen
subshells/subshells unnecessarily, especially if it can be substituted with native Python invocations. It'll hidestderr
printouts, and also slow things down. Examples:cp
is called https://github.com/ros-acceleration/colcon-acceleration/search?q=cp can potentially be replaced withshutil.copytree(dirs_exist_ok=True)
calls or similarmkdir
andls
can be replaced with the equivalent pathlib calls and then filtered accordingly either using pathlib's glob signature, or something likere
os.getcwd()
instead ofpwd
, sinceos.getcwd()
will be guaranteed to work across platforms (unless you deliberately want it to resolve symlinks instead of getting the original path).join()
instead.+
concatenates for the same reason! (example in repo (it's not the only file it happens in)) Every+
is another reallocate and copy operation because Python unrolls the expression and does the adds incremently.