ros2 / ci

ROS 2 CI Infrastructure
http://ci.ros2.org/
Apache License 2.0
48 stars 30 forks source link

Install RHEL-8 pip packages in Dockerfile-RHEL. #774

Closed clalancette closed 2 months ago

clalancette commented 3 months ago

This is so that on RHEL-8, where we actually do install pip packages, we install to user locations and not the system.

clalancette commented 3 months ago

Here's a Humble+RHEL8 packaging job to see how this goes: Build Status

clalancette commented 3 months ago

And success there, so switching this to a real PR.

clalancette commented 3 months ago

Here's the rest of CI (only on RHEL, since that is the only one affected):

CI: Rolling+RHEL-9 amd64 CI: Build Status Jazzy+RHEL-9 amd64 CI: Build Status Iron+RHEL-9 amd64 CI: Build Status Humble+RHEL-8 amd64 CI: Build Status

And packaging:

Rolling+RHEL-9 amd64: Build Status Jazzy+RHEL-9 amd64: Build Status Iron+RHEL-9 amd64: Build Status Humble+RHEL-8 amd64: Build Status

cottsay commented 3 months ago

Won't this cause the pip install operations that are part of this Dockerfile to happen under a different user account from the one we're actually running the job with?

Actually, the Humble+RHEL-8 amd64 job failed because of exactly this issue. The newer version of pytest isn't getting used.

clalancette commented 3 months ago

Won't this cause the pip install operations that are part of this Dockerfile to happen under a different user account from the one we're actually running the job with?

Actually, the Humble+RHEL-8 amd64 job failed because of exactly this issue. The newer version of pytest isn't getting used.

Oh, hm. Possibly. Do you have suggestions on how to fix it? Ideas that I have include:

  1. Migrate all of the needed pip installs into Dockerfile-RHEL (though I'm not sure that would fix the issue)
  2. Add --user to the pip install commands in main.py (though I'm not sure what effect that will have on other platforms, including Windows).
  3. ???
cottsay commented 3 months ago

If (2) is where the problem is, I feel like (2) should have the solution too. How is this not a problem on Ubuntu? Are they patching the default behavior of pip or something?

clalancette commented 3 months ago

If (2) is where the problem is, I feel like (2) should have the solution too. How is this not a problem on Ubuntu? Are they patching the default behavior of pip or something?

We're getting away with it on Ubuntu Jammy and Ubuntu Noble because we aren't installing any pip packages at present.

I'm not sure why we aren't seeing a problem on RHEL-9, where we are installing some packages via pip (flake8-blind-except, flake8-class-newline, flake8-deprecated, nose, pep8, and colcon-ros-domain-id-coordinator).

cottsay commented 3 months ago

We're getting away with it on Ubuntu Jammy and Ubuntu Noble because we aren't installing any pip packages at present.

Oof, seems sketchy, but I won't rock the boat.

Let's just move the new ENV line closer to the end of Dockerfile-RHEL and call it a day. The Dockerfile installs can continue to dump stuff in the system's pythonpath and the scripted installs can use PIP_USER.

clalancette commented 3 months ago

Let's just move the new ENV line closer to the end of Dockerfile-RHEL and call it a day. The Dockerfile installs can continue to dump stuff in the system's pythonpath and the scripted installs can use PIP_USER.

Yep, got it. Done in a0fa99a, let's see if the failing CI is happy first, then I'll go run everything else:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 3 months ago

Here's a try with a totally different tactic; installing the pip packages inside the Dockerfile-RHEL for RHEL-8. This has some other disadvantages (namely duplication with what is in __main__.py), but this is kind of the direction I want to go in anyway.

Let's see how this works, and if this is better, we can discuss details:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 3 months ago

Another fix, another try:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 3 months ago

OK, so what is in 314e224 right now actually works. And while going that direction is my long-term goal, I think that for the purposes of fixing Humble here, it will be easier to just add a constraint in __main__.py for pycodestyle. I'm going to try that, which is going to mean we also need to run CI on Windows.

clalancette commented 3 months ago

Another fix, another try:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 3 months ago

CI again:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 2 months ago

CI again:

Humble+RHEL-8 amd64 CI: Build Status

clalancette commented 2 months ago

All right. After discussion and a lot of small PRs here, I think the solution that is implemented in this PR is right way to go. In particular, we pip install what we need during the Dockerfile-RHEL; see the commit message for more information on why we do that. @cottsay I could use review on this from you.

Here is CI on all flavors of RHEL: Rolling+RHEL-9 amd64 CI: Build Status Jazzy+RHEL-9 amd64 CI: Build Status Iron+RHEL-9 amd64 CI: Build Status Humble+RHEL-8 amd64 CI: Build Status

sloretz commented 2 months ago

@cottsay friendly ping 🧇