Refactor mesa-vpu extension: add architecture-specific browser selection#9347
Merged
igorpecovnik merged 3 commits intomainfrom Feb 3, 2026
Merged
Refactor mesa-vpu extension: add architecture-specific browser selection#9347igorpecovnik merged 3 commits intomainfrom
igorpecovnik merged 3 commits intomainfrom
Conversation
github-actions
bot
added
the
size/medium
Contributor
📝 WalkthroughWalkthroughAdds riscv64 to GNOME architectures, refactors mesa-vpu multimedia and package-install flows (including a new architecture-aware browser installation function), and consolidates apt pinning for Firefox/Thunderbird to Armbian origin. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
02
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@extensions/mesa-vpu.sh`:
- Around line 108-130: The function post_armbian_repo_customize_image__browser
can call do_with_retries chroot_sdcard_apt_get_install with an uninitialized
pkgs array when BUILD_DESKTOP != "yes"; add an early return when BUILD_DESKTOP
is not "yes" (or alternatively move the do_with_retries call inside the
BUILD_DESKTOP == "yes" branch) so that pkgs is only referenced after being set;
update the logic around BUILD_DESKTOP, pkgs, and the do_with_retries
chroot_sdcard_apt_get_install invocation accordingly.
🧹 Nitpick comments (1)
extensions/mesa-vpu.sh (1)
54-57: Potential redundant chromium installation.Chromium is added to the package list here (line 56), and also installed separately in the new
post_armbian_repo_customize_image__browserfunction (line 119) for arm64/armhf architectures. This results in chromium being installed twice for those architectures.Consider whether the browser-related logic in lines 54-57 should be removed now that browser selection is handled by the dedicated
post_armbian_repo_customize_image__browserfunction.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@extensions/mesa-vpu.sh`:
- Around line 108-130: In post_armbian_repo_customize_image__browser(), do not
call do_with_retries chroot_sdcard_apt_get_install when BUILD_DESKTOP is not
"yes" because pkgs is only set inside that branch; either move the display_alert
and do_with_retries (chroot_sdcard_apt_get_install "${pkgs[@]}") lines inside
the if [[ "${BUILD_DESKTOP}" == "yes" ]] block after pkgs is populated, or add a
guard (e.g., check BUILD_DESKTOP or that pkgs is non-empty) before invoking
do_with_retries; reference symbols: post_armbian_repo_customize_image__browser,
BUILD_DESKTOP, pkgs, display_alert, do_with_retries,
chroot_sdcard_apt_get_install.
🧹 Nitpick comments (1)
extensions/mesa-vpu.sh (1)
54-57: Consider removing duplicate chromium installation.Chromium is added to
pkgshere for all desktop builds, but the newpost_armbian_repo_customize_image__browserfunction also installs chromium specifically for arm64/armhf. While apt handles duplicate requests gracefully, this creates redundant work.Consider either:
- Remove this block entirely (let the new function handle browser selection), or
- Exclude arm64/armhf from this block if chromium should remain in the base packages for some reason.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@extensions/mesa-vpu.sh`:
- Around line 108-128: post_install_kernel_debs__3d currently unconditionally
adds "chromium", conflicting with the arch-specific browser choice in
post_armbian_repo_customize_image__browser; update post_install_kernel_debs__3d
to stop unconditionally installing chromium and instead only add chromium when
BUILD_DESKTOP=="yes" and ARCH matches the arm64|armhf pattern (or remove the
chromium addition entirely), so post_armbian_repo_customize_image__browser can
control browser selection based on ARCH and avoid duplicate/conflicting
installs.
- Around line 108-128: In post_armbian_repo_customize_image__browser(): pkgs can
be unset when BUILD_DESKTOP != "yes", causing chroot_sdcard_apt_get_install to
run with no args and abort under set -e; make pkgs a local variable (e.g., local
pkgs) and add an early return when BUILD_DESKTOP is not "yes" so the function
exits before calling do_with_retries/chroot_sdcard_apt_get_install; keep the
existing ARCH-based assignments to pkgs inside the BUILD_DESKTOP branch and only
call display_alert/do_with_retries when pkgs is populated.
- Line 34: The release deny list in extension_prepare_config__3d does not match
post_install_kernel_debs__3d, which allows the config hook to run on releases
(bookworm, jammy) where kernel installation is skipped; update the deny list
used in extension_prepare_config__3d to include bookworm and jammy so it matches
the pattern used in post_install_kernel_debs__3d (i.e., deny
bookworm|bullseye|buster|focal|jammy) ensuring both hooks consistently skip the
same releases.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@extensions/mesa-vpu.sh`:
- Around line 54-57: The code currently unconditionally appends "chromium" to
the pkgs array when BUILD_DESKTOP=="yes" (the pkgs+=("chromium") block), which
conflicts with the later architecture-specific browser selection; remove that
unconditional addition so the desktop build does not force chromium and let the
subsequent browser-selection logic (the arch-specific branches that add
google-chrome-stable, chromium, or firefox) manage which browser package is
appended to pkgs.
- Around line 108-127: The function post_armbian_repo_customize_image__browser
should return early when BUILD_DESKTOP is not "yes" and should declare the
package array with local scope to avoid clobbering a global pkgs; update
post_armbian_repo_customize_image__browser to check BUILD_DESKTOP at the top and
return immediately if not "yes", and change the pkgs assignment lines to use a
local pkgs array (local pkgs) before setting values for the
amd64/arm64/armhf/other branches, then proceed with display_alert and
do_with_retries using the local pkgs.
In `@packages/bsp/common/etc/apt/preferences.d/armbian`:
- Around line 1-4: The apt preferences file currently pins "Package: firefox
thunderbird" to apt.armbian.com; add "firefox-esr" to that Package line so
Bookworm/Trixie desktop installs of firefox-esr are also pinned to
apt.armbian.com (update the "Package:" entry in the apt preferences block to
include firefox-esr).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@extensions/mesa-vpu.sh`:
- Around line 108-127: In post_armbian_repo_customize_image__browser, guard
non-desktop builds with an early return: after the RELEASE check, return
immediately if BUILD_DESKTOP is not "yes" so pkgs is never left uninitialized
and do_with_retries chroot_sdcard_apt_get_install "${pkgs[@]}" is not invoked
with an empty array; update the function to explicitly check BUILD_DESKTOP and
return early (refer to the BUILD_DESKTOP variable and the
chroot_sdcard_apt_get_install call) to match the pattern used by
post_install_kernel_debs__3d.
igorpecovnik
force-pushed
the
mesafixes
branch
3 times, most recently
from
ae9c5b3 to
6b1ede7
Compare
…election - Remove legacy panfork/kisak PPAs and image suffix logic - Restrict Rockchip multimedia to noble+vendor only - Simplify chromium installation (remove distribution checks) - Add new post_armbian_repo_customize_image__browser() function for architecture-dependent browser selection: - amd64: google-chrome-stable - arm64/armhf: chromium - other: firefox Signed-off-by: Igor Pecovnik <igor@armbian.com>
igorpecovnik
force-pushed
the
mesafixes
branch
from
6b1ede7 to
9e1d310
Compare
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
igorpecovnik
added
05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Also includes commits from branch:
Test plan
Summary by CodeRabbit
New Features
Improvements