Skip to content

Conversation

@tabrisnet
Copy link
Collaborator

@tabrisnet tabrisnet commented Nov 9, 2025

The goal here is that some boards may want to opt out entirely [at their own risk of course] of an extension or hook. Here I pick on the rockchip-rv1106 as an example.
this supplements but does not replace #8896.

Documentation summary for feature / change

likely needs to be documented here: https://docs.armbian.com/Developer-Guide_Extensions-Hooks/
But that needs some discussion.

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.

  • - edited linux-rockchip-rv1106-vendor.config to disable EXT4 entirely. ran rewrite-kernel-config. EXT4 remained disabled
  • took linux-filogic-current.config, flipped EXT4_FS=n, it changed it to m [in main it is currently y]
    • so it appears to preserve expected behaviour in the non-optout case

Checklist:

Please delete options that are not relevant.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features
    • Added the ability to disable specific extensions at the device configuration level, allowing for more granular control over enabled features per device.

@tabrisnet tabrisnet added the Needs Documentation New feature needs documentation entry label Nov 9, 2025
@tabrisnet tabrisnet requested review from a team and igorpecovnik as code owners November 9, 2025 01:10
@github-actions github-actions bot added size/small PR with less then 50 lines 11 Milestone: Fourth quarter release Needs review Seeking for review labels Nov 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Walkthrough

This PR introduces an extension opt-out mechanism that allows specific extensions to be disabled at the family or global configuration level. The rockchip-rv1106 family is configured to opt out of the armbian_kernel_config__enable_docker_support extension. Hooks matching entries in the extension_optout array are filtered out during extension processing.

Changes

Cohort / File(s) Summary
Extension opt-out filtering logic
lib/functions/general/extensions.sh
Declares global extension_optout array in initialize_extension_manager() and adds filtering loop in hook processing to skip extensions whose composed names match opted-out entries
Rockchip RV1106 family config
config/sources/families/rockchip-rv1106.conf
Initializes extension_optout array with armbian_kernel_config__enable_docker_support entry to disable docker support for this family

Sequence Diagram

sequenceDiagram
    participant Config as Rockchip RV1106<br/>Config
    participant InitEM as initialize_extension<br/>_manager()
    participant HookLoop as Hook Processing<br/>Loop
    
    Config->>InitEM: Declare extension_optout[]
    InitEM->>InitEM: Initialize empty array
    Note over InitEM: Array can be populated<br/>by config sources
    Config->>InitEM: extension_optout=(<br/>armbian_kernel_config<br/>__enable_docker_support)
    
    HookLoop->>HookLoop: For each hook_point
    HookLoop->>HookLoop: Derive hook_point<br/>__realname
    alt Hook in optout list
        HookLoop->>HookLoop: Skip (filter out)
    else Hook not opted out
        HookLoop->>HookLoop: Add to hook_point<br/>_functions[]
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Straightforward array initialization and filtering logic
  • Single-purpose mechanism with no complex state interactions
  • Minimal code footprint across two files
  • Focus areas: verify filtering logic correctly matches hook names and that initialization order is preserved

Possibly related PRs

Suggested reviewers

  • igorpecovnik
  • rpardini
  • chraac
  • teknoid

Poem

🐰 A hook here, a hook there,
But some we simply don't care!
Optout declares what we'll skip,
Docker support loses its grip,
RV1106 hops along light and fair! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately references the main change: adding an extension_optout mechanism for boards/families to opt out of extensions/hooks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added Hardware Hardware related like kernel, U-Boot, ... Framework Framework components labels Nov 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
lib/functions/general/extensions.sh (2)

282-286: Consider adding early exit after match.

Once a match is found, the loop continues checking remaining entries unnecessarily. Adding a break statement would improve efficiency.

Apply this diff:

 			for func in "${extension_optout[@]}"; do
 				if [[ "$func" == "${hook_point}__${hook_point_function}" ]]; then
 					opt_out=true
+					break
 				fi
 			done

288-290: Add debug logging for opted-out functions.

