container-images: pin mesa version to COPR #1603
Conversation
slp
requested review from
rhatdan,
ericcurtin,
bmahabirbu,
maxamillion,
swarajpande5,
jhjaggars,
cgruver and
engelmi
as code owners
Reviewer's GuideThe PR updates container image build scripts to pin and lock a COPR-provided mesa-vulkan-drivers version on RHEL/Fedora-based images, adjusts repository cleanup, and adds the “--nobest” flag to DNF updates for RHEL builds. Flow diagram for DNF update in build_rag.sh with --nobest flagflowchart TD
A[Start update_python] --> B{Is dnf available?}
B -- Yes --> C[dnf update -y --allowerasing --nobest]
C --> D[dnf install python, pip, devel, pkgs]
D --> E{Is python3.11?}
E -- Yes --> F[Symlink python3.11 to python3]
E -- No --> G[End]
F --> G
B -- No --> H[Skip dnf update]
H --> G
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @slp, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request enhances the container image build process by ensuring specific, patched Mesa drivers are consistently installed on RHEL-based systems for optimal GPU acceleration. Additionally, it improves the reliability of package updates by making dnf more resilient to dependency conflicts.
Highlights
- Mesa Version Pinning for RHEL: On RHEL-based systems, the
dnf_install_mesafunction inbuild_llama_and_whisper.shnow explicitly installs and version-locksmesa-vulkan-drivers-24.2.8-103.el9from theslp/mesa-krunkitCOPR. This ensures the correct Mesa driver, which includes virtio support and 16K alignment patches, is used for GPU acceleration, particularly relevant for macOS. - Improved DNF Update Robustness: The
dnf updatecommand inbuild_rag.shhas been updated to include the--nobestflag. This modification preventsdnffrom failing due to dependency resolution issues, especially when unable to install certain packages (likemesafromappstream), allowing the update process to proceed by skipping problematic packages.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the need to pin the mesa version from a COPR repository on RHEL-based systems and to use the --nobest flag for dnf update to prevent conflicts. The changes align well with the stated objectives. My primary feedback concerns the precise application of dnf versionlock to ensure only the intended packages are pinned, preventing potential issues with future package updates.
| @@ -100,9 +100,12 @@ dnf_install_mesa() { | |||
| if is_rhel_based; then | |||
| dnf copr enable -y slp/mesa-krunkit "epel-9-$uname_m" | |||
| add_stream_repo "AppStream" | |||
| dnf install -y mesa-vulkan-drivers-24.2.8-103.el9 "${vulkan_rpms[@]}" | |||
| dnf versionlock mesa-vulkan-drivers-24.2.8-103.el9 "${vulkan_rpms[@]}" | |||
There was a problem hiding this comment.
The dnf versionlock command is used to pin specific packages to specific versions. While mesa-vulkan-drivers-24.2.8-103.el9 is correctly specified for version locking, including ${vulkan_rpms[@]} in the same command will attempt to versionlock each of those packages (vulkan-headers, vulkan-loader-devel, etc.) to their currently installed versions. The pull request description focuses on pinning the mesa version. Unless these other vulkan_rpms also need to be explicitly locked to their current state (which can hinder future updates), it's best to only apply versionlock to the mesa-vulkan-drivers package as intended by the PR description.
| dnf versionlock mesa-vulkan-drivers-24.2.8-103.el9 "${vulkan_rpms[@]}" | |
| dnf versionlock mesa-vulkan-drivers-24.2.8-103.el9 |
|
I've just noticed the previous commit has switched to building on F42 by default. Let's put this one on hold for the moment. |
|
@slp @rhatdan we do need a solution though: If we released container images today, macOS support would be broken, we have at least two options:
|
|
Can we get the mesa stuff updated for Fedora 42 or is the a reason to stick with ubi9? |
When building on Fedora systems make sure we install the mesa version from the COPR, which has the patches to force alignment to 16K (needed for GPU acceleration on macOS, but harmless to other systems). We also need to add "--nobest" to "dnf update" to ensure it doesn't get frustrated by being unable to install the mesa package from appstream. Signed-off-by: Sergio Lopez <[email protected]>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @slp - I've reviewed your changes - here's some feedback:
- The new branch uses
["${ID}" = "fedora"]for COPR enablement but no longer handles RHEL-based systems as before—consider reintroducing the EL‐9 COPR enablement or adjusting the condition to cover RHEL similarly. - Hardcoding the mesa-vulkan-drivers version (25.0.7-100.fc42) will require manual updates for every bump; consider defining it as a variable or sourcing it from a central config to simplify future maintenance.
- The script invokes
dnf versionlockbut doesn’t install thednf-plugins-corepackage, which provides the versionlock plugin—ensure that plugin dependency is in place before locking versions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new branch uses `["${ID}" = "fedora"]` for COPR enablement but no longer handles RHEL-based systems as before—consider reintroducing the EL‐9 COPR enablement or adjusting the condition to cover RHEL similarly.
- Hardcoding the mesa-vulkan-drivers version (25.0.7-100.fc42) will require manual updates for every bump; consider defining it as a variable or sourcing it from a central config to simplify future maintenance.
- The script invokes `dnf versionlock` but doesn’t install the `dnf-plugins-core` package, which provides the versionlock plugin—ensure that plugin dependency is in place before locking versions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
I've created a new COPR with a patched F42 mesa. I couldn't pick up the Asahi changes because would have meant switching to 25.1.x, and I haven't had the time to test it properly yet. I think it's safer to wait a bit first. We should also see if it'd be possible to get the patch into mainline Fedora. |
|
LGTM |
When building on RHEL-based systems make sure we install the mesa version from the COPR, which enables the virtio driver and has the patches to force alignment to 16K (needed for GPU acceleration on macOS, but harmless to other systems).
We also need to add "--nobest" to "dnf update" to ensure it doesn't get frustrated by being unable to install the mesa package from appstream.
Summary by Sourcery
Update container image build scripts to install and maintain a specific COPR-provided mesa package for reliable GPU acceleration, preserve COPR repos during cleanup, and add the --nobest flag to dnf updates to prevent package resolution issues.
Build: