Introduce unified image flashing tool for Armbian #705
Conversation
WalkthroughAdds a new public shell module at tools/modules/software/module_images.sh exposing module_images() and many helpers to fetch, filter, download, cache, verify, manage, and flash Armbian OS images. Implements interactive commands: install, remove, purge, status, and help with multi-stage filtering (board_slug, preinstalled_application, kernel_branch, image_variant, repository), block-device discovery/selection with destructive confirmation, dialog UI with console fallbacks, and streaming download/flash pipelines (curl → xz → pv → dd) including optional verification. Updates tools/json/config.system.json (STO001 short/description and added FLASH1/FLASH2 entries), adds FLASH1 header/footer markdown docs, and adds xz-utils and pv to the Debian CI workflow dependencies. Also updates module_options metadata for Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
size/large
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tools/modules/software/module_images.sh (2)
19-32: Consider hybrid module pattern for standalone execution support.Per the armbian/configng "independent drop in module concept" (PR 668), modules can support both sourced and standalone execution using
BASH_SOURCEchecks. The current implementation requires external sourcing to work.If this module should be executable standalone (with built-in testing), add this pattern at the start:
if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then # Standalone execution for testing module_images "$@" fiAlternatively, if this module is intentionally sourced-only, this is fine. Clarify the design intent in comments.
99-110: Performance: Multiple jq passes parse JSON repeatedly, but acceptable for typical Armbian catalog size.The same jq filtering logic (board normalization, file_extension validation, preapp/repo/kernel filters) is repeated 8+ times across different helper functions. Each pass independently scans the entire JSON file.
For future optimization, consider:
- Caching the filtered results in a temporary file after first pass
- Using a single jq pass to generate all menu options at once
- Storing precomputed indices by board
Current performance is likely acceptable for typical Armbian image indexes, but this could become a bottleneck with larger catalogs. Monitor and optimize if needed.
Also applies to: 141-151, 170-184, 274-299, 346-372, 421-473, 525-549, 900-909, 914-924
📜 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 (1)
tools/modules/software/module_images.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (1)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
🪛 GitHub Actions: Coding style check
tools/modules/software/module_images.sh
[error] 101-104: Wrong indentation type(spaces instead of tabs)
[error] 143-146: Wrong indentation type(spaces instead of tabs)
[error] 173-178: Wrong indentation type(spaces instead of tabs)
[error] 277-280: Wrong indentation type(spaces instead of tabs)
[error] 282-284: Wrong indentation type(spaces instead of tabs)
[error] 286-293: Wrong indentation type(spaces instead of tabs)
[error] 349-352: Wrong indentation type(spaces instead of tabs)
[error] 354-356: Wrong indentation type(spaces instead of tabs)
[error] 358-366: Wrong indentation type(spaces instead of tabs)
[error] 528-531: Wrong indentation type(spaces instead of tabs)
[error] 533-535: Wrong indentation type(spaces instead of tabs)
[error] 537-545: Wrong indentation type(spaces instead of tabs)
[error] 903-906: Wrong indentation type(spaces instead of tabs)
[error] 917-921: Wrong indentation type(spaces instead of tabs)
🔇 Additional comments (8)
tools/modules/software/module_images.sh (8)
1-14: Module metadata is well-structured and compliant with project conventions.
16-50: Proper setup with robust dependency handling.
855-966: Command routing and overall structure is solid with proper error propagation.The main case statement (lines 855–966) correctly routes commands and error codes through the helper functions. Each command path either succeeds or returns 1 with appropriate user messaging.
26-26: Security posture is acceptable with HTTPS JSON source and jq argument injection prevention.JSON is downloaded via HTTPS and image URLs are passed through jq
--argparameters, preventing injection attacks. Device validation before dd operation is sound. No additional security hardening required for this context.Also applies to: 85-85, 696-696, 754-754
1-967: Compliance and PII handling is appropriate; no sensitive data exposed in logs.The module does not log passwords, API tokens, or user PII. Board slugs, device paths, and image URLs are public data appropriate for diagnostic logging.
475-497: Edge case handling is comprehensive with proper recovery flows.The module handles key edge cases: empty dialogs (console fallback), missing tools (pv optional), cached files (reuse prompt), stale cache (refresh), no matching images (retry filters), and device exclusions (safety checks). Error paths provide user guidance and recovery options.
Also applies to: 739-750, 72-89
562-671: PKNAME resolution is reliable and the code is correct.The
lsblk -no PKNAMEapproach correctly resolves partitions to parent disks across all naming schemes: SCSI/SATA partitions (sda1→sda), MMC partitions (mmcblk0p1→mmcblk0), and NVMe partitions (nvme0n1p1→nvme0n1). This is standard, documented behavior in util-linux and the Linux kernel.The code's fallback logic (lines 581–586, 589–594, 597–602) provides additional safety: if PKNAME returns empty or fails, it uses the original device. The subsequent exclusion checks (lines 632–640) then correctly identify and skip root/boot/EFI disks regardless of which device reference was obtained.
No issues found. The implementation is correct and handles edge cases defensively.
841-841: dd status options require GNU coreutils ≥8.24 (released 2015), not supported on BSD dd or significantly older systems.Lines 830 and 841 use
status=noneandstatus=progress, which are GNU extensions unavailable on BSD systems or coreutils versions predating 8.24. For Armbian's target platforms (modern Armbian images and contemporary systemd-based distributions), this is not a practical concern. However, if broader compatibility is intended, consider adding runtime detection or fallback options.
igorpecovnik
force-pushed
the
write_images
branch
8 times, most recently
from
19deffd to
cd101d1
Compare
igorpecovnik
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tools/modules/software/module_images.sh (1)
787-811: BLOCKER: Fix unsafe FIFO creation pattern (race condition vulnerability).Lines 790–791, 869–870, and 920–922 use
mktemp -ufollowed bymkfifo, creating a race window where the filename could be reused by another process. Previous review (commit flagged as fixing this) appears incomplete.Apply this fix to all three FIFO creation sites:
- gauge_fifo=$(mktemp -u) - mkfifo "$gauge_fifo" + gauge_fifo_dir=$(mktemp -dt gauge.XXXXXX) || return 1 + gauge_fifo="${gauge_fifo_dir}/fifo" + mkfifo "$gauge_fifo" || { rm -rf "$gauge_fifo_dir"; return 1; } + trap "rm -rf '$gauge_fifo_dir'" RETURNApply the same fix at lines 869–870 and 920–922, adjusting variable names (
v_fifo_dir, etc.) to avoid conflicts.
🧹 Nitpick comments (3)
tools/modules/software/module_images.sh (3)
1-14: Module structure follows conventions; optional: consider hybrid module pattern.The file correctly uses
module_options+=()to append metadata (not re-declare). Per the Armbian configng codebase conventions, module files should not include shebangs—this file correctly omits one. However, as noted in learnings from PR 668, newer modules are adopting an "independent drop-in module" pattern withBASH_SOURCEchecks for hybrid sourcing/execution. This is optional but worth considering for future resilience.Based on learnings from PR 668, you could implement the hybrid module pattern using
BASH_SOURCEchecks to allow the module to be both sourced and executed independently (similar toinit_args.sh), but this is optional for now.
163-271: Multi-stage filtering workflow is well-designed and comprehensive.Filtering supports all objectives: preinstalled app (with STABLE→archive special case), kernel branch, and image variant. File format exclusion properly rejects signatures/torrents. Presentation with fixed-width columns, promoted-image markers (★), and sorting by promoted-then-version provides good UX. The "EMPTY" barebone marker works but is slightly unconventional.
Consider using a separate explicit flag for barebone filtering instead of the "EMPTY" marker (e.g.,
$has_bareboneboolean and separate filter logic), which would improve code clarity.Also applies to: 274-348, 351-424, 427-586
26-28: Integration with armbian-config ecosystem is clean and follows established patterns.Correctly leverages pkg_install, $SOFTWARE_FOLDER, $DIALOG, and module_options infrastructure. URL is reasonable default; consider making configurable in future for flexibility with mirrors.
Optional: Make ALL_IMAGES_JSON_URL configurable via environment or config file for flexibility with regional mirrors or custom distributions.
Also applies to: 41-41, 44-44, 79-79
📜 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 (2)
tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:08:12.801Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (1)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
🔇 Additional comments (13)
tools/json/config.system.json (1)
93-114: Configuration structure looks good; verify module_images status command.The STO001 description update and new FLASH1 entry follow established patterns. The FLASH1 entry's condition delegates to
module_images status, which must return a clean exit code; please verify that the module implements this correctly.tools/modules/software/module_images.sh (12)
589-701: Excellent device safety design with multi-layer protection.Device selection correctly identifies and excludes system disks (/, /boot, /boot/efi) by resolving to parent devices via PKNAME. The logic properly handles edge cases (fallback when PKNAME is empty) and complements this with explicit user confirmation requiring "YES" typing. Verification phase provides additional data integrity assurance.
Also applies to: 704-718
171-171: Board slug normalization provides good user experience.The
norm()function (lowercase + non-alphanumeric→hyphen) consistently applied across filtering allows users to enter board slugs in various formats (UEFI_X86, uefi-x86, uefi x86, etc.). This aligns with the PR objective for robust and user-friendly filtering.Also applies to: 280-280, 357-357, 437-437, 552-552
722-836: Download and decompression logic is robust with proper error handling.Correctly derives image URL, handles existing files with reuse option, validates tool availability, and provides two-tier progress UI (gauge-based with pv for accuracy, fallback to simple infobox). Partial files cleaned on error. One minor oddity: line 809 closes FD 3 which is never explicitly opened in this context (harmless but defensive).
839-986: Flash and verification logic is comprehensive with proper data integrity checks.Multiple UI tiers (gauge+pv, pv-console, dd-only) gracefully handle tool availability. Verification phase with cmp ensures written data matches source. Block size (4MB) and fsync options are appropriate. One note:
dd conv=noerrorcontinues on write errors—intended for robustness, but typically fails on errors; confirm this behavior aligns with risk tolerance.
1030-1129: Status command provides good diagnostic output with appropriate error conditions.Reports index health, image counts (total and promoted), device availability, and tool support. Correctly returns error when no flashable devices available, making it safe for use in config conditions. Device counting properly excludes system disks using same logic as selection flow.
991-1158: Error handling follows appropriate patterns with command chaining and fallbacks.Install command chains helpers with
|| return 1to stop on first failure—correct approach. Remove/purge commands safely degrade gracefully. Status command returns error only when preconditions fail (appropriate for condition validation). Help command provides fallback for unknown commands.
44-49: Dependency handling is appropriate with graceful degradation for optional tools.Required tools (curl, jq) are automatically installed. Optional tools (pv, cmp) are used if available, with console fallbacks otherwise. Standard Linux tools (lsblk, findmnt, xz-utils) are assumed present. Consider explicitly installing xz-utils if missing (line 777–780) for better error messages, but current approach is reasonable for fundamental packages.
Also applies to: 777-780
32-38: Variable scoping pattern is appropriate for inter-function communication.Global-scoped variables (BOARD_SLUG, PREAPP_FILTER, etc.) are function-scoped within module_images() and properly sequenced in the install flow. No collision risk given careful naming conventions.
Also applies to: 992-1000
54-60: DIALOG/console fallback pattern is consistent and well-implemented.Correctly assumes $DIALOG variable is pre-validated by calling context (per project conventions from
interface_menu.sh). All UI branches have reasonable console equivalents for non-interactive contexts.Also applies to: 79-83, 124-137, 235-249
1131-1154: Help text is comprehensive and user-friendly.Clearly documents all commands, usage patterns, filtering options, and presentation details. Explains board slug defaults, file format exclusions, and case-insensitive matching. Well-organized with tab-based formatting for readability.
1-1159: Overall code quality is high with excellent safety design and comprehensive feature set.The module implements robust image download/flashing workflow with multi-stage filtering, device protection, data verification, and graceful fallbacks for missing tools. Integration with armbian-config conventions is clean. See critical issues above (FIFO race condition, jq indentation verification) that must be addressed before merge.
99-110: The indentation in jq queries uses tabs correctly throughout the file. All specified line ranges (99-110, 141-151, 170-184, 279-304, 356-382, 436-495, 551-575, 1034-1058) consistently use tab characters (^Iin cat -A notation) for jq query content, not spaces. No changes required.Likely an incorrect or invalid review comment.
igorpecovnik
force-pushed
the
write_images
branch
from
cd101d1 to
ca939d7
Compare
igorpecovnik
added
Needs review
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/modules/software/module_images.sh (1)
839-888: Minor: Fix FIFO race condition in download_image_file (and duplicate pattern in flash function).Lines 842–843 create a FIFO using the unsafe
mktemp -u+mkfifopattern. Between the name generation and file creation, another process could claim that name. This pattern repeats at lines 921–922 and 973–974.Use
mktemp -dto create a unique temporary directory, then create the FIFO inside:- gauge_fifo=$(mktemp -u) - mkfifo "$gauge_fifo" + gauge_fifo_dir=$(mktemp -d) || return 1 + gauge_fifo="${gauge_fifo_dir}/gauge" + mkfifo "$gauge_fifo" || { rm -rf "$gauge_fifo_dir"; return 1; }Update cleanup to remove both the FIFO and the directory:
- rm -f "$gauge_fifo" + rm -rf "$gauge_fifo_dir"Apply the same fix to lines 921–922 (flash phase) and 973–974 (verification phase).
📜 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 (2)
tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/json/config.system.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:08:12.801Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (1)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
🔇 Additional comments (2)
tools/modules/software/module_images.sh (2)
1-50: Good module structure and initialization. Module metadata is well-formed, dependencies (curl, jq) are properly checked and installed, and base directory creation is safe.
1083-1177: Good status reporting and device discovery. The status command correctly counts flashable block devices while excluding system disks (/, /boot, /boot/efi), mirroring the safety logic inselect_block_device. The block device filtering and parent-disk resolution (PKNAME) is robust.
igorpecovnik
force-pushed
the
write_images
branch
from
ca939d7 to
d19168b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
tools/modules/software/module_images.sh (2)
99-110: 🔴 BLOCKER: Convert jq indentation from spaces to tabs throughout the file.This issue was flagged in the previous review: all jq single-quoted blocks must use tabs for indentation, not spaces. GitHub Actions style checks will fail otherwise. Per project conventions, shell code uses tabs for indentation.
Affected regions (non-exhaustive list):
- Lines 99–110, 141–151 (choose_board_slug_from_index)
- Lines 220–234 (select_preapp_for_board)
- Lines 329–354 (select_kernel_branch_for_board)
- Lines 406–432 (select_image_variant_for_board)
- Lines 486–546, 603–627 (select_image_for_board)
- Lines 1087–1111 (status command)
Apply a global find-and-replace within jq single-quoted blocks: convert leading spaces to tabs, then run the project's style check to verify.
Also applies to: 141-151, 220-234, 329-354, 406-432, 486-546, 603-627, 1087-1111
842-843: 🟡 Fix FIFO race condition in three locations.Creating a FIFO via
mktemp -u(name only) followed bymkfifocreates a small window where another process could reuse the name. Usemktemp -dto create a unique temporary directory first, then create the FIFO inside it.Apply the safe pattern to all three locations (download progress gauge, flash gauge, and verification gauge):
- gauge_fifo=$(mktemp -u) - mkfifo "$gauge_fifo" + gauge_fifo_dir=$(mktemp -d) || { echo "Failed to create temp dir"; return 1; } + gauge_fifo="${gauge_fifo_dir}/gauge_fifo" + mkfifo "$gauge_fifo" || { rm -rf "$gauge_fifo_dir"; return 1; }Then update cleanup to remove both the FIFO and the directory:
- rm -f "$gauge_fifo" + rm -rf "$gauge_fifo_dir"Repeat for
v_fifo(verification FIFO) at lines 973–974.Also applies to: 921-922, 973-974
🧹 Nitpick comments (6)
tools/modules/software/module_images.sh (6)
43-49: Consider validating pkg_install success explicitly.The dependency installation block attempts to install curl and jq but doesn't verify success before proceeding. If
pkg_installfails silently, subsequent operations would fail with unclear error messages.Suggested improvement:
for dep in "${DEPS[@]}"; do if ! command -v "$dep" >/dev/null 2>&1; then - pkg_install "$dep" + if ! pkg_install "$dep"; then + echo "Failed to install required dependency: $dep" + return 1 + fi fi done
141-151: Validate numeric indices before passing to jq --argjson.Lines 141–151 and 603–627 use
--argjson i "$selected_idx", but$selected_idxcomes from user input or dialog selection. If the value is non-numeric or malformed, jq will fail. While the dialog menu constrains input to valid indices in practice, explicit validation would improve robustness.Suggested approach:
[[ -z "$selected_idx" ]] && return 1 + if ! [[ "$selected_idx" =~ ^[0-9]+$ ]]; then + echo "Invalid selection: $selected_idx" + return 1 + fiApply to both locations (lines 139–153 and 600–632).
Also applies to: 603-627
35-39: Optional: Isolate global filter state to prevent cross-invocation leakage.The module declares global filter variables (PREAPP_FILTER, DOWNLOAD_REPO_FILTER, KERNEL_FILTER, VARIANT_FILTER) at lines 35–38, and later IMAGE_JSON, TARGET_DEVICE, LOCAL_IMAGE_PATH without explicit cleanup at function exit. While the linear install flow (lines 1042–1054) works correctly in a single invocation, if the module is called multiple times in the same shell session, stale state could theoretically leak.
Consider resetting these globals at function entry:
module_images() { local title="images" # Reset state from potential prior calls PREAPP_FILTER="" DOWNLOAD_REPO_FILTER="" KERNEL_FILTER="" VARIANT_FILTER="" IMAGE_JSON="" TARGET_DEVICE="" LOCAL_IMAGE_PATH="" # ... rest of function }This is a defensive best practice but not critical for the current usage patterns.
Also applies to: 1041-1054
221-221: Optional: Consolidate jq norm() function definition.The board slug normalization function
norm()is defined identically in five separate jq blocks (lines 221, 330, 407, 488, 604). This duplicates logic and makes maintenance harder. Consider defining it once as a jq module or variable at module start.This is a nice-to-have improvement for code maintainability but not critical for functionality.
Also applies to: 330-330, 407-407, 488-488, 604-604
220-234: Optional: Capture jq stderr for better diagnostics.Multiple jq queries redirect stderr to
/dev/null, which masks JSON parse errors. While fallback error messages ("No boards found") are user-friendly, explicit jq error capture would improve debugging for corrupted JSON.Suggested improvement:
if ! temp_list=$(jq -r '...' "$IMAGES_JSON_PATH" 2>&1); then echo "Failed to parse images index: $temp_list" >&2 return 1 fiThis is a nice-to-have for preview-stage features and not critical if the JSON source is trusted.
Also applies to: 1087-1111
85-85: Optional: Add curl timeout to prevent hanging on unreachable networks.The curl commands use -fsSL/-fSL but no explicit timeout. If the network is slow or the server unresponsive, curl could hang indefinitely. Adding
--connect-timeout(connect phase) and--max-time(total operation) would improve responsiveness.Suggested improvement:
curl -fsSL --connect-timeout 10 --max-time 300 "$ALL_IMAGES_JSON_URL" -o "$IMAGES_JSON_PATH"Adjust timeouts based on expected image sizes and network conditions. This is a nice-to-have for robustness.
Also applies to: 851-851, 878-878
📜 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)
tools/include/markdown/FLASH1-header.md(1 hunks)tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tools/include/markdown/FLASH1-header.md
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🔇 Additional comments (8)
tools/json/config.system.json (2)
94-95: ✓ STO001 description update looks good.Updated description aligns with the new module's broader scope.
104-115: ✓ FLASH1 entry properly structured.The new FLASH1 entry follows existing conventions, references the module_images module correctly, and uses a sensible condition check. Integration with the Storage submenu is appropriate.
tools/modules/software/module_images.sh (6)
4-14: ✓ Module options declaration is appropriate.The use of
module_options+=(...)correctly appends to the parent-declared array per project conventions (not re-declaring it). This pattern is consistent with the sourced-module architecture.
646-718: ✓ System disk exclusion logic is sound and defensive.The logic correctly identifies and excludes system disks (/, /boot, /boot/efi) by resolving them to parent disks via lsblk PKNAME. The fallback handling for missing PKNAME is appropriate. This design prevents accidental data loss from flashing the running system.
Consistent application in both select_block_device() and the status command is good practice.
Also applies to: 1120-1177
103-103: ✓ Image format filtering is correct and comprehensive.The regex
^img(\\.(xz|gz|zst|bz2|lz4))?$correctly identifies flashable image formats and excludes non-flashable ones (cloud formats, signatures, torrents). Support for multiple compression codecs is good. The filtering is applied consistently throughout the file.Help text properly documents the filtering behavior.
Also applies to: 225-225, 526-526, 1196-1197
905-1038: ✓ Image flashing and verification logic is thorough.The flash_image_to_device function includes comprehensive pre-flight validation, two flash paths (with optional pv gauge), verification via cmp, and clear user feedback. The cmp verification (limited to image size via -n flag) ensures write success without relying solely on dd exit codes.
Relying on OS-level protection against writes to mounted partitions is appropriate.
54-60: ✓ Interactive menu UX is well-structured with good fallback support.The module provides consistent dialog-first UI with console fallbacks, ensuring usability in both graphical and headless environments. Menu options are clearly labeled, and validation is present. The approach is accessible and user-friendly.
Also applies to: 124-137, 285-299, 371-384, 447-460, 585-598
1041-1211: ✓ Command dispatch is clean, well-separated, and handles all workflows.The module properly implements install, remove, purge, status, and help commands with consistent error handling and both dialog and console UI paths. The status command's separate device count is particularly valuable for user transparency.
igorpecovnik
force-pushed
the
write_images
branch
from
d19168b to
a762343
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
tools/modules/software/module_images.sh (4)
220-234: jq filter definitions (norm, preapp_filter, repo_filter) are logically correct.Once the indentation is fixed per the earlier critical comment, the filters correctly:
- Normalize board slugs for case/separator–insensitive matching
- Distinguish barebone (empty preinstalled_application) from branded images
- Identify stable images via
download_repository == "archive"- Exclude non-flashable file extensions (.asc, .torrent, .sha*)
The logic is sound and consistent across all helper functions.
Also applies to: 329-354, 406-432, 486-546, 603-627
324-398: Kernel branch and image variant selection implement proper filter composition.Each helper correctly accumulates prior filters (board, preapp, repo) and applies them to subsequent selections. The ALL option at menu start is good UX. Both functions gracefully handle empty result sets (no kernel branches or variants available for current filters).
Also applies to: 401-474
477-638: Image display and formatting logic is well-designed for terminal usability.The jq block correctly formats a multi-column menu with padding for fixed-width alignment, handles missing fields gracefully, and highlights promoted images. Version normalization for trunk builds (DEV.N format) is helpful. Sorting by promoted status then version ensures best options surface first.
1083-1182: Status command correctly reports index state, device availability, and flashable devices.Image and promoted-image counts correctly apply board-slug normalization. Block-device enumeration (lines 1120–1177) mirrors select_block_device logic safely. The decision to return 1 when no flashable devices are found ensures the condition in config.system.json works correctly. Helper-tool detection (pv) is informative.
📜 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)
tools/include/markdown/FLASH1-header.md(1 hunks)tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/include/markdown/FLASH1-header.md
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (1)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
🔇 Additional comments (8)
tools/json/config.system.json (1)
94-115: Configuration entries well-formed and consistent.The STO001 refinement and new FLASH1 entry are properly integrated with the Storage submenu. The condition field correctly defers to the module's status check, and all required metadata fields are present.
tools/modules/software/module_images.sh (7)
641-753: Block device filtering and safety checks are well-implemented.The logic correctly identifies and excludes disks backing the system (/, /boot, /boot/efi), resolves to parent disks where applicable, and validates device sizes. The menu presentation clearly warns users and requires explicit device selection.
905-912: Flash and verification logic is sound with proper error handling and data integrity checks.The implementation correctly validates image size, uses sync calls for safety, employs dd with appropriate block size and fsync, and performs byte-by-byte verification via cmp. The three-state result reporting (success/failure/skipped) is clear. Note: the FIFO race condition at lines 921–922 should be fixed per earlier comment.
Also applies to: 919-957, 966-1006, 1013-1035
52-91: Helper functions ensure_board_slug and refresh_images_json are well-structured.Input fallback logic handles both interactive (DIALOG) and console modes gracefully. Cache logic with 1-day TTL and proper error handling for download failures is sound. Variable usage is safe from injection.
159-321: Board confirmation and preinstalled-application filtering logic is clear and user-friendly.The multi-choice menu correctly separates use-current from choose-other flows. Preapp detection handles barebone images, the STABLE filter maps to
download_repository == "archive", and hardcoded app descriptions are reasonable defaults. Dialog defaults to STABLE, which is sensible.
1055-1082: Remove and purge commands have appropriate safeguards.The remove command correctly uses
find -maxdepth 1to stay local to IMAGES_BASE and preserves all-images.json (avoiding unnecessary re-downloads). Both commands require confirmation via DIALOG or console prompt before destructive operations.
1183-1207: Help documentation is complete and accurate.Command descriptions, filter options, and column layout match the implementation. The explanation of STABLE (archive repo) and case-insensitive board matching clarifies non-obvious behavior.
1041-1054: Install flow orchestration properly chains helpers with error propagation.Each step returns 1 on failure, causing the workflow to abort immediately. The sequence is logical: detect board → fetch/refresh index → confirm board → multi-stage filters → select image → select device → final confirmation → download → flash. Error handling is consistent.
igorpecovnik
force-pushed
the
write_images
branch
from
a762343 to
f60403e
Compare
github-actions
bot
added
GitHub Actions
igorpecovnik
force-pushed
the
write_images
branch
from
c74e7cf to
5a2652a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tools/modules/software/module_images.sh (1)
43-49: Move xz to upfront dependency checks.The script requires xz for decompression (checked at line 819) but only validates it at runtime, after the user has navigated through board/image selection. Validate xz upfront to fail fast.
Apply this diff:
# Helper: ensure dependencies - local -a DEPS=("curl" "jq") + local -a DEPS=("curl" "jq" "xz") for dep in "${DEPS[@]}"; do if ! command -v "$dep" >/dev/null 2>&1; then pkg_install "$dep" fi done
🧹 Nitpick comments (1)
tools/modules/software/module_images.sh (1)
640-758: Consider extracting block device discovery into a helper function.The logic to find system disks (/, /boot, /boot/efi) and list flashable block devices is duplicated:
- Lines 646–726 in
select_block_device()- Lines 1154–1199 in
statuscommand handlerBoth implement the same algorithm: resolve mount points to parent disks, list candidates, filter out system disks. Extracting a shared helper (
discover_flashable_block_devices()) would reduce duplication and ensure consistency if logic is updated.This is a good-to-have refactor for maintainability; not a blocker.
Also applies to: 1151-1208
📜 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 (1)
tools/modules/software/module_images.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba.sh:1-1
Timestamp: 2025-04-12T18:00:39.337Z
Learning: In the armbian/configng project, development library files (.sh) don't require shebang directives as they're meant to be merged into production files that already have shebangs. These module files are intended to be sourced rather than executed directly.
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Learnt from: dimitry-ishenko
Repo: armbian/configng PR: 496
File: tools/modules/initialize/checkpoint.sh:1-2
Timestamp: 2025-03-26T01:54:52.213Z
Learning: Module files in the armbian/configng project like `tools/modules/initialize/checkpoint.sh` don't require a shebang line as they are meant to be sourced by other scripts rather than executed directly.
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:08:12.801Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (1)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
igorpecovnik
force-pushed
the
write_images
branch
5 times, most recently
from
16e9418 to
b45ad50
Compare
igorpecovnik
force-pushed
the
write_images
branch
5 times, most recently
from
46d311c to
36ab00b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/modules/software/module_images.sh (1)
1-49: Add xz to upfront dependency validation.xz must be included in the initial DEPS array (line 44) to validate the dependency before the user progresses through image selection and device configuration. Currently, the xz check occurs at download time (line 819), creating a poor UX when the dependency is missing after the user has already invested time in setup.
# Helper: ensure dependencies - local -a DEPS=("curl" "jq") + local -a DEPS=("curl" "jq" "xz") for dep in "${DEPS[@]}"; do if ! command -v "$dep" >/dev/null 2>&1; then pkg_install "$dep" fi doneRemove the redundant xz check at lines 819–822.
📜 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 ignored due to path filters (1)
tools/include/images/FLASH1.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.github/workflows/debian.yml(1 hunks)tools/include/markdown/FLASH1-header.md(1 hunks)tools/include/markdown/STO001-header.md(1 hunks)tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/debian.yml
- tools/include/markdown/STO001-header.md
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba.sh:1-1
Timestamp: 2025-04-12T18:00:39.337Z
Learning: In the armbian/configng project, development library files (.sh) don't require shebang directives as they're meant to be merged into production files that already have shebangs. These module files are intended to be sourced rather than executed directly.
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:08:12.801Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-12-05T15:06:19.923Z
Learnt from: igorpecovnik
Repo: armbian/configng PR: 705
File: tools/modules/software/module_images.sh:881-902
Timestamp: 2025-12-05T15:06:19.923Z
Learning: In the armbian/configng project, root privilege checking is handled centrally in bin/armbian-config at the entry point (`[[ $EUID != 0 ]] && exec sudo "$0" "$@"`), which re-executes the script with sudo if needed. All modules inherit root privileges from the main script, so module-level privilege checks are redundant and should not be added.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T18:00:39.337Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba.sh:1-1
Timestamp: 2025-04-12T18:00:39.337Z
Learning: In the armbian/configng project, development library files (.sh) don't require shebang directives as they're meant to be merged into production files that already have shebangs. These module files are intended to be sourced rather than executed directly.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase includes a set_interface() function in interface_menu.sh that validates and sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). This function needs to be called before interface_checklist() or other functions that depend on the $DIALOG variable.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-08-29T06:07:23.830Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-19T07:44:39.759Z
Learnt from: Tearran
Repo: armbian/configng PR: 534
File: tools/modules/software/module_apt_browsers.sh:22-22
Timestamp: 2025-04-19T07:44:39.759Z
Learning: In the Armbian ConfigNG browser module, packages like "google-chrome" that aren't available in standard repositories will automatically be filtered out from the checklist display. Adding external APT sources is beyond the scope of the browser module, which focuses only on package installation/removal of already-available packages.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-03-26T01:54:52.213Z
Learnt from: dimitry-ishenko
Repo: armbian/configng PR: 496
File: tools/modules/initialize/checkpoint.sh:1-2
Timestamp: 2025-03-26T01:54:52.213Z
Learning: Module files in the armbian/configng project like `tools/modules/initialize/checkpoint.sh` don't require a shebang line as they are meant to be sourced by other scripts rather than executed directly.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:07:19.898Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:63-69
Timestamp: 2025-04-12T20:07:19.898Z
Learning: In the Armbian ConfigNG project, `$(command)` subshell syntax is deliberately used to isolate command exit status from the parent shell, particularly for commands like `srv_disable` that might exit with error codes. This pattern allows scripts to continue execution even when these commands fail with non-zero exit codes, which is especially important when scripts use `set -e`.
Applied to files:
tools/modules/software/module_images.sh
🧬 Code graph analysis (2)
tools/modules/software/module_images.sh (1)
tools/modules/functions/package.sh (1)
pkg_install(41-44)
tools/json/config.system.json (3)
tools/modules/system/module_armbian_firmware.sh (1)
module_armbian_firmware(9-357)tools/modules/system/module_overlayfs.sh (1)
module_overlayfs(16-62)tools/modules/system/module_armbian_upgrades.sh (1)
module_armbian_upgrades(14-163)
🔇 Additional comments (9)
tools/json/config.system.json (2)
93-103: ✅ STO001 clarification is well-scoped.The description clarification ("Copy the running Armbian system to another device") and addition of the "short" field distinguish this from FLASH1 and improve menu readability.
104-115: ✅ FLASH1 entry is properly structured.Configuration entry aligns with the module_images module: correct module reference, install command, logical condition check, and "Preview" status appropriate for new feature rollout.
tools/include/markdown/FLASH1-header.md (1)
1-7: ✅ Documentation clearly articulates the feature's purpose.The bullet-point breakdown of use cases (internal storage, bootable media, recovery, OS/kernel variants, development acceleration) is user-centric and aligns with the module's capabilities.
tools/modules/software/module_images.sh (6)
1156-1213: ✅ Status command device detection is robust.The blockdev count logic properly replicates system-disk filtering from select_block_device, ensuring status accurately reflects available targets. Device exclusion (mmcblk pseudo-devices, system disks) is thorough.
1077-1090: ✅ Install workflow orchestration is logically sound.The sequential chain (ensure board → refresh index → confirm board → select filters → select image → select device → confirm → download → flash) provides a well-structured user journey with multiple exit/adjustment opportunities.
927-950: ✅ Dual-mode flash with fallback ensures broad compatibility.The gauge variant (pv + dialog) and console fallback (with or without pv) provide good UX across environment constraints. dd options (bs=4M, fsync, noerror) are appropriate for reliable block-device writes.
650-726: ✅ System disk protection is comprehensive and multi-layered.Resolution of parent disks via PKNAME (lines 655–684) and multi-layer exclusion (/, /boot, /boot/efi) prevents accidental system wipe. Pseudo-device filtering (mmcblk boot/rpmb) is thorough. This is a critical safety feature well-implemented.
486-546: ✅ Image selection display and sorting is well-designed.Custom jq formatting (version display, size formatting, padding) produces a clean, readable menu. Promoted images sorted first, version display showing "DEV.XX" for trunk builds—these UI touches improve user experience without compromising functionality.
833-851: ✅ Download FIFO creation uses safe, race-condition-free pattern.Temp directory creation (mktemp -d), FIFO placement inside, and cleanup of both artifact and temp dir addresses prior TOCTOU concerns. Pattern is consistent with flash/verify sections.
igorpecovnik
force-pushed
the
write_images
branch
from
36ab00b to
e57e94d
Compare
igorpecovnik
force-pushed
the
write_images
branch
from
e57e94d to
47f2b39
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/modules/software/module_images.sh (1)
44-49: Add xz to the initial dependency check.Prior review feedback (committed as 4d7014e) indicated xz should be added to the DEPS array for upfront validation. Currently, lines 44–49 only check for curl and jz. If xz is missing, the user won't discover this until after they've selected images and devices, wasting time.
# Helper: ensure dependencies - local -a DEPS=("curl" "jq") + local -a DEPS=("curl" "jq" "xz") for dep in "${DEPS[@]}"; do if ! command -v "$dep" >/dev/null 2>&1; then pkg_install "$dep" fi done
🧹 Nitpick comments (1)
tools/modules/software/module_images.sh (1)
4-5: Minor: Fill in empty author field for consistency.Line 5 has an empty author field, but other entries in config.system.json (FLASH1, FLASH2 at lines 113, 126 in the second file) identify the author as "@igorpecovnik". For consistency and attribution, consider filling this field.
module_options+=( - ["module_images,author"]="" + ["module_images,author"]="@igorpecovnik"
📜 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 ignored due to path filters (1)
tools/include/images/FLASH1.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.github/workflows/debian.yml(1 hunks)tools/include/markdown/FLASH1-header.md(1 hunks)tools/include/markdown/STO001-header.md(1 hunks)tools/json/config.system.json(2 hunks)tools/modules/software/module_images.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/debian.yml
- tools/include/markdown/FLASH1-header.md
- tools/include/markdown/STO001-header.md
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
📚 Learning: 2025-08-29T05:39:16.674Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:167-180
Timestamp: 2025-08-29T05:39:16.674Z
Learning: Tearran is introducing an "independent drop in module concept" in the armbian/configng project where modules can function both as sourceable components and as standalone executables using BASH_SOURCE checks. The init_args.sh module is the first to implement this hybrid pattern, allowing both traditional sourcing and direct execution with built-in testing capabilities.
Applied to files:
tools/modules/software/module_images.shtools/json/config.system.json
📚 Learning: 2025-04-17T17:36:45.817Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/software/module_aptwizard.sh:1-248
Timestamp: 2025-04-17T17:36:45.817Z
Learning: In the armbian/configng project, module files in the tools/modules directory typically don't include shebang lines since they're meant to be sourced by other scripts rather than executed directly. The project convention is to have 98% of module files without shebangs, and this pattern should be maintained for consistency.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:08:12.801Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:1-1
Timestamp: 2025-04-12T20:08:12.801Z
Learning: In the armbian/configng repository, module library files (e.g., `tools/modules/software/module_samba_lib.sh`) that are intended to be sourced by other scripts don't require their own shebang directives as they inherit the shell environment from the sourcing script.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-13T02:25:58.772Z
Learnt from: Tearran
Repo: armbian/configng PR: 264
File: tools/modules/software/module_atuin.sh:2-14
Timestamp: 2025-04-13T02:25:58.772Z
Learning: In Armbian's configng modules system, the `module_options` associative array is declared in a parent library script before individual module files are sourced, so module files should not re-declare it (which is why the declaration is often commented out in module files).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase already handles dialog tool validation in interface_menu.sh, which sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). Functions using $DIALOG in other files assume this validation has already occurred and don't need to repeat these checks.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-12-05T15:06:19.923Z
Learnt from: igorpecovnik
Repo: armbian/configng PR: 705
File: tools/modules/software/module_images.sh:881-902
Timestamp: 2025-12-05T15:06:19.923Z
Learning: In the armbian/configng project, root privilege checking is handled centrally in bin/armbian-config at the entry point (`[[ $EUID != 0 ]] && exec sudo "$0" "$@"`), which re-executes the script with sudo if needed. All modules inherit root privileges from the main script, so module-level privilege checks are redundant and should not be added.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T18:00:39.337Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba.sh:1-1
Timestamp: 2025-04-12T18:00:39.337Z
Learning: In the armbian/configng project, development library files (.sh) don't require shebang directives as they're meant to be merged into production files that already have shebangs. These module files are intended to be sourced rather than executed directly.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-15T22:02:57.986Z
Learnt from: Tearran
Repo: armbian/configng PR: 530
File: tools/modules/functions/interface_checklist.sh:28-52
Timestamp: 2025-04-15T22:02:57.986Z
Learning: The Armbian configng codebase includes a set_interface() function in interface_menu.sh that validates and sets the $DIALOG variable based on available tools (whiptail, dialog, or falling back to read). This function needs to be called before interface_checklist() or other functions that depend on the $DIALOG variable.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-08-29T06:07:23.830Z
Learnt from: Tearran
Repo: armbian/configng PR: 668
File: tools/modules/initialize/init_args.sh:0-0
Timestamp: 2025-08-29T06:07:23.830Z
Learning: The set_runtime_variables.sh in armbian/configng establishes these key variables that must be matched exactly in independent modules: NETWORK_RENDERER (detected via systemctl/NetworkManager), DIALOG_CANCEL=1, DIALOG_ESC=255, CONFIG_FILE="/etc/armbian-config", ARMBIAN (from /etc/armbian-release), DISTRO_STATUS, SOFTWARE_FOLDER="/armbian", TRANSMISSION_WHITELIST (includes Docker subnet), ZFS_DKMS_VERSION, and ZFS_KERNEL_MAX (with fallback version logic).
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-19T07:44:39.759Z
Learnt from: Tearran
Repo: armbian/configng PR: 534
File: tools/modules/software/module_apt_browsers.sh:22-22
Timestamp: 2025-04-19T07:44:39.759Z
Learning: In the Armbian ConfigNG browser module, packages like "google-chrome" that aren't available in standard repositories will automatically be filtered out from the checklist display. Adding external APT sources is beyond the scope of the browser module, which focuses only on package installation/removal of already-available packages.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-03-26T01:54:52.213Z
Learnt from: dimitry-ishenko
Repo: armbian/configng PR: 496
File: tools/modules/initialize/checkpoint.sh:1-2
Timestamp: 2025-03-26T01:54:52.213Z
Learning: Module files in the armbian/configng project like `tools/modules/initialize/checkpoint.sh` don't require a shebang line as they are meant to be sourced by other scripts rather than executed directly.
Applied to files:
tools/modules/software/module_images.sh
📚 Learning: 2025-04-12T20:07:19.898Z
Learnt from: Tearran
Repo: armbian/configng PR: 529
File: tools/modules/software/module_samba_lib.sh:63-69
Timestamp: 2025-04-12T20:07:19.898Z
Learning: In the Armbian ConfigNG project, `$(command)` subshell syntax is deliberately used to isolate command exit status from the parent shell, particularly for commands like `srv_disable` that might exit with error codes. This pattern allows scripts to continue execution even when these commands fail with non-zero exit codes, which is especially important when scripts use `set -e`.
Applied to files:
tools/modules/software/module_images.sh
🔇 Additional comments (1)
tools/json/config.system.json (1)
104-127: ✅ Well-integrated config entries for module_images.FLASH1 and FLASH2 entries are properly designed:
- FLASH1 condition uses
module_images status >/dev/nullto check if the module is functional and devices are available- FLASH2 condition uses the internal
module_images cache-statuscommand to show remove option only when images exist- Both correctly invoke the corresponding module commands
- Status "Preview" is appropriate for a new feature
- Descriptions clearly distinguish this from STO001 (Install running system)
igorpecovnik
force-pushed
the
write_images
branch
from
47f2b39 to
334ae38
Compare
igorpecovnik
force-pushed
the
write_images
branch
from
334ae38 to
5f3ca0a
Compare
igorpecovnik
changed the title
igorpecovnik
added
Ready to merge
Description
Adding functionality to download and flash images directly to block device. We are reading our official JSON file that contains all necessary information.
Users can now easily browse Armbian images and flash fast to internal media.
Key Improvements
Image Selection
Safer Device Handling
/(root filesystem)/boot/boot/efiImproved Download Experience
pv+ whiptail gauge where possibleTesting Procedure
Checklist