When a function is opted out, there's no visibility into this decision. Adding a debug log message would help users understand why certain extension functions aren't executing.

Apply this diff:

 			if $opt_out; then
+				display_alert "Extension opt-out" "Skipping ${hook_point}__${hook_point_function} (opted out via extension_optout)" "debug"
 				continue; #skip processing this function
 			fi
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0bd81c9 and c0600c3.

📒 Files selected for processing (2)
  • config/sources/families/rockchip-rv1106.conf (1 hunks)
  • lib/functions/general/extensions.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.
📚 Learning: 2025-05-21T06:04:48.711Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

Applied to files:

  • lib/functions/general/extensions.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.

Applied to files:

  • lib/functions/general/extensions.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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-06-25T03:40:52.109Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-06-25T03:42:09.086Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-26T12:56:29.185Z
Learnt from: rpardini
Repo: armbian/build PR: 8820
File: config/sources/families/include/meson64_common.inc:51-53
Timestamp: 2025-10-26T12:56:29.185Z
Learning: In the Armbian build framework, scripts/config correctly handles kernel configuration option names both with and without the CONFIG_ prefix, so opts_m+=("CONFIG_RTL8822CS") and opts_m+=("RTL8822CS") are both valid and will work correctly.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T11:37:35.089Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
🧬 Code graph analysis (1)
config/sources/families/rockchip-rv1106.conf (2)
lib/functions/compilation/armbian-kernel.sh (2)
  • armbian_kernel_config__enable_docker_support (357-429)
  • armbian_kernel_config_apply_opts_from_arrays (480-533)
lib/functions/compilation/kernel-config.sh (1)
  • call_extensions_kernel_config (88-121)
🪛 GitHub Actions: Shellcheck - PR #8897 ("framework extensions deep magic - add extension_optout for boards/families")
lib/functions/general/extensions.sh

[error] 280-280: ShellCheck SC2154: 'extension_optout' is referenced but not assigned.

🔇 Additional comments (1)
config/sources/families/rockchip-rv1106.conf (1)

34-34: LGTM! Appropriate opt-out for low-memory devices.

The declaration correctly opts out of Docker support kernel configuration for the RV1106 family, which is optimized for low-memory devices (64-256MB). This is a sensible decision given the resource constraints.

The function name format matches the expected pattern: hook_point__identifier.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/functions/general/extensions.sh (1)

241-260: ⚠️ HOOK_POINT_TOTAL_FUNCS counts non-opted-out functions incorrectly.

hook_point_functions_counter is computed at line 255 before the opt-out loop (lines 278-327), so it reflects the total count of functions, not accounting for opt-outs. This value is then exported as HOOK_POINT_TOTAL_FUNCS (line 255) and passed to extension functions as context.

If an extension hooks function is opted out, the remaining extension functions will receive an inflated HOOK_POINT_TOTAL_FUNCS value that doesn't match the actual number of functions being executed. This could mislead extension code that uses HOOK_ORDER and HOOK_POINT_TOTAL_FUNCS to determine progress (e.g., "if HOOK_ORDER == HOOK_POINT_TOTAL_FUNCS, I'm the last function").

Move the hook_point_functions_counter increment loop (lines 250-254) to after the opt-out loop, so it counts only the non-opted-out functions. Alternatively, decrement the counter for each skipped function.

-	hook_point_functions_counter=0
-	hook_points_counter=$((hook_points_counter + 1))
-
-	# determine the variables we'll pass to the hook function during execution.
-	# this helps the extension author create extensions that are portable between userpatches and official Armbian.
-	# shellcheck disable=SC2089
-	local common_function_vars="HOOK_POINT=\"${hook_point}\""
-
-	# loop over the functions for this hook_point (keep a total for the hook point and a grand running total)
-	declare hook_point_function
-	for hook_point_function in ${hook_point_functions}; do
-		hook_point_functions_counter=$((hook_point_functions_counter + 1))
-		hook_functions_counter=$((hook_functions_counter + 1))
-	done
-	common_function_vars="${common_function_vars} HOOK_POINT_TOTAL_FUNCS=\"${hook_point_functions_counter}\""
+	hook_points_counter=$((hook_points_counter + 1))
+
+	# determine the variables we'll pass to the hook function during execution.
+	# this helps the extension author create extensions that are portable between userpatches and official Armbian.
+	# shellcheck disable=SC2089
+	local common_function_vars="HOOK_POINT=\"${hook_point}\""
+	# HOOK_POINT_TOTAL_FUNCS will be set after filtering opt-outs below

