-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
LowMem extension (for boards with < 256MB RAM) #8839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a low-memory optimization extension and BSP assets. New Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas to focus review on:
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (16)📓 Common learnings📚 Learning: 2025-09-18T03:36:17.862ZApplied to files:
📚 Learning: 2025-07-17T04:12:33.125ZApplied to files:
📚 Learning: 2025-08-30T04:13:16.457ZApplied to files:
📚 Learning: 2025-09-14T06:19:06.828ZApplied to files:
📚 Learning: 2025-11-02T20:49:56.719ZApplied to files:
📚 Learning: 2025-06-14T05:53:10.627ZApplied to files:
📚 Learning: 2025-06-14T05:53:10.627ZApplied to files:
📚 Learning: 2025-09-18T03:36:17.862ZApplied to files:
📚 Learning: 2025-04-28T08:27:26.890ZApplied to files:
📚 Learning: 2025-10-24T04:46:22.901ZApplied to files:
📚 Learning: 2025-07-25T03:51:50.830ZApplied to files:
📚 Learning: 2025-07-25T03:51:50.830ZApplied to files:
📚 Learning: 2025-09-25T18:37:00.330ZApplied to files:
📚 Learning: 2025-07-21T04:12:02.439ZApplied to files:
📚 Learning: 2025-07-23T10:01:41.310ZApplied to files:
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
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 (3)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
extensions/lowmem.sh (3)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/rootfs/trap-rootfs.sh (1)
prepare_rootfs_build_params_and_trap(12-51)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_base(425-446)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Shell script analysis
🔇 Additional comments (9)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (3)
1-2: LGTM!Strict error handling with
set -euo pipefailis appropriate for a first-boot critical script.
4-13: LGTM!The safety check prevents overwriting an existing swapfile, which complements the systemd
ConditionPathExistsin the service unit.
15-21: LGTM!The free space check with 64MiB headroom provides reasonable safety margin before attempting swapfile creation.
packages/bsp/armbian-lowmem/lowmem-mkswap.service (2)
1-6: LGTM!The unit dependencies and ordering are correct: runs after filesystem resize, before basic.target, and only when swapfile doesn't exist.
13-14: LGTM!The install target
basic.targetis correct for this early-boot service.extensions/lowmem.sh (4)
1-10: LGTM!Clear documentation of the extension's purpose and scope.
11-21: Installation paths are correct; verify service file matches.The extension correctly installs the script as
/usr/bin/lowmem-mkswap.sh, but the service file content references/usr/bin/rv1106-firstboot-makeswap.sh. Ensure the service file is updated to match (see comment on lowmem-mkswap.service line 10).
23-30: LGTM!The service enablement using
chroot_sdcardis correct for the build process.
32-51: Sed patterns verified and function implementation is correct.The regex patterns correctly handle the default Armbian configuration files:
- Line 35:
s/^ENABLED=.*/ENABLED=false/correctly replacesENABLED=truein armbian-ramlog config- Line 38:
s/^#\?\s*SWAP=.*/SWAP=false/correctly uncomments and sets# SWAP=falsetoSWAP=falsein armbian-zram-config- Line 44: The fstab grep check appropriately prevents duplicate
/runtmpfs entriesAll memory optimization settings are applied correctly for low-memory devices.
|
have you considered installing dropbear with this extension instead of openssh? should save a few megabytes of memory. |
16578a9 to
0721f22
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (1)
10-10: Script name mismatch — service will fail to execute.The ExecStart path references
/usr/bin/lowmem-makeswap.sh, but the extension (extensions/lowmem.sh line 17) installs the script as/usr/bin/lowmem-mkswap.sh. The service will fail because the referenced script does not exist.Apply this diff:
-ExecStart=/usr/bin/lowmem-makeswap.sh +ExecStart=/usr/bin/lowmem-mkswap.sh
🧹 Nitpick comments (3)
extensions/lowmem.sh (3)
17-18: Quote variables and use consistent installation commands.Variables like
$SRCand$destinationshould be quoted to handle paths with spaces safely. Additionally, line 18 usescpwhile line 17 usesinstall— preferinstallthroughout for explicit permission and ownership control.Apply this diff:
- install -m 755 $SRC/packages/bsp/armbian-lowmem/${service_name}.sh $destination/usr/bin/${service_name}.sh - cp $SRC/packages/bsp/armbian-lowmem/${service_name}.service $destination/lib/systemd/system/${service_name}.service + install -m 755 "${SRC}/packages/bsp/armbian-lowmem/${service_name}.sh" "${destination}/usr/bin/${service_name}.sh" + install -m 644 "${SRC}/packages/bsp/armbian-lowmem/${service_name}.service" "${destination}/lib/systemd/system/${service_name}.service"
35-35: Inconsistent path quoting.The path uses both quoted and unquoted segments:
"${MOUNT}"/etc/default/armbian-ramlog. While this works, it's clearer to quote the entire path consistently.Apply this diff for consistency:
- sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}"/etc/default/armbian-ramlog + sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}/etc/default/armbian-ramlog"
35-38: Consider adding defensive checks for configuration files.The
sedcommands assume the target files exist and contain the expected patterns. While this follows existing project patterns (see lib/functions/bsp/armbian-bsp-cli-deb.sh lines 408, 434-436), adding file existence checks would make the script more robust.Example approach:
if [[ -f "${MOUNT}/etc/default/armbian-ramlog" ]]; then sed -i "s/^ENABLED=.*/ENABLED=false/" "${MOUNT}/etc/default/armbian-ramlog" fi if [[ -f "${MOUNT}/etc/default/armbian-zram-config" ]]; then sed -i "s/^#\?\s*SWAP=.*/SWAP=false/" "${MOUNT}/etc/default/armbian-zram-config" fi
📜 Review details
Configuration used: CodeRabbit UI
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 (3)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bsp/armbian-lowmem/lowmem-mkswap.sh
🧰 Additional context used
🧬 Code graph analysis (2)
extensions/lowmem.sh (3)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/rootfs/trap-rootfs.sh (1)
prepare_rootfs_build_params_and_trap(12-51)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (4)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_finish(448-471)board_side_bsp_cli_postinst_base(425-446)board_side_bsp_cli_postrm(417-423)
🔇 Additional comments (1)
extensions/lowmem.sh (1)
23-30: LGTM!The function properly uses
chroot_sdcardto enable the service, and variables are correctly quoted.
That's certainly interesting, given the memory limitations we're working with here. Are you planning to PR your |
No, probably not. If you want to adopt that partially or in full, feel free to go ahead. |
0721f22 to
f76336a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/lowmem.sh (1)
1-1: Consider removing the shebang per Armbian conventions.Extension scripts in the
extensions/directory are sourced by the build framework rather than executed directly, so the shebang line is unnecessary. While harmless, removing it would align with Armbian project conventions for extension files.Apply this diff:
-#!/usr/bin/env bash - # # Armbian Optimizations for Low-Memory BoardsBased on learnings
📜 Review details
Configuration used: CodeRabbit UI
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 (4)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bsp/armbian-lowmem/lowmem-mkswap.service
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: EvilOlaf
PR: armbian/build#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: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
📚 Learning: 2025-09-14T06:19:06.828Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.shpackages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.sh
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-08-30T04:13:16.457Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-30T04:13:16.457Z
Learning: Armbian kernel configuration files like linux-filogic-current.config are autogenerated overlays on top of arch defconfig. Comments added manually will be lost during future updates by maintainers, and explicit "CONFIG_OPTION is not set" statements aren't needed for mutually exclusive options since these are overlay configs that only specify changes from the base configuration.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
PR: armbian/build#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:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
PR: armbian/build#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:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-27T15:53:30.629Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
PR: armbian/build#8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Applied to files:
extensions/lowmem.sh
🧬 Code graph analysis (2)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_base(425-446)
extensions/lowmem.sh (3)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
board_side_bsp_cli_postinst_finish(448-471)
🔇 Additional comments (7)
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist (1)
1-8: LGTM! Configuration defaults are appropriate.The configuration file provides sensible defaults for low-memory boards (256 MiB swapfile at /swapfile) and addresses the configurability requirements raised in previous reviews. Users can edit this file to customize the swapfile size and location based on their specific hardware (e.g., choosing between microSD and eMMC).
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (3)
1-7: LGTM! Configuration override mechanism works correctly.The script provides sensible defaults while allowing users to override both swapfile location and size via
/etc/default/armbian-lowmem. This addresses the configurability requirements for boards with different storage configurations (microSD vs eMMC).
9-24: LGTM! Safety checks are thorough and correct.The script properly guards against overwriting existing swapfiles and ensures sufficient free space (swapfile size + 64 MiB headroom) before creating the swapfile. The use of
dirnameon line 18 to obtain the parent directory for thedfcheck is correct.
26-39: LGTM! Swapfile creation and service cleanup are correct.The script correctly creates the swapfile with secure permissions (600), initializes it with
mkswap, adds an appropriate fstab entry (without thediscardoption, addressing previous review feedback), and enables the swap. The service properly disables and masks itself after successful execution to prevent repeated runs.extensions/lowmem.sh (3)
11-21: LGTM! Configuration installation is idempotent.The function correctly copies the default configuration file only when it doesn't already exist, ensuring idempotent behavior during repeated builds.
35-42: LGTM! Service enablement uses the correct framework function.The function properly uses
chroot_sdcardto enable the service within the target image's systemd environment.
44-64: LGTM! Memory optimizations are well-designed and configurable.The function appropriately:
- Disables ramlog to reduce memory overhead
- Disables zram swap (aligning with the PR's objective to favor zswap for low-memory boards)
- Sets a fixed
/runtmpfs size (default 20M, configurable viaLOWMEM_TMPFS_RUN_MBenvironment variable) to prevent systemd daemon-reload errors on systems where the default 10% of RAM would be insufficient- Implements idempotent fstab modifications
f76336a to
327d1fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (2)
4-4: Consider adding explicit dependency on armbian-resize-filesystem.service.Currently,
After=ensures ordering but doesn't create a dependency. Ifarmbian-resize-filesystem.servicefails or is disabled, this service will still run, potentially creating the swapfile before the filesystem is properly resized.Consider applying this diff:
DefaultDependencies=no +Wants=armbian-resize-filesystem.service After=armbian-resize-filesystem.serviceThis ensures the resize service is pulled in when this service is enabled, improving robustness while maintaining the correct boot order.
7-10: Consider adding a timeout for boot reliability.The service creates a 256MB swapfile, which may take time on slow storage. Without an explicit timeout, the default (typically 90s) applies. If swapfile creation hangs or exceeds the timeout, the boot will be delayed or fail.
Consider applying this diff to add a reasonable timeout:
[Service] Type=oneshot ExecStart=/usr/bin/lowmem-mkswap.sh +TimeoutStartSec=180 RemainAfterExit=yesA 3-minute timeout provides sufficient time for slow embedded storage while preventing indefinite boot hangs. Adjust the value based on testing with target hardware.
📜 Review details
Configuration used: CodeRabbit UI
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 (4)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
- extensions/lowmem.sh
- packages/bsp/armbian-lowmem/lowmem-mkswap.sh
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: EvilOlaf
PR: armbian/build#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
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
📚 Learning: 2025-09-14T06:19:06.828Z
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.service
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.service
🧬 Code graph analysis (1)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (2)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_postinst_finish(448-471)board_side_bsp_cli_postrm(417-423)extensions/armbian-live-patch.sh (1)
post_family_tweaks_bsp__armbian-live-patch(1-97)
🔇 Additional comments (1)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (1)
1-13: Service structure looks correct for first-boot swap setup.The systemd unit follows appropriate patterns:
Type=oneshotwithRemainAfterExit=yesfor one-time setupBefore=basic.targetensures swap is available early (aligns with PR objectives)After=armbian-resize-filesystem.serviceensures proper filesystem stateWantedBy=basic.targetfor correct installation pointThe fixed
ExecStartpath matches the installation location from the extension. The self-masking behavior (mentioned in past review comments) handles the first-boot-only requirement.The suggested improvements above (timeout and explicit dependency) would enhance robustness but are not blockers.
@coderabbitai Further, I think the criticality of this on a low-RAM machine makes failure preferable. |
|
Tip For best results, initiate chat on the files or code changes. @tabrisnet You make excellent points. You're absolutely right that:
Your reasoning is sound—the timeout suggestion isn't necessary here, and the default behavior is appropriate for this use case. Thank you for the thoughtful pushback! 🧠 Learnings used |
For what it's worth. This oneshot script typically finishes in about 5 seconds 😅 on RV1103 (and my microSD card is nothing fancy) the default 90 should be plenty I agree. I suppose this could fail if the user decided to increase |
|
Tip For best results, initiate chat on the files or code changes. @vidplace7 Perfect! 5 seconds is excellent, and that real-world data confirms the default 90s timeout is more than sufficient. I agree with your assessment—and yes, if someone cranks that swapfile up to multiple GBs on slow storage, a timeout failure is indeed the appropriate consequence. 😄 The current configuration is sound as-is. Thank you for the empirical confirmation! 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. @vidplace7 Thanks for the real-world data point! 5 seconds on actual hardware confirms the default timeout is more than adequate. And I completely agree with your philosophy on the edge case—if someone cranks the swapfile to multiple GBs on a low-RAM board, a systemd timeout is the least of their worries. 😄 The original suggestion to add 🐰✨ 🧠 Learnings used |
|
Tip For best results, initiate chat on the files or code changes. @vidplace7 Perfect! That 5-second real-world measurement is exactly the kind of data that confirms the default timeout is more than adequate. Thanks for providing the empirical evidence—much appreciated! And yes, I completely agree with your philosophy on the edge case. If someone configures a multi-GB swapfile on a low-RAM board, the systemd timeout is indeed the appropriate feedback mechanism. 😄 🧠 Learnings used |
327d1fb to
a375b12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/lowmem.sh (1)
1-1: Remove unnecessary shebang for consistency.Extension scripts in the Armbian build system are sourced, not executed directly, so the shebang is unnecessary. As per established convention, remove this line.
-#!/usr/bin/env bash - #Based on learnings from project conventions for extension scripts.
📜 Review details
Configuration used: CodeRabbit UI
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 (4)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bsp/armbian-lowmem/lowmem-mkswap.service
- packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
- packages/bsp/armbian-lowmem/lowmem-mkswap.sh
🧰 Additional context used
🧠 Learnings (22)
📓 Common learnings
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: amazingfate
PR: armbian/build#8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
PR: armbian/build#8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-05-16T15:34:34.672Z
Learnt from: Grippy98
PR: armbian/build#8202
File: config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh:2-3
Timestamp: 2025-05-16T15:34:34.672Z
Learning: The shell scripts in the Armbian build system's desktop configuration files, such as config/desktop/trixie/environments/gnome/armbian/create_desktop_package.sh, rely on environment variables like $SRC and $destination that are expected to be set externally before the script is executed.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-05-29T01:52:21.381Z
Learnt from: djurny
PR: armbian/build#8235
File: packages/bsp/mvebu/helios4/helios4-wol.service:0-0
Timestamp: 2025-05-29T01:52:21.381Z
Learning: In systemd service ExecStart commands, shell variables like ${INTF} can cause issues because systemd may resolve them before the shell processes them, leading to "no device matches name" errors. Using xargs with -I{} is a more reliable approach for iterating over dynamic lists in systemd services, as it avoids variable substitution problems entirely.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-14T17:19:39.693Z
Learnt from: leggewie
PR: armbian/build#8502
File: config/desktop/trixie/environments/i3-wm/config_base/packages:44-44
Timestamp: 2025-08-14T17:19:39.693Z
Learning: When a PR author provides clear context about package transitions in the commit message and the changes are scoped to specific release pockets, focus the review on validating those specific changes rather than suggesting broader investigations across other releases or additional dependency verifications.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
PR: armbian/build#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.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-13T19:51:14.713Z
Learnt from: pyavitz
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-09-25T18:37:00.330Z
Learnt from: tabrisnet
PR: armbian/build#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.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
PR: armbian/build#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.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-29T18:44:47.732Z
Learnt from: leggewie
PR: armbian/build#0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-11T04:34:05.589Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-09-11T04:34:05.589Z
Learning: When users identify architectural issues during code review, they may prefer to split them into separate issues rather than expanding the scope of the current PR. This allows for focused fixes while ensuring broader problems get proper tracking and discussion.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
PR: armbian/build#0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-05-07T20:49:40.969Z
Learnt from: djurny
PR: armbian/build#8166
File: config/bootscripts/boot-mvebu.cmd:38-50
Timestamp: 2025-05-07T20:49:40.969Z
Learning: The 4KB (0x1000) boundary used for device tree size calculations in U-Boot scripts is specific to how the `fdt resize` command works internally and should remain hardcoded rather than being tied to adjustable alignment variables.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
PR: armbian/build#8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-27T15:53:30.629Z
Learnt from: pyavitz
PR: armbian/build#8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
PR: armbian/build#8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
PR: armbian/build#8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-03T15:21:20.148Z
Learnt from: pyavitz
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
PR: armbian/build#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:
extensions/lowmem.sh
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
PR: armbian/build#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:
extensions/lowmem.sh
🧬 Code graph analysis (1)
extensions/lowmem.sh (4)
.github/generate_CODEOWNERS.sh (1)
display_alert(6-6)lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_postinst_base(425-446)board_side_bsp_cli_postinst_finish(448-471)lib/functions/rootfs/post-tweaks.sh (1)
post_debootstrap_tweaks(10-34)
🔇 Additional comments (2)
extensions/lowmem.sh (2)
11-21: Verify file permissions for user-customizable config file.The config file is installed with mode
664(rw-rw-r--), which grants group write access. Most/etc/defaultfiles in Armbian use644(rw-r--r--) to prevent accidental group modifications. Confirm whether group write access is intentional forarmbian-lowmem.If group write is not needed:
- install -m 664 "$SRC/packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist" "$destination/etc/default/armbian-lowmem" + install -m 644 "$SRC/packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist" "$destination/etc/default/armbian-lowmem"
44-64: Solid structure and idempotent design.The memory optimization function correctly:
- Sets a configurable default for
/runtmpfs size (line 45)- Handles sed replacements safely (lines 48, 51)
- Checks for existing
/runentry before appending to fstab, preventing duplicates (line 57)- Uses a robust POSIX-compliant regex with
[[:space:]]+to handle multiple spaces/tabs in fstabThe overall extension properly integrates with the Armbian framework (hook naming,
chroot_sdcard, framework variables). 🎉
255c770 to
2099fbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
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 (4)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bsp/armbian-lowmem/lowmem-mkswap.service
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
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.
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`.
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-08-30T04:13:16.457Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-30T04:13:16.457Z
Learning: Armbian kernel configuration files like linux-filogic-current.config are autogenerated overlays on top of arch defconfig. Comments added manually will be lost during future updates by maintainers, and explicit "CONFIG_OPTION is not set" statements aren't needed for mutually exclusive options since these are overlay configs that only specify changes from the base configuration.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
📚 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:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.sh
📚 Learning: 2025-09-14T06:19:06.828Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distpackages/bsp/armbian-lowmem/lowmem-mkswap.sh
📚 Learning: 2025-11-02T20:49:56.697Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.697Z
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:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.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:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.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: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Applied to files:
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-distextensions/lowmem.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:
extensions/lowmem.sh
📚 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:
extensions/lowmem.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:
extensions/lowmem.sh
📚 Learning: 2025-05-29T01:52:21.381Z
Learnt from: djurny
Repo: armbian/build PR: 8235
File: packages/bsp/mvebu/helios4/helios4-wol.service:0-0
Timestamp: 2025-05-29T01:52:21.381Z
Learning: In systemd service ExecStart commands, shell variables like ${INTF} can cause issues because systemd may resolve them before the shell processes them, leading to "no device matches name" errors. Using xargs with -I{} is a more reliable approach for iterating over dynamic lists in systemd services, as it avoids variable substitution problems entirely.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-14T17:19:39.693Z
Learnt from: leggewie
Repo: armbian/build PR: 8502
File: config/desktop/trixie/environments/i3-wm/config_base/packages:44-44
Timestamp: 2025-08-14T17:19:39.693Z
Learning: When a PR author provides clear context about package transitions in the commit message and the changes are scoped to specific release pockets, focus the review on validating those specific changes rather than suggesting broader investigations across other releases or additional dependency verifications.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
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.
Applied to files:
extensions/lowmem.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:
extensions/lowmem.sh
📚 Learning: 2025-09-25T18:37:00.330Z
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.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
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.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-29T18:44:47.732Z
Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-11T04:34:05.589Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-09-11T04:34:05.589Z
Learning: When users identify architectural issues during code review, they may prefer to split them into separate issues rather than expanding the scope of the current PR. This allows for focused fixes while ensuring broader problems get proper tracking and discussion.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Applied to files:
extensions/lowmem.shpackages/bsp/armbian-lowmem/lowmem-mkswap.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:
extensions/lowmem.sh
📚 Learning: 2025-07-27T15:53:30.629Z
Learnt from: pyavitz
Repo: armbian/build PR: 8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/lowmem.sh
📚 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:
extensions/lowmem.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:
extensions/lowmem.sh
🧬 Code graph analysis (3)
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_finish(448-471)
extensions/lowmem.sh (2)
lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
board_side_bsp_cli_postinst_finish(448-471)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_base(425-446)
🔇 Additional comments (3)
packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist (1)
1-8: LGTM!Configuration values are appropriate for low-memory boards, comments are clear, and the format follows established Armbian conventions for default config files.
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
1-35: Well-structured script with proper safety checks.The script demonstrates good practices:
- Idempotent behavior (exits cleanly if swapfile exists)
- Safety checks for free space with 64MiB headroom
- Correct permissions (600) for swapfile
- Proper use of
fallocatefor fast allocation- Past review feedback addressed (
discard=once, configurable values via/etc/default/armbian-lowmem, correct use ofdirname)extensions/lowmem.sh (1)
1-63: Well-designed extension following Armbian conventions.The extension properly implements four hook functions with clear separation of concerns:
- Config installation with idempotent checks
- Script and service installation
- Service enablement
- Image-level memory optimizations
The implementation follows established patterns:
- Hook function naming enables automatic discovery by the framework
- Proper use of
display_alertfor debugging visibility- Idempotent operations (checks before copying/appending)
- Correct fstab regex pattern on line 56 (matches
tmpfs /run tmpfsentries)- Past feedback addressed (quoting, file permissions)
Note: The shebang on line 1 is optional for extensions (they're sourced, not executed) but harmless. Based on learnings.
2099fbc to
f82f6a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
31-31: CRITICAL: Fix the broken fstab regex pattern.The grep pattern on line 31 will never match existing swapfile entries in fstab, causing duplicate entries to be added on subsequent runs. The
[^#]*prefix is greedy and consumes the entire line including the swapfile path, leaving nothing for the literal${SWAPFILE_PATH}to match against.For example, with the fstab line
/swapfile swap swap defaults,nofail,discard=once,pri=0 0 0:
[^#]*matches the entire line- There's nothing left for
${SWAPFILE_PATH}to match- The grep fails even though the entry exists
Apply this diff to fix the pattern (consistent with standard fstab matching):
-if ! grep -qE "^[^#]*[[:space:]]+${SWAPFILE_PATH}[[:space:]]+swap[[:space:]]" /etc/fstab; then +if ! grep -qE "^[[:space:]]*${SWAPFILE_PATH}[[:space:]]+swap[[:space:]]+swap" /etc/fstab; thenThis pattern correctly matches:
- Optional leading whitespace
- The swapfile path
- Whitespace(s)
- "swap" (mount point field)
- Whitespace(s)
- "swap" (filesystem type field)
🧹 Nitpick comments (1)
extensions/lowmem.sh (1)
56-60: Consider making the fstab grep pattern more robust.The current pattern works for typical fstab entries starting at column 0, but could be more robust by handling optional leading whitespace for consistency with defensive coding practices.
Apply this diff for improved robustness:
- if ! grep -qE "tmpfs[[:space:]]+/run[[:space:]]+tmpfs" "${MOUNT}/etc/fstab"; then + if ! grep -qE "^[[:space:]]*tmpfs[[:space:]]+/run[[:space:]]+tmpfs" "${MOUNT}/etc/fstab"; thenThis anchors the pattern to the start of the line and handles any leading whitespace, though in practice fstab entries rarely have leading spaces.
📜 Review details
Configuration used: CodeRabbit UI
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 (4)
extensions/lowmem.sh(1 hunks)packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.service(1 hunks)packages/bsp/armbian-lowmem/lowmem-mkswap.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bsp/armbian-lowmem/etc/default/armbian-lowmem.dpkg-dist
🧰 Additional context used
🧠 Learnings (25)
📓 Common learnings
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: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Learnt from: 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`.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
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.
📚 Learning: 2025-09-14T06:19:06.828Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/kernel/linux-rockchip-vendor.config:0-0
Timestamp: 2025-09-14T06:19:06.828Z
Learning: CONFIG_ZSWAP has an implicit kernel-level dependency on CONFIG_SWAP, so when CONFIG_ZSWAP=y is set in Armbian overlay configs, CONFIG_SWAP gets automatically enabled during kernel build configuration processing, even if not explicitly specified in the overlay file.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.shpackages/bsp/armbian-lowmem/lowmem-mkswap.service
📚 Learning: 2025-10-24T04:46:22.901Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-10-24T04:46:22.901Z
Learning: In lib/functions/rootfs/rootfs-create.sh, the FIXME comment about mmdebstrap usage with --aptopt is a future note related to PR #8785, which hasn't been merged yet.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.shpackages/bsp/armbian-lowmem/lowmem-mkswap.serviceextensions/lowmem.sh
📚 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:
packages/bsp/armbian-lowmem/lowmem-mkswap.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:
packages/bsp/armbian-lowmem/lowmem-mkswap.serviceextensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
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.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.serviceextensions/lowmem.sh
📚 Learning: 2025-07-25T03:51:50.830Z
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.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.serviceextensions/lowmem.sh
📚 Learning: 2025-09-25T18:37:00.330Z
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.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.serviceextensions/lowmem.sh
📚 Learning: 2025-07-21T04:12:02.439Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
Applied to files:
packages/bsp/armbian-lowmem/lowmem-mkswap.service
📚 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:
packages/bsp/armbian-lowmem/lowmem-mkswap.service
📚 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: In Armbian build framework, extensions are loaded and executed via lib/functions/general/extensions.sh using call_extension_method. The framework sets set -o errexit and set -o errtrace globally, and individual commands run with pipefail protection through the command execution functions.
Applied to files:
extensions/lowmem.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:
extensions/lowmem.sh
📚 Learning: 2025-05-29T01:52:21.381Z
Learnt from: djurny
Repo: armbian/build PR: 8235
File: packages/bsp/mvebu/helios4/helios4-wol.service:0-0
Timestamp: 2025-05-29T01:52:21.381Z
Learning: In systemd service ExecStart commands, shell variables like ${INTF} can cause issues because systemd may resolve them before the shell processes them, leading to "no device matches name" errors. Using xargs with -I{} is a more reliable approach for iterating over dynamic lists in systemd services, as it avoids variable substitution problems entirely.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-08-14T17:19:39.693Z
Learnt from: leggewie
Repo: armbian/build PR: 8502
File: config/desktop/trixie/environments/i3-wm/config_base/packages:44-44
Timestamp: 2025-08-14T17:19:39.693Z
Learning: When a PR author provides clear context about package transitions in the commit message and the changes are scoped to specific release pockets, focus the review on validating those specific changes rather than suggesting broader investigations across other releases or additional dependency verifications.
Applied to files:
extensions/lowmem.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:
extensions/lowmem.sh
📚 Learning: 2025-08-29T18:44:47.732Z
Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
Applied to files:
extensions/lowmem.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:
extensions/lowmem.sh
📚 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:
extensions/lowmem.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:
extensions/lowmem.sh
📚 Learning: 2025-07-27T15:53:30.629Z
Learnt from: pyavitz
Repo: armbian/build PR: 8421
File: config/kernel/linux-sunxi64-edge.config:805-825
Timestamp: 2025-07-27T15:53:30.629Z
Learning: In the Armbian build system, kernel configurations prioritize broad hardware compatibility over optimization concerns. Even when including numerous legacy/obsolete drivers causes technical issues like increased initramfs size and slower enumeration, the project philosophy is to "appease the masses" by ensuring maximum device compatibility rather than optimizing for specific use cases.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/lowmem.sh
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).
Applied to files:
extensions/lowmem.sh
📚 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:
extensions/lowmem.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:
extensions/lowmem.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:
extensions/lowmem.sh
🧬 Code graph analysis (3)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (2)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_base(425-446)
packages/bsp/armbian-lowmem/lowmem-mkswap.service (1)
lib/functions/bsp/armbian-bsp-cli-deb.sh (3)
board_side_bsp_cli_preinst(360-415)board_side_bsp_cli_postinst_finish(448-471)board_side_bsp_cli_postrm(417-423)
extensions/lowmem.sh (3)
lib/functions/logging/runners.sh (1)
chroot_sdcard(133-136)lib/functions/bsp/armbian-bsp-cli-deb.sh (1)
board_side_bsp_cli_postinst_finish(448-471)lib/functions/rootfs/post-tweaks.sh (1)
post_debootstrap_tweaks(10-34)
🔇 Additional comments (9)
packages/bsp/armbian-lowmem/lowmem-mkswap.sh (4)
1-3: LGTM! Appropriate error handling for first-boot script.The shebang and strict error handling (
set -euo pipefail) are appropriate for this critical first-boot swapfile creation script.
4-8: LGTM! Sensible defaults with user override capability.The default swapfile configuration (256MB at /swapfile) is appropriate for low-memory boards, and the config file sourcing pattern allows users to customize these values as discussed in the PR.
11-15: LGTM! Graceful handling of existing swapfile.The exit-on-existing-swapfile logic is correct per the design discussion. Returning 0 allows the script to run on every boot without systemd marking it as failed when the swapfile is already present.
17-24: LGTM! Appropriate free-space validation with headroom.The free-space check with 64MB headroom is a good safety measure that prevents out-of-space errors during swapfile creation. The fail-fast behavior (exit 1) is appropriate for low-memory devices.
packages/bsp/armbian-lowmem/lowmem-mkswap.service (1)
1-14: LGTM! Well-structured systemd unit with appropriate dependencies.The systemd unit configuration is correct:
DefaultDependencies=nowithBefore=basic.targetensures swap is available early in the boot processWants=armbian-resize-filesystem.serviceestablishes a soft dependency that won't cause failure if the resize service is absentAfter=armbian-resize-filesystem.serviceensures proper ordering when both services are presentType=oneshotwithRemainAfterExit=yesis the standard pattern for one-time initializationPrevious issues (script path mismatch) have been addressed per earlier reviews.
extensions/lowmem.sh (4)
11-21: LGTM! Idiomatic config installation with overwrite protection.The function correctly installs the default lowmem configuration with appropriate permissions (664) while guarding against overwriting existing customizations. The pattern aligns with Armbian BSP conventions visible in lib/functions/bsp/armbian-bsp-cli-deb.sh.
23-33: LGTM! Proper installation of script and service with correct permissions.The function uses the
installcommand with appropriate modes (755 for executable script, 644 for service file) and properly quoted paths. Previous quoting issues have been addressed.
35-41: LGTM! Correct service enablement in chroot context.The function properly uses
chroot_sdcardto enable the lowmem-mkswap service within the target image. This aligns with the Armbian extension framework patterns. Based on learnings.
44-51: LGTM! Appropriate memory optimization defaults.The memory optimization settings are well-chosen for low-RAM boards:
- 20MB fixed size for /run tmpfs prevents systemd daemon-reload issues when free memory drops below ~16MB
- Disabling ramlog reduces memory consumption
- Disabling zram swap in favor of file-based swap + zswap is appropriate per the PR objectives
The sed patterns correctly handle both commented and uncommented configuration lines.
|
Tested this PR again after making all requested changes.
This should be good to merge 🙏 |
f82f6a4 to
aea06e7
Compare
Description
Adds new extension for userland tweaks for memory starved boards.
This work has been split off from PR #8797 as it will also apply to other memory starved boards outside of this family.
Optimizations
apt-getandlocale-gen.basic.target(available early)/runtmpfs size to 20M (configurable). systemd throws errors when less than 16MB is free in this partition duringdaemon-reload.Documentation summary for feature / change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist: