Skip to content

ci: restore blocking Rust package checks #684

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.

Sign up for GitHub

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

Open
espressolee wants to merge 12 commits into
base: main
Choose a base branch
from
Open

ci: restore blocking Rust package checks #684

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bc8e4ac
ci: match colcon's (ros.ament_cargo) type in the per-package loops
espressolee Sep 3, 2026
a86ee30
style: apply rustfmt to the two rclrs files it flags
espressolee Sep 3, 2026
ad828ae
build: exclude the CI workspace from the root cargo workspace
espressolee Sep 3, 2026
ec6e702
ci: run the per-package loops only over the packages this job builds
espressolee Sep 3, 2026
65ec9ec
ci: make the revived clippy/test/rustdoc checks non-blocking for now
espressolee Sep 4, 2026
9a5cdda
Fix auto-fixable clippy warnings
espressolee Sep 6, 2026
d3b3144
Resolve blocking rclrs lint and rustdoc errors
espressolee Sep 6, 2026
3133082
Serialize tests that replace the global logger
espressolee Sep 6, 2026
71537a4
Make restored Rust checks blocking and scoped
espressolee Sep 6, 2026
a2948d1
Drop logging lock superseded by upstream fix
espressolee Sep 6, 2026
0ecc15c
Run Linux workflow scripts with Bash
espressolee Sep 6, 2026
f4044d5
Make parameter separator comparison portable
espressolee Sep 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions .github/workflows/rust-minimal.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:

- name: Check formatting of Rust packages
run: |
for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" { print $2 }'); do
cd $path
rustup toolchain install nightly
cargo +nightly fmt -- --check
Expand All @@ -108,10 +108,11 @@ jobs:
vcs-repo-file-url: ros2_rust_${{ matrix.ros_distribution }}.repos

- name: Run clippy on Rust packages
continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. /opt/ros/${{ matrix.ros_distribution }}/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" { print $2 }'); do
cd $path
echo "Running clippy in $path"
# Run clippy for all features except use_ros_shim (needed for docs.rs)
Expand All @@ -124,10 +125,11 @@ jobs:
done

- name: Run cargo test on Rust packages
continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. install/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
cd $path
echo "Running cargo test in $path"
# Run cargo test for all features except use_ros_shim (needed for docs.rs)
Expand All @@ -140,10 +142,11 @@ jobs:
done

- name: Rustdoc check
continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. /opt/ros/${{ matrix.ros_distribution }}/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
cd $path
echo "Running rustdoc check in $path"
cargo rustdoc -- -D warnings
Expand Down
11 changes: 7 additions & 4 deletions .github/workflows/rust-stable.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:

- name: Check formatting of Rust packages
run: |
for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" { print $2 }'); do
cd $path
rustup toolchain install nightly
cargo +nightly fmt -- --check
Expand All @@ -108,10 +108,11 @@ jobs:
vcs-repo-file-url: ros2_rust_${{ matrix.ros_distribution }}.repos

- name: Run clippy on Rust packages
continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. /opt/ros/${{ matrix.ros_distribution }}/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" { print $2 }'); do

@azerupi azerupi Sep 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is being repeated multiple times, is it worth extracting into a step that adds the list of packages in an output that other steps can reuse?

@espressolee espressolee Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 71537a4. Repository and post-build Rust package paths are each computed once into step outputs and reused. Both selector steps fail explicitly if they produce an empty target set.

@esteve esteve Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer readability over reducing repetition if the solution is too complex or not easy to read, it's only a handful of packages.

cd $path
echo "Running clippy in $path"
# Run clippy for all features except use_ros_shim (needed for docs.rs)
Expand All @@ -124,10 +125,11 @@ jobs:
done

- name: Run cargo test on Rust packages

@azerupi azerupi Sep 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the step "Build and test" lines 102-108 already run the tests. At least tests were being run while this was broken. This means that now we are running tests twice. We should check which one we want to keep and avoid running tests twice.

@espressolee espressolee Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I removed the direct Linux cargo test loops. action-ros-ci remains the single Linux build/test runner; the separate per-package steps now cover only Clippy and rustdoc.

continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. install/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
cd $path
echo "Running cargo test in $path"
# Run cargo test for all features except use_ros_shim (needed for docs.rs)
Expand All @@ -140,10 +142,11 @@ jobs:
done

- name: Rustdoc check
continue-on-error: true # revived check: non-blocking until the pre-existing rclrs backlog is paid down
run: |
cd ${{ steps.build.outputs.ros-workspace-directory-name }}
. /opt/ros/${{ matrix.ros_distribution }}/setup.sh
for path in $(colcon list | awk '$3 == "(ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
for path in $(colcon list --packages-up-to $(echo "${{ steps.list_packages.outputs.package_list }}") | awk '$3 == "(ros.ament_cargo)" && $1 != "examples_rclrs_minimal_pub_sub" && $1 != "examples_rclrs_minimal_client_service" && $1 != "rust_pubsub" { print $2 }'); do
cd $path
echo "Running rustdoc check in $path"
cargo rustdoc -- -D warnings
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust-win.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
call C:\pixi_ws\ros2-windows\setup.bat
cd C:\workspace
for /f "tokens=1,2,3" %%A in ('pixi run --manifest-path C:\pixi_ws\pixi.toml colcon list') do (

@azerupi azerupi Sep 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the logs, cargo test fails but the step still succeeds and the failure is silently ignored

@espressolee espressolee Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 71537a4: the Windows batch loop now checks errorlevel immediately after each cargo test, returns a failing step on any package failure, and also fails if the selector matches zero target packages.

@espressolee espressolee Sep 6, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in : the Windows batch loop now checks immediately after each , returns a failing step on any package failure, and also fails if the selector matches zero target packages.

if "%%C"=="(ament_cargo)" (
if "%%C"=="(ros.ament_cargo)" (
if /I not "%%A"=="examples_rclrs_minimal_pub_sub" if /I not "%%A"=="examples_rclrs_minimal_client_service" if /I not "%%A"=="rust_pubsub" (
cd %%B
echo Running cargo test in %%B
Expand Down
6 changes: 6 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,10 @@
members = [
"rclrs",
]
# action-ros-ci builds the colcon workspace as `ros_ws/` inside this checkout. Without this
# exclusion every package under it inherits this manifest as its workspace root and cargo
# refuses to build them ("current package believes it's in a workspace when it's not").
exclude = [
"ros_ws",
]
resolver = "2"
2 changes: 1 addition & 1 deletion docs/building.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ colcon list
without errors and see a line like this in the output:

```
rclrs src/ros2_rust/rclrs (ament_cargo)
rclrs src/ros2_rust/rclrs (ros.ament_cargo)
```

The build type `ament_cargo` means that the `colcon-ros-cargo` plugin works as expected.
Expand Down
7 changes: 6 additions & 1 deletion rclrs/src/dynamic_message/message_structure.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
use std::{ffi::CStr, mem, num::NonZeroUsize, os::raw::c_char, os::raw::c_void};
use std::{
ffi::CStr,
mem,
num::NonZeroUsize,
os::raw::{c_char, c_void},
};

use super::TypeErasedSequence;
use crate::rcl_bindings::{
Expand Down
4 changes: 2 additions & 2 deletions rclrs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ pub use timer::*;
pub use wait_set::*;
pub use worker::*;

pub use rosidl_runtime_rs;
pub use rosidl_runtime_rs::{
Action as ActionIDL, Message as MessageIDL, RmwMessage as RmwMessageIDL, Service as ServiceIDL,
self, Action as ActionIDL, Message as MessageIDL, RmwMessage as RmwMessageIDL,
Service as ServiceIDL,
};
Loading