mvebu64: re-revitalize espressobin - can build (under noble) again
#9182
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- try to squash all shellcheck warnings - drop toolchain stuff we don't have anymore - then it becomes obvious there was a spurious `/` in `ATF1` - requires & works with `compile_atf()`'s LDFLAGS handling in armbian#9162 - build-tested on noble (default Docker image) - does NOT build on trixie+: - and apparently never will: https://lists.trustedfirmware.org/archives/list/[email protected]/thread/A2NLSTWUNTSH3WAGOW4LGATKYYCTZXXZ/ - the whole thing has been dropped from TF-A CI, so it's effectively junk now - also fixes `macchiatobin-doubleshot`, but that is .eos anyway - that builds on noble & trixie - Fixes: 81ad50f
Contributor
📝 WalkthroughWalkthroughThis pull request refactors the mvebu64 family configuration file to use explicit bash scoping declarations. Global variables are converted from simple assignments to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
02
This was referenced Jan 7, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
config/sources/families/mvebu64.conf (1)
110-114: Consider declaring local variables before first use.Line 110 uses
$ubootdirbefore it's declared as local on line 113. While this works (line 110 accesses the global variable set byatf_custom_postprocess, then line 113 shadows it with a local), it creates an implicit dependency on function call ordering that's not obvious from the code.♻️ Suggested refactor
Move the local variable declarations (lines 112-114) to immediately after the opening of the function, before line 110's usage:
function uboot_custom_postprocess() { + declare atfdir="$SRC/cache/sources/$ATFDIR/${ATFBRANCH##*:}" + declare ubootdir="$SRC/cache/sources/u-boot-worktree/$BOOTDIR/${BOOTBRANCH##*:}" + declare moxbootdir="$SRC/cache/sources/mox-boot" + # clean previous run_host_command_logged rm -f "$ubootdir"/flash-image* - - declare atfdir="$SRC/cache/sources/$ATFDIR/${ATFBRANCH##*:}" - declare ubootdir="$SRC/cache/sources/u-boot-worktree/$BOOTDIR/${BOOTBRANCH##*:}" - declare moxbootdir="$SRC/cache/sources/mox-boot" - cd "${atfdir}" || exit_with_error "Cannot enter ATF directory ${atfdir}"This makes it clear that these are function-local variables and removes the dependency on the global
ubootdirfromatf_custom_postprocess.
📜 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 (1)
config/sources/families/mvebu64.conf
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Learnt from: rpardini
Repo: armbian/build PR: 9159
File: patch/u-boot/u-boot-genio/0026-dts-configs-add-Grinn-GenioSBC-510.patch:161-161
Timestamp: 2026-01-03T20:46:29.189Z
Learning: For the Armbian genio family (config/sources/families/genio.conf and patch/u-boot/u-boot-genio/), when reviewing PRs that include vendor U-Boot patches from Collabora, avoid flagging potential issues in board configurations that are out of scope for the PR's primary focus (e.g., don't flag Genio 510/700 board issues when the PR is focused on radxa-nio-12l/Genio 1200). The maintainer prioritizes keeping vendor patches close to upstream for easier re-copying and maintenance, even if secondary board configs have potential mismatches.
<!-- </add_learning>
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: rpardini
Repo: armbian/build PR: 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: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.
Learnt from: tabrisnet
Repo: armbian/build PR: 9136
File: lib/functions/compilation/armbian-kernel.sh:529-537
Timestamp: 2025-12-28T01:49:34.661Z
Learning: In lib/functions/compilation/armbian-kernel.sh's opts_m handling, checking .config (kernel defconfig) is problematic because savedefconfig elides options matching defconfig, making it impossible to distinguish overlay silence from defconfig agreement. The correct approach is to check only kernel_config_source_filename (the overlay) for explicit =y, otherwise default to =m, letting savedefconfig handle deduplication with defconfig automatically.
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Learnt from: rpardini
Repo: armbian/build PR: 9159
File: config/sources/families/genio.conf:201-201
Timestamp: 2026-01-03T20:44:12.559Z
Learning: In the Armbian build system, when calling run_host_command_logged with arrays containing arguments that have shell metacharacters (e.g., semicolons, pipes), use "${array[*]Q}" instead of "${array[@]}". This is because run_host_command_logged executes via `bash -c "$*"`, and the Q operator shell-quotes each element to prevent bash from interpreting special characters as command separators or other shell syntax.
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-11-11T20:56:20.303Z
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
config/sources/families/mvebu64.conf
📚 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:
config/sources/families/mvebu64.conf
📚 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/sources/families/mvebu64.conf
📚 Learning: 2025-08-30T06:48:09.091Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-30T06:48:09.091Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the user prefers flexible grep patterns over anchored ones for BTRFS configuration checks, but agrees to use quiet grep (-q) to avoid polluting build logs.
Applied to files:
config/sources/families/mvebu64.conf
📚 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:
config/sources/families/mvebu64.conf
📚 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/sources/families/mvebu64.conf
📚 Learning: 2025-07-23T10:01:41.310Z
Learnt from: amazingfate
Repo: armbian/build PR: 8419
File: lib/functions/host/prepare-host.sh:272-275
Timestamp: 2025-07-23T10:01:41.310Z
Learning: In the Armbian build system, the design philosophy is to fail fast when host dependencies are not met rather than gracefully skipping unsupported architectures. This ensures build environment consistency and prevents silent failures. Host dependency checks should be explicit and non-negotiable.
Applied to files:
config/sources/families/mvebu64.conf
📚 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/sources/families/mvebu64.conf
📚 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:
config/sources/families/mvebu64.conf
📚 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:
config/sources/families/mvebu64.conf
📚 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:
config/sources/families/mvebu64.conf
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-11-10T22:05:40.490Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-11-13T14:50:44.662Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8925
File: lib/functions/host/docker.sh:141-141
Timestamp: 2025-11-13T14:50:44.662Z
Learning: In the Armbian build system, certain boards have U-Boot that doesn't build correctly on ubuntu:noble but works on ubuntu:jammy. For these boards, DOCKER_ARMBIAN_BASE_IMAGE can be set to "ubuntu:jammy" via an extension file loaded before docker_cli_prepare() runs, allowing legacy toolchains to work while the community works on proper fixes with limited resources.
Applied to files:
config/sources/families/mvebu64.conf
📚 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/sources/families/mvebu64.conf
📚 Learning: 2025-12-13T11:39:08.046Z
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:28-67
Timestamp: 2025-12-13T11:39:08.046Z
Learning: In the Armbian build system for SpacemiT U-Boot patches (patch/u-boot/legacy/u-boot-spacemit-k1/), alignment with mainline U-Boot behavior is prioritized. For example, in boot mode handling, leaving devnum unchanged in the default case (when devtype is cleared) follows mainline conventions rather than explicitly clearing it to handle edge cases.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 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.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-03-31T22:20:48.475Z
Learnt from: rpardini
Repo: armbian/build PR: 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.
Applied to files:
config/sources/families/mvebu64.conf
📚 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/sources/families/mvebu64.conf
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
Repo: armbian/build PR: 8455
File: config/sources/families/sun50iw1.conf:19-24
Timestamp: 2025-08-03T15:21:20.148Z
Learning: In the Armbian build system, when copying firmware files during family_tweaks_s(), use /lib/firmware/updates/ instead of /lib/firmware/ to avoid conflicts with the Armbian firmware package. The /lib/firmware/updates directory takes precedence in Linux firmware loading hierarchy and is the proper location for user-installed firmware files.
Applied to files:
config/sources/families/mvebu64.conf
📚 Learning: 2025-12-12T23:09:56.813Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9058
File: config/sources/families/spacemit.conf:39-45
Timestamp: 2025-12-12T23:09:56.813Z
Learning: In Armbian build configs for vendor kernel sources, prefer the following branch naming conventions: use 'vendor' or 'vendor-rt' for stable vendor releases, and 'vendor-edge' for bleeding-edge/pre-release vendor versions. The 'edge' naming without the 'vendor-' prefix is reserved for mainline kernel branches. Apply this pattern to family config files under config/sources/families (e.g., spacemit.conf) to ensure consistent vendor kernel sourcing naming across the repository.
Applied to files:
config/sources/families/mvebu64.conf
🔇 Additional comments (8)
config/sources/families/mvebu64.conf (8)
23-32: LGTM: Explicit global declarations improve clarity.Converting to
declare -gmakes the global scope explicit and addresses shellcheck warnings. This is a good practice for framework configuration variables.
43-54: LGTM: Board-specific global declarations are appropriate.The use of
declare -gfor board-specific configuration variables in conditional blocks is consistent with the global declaration pattern and appropriate for these runtime-selected configurations.
73-75: LGTM: CPU configuration declarations follow the established pattern.
77-84: LGTM: Explicit function keyword improves consistency.Adding the
functionkeyword makes the function declaration explicit and improves code consistency.
86-91: LGTM: Quoting improvements and function keyword addition.The improved quoting around
${SDCARD}is good defensive coding, and the explicitfunctionkeyword maintains consistency.
93-98: LGTM: Guard check improves robustness.The guard check on line 94 ensures
destinationis set before use, which is good defensive programming for BSP packaging functions.
100-106: LGTM: Variable handling improvements address PR objectives.The changes successfully address the PR objectives:
- Line 103:
ATF1="${ATF_COMPILER}"fixes the spurious/mentioned in the PR description- Line 105: Improved path construction with proper quoting (
"${ubootdir}/u-boot.bin"vs. the previous$ubootdir"/u-boot.bin")- Explicit
declare -gmakes the global scope of these build variables clear
116-171: LGTM: Excellent error handling and quoting improvements.The refactoring includes several important improvements:
- Error handling with
exit_with_errorfor directory changes (lines 116, 129, 140, 170) prevents silent failures- Extensive quoting throughout the function prevents word splitting and glob expansion issues
- Proper array declarations with
declare -a(lines 142-143)- Proper scoping for loop variables (line 146)
These changes significantly improve robustness and address shellcheck warnings as noted in the PR objectives.
rpardini
added
Ready to merge
igorpecovnik
approved these changes
Jan 8, 2026
This was referenced Jan 10, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
/inATF1compile_atf()'s LDFLAGS handling in atf: once againno-warn-rwx-segmentwoes #9162macchiatobin-doubleshot, but that is .eos anywaySummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.