-
Notifications
You must be signed in to change notification settings - Fork 97
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
Fix pytest-rerunfailures
installation by using apt instead of pip
#1020
Fix pytest-rerunfailures
installation by using apt instead of pip
#1020
Conversation
Signed-off-by: Cristóbal Arroyo <j.arroyo@uniandes.edu.co>
@@ -0,0 +1,4 @@ | |||
@[if os_name == 'debian' or os_name == 'ubuntu' and os_code_name in ['jammy', 'noble']]@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to match the debian os_code_names
as well otherwise this if won't be triggered on debian.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I honestly wouldn't even bother with the os_code_name
check. All of the Debian or Ubuntu distributions we support have this package in them.
os_name=os_name, | ||
os_code_name=os_code_name, | ||
))@ | ||
RUN pip3 install -U setuptools==59.6.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clalancette should we pin it for any other os distro that does not have it upstream? Not sure if we still have focal on build.ros2.org.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, all of focal is gone, so we don't need to worry about it anymore.
Signed-off-by: Cristóbal Arroyo <j.arroyo@uniandes.edu.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks reasonable to me. Is there any way to test it without deploying it?
Maybe @cottsay or @nuclearsandwich know better |
You can run a devel job locally when set up:
Since the ros_buildfarm scripts assume that your local branch name when generating is available in the origin remote, you either need to change origin or push the branch to the root repository rather than a fork. I did the latter in this case. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a small part of me which feels snippeting this is overkill, but not enough to block it as is. The local test I outlined gets the official Works On My Machine stamp of approval as well.
bf2bd68
into
ros-infrastructure:master
This did regress CI after all. The job "ROS 1 Devel (3.6, colcon)" fails with:
|
See ros2/ci#744
I used the same template as
install_python3.Dockerfile.em
for pytest-rerunfailures package