Enable bootscript templating #8315
Conversation
|
""" WalkthroughA new U-Boot boot script template, Possibly related PRs
Suggested labels
✨ 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/functions/rootfs/distro-agnostic.sh (1)
54-68: Template rendering function: consider shellcheck annotations.
render_bootscript_templatecorrectly exportsBOOTSCRIPT_TEMPLATE__CREATE_DATEand console vars, then invokesenvsubst. To silence SC2034 (unused var) and SC2046 (word splitting onexport $(…)), add above the function:# shellcheck disable=SC2034,SC2046
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/bootscripts/boot-generic.cmd.template(1 hunks)lib/functions/bsp/armbian-bsp-cli-deb.sh(1 hunks)lib/functions/host/prepare-host.sh(1 hunks)lib/functions/rootfs/distro-agnostic.sh(3 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
lib/functions/rootfs/distro-agnostic.sh
[warning] 58-58: BOOTSCRIPT_TEMPLATE__CREATE_DATE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 66-66: Quote this to prevent word splitting.
(SC2046)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Shell script analysis
- GitHub Check: Check kernel security options
🔇 Additional comments (11)
lib/functions/host/prepare-host.sh (1)
189-189: Add gettext to host dependencies: Looks correct.
Includinggettextensures thatenvsubstis available for template rendering. Verify that the package name matches the target distributions (e.g., Debian’sgettext-basevs.gettext).lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
145-153: Implement template rendering for.templatebootscripts.
Good use ofrender_bootscript_templateand validation viaproof_rendered_bootscript_templateto fail fast on unrendered placeholders.
155-159: Retain direct copy path for non-template bootscripts.
The fallback tocp -pvpreserves existing behavior for plain bootscripts.lib/functions/rootfs/distro-agnostic.sh (4)
10-30: New console export helpers look solid.
bootscript_export_display_consoleandbootscript_export_serial_consolecorrectly assemble console arguments and export them underBOOTSCRIPT_TEMPLATE__DISPLAY_CONSOLEandBOOTSCRIPT_TEMPLATE__SERIAL_CONSOLE.
70-72: Proof-rendering verification is concise and accurate.
The grep against${BOOTSCRIPT_TEMPLATE__ensures no placeholders remain.
182-191: Resolve bootscript source path with patch override support.
Thebootscript_src_pathselection betweenUSERPATCHES_PATHand defaultSRC/config/bootscriptsis correct.
226-235: Enable templating in rootfs installation.
Differentiates.templatesuffix, renders viarender_bootscript_template, and enforces completeness withproof_rendered_bootscript_template.config/bootscripts/boot-generic.cmd.template (4)
1-16: Template header and documentation are clear.
The introductory comments outline templating expectations and references to implementing scripts.
24-29: Default environment variable placeholders.
ALIGN_TO,DISPLAY_CONSOLE, andSERIAL_CONSOLEplaceholders are correctly positioned for substitution.
33-46: Board, rootfs, load address, and vendor placeholders.
Use of${BOOTSCRIPT_TEMPLATE__BOARD_FAMILY},${BOOTSCRIPT_TEMPLATE__ROOTFS_TYPE},${BOOTSCRIPT_TEMPLATE__LOAD_ADDR}, and${BOOTSCRIPT_TEMPLATE__BOARD_VENDOR}is appropriate and aligns with export functions.
49-52: Templating termination marker.
The# NOTEblock correctly signals that no furtherBOOTSCRIPT_TEMPLATE__variables should appear beyond this point.
|
Hi @The-going, |
5f93a68 to
7d42abc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/functions/rootfs/distro-agnostic.sh (1)
32-52: Duplicate logic: see previous commentThe
bootscript_export_serial_consolefunction mirrors the display-console one. Apply the same refactoring and quoting recommendations above to consolidate and secure the code.
🧹 Nitpick comments (4)
lib/functions/rootfs/distro-agnostic.sh (4)
10-30: Refactor duplicated console-export logic and harden variable handlingThe two functions for display- and serial-console exports share nearly identical loops and use unquoted expansions (
${DISPLAYCON//,/ },$SRC_CMDLINE) that can lead to word‐splitting or globbing. Consider extracting a generic helper (e.g.bootscript_export_console VAR_NAME INPUT_LIST FILTER_REGEX) to DRY up both functions. Also quote parameter expansions to prevent unintended splitting.
70-72: Suppress grep output and use exit codes
proof_rendered_bootscript_templateshould usegrep -qto avoid printing matches and make its intent clearer. For example:-function proof_rendered_bootscript_template() { - ! egrep '\$\{?BOOTSCRIPT_TEMPLATE__' "${1:?}" -} +function proof_rendered_bootscript_template() { + if egrep -q '\$\{?BOOTSCRIPT_TEMPLATE__' "${1:?}"; then + return 1 + fi + return 0 +}
182-190: Combine local declarations for clarityGroup related locals into a single declaration to improve readability and reduce boilerplate. Example:
- local bootscript_src_path - local bootscript_src=${BOOTSCRIPT%%:*} - local bootscript_dst=${BOOTSCRIPT##*:} + local bootscript_src_path bootscript_src bootscript_dst + bootscript_src=${BOOTSCRIPT%%:*} + bootscript_dst=${BOOTSCRIPT##*:}
227-240: Avoid useless use ofcatYou can pipe the template directly into
render_bootscript_templatewithoutcat:- run_host_command_logged cat "${bootscript_src_path}/${bootscript_src}" \ - | render_bootscript_template > "${SDCARD}/boot/${bootscript_dst}" + run_host_command_logged render_bootscript_template \ + < "${bootscript_src_path}/${bootscript_src}" \ + > "${SDCARD}/boot/${bootscript_dst}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/bootscripts/boot-generic.cmd.template(1 hunks)lib/functions/bsp/armbian-bsp-cli-deb.sh(1 hunks)lib/functions/host/prepare-host.sh(1 hunks)lib/functions/rootfs/distro-agnostic.sh(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/functions/host/prepare-host.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/functions/bsp/armbian-bsp-cli-deb.sh
- config/bootscripts/boot-generic.cmd.template
🧰 Additional context used
🪛 Shellcheck (0.10.0)
lib/functions/rootfs/distro-agnostic.sh
[warning] 58-58: BOOTSCRIPT_TEMPLATE__CREATE_DATE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 66-66: Quote this to prevent word splitting.
(SC2046)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Shell script analysis
Enable bootscript templating using `envsubst`. Add generic bootscript.
7d42abc to
4b06fe4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
config/bootscripts/boot-generic.cmd.template(1 hunks)lib/functions/bsp/armbian-bsp-cli-deb.sh(1 hunks)lib/functions/host/prepare-host.sh(1 hunks)lib/functions/rootfs/distro-agnostic.sh(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/functions/host/prepare-host.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/functions/bsp/armbian-bsp-cli-deb.sh
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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: 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: EvilOlaf
PR: armbian/build#8328
File: lib/functions/compilation/patch/drivers_network.sh:542-545
Timestamp: 2025-06-24T10:08:40.313Z
Learning: In the Armbian build system, when a PR removes build support for a specific kernel version, version check issues for that removed version become practically irrelevant even if they appear incorrect in isolation. Context about which kernel versions are being deprecated/removed is important for understanding the impact of version-related code changes.
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
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#8203
File: config/bootscripts/boot-sunxi.cmd:111-114
Timestamp: 2025-05-17T23:08:31.746Z
Learning: In U-Boot scripts, the `setexpr sub` command is valid for string operations when CONFIG_REGEX=y is enabled. This allows for pattern substitution in strings, as documented in https://docs.u-boot.org/en/latest/usage/cmd/setexpr.html. The syntax is `setexpr <variable> sub <pattern> <replacement> <string>`.
Learnt from: djurny
PR: armbian/build#8272
File: config/bootscripts/boot-mvebu.cmd:182-186
Timestamp: 2025-06-04T23:45:38.860Z
Learning: In U-Boot bootscripts, the `setexpr` command will not fail if a variable is empty - it simply won't perform any substitution operations and leaves the target variable unchanged.
Learnt from: djurny
PR: armbian/build#8287
File: config/bootscripts/boot-sunxi.cmd:38-44
Timestamp: 2025-06-12T21:14:36.024Z
Learning: In config/bootscripts/boot-sunxi.cmd the unconditional "+1" increment in func_align_addr_next (when align_overlap_oboe_avoidance="on") is intentional. It compensates for a known off-by-one error in U-Boot where the end address is calculated as start+size instead of start+size-1, so the extra page prevents overlap. This behavior should not be “optimized away”.
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
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#8272
File: config/bootscripts/boot-mvebu.cmd:61-63
Timestamp: 2025-06-04T23:52:39.493Z
Learning: In U-Boot shell scripting, the `else ;` construct followed by commands on subsequent lines is valid syntax, as demonstrated by empirical testing. Both `else command` and `else ; command` forms are acceptable in U-Boot's shell implementation.
config/bootscripts/boot-generic.cmd.template (13)
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:38-44
Timestamp: 2025-06-12T21:14:36.024Z
Learning: In config/bootscripts/boot-sunxi.cmd the unconditional "+1" increment in func_align_addr_next (when align_overlap_oboe_avoidance="on") is intentional. It compensates for a known off-by-one error in U-Boot where the end address is calculated as start+size instead of start+size-1, so the extra page prevents overlap. This behavior should not be “optimized away”.
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
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.
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: djurny
PR: armbian/build#8166
File: config/bootscripts/boot-mvebu.cmd:38-50
Timestamp: 2025-05-07T20:49:40.969Z
Learning: The 4KB (0x1000) boundary used for device tree size calculations in U-Boot scripts is specific to how the `fdt resize` command works internally and should remain hardcoded rather than being tied to adjustable alignment variables.
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: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
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: djurny
PR: armbian/build#8272
File: config/bootscripts/boot-mvebu.cmd:182-186
Timestamp: 2025-06-04T23:45:38.860Z
Learning: In U-Boot bootscripts, the `setexpr` command will not fail if a variable is empty - it simply won't perform any substitution operations and leaves the target variable unchanged.
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.
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.
lib/functions/rootfs/distro-agnostic.sh (16)
Learnt from: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
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: djurny
PR: armbian/build#8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:53:10.331Z
Learning: The bash construct `export $(set | sed -En '/^BOOTSCRIPT_TEMPLATE__/s/=.*$//p')` is safe from word splitting because it extracts only variable names (by removing everything after `=`), and bash variable names cannot contain whitespace or special characters.
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: rpardini
PR: armbian/build#8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
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: 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#8272
File: config/bootscripts/boot-mvebu.cmd:182-186
Timestamp: 2025-06-04T23:45:38.860Z
Learning: In U-Boot bootscripts, the `setexpr` command will not fail if a variable is empty - it simply won't perform any substitution operations and leaves the target variable unchanged.
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.
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.
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: 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#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: 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: 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:61-63
Timestamp: 2025-06-04T23:52:39.493Z
Learning: In U-Boot shell scripting, the `else ;` construct followed by commands on subsequent lines is valid syntax, as demonstrated by empirical testing. Both `else command` and `else ; command` forms are acceptable in U-Boot's shell implementation.
🧬 Code Graph Analysis (1)
lib/functions/rootfs/distro-agnostic.sh (2)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/logging/runners.sh (1)
run_host_command_logged(204-206)
🪛 Shellcheck (0.10.0)
lib/functions/rootfs/distro-agnostic.sh
[warning] 58-58: BOOTSCRIPT_TEMPLATE__CREATE_DATE appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 66-66: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (1)
lib/functions/rootfs/distro-agnostic.sh (1)
54-68: Template rendering pipeline looks solidThe new
render_bootscript_template/proof_rendered_bootscript_templatehelpers integrate cleanly, re-using existing logging and subshell isolation. Dependency handling forenvsubstis already covered upstream, and the variable-export construct is confirmed safe (per earlier discussion).
No further action needed.
| if test "${vendor}" == "allwinner" ; then | ||
| if test "${disp_mem_reserves}" = "off" ; then | ||
| setenv bootargs "${bootargs} sunxi_ve_mem_reserve=0 sunxi_g2d_mem_reserve=0 sunxi_fb_mem_reserve=16" | ||
| fi | ||
| fi | ||
| if test "${vendor}" == "marvell" ; then | ||
| # | ||
| fi | ||
| if test "${vendor}" == "rockchip" ; then | ||
| # | ||
| fi |
There was a problem hiding this comment.
test uses non-portable == operator – switch to = to avoid false negatives on some U-Boot builds
U-Boot’s built-in test command only guarantees support for the single-equals string operator (=).
Some versions do accept ==, but others return a non-zero status, breaking the vendor-specific option blocks below.
-if test "${vendor}" == "allwinner" ; then
+if test "${vendor}" = "allwinner" ; then
…
-fi
-if test "${vendor}" == "marvell" ; then
+fi
+if test "${vendor}" = "marvell" ; then
…
-fi
-if test "${vendor}" == "rockchip" ; then
+fi
+if test "${vendor}" = "rockchip" ; then
…
-fi
+fiUsing = keeps behaviour identical on Bash while restoring portability to every U-Boot shell I’ve encountered.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In config/bootscripts/boot-generic.cmd.template around lines 127 to 137, replace
all instances of the non-portable string comparison operator '==' in the test
commands with the single '=' operator to ensure compatibility with all U-Boot
shell versions while maintaining the same behavior.
envsubstfor bootscripts with the filename extension.template.Description
In order to prepare for a generic bootscript, introduce simple templating for bootscripts using
envsubst. The templating is needed as some things in the generic bootscript do require customization, where templating offers easy customization using already-existing shell variables. For example,load_addr, serial consoles, memory alignment settings andfamily/vendornames to enable proper name resolving for DT and DT overlay files.Documentation summary for feature / change
If bootscript name ends with the
.templateextension (could not find a more suitable extension),envsubstwill be invoked to render the bootscript [template]. Rendering will consist of replacing any occurrance of a variable named$BOOTSCRIPT_TEMPLATE__xxxwith the value in the environment of the build system at the time of rendering, in the bootscript template.Some things to take into consideration,
BOOTSCRIPTin theboards/config/xxxfiles can be overridden bysources/families/xxxorsources/families/includes/xxx. For example, forboards/config/orangepizero.csc, the setting forBOOTSCRIPTwill be overridden bysources/families/includes/sunxi-common.inc. This can be solved with the following construct insources/families/includes/sunxi-common.inc:For every board, this has to be inspected to make sure the bootscript selection is done as expected.
How Has This Been Tested?
BOOTSCRIPTdeclaration insources/families/includes/sunxi-common.inc.boot/boot.cmd:Checklist: