ti: Add more packages and make weston the OOB #8375
Conversation
Default and officially supported TI images will be the console image with weston installed and running on top of it. Therefore add weston in the list of packages to install. Also add a systemd service file to launch weston by-default on boot. Further, add an explicit step to bind dkms packages to kernel. Currently the dkms package gets installed before the kernel, therefore if this step is excluded, the binding would occur on first boot, which would cause the first boot time to get longer. Therefore add the "dkms autoinstall" step. Additionally, add firmware, gstreamer, chromium etc packages. Note: Weston does not launch on first boot, since the booting process kills it. It does launch on second boot onwards. Currently, full-fledged DEs make the armbian-firstlogin script launch them after the script finishes setup. But there is no logic yet to start other systemd services such as weston. Such logic should be added later. Signed-off-by: Suhaas Joshi <s-joshi@ti.com>
WalkthroughThis change updates board configuration files for SK-AM62B and SK-AM64B by introducing new configuration variables for GPU, display, Chromium support, SoC identification, and additional firmware packages. The global TI package list logic is expanded to conditionally include GPU, display, and Chromium-related packages, as well as board-specific firmware. Two new shell functions are added to handle enabling Weston services and activating DKMS modules during image customization. Environment variables for Weston are set, and new systemd unit files for Weston (service and socket) are introduced to manage the Wayland compositor on supported boards. Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
size/medium
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/bsp/ti/weston/weston.socket (1)
8-9: Consider uncommenting user/group settings for security.Running Weston as root (implied by the commented user/group settings) may not be ideal from a security perspective. Consider uncommenting and using dedicated users if they exist on the system.
extensions/ti-debpkgs.sh (1)
45-45: Consider improving error handling for service enablement.The current error handling only logs an alert if
systemctl enablefails. Consider whether this should be a fatal error or if the alert provides sufficient information for debugging.packages/bsp/ti/weston/weston.service (1)
40-40: Consider making Weston backend configurableThe service hard-codes the DRM backend with specific options. Consider making the backend and options configurable through the environment file for better flexibility across different hardware configurations.
-ExecStart=/usr/bin/weston --backend=drm-backend.so --continue-without-input --idle-time=0 +ExecStart=/usr/bin/weston $WESTON_OPTIONSThen define
WESTON_OPTIONSin/etc/default/weston.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
config/boards/sk-am62b.conf(1 hunks)config/boards/sk-am64b.conf(1 hunks)config/sources/families/k3.conf(1 hunks)extensions/ti-debpkgs.sh(1 hunks)packages/bsp/ti/weston/weston(1 hunks)packages/bsp/ti/weston/weston.service(1 hunks)packages/bsp/ti/weston/weston.socket(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: leggewie
PR: armbian/build#8357
File: config/boards/qcom-robotics-rb5.conf:88-88
Timestamp: 2025-07-04T18:33:19.873Z
Learning: In the Armbian build system, package transitions in Debian/Ubuntu don't happen uniformly across all releases. For example, the transition from plasma-workspace-wayland to plasma-workspace occurred in newer releases like trixie (Debian testing) around July 2024, but older releases like jammy (Ubuntu 22.04) still maintain the original plasma-workspace-wayland package. Release-specific package availability must be considered when updating package references.
Learnt from: libiunc
PR: armbian/build#8033
File: config/kernel/linux-starfive2-vendor.config:43-43
Timestamp: 2025-03-31T12:57:13.880Z
Learning: For StarFive2 platform kernel configurations, maintain alignment with vendor-provided configurations rather than modifying security settings like SECCOMP. This ensures hardware compatibility as intended by the manufacturer.
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Learnt from: pyavitz
PR: armbian/build#8361
File: config/kernel/linux-sunxi64-edge.config:594-597
Timestamp: 2025-07-06T16:27:45.349Z
Learning: In Armbian sunxi64-edge kernel configurations, PCI/PCIe support may be enabled for future-proofing purposes to support upcoming Allwinner SoCs like the T536 that include PCIe Gen2 interfaces, even if current primary targets like H616 lack PCIe controllers. This forward-looking approach prepares the configuration for next-generation hardware in the same SoC family.
Learnt from: Grippy98
PR: armbian/build#8152
File: lib/functions/configuration/interactive.sh:209-266
Timestamp: 2025-05-05T12:35:07.143Z
Learning: For the interactive kernel selection in Armbian, KERNEL_MAJOR_MINOR and KERNEL_DESCRIPTION are parsed from family.conf but deliberately not set as environment variables to avoid potential interference with other parts of the build system.
Learnt from: djurny
PR: armbian/build#8287
File: config/bootscripts/boot-sunxi.cmd:148-161
Timestamp: 2025-06-12T21:03:39.686Z
Learning: In Armbian U-Boot bootscripts, the `${cpu}` environment variable is always pre-set by U-Boot (or defaults to a valid value), so additional guards against it being unset are unnecessary.
packages/bsp/ti/weston/weston (1)
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
config/boards/sk-am62b.conf (2)
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
Learnt from: pyavitz
PR: armbian/build#8361
File: config/kernel/linux-sunxi64-edge.config:594-597
Timestamp: 2025-07-06T16:27:45.349Z
Learning: In Armbian sunxi64-edge kernel configurations, PCI/PCIe support may be enabled for future-proofing purposes to support upcoming Allwinner SoCs like the T536 that include PCIe Gen2 interfaces, even if current primary targets like H616 lack PCIe controllers. This forward-looking approach prepares the configuration for next-generation hardware in the same SoC family.
config/boards/sk-am64b.conf (8)
Learnt from: EvilOlaf
PR: armbian/build#0
File: :0-0
Timestamp: 2025-06-16T03:24:00.458Z
Learning: In Armbian board configuration files (.csc), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features.
Learnt from: libiunc
PR: armbian/build#8033
File: config/kernel/linux-starfive2-vendor.config:43-43
Timestamp: 2025-03-31T12:57:13.880Z
Learning: For StarFive2 platform kernel configurations, maintain alignment with vendor-provided configurations rather than modifying security settings like SECCOMP. This ensures hardware compatibility as intended by the manufacturer.
Learnt from: EvilOlaf
PR: armbian/build#8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.
Learnt from: pyavitz
PR: armbian/build#8361
File: config/kernel/linux-sunxi64-edge.config:594-597
Timestamp: 2025-07-06T16:27:45.349Z
Learning: In Armbian sunxi64-edge kernel configurations, PCI/PCIe support may be enabled for future-proofing purposes to support upcoming Allwinner SoCs like the T536 that include PCIe Gen2 interfaces, even if current primary targets like H616 lack PCIe controllers. This forward-looking approach prepares the configuration for next-generation hardware in the same SoC family.
Learnt from: EvilOlaf
PR: armbian/build#8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.
Learnt from: djurny
PR: armbian/build#8272
File: config/bootscripts/boot-mvebu.cmd:182-186
Timestamp: 2025-06-04T23:45:38.860Z
Learning: In config/bootscripts/boot-mvebu.cmd, the `fdtfile` variable is mandatory for booting and is pre-set by U-Boot, but can be overridden via armbianEnv.txt. If `fdtfile` is empty, the subsequent device tree file search logic will eventually fail and trigger the critical error "Cannot find DT!" with proper error handling.
Learnt from: djurny
PR: armbian/build#8287
File: config/bootscripts/boot-sunxi.cmd:148-161
Timestamp: 2025-06-12T21:03:39.686Z
Learning: In Armbian U-Boot bootscripts, the `${cpu}` environment variable is always pre-set by U-Boot (or defaults to a valid value), so additional guards against it being unset are unnecessary.
Learnt from: djurny
PR: armbian/build#8287
File: config/bootscripts/boot-sunxi.cmd:13-23
Timestamp: 2025-06-12T21:08:11.318Z
Learning: In config/bootscripts/boot-sunxi.cmd the variable name `align_overlap_oboe_avoidance` intentionally uses the acronym “OBOE” (Off-By-One Error); it is not a typo and should not be renamed.
extensions/ti-debpkgs.sh (5)
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Learnt from: djurny
PR: armbian/build#8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
config/sources/families/k3.conf (6)
Learnt from: EvilOlaf
PR: armbian/build#8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.
Learnt from: EvilOlaf
PR: armbian/build#8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.
Learnt from: Grippy98
PR: armbian/build#8152
File: lib/functions/configuration/interactive.sh:209-266
Timestamp: 2025-05-05T12:35:07.143Z
Learning: For the interactive kernel selection in Armbian, KERNEL_MAJOR_MINOR and KERNEL_DESCRIPTION are parsed from family.conf but deliberately not set as environment variables to avoid potential interference with other parts of the build system.
Learnt from: djurny
PR: armbian/build#8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Learnt from: leggewie
PR: armbian/build#8357
File: config/boards/qcom-robotics-rb5.conf:88-88
Timestamp: 2025-07-04T18:33:19.873Z
Learning: In the Armbian build system, package transitions in Debian/Ubuntu don't happen uniformly across all releases. For example, the transition from plasma-workspace-wayland to plasma-workspace occurred in newer releases like trixie (Debian testing) around July 2024, but older releases like jammy (Ubuntu 22.04) still maintain the original plasma-workspace-wayland package. Release-specific package availability must be considered when updating package references.
🧬 Code Graph Analysis (1)
extensions/ti-debpkgs.sh (3)
lib/functions/logging/runners.sh (2)
run_host_command_logged(204-206)chroot_sdcard(133-136).github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/compilation/utils-compilation.sh (1)
grab_version(23-32)
🔇 Additional comments (10)
packages/bsp/ti/weston/weston (1)
1-2: Verify socket path consistency with weston.socket.The environment variables look correct for a system-level Weston service. However, there's a potential mismatch:
WAYLAND_DISPLAY=wayland-1suggests the socket should be at/run/wayland-1, but the correspondingweston.socketfile creates a socket at/run/wayland-0.Please verify that the socket path and display name are consistent between this file and
weston.socket.config/boards/sk-am62b.conf (1)
19-22: LGTM! Feature flags are well-structured.The addition of GPU, display, and Chromium support flags follows Armbian conventions and integrates well with the broader TI package management changes in the k3 family configuration.
config/boards/sk-am64b.conf (1)
19-20: LGTM! Board-specific configuration is appropriate.The SOC_ID and firmware package additions are consistent with the AM62B board configuration approach and properly specify AM64-specific PRU firmware packages.
extensions/ti-debpkgs.sh (1)
48-54: LGTM! DKMS activation logic is sound.The function correctly:
- Checks the GPU_SUPPORT flag
- Uses the standard
grab_versionfunction- Constructs the kernel version string following Armbian conventions
- Runs DKMS autoinstall with appropriate verbosity
packages/bsp/ti/weston/weston.service (1)
34-34:.not_logged_in_yetLifecycle Is Fully Handled
- Creation
- lib/functions/rootfs/distro-agnostic.sh:
touch "${SDCARD}"/root/.not_logged_in_yet- extensions/preset-firstrun.sh: writes presets into
${SDCARD}/root/.not_logged_in_yetfor automated first-run configurations- Consumption
- etc/profile.d/armbian-check-first-login.sh and armbian-firstlogin: detect
/root/.not_logged_in_yetto drive the first-login flow- Removal
- armbian-firstlogin (multiple places):
rm -f /root/.not_logged_in_yetonce setup completes- config/templates/customize-image.sh.template & extensions/cloud-init/cloud-init.sh: clean up leftover
.not_logged_in_yetNo further changes required.
config/sources/families/k3.conf (5)
23-23: LGTM: Clean package array initializationThe TI_PACKAGES array is properly initialized with base packages. This provides a solid foundation for conditional package additions.
31-31: SOC_ID definitions verifiedSOC_ID is defined in all relevant board configuration files, ensuring
${SOC_ID}interpolates correctly ink3.conf.• config/boards/sk-am64b.conf: SOC_ID="am64"
• config/boards/sk-am62b.conf: SOC_ID="am62"No further action required.
30-34: Ensure GPU support packages exist on each target distributionThe TI GPU packages (
ti-img-rogue-driver-${SOC_ID}-dkms,ti-img-rogue-umlibs-${SOC_ID},ti-img-rogue-tools-${SOC_ID},ti-img-rogue-firmware-${SOC_ID}, plusmesa-vulkan-drivers,libgl1-mesa-dri,gstreamer1.0-*) may be named differently or missing in some distros. Add distribution-specific checks before appending them toTI_PACKAGES. For example:• Detect the distro via
/etc/os-releaseand dispatch accordingly
• Debian/Ubuntu:apt-cache policy "$pkg" &>/dev/null || missing+=("$pkg")• RHEL/CentOS/Fedora:
dnf repoquery --quiet --exactmatch "$pkg" || missing+=("$pkg")• Report any missing packages and skip or substitute as needed
Example snippet:
. /etc/os-release if [[ "${GPU_SUPPORT}" == "yes" ]] ; then + missing=() + add_pkg(){ + local pkg=$1 + case "$ID" in + debian|ubuntu) apt-cache policy "$pkg" &>/dev/null ;; + centos|rhel|fedora) dnf repoquery --quiet --exactmatch "$pkg" ;; + *) return 1 ;; + esac || missing+=("$pkg") + TI_PACKAGES+=("$pkg") + } + # TI image drivers - TI_PACKAGES+=("ti-img-rogue-driver-${SOC_ID}-dkms" "ti-img-rogue-umlibs-${SOC_ID}" "ti-img-rogue-tools-${SOC_ID}" "ti-img-rogue-firmware-${SOC_ID}") + for pkg in \ + "ti-img-rogue-driver-${SOC_ID}-dkms" \ + "ti-img-rogue-umlibs-${SOC_ID}" \ + "ti-img-rogue-tools-${SOC_ID}" \ + "ti-img-rogue-firmware-${SOC_ID}"; do + add_pkg "$pkg" + done + # Mesa / Vulkan - TI_PACKAGES+=("mesa-vulkan-drivers" "libgl1-mesa-dri") + for pkg in "mesa-vulkan-drivers" "libgl1-mesa-dri"; do + add_pkg "$pkg" + done + # GStreamer - TI_PACKAGES+=("gstreamer1.0-tools" "gstreamer1.0-plugins-good" "gstreamer1.0-plugins-bad" "gstreamer1.0-plugins-base") + for pkg in \ + "gstreamer1.0-tools" \ + "gstreamer1.0-plugins-good" \ + "gstreamer1.0-plugins-bad" \ + "gstreamer1.0-plugins-base"; do + add_pkg "$pkg" + done + + if [[ ${#missing[@]} -gt 0 ]]; then + echo "Warning: the following GPU packages are unavailable on $ID: ${missing[*]}" + fi fiPlease verify these package names against your TI repositories on each supported distribution.
46-48: Improve array handling for EXTRA_BOARD_PACKAGESThe current syntax
"${EXTRA_BOARD_PACKAGES[@]}"within the TI_PACKAGES+=() assignment may not work correctly if EXTRA_BOARD_PACKAGES is not an array. Consider using a more robust approach.-if [[ ! -z ${EXTRA_BOARD_PACKAGES} ]] ; then - TI_PACKAGES+=("${EXTRA_BOARD_PACKAGES[@]}") -fi +if [[ -n "${EXTRA_BOARD_PACKAGES}" ]] ; then + if [[ "$(declare -p EXTRA_BOARD_PACKAGES 2>/dev/null)" =~ "declare -a" ]] ; then + TI_PACKAGES+=("${EXTRA_BOARD_PACKAGES[@]}") + else + TI_PACKAGES+=("${EXTRA_BOARD_PACKAGES}") + fi +fiThis handles both string and array cases properly.
⛔ Skipped due to learnings
Learnt from: djurny PR: armbian/build#8166 File: config/bootscripts/boot-mvebu.cmd:38-50 Timestamp: 2025-05-07T20:49:40.969Z Learning: In U-Boot scripts, `itest.s` should be used for explicit string comparisons rather than plain `itest` with string operators like `==`.Learnt from: djurny PR: armbian/build#8315 File: lib/functions/rootfs/distro-agnostic.sh:54-68 Timestamp: 2025-06-22T00:45:26.182Z Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
44-44: Action Required: Verify k3 family inclusion and firmware package namesEnsure
config/sources/families/k3.confis actually applied to each k3-based board and that the SOC_ID interpolation produces valid firmware package names. Based on the current board configs:
- config/boards/sk-am64b.conf → SOC_ID="am64" → would generate
firmware-ti-ipc-am64- config/boards/sk-am62b.conf → SOC_ID="am62" → would generate
firmware-ti-ipc-am62Please confirm:
- That
k3.confis sourced (directly or via common include) in each of these board files- The interpolated packages (
firmware-ti-ipc-am64,firmware-ti-ipc-am62) actually exist and match your firmware repository naming
| RequiresMountsFor=/run | ||
|
|
||
| [Socket] | ||
| ListenStream=/run/wayland-0 |
There was a problem hiding this comment.
Fix socket path mismatch with environment configuration.
The socket path /run/wayland-0 doesn't match the WAYLAND_DISPLAY=wayland-1 setting in the weston environment file. This will cause connection failures.
Choose one of these solutions:
- Change the socket path to match the display name:
-ListenStream=/run/wayland-0
+ListenStream=/run/wayland-1- Or change the environment variable to match the socket:
-WAYLAND_DISPLAY=wayland-1
+WAYLAND_DISPLAY=wayland-0Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/bsp/ti/weston/weston.socket at line 6, the socket path
ListenStream=/run/wayland-0 does not match the WAYLAND_DISPLAY=wayland-1
environment setting, causing connection failures. Fix this by either updating
the socket path to /run/wayland-1 to match the environment variable or change
the WAYLAND_DISPLAY environment variable to wayland-0 to match the socket path,
ensuring both are consistent.
| @@ -35,3 +35,20 @@ function custom_apt_repo__install_ti_packages() { | |||
| display_alert "Valid Options Would Have Been: ${valid_suites[@]}" | |||
| fi | |||
| } | |||
|
|
|||
| function pre_customize_image__enable_services() { | |||
| run_host_command_logged "mkdir -p $DEST/lib/systemd/system/" | |||
There was a problem hiding this comment.
Fix incorrect path variable.
The mkdir command uses $DEST but should use $SDCARD to be consistent with the rest of the function and the Armbian build system conventions.
- run_host_command_logged "mkdir -p $DEST/lib/systemd/system/"
+ run_host_command_logged "mkdir -p $SDCARD/lib/systemd/system/"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run_host_command_logged "mkdir -p $DEST/lib/systemd/system/" | |
| - run_host_command_logged "mkdir -p $DEST/lib/systemd/system/" | |
| + run_host_command_logged "mkdir -p $SDCARD/lib/systemd/system/" |
🤖 Prompt for AI Agents
In extensions/ti-debpkgs.sh at line 40, the mkdir command incorrectly uses the
$DEST variable for the path, but it should use $SDCARD to align with the rest of
the function and Armbian build system conventions. Replace $DEST with $SDCARD in
the mkdir command to fix the path variable usage.
| User=root | ||
| Group=root |
There was a problem hiding this comment.
Security concern: Running Weston as root user
Running the Weston compositor as root presents significant security risks. Display servers typically run with lower privileges for security isolation. Consider creating a dedicated user account for Weston or using a login manager that can handle privilege separation.
Consider this approach instead:
-User=root
-Group=root
+User=weston
+Group=westonAnd ensure the weston user is created with appropriate permissions for accessing graphics hardware.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| User=root | |
| Group=root | |
| User=weston | |
| Group=weston |
🤖 Prompt for AI Agents
In packages/bsp/ti/weston/weston.service at lines 47 to 48, the service is
configured to run Weston as the root user, which poses security risks. Modify
the User and Group directives to run Weston under a dedicated, non-root user
account (e.g., 'weston'). Ensure this user is created beforehand with the
necessary permissions to access graphics hardware, and update the service file
accordingly to enhance security by limiting privileges.
| WorkingDirectory=/root/ | ||
|
|
||
| # Set up a full user session for the user, required by Weston. | ||
| PAMName=weston-autologin |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify PAM configuration for weston-autologin
The service references a PAM configuration named "weston-autologin" but this PAM configuration file is not provided in the PR. Without proper PAM configuration, the service may fail to start.
Please ensure the corresponding PAM configuration file exists:
🏁 Script executed:
#!/bin/bash
# Description: Check if PAM configuration for weston-autologin exists
# Expected: PAM config file should exist and be properly configured
fd -t f "weston-autologin" /etc/pam.d/Length of output: 137
🏁 Script executed:
#!/bin/bash
set -e
echo "Searching for pam.d directories in the repository:"
fd -t d pam.d .
echo
echo "Searching for files named 'weston-autologin' in the repository:"
fd -t f weston-autologin .
echo
echo "Searching for occurrences of 'weston-autologin' in file contents:"
rg -n "weston-autologin"Length of output: 563
Add missing PAM configuration for “weston-autologin”
The service unit at packages/bsp/ti/weston/weston.service (line 54) specifies
PAMName=weston-autologinbut no corresponding PAM config file (pam.d/weston-autologin) is present in this PR. Without it, the service will fail to start.
• Please add the PAM configuration file under your package’s pam.d/ directory (e.g. files/etc/pam.d/weston-autologin) with the required rules.
• Verify that it’s installed to /etc/pam.d/weston-autologin on target systems.
🤖 Prompt for AI Agents
In packages/bsp/ti/weston/weston.service at line 54, the PAMName is set to
"weston-autologin" but the corresponding PAM configuration file
pam.d/weston-autologin is missing. To fix this, create the PAM config file with
the necessary authentication rules under your package's pam.d/ directory (e.g.,
files/etc/pam.d/weston-autologin) and ensure it is installed to
/etc/pam.d/weston-autologin on the target system.
| [Service] | ||
| # Requires systemd-notify.so Weston plugin. | ||
| Type=simple | ||
| EnvironmentFile=/etc/default/weston |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Verify weston environment file
The service references /etc/default/weston but this environment file is not provided in the PR. Missing environment configuration could cause the service to fail or behave unexpectedly.
Please ensure the weston environment file exists:
🏁 Script executed:
#!/bin/bash
# Description: Check for weston environment file
# Expected: Environment file should exist with proper variables
fd -t f "weston" /etc/default/Length of output: 131
🏁 Script executed:
#!/bin/bash
# Search for the weston environment file within the repository
find . -type f | grep '/etc/default/weston$'Length of output: 46
Provide Weston environment file or remove unused reference
The weston.service unit at packages/bsp/ti/weston/weston.service still references an external file:
39 │ EnvironmentFile=/etc/default/weston
However, no /etc/default/weston is present in the repo or packaging. This can cause the service to fail or skip needed settings.
Please address one of the following:
-
Include the default environment file
Add a file (e.g.packages/bsp/ti/weston/weston.env) and install it to/etc/default/westonwith the necessary variables for Weston. -
Remove or adjust the
EnvironmentFile=directive
If Weston doesn’t require custom environment variables, drop or comment out the line to avoid loading a non-existent file.
🤖 Prompt for AI Agents
In packages/bsp/ti/weston/weston.service at line 39, the EnvironmentFile
directive points to /etc/default/weston, which does not exist in the repo or
packaging. To fix this, either create a default environment file with necessary
Weston variables (e.g., packages/bsp/ti/weston/weston.env) and ensure it is
installed to /etc/default/weston, or remove/comment out the EnvironmentFile line
if no custom environment variables are needed to prevent service failure.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
TI Debian images support different features, such as IPC, Weston (as OOB screen), GPU acceleration, camera etc. So to have these working by-default in our images, this PR adds certain packages such as:
Additionally, there are 2 more things done here:
/lib/systemd/system/. This is to start weston on boot by-default. Note that weston doesn't launch on first boot when the armbian-firstlogin script runs, but starts launching from subsequent boots.dkms autoinstallbuilds and installs the ti-img-rogue--dkms package for the current kernel. If this isn't done here, it will done during first boot, which would make the first boot very slow.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist:
Please delete options that are not relevant.