Feat/bootscript templating #8935
Conversation
- Enable bootscript templating using `envsubst`. - Add generic bootscript. - Fix: `test` does not support `==`, but deceptively returns `true` in case you use it. - De-duplicate kernel commandline console options.
Some background. How to use it.
djurny
requested review from
a team,
EvilOlaf,
TRSx80,
igorpecovnik and
littlecxm
as code owners
WalkthroughA templating system for U-Boot boot scripts is introduced, adding a generic boot script template with comprehensive boot logic (device tree handling, kernel/initrd loading, address calculation), supporting shell functions for rendering and validation, and integrating template rendering into the bootscript deployment pipeline. Changes
Sequence DiagramsequenceDiagram
participant Build as Build System
participant Detect as Bootscript Resolver
participant Render as Template Renderer
participant Validate as Validator
participant Deploy as Deployment
Build->>Detect: locate bootscript_src
Detect->>Detect: check userpatches/<br/>or default path
Detect-->>Render: bootscript_src
alt bootscript_src ends with .template
Render->>Render: export_display_console()
Render->>Render: export_serial_console()
Render->>Render: envsubst template
Render-->>Validate: rendered bootscript
Validate->>Validate: proof_rendered_<br/>bootscript_template()
alt validation passes
Validate-->>Deploy: ✓ bootscript
Deploy->>Deploy: write to destination
else validation fails
Validate-->>Build: ✗ error exit
end
else bootscript_src is static file
Render-->>Deploy: bootscript (copy path)
Deploy->>Deploy: cp to destination
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
11
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
config/bootscripts/boot-generic.cmd.template.md (2)
1-41: Fix markdown formatting issues flagged by linting.Three minor formatting issues need correction:
- Lines 11–13: Three consecutive sentences begin with "Process" — consider rewording one.
- Line 36: Table at line 36–39 should be surrounded by blank lines above and below.
- Lines 44, 84: Fenced code blocks should specify language (e.g., ```bash or ```conf) for syntax highlighting.
These are linting violations and should be fixed for consistency with project standards.
Apply these fixes:
-1. Define U-Boot environment variables. -1. Load a U-Boot environment (`armbianEnv.txt` in our case). -1. Prepare kernel commandline parameters based on loaded environment settings. -1. Process device tree (DT). -1. Process the kernel image. -1. Process the initial ramdisk. -1. Load the initial ramdisk. -1. Boot the kernel with locations of the initial ramdisk and device tree. +1. Define U-Boot environment variables. +1. Load a U-Boot environment (`armbianEnv.txt` in our case). +1. Prepare kernel commandline parameters based on loaded environment settings. +1. Handle device tree (DT) processing. +1. Load the kernel image. +1. Handle the initial ramdisk. +1. Load the initial ramdisk. +1. Boot the kernel with locations of the initial ramdisk and device tree.|Variable|Usage| |-|-| |`BOOTSCRIPT_TEMPLATE__ALIGN_TO`|For the calculation of load addresses by the generic bootscript, addresses need to be aligned for most CPU types. For example, for ARM64 CPUs, the start of the `Image` file to be aligned to a 2MiB (`0x00200000`) boundary. For most architectures, aligning these addresses to a 4KiB (`0x1000`) address boundary is good practice.| + Two exceptions for templating are listed below: + |Variable|Usage|-Example of a board configuration file for the Orange Pi Zero: -``` +Example of a board configuration file for the Orange Pi Zero: +```bash # Allwinner H2+ quad core 256/512MB RAM SoC WiFi SPI ... -``` +```Multiple console devices can be defined by separating them with a `,` (comma). Standard Linux kernel arguments are allowed: -``` +```bash SERIALCON="ttyS0:115200,ttyGS0" -``` +```
1-102: Fix markdown formatting issues flagged by linting tools.Three formatting issues should be addressed:
- Lines 11–13: Three consecutive list items begin with "Process" — consider rewording one to improve readability.
- Line 36: Table should be surrounded by blank lines (add blank line before line 27 and after line 35).
- Lines 44, 84: Fenced code blocks should specify language for syntax highlighting (use ```bash or ```conf).
These are minor linting violations but should be fixed for consistency.
-1. Define U-Boot environment variables. -1. Load a U-Boot environment (`armbianEnv.txt` in our case). -1. Prepare kernel commandline parameters based on loaded environment settings. -1. Process device tree (DT). -1. Process the kernel image. -1. Process the initial ramdisk. -1. Load the initial ramdisk. -1. Boot the kernel with locations of the initial ramdisk and device tree. +1. Define U-Boot environment variables. +1. Load a U-Boot environment (`armbianEnv.txt` in our case). +1. Prepare kernel commandline parameters based on loaded environment settings. +1. Handle device tree (DT) processing. +1. Load the kernel image. +1. Handle the initial ramdisk. +1. Load the initial ramdisk. +1. Boot the kernel with locations of the initial ramdisk and device tree.-|Variable|Usage| + +|Variable|Usage| |-|-| |`BOOTSCRIPT_TEMPLATE__ALIGN_TO`|For the calculation of load addresses by the generic bootscript... + Two exceptions for templating are listed below:-Example of a board configuration file for the Orange Pi Zero: -``` +Example of a board configuration file for the Orange Pi Zero: +```bash # Allwinner H2+ quad core 256/512MB RAM SoC WiFi SPIMultiple console devices can be defined by separating them with a `,` (comma). Standard Linux kernel arguments are allowed: -``` +```bash SERIALCON="ttyS0:115200,ttyGS0"lib/functions/rootfs/distro-agnostic.sh (2)
72-74: proof_rendered_bootscript_template validation is correct.The function uses
! egrep '\$\{?BOOTSCRIPT_TEMPLATE__' "${1:?}"to verify no unrendered BOOTSCRIPT_TEMPLATE__ variables remain. This correctly checks for both$VARIABLEand${VARIABLE}syntax. The:?ensures a file path is provided.One minor note:
egrepis deprecated in favor ofgrep -E; consider modernizing, but this is not a blocker.
72-74: Template validation function correctly detects unrendered variables. Theproof_rendered_bootscript_templatefunction usesegrepto check for both$VARIABLEand${VARIABLE}syntax patterns. Minor nitpick:egrepis deprecated; consider modernizing togrep -Ein future maintenance, but this is not a blocker.lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
309-344: get_bootscript_info correctly populates metadata for template detection and rendering.The function extracts bootscript source and destination (lines 315–316), determines fullpath with user-override logic (lines 324–327), and stores in bootscript_info array. The bootscript_src key (set at line 320) is used by the case statement to detect .template suffix.
One minor observation: The function reads file contents into bootscript_info[bootscript_file_contents] (line 328) but this is never used in the case statement; instead, the raw file is re-read via
cat. This is not an issue but represents a small inefficiency (redundant reads). Not worth changing unless rendering performance becomes a concern.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
-
config/bootscripts/boot-generic.cmd.template(1 hunks) -
config/bootscripts/boot-generic.cmd.template.md(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
🧠 Learnings (24)
📓 Common learnings
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/bootscripts/boot-rk3506.cmd:46-47
Timestamp: 2025-09-12T15:46:16.772Z
Learning: In Armbian boot scripts, uInitrd is always packaged by the build system, so unconditional loading of uInitrd without existence checks is acceptable and won't cause boot failures.
Learnt from: djurny
Repo: armbian/build PR: 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: Grippy98
Repo: armbian/build PR: 8622
File: config/sources/families/k3.conf:66-66
Timestamp: 2025-09-12T19:28:38.491Z
Learning: In the Armbian k3 family build system (config/sources/families/k3.conf), builds do not fail when TIBOOT3_BOOTCONFIG is unset, even though tiboot3.bin is still listed in UBOOT_TARGET_MAP. The gating mechanism in pre_config_uboot_target__build_first_stage function works as intended to conditionally build/copy tiboot3.bin only when TIBOOT3_BOOTCONFIG is defined.
Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Learnt from: djurny
Repo: armbian/build PR: 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
Repo: armbian/build PR: 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.
Learnt from: djurny
Repo: armbian/build PR: 8315
File: config/bootscripts/boot-generic.cmd.template:62-62
Timestamp: 2025-07-13T14:00:39.402Z
Learning: In U-Boot bootscripts, the verbosity check pattern `test "${verbosity}" = "" || itest ${verbosity} -gt 0 && echo "${l_message}"` correctly implements quiet behavior when verbosity is unset or "0", and verbose behavior when verbosity > 0, due to proper operator precedence where || has lower precedence than &&.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.
📚 Learning: 2025-06-22T00:45:26.182Z
Learnt from: djurny
Repo: armbian/build PR: 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.
Applied to files:
lib/functions/host/prepare-host.shlib/functions/rootfs/distro-agnostic.shlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-06-04T23:45:38.860Z
Learnt from: djurny
Repo: armbian/build PR: 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.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.template
📚 Learning: 2025-06-12T21:14:36.024Z
Learnt from: djurny
Repo: armbian/build PR: 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”.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.template
📚 Learning: 2025-09-12T15:46:16.772Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/bootscripts/boot-rk3506.cmd:46-47
Timestamp: 2025-09-12T15:46:16.772Z
Learning: In Armbian boot scripts, uInitrd is always packaged by the build system, so unconditional loading of uInitrd without existence checks is acceptable and won't cause boot failures.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.templatelib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-05-07T20:49:40.969Z
Learnt from: djurny
Repo: armbian/build PR: 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.
Applied to files:
config/bootscripts/boot-generic.cmd.template.md
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-06-12T21:03:39.686Z
Learnt from: djurny
Repo: armbian/build PR: 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.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.templatelib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-09-12T21:44:09.061Z
Learnt from: Grippy98
Repo: armbian/build PR: 8624
File: config/boards/sk-am62p.conf:8-8
Timestamp: 2025-09-12T21:44:09.061Z
Learning: For TI K3 family boards in Armbian, BOOT_FDT_FILE uses .dts extension (not .dtb) as the standard convention. The build system handles this correctly by automatically compiling .dts to .dtb during kernel build and using the BOOT_FDT_FILE value directly in bootloader configurations.
Applied to files:
config/bootscripts/boot-generic.cmd.template.md
📚 Learning: 2025-09-01T06:11:43.476Z
Learnt from: wei633
Repo: armbian/build PR: 8557
File: config/bootscripts/boot-xpressreal-t3.cmd:0-0
Timestamp: 2025-09-01T06:11:43.476Z
Learning: In Armbian boot scripts without initramfs support, UUID= and LABEL= style device paths cannot be used for rootdev as they require initramfs to resolve during boot. Only direct device paths (/dev/mmcblkXpY) or PARTUUID= references (which U-Boot can resolve) will work.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.template
📚 Learning: 2025-09-12T19:28:38.491Z
Learnt from: Grippy98
Repo: armbian/build PR: 8622
File: config/sources/families/k3.conf:66-66
Timestamp: 2025-09-12T19:28:38.491Z
Learning: In the Armbian k3 family build system (config/sources/families/k3.conf), builds do not fail when TIBOOT3_BOOTCONFIG is unset, even though tiboot3.bin is still listed in UBOOT_TARGET_MAP. The gating mechanism in pre_config_uboot_target__build_first_stage function works as intended to conditionally build/copy tiboot3.bin only when TIBOOT3_BOOTCONFIG is defined.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdconfig/bootscripts/boot-generic.cmd.template
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.
Applied to files:
config/bootscripts/boot-generic.cmd.template.mdlib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
Repo: armbian/build PR: 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.
Applied to files:
lib/functions/rootfs/distro-agnostic.shlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-06-22T00:53:10.331Z
Learnt from: djurny
Repo: armbian/build PR: 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.
Applied to files:
lib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 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`.
Applied to files:
lib/functions/rootfs/distro-agnostic.shlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.
Applied to files:
lib/functions/rootfs/distro-agnostic.shlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.
Applied to files:
lib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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.
Applied to files:
lib/functions/rootfs/distro-agnostic.sh
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.
Applied to files:
lib/functions/rootfs/distro-agnostic.shlib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.
Applied to files:
lib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-27T21:50:04.845Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-edge.config:80-82
Timestamp: 2025-09-27T21:50:04.845Z
Learning: In the Armbian build system, kernel configuration files are generated through this automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-13T19:51:14.713Z
Learnt from: pyavitz
Repo: armbian/build PR: 8631
File: config/boards/khadas-vim4.conf:42-42
Timestamp: 2025-09-13T19:51:14.713Z
Learning: In the Armbian build system, run_host_command_logged properly handles shell redirection operators like ">>" when passed as string arguments. This pattern is used consistently throughout the codebase, such as in lib/functions/rootfs/distro-agnostic.sh where it's used to append to armbianEnv.txt files. The function has been tested and confirmed to work correctly with this syntax.
Applied to files:
lib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-13T19:51:14.713Z
Learnt from: pyavitz
Repo: armbian/build PR: 8631
File: config/boards/khadas-vim4.conf:42-42
Timestamp: 2025-09-13T19:51:14.713Z
Learning: In the Armbian build system, run_host_command_logged can properly handle shell redirection operators like >> when passed as arguments, contrary to typical command wrapper behavior. The function has been tested to work correctly with redirection for appending to files like armbianEnv.txt.
Applied to files:
lib/functions/bsp/armbian-bsp-cli-deb.sh
📚 Learning: 2025-09-13T19:51:14.713Z
Learnt from: pyavitz
Repo: armbian/build PR: 8631
File: config/boards/khadas-vim4.conf:42-42
Timestamp: 2025-09-13T19:51:14.713Z
Learning: The run_host_command_logged function in the Armbian build system works by executing all its arguments through `bash -c "$*"`. This means shell redirection operators like ">>" are properly processed by bash, making commands like `run_host_command_logged echo "content" >> file.txt` work correctly. The function does not treat redirection operators as literal arguments.
Applied to files:
lib/functions/bsp/armbian-bsp-cli-deb.sh
🧬 Code graph analysis (3)
lib/functions/host/prepare-host.sh (1)
lib/functions/host/basic-deps.sh (1)
prepare_host_basic(14-49)
lib/functions/rootfs/distro-agnostic.sh (1)
lib/functions/logging/runners.sh (1)
run_host_command_logged(204-206)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
lib/functions/logging/runners.sh (1)
run_host_command_logged(204-206)lib/functions/rootfs/distro-agnostic.sh (2)
render_bootscript_template(56-70)proof_rendered_bootscript_template(72-74)
🪛 LanguageTool
config/bootscripts/boot-generic.cmd.template.md
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...e (DT). 1. Process the kernel image. 1. Process the initial ramdisk. 1. Load the initia...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
config/bootscripts/boot-generic.cmd.template.md
36-36: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
84-84: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (30)
lib/functions/host/prepare-host.sh (2)
190-190: Dependency addition supports template rendering. The addition ofgettextto the host dependencies ensuresenvsubstis available for bootscript template rendering. This aligns with the templating infrastructure introduced in other files.
190-190: ✓ Dependency addition supports template rendering. The addition ofgettextto the host dependencies ensuresenvsubstis available for bootscript template rendering. This aligns with the templating infrastructure introduced in other files.lib/functions/rootfs/distro-agnostic.sh (9)
10-31: Console filtering logic appears sound but warrants clarification.The
bootscript_export_display_consolefunction filters console arguments by checking if they matchtty[AGSU]*. The logic correctly skips ttyA/ttyG/ttyS/ttyU (serial consoles) for display console collection.Line 16 and 23–26 work correctly: display console args are built from DISPLAYCON, then console args from SRC_CMDLINE are filtered to include only non-serial types. Deduplication via
xargs -n1 | sort -u | xargsis idiomatic and correct.Minor: Verify that the
tty[AGSU]*pattern covers all serial console device types expected in this codebase (not a blocker if documented elsewhere).
33-54: Serial console filtering is logically sound.The
bootscript_export_serial_consolefunction mirrorsbootscript_export_display_consolebut filters inversely (lines 42–48): it collects console args from SRC_CMDLINE only if they do match thetty[AGSU]*serial pattern. The logic is correct and symmetric.
56-70: Verify shell variable export syntax in render_bootscript_template.Line 68 exports BOOTSCRIPT_TEMPLATE__ variables using
export $(set | sed -En '/^BOOTSCRIPT_TEMPLATE__/s/=.*$//p'). This pattern is safe from word-splitting (as confirmed by retrieved learning), but the subshell scope (line 56) means these exports are local to the function's subshell.Question: Are the exported variables intended to be visible to the
envsubstcommand on line 69 (which runs in the same subshell)? If so, this is correct. If they need to persist after the function returns, they won't — but the function pipes the result, so this should be fine.Verified against retrieved learning (PR 8315): this pattern has been tested and works correctly.
190-198: Bootscript source path resolution follows best-practice user-override pattern.Lines 190–198 implement a clear priority: prefer user-provided bootscripts (${USERPATCHES_PATH}/bootscripts) over system defaults (${SRC}/config/bootscripts). This matches the documented design and aligns with earlier user-patching conventions in the codebase.
Line 191–192 correctly extract source and destination from BOOTSCRIPT variable (format:
source:destination).
235-248: Bootscript deployment case statement correctly routes templates vs. plain scripts.
- Lines 236–243: Template branch detects
.templatesuffix, renders viacat | render_bootscript_template, writes output, validates withproof_rendered_bootscript_template, and exits on validation failure. Error message is informative.- Lines 244–247: Fallback branch uses
cpfor non-template scripts.Flow is clear and handles both cases properly. The validation error (line 241) provides actionable guidance to inspect the rendered file.
Minor: Consider adding a debug log before validation to aid troubleshooting if proof_rendered_bootscript_template fails.
10-54: Console filtering logic correctly separates display and serial consoles. Bothbootscript_export_display_consoleandbootscript_export_serial_consolefunctions correctly filter console arguments: the former skipstty[AGSU]*devices (serial), the latter includes only those patterns. Deduplication viaxargs -n1 | sort -uis idiomatic and correct. The variables are properly exported for template rendering.
56-70: Bootscript template rendering function is correct. Therender_bootscript_templatefunction properly: (1) captures creation date; (2) exports console variables via the helper functions; (3) constructs SHELL_FORMAT for envsubst; (4) logs available variables for debugging; (5) exports BOOTSCRIPT_TEMPLATE__ vars and invokes envsubst. The subshell scope is appropriate since the function pipes the result to the caller. Matches retrieved learning from PR 8315 confirming this pattern works correctly.
190-198: Bootscript source path resolution implements correct user-override priority. Lines 190–198 implement clear precedence: prefer user patches (${USERPATCHES_PATH}/bootscripts) if present, otherwise fall back to system defaults (${SRC}/config/bootscripts). This pattern aligns with established Armbian conventions. Source/destination extraction (lines 191–192) correctly parses thesource:destinationformat.
235-248: Bootscript deployment case statement correctly routes templates and plain scripts. The conditional on line 236 detects.templatesuffix and branches appropriately: template branch renders viarender_bootscript_template, validates withproof_rendered_bootscript_template, and exits on failure (line 241 provides informative error message). Non-template branch usescpas before. Flow is clear and error handling is appropriate.lib/functions/bsp/armbian-bsp-cli-deb.sh (3)
145-159: BSP package bootscript deployment correctly mirrors distro-agnostic.sh pattern.Lines 145–159 implement the same case-based routing as distro-agnostic.sh:
- Template branch (lines 146–154): renders via
cat | render_bootscript_template, validates, exits on failure.- Plain script branch (lines 155–157): copies via
cp.The logic is consistent and correct. The destination (
${destination}/usr/share/armbian/) aligns with BSP package structure expectations. Error handling (line 152) matches the distro-agnostic.sh approach.Observation: Both distro-agnostic.sh and armbian-bsp-cli-deb.sh now share identical template rendering logic. This duplication is acceptable given the different contexts (image build vs. package compilation), but if rendered bootscripts are later shared, consolidation could reduce maintenance burden.
145-159: BSP package bootscript deployment mirrors distro-agnostic.sh pattern correctly. The case statement (lines 145–159) implements identical template-vs-plain routing as distro-agnostic.sh: template branch renders, validates, and exits on failure; plain branch copies viacp. Destination path (${destination}/usr/share/armbian/) aligns with BSP package structure. Error message (line 152) is descriptive.Design note: Both distro-agnostic.sh and armbian-bsp-cli-deb.sh now duplicate this template rendering logic. While acceptable given different contexts (image build vs. package compilation), future consolidation into a shared helper function could reduce maintenance burden if additional callsites emerge.
309-344: Bootscript metadata extraction correctly supports template detection. Theget_bootscript_infofunction extracts source/destination (lines 315–316), determines fullpath with user-override logic (lines 324–327), and populates thebootscript_infoarray. Thebootscript_srckey (line 320) is correctly used by the case statement to detect.templatesuffix.Minor observation: Line 328 reads file contents into
bootscript_info[bootscript_file_contents]but this cached value is never used; the raw file is re-read in the case statement instead. This represents a small inefficiency (redundant file reads) but is not functionally incorrect. Not worth changing unless rendering performance becomes a concern.config/bootscripts/boot-generic.cmd.template (16)
1-52: Template structure and variable declarations are clear and well-documented.The header comments (lines 1–22) provide good guidance on templating expectations and limitations. Template variables are declared and documented:
- Line 6:
BOOTSCRIPT_TEMPLATE__CREATE_DATE— set byrender_bootscript_templateat render time.- Lines 26–44: Five board-specific variables (
ALIGN_TO,DISPLAY_CONSOLE,SERIAL_CONSOLE,BOARD_FAMILY,BOARD_VENDOR,ROOTFS_TYPE,LOAD_ADDR) — should be defined in board configs or auto-set by rendering helpers.The comment at line 51 clearly marks the end of template variable usage, which aids readability and validation.
Note:
DISPLAY_CONSOLEandSERIAL_CONSOLEare documented as auto-populated by distro-agnostic.sh (lines 38–39 in boot-generic.cmd.template.md), so board configs should leave these empty — this matches the documented example.
54-70: Address alignment function correctly implements off-by-one compensation.Lines 54–61 define
func_align_addr_next. The logic:
- Line 55: Conditionally increments by 1 if
align_overlap_oboe_avoidance="on"(off-by-one compensation).- Lines 56–60: Aligns address upward to
align_toboundary.This matches the retrieved learning (PR 8287): the unconditional "+1" is intentional to prevent DT/kernel overlap due to a known U-Boot address calculation quirk. The implementation is correct as written.
158-232: Device tree file search logic covers fallback cases correctly.Lines 158–232 implement a multi-level fallback search for the DT file:
- Line 200: Try user-specified
fdtdir/fdtfile(from armbianEnv.txt).- Lines 203–206: Try vendor-specific path:
dtb/${vendor}/file.- Lines 208–210: Try base path:
dtb/file.- Lines 213–215: Try original
fdtdir/fdtfileagain.- Lines 218–220: Try preset values (U-Boot defaults).
- Lines 229–232: Exit with error if none found.
This is comprehensive. The nested conditionals are readable, though deeply nested. Line 193 (stripexpr) correctly extracts basename from fully-qualified DT paths. Per the retrieved learning (PR 8272),
fdtfileis mandatory and can be overridden via armbianEnv.txt — the script handles this correctly.
248-304: Overlay processing covers both kernel and user overlays with appropriate error handling.Lines 248–304 process kernel-provided overlays from
overlays=variable (line 249):
- Lines 256–283: Three-level prefix search (overlay_prefix, vendor, vendor-soc) to find each overlay.
- Lines 285–302: Load and apply each overlay; warn on failure but continue.
- Lines 334–379: If any overlay failed, reload original DT without overlays (fallback).
User overlays (lines 306–333) follow a simpler pattern (no prefix search).
The logic is sound. Overlay failures don't halt the boot process (line 291 sets
overlay_error=true), allowing the script to continue with the original DT. This aligns with the documented behavior and is user-friendly (boots even if overlays fail, rather than hard-failing).Minor: Lines 266–269 and 275–278 warn if the found overlay prefix differs from the requested one, which is good debugging feedback.
386-411: Address calculation for kernel load correctly uses fdt totalsize.Lines 386–411 calculate the kernel load address after the DT:
- Lines 388–403: Determine DT total size via
fdt header get(line 389) or calculate if unavailable (lines 397–402).- Lines 405–407: Align the next address after DT and assign to
l_kernel_addr_r.Calculation logic: DT filesize is aligned to 4K boundary (line 399), then fdt_extrasize is added (line 401). This is the standard approach. The
setexprmath correctly implements(filesize / 0x1000 + 1) * 0x1000to align upward.Per retrieved learning (PR 8166), the 4KB (0x1000) boundary is hardcoded for a reason (how
fdt resizeworks), so this should not be made adjustable.
431-476: Kernel image type detection via magic bytes and size extraction are correct.Lines 431–476 detect kernel image type (flat vs. compressed) and calculate ramdisk load address:
- Lines 433–442: Read magic bytes at offset 0x38–0x3a to detect ARMd signature (flat Image format).
- Lines 445–471: If flat, extract image_size from header (line 448) and text_offset (line 455); if compressed, use filesize.
- Lines 460–464: Align ramdisk address after kernel, accounting for text_offset for flat images.
This logic is sound and matches U-Boot kernel image header specifications. The alignment after kernel + text_offset (line 464) correctly accounts for relocation by booti_setup().
Per retrieved learning (PR 8287),
${cpu}is always pre-set by U-Boot, so line 165's check is reasonable (though the learning suggests guards are unnecessary, this is defensive).
498-524: Kernel boot logic correctly branches on image type.Lines 498–524 prepare for KASLR (line 499) and boot:
- Lines 514–523: Branch on kernel image type (flat → booti, compressed → bootz, else → bootz default).
- Lines 527–534: Error restoration and critical error message if boot fails.
The boot command order (kernel, ramdisk, DT) is correct for both booti and bootz.
One observation: Line 499 attempts
kaslrseedbut doesn't validate success (line 500 always succeeds). Per U-Boot docs, kaslrseed fails gracefully if the hardware doesn't support it, so the current approach (log and continue) is appropriate.
25-49: Template variable assignments use consistent setenv syntax.Lines 25–44 assign BOOTSCRIPT_TEMPLATE__ values to environment variables:
- Line 26:
setenv align_to "${BOOTSCRIPT_TEMPLATE__ALIGN_TO}"- Line 27–28: Console args
- Line 33: Board family
- Line 43: Rootfs type
- Line 44: Vendor
- Line 48: Load address
All are correctly wrapped in
${...}for envsubst substitution. Missing values will be replaced with empty strings (envsubst's default behavior), which could cause issues if not validated byproof_rendered_bootscript_template. The rendering proof function should catch any omitted variables — this is correct.Potential concern: If any BOOTSCRIPT_TEMPLATE__ var is missing at board config time, the template will render with empty values (e.g.,
setenv align_to ""), andproof_rendered_bootscript_templatewill fail due to unresolved${BOOTSCRIPT_TEMPLATE__*}syntax remaining. This error-checking is correct.
1-52: Template structure and variable declarations are clear and well-organized. Header comments (lines 1–22) provide good guidance on templating expectations. Template variables are declared consistently with setenv:ALIGN_TO,DISPLAY_CONSOLE,SERIAL_CONSOLE,BOARD_FAMILY,BOARD_VENDOR,ROOTFS_TYPE,LOAD_ADDR. Line 51 clearly marks the end of template variable usage, aiding readability and validation. Note:DISPLAY_CONSOLEandSERIAL_CONSOLEare auto-populated by distro-agnostic.sh rendering helpers, so board configs should leave these empty — aligns with documented example at lines 68–69 of the markdown.
54-61: Address alignment function correctly implements intentional off-by-one compensation. Lines 54–61 definefunc_align_addr_next, which: (1) conditionally increments address by 1 ifalign_overlap_oboe_avoidance="on"(line 55); (2) aligns upward toalign_toboundary (lines 56–60). Per retrieved learning (PR 8287), the "+1" increment is intentional to prevent DT/kernel overlap due to a known U-Boot address calculation quirk. Implementation is correct as written and should not be "optimized away."
158-232: Device tree file search implements comprehensive fallback logic. Lines 158–232 search for DT files across five fallback paths: (1) user-specified dir/file (from armbianEnv.txt); (2) vendor-specific path; (3) base dtb path; (4) original dir/file; (5) U-Boot preset values. Exits with critical error if none found. The nested conditionals are readable. Line 193 correctly extracts basename usingsetexpr. Per retrieved learning (PR 8272),fdtfileis mandatory and can be overridden via armbianEnv.txt — the script handles this correctly.
248-304: Overlay processing provides comprehensive error handling and fallback. Lines 248–304 process kernel overlays: three-level prefix search (overlay_prefix, vendor, vendor-soc) finds each overlay, loads and applies with warnings on failure. User overlays (lines 306–333) follow a simpler pattern. If any overlay fails (line 291 setsoverlay_error=true), the script reloads the original DT without overlays (lines 334–379), allowing boot to proceed. This is sound and user-friendly (boots with original DT rather than hard-failing). Warnings for mismatched overlay prefixes (lines 266–269, 275–278) provide good debugging feedback.
386-411: Kernel load address calculation correctly follows DT. Lines 386–411 calculate kernel load address after DT: determine DT total size viafdt header get(line 389) or calculate if unavailable (lines 397–402), then align the next address. Calculation logic correctly implements(filesize / 0x1000 + 1) * 0x1000to align upward to 4K boundary. Per retrieved learning (PR 8166), the 4KB boundary is hardcoded for a reason (howfdt resizeworks internally) and should remain fixed.
431-476: Kernel image type detection via magic bytes is correct. Lines 431–476 detect image type by reading magic bytes at offset 0x38–0x3a (ARMd signature indicates flat Image), then extract image_size (flat) or use filesize (compressed). Ramdisk address is aligned after kernel, accounting for text_offset (line 464) to handle booti_setup() relocation. Logic matches U-Boot kernel image header specifications and is sound.
498-524: Kernel boot flow correctly branches on image type with proper fallback. Lines 498–524 attempt KASLR setup (line 499, non-fatal on failure per U-Boot design), then branch on image type: flat → booti, compressed → bootz, else → bootz default. Boot command argument order (kernel, ramdisk, DT) is correct. Error restoration (lines 527–534) restores preset values if boot fails. Error message provides guidance to troubleshoot. Flow is sound.
25-49: Template variable assignments use consistent envsubst-compatible syntax. All BOOTSCRIPT_TEMPLATE__ assignments are correctly wrapped in${...}(lines 26–28, 33, 43–44, 48). If any variable is missing at render time, envsubst will leave the${VARIABLE}unexpanded, andproof_rendered_bootscript_templatewill catch this and fail the build with an informative error. This validation chain is correct and ensures no partially-rendered bootscripts reach the boot process.
leggewie
left a comment
leggewie
left a comment
There was a problem hiding this comment.
great stuff, thank you for your work and for including documentation so that people can actually make use of it
|
✅ This PR has been reviewed and approved — all set for merge! |
igorpecovnik
left a comment
igorpecovnik
left a comment
There was a problem hiding this comment.
Quite a voyage getting here! I recommend moving the documentation into the main docs. Let’s go ahead and merge it.
3 participants
Due to my inability to use git properly, this is a cleaned up PR based on the same but squashed commits from #8315.
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:
Summary by CodeRabbit
New Features
Documentation
Chores