Skip to content
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

CI: Install a fresh python virtual env every time (Linux) #10797

Merged
merged 2 commits into from
Feb 28, 2025

Conversation

osteffenrh
Copy link
Contributor

Description

Install a fresh python virtual environment every time to ensure
correct permissions and compatibility of the packages.
This is more robust than relying on being able to upgrade
an existing one.

pip upgrades are currently failing due to incorrect file permissions
in the container image virtual environment.

Let's not make use of the venv in the container and set up a fresh one
each time.

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

Test CI runs, for example #10796

Integration Instructions

--

@tianocore-assign-reviewers
Copy link

WARNING: Cannot add some reviewers: A user specified as a reviewer for this PR is not a collaborator of the repository. Please add them as a collaborator to the repository so they can be requested in the future.

Non-collaborators requested:

Attn Admins:


Admin Instructions:

  • Add the non-collaborators as collaborators to the appropriate team(s) listed in teams
  • If they are no longer needed as reviewers, remove them from Maintainers.txt

Copy link
Contributor

@Javagedes Javagedes left a comment

Choose a reason for hiding this comment

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

I'll preface with the fact I have no problem with this (and my approval of the PR shows that too); however, I just want to understand the problem a little more. What is the use case in which upgrading/installing dependencies is failing? Are there CI pipelines in which this is happening, or is this internal usage of the edk2 container? If so what's the different use case that it's failing there then here?

@makubacki
Copy link
Member

makubacki commented Feb 28, 2025

I'll preface with the fact I have no problem with this (and my approval of the PR shows that too); however, I just want to understand the problem a little more. What is the use case in which upgrading/installing dependencies is failing? Are there CI pipelines in which this is happening, or is this internal usage of the edk2 container? If so what's the different use case that it's failing there then here?

It is to unblock #10795. Issue was originally filed in the container repo but that was just to track the issue with the container it appears in. The solution here in edk2 is fine to me because outside the pipeline local users should be running as root in the container and/or can set up their own virtual env.

tianocore/containers#107

Install a fresh python virtual environment every time to ensure
correct permissions and compatibility of the packages.
This is more robust than relying on being able to upgrade
an existing one.

Signed-off-by: Oliver Steffen <[email protected]>
@Javagedes Javagedes added the push Auto push patch series in PR if all checks pass label Feb 28, 2025
@mergify mergify bot merged commit a1b2eeb into tianocore:master Feb 28, 2025
126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants