search

ros-tooling / setup-ros

Github Action to set up ROS 2 on hosts
Apache License 2.0
84 stars 39 forks source link

Pin version of empy to <4 #632

Closed Taka-Kazu closed 9 months ago

Taka-Kazu commented 9 months ago

The main purpose of this PR is to use old version (<4) of Empy.

See https://github.com/colcon/colcon-core/pull/603 for details.

Other changes are made by the git hooks.

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (7293e28) 92.47% compared to head (f7ce8ef) 92.47%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #632 +/- ## ======================================= Coverage 92.47% 92.47% ======================================= Files 8 8 Lines 186 186 Branches 22 22 ======================================= Hits 172 172 Misses 14 14 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

christophebedard commented 9 months ago

Thanks for the PR! Could we instead try to fix it by bumping to colcon-core==0.15.1? I think that would be preferable, but I'm not sure if we still need to then pin empy<4.

christophebedard commented 9 months ago

Also, please rebase on master to fix CI.

Taka-Kazu commented 9 months ago

@christophebedard Thank you for your comment.

Thanks for the PR! Could we instead try to fix it by bumping to colcon-core==0.15.1? I think that would be preferable, but I'm not sure if we still need to then pin empy<4.

I tried several configuration about colcon-core and empy.

ROS_DISTRO colcon-core empy result
humble 0.11.0 (no constraint) passed
humble 0.15.1 (no constraint) passed
humble 0.11.0 <4 passed
humble 0.15.1 <4 passed
noetic 0.11.0 (no constraint) failed
noetic 0.15.1 (no constraint) failed
noetic 0.11.0 <4 passed but with many version conflict errors
noetic 0.15.1 <4 passed

It seems that the problem is already fixed for humble without additional modification. But for noetic, the workflow is still broken even with colcon-core==0.15.1. It might be because colcon is installed via apt for humble but pip for noetic in setup-ros workflow.

So I think it's better to specify both colcon-core==0.15.1 and empy<4. What do you think?

christophebedard commented 9 months ago

Yeah, you're right. For some reason, I thought that empy was only used by colcon-core, but of course that's not true!

So I think it's better to specify both colcon-core==0.15.1 and empy<4. What do you think?

Sounds good!

christophebedard commented 9 months ago

Released as 0.7.1/v0.7.

Taka-Kazu commented 9 months ago

It works! Thank you!

beniaminopozzan commented 8 months ago

@christophebedard , could this fix be backported to v6 for those who build against EOL distribution?

christophebedard commented 8 months ago

could this fix be backported to v6 for those who build against EOL distribution?

Please see #634