"get completely rid of dead code toolchain stuff", pt3 #9252
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
- if/when we change `ATFSOURCE`, we want u-boot artifacts to be rebuilt
- no need to keep repeating this across families
- those are the arch defaults, no need to repeat
- "are we on arch_xyz? no? then use this `-none-` compiler. Oh wait, now remove all `-none-` substrings from compiler. Deal."
- it doesn't matter on what it is running, it's always the same compiler
- no longer needed/useful/real
Contributor
📝 WalkthroughWalkthroughThis PR consolidates and simplifies build configuration for ARM Trusted Firmware (ATF) and cross-compilation toolchains across multiple architectures. It removes architecture-specific compiler detection logic, removes outdated GCC toolchain checksums, and introduces defaults for ATF source/branch variables while expanding ATF configuration exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
rpardini
requested review from
a team,
hzyitc,
iav,
igorpecovnik and
mhoffrog
as code owners
github-actions
bot
added
Needs review
![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: 1
🤖 Fix all issues with AI agents
In `@config/sources/arm64.conf`:
- Around line 26-31: The file contains a duplicate default assignment for the
KERNEL_COMPILER variable (two occurrences of [[ -z $KERNEL_COMPILER ]] &&
KERNEL_COMPILER="aarch64-linux-gnu-"); remove the redundant second assignment so
only the initial KERNEL_COMPILER default remains, or if a different variable was
intended there, replace it with the correct variable name instead.
🧹 Nitpick comments (1)
config/sources/amd64.conf (1)
21-21: Dead code:KERNEL_COMPILERon line 21 is overwritten by line 30.Line 21's declaration (
KERNEL_COMPILER=' ') with the "single space hack" comment is now dead code since line 30 unconditionally overwrites it. Consider removing line 21 to avoid confusion.♻️ Suggested fix
declare -g MAIN_CMDLINE='' # we set it in common, it was not set before -declare -g KERNEL_COMPILER=' ' # hack: use single space for host gcc. won't work on arm64 hosts declare -g KERNEL_EXTRA_TARGETS='modules' # default is "modules dtb" but x86_64 has no DTB declare -g KERNEL_BUILD_DTBS="no" # amd64 has no DTBs. that I know of.Also applies to: 30-31
📜 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 (41)
config/sources/amd64.confconfig/sources/arm64.confconfig/sources/armhf.confconfig/sources/families/d1.confconfig/sources/families/include/rockchip64_common.incconfig/sources/families/mvebu64.confconfig/sources/families/nuvoton-ma35d1.confconfig/sources/families/spacemit.confconfig/sources/riscv64.confconfig/torrents/gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu.tar.xz.torrentconfig/torrents/gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu.tar.xz.torrent.md5config/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-eabi.tar.xz.torrentconfig/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-eabi.tar.xz.torrent.md5config/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-linux-gnueabihf.tar.xz.torrentconfig/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-linux-gnueabihf.tar.xz.torrent.md5config/torrents/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz.ascconfig/torrents/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz.md5config/torrents/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz.torrentconfig/torrents/gcc-arm-8.3-2019.03-x86_64-arm-linux-gnueabihf.tar.xz.ascconfig/torrents/gcc-arm-8.3-2019.03-x86_64-arm-linux-gnueabihf.tar.xz.md5config/torrents/gcc-arm-8.3-2019.03-x86_64-arm-linux-gnueabihf.tar.xz.torrentconfig/torrents/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz.ascconfig/torrents/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz.md5config/torrents/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz.torrentconfig/torrents/gcc-arm-9.2-2019.12-x86_64-arm-none-linux-gnueabihf.tar.xz.ascconfig/torrents/gcc-arm-9.2-2019.12-x86_64-arm-none-linux-gnueabihf.tar.xz.md5config/torrents/gcc-arm-9.2-2019.12-x86_64-arm-none-linux-gnueabihf.tar.xz.torrentconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz.ascconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz.md5config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz.torrentconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-eabi.tar.xz.ascconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-eabi.tar.xz.md5config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-eabi.tar.xz.torrentconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi.tar.xz.ascconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi.tar.xz.md5config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi.tar.xz.torrentconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabihf.tar.xz.ascconfig/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabihf.tar.xz.md5config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabihf.tar.xz.torrentlib/functions/artifacts/artifact-uboot.shlib/tools/info/inventory-boards-csv.py
💤 Files with no reviewable changes (24)
- config/sources/families/nuvoton-ma35d1.conf
- config/torrents/gcc-arm-8.3-2019.03-x86_64-arm-linux-gnueabihf.tar.xz.asc
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabihf.tar.xz.asc
- config/sources/families/include/rockchip64_common.inc
- config/sources/families/d1.conf
- config/torrents/gcc-arm-8.3-2019.03-x86_64-arm-linux-gnueabihf.tar.xz.md5
- config/torrents/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz.md5
- config/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-linux-gnueabihf.tar.xz.torrent.md5
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi.tar.xz.asc
- config/torrents/gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu.tar.xz.torrent.md5
- config/torrents/gcc-arm-9.2-2019.12-x86_64-arm-none-linux-gnueabihf.tar.xz.md5
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabi.tar.xz.md5
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-eabi.tar.xz.md5
- config/sources/families/spacemit.conf
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-eabi.tar.xz.asc
- config/torrents/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz.asc
- config/torrents/gcc-arm-11.2-2022.02-x86_64-arm-none-eabi.tar.xz.torrent.md5
- config/torrents/gcc-arm-9.2-2019.12-x86_64-aarch64-none-linux-gnu.tar.xz.md5
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz.md5
- config/torrents/gcc-arm-8.3-2019.03-x86_64-aarch64-linux-gnu.tar.xz.asc
- config/torrents/gcc-arm-9.2-2019.12-x86_64-arm-none-linux-gnueabihf.tar.xz.asc
- config/sources/families/mvebu64.conf
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_aarch64-linux-gnu.tar.xz.asc
- config/torrents/gcc-linaro-7.4.1-2019.02-x86_64_arm-linux-gnueabihf.tar.xz.md5
🧰 Additional context used
🧠 Learnings (32)
📓 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: EvilOlaf
Repo: armbian/build PR: 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: 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: 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: 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: 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: 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: 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).
📚 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/amd64.confconfig/sources/armhf.confconfig/sources/arm64.conf
📚 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/amd64.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/amd64.confconfig/sources/armhf.confconfig/sources/arm64.conf
📚 Learning: 2025-06-25T03:40:52.109Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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.
Applied to files:
config/sources/amd64.confconfig/sources/arm64.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/amd64.confconfig/sources/armhf.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/amd64.conflib/functions/artifacts/artifact-uboot.shconfig/sources/armhf.confconfig/sources/arm64.conf
📚 Learning: 2026-01-03T20:46:29.189Z
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>
Applied to files:
config/sources/amd64.conf
📚 Learning: 2025-12-28T01:49:34.661Z
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.
Applied to files:
config/sources/amd64.confconfig/sources/armhf.confconfig/sources/arm64.conf
📚 Learning: 2025-06-25T03:42:09.086Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 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.
Applied to files:
config/sources/amd64.conflib/functions/artifacts/artifact-uboot.sh
📚 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/amd64.confconfig/sources/armhf.conf
📚 Learning: 2025-09-12T09:49:31.957Z
Learnt from: amazingfate
Repo: armbian/build PR: 8617
File: lib/functions/compilation/kernel.sh:126-127
Timestamp: 2025-09-12T09:49:31.957Z
Learning: In the Armbian build system, the "all" target in kernel build has been used for a long time without issues. The "all" target builds vmlinux/Image/Image.gz defaults for the architecture and works correctly alongside KERNEL_IMAGE_TYPE in the build_targets_build array.
Applied to files:
config/sources/amd64.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/amd64.confconfig/sources/armhf.confconfig/sources/arm64.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/amd64.confconfig/sources/armhf.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/amd64.confconfig/sources/armhf.confconfig/sources/arm64.conf
📚 Learning: 2025-08-21T08:10:25.459Z
Learnt from: leggewie
Repo: armbian/build PR: 8524
File: config/boards/nanopiduo2.csc:12-12
Timestamp: 2025-08-21T08:10:25.459Z
Learning: KERNEL_TARGET and KERNEL_TEST_TARGET in Armbian build system do not have enforced validation relationships. KERNEL_TARGET is used for interactive menu ordering and branch validation, while KERNEL_TEST_TARGET is used for build list generation and recorded in /etc/armbian-release. No existing codebase validation enforces that KERNEL_TEST_TARGET values must be present in KERNEL_TARGET.
Applied to files:
config/sources/amd64.conf
📚 Learning: 2025-05-05T12:35:07.143Z
Learnt from: Grippy98
Repo: armbian/build PR: 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.
Applied to files:
config/sources/amd64.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/amd64.conflib/functions/artifacts/artifact-uboot.shconfig/sources/armhf.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:
lib/tools/info/inventory-boards-csv.py
📚 Learning: 2025-12-19T13:56:45.124Z
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.
Applied to files:
lib/functions/artifacts/artifact-uboot.sh
📚 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:
lib/functions/artifacts/artifact-uboot.sh
📚 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:
lib/functions/artifacts/artifact-uboot.sh
📚 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:
lib/functions/artifacts/artifact-uboot.sh
📚 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:
lib/functions/artifacts/artifact-uboot.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/artifacts/artifact-uboot.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
lib/functions/artifacts/artifact-uboot.sh
📚 Learning: 2025-10-03T11:42:46.660Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8705
File: .github/workflows/rewrite-kernel-configs.yml:96-169
Timestamp: 2025-10-03T11:42:46.660Z
Learning: In the Armbian build repository's kernel config rewrite workflow (.github/workflows/rewrite-kernel-configs.yml), deletion of kernel config files does not need to be handled. The workflow intentionally only captures ACMR (added/copied/modified/renamed) changes and does not need to propagate deletions to the aggregator.
Applied to files:
config/sources/armhf.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/armhf.conf
📚 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/sources/armhf.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/armhf.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/armhf.conf
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
config/sources/armhf.conf
🔇 Additional comments (6)
lib/functions/artifacts/artifact-uboot.sh (1)
102-102: LGTM!Including
ATFSOURCEin the artifact hash ensures that changes to the ATF source repository will correctly trigger u-boot artifact rebuilds. This completes the set of ATF-related variables being tracked for version computation.config/sources/riscv64.conf (1)
27-27: LGTM!The conditional default for
ATF_COMPILERfollows the established pattern in this file and correctly uses theriscv64-linux-gnu-prefix. The guard conditions ensure it only applies when ATF compilation is enabled and no override is set.lib/tools/info/inventory-boards-csv.py (1)
178-179: LGTM!Adding
ATFSOURCEandATFBRANCHto the inventory export provides visibility into ATF configuration across boards, complementing the artifact hashing changes.config/sources/armhf.conf (1)
28-28: LGTM!The simplified
UBOOT_COMPILERdefault follows the same pattern asKERNEL_COMPILERon line 27, maintaining consistency and removing unnecessary complexity.config/sources/arm64.conf (2)
33-36: ATF source defaults look good.The conditional defaults for
ATFSOURCE,ATFDIR, andATFBRANCHwhenATF_COMPILE != "no"are appropriate. These integrate well with the artifact hash computation changes mentioned in the PR objectives, ensuring that ATF source changes trigger u-boot artifact rebuilds.The TODO comment appropriately flags this for potential future refactoring into family-specific code.
38-38: Good defensive practice.The trailing
trueprevents non-zero exit status from unmatched short-circuit conditionals above.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
igorpecovnik
approved these changes
Jan 15, 2026
Contributor
|
✅ This PR has been reviewed and approved — all set for merge! |
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.
"get completely rid of dead code toolchain stuff", pt3
ATFSOURCEATFSOURCE, we want u-boot artifacts to be rebuiltATFSOURCEandATFBRANCH-none-compiler. Oh wait, now remove all-none-substrings from compiler. Deal."x86_64-linux-gnu-for x86 stuffATF_USE_GCCSummary by CodeRabbit
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.