TI: Add TI's custom debian repository and install a few packages by-default #8305
Conversation
|
""" WalkthroughThis change updates the SK-AM62B board configuration by adding the 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/small
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
config/boards/sk-am62b.conf (1)
18-18: Add a short comment explaining the newCC33XX_SUPPORTswitchFuture maintainers unfamiliar with TI’s Wi-Fi stack won’t know what turning this flag on/off implies for the image size or driver set. A one-liner right above the variable would help.
extensions/ti-debpkgs.sh (2)
17-18: Potential sed delimiter collisionIf
${RELEASE}ever contains a “/” (unlikely but possible, e.g.testing/rolling), the substitution breaks. Considersed -i "s|bookworm|${RELEASE}|g" …to switch to a safer delimiter.
25-26: Copy runs outside chroot – ownership & uid may differ
cp … $SDCARD/etc/apt/preferences.d/happens on the host, so the file ends up owned by the invoking user (often root:root). If the rootfs later enforces different ownership (e.g._apt), move the copy insidechroot_sdcardto inherit correct permissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/boards/sk-am62b.conf(1 hunks)config/sources/families/k3.conf(2 hunks)extensions/ti-debpkgs.sh(1 hunks)packages/bsp/ti/ti-debpkgs/ti-debpkgs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
extensions/ti-debpkgs.sh (1)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)
🪛 Shellcheck (0.10.0)
extensions/ti-debpkgs.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 9-9: Prefer mapfile or read -a to split command output (or quote to avoid splitting).
(SC2207)
[error] 10-10: Argument mixes string and array. Use * or separate argument.
(SC2145)
[error] 12-12: Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
(SC2199)
[warning] 12-12: Remove quotes from right-hand side of =~ to match as a regex rather than literally.
(SC2076)
[error] 31-31: Argument mixes string and array. Use * or separate argument.
(SC2145)
🔇 Additional comments (2)
packages/bsp/ti/ti-debpkgs/ti-debpkgs (1)
1-3: Pin-priority1001may unintentionally override security updatesA priority above 1000 forces TI packages to win every dependency race – even over Debian security/point-release updates from official mirrors. Confirm this is the desired policy for all packages, or consider lowering to
990and use per-package pins where absolutely required.config/sources/families/k3.conf (1)
22-23: Unconditionalenable_extension "ti-debpkgs"can break non-TI derivativesIf a downstream board reuses this family file but does not host the TI repo key,
apt updatewill warn/fail on the missing source. Consider gating the call behind[[ "${TI_PACKAGES[*]}" ]]or another feature flag.
jsuhaas22
force-pushed
the
add-ti-debpkgs
branch
from
59b66e4 to
7fafbda
Compare
github-actions
bot
added
size/medium
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
extensions/ti-debpkgs.sh (1)
17-18: Quote the output path inwget
You correctly added the temp-dir creation but still need to quote the target file to handle paths with spaces (SC2086):- run_host_command_logged "wget -qO $SDCARD/tmp/ti-debpkgs.sources https://…" + run_host_command_logged "wget -qO \"$SDCARD/tmp/ti-debpkgs.sources\" https://…"
🧹 Nitpick comments (3)
extensions/ti-debpkgs.sh (3)
1-1: Add a shebang or shell directive
The script has no interpreter declaration (SC2148). Include a shebang and/or a ShellCheck directive to specify Bash explicitly.+#!/usr/bin/env bash +# shellcheck shell=bash
13-13: Avoid mixing array expansion inside quoted strings
ShellCheck SC2145 warns about mixing strings and arrays. Consider using${valid_suites[*]}or separate arguments:- display_alert "TI Repo has the following valid suites - ${valid_suites[@]}..." + display_alert "TI Repo has the following valid suites - ${valid_suites[*]}..."- display_alert "Valid Options Would Have Been: ${valid_suites[@]}" + display_alert "Valid Options Would Have Been: ${valid_suites[*]}"Also applies to: 35-35
32-36: Error-path alert formatting
Same array-expansion mix ondisplay_alertas above—use${valid_suites[*]}for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/boards/sk-am62b.conf(1 hunks)config/sources/families/k3.conf(2 hunks)extensions/ti-debpkgs.sh(1 hunks)packages/bsp/ti/ti-debpkgs/ti-debpkgs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bsp/ti/ti-debpkgs/ti-debpkgs
- config/boards/sk-am62b.conf
- config/sources/families/k3.conf
🧰 Additional context used
🧬 Code Graph Analysis (1)
extensions/ti-debpkgs.sh (1)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)
🪛 Shellcheck (0.10.0)
extensions/ti-debpkgs.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[error] 13-13: Argument mixes string and array. Use * or separate argument.
(SC2145)
[error] 35-35: Argument mixes string and array. Use * or separate argument.
(SC2145)
🔇 Additional comments (4)
extensions/ti-debpkgs.sh (4)
1-5: Theextension_prepare_config__add_packagesfunction correctly checks and expandsTI_PACKAGESintoadd_packages_to_rootfs. No changes needed here.
9-12: Good use ofmapfilefor JSON parsing
Usingmapfile -tto populatevalid_suitesavoids word-splitting issues and addresses earlier ShellCheck concerns.
15-16: Suite membership check is robust
Usingprintf … | grep -qxsafely tests for exact matches.
21-27: Sed replacement and chroot copy are sound
The in-placesedsubstitution for${RELEASE}and subsequent chroot operations look correct.
Texas Instruments maintains a custom apt repository [0] that contains:
* tools like k3conf, which run on K3 devices
* TI's versions of upstream packages (such as mesa)
* out-of-tree drivers and firmware for graphics, wifi etc
Therefore, add TI's custom repository as the highest priority repository
in the filesystem. Doing this ensures that if apt finds a version of a
package that exists in both upstream Debian and the TI repository, it
picks the latter.
Additionally, introduce K3_PACKAGES variable to store a list of packages
that should be installed by-default in a K3 image. Initialize it to hold
TI's CC33xx packages.
Also set EXTRAWIFI to "no" in `current` image.
[0] https://github.com/TexasInstruments/ti-debpkgs
Co-authored-by: Suhaas Joshi <s-joshi@ti.com>
Signed-off-by: Suhaas Joshi <s-joshi@ti.com>
Change U-Boot repository link from git.ti.com to Github. In the past, there have been CI issues caused due to git.ti.com failing. Signed-off-by: Suhaas Joshi <s-joshi@ti.com>
jsuhaas22
force-pushed
the
add-ti-debpkgs
branch
from
7fafbda to
43fb4a8
Compare
github-actions
bot
removed
the
Ready to merge
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
extensions/ti-debpkgs.sh (1)
17-17: Ensure temporary directory creationThe addition of
mkdir -p "$SDCARD/tmp"addresses the missing-directory issue raised previously.
🧹 Nitpick comments (4)
extensions/ti-debpkgs.sh (4)
1-1: Specify shell interpreter & enable strict modeAdd a shebang (
#!/usr/bin/env bash) and considerset -euo pipefailat the top to ensure the script runs under Bash and fails fast on errors.
13-13: Quote array expansion indisplay_alert
display_alert "TI Repo has the following valid suites - ${valid_suites[@]}..."mixes string and array. Consider using${valid_suites[*]}or wrapping the expansion in quotes to avoid unintended word-splitting.
18-18: Quote wget output path
wget -qO $SDCARD/tmp/ti-debpkgs.sources …is unquoted; wrap the path in quotes to handle potential spaces in$SDCARD/tmp.-run_host_command_logged "wget -qO $SDCARD/tmp/ti-debpkgs.sources https://raw.githubusercontent.com/TexasInstruments/ti-debpkgs/main/ti-debpkgs.sources" +run_host_command_logged "wget -qO \"$SDCARD/tmp/ti-debpkgs.sources\" https://raw.githubusercontent.com/TexasInstruments/ti-debpkgs/main/ti-debpkgs.sources"
35-35: Quote array expansion in error messageSame as above in the failure path: use
${valid_suites[*]}or quote the expansion to prevent word-splitting.-display_alert "Valid Options Would Have Been: ${valid_suites[@]}" +display_alert "Valid Options Would Have Been: ${valid_suites[*]}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/boards/sk-am62b.conf(1 hunks)config/sources/families/k3.conf(2 hunks)extensions/ti-debpkgs.sh(1 hunks)packages/bsp/ti/ti-debpkgs/ti-debpkgs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- config/boards/sk-am62b.conf
- packages/bsp/ti/ti-debpkgs/ti-debpkgs
- config/sources/families/k3.conf
🧰 Additional context used
🪛 Shellcheck (0.10.0)
extensions/ti-debpkgs.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[error] 13-13: Argument mixes string and array. Use * or separate argument.
(SC2145)
[error] 35-35: Argument mixes string and array. Use * or separate argument.
(SC2145)
🔇 Additional comments (3)
extensions/ti-debpkgs.sh (3)
1-5: ValidateTI_PACKAGEShandling
extension_prepare_config__add_packagescorrectly checks for populatedTI_PACKAGESand invokesadd_packages_to_rootfs. Implementation looks solid.
8-12: Use ofmapfileis correctThe JSON-to-array conversion via
mapfile -t valid_suitesavoids word-splitting and is a robust replacement for the previous$(...)approach.
21-21: Fixsedquoting so${RELEASE}is expandedUsing single quotes around the substitution prevents
${RELEASE}from being interpolated. Switch to a quoting style that allows host-level expansion:
[sraise_critical_issue]-chroot_sdcard "sed -i 's/bookworm/${RELEASE}/g' /tmp/ti-debpkgs.sources" +chroot_sdcard "sed -i \"s/bookworm/${RELEASE}/g\" /tmp/ti-debpkgs.sources"
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 maintains its own repository of debian packages here.
This PR adds this repository in TI images by-default through a new extension. So when the user performs
apt update,apt installetc in Armbian images for TI boards, even TI's packages can be installed. Further, the PR also installs a few of TI's packages by-default in its images.Additionally: it also changes TI's U-boot link from git.ti.com to Github.
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.
This has been tested on SK-AM62B trixie image:
apt updateandapt installtestsChecklist:
Please delete options that are not relevant.