Then, after the opt-out loop ends (around line 327), compute hook_point_functions_counter and update common_function_vars:

 	done
+	common_function_vars="${common_function_vars} HOOK_POINT_TOTAL_FUNCS=\"${hook_point_functions_loop_counter}\""
🧹 Nitpick comments (1)
lib/functions/general/extensions.sh (1)

278-290: Silent failures if opt-out entries don't match any function.

The current implementation doesn't warn if an extension_optout entry (e.g., armbian_kernel_config__enable_docker_support) doesn't match any function in a given hook point. If a user misspells a function name in the opt-out list, the mis-entry will silently do nothing, potentially leaving the user believing a function is opted out when it isn't.

Consider adding debug-level logging when an opt-out entry is successfully matched, and a warning when an entry is never matched against any function across all hook points.

+ # Track which opt-out entries were actually matched, for diagnostics
+ declare -A optout_matched_count
+ for opt_entry in "${extension_optout[@]}"; do
+     optout_matched_count["${opt_entry}"]=0
+ done

  for hook_point in ${all_hook_points}; do
      # ... existing code ...
      for hook_point_function in ${hook_point_functions}; do
          opt_out=false
          if [[ -v extension_optout && ${#extension_optout[@]} -ne 0 ]]; then
              for func in "${extension_optout[@]}"; do
                  if [[ "$func" == "${hook_point}__${hook_point_function}" ]]; then
                      opt_out=true
+                     optout_matched_count["${func}"]=$((optout_matched_count["${func}"] + 1))
+                     [[ "${SHOW_EXTENSIONS}" == "yes" ]] && display_alert "Extension opt-out" "Skipping ${hook_point}__${hook_point_function}" "debug"
                  fi
              done
          fi
          # ... rest of loop ...
      done
  done

+ # After all hooks processed, warn about unmatched opt-outs
+ for opt_entry in "${!optout_matched_count[@]}"; do
+     if [[ ${optout_matched_count["${opt_entry}"]} -eq 0 ]]; then
+         display_alert "Extension opt-out warning" "No matching function found for opt-out entry: ${opt_entry}" "wrn"
+     fi
+ done
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c0600c3 and e41a434.

📒 Files selected for processing (1)
  • lib/functions/general/extensions.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 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: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.
📚 Learning: 2025-05-21T06:04:48.711Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

Applied to files:

  • lib/functions/general/extensions.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.

Applied to files:

  • lib/functions/general/extensions.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:

  • lib/functions/general/extensions.sh
🔇 Additional comments (2)
lib/functions/general/extensions.sh (2)

279-290: Extension opt-out logic verified and correctly implemented.

The implementation properly addresses the unbound variable concern with the -v extension_optout check at line 280. The family config file (config/sources/families/rockchip-rv1106.conf) correctly declares the opt-out array with declare -ga extension_optout=( armbian_kernel_config__enable_docker_support ), using the proper hook_point__function_name naming convention. The skip mechanism will work as intended—when the extension manager processes armbian_kernel_config__enable_docker_support, it will match the pattern, set the flag, and skip registration via the continue statement.


17-27: Documentation for extension_optout is missing; code design is correct and safe.

Verification confirms the original review comment's analysis is accurate:

  • extension_optout is intentionally not in extension_manager_declare_globals()
  • The -v check at line 280 correctly guards against undefined variables
  • rockchip-rv1106.conf properly declares it with correct syntax: declare -ga extension_optout=( ... )

However, the suggested documentation is not present. Add a comment or update config/sources/families/README.md to explain that families/boards using extension hooks may declare extension_optout as a global array to exclude specific hook functions from execution (e.g., declare -g -a extension_optout=(...)).

@tabrisnet
Copy link
Collaborator Author

@coderabbitai that issue with HOOK_POINT_TOTAL_FUNCS looks a bit gnarly. I have an incoming commit which may fix it or may mess it up worse. do let me know.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the PR file changes to see the actual modifications
gh pr view 8897 --repo armbian/build --json files --jq '.files[] | {path: .path, additions: .additions, deletions: .deletions}'

Length of output: 159


🏁 Script executed:

#!/bin/bash
# Get the actual diff for the extensions.sh file
gh pr diff 8897 --repo armbian/build -- lib/functions/general/extensions.sh

Length of output: 36


🏁 Script executed:

#!/bin/bash
# Search for HOOK_POINT_TOTAL_FUNCS usage in the codebase
rg -n "HOOK_POINT_TOTAL_FUNCS" -C5

Length of output: 4750


🏁 Script executed:

#!/bin/bash
# Get the diff for the rockchip config file as well
gh pr diff 8897 --repo armbian/build -- config/sources/families/rockchip-rv1106.conf

Length of output: 36


🏁 Script executed:

#!/bin/bash
# Get the actual diff for the PR
gh pr diff 8897 --repo armbian/build

Length of output: 1566


🏁 Script executed:

#!/bin/bash
# Get the current content of extensions.sh to understand the context
cat lib/functions/general/extensions.sh | sed -n '245,330p'

Length of output: 5659


🏁 Script executed:

#!/bin/bash
# Look for any comments or issues in the PR about HOOK_POINT_TOTAL_FUNCS
gh pr view 8897 --repo armbian/build --comments

Length of output: 34780


@tabrisnet Yes, you're right - the HOOK_POINT_TOTAL_FUNCS issue is indeed gnarly! Let me explain what's happening:

The Problem

@tabrisnet Yes, you're right - the HOOK_POINT_TOTAL_FUNCS issue is indeed gnarly! Let me explain what's happening:

The Problem

Currently, HOOK_POINT_TOTAL_FUNCS is computed before the opt-out filtering happens:

  1. Lines 250-255: The code counts ALL functions for a hook point and sets HOOK_POINT_TOTAL_FUNCS:

    for hook_point_function in ${hook_point_functions}; do
        hook_point_functions_counter=$((hook_point_functions_counter + 1))
    done
    common_function_vars="${common_function_vars} HOOK_POINT_TOTAL_FUNCS=\"${hook_point_functions_counter}\""
  2. Lines 278-290: Your new opt-out logic runs and skips some functions:

    if $opt_out; then
        continue; #skip processing this function
    fi
  3. Line 291: Only non-opted-out functions increment the actual execution counter:

    hook_point_functions_loop_counter=$((hook_point_functions_loop_counter + 1))

The Impact

If you have 5 functions for a hook point and opt out of 2:

  • HOOK_POINT_TOTAL_FUNCS will be 5 (wrong!)
  • Only 3 functions actually execute
  • HOOK_ORDER will be 1, 2, 3 (correct)
  • Extension functions checking if [ "$HOOK_ORDER" == "$HOOK_POINT_TOTAL_FUNCS" ] to detect "am I the last function?" will get confused

The Fix

You need to either:

  1. Move the counter calculation to after the opt-out loop, so it only counts functions that will execute
  2. Decrement the counter each time you skip an opted-out function
  3. Set HOOK_POINT_TOTAL_FUNCS later using hook_point_functions_loop_counter after the loop completes

The cleanest approach is probably #3 - set HOOK_POINT_TOTAL_FUNCS="${hook_point_functions_loop_counter}" after the loop finishes (around line 327), instead of at line 255.

Please Push Your Commit

Go ahead and push your incoming commit, and I'll review it to see if it addresses this issue correctly or if there are any other concerns! 🔍


🧠 Learnings used
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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
lib/functions/general/extensions.sh (2)

17-27: Optional: Declare extension_optout globally for clarity and explicitness.

The extension_optout array is not declared in extension_manager_declare_globals(), relying instead on external config files to set it. While the defensive check at line 236 safely handles an unset array, adding a global declaration would make the contract explicit and improve static analysis support.

Consider adding this to extension_manager_declare_globals() (after line 26):

 	declare -g -a enable_extension_recurse_stack
+	declare -g -a extension_optout=()  # boards/families can add hook names to opt out (e.g., "my_hook__impl_name")

This makes it clear that extension_optout is managed globally and documents its purpose.


234-246: Add documentation explaining the opt-out mechanism.

The opt-out logic is correct but lacks inline documentation. Future maintainers and extension authors should understand what extension_optout is, its expected format, and how to use it.

Consider adding a comment block before line 234 (e.g., above the outer for loop) to document the mechanism:

# If a board/family has defined the global extension_optout array,
# skip any hook implementations matching those names exactly.
# Format: extension_optout=( "hook_point__impl_name1" "hook_point__impl_name2" )
# Example: extension_optout=( "user_config__early_config" ) would skip that hook.

This would be especially useful if linked to the extension documentation mentioned in the PR.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e41a434 and dbd5200.

📒 Files selected for processing (1)
  • lib/functions/general/extensions.sh (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: 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: 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: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.

Applied to files:

  • lib/functions/general/extensions.sh
📚 Learning: 2025-05-21T06:04:48.711Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

Applied to files:

  • lib/functions/general/extensions.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:

  • lib/functions/general/extensions.sh
🔇 Additional comments (1)
lib/functions/general/extensions.sh (1)

235-245: Opt-out filter logic is sound; defensive array check properly in place.

The filtering correctly happens before counting (line 265 sets HOOK_POINT_TOTAL_FUNCS on the filtered list), which resolves the reported concern about miscounting functions. The -v extension_optout check at line 236 defensively guards against unset-array errors under strict mode, addressing the prior ShellCheck warning.

To confirm this resolves the HOOK_POINT_TOTAL_FUNCS issue raised in PR comments: please verify that HOOK_ORDER now correctly reflects the position among actually-executing functions (not total declared), so comparisons like [ "$HOOK_ORDER" == "$HOOK_POINT_TOTAL_FUNCS" ] work as intended for detecting the last hook.

@tabrisnet tabrisnet requested a review from vidplace7 as a code owner November 9, 2025 02:10
@github-actions github-actions bot added size/large PR with 250 lines or more and removed size/small PR with less then 50 lines labels Nov 9, 2025
@tabrisnet
Copy link
Collaborator Author

@coderabbitai re declaring extension_optout globally in general/extensions.sh, is bash a multi-pass compiler? If not, then it seems plausible that since the board config gets loaded early that declaring/initializing it in general/extensions.sh may overwrite the variable contents.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the PR diff to see if there's a declaration being added to extensions.sh
gh pr diff 8897 --repo armbian/build

echo "---SEPARATOR---"

# Search for where board config files are loaded/sourced
rg -n "source.*families.*\.conf" -C3

echo "---SEPARATOR---"

# Check if extensions.sh currently has any declaration of extension_optout
rg -n "extension_optout" lib/functions/general/extensions.sh -C5

Length of output: 24609


🏁 Script executed:

#!/bin/bash
# Look for the general initialization/sourcing order in the main build script
rg -n "source.*extensions\.sh" -C5

echo "---SEPARATOR---"

# Check how board/family configs are sourced
rg -n "sources/families" -C2 | head -50

Length of output: 5688


🏁 Script executed:

#!/bin/bash
# Check the exact order of sourcing by looking at where extension manager is initialized
# and where family configs are sourced

echo "=== Finding where extension manager is initialized ==="
rg -n "initialize_extension_manager" -C3

echo -e "\n=== Finding where family config is sourced ==="
rg -n "source_family_config_and_arch" -C3

echo -e "\n=== Looking at the main compile.sh flow ==="
cat compile.sh | head -100

Length of output: 11212


🏁 Script executed:

#!/bin/bash
# Check if there's any existing declaration of extension_optout in extensions.sh
echo "=== Current content around extension_optout usage in extensions.sh ==="
cat lib/functions/general/extensions.sh | sed -n '230,250p'

echo -e "\n=== Check if extensions.sh declares extension_optout anywhere ==="
rg -n "declare.*extension_optout" lib/functions/general/extensions.sh

Length of output: 1102


@tabrisnet Great question! You're absolutely right to think about execution order. Let me clarify what's happening:

Bash Execution Order

Bash is single-pass (not a multi-pass compiler), so execution order matters. Here's the actual sequence:

  1. Line 677 of lib/library-functions.sh: extensions.sh is sourced
  2. Line 291 of lib/functions/configuration/main-config.sh: source_family_config_and_arch() sources your family config (rockchip-rv1106.conf)
  3. Line 332 of lib/functions/configuration/main-config.sh: initialize_extension_manager() is called (where the opt-out logic runs)

So family configs are sourced before the extension manager initializes. ✅

Current PR - No Declaration in extensions.sh

The good news is your current PR does not declare extension_optout in extensions.sh. It only checks for its existence:

if [[ -v extension_optout && ${#extension_optout[@]} -ne 0 ]]; then

This is the correct approach! 🎯

Bash declare Behavior

If you were to add a declaration in extensions.sh, here's what would happen:

  • WITH initialization: declare -ga extension_optout=() would clear/overwrite any values set in the family config
  • WITHOUT initialization: declare -ga extension_optout would NOT clear existing values (just ensures it's a global array type)
  • No declaration (current approach): Just check with [[ -v extension_optout ]] and let the family config declare it

Recommendation

Keep the current approach - don't declare extension_optout in extensions.sh at all. The family config declares and initializes it, and extensions.sh just checks for its existence. This is clean and avoids any possibility of overwriting.

# In family config (rockchip-rv1106.conf) - DECLARE & INITIALIZE
declare -ga extension_optout=( armbian_kernel_config__enable_docker_support )

# In extensions.sh - JUST CHECK
if [[ -v extension_optout && ${#extension_optout[@]} -ne 0 ]]; then

Your instinct to question this was spot-on! But fortunately, the current design is safe. 👍


🧠 Learnings used
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: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: 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: 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.

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: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.

@tabrisnet
Copy link
Collaborator Author

can you declare -g the same variable twice but only init once?
@coderabbitai

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Test bash declare behavior with multiple declarations

echo "=== Test 1: declare with init, then declare without init ==="
echo "Code: declare -g test1=initial; declare -g test1; echo \$test1"
(
  declare -g test1="initial"
  echo "After first declare -g test1='initial': test1=$test1"
  declare -g test1
  echo "After second declare -g test1 (no init): test1=$test1"
)

echo ""
echo "=== Test 2: declare without init, then declare with init ==="
echo "Code: declare -g test2; declare -g test2=later; echo \$test2"
(
  declare -g test2
  echo "After first declare -g test2 (no init): test2='$test2'"
  declare -g test2="later"
  echo "After second declare -g test2='later': test2=$test2"
)

echo ""
echo "=== Test 3: declare with init, then declare with different init ==="
echo "Code: declare -g test3=first; declare -g test3=second; echo \$test3"
(
  declare -g test3="first"
  echo "After first declare -g test3='first': test3=$test3"
  declare -g test3="second"
  echo "After second declare -g test3='second': test3=$test3"
)

echo ""
echo "=== Test 4: with arrays (like extension_optout) ==="
echo "Code: declare -ga arr=(a b); declare -ga arr; echo \${arr[@]}"
(
  declare -ga arr=(a b)
  echo "After first declare -ga arr=(a b): arr=(${arr[@]})"
  declare -ga arr
  echo "After second declare -ga arr (no init): arr=(${arr[@]})"
)

echo ""
echo "=== Test 5: array - declare without init, then with init ==="
echo "Code: declare -ga arr2; declare -ga arr2=(x y); echo \${arr2[@]}"
(
  declare -ga arr2
  echo "After first declare -ga arr2 (no init): arr2=(${arr2[@]})"
  declare -ga arr2=(x y)
  echo "After second declare -ga arr2=(x y): arr2=(${arr2[@]})"
)

Length of output: 1123


@tabrisnet Yes! You absolutely can, and the behavior is exactly what you'd hope for. Here are the test results:

Key Finding: ✅ Second declare WITHOUT init preserves the value

Test 4 (arrays like extension_optout):

declare -ga arr=(a b)         # First: declare with init
declare -ga arr               # Second: declare WITHOUT init
# Result: arr=(a b) ← VALUES PRESERVED! 🎯

Test 1 (regular variables):

declare -g test1="initial"    # First: declare with init
declare -g test1              # Second: declare WITHOUT init
# Result: test1=initial ← VALUE PRESERVED!

But Watch Out: ❌ Second declare WITH init overwrites

Test 3:

declare -g test3="first"      # First: declare with init
declare -g test3="second"     # Second: declare WITH DIFFERENT init
# Result: test3=second ← VALUE OVERWRITTEN!

What This Means for extension_optout

You could safely do this:

# In family config (rockchip-rv1106.conf) - sourced first
declare -ga extension_optout=(armbian_kernel_config__enable_docker_support)

# In extensions.sh - executed later
declare -ga extension_optout  # ← No init, so values are preserved! ✅

This would work because the second declare without initialization preserves the array contents set by the first one.

My Recommendation

Keep your current approach (no declaration in extensions.sh). Even though the double-declare works, it's:

  • Clearer: Single source of truth for the variable
  • Safer: Less risk of accidental =() being added later
  • Simpler: [[ -v extension_optout ]] is enough to check existence

But now you know that if you ever need to ensure type safety (e.g., guarantee it's an array type), you could add declare -ga extension_optout in extensions.sh without breaking anything. 👍


🧠 Learnings used
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: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Learnt from: 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: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.

Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.

Learnt from: 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: 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: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

…ilies

The goal here is that some boards may want to opt out entirely
[at their own risk of course] of an extension or hook.
Here I pick on the rockchip-rv1106 as an example.
@github-actions github-actions bot added size/small PR with less then 50 lines and removed size/large PR with 250 lines or more labels Nov 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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)
lib/functions/general/extensions.sh (1)

236-250: Opt-out filtering logic is correct.

The implementation correctly filters opted-out functions before they're added to hook_point_functions, ensuring HOOK_POINT_TOTAL_FUNCS (set at line 270) accurately reflects only the functions that will execute. The continue 2 properly skips the outer loop to exclude matched functions.

Minor comment accuracy improvement—since line 131 declares the array, it cannot be "non-existent":

-			if [[ ${#extension_optout[@]} -ne 0 ]]; then
-			# most common case this array is empty/non-existent, so bail out soonest
+			if [[ ${#extension_optout[@]} -ne 0 ]]; then
+			# most common case this array is empty, so bail out soonest
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 421dbb7 and 7c8f2a6.

📒 Files selected for processing (2)
  • config/sources/families/rockchip-rv1106.conf (1 hunks)
  • lib/functions/general/extensions.sh (2 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 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: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: 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/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.
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.
📚 Learning: 2025-05-21T06:04:48.711Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8211
File: extensions/linux-source-package.sh:7-10
Timestamp: 2025-05-21T06:04:48.711Z
Learning: In the Armbian build system's extension framework, function names follow the pattern `hook_point__unique_identifier()`. The part before `__` defines when in the build process the function is executed (the hook point), while the part after `__` is just a unique identifier and doesn't need to match the extension name. Functions with the same hook point are executed in alphanumerical order.

Applied to files:

  • lib/functions/general/extensions.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.

Applied to files:

  • lib/functions/general/extensions.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:

  • lib/functions/general/extensions.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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:29:18.958Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:64-70
Timestamp: 2025-09-14T06:29:18.958Z
Learning: In the Armbian build system, vendor branch configurations in family files are designed to be shared across multiple SoCs within the same family that use the same vendor kernel tree. For example, rk35xx and rockchip-rk3588 families both use identical vendor branch settings (same KERNELSOURCE, KERNELBRANCH, and KERNELPATCHDIR), demonstrating that vendor branches are intentionally generic rather than SoC-specific.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-06-25T03:40:52.109Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-06-25T03:42:09.086Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-14T05:08:11.785Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8754
File: config/boards/bestv-r3300-l.csc:14-16
Timestamp: 2025-10-14T05:08:11.785Z
Learning: In the Armbian build system, BOOTBRANCH_BOARD is a valid framework variable used as a fallback when BOOTBRANCH is unset. The framework checks BOOTBRANCH_BOARD before applying the default bootloader branch value (see config/sources/common.conf). Board configuration files can use BOOTBRANCH_BOARD to specify the bootloader branch.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-07T17:39:32.272Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8586
File: config/boards/nanopi-r76s.conf:15-21
Timestamp: 2025-09-07T17:39:32.272Z
Learning: In the Armbian build system, the variables $BOARD and $SDCARD are always set by the build framework, so guard checks for these variables are unnecessary in board configuration files and hook functions.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-10-26T12:56:29.185Z
Learnt from: rpardini
Repo: armbian/build PR: 8820
File: config/sources/families/include/meson64_common.inc:51-53
Timestamp: 2025-10-26T12:56:29.185Z
Learning: In the Armbian build framework, scripts/config correctly handles kernel configuration option names both with and without the CONFIG_ prefix, so opts_m+=("CONFIG_RTL8822CS") and opts_m+=("RTL8822CS") are both valid and will work correctly.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 Learning: 2025-09-14T11:37:35.089Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.

Applied to files:

  • config/sources/families/rockchip-rv1106.conf
📚 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:

  • config/sources/families/rockchip-rv1106.conf
🧬 Code graph analysis (1)
config/sources/families/rockchip-rv1106.conf (2)
lib/functions/compilation/armbian-kernel.sh (2)
  • armbian_kernel_config__enable_docker_support (357-429)
  • armbian_kernel_config_apply_opts_from_arrays (480-533)
lib/functions/compilation/kernel-config.sh (1)
  • call_extensions_kernel_config (88-121)
🔇 Additional comments (2)
config/sources/families/rockchip-rv1106.conf (1)

34-34: Excellent use of the opt-out mechanism for resource-constrained devices.

Opting out of Docker support for the RV1106 family is the right choice for devices with 64-256MB of memory. The armbian_kernel_config__enable_docker_support extension enables ~70 kernel features (cgroups, namespaces, BTRFS, OverlayFS, etc.) that would consume significant kernel memory and storage on these constrained devices.

lib/functions/general/extensions.sh (1)

131-131: Correct approach: declare without initialization.

The declare -ga extension_optout without initialization is the right choice. It ensures the array exists (as empty) when checking ${#extension_optout[@]} at line 241, while not overwriting values set by family/board configs. The comment "do not init here!" correctly documents this critical behavior.

@tabrisnet tabrisnet added 02 Milestone: First quarter release and removed 11 Milestone: Fourth quarter release labels Nov 9, 2025
@rpardini
Copy link
Member

rpardini commented Nov 9, 2025

Hmm. This seems quite complicated (ignoring a hook function, which needs to be done in multiple spots).

I'd just have rockchip-rv1106 do

unset -f armbian_kernel_config__enable_docker_support # opt-out of stuff by undefining the function

with no other changes to extensions mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First quarter release Framework Framework components Hardware Hardware related like kernel, U-Boot, ... Needs Documentation New feature needs documentation entry Needs review Seeking for review size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

2 participants